gh-Mbed-TLS/TF-PSA-Crypto
Watch
1
0
Fork
You've already forked TF-PSA-Crypto
 0
Code Issues 167 Pull requests 92 Releases 5 Activity

feat(drivers): Builtin CMAC driver use PSA cipher interface #249

Open
Harshal5 wants to merge 1 commit from Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface into development
pull from: Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface
merge into: gh-Mbed-TLS:development
Conversation 2 Commits 1 Files changed 3 +323 -32
Harshal5 commented 2025-04-07 21:50:07 -05:00 (Migrated from github.com)

Description

Current issue:

As far as I can understand for the psa_mac_ APIs to internally use an external cipher driver we would need to write a custom complete MAC driver. Writing a custom MAC driver would involve almost rewriting the mbedtls_psa_mac_ APIs.

Solution:

Instead we could update the builtin driver's mbedtls_psa_mac_ API usage of mbedtls_cipher_ with psa_cipher_ APIs. This would allow the usage of an external cipher driver with psa_mac_ API calls without rewriting the CMAC code.

Related

Fixes https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/211

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

  • changelog provided | not required because:
  • framework PR provided: https://github.com/Mbed-TLS/mbedtls-framework/pull/155 Not needed
  • mbedtls development PR provided Mbed-TLS/mbedtls# | not required because:
  • mbedtls 3.6 PR not required
  • mbedtls 2.28 PR not required
  • tests not required because: Current tests follow the updated code workflow
## Description Current issue: As far as I can understand for the `psa_mac_` APIs to internally use an external cipher driver we would need to write a custom complete MAC driver. Writing a custom MAC driver would involve almost rewriting the `mbedtls_psa_mac_` APIs. Solution: Instead we could update the builtin driver's `mbedtls_psa_mac_` API usage of `mbedtls_cipher_` with `psa_cipher_` APIs. This would allow the usage of an external cipher driver with `psa_mac_` API calls without rewriting the CMAC code. ## Related Fixes https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/211 ## PR checklist Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line. If the provided content is part of the present PR remove the # symbol. - [ ] **changelog** provided | not required because: - [x] **framework PR** provided: ~~https://github.com/Mbed-TLS/mbedtls-framework/pull/155~~ **Not needed** - [x] **mbedtls development PR** provided Mbed-TLS/mbedtls# | not required because: - [x] **mbedtls 3.6 PR** not required - [x] **mbedtls 2.28 PR** not required - [x] **tests** not required because: Current tests follow the updated code workflow
Harshal5 commented 2025-07-09 00:09:14 -05:00 (Migrated from github.com)

@gilles-peskine-arm I have now updated the code workflow now, could you PTAL once?

The major change in the current version of the workflow as compared to the earlier one is that instead of adding the PSA_KEY_USAGE_ENCRYPT attribute to the CMAC key, I have now created a new instance/copy of CMAC key with type AES and the encrypt attribute for allowing us to pass the key to the cipher APIs. This key is created in the _mac_start() API and the key id is stored in the operation context, finally is destroyed when _mac_abort() API is called for the operation context (_mac_finish also calls _mac_abort internally).

Therefore, now we do not need the "hack" of adding the encrypt attribute to the CMAC key thus solving the incorrect key policy issue. Also, even if the CMAC API layer holds a lock on the CMAC key, the cipher API layer can use the key for performing encrypt operations and can hold a lock as well.

Here is the current workflow for an PSA CMAC operation that can internally use an external cipher driver as well:
psa_mac_ (PSA API) -> psa_mac_driver_wrapper_ (driver dispatch) -> mbedtls_psa_mac_ (builtin driver) -> psa_cipher_ (PSA API) -> psa_cipher_driver_wrapper_ (driver dispatch) -> mbedtls_psa_cipher_ (builtin driver).

Having this change would also help disabling MBEDTLS_AES_C when using a custom AES driver layer thus reducing the code size significantly.

Let me know what do you think about this approach!

@gilles-peskine-arm I have now updated the code workflow now, could you PTAL once? The major change in the current version of the workflow as compared to the earlier one is that instead of adding the `PSA_KEY_USAGE_ENCRYPT` attribute to the CMAC key, I have now created a new instance/copy of CMAC key with type AES and the encrypt attribute for allowing us to pass the key to the cipher APIs. This key is created in the `_mac_start()` API and the key id is stored in the operation context, finally is destroyed when `_mac_abort()` API is called for the operation context (`_mac_finish` also calls `_mac_abort` internally). Therefore, now we do not need the "_hack_" of adding the encrypt attribute to the CMAC key thus solving the incorrect key policy issue. Also, even if the CMAC API layer holds a lock on the CMAC key, the cipher API layer can use the key for performing encrypt operations and can hold a lock as well. Here is the current workflow for an PSA CMAC operation that can internally use an external cipher driver as well: `psa_mac_` (PSA API) -> `psa_mac_driver_wrapper_` (driver dispatch) -> `mbedtls_psa_mac_` (builtin driver) -> `psa_cipher_` (PSA API) -> `psa_cipher_driver_wrapper_` (driver dispatch) -> `mbedtls_psa_cipher_` (builtin driver). Having this change would also help disabling `MBEDTLS_AES_C` when using a custom AES driver layer thus reducing the code size significantly. Let me know what do you think about this approach!
Harshal5 commented 2025-07-23 04:09:43 -05:00 (Migrated from github.com)

@gilles-peskine-arm @yanesca Could you PTAL?

@gilles-peskine-arm @yanesca Could you PTAL?
This pull request has changes conflicting with the target branch.
  • core/tf_psa_crypto_check_config.h
  • drivers/builtin/src/psa_crypto_mac.c
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface:Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git switch 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'development'
git merge --no-ff 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git switch 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git rebase 'development'
git switch 'development'
git merge --ff-only 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git switch 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git rebase 'development'
git switch 'development'
git merge --no-ff 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git switch 'development'
git merge --squash 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git switch 'development'
git merge --ff-only 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git switch 'development'
git merge 'Harshal5/feat/builtin_cmac_driver_use_psa_crypto_interface'
git push origin 'development'
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
api-break
This issue/PR breaks the API and must wait for a new major version

  
approved
Design and code approved - may be waiting for CI or backports

  
bug
Something isn't working

  
component-docs
Docs / website issues filed here for tracking

  
component-platform
Portability layer and build scripts

  
component-test
Test framework and CI scripts

  
dependencies
Pull requests that update a dependency file

  
deprecation
needs deprecation announcement

  
DO-NOT-MERGE

   
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
This issue is not being actively worked on, but PRs welcome.

  
needs-adoption
stalled PR that someone should pick up and complete

  
needs-backports
Backports are missing or are pending review and approval.

  
needs-ci
Needs to pass CI tests

  
needs-design-approval
Needs design discussion / approval

  
needs-info
An issue or PR which needs further info from the reporter / author

  
needs-preceding-pr
Requires another PR to be merged first

  
needs-review
Every commit must be reviewed by at least two team members

  
needs-reviewer
This PR needs someone to pick it up for review

  
needs-work

   
priority-high
High priority - will be reviewed soon

  
priority-low
Low priority - this may not receive review soon

  
priority-medium
Medium priority - this can be reviewed as time permits

  
priority-scheduled
This PR is big - it will require time to be scheduled for review

  
priority-very-high
Highest priority - prioritise this over other review work

  
resolved-after-1.1
The issue is resolved in development, but still present in 1.1 LTS.

  
single-reviewer
This PR qualifies for having only one reviewer

  
size-l
Estimated task size: large (2w+)

  
size-m
Estimated task size: medium (~1w)

  
size-optimisation

   
size-s
Estimated task size: small (~2d)

  
size-xs
Estimated task size: extra small (a few hours at most)

No labels
api-break
approved
bug
component-docs
component-platform
component-test
dependencies
deprecation
DO-NOT-MERGE
enhancement
good first issue
help wanted
needs-adoption
needs-backports
needs-ci
needs-design-approval
needs-info
needs-preceding-pr
needs-review
needs-reviewer
needs-work
priority-high
priority-low
priority-medium
priority-scheduled
priority-very-high
resolved-after-1.1
single-reviewer
size-l
size-m
size-optimisation
size-s
size-xs
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/TF-PSA-Crypto!249
No description provided.