Merge psa api branch into development #231
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a few test cases to ensure that alg=0 in policy does not allow using the key for an operation. Add a test case to ensure that ANY_HASH does not have a wildcard meaning for HMAC.
Add infrastructure for internal, external and dual-use flags, with a compile-time check (if static_assert is available) to ensure that the same numerical value doesn't get declared for two different purposes in crypto_struct.h (external or dual-use) and psa_crypto_core.h (internal).
Add a slot_number field to psa_key_attributes_t and getter/setter functions. Since slot numbers can have the value 0, indicate the presence of the field via a separate flag. In psa_get_key_attributes(), report the slot number if the key is in a secure element. When creating a key, for now, applications cannot choose a slot number. A subsequent commit will add this capability in the secure element HAL.
Test the behavior of the getter/setter functions. Test that psa_get_key_slot_number() reports a slot number for a key in a secure element, and doesn't report a slot number for a key that is not in a secure element. Test that psa_get_key_slot_number() reports the correct slot number for a key in a secure element.
check-names.sh rejects MBEDTLS_XXX identifiers that are not defined in a public header.
This didn't break anything now, but would have broken things once we start to add internal flags.
This function no longer modifies anything, so it doesn't actually allocate the slot. Now, it just returns the empty key slot, and it's up to the caller to cause the slot to be in use (or not).
Pave the way for allowing the application to choose the slot number in a secure element, rather than always letting the driver choose.
Allow the application to choose the slot number in a secure element, rather than always letting the driver choose. With this commit, any application may request any slot. In an implementation with isolation, it's up to the service to filter key creation requests and apply policies to limit which applications can request which slot.
…set_key_slot_number Add slot number attribute
…y_bad_algorithm Add some negative tests for policy checks
…choose_key_slot_number Let applications create a key in a specific secure element slot
Let psa_start_key_creation know what type of key creation this is. This will be used at least for key registration in a secure element, which is a peculiar kind of creation since it uses existing key material.
Pass the key creation method (import/generate/derive/copy) to the driver methods to allocate or validate a slot number. This allows drivers to enforce policies such as "this key slot can only be used for keys generated inside the secure element".
Register an existing key in a secure element. Minimal implementation that doesn't call any driver method and just lets the application declare whatever it wants.
When registering a key in a secure element, if the driver has a p_validate_slot_number method, call it.
…register_key Secure element key registration
The methods to import and generate a key in a secure element drivers were written for an earlier version of the application-side interface. Now that there is a psa_key_attributes_t structure that combines all key metadata including its lifetime (location), type, size, policy and extra type-specific data (domain parameters), pass that to drivers instead of separate arguments for each piece of metadata. This makes the interface less cluttered. Update parameter names and descriptions to follow general conventions. Document the public-key output on key generation more precisely. Explain that it is optional in a driver, and when a driver would implement it. Declare that it is optional in the core, too (which means that a crypto core might not support drivers for secure elements that do need this feature). Update the implementation and the tests accordingly.
Factor common code of ram_import and ram_fake_generate into a common auxiliary function. Reject key types that aren't supported by this test code. Report the bit size correctly for EC key pairs.
Add a flow where the key is imported or fake-generated in the secure element, then call psa_export_public_key and do the software verification with the public key.
…generate_key Add hooks for generate and sign in a secure element
When a key slot is wiped, a copy of the key material may remain in operations. This is undesirable, but does not violate the safety of the code. Tracked in https://github.com/ARMmbed/mbed-crypto/issues/86
Adopt a simple method for tracking whether there was a failure: each fallible operation sets overall_status, unless overall_status is already non-successful. Thus in case of multiple failures, the function always reports whatever failed first. This may not always be the right thing, but it's simple. This revealed a bug whereby if the only failure was the call to psa_destroy_se_key(), i.e. if the driver reported a failure or if the driver lacked support for destroying keys, psa_destroy_key() would ignore that failure. For a key in a secure element, if creating a transaction file fails, don't touch storage, but close the key in memory. This may not be right, but it's no wronger than it was before. Tracked in ARMmbed#215
Drivers that allow destroying a key must have a destroy method. This test bug was previously not caught because of an implementation bug that lost the error triggered by the missing destroy method.
…comments-20190813 SE keys: fix psa_destroy_key; add Github issue numbers for missing code
|
Looks like all merge conflicts were resolved by taking the API branch, which is the same as how I resolved all the conflicts myself. LGTM. |
Patater
approved these changes
Aug 21, 2019
|
CI failure is API test. We changed attribute flags from |
|
CI merge testing all passed except for the aforementioned API test. We don't need to wait for the HEAD testing result. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were a few trivial conflicts, all relating to making the TODO comments consistent. Otherwise it was a clean merge.