Self Signed FMC Alias Csr #1863
Conversation
rusty1968
commented
Dec 19, 2024
rusty1968
commented
- FMC modfied to generate a self signed FMC Alias CSR test upon cold boot.
- Persistent driver modified to add persistent memory for the FMC Alias CSR
- Runtime modified to expose an API to retrieve it.
- Test case created to verify the self signed FMC Alias CSR.
- Test case created to verify the RT Alias Certificate with the pub key of the FMC Alias CSR.
- FMC modfied to generate a self signed FMC Alias CSR test upon cold boot. - Persistent driver modified to add persistent memory for the FMC Alias CSR - Runtime modified to expose an API to retrieve it. - Test case created to verify the self signed FMC Alias CSR. - Test case created to verify the RT Alias Certificate with the pub key of the FMC Alias CSR.
rusty1968
requested review from
jhand2,
fdamato,
bluegate010,
mhatrevi,
swenson,
vsonims,
FerralCoder,
ajisaxena,
jlmahowa-amd,
korran and
JohnTraverAmd
as code owners
| @@ -1010,6 +1016,41 @@ impl Default for GetIdevCsrResp { | |||
| } | |||
| } | |||
|
|
|||
| // GET_IDEVID_CSR | |||
There was a problem hiding this comment.
nit: Minor copy paste error
| #[cfg(feature = "fmc")] | ||
| reserved11: [u8; memory_layout::FMC_ALIAS_CSR_SIZE as usize - size_of::<FmcAliasCsr>()], |
There was a problem hiding this comment.
When the fmc is not enabled, a slice of memory_layout::FMC_ALIAS_CSR_SIZE should still be reserved so PersistentData is the same size / shape between image types.
| @@ -450,6 +450,9 @@ impl CaliptraError { | |||
| pub const RUNTIME_AUTH_MANIFEST_IMAGE_METADATA_LIST_DUPLICATE_FIRMWARE_ID: CaliptraError = | |||
| CaliptraError::new_const(0x000E0053); | |||
|
|
|||
| pub const RUNTIME_GET_FMC_CSR_UNPROVISIONED: CaliptraError = | |||
| CaliptraError::new_const(0x000E0053); | |||
There was a problem hiding this comment.
0x000E0053 => 0x000E0054 so RUNTIME_GET_FMC_CSR_UNPROVISIONED has a unique error code.
| /// # Arguments | ||
| /// | ||
| /// * `env` - FMC Environment | ||
| /// * `priv_key` - Key slot to retrieve the private key |
There was a problem hiding this comment.
Looks like pub_key is missing from the doc comment.
| /// * `hand_off` - HandOff | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// * `DiceInput` - DICE Layer Input |
There was a problem hiding this comment.
Looks like the doc string doesn't match the function signature.
| let csr_persistent_mem = &drivers.persistent_data.get().fmc_alias_csr; | ||
|
|
||
| match csr_persistent_mem.get_csr_len() { | ||
| FmcAliasCsr::UNPROVISIONED_CSR => Err(CaliptraError::RUNTIME_GET_FMC_CSR_UNPROVISIONED), |
There was a problem hiding this comment.
Does the 0 case matter? E.g. for FMC images that did not support this feature?
|
|
||
| pub struct GetFmcAliasCsrCmd; | ||
| impl GetFmcAliasCsrCmd { | ||
| // #[cfg_attr(not(feature = "no-cfi"), cfi_impl_fn)] |
| // A valid `IDevIDCsr` cannot be larger than `MAX_CSR_SIZE`, which is the max | ||
| // size of the buffer in `GetIdevCsrResp` |
There was a problem hiding this comment.
nit: Copy and paste mismatch
| @@ -38,9 +38,9 @@ pub use fuse::{FuseLogEntry, FuseLogEntryId}; | |||
| pub use pcr::{PcrLogEntry, PcrLogEntryId, RT_FW_CURRENT_PCR, RT_FW_JOURNEY_PCR}; | |||
|
|
|||
| pub const FMC_ORG: u32 = 0x40000000; | |||
| pub const FMC_SIZE: u32 = 20 * 1024; | |||
There was a problem hiding this comment.
According to the size-history CI job, we're still only using ~18KiB in FMC. Do we need to change the size here?
| @@ -74,7 +75,8 @@ pub const DPE_SIZE: u32 = 5 * 1024; | |||
| pub const PCR_RESET_COUNTER_SIZE: u32 = 1024; | |||
| pub const AUTH_MAN_IMAGE_METADATA_MAX_SIZE: u32 = 7 * 1024; | |||
| pub const IDEVID_CSR_SIZE: u32 = 1024; | |||
| pub const DATA_SIZE: u32 = 27 * 1024; | |||
There was a problem hiding this comment.
nit: now that #1722 is merged, should be able to just add to the PersistentData driver and leave memory layout as-is
| @@ -164,6 +231,12 @@ pub struct PersistentData { | |||
|
|
|||
| pub idevid_csr: IdevIdCsr, | |||
| reserved10: [u8; memory_layout::IDEVID_CSR_SIZE as usize - size_of::<IdevIdCsr>()], | |||
|
|
|||
| #[cfg(feature = "fmc")] | |||
| pub fmc_alias_csr: FmcAliasCsr, | |||
There was a problem hiding this comment.
Doesn't this also need to be accessible in RT?
| \____\__,_|_|_| .__/ \__|_| \__,_| |_| \_\|_| | ||
| |_| | ||
| "#; | ||
| const BANNER: &str = r#"Caliptra RT"#; |
There was a problem hiding this comment.
optional: might be good to put size optimizations in a separate PR
