Key wrapping API proposal #364
No reviewers
Labels
No labels
api-spec
bug
CLA requested
CLA valid
compliance
DO NOT MERGE
duplicate
enhancement
feature branch
good first issue
HEADER FORMATTING ISSUE
help wanted
Information
invalid
Jira status: AWAITING REVIEW
Jira status: CLOSED
Jira status: DONE
Jira status: IN PROGRESS
Jira status: IN REVIEW
Jira status: OPEN
Jira status: REOPENED
Jira status: RESOLVED
mirrored
mirror internally
needs: backports
needs: ci
needs: design review
needs: preceding PR
needs: review
needs: work
notabug
open for community contribution
Product Backlog
question
ready for merge
security
stale
type: bug
type: enhancement
type: question
web pages
wontfix
writing
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-ARMmbed/mbed-crypto!364
Loading…
Reference in a new issue
No description provided.
Delete branch "gilles-peskine-arm/psa-wrap-api"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This is a first draft of the key wrapping API for PSA Cryptography 1.x.
This pull request only provides function prototypes and associated macro definitions. Tests and implementations will come later.
I implemented the proposal discussed in private. I'm not attached to function/macro names if you have better suggestions. The API comprises:
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,You have used both source key and \p data which I believe both refer to the key to unwrap.
I would prefer consistency as it is quite hard to understand.
I believe that from this description the caller must set attributes -> location.
So the caller needs to call psa_get_wrapped_key_attributes
Is there a reason why they should not be able to leave it unset and use the one in the wrapped key?
And potentiall not offer psa_get_wrapped_key_attributes?
Is it worth adding a line checking if the copy permission is set.
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,fixed
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,I'd prefer not to have
psa_get_wrapped_key_attributes, but I added it because I couldn't find a better way.“Leave unset” works for the key type and the key size because for those 0 is not a valid value, so it's interpreted as unset. But for lifetimes, 0 is a valid value, indicating a volatile key: keys are volatile by default, you only need to specify a lifetime explicitly if you want the key to live longer. If a wrapped key is non-copyable and persistent, attempting to unwrap it as volatile should fail.
Likewise, for the policy, 0 means “you can't do anything”. That's not the same thing as “use whatever is in the wrapped data”.
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Why would the calling code check the copy permission? I'd expect typical application code to either expect a copyable key and specify a location here, or not expect a copyable key and not specify the location. Neither case calls for checking the copy permission in the application code.
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Ok - good answer.
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Thanks
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Yes you are probably right for IOT.
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Good use cases. How about one or moe of these options as alternatives:
attributes == NULL- this indicates that the key should be unwrapped exactly as it was wrapped. No override attributes.ANY- i.e. "as wrapped"@ -1609,0 +1666,4 @@** A wrapped form does not preserve the key metadata in general, so it is up* to the application to ensure that the key is unwrapped with the correct* type and policy.Is there any particular significance to the value chosen? - why MIN_UINT32 + 0x62?
Do we also need a usage flag for WRAP and UNWRAP, as well as the defined flags for a key being exporting in wrapped form? Keys used as wrapping keys in
psa_wrap_key_*()orpsa_unwrap_key_*()would then require the applicable usage flag.A few corrections and comments
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,There is no metadata in
datafor this API.@ -856,0 +1086,4 @@* by \p handle.* \retval #PSA_ERROR_BUFFER_TOO_SMALL* The size of the \p data buffer is too small. You can determine a* sufficient buffer size by callingcopy-pasta: presume
PSA_KEY_USAGE_EXPORT_WRAPPED@ -1082,6 +1094,24 @@PSA_ALG_AEAD_WITH_TAG_LENGTH(ref, 0) ? \This does not satisfy
PSA_ALG_IS_KEY_WRAP()- the category should be0x0e000000?This does not satisfy
PSA_ALG_IS_KEY_WRAP()- the category should be0x0e000000?@ -1609,0 +1666,4 @@** A wrapped form does not preserve the key metadata in general, so it is up* to the application to ensure that the key is unwrapped with the correct* type and policy.Not enamoured with the name, as we haven't used "bound" anywhere else in the API or documentation. Is
PSA_KEY_ID_WRAP_BUILT_INany better, or worse?If we used
PSA_KEY_ID_WRAP_BOUNDthen I would be inclined to make thisPSA_KEY_USAGE_EXPORT_BOUNDor perhapsPSA_KEY_USAGE_WRAP_BOUND.BACKUPis quite cute, but then we should use 'backup' in the wrap-with-policy API name and the built-in wrap-with-policy binding key to strengthen the association of these API elements.@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Doesn't help if you want to use the wrapped policy but specify a different lifetime or id, which I think is a very common use case.
That does in fact work for the usage flags, but not for the algorithm. It could be added as a feature for the algorithm.
@ -1609,0 +1666,4 @@** A wrapped form does not preserve the key metadata in general, so it is up* to the application to ensure that the key is unwrapped with the correct* type and policy.0x80000000 is the start of the range reserved for ids defined by PSA.
0x80000000 + 'b'(for “bound”) because why not.I'm very open to naming suggestions. Once a few people have made theirs I'll do a pass of renaming stuff with an eye to consistency and the other eye to comprehensibility.
@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,The second bullet implies that we define (within the new structured framework) a special lifetime value, e.g.
This value is only valid in attributes passed to
psa_unwrap_key_with_policy()to indicate that the key should be unwrapped into the same lifetime that it was wrapped from.You could also support all of the above mechanisms:
attributes = NULL. (Volatile keys might have a different key handle/id after unwrapping)PSA_KEY_USAGE_UNSPECIFIED- all bits 1, not all known bits one: only valid for unwrapping/copying), and set the lifetime/id for the desired locationPSA_KEY_LIFETIME_UNSPECIFIED(only valid for unwrap).For (1), it would be possible to just use
UNSPECIFIEDfor both the usage flags and the lifetime, but I suspect that the compactattributes = NULLform might be a common usage?@ -852,7 +854,320 @@ psa_status_t psa_export_public_key(psa_key_handle_t handle,size_t data_size,Instead of
*_UNSPECIFIED, other terms could be*_AS_SOURCEor*_ORIGINAL.@MarcusJGStreets @athoelke I pushed an update with a rewritten history that should resolve all your comments and cover what we discussed orally today, other than the naming. The original version is in https://github.com/gilles-peskine-arm/mbed-crypto/tree/psa-wrap-api-1
psa_get_wrapped_key_attributes, and instead made a new commit introducingpsa_unwrap_key_to_alternate_lifetimeand removing theattributesparameter topsa_unwrap_key_with_policy.I'll subsequently make another update to fix the terminology issues (which includes finding a decent name for
psa_unwrap_key_to_alternate_lifetime; I couldn't resolve myself topsa_unwrap_key_with_policy_to_alternate_lifetime). I'm more and more warming up to backup/restore for wrap-with-policy. It isn't the only use case for this function (there's also sharing, when another entity has the unwrapping key), but it's a common one. I think anyone would understand backup/restore, even if it isn't what crypto architects would use, unlike “binding”, which even crypto architects don't fully agree on.Still some oopses in the usage flag association with the APIs.
@ -855,0 +876,4 @@* \param handle Handle to the key to export in wrapped form.* \param[out] data Buffer where the wrapped key data is to be written.* \param data_size Size of the \p data buffer in bytes.* \param[out] data_length On success, the number of bytesNeed to specify which key needs this policy?
@ -855,0 +877,4 @@* \param[out] data Buffer where the wrapped key data is to be written.* \param data_size Size of the \p data buffer in bytes.* \param[out] data_length On success, the number of bytes* that make up the wrapped key data.#PSA_KEY_USAGE_BACKUP for this API
@ -856,0 +1086,4 @@* by \p handle.* \retval #PSA_ERROR_BUFFER_TOO_SMALL* The size of the \p data buffer is too small. You can determine a* sufficient buffer size by callingThis issue is still present.
Hi @gilles-peskine-arm - thanks for the update.
I'm quite happy with the idea of backup/restore terminology. Would you drop the
_materialfrom the other wrapping functions as a result?Some ideas for the tricky-to-name API:
psa_restore_key_as_volatile()psa_restore_key_elsewhere():-)The usage flag naming gets interesting now that we have "can be wrapped" and "can use to wrap" policy flags. It is clear there are two categories of flags:
I wonder now if we should have named these categories of flag separately to make it easier to distinguish can-wrap-with from can-be-wrapped.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.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.