Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make key ids global and define their range #104

Merged

Conversation

gilles-peskine-arm
Copy link
Collaborator

Make key ids global: you no longer need to specify a key's lifetime when you open it, the key id is enough. This is the “global model” of key ids, as opposed to the former “drive letter” model. Internal ref: https://github.com/ARMmbed/psa-crypto/issues/138

Carve the range of possible key ids into 3 sub-ranges: one for the application, one for the implementation and one reserved for future use. Internal ref: https://github.com/ARMmbed/psa-crypto/issues/136

Record what key ids have been used in a test case and purge them. The
cleanup code no longer requires the key identifiers used in the tests
to be in a certain small range.
Change the scope of key identifiers to be global, rather than
per lifetime. As a result, you now need to specify the lifetime of a
key only when creating it.
Define a range of key identifiers for use by the application
(0..2^30-1), a range for use by implementations (2^30..2^31), and a
range that is reserved for future use (2^31..2^32-1).
Only allow creating keys in the application (user) range. Allow
opening keys in the implementation (vendor) range as well.

Compared with what the implementation allowed, which was undocumented:
0 is now allowed; values from 0x40000000 to 0xfffeffff are now
forbidden.
@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. api-spec Issue or PR about the PSA specifications labels May 6, 2019
Copy link
Contributor

@athoelke athoelke left a comment

Choose a reason for hiding this comment

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

The decision to permit key id zero is not discussed anywhere - not sure if this is deliberately to leave that decision to the application?

include/psa/crypto_types.h Outdated Show resolved Hide resolved
include/psa/crypto_types.h Outdated Show resolved Hide resolved
include/psa/crypto_values.h Outdated Show resolved Hide resolved
@gilles-peskine-arm gilles-peskine-arm added this to the api-1.0-beta-3 milestone May 15, 2019
@gilles-peskine-arm gilles-peskine-arm added needs: work The pull request needs rework before it can be merged. and removed needs: review The pull request is ready for review. This generally means that it has no known issues. labels May 15, 2019
In keeping with other integral types, declare 0 to be an invalid key
identifier.

Documented, implemented and tested.
@gilles-peskine-arm gilles-peskine-arm added needs: review The pull request is ready for review. This generally means that it has no known issues. and removed needs: work The pull request needs rework before it can be merged. labels May 15, 2019
@gilles-peskine-arm
Copy link
Collaborator Author

@athoelke @Patater The PR is now ready for re-review. I haven't addressed all your comments, but for some of them I'm waiting for clarifications since I'm not sure whether you're requesting a change.

@gilles-peskine-arm gilles-peskine-arm added the needs: ci Needs a passing full CI run label May 15, 2019
Patater
Patater previously approved these changes May 16, 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

@Patater Patater removed the needs: ci Needs a passing full CI run label May 16, 2019
@Patater
Copy link
Contributor

Patater commented May 16, 2019

CI failure is ABI job (known to fail until Mbed-TLS/mbedtls#2636 lands in the development branch) and USE_PSA_CRYPTO, which doesn't work on the API branch.

@Patater Patater merged commit 99e8d26 into ARMmbed:psa-api-1.0-beta May 16, 2019
@Patater Patater removed the needs: review The pull request is ready for review. This generally means that it has no known issues. label May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-spec Issue or PR about the PSA specifications enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants