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

[doc] Cryptographic Mailbox #1745

Merged
merged 11 commits into from
Oct 30, 2024
Merged

[doc] Cryptographic Mailbox #1745

merged 11 commits into from
Oct 30, 2024

Conversation

swenson
Copy link
Contributor

@swenson swenson commented Oct 28, 2024

This adds sections describing the Cryptographic Mailbox system, its storage system, and related commands.

I cleaned up a little bit of the overview sections for what is new in the 1.1 and 2.0 runtime firmware to help keep things organized.

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 Outdated 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
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 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 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 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
@mhatrevi mhatrevi added the Caliptra v2.0 Items to be considered for v2.0 Release label Oct 29, 2024
@swenson swenson changed the title [doc] Cryptographic Mailbox, Manifest-Based Image Auth [doc] Cryptographic Mailbox Oct 29, 2024
@swenson
Copy link
Contributor Author

swenson commented Oct 29, 2024

I believe I have addressed the comments relevant to the cryptographic mailbox commands. I'm creating a separate PR for the manifest-based image auth doc updates, as suggested.

PTAL.

runtime/README.md 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 Outdated Show resolved Hide resolved
runtime/README.md Show resolved Hide resolved
runtime/README.md Outdated Show resolved Hide resolved
runtime/README.md Show resolved Hide resolved
This adds sections describing the Manifest-Based Image Authorization
system and related mailbox commands as well as the Cryptographic Mailbox
system, its storage system, and related commands.

I cleaned up a little bit of the manifest-based image auth commands and
added some overview sections for what is new in the 1.1, 1.2, and 2.0
runtime firmware to help keep things organized.
runtime/README.md Show resolved Hide resolved
@vsonims vsonims merged commit 37163d1 into main-2.x Oct 30, 2024
8 checks passed
@swenson
Copy link
Contributor Author

swenson commented Oct 30, 2024

Apologies for merging a little early, but we wanted to have this in before the deadline. I'm happy to do further revisions and have more discussions.

@swenson swenson deleted the fw-spec-2.0 branch October 30, 2024 18:03

The context must be passed from the `CM_ECDH_GENERATE` command.

The incoming exchange data MUST be the concatenation of the x- and y- coordinates of the other side's public point.
Copy link
Contributor

Choose a reason for hiding this comment

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

How are the coordinates encoded? Big-endian?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, big-endian.


The contexts contain the internal structures necessary to resume operations to support data that may exceed the size of a single mailbox command.

These contexts are intended to be opaque to the user, and SHALL be encrypted and authenticated if they contain sensitive internal data.

Choose a reason for hiding this comment

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

what threat is this intended to solve/mitigate? it is defense in depth mostly?
Asked another way, what happens if you leave it unencrypted? presumably, a runtime attack on FW has access to the key (indirectly?) used to encrypt? or is that not hte case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The context might have intermediate state that is sensitive. For example, intermediate GHASH computations in AES could be used to reveal the additional authenticated data, which may or may not be sensitive.

The authenticated encryption is more defense in depth.

Choose a reason for hiding this comment

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

thx, agree on the intermediate state being sensitive, but was wondering HOW you envision an attacker getting access to it, mostly to try and answer the question: if the attacker can get access to context data, can the attacker also not get access to the key?
What makes getting access to the key (or its handle) more or less difficult than having access to the context data?

Note, not asking for change, trying to better understand thought process behind decision to do so.

Choose a reason for hiding this comment

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

i think i may have misread/misunderstood. I thought contexts are stored in the data storage, but i see that you provide it as output in commands below, so the question above may be moot, but would appreciate confirmation that data storage below, is for the KEYS, not contexts :)
Do we care about replay protection of the contexts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Contexts could be stored insecurely in the SoC, so they are more vulnerable to being intercepted.

In this version, the keys are only accessible through handles, but the keys themselves are stored only in DCCM in Caliptra. (This may change so that the keys are also returned to the SoC in encrypted form to minimize storage requirements in Caliptra.)

Contexts are also encrypted and authenticated. For many of the commands, I don't think we're too worried about replay attacks (e.g., SHA). But, I think you're right that some commands could use replay attack protection. For instance, with AES-GCM, if you have a context and an associated ciphertext block, you could replay the context with a new plaintext and use that to recover the other plaintext.

Theoretically this is all over a trusted channel, so these attacks may not be part of the threat model, but we should be able to apply some internal counters in DCCM to help with this. I've filed an issue to address this: #1761


The IV is the concatenation of `block offset || size`.

This scheme avoids having to store the IV while avoiding IV reuse problems.

Choose a reason for hiding this comment

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

can you elaborate on why IV reuse is not a problem? if you store the different data twice at the same offset with same size, would you not generate the same key and same IV again to encrypt the new data?

Copy link
Contributor Author

@swenson swenson Oct 30, 2024

Choose a reason for hiding this comment

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

I noticed that after I wrote it. It's definitely true (if the data being stored is the same length).

I'm debating a few different ways to fix this, but all solutions I can think of essentially involve storing the IV or a part of the IV either in DCCM or in the CMID.

Copy link
Contributor

Choose a reason for hiding this comment

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

if you store the different data twice at the same offset with same size, would you not generate the same key and same IV again to encrypt the new data?

I don't think you would, right? Because the CMID does not hold any data related to the key itself, just the offset and size. So if you store two different keys with the same size at the same offset, the same key will encrypt the same CMID.

Choose a reason for hiding this comment

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

okay, maybe i misunderstood, what this is encrypting. I thought this is a key, being derived, to encrypt context data. The key being derived, look like a deterministic key HKDF(CDI_RT, "CMID||block offset||size"), and that key would AES GCM the context data (which can be different data bits).
I did not think this is the key used to encrypt the CMID itself, given to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scheme here was K = HKDF(CDI_RT, block offset || size), which would then be used to encrypt that portion of DCCM that holds the key associated with the CMID.

But I think we're going to move away from storing keys in DCCM shortly, so this shouldn't matter. Though we will still have to be very careful about IV reuse.

| chksum | u32 | |
| context size | u32 | Size of the context. |
| context | u8[...] | From `CM_SHA_INIT` / `CM_SHA_UPDATE` |
| data size | u32 | |

Choose a reason for hiding this comment

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

do we care that data size field may become an unaligned access depending on context size?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could define a fixed-size context struct, sized to hold the largest context data possible, and make sure it's aligned properly.

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 worry about a fixed-size struct, as it means that if we define a new command or want to redefine the context for one command, we break compatibility with all commands that use contexts. With the variable-sized contexts, we wouldn't need to break compatibility if we needed to add a field to any context.

But we can stipulate that all contexts have sizes that are multiples of 32 bits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Caliptra v2.0 Items to be considered for v2.0 Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants