-
Notifications
You must be signed in to change notification settings - Fork 31
Add support for key wrapping (v2) #224
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
base: main
Are you sure you want to change the base?
Conversation
After another round of review and consideration, this approach is not convincing. Although it avoids the challenge of designing the key policy handling that comes with an API for handling wrapped-key formats (see #215), it will result in multiple APIs for key wrapping which is not ideal. This PR will remain as Draft for the time being - this API should be considered experimentatal/Beta. We will revisit the API for wrapping and unwrapping keys after v1.3. |
* psa_wrap_key() and psa_unwrap_key() functions * AES-KW and AES-KWP algorithms
500d814
to
33da350
Compare
I think it is time to revive this PR and decide on the design of key wrapping APIs? This simplified wrappign API for encrypting key material without structured output and key metadata, is vaulable for use cases such as encrypted firmware. For example, constrained bootloader runtimes benefit from the elimination of unused parameters and behavioral requirements that a unified, but more complex, wrapping/formatting API would entail. This current proposed API is in use in some implementations of the specification. |
We think this proposal is good, it works for us. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be NIST-KW or some such, not AES-KW. Apart from that I just have a couple of nits.
doc/crypto/api/ops/key-wrapping.rst
Outdated
The AES-KW key-wrapping algorithm. | ||
|
||
.. todo:: | ||
Decide if we should support any 128-bit block-cipher, as described in SP800-38F. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KW is usually associated with AES because it's the only block cipher both in RFC 3394 and in NIST specs. However, nothing in it prevents it from being used with a different block cipher. RFC 5794 declares that ARIA-KW exists. RFC 4051 assumes that CAMELLIA-KW exists.
In Mbed TLS, we currently only allow only AES for KW, but that costs us a few extra bytes of code size just to reject other block ciphers.
So I think this should be considered a block cipher mode that works with any block cipher of a suitable block size, like PSA_ALG_CBC_NONE
, PSA_ALG_CFB
, etc.
The same applies for KWP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the block-cipher mode names are super-short, and no others include a provenance infix. Can we live with something as brief as PSA_ALG_KW
and PSA_ALG_KWP
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from renaming the algorithm, I will rework the description and the citation references to remove the hard link to AES.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed this in a801ebb, definitely warrants a re-review as it required a rewrite of the whole definition (and for KWP as well).
MAC, ``0x03``, See :secref:`mac-encoding` | ||
Cipher, ``0x04``, See :secref:`cipher-encoding` | ||
AEAD, ``0x05``, See :secref:`aead-encoding` | ||
Key wrapping, ``0x0B``, See :secref:`key-wrap-encoding` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: isn't this table meant to be sorted by numerical value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe.
As new algorithm categories have been added to the specification, I have added the sections into Chapter 10 to maintain some kind of logical order, rather than just add them to the end. So XOF has gone between hashes and MACs, key wrapping after AEAD, encapsulation after key agreement, and PAKE at the end.
In contrast, the algorithm identifier categories have been added sequentially.
This table is currently in the same order as the sections that document the API. I agree that is looks a little odd - but if we sort it by category value - should we do the same for the other tables in this appendix, which would mix up some of the logical ordering within those tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is a logical ordering algorithm categories in the operation documentation (chapter 10) the best approach (rather than appending new categories to the end)?
- Should the table here use that same ordering, or use the numerical category ordering?
- If these are different, should the encoding sub-sections (B.1.x) be ordered as in chapter 10, or as in the Algorithm category table here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK to me.
While I would never use AES-KW, and while I doubt anyone will use it with a different algorithm - the construction works just as well, or indeed badly, with any other algorithm. Which makes this a sensible approach.
This PR replaces #215, simplifying the API to only support key wrapping algorithms, and not wrapped-key formats. Wrapped-key formats will be considered later, along with import and export of formatted key data (see also #50, #149 and #207 for more discussion).
This added the
psa_wrap_key()
andpsa_unwrap_key()
functions, and support for AES-KW and AES-KWP algorithms.Open issues:
NIST 800-38F describes a generalised AES-KW and AES-KWP to support any 128-bit 'approved block cipher', which exactly matches RFC 3394 and RFC 5649 definitions for the AES block cipher. Is it valuable to define this as the more general algorithm identifier, compatible with any 128-bit block cipher key, or just keep the AES-specific name and AES-only compatibility?
I have allocated a new algorithm category, as key-wrapping algorithms tend to be specialized authenticated encryption. However, AES-SIV can be used as a general AEAD algorithm, as well as a key-wrapping algorithm. Would that be problematic?
I have recycled the 'S' bit in the algorithm identifier to flag whether the algorithm has alignment constraints on the input data (AES-KW does, AES-KWP does not) - similar to block-aligned sizes for some block-mode ciphers. Is this at all useful, or should we just make this bit 0 for key-wrap algorithms?
Partly fixes #50