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

runtime docs: Remove storage from cryptographic mailbox #1795

Open
wants to merge 3 commits into
base: main-2.x
Choose a base branch
from

Conversation

swenson
Copy link
Contributor

@swenson swenson commented Nov 15, 2024

Mostly.

We need to keep a limited amount of storage per AES-256-GCM key to track usage, since these keys can only be used a certain number of times due to their IV problems. We can keep track of this storage (and remove things from it) using the CM_STATUS, etc., commands we had previously, though we will have to also keep track that we don't allow deleted CMKs to be used again.

We also have some potential issues across resets, since this data is not persistent.

For HMAC, we don't specify a key usage on the output MAC, since it has to be imported back into Caliptra to be used as a key, where its usage will have to be specified.

We also fix a few other minor issues with the mailbox documentation:

Fixes #1753
Fixes #1754
Fixes #1755
Fixes #1756

runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Show resolved Hide resolved
runtime/README.md Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
@swenson
Copy link
Contributor Author

swenson commented Nov 18, 2024

(Discussion offline: we'll change AES to use the init / update / final flow, so that we can remove CMK from the update / final ones, which reduces some of the confusion around that I think.)

Mostly.

We need to keep a limited amount of storage per AES-256-GCM key to track
usage, since these keys can only be used a certain number of times due
to their IV problems.

For HMAC, we don't specify a key usage on the output MAC, since it has
to be imported back into Caliptra to be used as a key, where its usage
will have to be specified.

We also fix a few other minor issues with the mailbox documentation:

Fixes #1753
Fixes #1754
Fixes #1755
Fixes #1756
@swenson swenson force-pushed the docs/crypto-mailbox-fixes branch from 0ef0012 to c6b1916 Compare November 20, 2024 18:28
runtime/README.md Show resolved Hide resolved
| block offset | 24 | Starting block number |
| size | 24 | Size (in bytes) of the data stored. |
| | | Does not have to be a multiple of 16 bytes. |
The key used to encrypt the CMKs is randomized on reset, which means that CMKS cannot be used between resets.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/CMKS/CMKs/

| size | 24 | Size (in bytes) of the data stored. |
| | | Does not have to be a multiple of 16 bytes. |
The key used to encrypt the CMKs is randomized on reset, which means that CMKS cannot be used between resets.
The iv is a randomized 1-up counter that is incremented for every key created.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: IV

| ---------- | ----------- |
| 0 | Reserved |
| 1 | HMAC |
| 2 | HKDF |
Copy link
Contributor

Choose a reason for hiding this comment

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

"HMAC-SHA2-384", "HKDF-SHA2-384"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might support multiple sizes for each, though, and I don't know if it is worth having values for each?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, just seems a bit odd that the AES entry is fully-described, whereas HMAC/HKDF are not. Seems like they should be consistent. So, maybe for consistency with the HMAC commands below, we just have this table include an "AES" entry, and pass the AES algorithm to CM_AES_GCM_[ENCRYPT/DECRYPT]_INIT (while taking care to ensure that one can't create a non-block-sized AES key).


The IV is the concatenation of `block offset || size`.
This is necessary for AES-256-GCM in particular to ensure that keys are only used a certain number of times, as per [NIST SP 800-38D, Section 8.3](https://doi.org/10.6028/NIST.SP.800-38D).
Only AES-256-GCM keys need to be tracked in this table.
Copy link
Contributor

@bluegate010 bluegate010 Nov 20, 2024

Choose a reason for hiding this comment

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

s/need to be tracked/are tracked/? (Though I'm ok if we just track everything for consistency.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add some language to clarify that we might track others, but it is not guaranteed for now. It will depend on the implementation I think.

| context size | u32 | |
| context | u8[...] | |
| iv | u8[12] | |
| cipertext size | u32 | MAY be 0 |
Copy link
Contributor

Choose a reason for hiding this comment

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

MAY be 0

We would expect this to always equal the plaintext size from the input args, right? Ditto for other ciphertext/plaintext size return args below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I was probably thinking of the SHA cases, where you have to buffer a whole block, but this should always be equal to the plaintext size. I'll adjust this language.

| -------------- | -------- | ------------------------------------ |
| chksum | u32 | |
| fips_status | u32 | FIPS approved or an error |
| tag verified | u32 | 1 if tags matched, 0 if they did not |
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we should just fail the mailbox command if the tag mis-matched? Or with the tag arg are you anticipating a debugging use-case?

Risk is that someone could forget to check the tag verified field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am anticipating a debug use case. I've seen cases before where something was wrong with the AAD or the tag computation but it is useful to know what the hardware / firmware thought the tag should be.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Suggest putting language up top noting that callers MUST check the "tag verified" output argument to verify integrity.

runtime/README.md Show resolved Hide resolved
runtime/README.md Show resolved Hide resolved
| ---------- | -------- | --------------------------------------- |
| chksum | u32 | |
| key usage | u32 | Tag to specify how the data can be used |
| input size | u32 | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the input size must agree with the key usage?

| context size | u32 | |
| context | u8[...] | |
| cipertext size | u32 | MUST be equal to ciphertext size |
| ciphertext | u8[...] | |
Copy link
Contributor

Choose a reason for hiding this comment

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

The "MUST be equal to ciphertext size" note should also for consistency apply to the context as well as the ciphertext.

That being said, I think we could try and clean this up a bit. Something like:

| context_size   | u32                |
| context        | u8[context_size]   |
| cipertext_size | u32                |
| ciphertext     | u8[cipertext_size] |

runtime/README.md Show resolved Hide resolved
| ---------- | ----------- |
| 0 | Reserved |
| 1 | HMAC |
| 2 | HKDF |
Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, just seems a bit odd that the AES entry is fully-described, whereas HMAC/HKDF are not. Seems like they should be consistent. So, maybe for consistency with the HMAC commands below, we just have this table include an "AES" entry, and pass the AES algorithm to CM_AES_GCM_[ENCRYPT/DECRYPT]_INIT (while taking care to ensure that one can't create a non-block-sized AES key).

@@ -1120,13 +1128,15 @@ The sequence to use these are:

For each command, the context from the previous command's output must be passed as an input.

The CMK must have been created for HMAC / HKDF usage.
Copy link
Contributor

Choose a reason for hiding this comment

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

So we will allow a CMK whose usage tag says HKDF (and not HMAC) to be passed to CM_HMAC commands? Seems like we should either disallow HKDF-tagged keys here, or just remove the HKDF entry in the key tag list (and maybe rename HMAC to HMAC_HKDF). Similar comment for the language under CM_HKDF_EXTRACT.

@@ -1206,6 +1216,8 @@ Command Code: `0x434D_4846` ("CMHF")

Implements HKDF-Extract as specified in [RFC 5869](https://www.rfc-editor.org/rfc/rfc5869.html).

The CMK must have been created for HMAC / HKDF usage.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's document which usage tag the resulting CMK is given.

| | | with HKDF-Expand |

### CM_HKDF_EXPAND

Implements HKDF-Expand as specified in [RFC 5869](https://www.rfc-editor.org/rfc/rfc5869.html).

The CMK must have been created for HMAC / HKDF usage.

The output length will be automatically chosen to match the key usage.
Copy link
Contributor

Choose a reason for hiding this comment

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

The key usage tag enum currently has three entries: HMAC, HKDF, and AES-256-GCM. If the caller passes AES-256-GCM, there's no ambiguity and Caliptra can pull out the correct amount of keys.

However, if the caller passes HMAC or HKDF, the key size is ambiguous.

Since the key size is not fully described by the key usage tag, it sounds like we can't actually just make the output size automatic.

However, it would be nice to prevent callers from doing silly things like creating a 10-byte HMAC key, or a non-block-sized AES key.

| PRK CMK | CMK | |
| key usage | u32 | usage tag of the kind of key that will be output |
| Info length | u32 | |
| Info | u8[...] | |
Copy link
Contributor

Choose a reason for hiding this comment

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

A potential use-case for key derivation could involve pulling multiple keys out of one larger one. (Bits 0..255 are an AES key, bits 256..511 are an HMAC key, etc.)

We could enable that use-case without too much trouble by allowing the caller to pass in byte offsets when requesting a key be expanded. That would require the caller to invoke this command multiple times to get all the keys out, but that should be ok.

This would dovetail with the comment made above about how we maybe can't just infer the key size based on the usage tag.

| -------------- | -------- | ------------------------------------ |
| chksum | u32 | |
| fips_status | u32 | FIPS approved or an error |
| tag verified | u32 | 1 if tags matched, 0 if they did not |
Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Suggest putting language up top noting that callers MUST check the "tag verified" output argument to verify integrity.

| free blocks | u32 | Available 16-byte blocks |
| total blocks | u32 | Total 16-byte blocks |
| free CMKs | u32 | Available CMKs |
| total CMKs | u32 | Total CMKs |
Copy link
Contributor

Choose a reason for hiding this comment

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

The figures reported by this command would only correspond to CMEKs for which Caliptra tracks usage counters, right? If the implementation doesn't allocate any internal memory for HMAC/HKDF keys, those keys would not be reflected by this command response. A few options I see:

  • Just track usage for every key and make it consistent.
  • Specify that only AES keys are tracked, and have this command return stats only relevant for AES keys.
  • Continue to allow the implementation to be flexible, and somehow report to the user whether a given key type is reflected in this stat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants