Skip to content

Conversation

@gilles-peskine-arm
Copy link
Collaborator

@gilles-peskine-arm gilles-peskine-arm commented Oct 1, 2019

Secure element interface changes:

  • A driver without a p_validate_slot_number method no longer allows registering an existing key with mbedtls_psa_register_se_key.
  • The p_validate_slot_number method now takes an extra read-write parameter persistent_data, like p_allocate.

New features and bug fixes:

  • Call the p_init driver method during psa_crypto_init.
  • Fix loading of the persistent data during init.
  • Add some minimal testing (good cases only) for saved persistent data.

When registering a key in a secure element, go through the transaction
mechanism. This makes the code simpler, at the expense of a few extra
storage operations. Given that registering a key is typically very
rare over the lifetime of a device, this is an acceptable loss.

Drivers must now have a p_validate_slot_number method, otherwise
registering a key is not possible. This reduces the risk that due to a
mistake during the integration of a device, an application might claim
a slot in a way that is not supported by the driver.
Add a parameter to the p_validate_slot_number method to allow the
driver to modify the persistent data.

With the current structure of the core, the persistent data is already
updated. All it took was adding a way to modify it.
@gilles-peskine-arm gilles-peskine-arm added enhancement New feature or request needs: review The pull request is ready for review. This generally means that it has no known issues. labels Oct 1, 2019
Patater
Patater previously approved these changes Oct 1, 2019
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have some more context or justification described in the commit messages, but otherwise OK

The persistent data was not loaded correctly (the code was loading 0
bytes instead of the correct size).
Add invasive checks that peek at the stored persistent data after some
successful import, generation or destruction operations and after
reinitialization to ensure that the persistent data in storage has the
expected content.
@gilles-peskine-arm gilles-peskine-arm changed the title SE driver: let p_validate_slot_number change the persistent data SE driver: make persistent data work Oct 1, 2019
Copy link
Contributor

@Patater Patater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gilles-peskine-arm
Copy link
Collaborator Author

CI passed on -head. The failures on -merge are due to a recent CI instability. The PR is from a recent branch and we'd have been happy to merge with a CI job that had run on exactly the same content as the -head job. So CI is ok to merge.

@gilles-peskine-arm gilles-peskine-arm removed the needs: review The pull request is ready for review. This generally means that it has no known issues. label Oct 9, 2019
@gilles-peskine-arm gilles-peskine-arm merged commit 3602938 into ARMmbed:development Oct 9, 2019
gilles-peskine-arm added a commit to gilles-peskine-arm/mbed-crypto that referenced this pull request Nov 15, 2019
* ARMmbed#272: Insert doxygen comments on old algorithms so they appear in PSA documentation
* ARMmbed#285: SE driver: make persistent data work
* ARMmbed#279: Include IANA reference in the definition of ECC curves and DH groups
* ARMmbed#287: DRBG documentation improvements
* ARMmbed#297: Fix int overflow in mbedtls_asn1_get_int (Credit to OSS-Fuzz)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants