Implement DeriveChild inputs which allow X.509 format certs and IsCA=True

Spec: https://github.com/TrustedComputingGroup/Server-Internal/pull/123

We need to extend the platform trait to allow the vendor_id, vendor_sku, auto_init_locality, and max_tci_nodes be implementation defined. Further, we need to remove the array of localities from DpeInstance::new and instead, just trust that each DPE command is called with the correct locality.

The spec says for RotateContext it says:

If TARGET_IS_DEFAULT is set, NEW_CONTEXT_HANDLE SHALL be made the default in the caller's locality. If the default context handle is already defined in the caller's locality, this command SHALL fail.

Right now there is no concept of a default context per locality. This needs to be added along with this check for the command.

Split the tests based on test type for better manageability.

We have a CryptoBuf type which holds a max buffer and a size. It may be more idiomatic to use https://docs.rs/arrayvec/latest/arrayvec/struct.ArrayVec.html, which would let us treat them like vectors.

There are many tests that should be added to DPEs verification tests to prove correctness and spec compliance of DPE. Some areas for improvement

• Integrate the tests with Caliptra's CModel API
• Better X.509 validation
• Better coverage of commands and input combinations
• End-to-end scenario tests of DPE

See NCC report, page 21

DeriveChild - command tests
CertifyKey - changing inputs
GetCertificateChain - Test edge cases

Make issuer match cert chain

The DPE default platform has

1. One self-signed cert in the cert chain:

2. An issuer name:

We should make the issuer name in 2 be the same as the subject name in 1. This will ensure that the issuer name in DPE leaf certs will match the issuing cert and cert chain validation will work.

Make the alias cert match the algorithm of the platform being used

1. If profile is p256, cert chain key should be p256
2. If p384, cert chain should have p384 key

Make crypto key and cert chain match

The OpensslCrypto implementation can take a key at build time to use as the alias key:

We should make this key the same key that's in the cert in DefaultPlatform

Internal state in DpeInstance should only change if a command succeeds. We should have the fuzz tests check for cases where

• DPE command failed
• Any state in DpeInstance changed

And raise a fatal error.

Types such as the support flags or flags in various command structures are just u32's in the spec. In the DPE codebase we expand these to be represented as bools.

Instead, we should consider using the bitflags crate, which wraps numeric types with a convenient API to write/read booleans.

Run X.509 linter on full certificate chain and file issues for errors (today the linteris only run on the DPE leaf cert) - For certificates produced by CertifyKey and GetCertificateChain

NCC report page 27

See NCC Caliptra Security Report, page 16.

NCC report page 33

CertifyKey, CertifyCsr, and Sign are all supposed to support non-deterministic key derivation for their implementations. Support for this capability will need to be added to the Crypto trait along with updating the commands to accept symmetric signing as an option.

See https://github.com/chipsalliance/caliptra-sw/tree/main/runtime#dpe-pcr-usage

NCC report page 25

See NCC report, page 19

1. Localities & Isolation
   Add tests to ensure localities are properly isolated from each other for all commands
   Test that DeriveChild privileges (like ALLOW_CA) are correctly bounded and that a DPE context cannot escalate its privilege

2. Validate proper behavior to against the feature support matrix (exposed by
   GetProfile)

In DPE, we use InternalError to check for cases that should never happen (e.g. context data is in a bad/unrecoverable state). We should check for this in fuzz tests and fail if we see this error.

Satisfying TPM policies with PolicySigned
Imitate an SoC manager performing SPDM Responder-like flows

See https://github.com/TrustedComputingGroup/Server-Internal/blob/main/dpe-irot-profile/main.md#certifycsr for specification.

In Caliptra, we have found that we are running out of stack space frequently, due to the space that DPE uses. Thus, it would be good to reduce stack space in DPE so that we don't have to continuously find ways to reduce the memory footprint in Caliptra itself, whenever we write a new mbox cmd in the runtime.

Some ways we could go about doing this in order of priority:

1. Remove temporary buffers
2. Allocate only as many bytes as is actually necessary (in a few places, we allocate a buffer of many more bytes than we will ever actually fill out)
3. Avoid recursion (I believe it is used in one place in dpe_instance.rs currently)
4. Instead of passing many arguments to a function, pass a reference to a struct containing those arguments
5. Remove other superfluous local variables

NCC report page 30

Implement a transport to allow the DPE verification tests to send DPE commands to the Caliptra Emulator via the CModel bindings

Cgo bindings to be used

We should check our certificates against zlint to make sure we are constructing the certificates correctly.

Failure cases defined in iRoT profile
Test case for each combination - all flags of DPE commands

Attempting to parse the x509 cert using crypto/x509.ParseCertificate:

"Could not parse certificate: x509: invalid RDNSequence: invalid attribute value: invalid PrintableString"

Here is a dump of the cert: https://lapo.it/asn1js/#MIICDzCCAbWgAwIBAgIhAOFNwBqXmBqR15o3po1MNVvMpJlyA4bj6v2_oUvgkXfkMAoGCCqGSM49BAMCMD4xPDARBgNVBAMTCkRQRSBJc3N1ZXIwJwYDVQQFEyAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAADAiGA8yMDIzMDIyNzAwMDAwMFoYDzk5OTkxMjMxMjM1OTU5WjA8MTowDwYDVQQDEwhEUEUgTGVhZjAnBgNVBAUTIOFNwBqXmBqR15o3po1MNVvMpJlyA4bj6v2_oUvgkXfkMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAENjFEK7nYsmP_32T0Vh8L6h7wMkH9ujI5vFMAKlukJo2T0KaX8vheO3O6lNkSIyelpvH0uKWoBETQ3BUvN7tkDaOBgTB_MH0GBmeBBQUEBQEB_wRwMG4wbKZeMC0GCWCGSAFlAwQCAQQgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwLQYJYIZIAWUDBAIBBCAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIgEVVNFUokEAAAAADAKBggqhkjOPQQDAgNIADBFAiBE1KCwi5PJ2wnKpubzcNHlwnyxP8Up3q-DLGGj5rBH3AIhAMr7A6MpESaYGjUq4O5SQk2MvKjeFNx-jEYwksfMLX7V

Here we see that the serialNumber for the issuer is

00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00

and the serial number for the leaf is

E1 4D
C0 1A 97 98 1A 91 D7 9A 37 A6 8D 4C 35 5B CC A4
99 72 03 86 E3 EA FD BF A1 4B E0 91 77 E4

neither of which appear to be printable strings (according to Go's x509 library)

2.5.4.5 appears to say that the serial number should be a printable string: https://www.alvestrand.no/objectid/2.5.4.5.html

See https://github.com/TrustedComputingGroup/Server-Internal/blob/main/dpe-irot-profile/main.md#getcertificatechain for specification. We will also need to implement serialization of this command.

NCC report page 31

CertifyKey, CertifyCsr, and Sign are all supposed to support signing with a symmetric key (i.e. HMAC). Support for this capability must be added to the Crypto trait along with updating the commands to accept symmetric signing as an option.

Followup from Zach's comment - #77 (comment)

All of the flags in DeriveChild have been implemented besides INTERNAL_INPUT_INFO and INTERNAL_INPUT_DICE. Our implementation needs to be able to support these two variants.

Now that we pass a reference. We could probably remove the crypto generic and pass &mut impl. That might make things cleaner for everything.

Originally posted by @zhalvorsen in #121 (comment)

We implement TryFrom for all the command/response structures to convert them to/from bytes. This involves copying field-by-field.

Instead we could use zerocopy. Today we don't to avoid the external dependency, but we should consider doing this in the future to reduce the amount of error-prone boilerplate needed for each new command.

Currently EcdsaPriv is passed directly as a CryptoBuf which makes sense in the OpenSSL implementation, but will not be the case for Caliptra. Caliptra will take a key handle and will never see the raw private key directly. We should make the Crypto implementation more generic to allow for different values to be passed in place of the private key.

The Crypto definition will need to add a new type trait:

pub trait Crypto {
    type Cdi;
    type Hasher: Hasher;
    type EcdsaSigner: Signer;
...

The Signer trait will likely only need one function to implement (sign). This might look something like this:

pub trait Signer {
    /// Sign `digest`.
    ///
    /// # Arguments
    ///
    /// * `digest` - Digest of data to be signed.
    fn sign(&self, digest: &Digest) -> Result<EcdsaSig, CryptoError>;
}

We might also want to make Signer more generic to allow the trait to be used with symmetric signing.

As we have added more commands, the dpe_instance.rs file has gotten quite large. Almost 40% of the lines of code in the dpe directory are in dpe_instance.rs. I'm thinking we should refactor this a little. Instead of having all of the command execution in the DpeInstance directly. We could move each command into is own file, this would pull the logic into smaller, more easily consumable pieces along with their unit tests.

I can think of a couple ways to split this up.

1. Move just the implementation to the separate file

get_profile.rs would look something like this:

use super::DpeInstance;

impl DpeInstance {
   fn get_profile(&self) -> Result<GetProfileResp, DpeErrorCode> {
        Ok(GetProfileResp::new(self.support.get_flags()))
    }
}

#[cfg(test)]
pub mod tests {
    use super::*;

    #[test]
    fn test_get_profile() {
        let dpe = DpeInstance::<OpensslCrypto>::new(SUPPORT, &TEST_LOCALITIES).unwrap();
        let profile = dpe.get_profile().unwrap();
        assert_eq!(profile.version, CURRENT_PROFILE_VERSION);
        assert_eq!(profile.flags, SUPPORT.get_flags());
    }
}

The directory tree would look something like this:

dpe/src
├── dpe_instance
│   ├── get_profile.rs
│   ├── initialize_context.rs
│   ├── ...
│   └── mod.rs
├── command.rs
├── lib.rs
├── response.rs
└── x509.rs

2. Add a trait implemented for each command

pub trait Command {
    fn execute(&self, dpe: &mut DpeInstance<C>, locality: u32) -> Result<Response, DpeErrorCode>;
}

The directory tree would look something like this:

dpe/src
├── commands
│   ├── get_profile.rs
│   ├── initialize_context.rs
│   ├── ...
│   └── mod.rs
├── command.rs
├── dpe_instance.rs
├── lib.rs
├── response.rs
└── x509.rs

ROM and FMC both have tests to make sure that they're panic free (ensure the panic handler isn't linked). Might be good to go through and make DPE panic-free as well.

This might be easier to test once we link DPE into the RT image.

Making the vendor_info "LOC:" ++ TARGET_LOCALITY messes up parsing of the x.509 cert since it expects the vendor_info to only be 4 bytes. We should review whether or not we want to include the "LOC:" prefix in the vendor_info. https://github.com/TrustedComputingGroup/Server-Internal/blob/main/dpe-irot-profile/main.md#populating-tcg-dice-multitcbinfo

DPE certs should include the following extensions:

• BasicConstraints: IsCA = False
• KeyUsage: digitalSignature

We have previously discussed proposing well-known contexts to DPE. It adds some complexity, so I want to discuss if it's actually necessary.

What is a well-known context?

A DPE context where the value is chosen by the caller. This is similar to the default context (which has the well-known handle value of 0). Well-known contexts expand this idea to allow the caller to choose the handle value for any context, not just one default context.

Why might we want this?

In general, well-known handles are useful when a component needs to hold a handle but may not always have a place to store that handle or a way to receive it from another component. This mostly relates to measuring updates.

Case 1:

• Component A manages updates for itself. It needs to hold its to use after update.
• A can be impactlessly updated. Across its own updates, it does not have a place to store a random value.
• A cannot use the default context because another component in the same Locality is already using the default context.
• A can make well-known contexts 1 hard code use of that values instead of storing a handle across updates.

Case 2:

• Component A manages updates B and itself. It needs to hold handles to each to make measurements of impactless updates.
• Component A can itself be impactlessly updated. Across its own updates, it does not have a place to store a random value.
• A can make well-known contexts 1 and 2 and hard code use of those values instead of storing a handle across updates.

Why might this be problematic?

Well-known contexts are a bit of a foot-gun. If there are other components in your locality, they can also use this handle. I think this makes case 1 a non-starter. If UEFI is in the same locality, it could use well-known context 1 to impersonate A.

To use them securely, there can't be anything else in the same locality that is further down the TCB chain.

What do we do?

My hope is we can convince ourselves the above cases aren't real. That is, either the default context can be used, or these components can hold onto a random handle.

If not, we may need to come up with a different concept. I think possibility of accidental misuse of this feature is high.

Since the DPE implementation doesn't really have anything useful to do with a Debug string, we could probably bulk-update the derive attributes and move Debug into #[cfg_attr(test

If an error happens late in a command, the state of the system could have undefined behavior.

Here is a single example, but there are several other cases which could have similar problems.

In the DeriveChild command, we generate a child, add it to the parent's list of children, activate it, and then add the TCI measurement given in the command. If the hashing engine fails while adding the TCI measurement, we immediately cascade the error upwards. This would leave a dangling child context that nothing has a handle to.

We should be able to handle errors such that the state of the DPE instance in unchanged. The easiest way to accomplish this would to copy out the Context, modify it, and then commit it back to the DPE instance. This will allow us to return at any point in the command without affecting the persistent state of the DPE instance.

After the "platform" refactor, fuzzing DPE with #129 - specifically, libFuzzer - panics with the following:
thread '<unnamed>' panicked at 'source slice length (0) does not match destination slice length (2048)', <caliptra-sw>/runtime/dpe/platform/src/default.rs:158:13

Specifically, the following (deserialised) input panics:
GetCertificateChain(GetCertificateChainCmd { offset: 274, size: 0 })

NCC report page 24

NCC report page 28

Validate that proper TcbInfo extensions are populated with expected measurements and values as defined in the DPE iRoT profile

Validate correct KeyUsage and BasicConstraints extensions for all DICE certificates

Ensure full certificate chain has valid signatures

Too often we use the same error code for the different conditions. This will make DPE really hard to debug in production. We should differentiate error codes better to help with this.