From 3f092e70491f0e1f283dde54e2a207d1fc343990 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Tue, 8 Oct 2024 14:28:27 -0700 Subject: [PATCH 01/11] Remove PartialEq and Eq from some sensitive arrays --- Cargo.lock | 1 + Cargo.toml | 1 + drivers/Cargo.toml | 1 + drivers/src/array.rs | 15 ++++++++++- drivers/src/ecc384.rs | 8 +++--- drivers/test-fw/src/lib.rs | 2 +- image/verify/src/verifier.rs | 4 +-- kat/src/ecc384_kat.rs | 2 +- kat/src/sha1_kat.rs | 2 +- kat/src/sha256_kat.rs | 2 +- kat/src/sha2_512_384acc_kat.rs | 2 +- kat/src/sha384_kat.rs | 2 +- lms-types/src/lib.rs | 4 +-- runtime/src/drivers.rs | 2 +- runtime/src/set_auth_manifest.rs | 44 +++++++++++++------------------- 15 files changed, 50 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 70faca31c3..7b4552cdee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -378,6 +378,7 @@ dependencies = [ "caliptra-registers", "caliptra-test", "cfg-if 1.0.0", + "constant_time_eq", "dpe", "openssl", "ufmt", diff --git a/Cargo.toml b/Cargo.toml index ad3a1b5fa4..6755ab5d0b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -143,6 +143,7 @@ cfg-if = "1.0.0" chrono = "0.4" clap = { version = "3.2.14", default-features = false, features = ["std"] } cms = "0.2.2" +constant_time_eq = "0.3.0" convert_case = "0.6.0" dpe = { path = "dpe/dpe", default-features = false, features = ["dpe_profile_p384_sha384"] } crypto = { path = "dpe/crypto", default-features = false } diff --git a/drivers/Cargo.toml b/drivers/Cargo.toml index e112f63655..5262e50a25 100644 --- a/drivers/Cargo.toml +++ b/drivers/Cargo.toml @@ -16,6 +16,7 @@ caliptra-image-types.workspace = true caliptra-lms-types.workspace = true caliptra-auth-man-types.workspace = true caliptra-registers.workspace = true +constant_time_eq.workspace = true cfg-if.workspace = true dpe = { workspace = true, optional = true } ufmt.workspace = true diff --git a/drivers/src/array.rs b/drivers/src/array.rs index 4ad03cb5b5..3890e644ec 100644 --- a/drivers/src/array.rs +++ b/drivers/src/array.rs @@ -26,12 +26,25 @@ macro_rules! static_assert { /// The `Array4xN` type represents large arrays in the native format of the Caliptra /// cryptographic hardware, and provides From traits for converting to/from byte arrays. #[repr(transparent)] -#[derive(Debug, Clone, Copy, PartialEq, Eq, Zeroize)] +#[derive(Debug, Clone, Copy, Zeroize)] pub struct Array4xN(pub [u32; W]); impl Array4xN { pub const fn new(val: [u32; W]) -> Self { Self(val) } + // secure comparison between two arrays. + // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. + pub fn eq(&self, other: &Array4xN) -> bool { + let a = self.0.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, B * 4) }; + let b = other.0.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, B * 4) }; + constant_time_eq::constant_time_eq(aslice, bslice) + } + + pub fn ne(&self, other: &Array4xN) -> bool { + !self.eq(other) + } } impl Default for Array4xN { diff --git a/drivers/src/ecc384.rs b/drivers/src/ecc384.rs index 48bc925ee1..4f05fc7e9a 100644 --- a/drivers/src/ecc384.rs +++ b/drivers/src/ecc384.rs @@ -116,7 +116,7 @@ impl<'a> From> for Ecc384PrivKeyIn<'a> { /// ECC-384 Public Key #[repr(C)] -#[derive(AsBytes, FromBytes, Debug, Default, Copy, Clone, Eq, PartialEq, Zeroize)] +#[derive(AsBytes, FromBytes, Debug, Default, Copy, Clone, Zeroize)] pub struct Ecc384PubKey { /// X coordinate pub x: Ecc384Scalar, @@ -135,7 +135,7 @@ impl Ecc384PubKey { /// ECC-384 Signature #[repr(C)] -#[derive(Debug, Default, AsBytes, FromBytes, Copy, Clone, Eq, PartialEq, Zeroize)] +#[derive(Debug, Default, AsBytes, FromBytes, Copy, Clone, Zeroize)] pub struct Ecc384Signature { /// Random point pub r: Ecc384Scalar, @@ -466,8 +466,8 @@ impl Ecc384 { // Get the verify r result let mut verify_r = self.verify_r(pub_key, digest, signature)?; - // compare the hardware generate `r` with one in signature - let result = if verify_r == signature.r { + // compare the hardware generated `r` with one in signature + let result = if verify_r.eq(&signature.r) { caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &signature.r.0); Ecc384Result::Success } else { diff --git a/drivers/test-fw/src/lib.rs b/drivers/test-fw/src/lib.rs index 903227d934..e9b8dc72af 100644 --- a/drivers/test-fw/src/lib.rs +++ b/drivers/test-fw/src/lib.rs @@ -16,7 +16,7 @@ pub const DOE_TEST_HMAC_KEY: [u32; 12] = [ 0xc6879874, 0x0aa49a0f, 0x4e740e9c, 0x2c9f9aad, ]; -#[derive(AsBytes, Clone, Copy, Default, Eq, PartialEq, FromBytes)] +#[derive(AsBytes, Clone, Copy, Default, FromBytes)] #[repr(C)] pub struct DoeTestResults { /// HMAC result of the UDS as key, and b"Hello world!" as data. diff --git a/image/verify/src/verifier.rs b/image/verify/src/verifier.rs index 0b5193eb6e..b0eb36f8f4 100644 --- a/image/verify/src/verifier.rs +++ b/image/verify/src/verifier.rs @@ -506,7 +506,7 @@ impl ImageVerifier { CaliptraError::IMAGE_VERIFIER_ERR_OWNER_ECC_VERIFY_FAILURE })?; - if cfi_launder(verify_r) != caliptra_drivers::Array4xN(sig.r) { + if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN(sig.r)) { Err(CaliptraError::IMAGE_VERIFIER_ERR_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &sig.r); @@ -546,7 +546,7 @@ impl ImageVerifier { CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_VERIFY_FAILURE })?; - if cfi_launder(verify_r) != caliptra_drivers::Array4xN(ecc_sig.r) { + if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN(ecc_sig.r)) { Err(CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &ecc_sig.r); diff --git a/kat/src/ecc384_kat.rs b/kat/src/ecc384_kat.rs index c8513ee7df..85202b0654 100644 --- a/kat/src/ecc384_kat.rs +++ b/kat/src/ecc384_kat.rs @@ -76,7 +76,7 @@ impl Ecc384Kat { .sign(&Ecc384PrivKeyIn::from(&PRIV_KEY), &PUB_KEY, &digest, trng) .map_err(|_| CaliptraError::KAT_ECC384_SIGNATURE_GENERATE_FAILURE)?; - if signature != SIGNATURE { + if signature.r.ne(&SIGNATURE.r) || signature.s.ne(&SIGNATURE.s) { Err(CaliptraError::KAT_ECC384_SIGNATURE_VERIFY_FAILURE)?; } Ok(()) diff --git a/kat/src/sha1_kat.rs b/kat/src/sha1_kat.rs index 996d8f9c66..b7e445dc5b 100644 --- a/kat/src/sha1_kat.rs +++ b/kat/src/sha1_kat.rs @@ -43,7 +43,7 @@ impl Sha1Kat { .digest(&data) .map_err(|_| CaliptraError::KAT_SHA1_DIGEST_FAILURE)?; - if digest != EXPECTED_DIGEST { + if digest.ne(&EXPECTED_DIGEST) { Err(CaliptraError::KAT_SHA1_DIGEST_MISMATCH)?; } diff --git a/kat/src/sha256_kat.rs b/kat/src/sha256_kat.rs index 2aa5515f79..72a7608dae 100644 --- a/kat/src/sha256_kat.rs +++ b/kat/src/sha256_kat.rs @@ -45,7 +45,7 @@ impl Sha256Kat { .digest(&data) .map_err(|_| CaliptraError::KAT_SHA256_DIGEST_FAILURE)?; - if digest != EXPECTED_DIGEST { + if digest.ne(&EXPECTED_DIGEST) { return Err(CaliptraError::KAT_SHA256_DIGEST_MISMATCH); } diff --git a/kat/src/sha2_512_384acc_kat.rs b/kat/src/sha2_512_384acc_kat.rs index 1f8ad5c714..ef38a91332 100644 --- a/kat/src/sha2_512_384acc_kat.rs +++ b/kat/src/sha2_512_384acc_kat.rs @@ -60,7 +60,7 @@ impl Sha2_512_384AccKat { sha_acc_op .digest_512(0, 0, false, &mut digest) .map_err(|_| CaliptraError::KAT_SHA2_512_384_ACC_DIGEST_FAILURE)?; - if digest != SHA512_EXPECTED_DIGEST { + if digest.ne(&SHA512_EXPECTED_DIGEST) { Err(CaliptraError::KAT_SHA2_512_384_ACC_DIGEST_MISMATCH)?; } diff --git a/kat/src/sha384_kat.rs b/kat/src/sha384_kat.rs index c6a2f027fa..925b72a0a3 100644 --- a/kat/src/sha384_kat.rs +++ b/kat/src/sha384_kat.rs @@ -45,7 +45,7 @@ impl Sha384Kat { .digest(data) .map_err(|_| CaliptraError::KAT_SHA384_DIGEST_FAILURE)?; - if digest != SHA384_EXPECTED_DIGEST { + if digest.ne(&SHA384_EXPECTED_DIGEST) { Err(CaliptraError::KAT_SHA384_DIGEST_MISMATCH)?; } diff --git a/lms-types/src/lib.rs b/lms-types/src/lib.rs index 11b3cc66dc..5717519b24 100644 --- a/lms-types/src/lib.rs +++ b/lms-types/src/lib.rs @@ -96,7 +96,7 @@ unsafe impl FromBytes for LmsPublicKey { } #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] -#[derive(Copy, Clone, Debug, PartialEq, Eq, Zeroize)] +#[derive(Copy, Clone, Debug, Zeroize)] #[repr(C)] pub struct LmotsSignature { #[zeroize(skip)] @@ -133,7 +133,7 @@ unsafe impl FromBytes for LmotsSignature { } #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug)] #[repr(C)] pub struct LmsSignature { pub q: U32, diff --git a/runtime/src/drivers.rs b/runtime/src/drivers.rs index 4b1ef53883..35797d545d 100644 --- a/runtime/src/drivers.rs +++ b/runtime/src/drivers.rs @@ -297,7 +297,7 @@ impl Drivers { let latest_pcr = drivers.pcr_bank.read_pcr(RT_FW_JOURNEY_PCR); // Ensure TCI from SRAM == RT_FW_JOURNEY_PCR - if latest_pcr != latest_tci { + if latest_pcr.ne(&latest_tci) { // If latest pcr validation fails, disable attestation let result = DisableAttestationCmd::execute(drivers); if cfi_launder(result.is_ok()) { diff --git a/runtime/src/set_auth_manifest.rs b/runtime/src/set_auth_manifest.rs index cf5146fad8..b843ffe485 100644 --- a/runtime/src/set_auth_manifest.rs +++ b/runtime/src/set_auth_manifest.rs @@ -131,11 +131,9 @@ impl SetAuthManifestCmd { &auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r) - != caliptra_drivers::Array4xN( - auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig.r, - ) - { + if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( + auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig.r, + )) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( @@ -195,11 +193,9 @@ impl SetAuthManifestCmd { &auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r) - != caliptra_drivers::Array4xN( - auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r, - ) - { + if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( + auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r, + )) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( @@ -252,14 +248,12 @@ impl SetAuthManifestCmd { .ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r) - != caliptra_drivers::Array4xN( - auth_manifest_preamble - .vendor_image_metdata_signatures - .ecc_sig - .r, - ) - { + if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( + auth_manifest_preamble + .vendor_image_metdata_signatures + .ecc_sig + .r, + )) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( @@ -311,14 +305,12 @@ impl SetAuthManifestCmd { .ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r) - != caliptra_drivers::Array4xN( - auth_manifest_preamble - .owner_image_metdata_signatures - .ecc_sig - .r, - ) - { + if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( + auth_manifest_preamble + .owner_image_metdata_signatures + .ecc_sig + .r, + )) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( From 12a2b8f462f1b9a520f2c7b5596a5f42eccc0fb2 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Tue, 8 Oct 2024 15:47:12 -0700 Subject: [PATCH 02/11] wip on lms --- drivers/src/array.rs | 9 ++++----- drivers/src/lms.rs | 2 +- lms-types/src/lib.rs | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/src/array.rs b/drivers/src/array.rs index 3890e644ec..db7be7e490 100644 --- a/drivers/src/array.rs +++ b/drivers/src/array.rs @@ -32,19 +32,18 @@ impl Array4xN { pub const fn new(val: [u32; W]) -> Self { Self(val) } +} + +impl PartialEq for Array4xN { // secure comparison between two arrays. // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. - pub fn eq(&self, other: &Array4xN) -> bool { + fn eq(&self, other: &Self) -> bool { let a = self.0.as_ptr() as *const u8; let aslice = unsafe { core::slice::from_raw_parts(a, B * 4) }; let b = other.0.as_ptr() as *const u8; let bslice = unsafe { core::slice::from_raw_parts(b, B * 4) }; constant_time_eq::constant_time_eq(aslice, bslice) } - - pub fn ne(&self, other: &Array4xN) -> bool { - !self.eq(other) - } } impl Default for Array4xN { diff --git a/drivers/src/lms.rs b/drivers/src/lms.rs index 7e2a239953..1eaf516a62 100644 --- a/drivers/src/lms.rs +++ b/drivers/src/lms.rs @@ -45,7 +45,7 @@ pub enum LmsResult { } #[repr(transparent)] -#[derive(Debug, Clone, Copy, Eq, PartialEq)] +#[derive(Debug, Clone, Copy)] pub struct HashValue(pub [u32; N]); impl Default for HashValue { diff --git a/lms-types/src/lib.rs b/lms-types/src/lib.rs index 5717519b24..fcbe7bc4cf 100644 --- a/lms-types/src/lib.rs +++ b/lms-types/src/lib.rs @@ -170,7 +170,7 @@ unsafe impl FromBytes for LmsSig fn only_derive_is_allowed_to_implement_this_trait() {} } -#[derive(Debug, Copy, Clone, Eq, PartialEq)] +#[derive(Debug, Copy, Clone)] #[repr(C)] pub struct LmsPrivateKey { pub tree_type: LmsAlgorithmType, From 236f510d06a83f3743bf4594989252e6b2364aec Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 9 Oct 2024 10:33:42 -0700 Subject: [PATCH 03/11] Enable eq and peq in tests and std --- Cargo.lock | 1 - drivers/Cargo.toml | 1 - drivers/src/array.rs | 13 +------------ drivers/src/ecc384.rs | 2 ++ drivers/src/lms.rs | 1 + drivers/test-fw/src/lib.rs | 1 + image/types/src/lib.rs | 3 ++- lms-types/src/lib.rs | 2 ++ 8 files changed, 9 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7b4552cdee..70faca31c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -378,7 +378,6 @@ dependencies = [ "caliptra-registers", "caliptra-test", "cfg-if 1.0.0", - "constant_time_eq", "dpe", "openssl", "ufmt", diff --git a/drivers/Cargo.toml b/drivers/Cargo.toml index 5262e50a25..e112f63655 100644 --- a/drivers/Cargo.toml +++ b/drivers/Cargo.toml @@ -16,7 +16,6 @@ caliptra-image-types.workspace = true caliptra-lms-types.workspace = true caliptra-auth-man-types.workspace = true caliptra-registers.workspace = true -constant_time_eq.workspace = true cfg-if.workspace = true dpe = { workspace = true, optional = true } ufmt.workspace = true diff --git a/drivers/src/array.rs b/drivers/src/array.rs index db7be7e490..205fdb95ed 100644 --- a/drivers/src/array.rs +++ b/drivers/src/array.rs @@ -27,6 +27,7 @@ macro_rules! static_assert { /// cryptographic hardware, and provides From traits for converting to/from byte arrays. #[repr(transparent)] #[derive(Debug, Clone, Copy, Zeroize)] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] pub struct Array4xN(pub [u32; W]); impl Array4xN { pub const fn new(val: [u32; W]) -> Self { @@ -34,18 +35,6 @@ impl Array4xN { } } -impl PartialEq for Array4xN { - // secure comparison between two arrays. - // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. - fn eq(&self, other: &Self) -> bool { - let a = self.0.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, B * 4) }; - let b = other.0.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, B * 4) }; - constant_time_eq::constant_time_eq(aslice, bslice) - } -} - impl Default for Array4xN { fn default() -> Self { Self([0u32; W]) diff --git a/drivers/src/ecc384.rs b/drivers/src/ecc384.rs index 4f05fc7e9a..ad182b1c3d 100644 --- a/drivers/src/ecc384.rs +++ b/drivers/src/ecc384.rs @@ -117,6 +117,7 @@ impl<'a> From> for Ecc384PrivKeyIn<'a> { /// ECC-384 Public Key #[repr(C)] #[derive(AsBytes, FromBytes, Debug, Default, Copy, Clone, Zeroize)] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] pub struct Ecc384PubKey { /// X coordinate pub x: Ecc384Scalar, @@ -136,6 +137,7 @@ impl Ecc384PubKey { /// ECC-384 Signature #[repr(C)] #[derive(Debug, Default, AsBytes, FromBytes, Copy, Clone, Zeroize)] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] pub struct Ecc384Signature { /// Random point pub r: Ecc384Scalar, diff --git a/drivers/src/lms.rs b/drivers/src/lms.rs index 1eaf516a62..1bad920cd0 100644 --- a/drivers/src/lms.rs +++ b/drivers/src/lms.rs @@ -46,6 +46,7 @@ pub enum LmsResult { #[repr(transparent)] #[derive(Debug, Clone, Copy)] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] pub struct HashValue(pub [u32; N]); impl Default for HashValue { diff --git a/drivers/test-fw/src/lib.rs b/drivers/test-fw/src/lib.rs index e9b8dc72af..f1a3a314e0 100644 --- a/drivers/test-fw/src/lib.rs +++ b/drivers/test-fw/src/lib.rs @@ -17,6 +17,7 @@ pub const DOE_TEST_HMAC_KEY: [u32; 12] = [ ]; #[derive(AsBytes, Clone, Copy, Default, FromBytes)] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] #[repr(C)] pub struct DoeTestResults { /// HMAC result of the UDS as key, and b"Hello world!" as data. diff --git a/image/types/src/lib.rs b/image/types/src/lib.rs index 35279d2c8e..9c8b3956ba 100644 --- a/image/types/src/lib.rs +++ b/image/types/src/lib.rs @@ -66,7 +66,8 @@ pub type ImageLmsPublicKey = LmsPublicKey; pub type ImageLmsPrivKey = LmsPrivateKey; #[repr(C)] -#[derive(AsBytes, FromBytes, Default, Debug, Copy, Clone, Eq, PartialEq, Zeroize)] +#[derive(AsBytes, FromBytes, Default, Debug, Copy, Clone, Zeroize)] +#[cfg_attr(test, derive(PartialEq, Eq))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ImageEccSignature { /// Random point diff --git a/lms-types/src/lib.rs b/lms-types/src/lib.rs index fcbe7bc4cf..c11b8e45a8 100644 --- a/lms-types/src/lib.rs +++ b/lms-types/src/lib.rs @@ -97,6 +97,7 @@ unsafe impl FromBytes for LmsPublicKey { #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] #[derive(Copy, Clone, Debug, Zeroize)] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] #[repr(C)] pub struct LmotsSignature { #[zeroize(skip)] @@ -133,6 +134,7 @@ unsafe impl FromBytes for LmotsSignature { } #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] +#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] #[derive(Copy, Clone, Debug)] #[repr(C)] pub struct LmsSignature { From 8121f795d44c008ad11f9f1e51c56eb066405e00 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 9 Oct 2024 10:57:28 -0700 Subject: [PATCH 04/11] Add PartialEq for lms types --- Cargo.lock | 2 + drivers/Cargo.toml | 1 + drivers/src/array.rs | 13 +++- drivers/src/ecc384.rs | 6 +- drivers/src/lms.rs | 13 +++- drivers/test-fw/src/lib.rs | 3 +- .../tests/drivers_integration_tests/main.rs | 2 +- image/verify/src/verifier.rs | 4 +- kat/src/ecc384_kat.rs | 2 +- kat/src/sha1_kat.rs | 2 +- kat/src/sha256_kat.rs | 2 +- kat/src/sha2_512_384acc_kat.rs | 2 +- kat/src/sha384_kat.rs | 2 +- lms-types/Cargo.toml | 1 + lms-types/src/lib.rs | 76 ++++++++++++++++++- runtime/src/drivers.rs | 2 +- runtime/src/set_auth_manifest.rs | 44 ++++++----- 17 files changed, 139 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 70faca31c3..5537fa3ab1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -378,6 +378,7 @@ dependencies = [ "caliptra-registers", "caliptra-test", "cfg-if 1.0.0", + "constant_time_eq", "dpe", "openssl", "ufmt", @@ -708,6 +709,7 @@ name = "caliptra-lms-types" version = "0.1.0" dependencies = [ "arbitrary", + "constant_time_eq", "zerocopy", "zeroize", ] diff --git a/drivers/Cargo.toml b/drivers/Cargo.toml index e112f63655..5262e50a25 100644 --- a/drivers/Cargo.toml +++ b/drivers/Cargo.toml @@ -16,6 +16,7 @@ caliptra-image-types.workspace = true caliptra-lms-types.workspace = true caliptra-auth-man-types.workspace = true caliptra-registers.workspace = true +constant_time_eq.workspace = true cfg-if.workspace = true dpe = { workspace = true, optional = true } ufmt.workspace = true diff --git a/drivers/src/array.rs b/drivers/src/array.rs index 205fdb95ed..362a8e3c6e 100644 --- a/drivers/src/array.rs +++ b/drivers/src/array.rs @@ -27,7 +27,6 @@ macro_rules! static_assert { /// cryptographic hardware, and provides From traits for converting to/from byte arrays. #[repr(transparent)] #[derive(Debug, Clone, Copy, Zeroize)] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] pub struct Array4xN(pub [u32; W]); impl Array4xN { pub const fn new(val: [u32; W]) -> Self { @@ -41,6 +40,18 @@ impl Default for Array4xN { } } +impl PartialEq for Array4xN { + // secure comparison between two arrays. + // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. + fn eq(&self, other: &Self) -> bool { + let a = self.0.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, B * 4) }; + let b = other.0.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, B * 4) }; + constant_time_eq::constant_time_eq(aslice, bslice) + } +} + //// Ensure there is no padding in the struct static_assert!(core::mem::size_of::>() == 4); unsafe impl AsBytes for Array4xN { diff --git a/drivers/src/ecc384.rs b/drivers/src/ecc384.rs index ad182b1c3d..b0ace7a2b8 100644 --- a/drivers/src/ecc384.rs +++ b/drivers/src/ecc384.rs @@ -116,8 +116,7 @@ impl<'a> From> for Ecc384PrivKeyIn<'a> { /// ECC-384 Public Key #[repr(C)] -#[derive(AsBytes, FromBytes, Debug, Default, Copy, Clone, Zeroize)] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] +#[derive(AsBytes, FromBytes, Debug, Default, Copy, Clone, PartialEq, Zeroize)] pub struct Ecc384PubKey { /// X coordinate pub x: Ecc384Scalar, @@ -136,8 +135,7 @@ impl Ecc384PubKey { /// ECC-384 Signature #[repr(C)] -#[derive(Debug, Default, AsBytes, FromBytes, Copy, Clone, Zeroize)] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] +#[derive(Debug, Default, AsBytes, FromBytes, Copy, Clone, PartialEq, Zeroize)] pub struct Ecc384Signature { /// Random point pub r: Ecc384Scalar, diff --git a/drivers/src/lms.rs b/drivers/src/lms.rs index 1bad920cd0..dda1203eef 100644 --- a/drivers/src/lms.rs +++ b/drivers/src/lms.rs @@ -46,7 +46,6 @@ pub enum LmsResult { #[repr(transparent)] #[derive(Debug, Clone, Copy)] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] pub struct HashValue(pub [u32; N]); impl Default for HashValue { @@ -56,6 +55,18 @@ impl Default for HashValue { } } +impl PartialEq for HashValue { + // secure comparison between two arrays. + // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. + fn eq(&self, other: &Self) -> bool { + let a = self.0.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, N * 4) }; + let b = other.0.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, N * 4) }; + constant_time_eq::constant_time_eq(aslice, bslice) + } +} + impl HashValue { pub fn new(data: [u32; N]) -> Self { HashValue(data) diff --git a/drivers/test-fw/src/lib.rs b/drivers/test-fw/src/lib.rs index f1a3a314e0..71c82095d7 100644 --- a/drivers/test-fw/src/lib.rs +++ b/drivers/test-fw/src/lib.rs @@ -16,8 +16,7 @@ pub const DOE_TEST_HMAC_KEY: [u32; 12] = [ 0xc6879874, 0x0aa49a0f, 0x4e740e9c, 0x2c9f9aad, ]; -#[derive(AsBytes, Clone, Copy, Default, FromBytes)] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] +#[derive(AsBytes, Clone, Copy, Default, FromBytes, PartialEq)] #[repr(C)] pub struct DoeTestResults { /// HMAC result of the UDS as key, and b"Hello world!" as data. diff --git a/drivers/tests/drivers_integration_tests/main.rs b/drivers/tests/drivers_integration_tests/main.rs index 206d8e3cf0..ef268b8c49 100644 --- a/drivers/tests/drivers_integration_tests/main.rs +++ b/drivers/tests/drivers_integration_tests/main.rs @@ -50,7 +50,7 @@ fn run_driver_test(test_rom: &'static FwId) { model.step_until_exit_success().unwrap(); } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] struct DoeTestVectors { // The keys output by the DOE block (mostly for reference) doe_output: DoeOutput, diff --git a/image/verify/src/verifier.rs b/image/verify/src/verifier.rs index b0eb36f8f4..0b5193eb6e 100644 --- a/image/verify/src/verifier.rs +++ b/image/verify/src/verifier.rs @@ -506,7 +506,7 @@ impl ImageVerifier { CaliptraError::IMAGE_VERIFIER_ERR_OWNER_ECC_VERIFY_FAILURE })?; - if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN(sig.r)) { + if cfi_launder(verify_r) != caliptra_drivers::Array4xN(sig.r) { Err(CaliptraError::IMAGE_VERIFIER_ERR_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &sig.r); @@ -546,7 +546,7 @@ impl ImageVerifier { CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_VERIFY_FAILURE })?; - if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN(ecc_sig.r)) { + if cfi_launder(verify_r) != caliptra_drivers::Array4xN(ecc_sig.r) { Err(CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &ecc_sig.r); diff --git a/kat/src/ecc384_kat.rs b/kat/src/ecc384_kat.rs index 85202b0654..c8513ee7df 100644 --- a/kat/src/ecc384_kat.rs +++ b/kat/src/ecc384_kat.rs @@ -76,7 +76,7 @@ impl Ecc384Kat { .sign(&Ecc384PrivKeyIn::from(&PRIV_KEY), &PUB_KEY, &digest, trng) .map_err(|_| CaliptraError::KAT_ECC384_SIGNATURE_GENERATE_FAILURE)?; - if signature.r.ne(&SIGNATURE.r) || signature.s.ne(&SIGNATURE.s) { + if signature != SIGNATURE { Err(CaliptraError::KAT_ECC384_SIGNATURE_VERIFY_FAILURE)?; } Ok(()) diff --git a/kat/src/sha1_kat.rs b/kat/src/sha1_kat.rs index b7e445dc5b..996d8f9c66 100644 --- a/kat/src/sha1_kat.rs +++ b/kat/src/sha1_kat.rs @@ -43,7 +43,7 @@ impl Sha1Kat { .digest(&data) .map_err(|_| CaliptraError::KAT_SHA1_DIGEST_FAILURE)?; - if digest.ne(&EXPECTED_DIGEST) { + if digest != EXPECTED_DIGEST { Err(CaliptraError::KAT_SHA1_DIGEST_MISMATCH)?; } diff --git a/kat/src/sha256_kat.rs b/kat/src/sha256_kat.rs index 72a7608dae..2aa5515f79 100644 --- a/kat/src/sha256_kat.rs +++ b/kat/src/sha256_kat.rs @@ -45,7 +45,7 @@ impl Sha256Kat { .digest(&data) .map_err(|_| CaliptraError::KAT_SHA256_DIGEST_FAILURE)?; - if digest.ne(&EXPECTED_DIGEST) { + if digest != EXPECTED_DIGEST { return Err(CaliptraError::KAT_SHA256_DIGEST_MISMATCH); } diff --git a/kat/src/sha2_512_384acc_kat.rs b/kat/src/sha2_512_384acc_kat.rs index ef38a91332..1f8ad5c714 100644 --- a/kat/src/sha2_512_384acc_kat.rs +++ b/kat/src/sha2_512_384acc_kat.rs @@ -60,7 +60,7 @@ impl Sha2_512_384AccKat { sha_acc_op .digest_512(0, 0, false, &mut digest) .map_err(|_| CaliptraError::KAT_SHA2_512_384_ACC_DIGEST_FAILURE)?; - if digest.ne(&SHA512_EXPECTED_DIGEST) { + if digest != SHA512_EXPECTED_DIGEST { Err(CaliptraError::KAT_SHA2_512_384_ACC_DIGEST_MISMATCH)?; } diff --git a/kat/src/sha384_kat.rs b/kat/src/sha384_kat.rs index 925b72a0a3..c6a2f027fa 100644 --- a/kat/src/sha384_kat.rs +++ b/kat/src/sha384_kat.rs @@ -45,7 +45,7 @@ impl Sha384Kat { .digest(data) .map_err(|_| CaliptraError::KAT_SHA384_DIGEST_FAILURE)?; - if digest.ne(&SHA384_EXPECTED_DIGEST) { + if digest != SHA384_EXPECTED_DIGEST { Err(CaliptraError::KAT_SHA384_DIGEST_MISMATCH)?; } diff --git a/lms-types/Cargo.toml b/lms-types/Cargo.toml index f9a05dd372..4dc0aa5c05 100644 --- a/lms-types/Cargo.toml +++ b/lms-types/Cargo.toml @@ -9,5 +9,6 @@ edition = "2021" [dependencies] arbitrary = { workspace = true, optional = true } +constant_time_eq.workspace = true zerocopy.workspace = true zeroize.workspace = true diff --git a/lms-types/src/lib.rs b/lms-types/src/lib.rs index c11b8e45a8..9c04350625 100644 --- a/lms-types/src/lib.rs +++ b/lms-types/src/lib.rs @@ -60,7 +60,7 @@ impl LmotsAlgorithmType { pub const LmotsSha256N24W8: Self = Self::new(8); } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug)] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] #[repr(C)] pub struct LmsPublicKey { @@ -79,6 +79,28 @@ impl Default for LmsPublicKey { } } } + +impl PartialEq for LmsPublicKey { + // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. + fn eq(&self, other: &Self) -> bool { + if self.tree_type != other.tree_type { + return false; + } + if self.otstype != other.otstype { + return false; + } + if constant_time_eq::constant_time_eq(&self.id, &other.id) { + return false; + } + + let a = self.digest.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, N * 4) }; + let b = other.digest.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, N * 4) }; + constant_time_eq::constant_time_eq(aslice, bslice) + } +} + // Ensure there is no padding (required for AsBytes safety) static_assert!( size_of::>() @@ -97,7 +119,6 @@ unsafe impl FromBytes for LmsPublicKey { #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] #[derive(Copy, Clone, Debug, Zeroize)] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] #[repr(C)] pub struct LmotsSignature { #[zeroize(skip)] @@ -109,6 +130,7 @@ pub struct LmotsSignature { #[zeroize(skip)] pub y: [[U32; N]; P], } + impl Default for LmotsSignature { fn default() -> Self { Self { @@ -118,6 +140,31 @@ impl Default for LmotsSignature { } } } + +impl PartialEq for LmotsSignature { + // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. + + fn eq(&self, other: &Self) -> bool { + if self.ots_type != other.ots_type { + return false; + } + + let a = self.nonce.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, N * 4) }; + let b = other.nonce.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, N * 4) }; + if !constant_time_eq::constant_time_eq(aslice, bslice) { + return false; + } + + let a = self.y.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, N * P * 4) }; + let b = other.y.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, N * P * 4) }; + constant_time_eq::constant_time_eq(aslice, bslice) + } +} + // Ensure there is no padding (required for AsBytes safety) static_assert!( size_of::>() @@ -134,7 +181,6 @@ unsafe impl FromBytes for LmotsSignature { } #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] -#[cfg_attr(any(not(nostd), test), derive(PartialEq, Eq))] #[derive(Copy, Clone, Debug)] #[repr(C)] pub struct LmsSignature { @@ -156,6 +202,30 @@ impl Default for LmsSignature PartialEq for LmsSignature { + // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. + fn eq(&self, other: &Self) -> bool { + if self.q != other.q { + return false; + } + + if self.ots != other.ots { + return false; + } + + if self.tree_type != other.tree_type { + return false; + } + + let a = self.tree_path.as_ptr() as *const u8; + let aslice = unsafe { core::slice::from_raw_parts(a, N * H * 4) }; + let b = other.tree_path.as_ptr() as *const u8; + let bslice = unsafe { core::slice::from_raw_parts(b, N * H * 4) }; + constant_time_eq::constant_time_eq(aslice, bslice) + } +} + // Ensure there is no padding (required for AsBytes safety) static_assert!( size_of::>() diff --git a/runtime/src/drivers.rs b/runtime/src/drivers.rs index 35797d545d..4b1ef53883 100644 --- a/runtime/src/drivers.rs +++ b/runtime/src/drivers.rs @@ -297,7 +297,7 @@ impl Drivers { let latest_pcr = drivers.pcr_bank.read_pcr(RT_FW_JOURNEY_PCR); // Ensure TCI from SRAM == RT_FW_JOURNEY_PCR - if latest_pcr.ne(&latest_tci) { + if latest_pcr != latest_tci { // If latest pcr validation fails, disable attestation let result = DisableAttestationCmd::execute(drivers); if cfi_launder(result.is_ok()) { diff --git a/runtime/src/set_auth_manifest.rs b/runtime/src/set_auth_manifest.rs index b843ffe485..cf5146fad8 100644 --- a/runtime/src/set_auth_manifest.rs +++ b/runtime/src/set_auth_manifest.rs @@ -131,9 +131,11 @@ impl SetAuthManifestCmd { &auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( - auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig.r, - )) { + if cfi_launder(verify_r) + != caliptra_drivers::Array4xN( + auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig.r, + ) + { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( @@ -193,9 +195,11 @@ impl SetAuthManifestCmd { &auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( - auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r, - )) { + if cfi_launder(verify_r) + != caliptra_drivers::Array4xN( + auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r, + ) + { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( @@ -248,12 +252,14 @@ impl SetAuthManifestCmd { .ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( - auth_manifest_preamble - .vendor_image_metdata_signatures - .ecc_sig - .r, - )) { + if cfi_launder(verify_r) + != caliptra_drivers::Array4xN( + auth_manifest_preamble + .vendor_image_metdata_signatures + .ecc_sig + .r, + ) + { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( @@ -305,12 +311,14 @@ impl SetAuthManifestCmd { .ecc_sig, ) .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; - if cfi_launder(verify_r).ne(&caliptra_drivers::Array4xN( - auth_manifest_preamble - .owner_image_metdata_signatures - .ecc_sig - .r, - )) { + if cfi_launder(verify_r) + != caliptra_drivers::Array4xN( + auth_manifest_preamble + .owner_image_metdata_signatures + .ecc_sig + .r, + ) + { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( From 1e82fdc2230ca49daae016c957b1b319cf9800a7 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 9 Oct 2024 12:05:59 -0700 Subject: [PATCH 05/11] Port hardened memeq from OpenTitan C to Rust --- Cargo.lock | 3 +- cfi/lib/src/lib.rs | 2 + cfi/lib/src/secmem.rs | 212 ++++++++++++++++++++++++++++++++++++++++++ drivers/Cargo.toml | 11 ++- drivers/src/array.rs | 8 +- drivers/src/lms.rs | 8 +- lms-types/Cargo.toml | 2 +- lms-types/src/lib.rs | 56 ++++++----- 8 files changed, 257 insertions(+), 45 deletions(-) create mode 100644 cfi/lib/src/secmem.rs diff --git a/Cargo.lock b/Cargo.lock index 5537fa3ab1..4e6a67242b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -378,7 +378,6 @@ dependencies = [ "caliptra-registers", "caliptra-test", "cfg-if 1.0.0", - "constant_time_eq", "dpe", "openssl", "ufmt", @@ -709,7 +708,7 @@ name = "caliptra-lms-types" version = "0.1.0" dependencies = [ "arbitrary", - "constant_time_eq", + "caliptra-cfi-lib", "zerocopy", "zeroize", ] diff --git a/cfi/lib/src/lib.rs b/cfi/lib/src/lib.rs index 32145ce68c..716dd4bff0 100644 --- a/cfi/lib/src/lib.rs +++ b/cfi/lib/src/lib.rs @@ -13,10 +13,12 @@ extern crate core; mod cfi; mod cfi_counter; +mod secmem; mod xoshiro; pub use cfi::*; pub use cfi_counter::{CfiCounter, CfiInt}; +pub use secmem::memeq; pub use xoshiro::Xoshiro128; #[repr(C)] diff --git a/cfi/lib/src/secmem.rs b/cfi/lib/src/secmem.rs new file mode 100644 index 0000000000..262eb341fc --- /dev/null +++ b/cfi/lib/src/secmem.rs @@ -0,0 +1,212 @@ +/*++ + +Licensed under the Apache-2.0 license. + +File Name: + + secmem.rs + +Abstract: + + File contains support routines and macros for secure memory operations. + +--*/ + +use core::ptr; + +use crate::{cfi_assert_eq, cfi_assert_ne, cfi_launder}; + +// Adapted from https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_asm.h +// and https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c +// which are: +// Copyright lowRISC contributors. + +/// Values for a hardened boolean type. +/// +/// The intention is that this is used instead of ``'s #bool, where a +/// higher hamming distance is required between the truthy and the falsey value. +/// +/// The values below were chosen at random, with some specific restrictions. They +/// have a Hamming Distance of 8, and they are 11-bit values so they can be +/// materialized with a single instruction on RISC-V. They are also specifically +/// not the complement of each other. +pub type HardenedBool = u32; +pub const HARDENED_BOOL_TRUE: HardenedBool = 0x739; +pub const HARDENED_BOOL_FALSE: HardenedBool = 0x1d4; + +struct RandomOrder { + state: u32, + max: u32, +} + +/// Context for a random traversal order. +/// +/// A "random traversal order" specifies a random order to walk through some +/// buffer of length `n`, which is an important building block for +/// constant-power code. Given `n`, the random order emits integers in the +/// range `0..m`, where `m` is an implementation-defined, per-random-order +/// value greater than `n`. The order is guaranteed to visit each integer in +/// `0..n` at least once, but with some caveats: +/// - Values greater than `n` may be returned. +/// - The same value may be returned multiple times. +/// +/// Users must be mindful of these constraints when using `RandomOrder`. +/// These caveats are intended to allow for implementation flexibility, such as +/// intentionally adding decoys to the sequence. +impl RandomOrder { + /// Constructs a new, randomly-seeded traversal order, + /// running from `0` to at least `min_len`. + /// + /// This function does not take a seed as input; instead, the seed is + /// extracted, in some manner or another, from the hardware by this function. + /// + /// @param min_len The minimum length this traversal order must visit. + fn new(min_len: u32) -> RandomOrder { + RandomOrder { + state: 0, + max: min_len * 2, + } + } + + /// Returns the length of the sequence represented by `ctx`. + /// + /// This value may be greater than `min_len` specified in + /// `random_order_init()`, but the sequence is guaranteed to contain every + /// integer in `0..min_len`. + /// + /// This value represents the number of times `random_order_advance()` may be + /// called. + /// + /// @param ctx The context to query. + /// @return The length of the sequence. + fn len(&self) -> u32 { + self.max + } + + /// Returns the next element in the sequence represented by `ctx`. + /// + /// See `random_order_len()` for discovering how many times this function can + /// be called. + /// + /// @param ctx The context to advance. + /// @return The next value in the sequence. + fn advance(&mut self) -> u32 { + // TODO: The current implementation is just a skeleton, and currently just + // traverses from 0 to `min_len * 2`. + let s = self.state; + self.state += 1; + s + } +} + +#[inline(always)] +pub fn memeq(lhs: &[u32], rhs: &[u32]) -> bool { + hardened_memeq(lhs, rhs) == HARDENED_BOOL_TRUE +} + +#[inline(never)] +pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> HardenedBool { + let word_len = lhs.len(); + //assert_eq!(word_len, rhs.len()); + if word_len != rhs.len() { + return HARDENED_BOOL_FALSE; + } + + let mut order = RandomOrder::new(word_len as u32); + + let mut count = 0; + let expected_count = order.len(); + + let lhs_addr = lhs.as_ptr() as usize; + let rhs_addr = rhs.as_ptr() as usize; + + // `decoys` is a small stack array that is filled with values with a Hamming weight + // of around 16, which is the most common Hamming weight among 32-bit words. + // + // It is scratch space for us to do "extra" operations, when the number of + // iteration indices the chosen random order is different from `word_len`. + // + // These extra operations also introduce noise that an attacker must do work + // to filter, such as by applying side-channel analysis to obtain an address + // trace. + const DECOYS: usize = 8; + let decoys: [u32; DECOYS] = [0xaaaaaaaa; DECOYS]; + let decoy_addr = decoys.as_ptr() as usize; + + let mut zeros = 0; + let mut ones = u32::MAX; + + let byte_len = word_len * core::mem::size_of::(); + while count < expected_count { + // The order values themselves are in units of words, but we need `byte_idx` + // to be in units of bytes. + // + // The value obtained from `advance()` is laundered to prevent + // implementation details from leaking across procedures. + let byte_idx = cfi_launder(order.advance()) as usize * core::mem::size_of::(); + + // Prevent the compiler from reordering the loop; this ensures a + // happens-before among indices consistent with `order`. + barrier(byte_idx as u32); + + // Compute putative offsets into `src`, `dest`, and `decoys`. Some of these + // may go off the end of `src` and `dest`, but they will not be cast to + // pointers in that case. (Note that casting out-of-range addresses to + // pointers is UB.) + let ap = lhs_addr + byte_idx; + let bp = rhs_addr + byte_idx; + let decoy1 = decoy_addr + (byte_idx % core::mem::size_of_val(&decoys)); + let decoy2 = decoy_addr + + ((byte_idx + core::mem::size_of_val(&decoys) / 2) % core::mem::size_of_val(&decoys)); + + // Branchlessly select whether to do a "real" comparison or a decoy comparison, + // depending on whether we've gone off the end of the array or not. + // + // Pretty much everything needs to be laundered: we need to launder + // `byte_idx` for obvious reasons, and we need to launder the result of the + // select, so that the compiler cannot delete the resulting loads and + // stores. This is similar to having used `volatile uint32_t *`. + let av = if byte_idx < byte_len { ap } else { decoy1 } as *const u32; + let bv = if byte_idx < byte_len { bp } else { decoy2 } as *const u32; + + let a = unsafe { ptr::read_volatile(av) }; + let b = unsafe { ptr::read_volatile(bv) }; + + // Launder one of the operands so that the compiler cannot cache the result + // of the xor for use in the next operation. + // + // We launder `zeroes` so that compiler cannot learn that `zeroes` has + // strictly more bits set at the end of the loop. + zeros = cfi_launder(zeros) | (cfi_launder(a) ^ b); + + // Same as above. The compiler can cache the value of `a[offset]` but it + // has no chance to strength-reduce this operation. + ones = cfi_launder(ones) & (cfi_launder(a) ^ !b); + + // We need to launder `count` so that the SW.LOOP-COMPLETION check is not + // deleted by the compiler. + count = cfi_launder(count) + 1; + } + + if cfi_launder(zeros) == 0 { + cfi_assert_eq(ones, u32::MAX); + if ones == u32::MAX { + return HARDENED_BOOL_TRUE; + } + } + + cfi_assert_ne(ones, u32::MAX); + HARDENED_BOOL_FALSE +} + +#[allow(asm_sub_register)] // otherwise x86 complains about the no-op asm +pub fn barrier(val: u32) { + if cfg!(feature = "cfi") { + unsafe { + core::arch::asm!( + "/* {t} */", + t = in(reg) val, + ); + } + } +} diff --git a/drivers/Cargo.toml b/drivers/Cargo.toml index 5262e50a25..cd174f4c97 100644 --- a/drivers/Cargo.toml +++ b/drivers/Cargo.toml @@ -16,16 +16,21 @@ caliptra-image-types.workspace = true caliptra-lms-types.workspace = true caliptra-auth-man-types.workspace = true caliptra-registers.workspace = true -constant_time_eq.workspace = true cfg-if.workspace = true dpe = { workspace = true, optional = true } ufmt.workspace = true ureg.workspace = true zerocopy.workspace = true zeroize.workspace = true -caliptra-cfi-lib = { workspace = true, default-features = false, features = ["cfi", "cfi-counter" ] } +caliptra-cfi-lib = { workspace = true, default-features = false, features = [ + "cfi", + "cfi-counter", +] } caliptra-cfi-derive.workspace = true -caliptra-cfi-lib-git = { workspace = true, default-features = false, features = ["cfi", "cfi-counter" ], optional = true } +caliptra-cfi-lib-git = { workspace = true, default-features = false, features = [ + "cfi", + "cfi-counter", +], optional = true } caliptra-cfi-derive-git = { workspace = true, optional = true } [features] diff --git a/drivers/src/array.rs b/drivers/src/array.rs index 362a8e3c6e..5a126d90b7 100644 --- a/drivers/src/array.rs +++ b/drivers/src/array.rs @@ -41,14 +41,8 @@ impl Default for Array4xN { } impl PartialEq for Array4xN { - // secure comparison between two arrays. - // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. fn eq(&self, other: &Self) -> bool { - let a = self.0.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, B * 4) }; - let b = other.0.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, B * 4) }; - constant_time_eq::constant_time_eq(aslice, bslice) + caliptra_cfi_lib::memeq(&self.0, &other.0) } } diff --git a/drivers/src/lms.rs b/drivers/src/lms.rs index dda1203eef..86c5d59b2f 100644 --- a/drivers/src/lms.rs +++ b/drivers/src/lms.rs @@ -56,14 +56,8 @@ impl Default for HashValue { } impl PartialEq for HashValue { - // secure comparison between two arrays. - // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. fn eq(&self, other: &Self) -> bool { - let a = self.0.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, N * 4) }; - let b = other.0.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, N * 4) }; - constant_time_eq::constant_time_eq(aslice, bslice) + caliptra_cfi_lib::memeq(&self.0, &other.0) } } diff --git a/lms-types/Cargo.toml b/lms-types/Cargo.toml index 4dc0aa5c05..8b390bb54c 100644 --- a/lms-types/Cargo.toml +++ b/lms-types/Cargo.toml @@ -9,6 +9,6 @@ edition = "2021" [dependencies] arbitrary = { workspace = true, optional = true } -constant_time_eq.workspace = true +caliptra-cfi-lib.workspace = true zerocopy.workspace = true zeroize.workspace = true diff --git a/lms-types/src/lib.rs b/lms-types/src/lib.rs index 9c04350625..0240e93c59 100644 --- a/lms-types/src/lib.rs +++ b/lms-types/src/lib.rs @@ -81,7 +81,6 @@ impl Default for LmsPublicKey { } impl PartialEq for LmsPublicKey { - // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. fn eq(&self, other: &Self) -> bool { if self.tree_type != other.tree_type { return false; @@ -89,15 +88,22 @@ impl PartialEq for LmsPublicKey { if self.otstype != other.otstype { return false; } - if constant_time_eq::constant_time_eq(&self.id, &other.id) { + + // Safety: the array is fixed-size, we're just casting the entries to u32. + let a = self.id.as_ptr() as *const u32; + let aslice = unsafe { core::slice::from_raw_parts(a, 4) }; + let b = other.id.as_ptr() as *const u32; + let bslice = unsafe { core::slice::from_raw_parts(b, 4) }; + if !caliptra_cfi_lib::memeq(aslice, bslice) { return false; } - let a = self.digest.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, N * 4) }; - let b = other.digest.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, N * 4) }; - constant_time_eq::constant_time_eq(aslice, bslice) + // Safety: we're just casting the entries to native u32. + let a = self.digest.as_ptr() as *const u32; + let aslice = unsafe { core::slice::from_raw_parts(a, N) }; + let b = other.digest.as_ptr() as *const u32; + let bslice = unsafe { core::slice::from_raw_parts(b, N) }; + caliptra_cfi_lib::memeq(aslice, bslice) } } @@ -142,26 +148,26 @@ impl Default for LmotsSignature { } impl PartialEq for LmotsSignature { - // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. - fn eq(&self, other: &Self) -> bool { if self.ots_type != other.ots_type { return false; } - let a = self.nonce.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, N * 4) }; - let b = other.nonce.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, N * 4) }; - if !constant_time_eq::constant_time_eq(aslice, bslice) { + // Safety: the slice is the same, we're just casting the entries to native u32. + let a = self.nonce.as_ptr() as *const u32; + let aslice = unsafe { core::slice::from_raw_parts(a, N) }; + let b = other.nonce.as_ptr() as *const u32; + let bslice = unsafe { core::slice::from_raw_parts(b, N) }; + if !caliptra_cfi_lib::memeq(aslice, bslice) { return false; } - let a = self.y.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, N * P * 4) }; - let b = other.y.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, N * P * 4) }; - constant_time_eq::constant_time_eq(aslice, bslice) + // Safety: the array is fixed-size, we're just reinterpreting it as a single slice. + let a = self.y.as_ptr() as *const u32; + let aslice = unsafe { core::slice::from_raw_parts(a, N * P) }; + let b = other.y.as_ptr() as *const u32; + let bslice = unsafe { core::slice::from_raw_parts(b, N * P) }; + caliptra_cfi_lib::memeq(aslice, bslice) } } @@ -204,7 +210,6 @@ impl Default for LmsSignature PartialEq for LmsSignature { - // TODO: we whould make a Rust version of the OpenTitan hardened comparisons https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c. fn eq(&self, other: &Self) -> bool { if self.q != other.q { return false; @@ -218,11 +223,12 @@ impl PartialEq for LmsSignature< return false; } - let a = self.tree_path.as_ptr() as *const u8; - let aslice = unsafe { core::slice::from_raw_parts(a, N * H * 4) }; - let b = other.tree_path.as_ptr() as *const u8; - let bslice = unsafe { core::slice::from_raw_parts(b, N * H * 4) }; - constant_time_eq::constant_time_eq(aslice, bslice) + // Safety: the array is fixed-size, we're just reinterpreting it as a single slice. + let a = self.tree_path.as_ptr() as *const u32; + let aslice = unsafe { core::slice::from_raw_parts(a, N * H) }; + let b = other.tree_path.as_ptr() as *const u32; + let bslice = unsafe { core::slice::from_raw_parts(b, N * H) }; + caliptra_cfi_lib::memeq(aslice, bslice) } } From bc3d136428b58d7717d32075790289216e5e0533 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 9 Oct 2024 13:58:32 -0700 Subject: [PATCH 06/11] Remove unnecessary PartialEq and Eq --- image/types/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/image/types/src/lib.rs b/image/types/src/lib.rs index 9c8b3956ba..998c8b2f56 100644 --- a/image/types/src/lib.rs +++ b/image/types/src/lib.rs @@ -52,7 +52,7 @@ pub type ImageRevision = [u8; IMAGE_REVISION_BYTE_SIZE]; pub type ImageEccPrivKey = ImageScalar; #[repr(C)] -#[derive(AsBytes, FromBytes, Default, Debug, Copy, Clone, Eq, PartialEq, Zeroize)] +#[derive(AsBytes, FromBytes, Default, Debug, Copy, Clone, Zeroize)] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ImageEccPubKey { /// X Coordinate From 3965139c55b9355f3a84be9e06ebf0e6895ac530 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 9 Oct 2024 16:15:13 -0700 Subject: [PATCH 07/11] Secure Eq and PartialEq for image types --- Cargo.lock | 1 + api/src/mailbox.rs | 2 + builder/src/lib.rs | 20 ++-- cfi/lib/src/cfi.rs | 4 +- common/src/verifier.rs | 24 ++-- .../fmc_integration_tests/test_rtalias.rs | 4 +- image/app/src/create/mod.rs | 4 +- image/crypto/src/openssl.rs | 22 ++-- image/fake-keys/src/lib.rs | 62 +++++------ image/types/Cargo.toml | 1 + image/types/src/lib.rs | 48 +++++++- image/verify/src/verifier.rs | 104 +++++++++--------- rom/dev/src/flow/cold_reset/fw_processor.rs | 6 +- rom/dev/src/flow/update_reset.rs | 2 +- .../rom_integration_tests/test_fake_rom.rs | 2 + .../test_fmcalias_derivation.rs | 23 ++-- .../test_image_validation.rs | 84 +++++++++++--- runtime/src/info.rs | 10 +- runtime/src/set_auth_manifest.rs | 44 +++++--- .../runtime_integration_tests/test_info.rs | 24 +++- .../test_pauser_privilege_levels.rs | 5 +- .../caliptra_integration_tests/smoke_test.rs | 10 +- test/tests/fips_test_suite/fw_load.rs | 30 ++--- .../fips_test_suite/security_parameters.rs | 4 +- test/tests/fips_test_suite/services.rs | 4 +- 25 files changed, 335 insertions(+), 209 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e6a67242b..551a69cd18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -672,6 +672,7 @@ name = "caliptra-image-types" version = "0.1.0" dependencies = [ "arbitrary", + "caliptra-cfi-lib", "caliptra-error", "caliptra-lms-types", "memoffset 0.8.0", diff --git a/api/src/mailbox.rs b/api/src/mailbox.rs index 46508b0d83..696c722fc1 100644 --- a/api/src/mailbox.rs +++ b/api/src/mailbox.rs @@ -764,6 +764,8 @@ impl Response for FipsVersionResp {} // FW_INFO // No command-specific input args +// Safety: Okay to derive PartialEq and Eq because is being sent +// and we aren't doing sensitive comparisons to its contents. #[repr(C)] #[derive(Debug, AsBytes, FromBytes, PartialEq, Eq)] pub struct FwInfoResp { diff --git a/builder/src/lib.rs b/builder/src/lib.rs index 17fbe0353f..3f005a5e20 100644 --- a/builder/src/lib.rs +++ b/builder/src/lib.rs @@ -20,7 +20,7 @@ use caliptra_image_elf::ElfExecutable; use caliptra_image_gen::{ ImageGenerator, ImageGeneratorConfig, ImageGeneratorOwnerConfig, ImageGeneratorVendorConfig, }; -use caliptra_image_types::{ImageBundle, ImageRevision, RomInfo}; +use caliptra_image_types::{ImageBundle, ImageRevision, RomInfo, IMAGE_REVISION_BYTE_SIZE}; use elf::endian::LittleEndian; use nix::fcntl::FlockArg; use zerocopy::AsBytes; @@ -509,7 +509,7 @@ fn image_revision() -> io::Result { if std::env::var_os("CALIPTRA_IMAGE_NO_GIT_REVISION").is_some() { // Sometimes needed to build a consistent ROM image from different // commits. - Ok(*b"~~~~~NO_GIT_REVISION") + Ok(ImageRevision(*b"~~~~~NO_GIT_REVISION")) } else { image_revision_from_git_repo() } @@ -525,7 +525,7 @@ fn image_revision_from_str(commit_id_str: &str, is_clean: bool) -> io::Result io::Result, Ord, PartialOrd, AssertGtFail); cfi_assert_macro!(cfi_assert_lt, <, Ord, PartialOrd, AssertLtFail); cfi_assert_macro!(cfi_assert_ge, >=, Ord, PartialOrd, AssertGeFail); diff --git a/common/src/verifier.rs b/common/src/verifier.rs index e4dce69a2d..c46f172089 100644 --- a/common/src/verifier.rs +++ b/common/src/verifier.rs @@ -40,7 +40,7 @@ impl<'a, 'b> ImageVerificationEnv for &mut FirmwareImageVerificationEnv<'a, 'b> .ok_or(err)? .get(..len as usize) .ok_or(err)?; - Ok(self.sha384.digest(data)?.0) + Ok(ImageDigest(self.sha384.digest(data)?.0)) } /// ECC-384 Verification routine @@ -51,15 +51,15 @@ impl<'a, 'b> ImageVerificationEnv for &mut FirmwareImageVerificationEnv<'a, 'b> sig: &ImageEccSignature, ) -> CaliptraResult> { let pub_key = Ecc384PubKey { - x: pub_key.x.into(), - y: pub_key.y.into(), + x: pub_key.x.0.into(), + y: pub_key.y.0.into(), }; - let digest: Array4x12 = digest.into(); + let digest: Array4x12 = digest.0.into(); let sig = Ecc384Signature { - r: sig.r.into(), - s: sig.s.into(), + r: sig.r.0.into(), + s: sig.s.0.into(), }; self.ecc384.verify_r(&pub_key, &digest, &sig) @@ -72,15 +72,15 @@ impl<'a, 'b> ImageVerificationEnv for &mut FirmwareImageVerificationEnv<'a, 'b> sig: &ImageLmsSignature, ) -> CaliptraResult> { let mut message = [0u8; SHA384_DIGEST_BYTE_SIZE]; - for i in 0..digest.len() { - message[i * 4..][..4].copy_from_slice(&digest[i].to_be_bytes()); + for i in 0..digest.0.len() { + message[i * 4..][..4].copy_from_slice(&digest.0[i].to_be_bytes()); } Lms::default().verify_lms_signature_cfi(self.sha256, &message, pub_key, sig) } /// Retrieve Vendor Public Key Digest fn vendor_pub_key_digest(&self) -> ImageDigest { - self.soc_ifc.fuse_bank().vendor_pub_key_hash().into() + ImageDigest(self.soc_ifc.fuse_bank().vendor_pub_key_hash().into()) } /// Retrieve Vendor ECC Public Key Revocation Bitmask @@ -95,7 +95,7 @@ impl<'a, 'b> ImageVerificationEnv for &mut FirmwareImageVerificationEnv<'a, 'b> /// Retrieve Owner Public Key Digest from fuses fn owner_pub_key_digest_fuses(&self) -> ImageDigest { - self.soc_ifc.fuse_bank().owner_pub_key_hash().into() + ImageDigest(self.soc_ifc.fuse_bank().owner_pub_key_hash().into()) } /// Retrieve Anti-Rollback disable fuse value @@ -120,12 +120,12 @@ impl<'a, 'b> ImageVerificationEnv for &mut FirmwareImageVerificationEnv<'a, 'b> /// Get the owner public key digest saved in the dv on cold boot fn owner_pub_key_digest_dv(&self) -> ImageDigest { - self.data_vault.owner_pk_hash().into() + ImageDigest(self.data_vault.owner_pk_hash().into()) } // Get the fmc digest from the data vault on cold boot fn get_fmc_digest_dv(&self) -> ImageDigest { - self.data_vault.fmc_tci().into() + ImageDigest(self.data_vault.fmc_tci().into()) } // Get Fuse FMC Key Manifest SVN diff --git a/fmc/tests/fmc_integration_tests/test_rtalias.rs b/fmc/tests/fmc_integration_tests/test_rtalias.rs index a36abc35ab..2d161b65f3 100644 --- a/fmc/tests/fmc_integration_tests/test_rtalias.rs +++ b/fmc/tests/fmc_integration_tests/test_rtalias.rs @@ -137,7 +137,7 @@ fn test_pcr_log() { (fht.pcr_log_index as usize) * PCR_ENTRY_SIZE ); - let rt_tci1 = swap_word_bytes(&image1.manifest.runtime.digest); + let rt_tci1 = swap_word_bytes(&image1.manifest.runtime.digest.0); let manifest_digest1 = openssl::sha::sha384(image1.manifest.as_bytes()); check_pcr_log_entry( @@ -203,7 +203,7 @@ fn test_pcr_log() { (fht.pcr_log_index as usize) * PCR_ENTRY_SIZE ); - let rt_tci2 = swap_word_bytes(&image2.manifest.runtime.digest); + let rt_tci2 = swap_word_bytes(&image2.manifest.runtime.digest.0); let manifest_digest2 = openssl::sha::sha384(image2.manifest.as_bytes()); check_pcr_log_entry( diff --git a/image/app/src/create/mod.rs b/image/app/src/create/mod.rs index 32fd8028b5..7a586cd24d 100644 --- a/image/app/src/create/mod.rs +++ b/image/app/src/create/mod.rs @@ -155,7 +155,7 @@ pub(crate) fn run_cmd(args: &ArgMatches) -> anyhow::Result<()> { fmc_path, *fmc_version, *fmc_svn, - fmc_rev[..IMAGE_REVISION_BYTE_SIZE].try_into()?, + ImageRevision(fmc_rev[..IMAGE_REVISION_BYTE_SIZE].try_into()?), )?; let runtime_rev = hex::decode(runtime_rev)?; @@ -163,7 +163,7 @@ pub(crate) fn run_cmd(args: &ArgMatches) -> anyhow::Result<()> { runtime_path, *runtime_version, *runtime_svn, - runtime_rev[..IMAGE_REVISION_BYTE_SIZE].try_into()?, + ImageRevision(runtime_rev[..IMAGE_REVISION_BYTE_SIZE].try_into()?), )?; let config_dir = config_path diff --git a/image/crypto/src/openssl.rs b/image/crypto/src/openssl.rs index 1ceca85956..4dd853d33d 100644 --- a/image/crypto/src/openssl.rs +++ b/image/crypto/src/openssl.rs @@ -57,7 +57,7 @@ impl ImageGeneratorCrypto for OsslCrypto { fn sha384_digest(&self, data: &[u8]) -> anyhow::Result { let mut engine = Sha384::new(); engine.update(data); - Ok(to_hw_format(&engine.finish())) + Ok(ImageDigest(to_hw_format(&engine.finish()))) } fn ecdsa384_sign( @@ -66,10 +66,10 @@ impl ImageGeneratorCrypto for OsslCrypto { priv_key: &ImageEccPrivKey, pub_key: &ImageEccPubKey, ) -> anyhow::Result { - let priv_key: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(priv_key); - let pub_key_x: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(&pub_key.x); - let pub_key_y: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(&pub_key.y); - let digest: [u8; SHA384_DIGEST_BYTE_SIZE] = from_hw_format(digest); + let priv_key: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(&priv_key.0); + let pub_key_x: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(&pub_key.x.0); + let pub_key_y: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(&pub_key.y.0); + let digest: [u8; SHA384_DIGEST_BYTE_SIZE] = from_hw_format(&digest.0); let group = EcGroup::from_curve_name(Nid::SECP384R1)?; let mut ctx = BigNumContext::new()?; @@ -88,8 +88,8 @@ impl ImageGeneratorCrypto for OsslCrypto { let s = sig.s().to_vec_padded(ECC384_SCALAR_BYTE_SIZE as i32)?; let image_sig = ImageEccSignature { - r: to_hw_format(&r), - s: to_hw_format(&s), + r: ImageScalar(to_hw_format(&r)), + s: ImageScalar(to_hw_format(&s)), }; Ok(image_sig) } @@ -99,7 +99,7 @@ impl ImageGeneratorCrypto for OsslCrypto { digest: &ImageDigest, priv_key: &ImageLmsPrivKey, ) -> anyhow::Result { - let message: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(digest); + let message: [u8; ECC384_SCALAR_BYTE_SIZE] = from_hw_format(&digest.0); let mut nonce = [0u8; SHA192_DIGEST_BYTE_SIZE]; rand_bytes(&mut nonce)?; sign_with_lms_key::(priv_key, &message, &nonce, SUPPORTED_LMS_Q_VALUE) @@ -121,8 +121,8 @@ impl ImageGeneratorCrypto for OsslCrypto { let y = y.to_vec_padded(ECC384_SCALAR_BYTE_SIZE as i32)?; let image_key = ImageEccPubKey { - x: to_hw_format(&x), - y: to_hw_format(&y), + x: ImageScalar(to_hw_format(&x)), + y: ImageScalar(to_hw_format(&y)), }; Ok(image_key) } @@ -137,7 +137,7 @@ impl ImageGeneratorCrypto for OsslCrypto { .private_key() .to_vec_padded(ECC384_SCALAR_BYTE_SIZE as i32)?; - Ok(to_hw_format(&priv_key)) + Ok(ImageScalar(to_hw_format(&priv_key))) } } diff --git a/image/fake-keys/src/lib.rs b/image/fake-keys/src/lib.rs index 554e511d8f..f2dd8980b2 100644 --- a/image/fake-keys/src/lib.rs +++ b/image/fake-keys/src/lib.rs @@ -3,7 +3,7 @@ use caliptra_image_gen::{ImageGeneratorOwnerConfig, ImageGeneratorVendorConfig}; use caliptra_image_types::{ ImageEccPrivKey, ImageEccPubKey, ImageLmsPrivKey, ImageLmsPublicKey, ImageOwnerPrivKeys, - ImageOwnerPubKeys, ImageVendorPrivKeys, ImageVendorPubKeys, IMAGE_LMS_OTS_TYPE, + ImageOwnerPubKeys, ImageScalar, ImageVendorPrivKeys, ImageVendorPubKeys, IMAGE_LMS_OTS_TYPE, IMAGE_LMS_TREE_TYPE, }; use caliptra_lms_types::bytes_to_words_6; @@ -43,61 +43,61 @@ use zerocopy::AsBytes; /// print_private_key("OWNER_KEY", "../../target/riscv32imc-unknown-none-elf/firmware/own-priv-key.pem"); /// ``` pub const VENDOR_ECC_KEY_0_PUBLIC: ImageEccPubKey = ImageEccPubKey { - x: [ + x: ImageScalar([ 0xc69fe67f, 0x97ea3e42, 0x21a7a603, 0x6c2e070d, 0x1657327b, 0xc3f1e7c1, 0x8dccb9e4, 0xffda5c3f, 0x4db0a1c0, 0x567e0973, 0x17bf4484, 0x39696a07, - ], - y: [ + ]), + y: ImageScalar([ 0xc126b913, 0x5fc82572, 0x8f1cd403, 0x19109430, 0x994fe3e8, 0x74a8b026, 0xbe14794d, 0x27789964, 0x7735fde8, 0x328afd84, 0xcd4d4aa8, 0x72d40b42, - ], + ]), }; -pub const VENDOR_ECC_KEY_0_PRIVATE: ImageEccPrivKey = [ +pub const VENDOR_ECC_KEY_0_PRIVATE: ImageEccPrivKey = ImageScalar([ 0x29f939ea, 0x41746499, 0xd550c6fa, 0x6368b0d7, 0x61e09b4c, 0x75b21922, 0x86f96240, 0x00ea1d99, 0xace94ba6, 0x7ae89b0e, 0x3f210cf1, 0x9a45b6b5, -]; +]); pub const VENDOR_ECC_KEY_1_PUBLIC: ImageEccPubKey = ImageEccPubKey { - x: [ + x: ImageScalar([ 0xa6309750, 0xf0a05ddb, 0x956a7f86, 0x2812ec4f, 0xec454e95, 0x3b53dbfb, 0x9eb54140, 0x15ea7507, 0x084af93c, 0xb7fa33fe, 0x51811ad5, 0xe754232e, - ], - y: [ + ]), + y: ImageScalar([ 0xef5a5987, 0x7a0ce0be, 0x2621d2a9, 0x8bf3c5df, 0xaf7b3d6d, 0x97f24183, 0xa4a42038, 0x58c39b86, 0x272ef548, 0xe572b937, 0x1ecf1994, 0x1b8d4ea7, - ], + ]), }; -pub const VENDOR_ECC_KEY_1_PRIVATE: ImageEccPrivKey = [ +pub const VENDOR_ECC_KEY_1_PRIVATE: ImageEccPrivKey = ImageScalar([ 0xf2ee427b, 0x4412f46f, 0x8fb020a5, 0xc23b0154, 0xb3fcb201, 0xf93c2ee2, 0x923fd577, 0xf85320bb, 0x289eb276, 0x2b6b21d3, 0x5cdb3925, 0xa57d5043, -]; +]); pub const VENDOR_ECC_KEY_2_PUBLIC: ImageEccPubKey = ImageEccPubKey { - x: [ + x: ImageScalar([ 0xa0d25693, 0xc4251e48, 0x185615b0, 0xa6c27f6d, 0xe62c39f5, 0xa9a32f75, 0x9553226a, 0x4d1926c1, 0x7928910f, 0xb7adc1b6, 0x89996733, 0x10134881, - ], - y: [ + ]), + y: ImageScalar([ 0xbbdf72d7, 0x07c08100, 0xd54fcdad, 0xb1567bb0, 0x0522762b, 0x76b8dc4a, 0x846c175a, 0x3fbd0501, 0x9bdc8118, 0x4be5f33c, 0xbb21b41d, 0x93a8c523, - ], + ]), }; -pub const VENDOR_ECC_KEY_2_PRIVATE: ImageEccPrivKey = [ +pub const VENDOR_ECC_KEY_2_PRIVATE: ImageEccPrivKey = ImageScalar([ 0xaf72a74c, 0xfbbacc3c, 0x7ad2f9d9, 0xc969d1c9, 0x19c2d803, 0x0a53749a, 0xee730267, 0x7c11a52d, 0xee63e4c8, 0x0b5c0293, 0x28d35c27, 0x5f959aee, -]; +]); pub const VENDOR_ECC_KEY_3_PUBLIC: ImageEccPubKey = ImageEccPubKey { - x: [ + x: ImageScalar([ 0x002a82b6, 0x8e03e9a0, 0xfd3b4c14, 0xca2cb3e8, 0x14350a71, 0x0e43956d, 0x21694fb4, 0xf34485e8, 0xf0e33583, 0xf7ea142d, 0x50e16f8b, 0x0225bb95, - ], - y: [ + ]), + y: ImageScalar([ 0x5802641c, 0x7c45a4a2, 0x408e03a6, 0xa4100a92, 0x50fcc468, 0xd238cd0d, 0x449cc3e5, 0x1abc25e7, 0x0b05c426, 0x843dcd6f, 0x944ef6ff, 0xfa53ec5b, - ], + ]), }; -pub const VENDOR_ECC_KEY_3_PRIVATE: ImageEccPrivKey = [ +pub const VENDOR_ECC_KEY_3_PRIVATE: ImageEccPrivKey = ImageScalar([ 0xafbdfc7d, 0x36b54629, 0xd12c4cb5, 0x33926c30, 0x20611617, 0x86b50b23, 0x6046ff93, 0x17ea0144, 0xbc900c70, 0xb8cb36ac, 0x268b8079, 0xe3aeaaaf, -]; +]); pub const VENDOR_LMS_KEY_0_PRIVATE: ImageLmsPrivKey = ImageLmsPrivKey { tree_type: IMAGE_LMS_TREE_TYPE, @@ -224,19 +224,19 @@ pub const OWNER_LMS_KEY_PUBLIC: ImageLmsPublicKey = ImageLmsPublicKey { ]), }; pub const OWNER_ECC_KEY_PUBLIC: ImageEccPubKey = ImageEccPubKey { - x: [ + x: ImageScalar([ 0xc6f82e2b, 0xdcf3e157, 0xa162e7f3, 0x3eca35c4, 0x55ea08a9, 0x13811779, 0xb6f2646d, 0x92c817cd, 0x4094bd1a, 0xdb215f62, 0xcf36f017, 0x012d5aeb, - ], - y: [ + ]), + y: ImageScalar([ 0xa4674593, 0x6cb5a379, 0x99b08264, 0x862b2c1c, 0x517f12c6, 0x573e1f94, 0x7142291a, 0xf9624bd7, 0x2733dcdd, 0xce24ec5e, 0x961c00e3, 0x4372ba17, - ], + ]), }; -pub const OWNER_ECC_KEY_PRIVATE: ImageEccPrivKey = [ +pub const OWNER_ECC_KEY_PRIVATE: ImageEccPrivKey = ImageScalar([ 0x59fdf849, 0xe39f4256, 0x19342ed2, 0x81d28d3d, 0x45ab3219, 0x5174582c, 0xecb4e9df, 0x9cc2e991, 0xb75f88fd, 0xfa4bc6a4, 0x6b88340f, 0x05dd8890, -]; +]); pub const VENDOR_PUBLIC_KEYS: ImageVendorPubKeys = ImageVendorPubKeys { ecc_pub_keys: [ VENDOR_ECC_KEY_0_PUBLIC, diff --git a/image/types/Cargo.toml b/image/types/Cargo.toml index d1a27d45cc..0396e80b3f 100644 --- a/image/types/Cargo.toml +++ b/image/types/Cargo.toml @@ -10,6 +10,7 @@ doctest = false [dependencies] arbitrary = { workspace = true, optional = true } +caliptra-cfi-lib.workspace = true caliptra-lms-types.workspace = true memoffset.workspace = true zerocopy.workspace = true diff --git a/image/types/src/lib.rs b/image/types/src/lib.rs index 998c8b2f56..f964992d28 100644 --- a/image/types/src/lib.rs +++ b/image/types/src/lib.rs @@ -46,13 +46,51 @@ pub const IMAGE_LMS_TREE_TYPE: LmsAlgorithmType = LmsAlgorithmType::LmsSha256N24 pub const IMAGE_LMS_OTS_TYPE: LmotsAlgorithmType = LmotsAlgorithmType::LmotsSha256N24W4; pub const IMAGE_MANIFEST_BYTE_SIZE: usize = core::mem::size_of::(); -pub type ImageScalar = [u32; ECC384_SCALAR_WORD_SIZE]; -pub type ImageDigest = [u32; SHA384_DIGEST_WORD_SIZE]; -pub type ImageRevision = [u8; IMAGE_REVISION_BYTE_SIZE]; +#[repr(C)] +#[derive(AsBytes, FromBytes, Default, Debug, Clone, Copy, Zeroize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +pub struct ImageScalar(pub [u32; ECC384_SCALAR_WORD_SIZE]); +#[repr(C)] +#[derive(AsBytes, FromBytes, Default, Debug, Clone, Copy, Zeroize)] +pub struct ImageDigest(pub [u32; SHA384_DIGEST_WORD_SIZE]); + +impl PartialEq for ImageDigest { + fn eq(&self, other: &ImageScalar) -> bool { + caliptra_cfi_lib::memeq(&self.0, &other.0) + } +} + +impl PartialEq for ImageScalar { + fn eq(&self, other: &ImageDigest) -> bool { + caliptra_cfi_lib::memeq(&self.0, &other.0) + } +} + +impl PartialEq for ImageDigest { + fn eq(&self, other: &ImageDigest) -> bool { + caliptra_cfi_lib::memeq(&self.0, &other.0) + } +} + +#[repr(C)] +#[derive(AsBytes, FromBytes, Default, Debug, Clone, Copy, Zeroize)] +pub struct ImageRevision(pub [u8; IMAGE_REVISION_BYTE_SIZE]); + +impl PartialEq for ImageRevision { + fn eq(&self, other: &ImageRevision) -> bool { + // Safety: the array is fixed-size and 4-byte-aligned, we're just casting the entries to u32. + let a = self.0.as_ptr() as *const u32; + let aslice = unsafe { core::slice::from_raw_parts(a, IMAGE_REVISION_BYTE_SIZE / 4) }; + let b = other.0.as_ptr() as *const u32; + let bslice = unsafe { core::slice::from_raw_parts(b, IMAGE_REVISION_BYTE_SIZE / 4) }; + caliptra_cfi_lib::memeq(aslice, bslice) + } +} + pub type ImageEccPrivKey = ImageScalar; #[repr(C)] -#[derive(AsBytes, FromBytes, Default, Debug, Copy, Clone, Zeroize)] +#[derive(AsBytes, FromBytes, Default, Debug, Clone, Copy, Zeroize)] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ImageEccPubKey { /// X Coordinate @@ -66,7 +104,7 @@ pub type ImageLmsPublicKey = LmsPublicKey; pub type ImageLmsPrivKey = LmsPrivateKey; #[repr(C)] -#[derive(AsBytes, FromBytes, Default, Debug, Copy, Clone, Zeroize)] +#[derive(AsBytes, FromBytes, Default, Debug, Clone, Copy, Zeroize)] #[cfg_attr(test, derive(PartialEq, Eq))] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub struct ImageEccSignature { diff --git a/image/verify/src/verifier.rs b/image/verify/src/verifier.rs index 0b5193eb6e..ff39435d6b 100644 --- a/image/verify/src/verifier.rs +++ b/image/verify/src/verifier.rs @@ -24,7 +24,7 @@ use caliptra_drivers::*; use caliptra_image_types::*; use memoffset::offset_of; -const ZERO_DIGEST: ImageDigest = [0u32; SHA384_DIGEST_WORD_SIZE]; +const ZERO_DIGEST: ImageDigest = ImageDigest([0u32; SHA384_DIGEST_WORD_SIZE]); /// Header Info struct HeaderInfo<'a> { @@ -342,7 +342,7 @@ impl ImageVerifier { if cfi_launder(expected) != actual { Err(CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_PUB_KEY_DIGEST_MISMATCH)?; } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&expected, &actual); + caliptra_cfi_lib::cfi_assert_eq_12_words(&expected.0, &actual.0); } Ok(()) @@ -375,11 +375,11 @@ impl ImageVerifier { let fuses_digest = self.env.owner_pub_key_digest_fuses(); if fuses_digest == ZERO_DIGEST { - caliptra_cfi_lib::cfi_assert_eq_12_words(&fuses_digest, &ZERO_DIGEST); + caliptra_cfi_lib::cfi_assert_eq_12_words(&fuses_digest.0, &ZERO_DIGEST.0); } else if fuses_digest != actual { return Err(CaliptraError::IMAGE_VERIFIER_ERR_OWNER_PUB_KEY_DIGEST_MISMATCH); } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&fuses_digest, &actual); + caliptra_cfi_lib::cfi_assert_eq_12_words(&fuses_digest.0, &actual.0); } if cfi_launder(reason) == ResetReason::UpdateReset { @@ -387,7 +387,7 @@ impl ImageVerifier { if cfi_launder(cold_boot_digest) != actual { return Err(CaliptraError::IMAGE_VERIFIER_ERR_UPDATE_RESET_OWNER_DIGEST_FAILURE); } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&cold_boot_digest, &actual); + caliptra_cfi_lib::cfi_assert_eq_12_words(&cold_boot_digest.0, &actual.0); } } else { cfi_assert_ne(reason, ResetReason::UpdateReset); @@ -506,10 +506,10 @@ impl ImageVerifier { CaliptraError::IMAGE_VERIFIER_ERR_OWNER_ECC_VERIFY_FAILURE })?; - if cfi_launder(verify_r) != caliptra_drivers::Array4xN(sig.r) { + if cfi_launder(verify_r) != caliptra_drivers::Array4xN(sig.r.0) { Err(CaliptraError::IMAGE_VERIFIER_ERR_OWNER_ECC_SIGNATURE_INVALID)?; } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &sig.r); + caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &sig.r.0); } Ok(()) @@ -546,10 +546,10 @@ impl ImageVerifier { CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_VERIFY_FAILURE })?; - if cfi_launder(verify_r) != caliptra_drivers::Array4xN(ecc_sig.r) { + if cfi_launder(verify_r) != caliptra_drivers::Array4xN(ecc_sig.r.0) { Err(CaliptraError::IMAGE_VERIFIER_ERR_VENDOR_ECC_SIGNATURE_INVALID)?; } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &ecc_sig.r); + caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_r.0, &ecc_sig.r.0); } #[cfg(feature = "fips-test-hooks")] @@ -652,7 +652,7 @@ impl ImageVerifier { if cfi_launder(*verify_info.digest) != actual { Err(CaliptraError::IMAGE_VERIFIER_ERR_TOC_DIGEST_MISMATCH)?; } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(verify_info.digest, &actual); + caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_info.digest.0, &actual.0); } // Verify the FMC size is not zero. @@ -758,7 +758,7 @@ impl ImageVerifier { if cfi_launder(verify_info.digest) != actual { Err(CaliptraError::IMAGE_VERIFIER_ERR_FMC_DIGEST_MISMATCH)?; } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_info.digest, &actual); + caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_info.digest.0, &actual.0); } // Overflow/underflow is checked in verify_toc @@ -851,7 +851,7 @@ impl ImageVerifier { if cfi_launder(verify_info.digest) != actual { Err(CaliptraError::IMAGE_VERIFIER_ERR_RUNTIME_DIGEST_MISMATCH)?; } else { - caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_info.digest, &actual); + caliptra_cfi_lib::cfi_assert_eq_12_words(&verify_info.digest.0, &actual.0); } // Overflow/underflow is checked in verify_toc @@ -932,13 +932,15 @@ mod tests { 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef, ]; + const DUMMY_SCALAR: ImageScalar = ImageScalar(DUMMY_DATA); + const DUMMY_DIGEST: ImageDigest = ImageDigest(DUMMY_DATA); const VENDOR_ECC_PUBKEY: ImageEccPubKey = ImageEccPubKey { - x: DUMMY_DATA, - y: DUMMY_DATA, + x: DUMMY_SCALAR, + y: DUMMY_SCALAR, }; const VENDOR_ECC_SIG: ImageEccSignature = ImageEccSignature { - r: DUMMY_DATA, - s: DUMMY_DATA, + r: DUMMY_SCALAR, + s: DUMMY_SCALAR, }; fn vendor_lms_pubkey() -> ImageLmsPublicKey { ImageLmsPublicKey::default() @@ -947,12 +949,12 @@ mod tests { ImageLmsSignature::default() } const OWNER_ECC_PUBKEY: ImageEccPubKey = ImageEccPubKey { - x: DUMMY_DATA, - y: DUMMY_DATA, + x: DUMMY_SCALAR, + y: DUMMY_SCALAR, }; const OWNER_ECC_SIG: ImageEccSignature = ImageEccSignature { - r: DUMMY_DATA, - s: DUMMY_DATA, + r: DUMMY_SCALAR, + s: DUMMY_SCALAR, }; #[test] @@ -992,9 +994,9 @@ mod tests { fn test_owner_pk_digest_update_rst() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, - digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, + digest: DUMMY_DIGEST, ..Default::default() }; @@ -1009,17 +1011,17 @@ mod tests { fn test_verify_fmc_update_rst() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, - digest: DUMMY_DATA, - fmc_digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, + digest: DUMMY_DIGEST, + fmc_digest: DUMMY_DIGEST, ..Default::default() }; let mut verifier = ImageVerifier::new(test_env); let verify_info = ImageTocEntry { - digest: DUMMY_DATA, + digest: DUMMY_DIGEST, load_addr: ICCM_ORG, entry_point: ICCM_ORG, size: 100, @@ -1034,15 +1036,15 @@ mod tests { fn test_verify_fmc_mismatch_update_rst() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, - digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, + digest: DUMMY_DIGEST, ..Default::default() }; let mut verifier = ImageVerifier::new(test_env); let verify_info = ImageTocEntry { - digest: DUMMY_DATA, + digest: DUMMY_DIGEST, load_addr: ICCM_ORG, entry_point: ICCM_ORG, size: 100, @@ -1060,10 +1062,10 @@ mod tests { fn test_owner_verify_preamble_update_rst() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, - digest: DUMMY_DATA, - fmc_digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, + digest: DUMMY_DIGEST, + fmc_digest: DUMMY_DIGEST, ..Default::default() }; @@ -1122,9 +1124,9 @@ mod tests { fn test_preamble_owner_pubkey_digest() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, - digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, + digest: DUMMY_DIGEST, ..Default::default() }; let mut verifier = ImageVerifier::new(test_env); @@ -1138,8 +1140,8 @@ mod tests { fn test_preamble_vendor_pubkey() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, ..Default::default() }; let mut verifier = ImageVerifier::new(test_env); @@ -1216,8 +1218,8 @@ mod tests { fn test_header_vendor_signature_invalid() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, ..Default::default() }; let mut verifier = ImageVerifier::new(test_env); @@ -1251,8 +1253,8 @@ mod tests { fn test_header_vendor_lms_signature_invalid() { let test_env = TestEnv { lifecycle: Lifecycle::Production, - vendor_pub_key_digest: DUMMY_DATA, - owner_pub_key_digest: DUMMY_DATA, + vendor_pub_key_digest: DUMMY_DIGEST, + owner_pub_key_digest: DUMMY_DIGEST, verify_result: true, ..Default::default() }; @@ -1428,7 +1430,7 @@ mod tests { let mut verifier = ImageVerifier::new(test_env); let header = ImageHeader { toc_len: 100, - toc_digest: DUMMY_DATA, + toc_digest: DUMMY_DIGEST, ..Default::default() }; let owner_lms_pubkey = ImageLmsPublicKey::default(); @@ -1449,7 +1451,7 @@ mod tests { }; let toc_info = verifier.verify_header(&header, &header_info).unwrap(); assert_eq!(toc_info.len, 100); - assert_eq!(toc_info.digest, &DUMMY_DATA); + assert_eq!(toc_info.digest, &DUMMY_DIGEST); } #[test] @@ -1475,7 +1477,7 @@ mod tests { let mut verifier = ImageVerifier::new(test_env); let toc_info = TocInfo { len: MAX_TOC_ENTRY_COUNT, - digest: &DUMMY_DATA, + digest: &DUMMY_DIGEST, }; let result = verifier.verify_toc(&manifest, &toc_info, manifest.size); assert_eq!( @@ -1797,7 +1799,7 @@ mod tests { let test_env = TestEnv::default(); let mut verifier = ImageVerifier::new(test_env); let verify_info = ImageTocEntry { - digest: DUMMY_DATA, + digest: DUMMY_DIGEST, ..Default::default() }; let result = verifier.verify_fmc(&verify_info, ResetReason::ColdReset); @@ -1833,7 +1835,7 @@ mod tests { let test_env = TestEnv::default(); let mut verifier = ImageVerifier::new(test_env); let verify_info = ImageTocEntry { - digest: DUMMY_DATA, + digest: DUMMY_DIGEST, ..Default::default() }; let result = verifier.verify_runtime(&verify_info); @@ -1931,7 +1933,7 @@ mod tests { sig: &ImageEccSignature, ) -> CaliptraResult> { if self.verify_result { - Ok(Array4x12::from(sig.r)) + Ok(Array4x12::from(sig.r.0)) } else { Ok(Array4x12::from(&[0xFF; 48])) } diff --git a/rom/dev/src/flow/cold_reset/fw_processor.rs b/rom/dev/src/flow/cold_reset/fw_processor.rs index 8a29b44298..1290a5de88 100644 --- a/rom/dev/src/flow/cold_reset/fw_processor.rs +++ b/rom/dev/src/flow/cold_reset/fw_processor.rs @@ -511,7 +511,7 @@ impl FirmwareProcessor { info: &ImageVerificationInfo, persistent_data: &PersistentDataAccessor, ) { - data_vault.write_cold_reset_entry48(ColdResetEntry48::FmcTci, &info.fmc.digest.into()); + data_vault.write_cold_reset_entry48(ColdResetEntry48::FmcTci, &info.fmc.digest.0.into()); data_vault.write_cold_reset_entry4(ColdResetEntry4::FmcSvn, info.fmc.svn); @@ -519,7 +519,7 @@ impl FirmwareProcessor { data_vault.write_cold_reset_entry48( ColdResetEntry48::OwnerPubKeyHash, - &info.owner_pub_keys_digest.into(), + &info.owner_pub_keys_digest.0.into(), ); data_vault.write_cold_reset_entry4( @@ -534,7 +534,7 @@ impl FirmwareProcessor { info.vendor_lms_pub_key_idx.unwrap_or(u32::MAX), ); - data_vault.write_warm_reset_entry48(WarmResetEntry48::RtTci, &info.runtime.digest.into()); + data_vault.write_warm_reset_entry48(WarmResetEntry48::RtTci, &info.runtime.digest.0.into()); data_vault.write_warm_reset_entry4(WarmResetEntry4::RtSvn, info.runtime.svn); diff --git a/rom/dev/src/flow/update_reset.rs b/rom/dev/src/flow/update_reset.rs index e6a46ce848..b0b0b609b6 100644 --- a/rom/dev/src/flow/update_reset.rs +++ b/rom/dev/src/flow/update_reset.rs @@ -209,7 +209,7 @@ impl UpdateResetFlow { /// * `env` - ROM Environment /// * `info` - Image Verification Info fn populate_data_vault(data_vault: &mut DataVault, info: &ImageVerificationInfo) { - data_vault.write_warm_reset_entry48(WarmResetEntry48::RtTci, &info.runtime.digest.into()); + data_vault.write_warm_reset_entry48(WarmResetEntry48::RtTci, &info.runtime.digest.0.into()); data_vault.write_warm_reset_entry4(WarmResetEntry4::RtSvn, info.runtime.svn); diff --git a/rom/dev/tests/rom_integration_tests/test_fake_rom.rs b/rom/dev/tests/rom_integration_tests/test_fake_rom.rs index 6ef5c49f37..cc83d68308 100644 --- a/rom/dev/tests/rom_integration_tests/test_fake_rom.rs +++ b/rom/dev/tests/rom_integration_tests/test_fake_rom.rs @@ -218,9 +218,11 @@ fn test_image_verify() { // Modify the vendor public key. image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx] .x + .0 .clone_from_slice(Array4x12::from(PUB_KEY_X).0.as_slice()); image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx] .y + .0 .clone_from_slice(Array4x12::from(PUB_KEY_Y).0.as_slice()); assert_eq!( diff --git a/rom/dev/tests/rom_integration_tests/test_fmcalias_derivation.rs b/rom/dev/tests/rom_integration_tests/test_fmcalias_derivation.rs index e2cd0f096a..903ff43e8e 100644 --- a/rom/dev/tests/rom_integration_tests/test_fmcalias_derivation.rs +++ b/rom/dev/tests/rom_integration_tests/test_fmcalias_derivation.rs @@ -141,8 +141,8 @@ fn test_pcr_log() { let fuses = Fuses { anti_rollback_disable: true, lms_verify: true, - key_manifest_pk_hash: vendor_pubkey_digest, - owner_pk_hash: owner_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, + owner_pk_hash: owner_pubkey_digest.0, ..Default::default() }; let rom = caliptra_builder::build_firmware_rom(firmware::rom_from_env()).unwrap(); @@ -212,7 +212,7 @@ fn test_pcr_log() { 1, PcrLogEntryId::VendorPubKeyHash, PCR0_AND_PCR1_EXTENDED_ID, - swap_word_bytes(&vendor_pubkey_digest).as_bytes(), + swap_word_bytes(&vendor_pubkey_digest.0).as_bytes(), ); check_pcr_log_entry( @@ -220,7 +220,7 @@ fn test_pcr_log() { 2, PcrLogEntryId::OwnerPubKeyHash, PCR0_AND_PCR1_EXTENDED_ID, - swap_word_bytes(&owner_pubkey_digest).as_bytes(), + swap_word_bytes(&owner_pubkey_digest.0).as_bytes(), ); check_pcr_log_entry( @@ -228,7 +228,7 @@ fn test_pcr_log() { 3, PcrLogEntryId::FmcTci, PCR0_AND_PCR1_EXTENDED_ID, - swap_word_bytes(&image_bundle.manifest.fmc.digest).as_bytes(), + swap_word_bytes(&image_bundle.manifest.fmc.digest.0).as_bytes(), ); } @@ -246,7 +246,8 @@ fn test_pcr_log_no_owner_key_digest_fuse() { lms_verify: true, key_manifest_pk_hash: gen .vendor_pubkey_digest(&image_bundle.manifest.preamble) - .unwrap(), + .unwrap() + .0, ..Default::default() }; let rom = caliptra_builder::build_firmware_rom(firmware::rom_from_env()).unwrap(); @@ -314,7 +315,7 @@ fn test_pcr_log_no_owner_key_digest_fuse() { 2, PcrLogEntryId::OwnerPubKeyHash, PCR0_AND_PCR1_EXTENDED_ID, - swap_word_bytes(&owner_pubkey_digest).as_bytes(), + swap_word_bytes(&owner_pubkey_digest.0).as_bytes(), ); } @@ -336,8 +337,8 @@ fn test_pcr_log_fmc_fuse_svn() { let fuses = Fuses { anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, - owner_pk_hash: owner_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, + owner_pk_hash: owner_pubkey_digest.0, fmc_key_manifest_svn: FMC_FUSE_SVN, ..Default::default() }; @@ -478,8 +479,8 @@ fn test_pcr_log_across_update_reset() { let fuses = Fuses { anti_rollback_disable: false, fmc_key_manifest_svn: FMC_FUSE_SVN, - key_manifest_pk_hash: vendor_pubkey_digest, - owner_pk_hash: owner_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, + owner_pk_hash: owner_pubkey_digest.0, ..Default::default() }; let rom = caliptra_builder::build_firmware_rom(firmware::rom_from_env()).unwrap(); diff --git a/rom/dev/tests/rom_integration_tests/test_image_validation.rs b/rom/dev/tests/rom_integration_tests/test_image_validation.rs index 650c7ff6f7..3a471a151a 100644 --- a/rom/dev/tests/rom_integration_tests/test_image_validation.rs +++ b/rom/dev/tests/rom_integration_tests/test_image_validation.rs @@ -408,6 +408,7 @@ fn test_header_verify_vendor_sig_zero_ecc_pubkey() { image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx].x; image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx] .x + .0 .fill(0); assert_eq!( @@ -432,6 +433,7 @@ fn test_header_verify_vendor_sig_zero_ecc_pubkey() { ecc_pub_key_x_backup; image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx] .y + .0 .fill(0); assert_eq!( @@ -455,7 +457,14 @@ fn test_header_verify_vendor_sig_zero_ecc_signature() { // Set vendor_sig.r to zero. let vendor_sig_r_backup = image_bundle.manifest.preamble.vendor_sigs.ecc_sig.r; - image_bundle.manifest.preamble.vendor_sigs.ecc_sig.r.fill(0); + image_bundle + .manifest + .preamble + .vendor_sigs + .ecc_sig + .r + .0 + .fill(0); assert_eq!( ModelError::MailboxCmdFailed( @@ -476,7 +485,14 @@ fn test_header_verify_vendor_sig_zero_ecc_signature() { // Set vendor_sig.s to zero. image_bundle.manifest.preamble.vendor_sigs.ecc_sig.r = vendor_sig_r_backup; - image_bundle.manifest.preamble.vendor_sigs.ecc_sig.s.fill(0); + image_bundle + .manifest + .preamble + .vendor_sigs + .ecc_sig + .s + .0 + .fill(0); assert_eq!( ModelError::MailboxCmdFailed( @@ -504,9 +520,11 @@ fn test_header_verify_vendor_ecc_sig_mismatch() { image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx] .x + .0 .clone_from_slice(Array4x12::from(PUB_KEY_X).0.as_slice()); image_bundle.manifest.preamble.vendor_pub_keys.ecc_pub_keys[vendor_ecc_pub_key_idx] .y + .0 .clone_from_slice(Array4x12::from(PUB_KEY_Y).0.as_slice()); assert_eq!( @@ -535,6 +553,7 @@ fn test_header_verify_vendor_ecc_sig_mismatch() { .vendor_sigs .ecc_sig .r + .0 .clone_from_slice(Array4x12::from(SIGNATURE_R).0.as_slice()); image_bundle .manifest @@ -542,6 +561,7 @@ fn test_header_verify_vendor_ecc_sig_mismatch() { .vendor_sigs .ecc_sig .s + .0 .clone_from_slice(Array4x12::from(SIGNATURE_S).0.as_slice()); assert_eq!( @@ -848,6 +868,7 @@ fn test_header_verify_owner_ecc_sig_zero_pubkey_x() { .owner_pub_keys .ecc_pub_key .x + .0 .fill(0); let gen = ImageGenerator::new(Crypto::default()); @@ -856,7 +877,7 @@ fn test_header_verify_owner_ecc_sig_zero_pubkey_x() { .unwrap(); let fuses = caliptra_hw_model::Fuses { - owner_pk_hash: digest, + owner_pk_hash: digest.0, ..Default::default() }; @@ -903,6 +924,7 @@ fn test_header_verify_owner_ecc_sig_zero_pubkey_y() { .owner_pub_keys .ecc_pub_key .y + .0 .fill(0); let gen = ImageGenerator::new(Crypto::default()); @@ -911,7 +933,7 @@ fn test_header_verify_owner_ecc_sig_zero_pubkey_y() { .unwrap(); let fuses = caliptra_hw_model::Fuses { - owner_pk_hash: digest, + owner_pk_hash: digest.0, ..Default::default() }; @@ -957,7 +979,7 @@ fn test_header_verify_owner_ecc_sig_zero_signature_r() { .unwrap(); let fuses = caliptra_hw_model::Fuses { - owner_pk_hash: digest, + owner_pk_hash: digest.0, ..Default::default() }; @@ -976,7 +998,14 @@ fn test_header_verify_owner_ecc_sig_zero_signature_r() { .unwrap(); // Set owner_sig.r to zero. - image_bundle.manifest.preamble.owner_sigs.ecc_sig.r.fill(0); + image_bundle + .manifest + .preamble + .owner_sigs + .ecc_sig + .r + .0 + .fill(0); assert_eq!( ModelError::MailboxCmdFailed( @@ -1006,7 +1035,7 @@ fn test_header_verify_owner_ecc_sig_zero_signature_s() { .unwrap(); let fuses = caliptra_hw_model::Fuses { - owner_pk_hash: digest, + owner_pk_hash: digest.0, ..Default::default() }; @@ -1025,7 +1054,14 @@ fn test_header_verify_owner_ecc_sig_zero_signature_s() { .unwrap(); // Set owner_sig.s to zero. - image_bundle.manifest.preamble.owner_sigs.ecc_sig.s.fill(0); + image_bundle + .manifest + .preamble + .owner_sigs + .ecc_sig + .s + .0 + .fill(0); assert_eq!( ModelError::MailboxCmdFailed( @@ -1055,7 +1091,7 @@ fn test_header_verify_owner_ecc_sig_invalid_signature_r() { .unwrap(); let fuses = caliptra_hw_model::Fuses { - owner_pk_hash: digest, + owner_pk_hash: digest.0, ..Default::default() }; @@ -1074,7 +1110,14 @@ fn test_header_verify_owner_ecc_sig_invalid_signature_r() { .unwrap(); // Set an invalid owner_sig.r. - image_bundle.manifest.preamble.owner_sigs.ecc_sig.r.fill(1); + image_bundle + .manifest + .preamble + .owner_sigs + .ecc_sig + .r + .0 + .fill(1); assert_eq!( ModelError::MailboxCmdFailed( @@ -1104,7 +1147,7 @@ fn test_header_verify_owner_ecc_sig_invalid_signature_s() { .unwrap(); let fuses = caliptra_hw_model::Fuses { - owner_pk_hash: digest, + owner_pk_hash: digest.0, ..Default::default() }; @@ -1123,7 +1166,14 @@ fn test_header_verify_owner_ecc_sig_invalid_signature_s() { .unwrap(); // Set an invalid owner_sig.s. - image_bundle.manifest.preamble.owner_sigs.ecc_sig.s.fill(1); + image_bundle + .manifest + .preamble + .owner_sigs + .ecc_sig + .s + .0 + .fill(1); assert_eq!( ModelError::MailboxCmdFailed( @@ -1168,7 +1218,7 @@ fn test_toc_invalid_toc_digest() { helpers::build_hw_model_and_image_bundle(Fuses::default(), ImageOptions::default()); // Change the TOC digest. - image_bundle.manifest.header.toc_digest[0] = 0xDEADBEEF; + image_bundle.manifest.header.toc_digest.0[0] = 0xDEADBEEF; update_header(&mut image_bundle); assert_eq!( @@ -1510,7 +1560,7 @@ fn test_fmc_svn_greater_than_32() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, ..Default::default() }; @@ -1545,7 +1595,7 @@ fn test_fmc_svn_less_than_fuse_svn() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, fmc_key_manifest_svn: 0b11, // fuse svn = 2 ..Default::default() }; @@ -1759,7 +1809,7 @@ fn test_runtime_svn_greater_than_max() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, ..Default::default() }; let image_options = ImageOptions { @@ -1794,7 +1844,7 @@ fn test_runtime_svn_less_than_fuse_svn() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, runtime_svn: fuse_svn, ..Default::default() }; diff --git a/runtime/src/info.rs b/runtime/src/info.rs index de9aac2f18..d4bd15484a 100644 --- a/runtime/src/info.rs +++ b/runtime/src/info.rs @@ -39,12 +39,12 @@ impl FwInfoCmd { min_runtime_svn, fmc_manifest_svn, attestation_disabled: pdata.attestation_disabled.get().into(), - rom_revision: rom_info.revision, - fmc_revision: pdata.manifest1.fmc.revision, - runtime_revision: pdata.manifest1.runtime.revision, + rom_revision: rom_info.revision.0, + fmc_revision: pdata.manifest1.fmc.revision.0, + runtime_revision: pdata.manifest1.runtime.revision.0, rom_sha256_digest: rom_info.sha256_digest, - fmc_sha384_digest: pdata.manifest1.fmc.digest, - runtime_sha384_digest: pdata.manifest1.runtime.digest, + fmc_sha384_digest: pdata.manifest1.fmc.digest.0, + runtime_sha384_digest: pdata.manifest1.runtime.digest.0, })) } } diff --git a/runtime/src/set_auth_manifest.rs b/runtime/src/set_auth_manifest.rs index cf5146fad8..42ec940439 100644 --- a/runtime/src/set_auth_manifest.rs +++ b/runtime/src/set_auth_manifest.rs @@ -60,7 +60,7 @@ impl SetAuthManifestCmd { .ok_or(err)? .get(..len as usize) .ok_or(err)?; - Ok(sha384.digest(data)?.0) + Ok(ImageDigest(sha384.digest(data)?.0)) } fn ecc384_verify( @@ -70,15 +70,15 @@ impl SetAuthManifestCmd { sig: &ImageEccSignature, ) -> CaliptraResult> { let pub_key = Ecc384PubKey { - x: pub_key.x.into(), - y: pub_key.y.into(), + x: pub_key.x.0.into(), + y: pub_key.y.0.into(), }; - let digest: Array4x12 = digest.into(); + let digest: Array4x12 = digest.0.into(); let sig = Ecc384Signature { - r: sig.r.into(), - s: sig.s.into(), + r: sig.r.0.into(), + s: sig.s.0.into(), }; ecc384.verify_r(&pub_key, &digest, &sig) @@ -95,8 +95,8 @@ impl SetAuthManifestCmd { sig: &ImageLmsSignature, ) -> CaliptraResult> { let mut message = [0u8; SHA384_DIGEST_BYTE_SIZE]; - for i in 0..digest.len() { - message[i * 4..][..4].copy_from_slice(&digest[i].to_be_bytes()); + for i in 0..digest.0.len() { + message[i * 4..][..4].copy_from_slice(&digest.0[i].to_be_bytes()); } Lms::default().verify_lms_signature_cfi(sha256, &message, pub_key, sig) } @@ -133,14 +133,22 @@ impl SetAuthManifestCmd { .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; if cfi_launder(verify_r) != caliptra_drivers::Array4xN( - auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig.r, + auth_manifest_preamble + .vendor_pub_keys_signatures + .ecc_sig + .r + .0, ) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( &verify_r.0, - &auth_manifest_preamble.vendor_pub_keys_signatures.ecc_sig.r, + &auth_manifest_preamble + .vendor_pub_keys_signatures + .ecc_sig + .r + .0, ); } @@ -197,14 +205,14 @@ impl SetAuthManifestCmd { .map_err(|_| CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; if cfi_launder(verify_r) != caliptra_drivers::Array4xN( - auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r, + auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r.0, ) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; } else { caliptra_cfi_lib_git::cfi_assert_eq_12_words( &verify_r.0, - &auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r, + &auth_manifest_preamble.owner_pub_keys_signatures.ecc_sig.r.0, ); } @@ -257,7 +265,8 @@ impl SetAuthManifestCmd { auth_manifest_preamble .vendor_image_metdata_signatures .ecc_sig - .r, + .r + .0, ) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_VENDOR_ECC_SIGNATURE_INVALID)?; @@ -267,7 +276,8 @@ impl SetAuthManifestCmd { &auth_manifest_preamble .vendor_image_metdata_signatures .ecc_sig - .r, + .r + .0, ); } @@ -316,7 +326,8 @@ impl SetAuthManifestCmd { auth_manifest_preamble .owner_image_metdata_signatures .ecc_sig - .r, + .r + .0, ) { Err(CaliptraError::RUNTIME_AUTH_MANIFEST_OWNER_ECC_SIGNATURE_INVALID)?; @@ -326,7 +337,8 @@ impl SetAuthManifestCmd { &auth_manifest_preamble .owner_image_metdata_signatures .ecc_sig - .r, + .r + .0, ); } diff --git a/runtime/tests/runtime_integration_tests/test_info.rs b/runtime/tests/runtime_integration_tests/test_info.rs index 4b13cc6097..e4466ae6c2 100644 --- a/runtime/tests/runtime_integration_tests/test_info.rs +++ b/runtime/tests/runtime_integration_tests/test_info.rs @@ -13,7 +13,7 @@ use caliptra_common::{ }, }; use caliptra_hw_model::{BootParams, DefaultHwModel, HwModel, InitParams}; -use caliptra_image_types::RomInfo; +use caliptra_image_types::{ImageDigest, ImageRevision, RomInfo}; use core::mem::size_of; use zerocopy::{AsBytes, FromBytes}; @@ -111,12 +111,24 @@ fn test_fw_info() { assert_eq!(info.runtime_svn, 10); assert_eq!(info.min_runtime_svn, 10); // Verify revision (Commit ID) and digest of each component - assert_eq!(info.rom_revision, rom_info.revision); - assert_eq!(info.fmc_revision, image.manifest.fmc.revision); - assert_eq!(info.runtime_revision, image.manifest.runtime.revision); + assert_eq!(ImageRevision(info.rom_revision), rom_info.revision); + assert_eq!( + ImageRevision(info.fmc_revision), + image.manifest.fmc.revision + ); + assert_eq!( + ImageRevision(info.runtime_revision), + image.manifest.runtime.revision + ); assert_eq!(info.rom_sha256_digest, rom_info.sha256_digest); - assert_eq!(info.fmc_sha384_digest, image.manifest.fmc.digest); - assert_eq!(info.runtime_sha384_digest, image.manifest.runtime.digest); + assert_eq!( + ImageDigest(info.fmc_sha384_digest), + image.manifest.fmc.digest + ); + assert_eq!( + ImageDigest(info.runtime_sha384_digest), + image.manifest.runtime.digest + ); // Make image with newer SVN. let mut image_opts20 = image_opts.clone(); diff --git a/runtime/tests/runtime_integration_tests/test_pauser_privilege_levels.rs b/runtime/tests/runtime_integration_tests/test_pauser_privilege_levels.rs index f2bf0eda35..e37feff24a 100644 --- a/runtime/tests/runtime_integration_tests/test_pauser_privilege_levels.rs +++ b/runtime/tests/runtime_integration_tests/test_pauser_privilege_levels.rs @@ -15,6 +15,7 @@ use caliptra_hw_model::{BootParams, Fuses, HwModel, InitParams, SecurityState}; use caliptra_image_crypto::OsslCrypto as Crypto; use caliptra_image_elf::ElfExecutable; use caliptra_image_gen::{ImageGenerator, ImageGeneratorConfig}; +use caliptra_image_types::ImageRevision; use caliptra_runtime::{ RtBootStatus, PL0_DPE_ACTIVE_CONTEXT_THRESHOLD, PL1_DPE_ACTIVE_CONTEXT_THRESHOLD, }; @@ -495,14 +496,14 @@ fn test_pl0_unset_in_header() { &fmc_elf, opts.fmc_version as u32, opts.fmc_svn, - *b"~~~~~NO_GIT_REVISION", + ImageRevision(*b"~~~~~NO_GIT_REVISION"), ) .unwrap(), runtime: ElfExecutable::new( &app_elf, opts.app_version, opts.app_svn, - *b"~~~~~NO_GIT_REVISION", + ImageRevision(*b"~~~~~NO_GIT_REVISION"), ) .unwrap(), vendor_config: opts.vendor_config, diff --git a/test/tests/caliptra_integration_tests/smoke_test.rs b/test/tests/caliptra_integration_tests/smoke_test.rs index 0376c1dcd5..cd4ed720fa 100644 --- a/test/tests/caliptra_integration_tests/smoke_test.rs +++ b/test/tests/caliptra_integration_tests/smoke_test.rs @@ -299,7 +299,7 @@ fn smoke_test() { fwids: vec![DiceFwid { // FMC hash_alg: asn1::oid!(2, 16, 840, 1, 101, 3, 4, 2, 2), - digest: swap_word_bytes(&image.manifest.fmc.digest) + digest: swap_word_bytes(&image.manifest.fmc.digest.0) .as_bytes() .to_vec(), },], @@ -317,7 +317,7 @@ fn smoke_test() { owner_pub_key_hash: owner_pk_hash_words, owner_pub_key_hash_from_fuses: true, ecc_vendor_pub_key_index: image.manifest.preamble.vendor_ecc_pub_key_idx, - fmc_digest: image.manifest.fmc.digest, + fmc_digest: image.manifest.fmc.digest.0, fmc_svn: image.manifest.fmc.svn, // This is from the SVN in the fuses (7 bits set) fmc_fuse_svn: 7, @@ -432,7 +432,7 @@ fn smoke_test() { ); let expected_rt_alias_key = RtAliasKey::derive( &PcrRtCurrentInput { - runtime_digest: image.manifest.runtime.digest, + runtime_digest: image.manifest.runtime.digest.0, manifest: image.manifest, }, &expected_fmc_alias_key, @@ -465,7 +465,7 @@ fn smoke_test() { fwids: vec![DiceFwid { // RT hash_alg: asn1::oid!(2, 16, 840, 1, 101, 3, 4, 2, 2), - digest: swap_word_bytes(&image.manifest.runtime.digest) + digest: swap_word_bytes(&image.manifest.runtime.digest.0) .as_bytes() .to_vec(), },], @@ -615,7 +615,7 @@ fn smoke_test() { fwids: vec![DiceFwid { // FMC hash_alg: asn1::oid!(2, 16, 840, 1, 101, 3, 4, 2, 2), - digest: swap_word_bytes(&image2.manifest.runtime.digest) + digest: swap_word_bytes(&image2.manifest.runtime.digest.0) .as_bytes() .to_vec(), },], diff --git a/test/tests/fips_test_suite/fw_load.rs b/test/tests/fips_test_suite/fw_load.rs index abbdd698ec..2b8c603466 100755 --- a/test/tests/fips_test_suite/fw_load.rs +++ b/test/tests/fips_test_suite/fw_load.rs @@ -102,8 +102,8 @@ fn safe_fuses(fw_image: &ImageBundle) -> Fuses { .unwrap(); Fuses { - key_manifest_pk_hash: vendor_pubkey_digest, - owner_pk_hash: owner_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, + owner_pk_hash: owner_pubkey_digest.0, ..Default::default() } } @@ -411,7 +411,7 @@ fn fw_load_error_vendor_ecc_signature_invalid() { // Generate image let mut fw_image = build_fw_image(ImageOptions::default()); // Corrupt vendor ECC sig - fw_image.manifest.preamble.vendor_sigs.ecc_sig.r.fill(1); + fw_image.manifest.preamble.vendor_sigs.ecc_sig.r.0.fill(1); fw_load_error_flow( Some(fw_image), @@ -452,7 +452,7 @@ fn fw_load_error_owner_ecc_signature_invalid() { // Generate image let mut fw_image = build_fw_image(ImageOptions::default()); // Corrupt owner ECC sig - fw_image.manifest.preamble.owner_sigs.ecc_sig.r.fill(1); + fw_image.manifest.preamble.owner_sigs.ecc_sig.r.0.fill(1); fw_load_error_flow( Some(fw_image), @@ -492,7 +492,7 @@ fn fw_load_error_toc_digest_mismatch() { // Generate image let mut fw_image = build_fw_image(ImageOptions::default()); // Change the TOC digest. - fw_image.manifest.header.toc_digest[0] = 0xDEADBEEF; + fw_image.manifest.header.toc_digest.0[0] = 0xDEADBEEF; update_manifest(&mut fw_image, HdrDigest::Update, TocDigest::Skip); fw_load_error_flow( @@ -600,6 +600,7 @@ fn fw_load_error_owner_ecc_pub_key_invalid_arg() { .owner_pub_keys .ecc_pub_key .y + .0 .fill(0); fw_load_error_flow( @@ -614,7 +615,7 @@ fn fw_load_error_owner_ecc_signature_invalid_arg() { // Generate image let mut fw_image = build_fw_image(ImageOptions::default()); // Set owner_sig.s to zero. - fw_image.manifest.preamble.owner_sigs.ecc_sig.s.fill(0); + fw_image.manifest.preamble.owner_sigs.ecc_sig.s.0.fill(0); fw_load_error_flow( Some(fw_image), @@ -631,6 +632,7 @@ fn fw_load_error_vendor_pub_key_digest_invalid_arg() { fw_image.manifest.preamble.vendor_pub_keys.ecc_pub_keys [fw_image.manifest.preamble.vendor_ecc_pub_key_idx as usize] .x + .0 .fill(0); fw_load_error_flow( @@ -645,7 +647,7 @@ fn fw_load_error_vendor_ecc_signature_invalid_arg() { // Generate image let mut fw_image = build_fw_image(ImageOptions::default()); // Set vendor_sig.r to zero. - fw_image.manifest.preamble.vendor_sigs.ecc_sig.r.fill(0); + fw_image.manifest.preamble.vendor_sigs.ecc_sig.r.0.fill(0); fw_load_error_flow( Some(fw_image), @@ -666,6 +668,7 @@ fn fw_load_error_update_reset_owner_digest_failure() { .owner_pub_keys .ecc_pub_key .y + .0 .fill(0x1234abcd); update_fw_error_flow( @@ -803,7 +806,7 @@ fn fw_load_error_fmc_svn_greater_than_max_supported() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, ..Default::default() }; @@ -833,7 +836,7 @@ fn fw_load_error_fmc_svn_less_than_fuse() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, fmc_key_manifest_svn: 0b11, // fuse svn = 2 ..Default::default() }; @@ -922,7 +925,7 @@ fn fw_load_error_runtime_svn_greater_than_max_supported() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, ..Default::default() }; @@ -952,7 +955,7 @@ fn fw_load_error_runtime_svn_less_than_fuse() { let fuses = caliptra_hw_model::Fuses { life_cycle: DeviceLifecycle::Manufacturing, anti_rollback_disable: false, - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, runtime_svn: [0xffff_ffff, 0x7fff_ffff, 0, 0], // fuse svn = 63 ..Default::default() }; @@ -1303,7 +1306,8 @@ fn fw_load_bad_vendor_ecc_pub_key() { // Modify the pub key fw_image.manifest.preamble.vendor_pub_keys.ecc_pub_keys [fw_image.manifest.preamble.vendor_ecc_pub_key_idx as usize] - .x[0] ^= 0x1; + .x + .0[0] ^= 0x1; fw_load_bad_pub_key_flow( fw_image, @@ -1317,7 +1321,7 @@ fn fw_load_bad_owner_ecc_pub_key() { let mut fw_image = build_fw_image(ImageOptions::default()); // Modify the pub key - fw_image.manifest.preamble.owner_pub_keys.ecc_pub_key.x[0] ^= 0x1; + fw_image.manifest.preamble.owner_pub_keys.ecc_pub_key.x.0[0] ^= 0x1; fw_load_bad_pub_key_flow( fw_image, diff --git a/test/tests/fips_test_suite/security_parameters.rs b/test/tests/fips_test_suite/security_parameters.rs index 795a2649e2..9665439cf8 100755 --- a/test/tests/fips_test_suite/security_parameters.rs +++ b/test/tests/fips_test_suite/security_parameters.rs @@ -163,7 +163,7 @@ pub fn attempt_ssp_access_fw_load() { let vendor_pubkey_digest = gen.vendor_pubkey_digest(&manifest.preamble).unwrap(); let fuses = caliptra_hw_model::Fuses { //field_entropy - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, life_cycle: DeviceLifecycle::Production, ..Default::default() }; @@ -214,7 +214,7 @@ pub fn attempt_ssp_access_rt() { let vendor_pubkey_digest = gen.vendor_pubkey_digest(&manifest.preamble).unwrap(); let fuses = caliptra_hw_model::Fuses { //field_entropy - key_manifest_pk_hash: vendor_pubkey_digest, + key_manifest_pk_hash: vendor_pubkey_digest.0, life_cycle: DeviceLifecycle::Production, ..Default::default() }; diff --git a/test/tests/fips_test_suite/services.rs b/test/tests/fips_test_suite/services.rs index e05788902c..6a5aea8315 100755 --- a/test/tests/fips_test_suite/services.rs +++ b/test/tests/fips_test_suite/services.rs @@ -342,8 +342,8 @@ pub fn exec_fw_info(hw: &mut T, fw_image: &Vec) { let manifest = ImageManifest::read_from_prefix(&**fw_image).unwrap(); // Verify command-specific response data - assert_eq!(fw_info_resp.fmc_revision, manifest.fmc.revision); - assert_eq!(fw_info_resp.runtime_revision, manifest.runtime.revision); + assert_eq!(fw_info_resp.fmc_revision, manifest.fmc.revision.0); + assert_eq!(fw_info_resp.runtime_revision, manifest.runtime.revision.0); assert!(contains_some_data(&fw_info_resp.rom_revision)); assert!(contains_some_data(&fw_info_resp.rom_sha256_digest)); assert!(contains_some_data(&fw_info_resp.fmc_sha384_digest)); From ac07f10c0625c876ff0d39f11b3d1f2caa8c600c Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 10 Oct 2024 08:49:24 -0700 Subject: [PATCH 08/11] Remove constant_time_eq from main Cargo --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 6755ab5d0b..ad3a1b5fa4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -143,7 +143,6 @@ cfg-if = "1.0.0" chrono = "0.4" clap = { version = "3.2.14", default-features = false, features = ["std"] } cms = "0.2.2" -constant_time_eq = "0.3.0" convert_case = "0.6.0" dpe = { path = "dpe/dpe", default-features = false, features = ["dpe_profile_p384_sha384"] } crypto = { path = "dpe/crypto", default-features = false } From 06212352dc01536c489b8ca7ce03ca163e2e76cb Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 10 Oct 2024 11:11:18 -0700 Subject: [PATCH 09/11] Fix fake rom flow for constant time changes --- rom/dev/src/flow/fake.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/rom/dev/src/flow/fake.rs b/rom/dev/src/flow/fake.rs index 57dd203d53..5b684127b0 100644 --- a/rom/dev/src/flow/fake.rs +++ b/rom/dev/src/flow/fake.rs @@ -242,7 +242,7 @@ impl<'a, 'b> ImageVerificationEnv for &mut FakeRomImageVerificationEnv<'a, 'b> { .ok_or(err)? .get(..len as usize) .ok_or(err)?; - Ok(self.sha384.digest(data)?.0) + Ok(ImageDigest(self.sha384.digest(data)?.0)) } /// ECC-384 Verification routine @@ -254,21 +254,21 @@ impl<'a, 'b> ImageVerificationEnv for &mut FakeRomImageVerificationEnv<'a, 'b> { ) -> CaliptraResult> { if self.soc_ifc.verify_in_fake_mode() { let pub_key = Ecc384PubKey { - x: pub_key.x.into(), - y: pub_key.y.into(), + x: pub_key.x.0.into(), + y: pub_key.y.0.into(), }; - let digest: Array4x12 = digest.into(); + let digest: Array4x12 = digest.0.into(); let sig = Ecc384Signature { - r: sig.r.into(), - s: sig.s.into(), + r: sig.r.0.into(), + s: sig.s.0.into(), }; self.ecc384.verify_r(&pub_key, &digest, &sig) } else { // Mock verify, just always return success - Ok(Array4x12::from(sig.r)) + Ok(Array4x12::from(sig.r.0)) } } @@ -280,8 +280,8 @@ impl<'a, 'b> ImageVerificationEnv for &mut FakeRomImageVerificationEnv<'a, 'b> { ) -> CaliptraResult> { if self.soc_ifc.verify_in_fake_mode() { let mut message = [0u8; SHA384_DIGEST_BYTE_SIZE]; - for i in 0..digest.len() { - message[i * 4..][..4].copy_from_slice(&digest[i].to_be_bytes()); + for i in 0..digest.0.len() { + message[i * 4..][..4].copy_from_slice(&digest.0[i].to_be_bytes()); } Lms::default().verify_lms_signature_cfi(self.sha256, &message, pub_key, sig) } else { @@ -292,7 +292,7 @@ impl<'a, 'b> ImageVerificationEnv for &mut FakeRomImageVerificationEnv<'a, 'b> { /// Retrieve Vendor Public Key Digest fn vendor_pub_key_digest(&self) -> ImageDigest { - self.soc_ifc.fuse_bank().vendor_pub_key_hash().into() + ImageDigest(self.soc_ifc.fuse_bank().vendor_pub_key_hash().0.into()) } /// Retrieve Vendor ECC Public Key Revocation Bitmask @@ -307,7 +307,7 @@ impl<'a, 'b> ImageVerificationEnv for &mut FakeRomImageVerificationEnv<'a, 'b> { /// Retrieve Owner Public Key Digest from fuses fn owner_pub_key_digest_fuses(&self) -> ImageDigest { - self.soc_ifc.fuse_bank().owner_pub_key_hash().into() + ImageDigest(self.soc_ifc.fuse_bank().owner_pub_key_hash().0.into()) } /// Retrieve Anti-Rollback disable fuse value @@ -332,12 +332,12 @@ impl<'a, 'b> ImageVerificationEnv for &mut FakeRomImageVerificationEnv<'a, 'b> { /// Get the owner public key digest saved in the dv on cold boot fn owner_pub_key_digest_dv(&self) -> ImageDigest { - self.data_vault.owner_pk_hash().into() + ImageDigest(self.data_vault.owner_pk_hash().0.into()) } // Get the fmc digest from the data vault on cold boot fn get_fmc_digest_dv(&self) -> ImageDigest { - self.data_vault.fmc_tci().into() + ImageDigest(self.data_vault.fmc_tci().0.into()) } // Get Fuse FMC Key Manifest SVN From 3cf58cb51e6d7f9dc5a86ac94fabf1319c98442a Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 10 Oct 2024 14:27:08 -0700 Subject: [PATCH 10/11] Use constant-time select code from OpenTitan --- cfi/lib/src/secmem.rs | 112 +++++++++++++++++++++++++++++++----------- 1 file changed, 82 insertions(+), 30 deletions(-) diff --git a/cfi/lib/src/secmem.rs b/cfi/lib/src/secmem.rs index 262eb341fc..d822167998 100644 --- a/cfi/lib/src/secmem.rs +++ b/cfi/lib/src/secmem.rs @@ -14,7 +14,7 @@ Abstract: use core::ptr; -use crate::{cfi_assert_eq, cfi_assert_ne, cfi_launder}; +//use crate::{cfi_assert_eq, cfi_assert_ne}; // Adapted from https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_asm.h // and https://github.com/lowRISC/opentitan/blob/7a61300cf7c409fa68fd892942c1d7b58a7cd4c0/sw/device/lib/base/hardened_memory.c @@ -30,9 +30,9 @@ use crate::{cfi_assert_eq, cfi_assert_ne, cfi_launder}; /// have a Hamming Distance of 8, and they are 11-bit values so they can be /// materialized with a single instruction on RISC-V. They are also specifically /// not the complement of each other. -pub type HardenedBool = u32; -pub const HARDENED_BOOL_TRUE: HardenedBool = 0x739; -pub const HARDENED_BOOL_FALSE: HardenedBool = 0x1d4; +// pub type HardenedBool = u32; +// pub const HARDENED_BOOL_TRUE: HardenedBool = 0x739; +// pub const HARDENED_BOOL_FALSE: HardenedBool = 0x1d4; struct RandomOrder { state: u32, @@ -64,7 +64,7 @@ impl RandomOrder { fn new(min_len: u32) -> RandomOrder { RandomOrder { state: 0, - max: min_len * 2, + max: (min_len + 1).next_power_of_two(), } } @@ -93,7 +93,7 @@ impl RandomOrder { fn advance(&mut self) -> u32 { // TODO: The current implementation is just a skeleton, and currently just // traverses from 0 to `min_len * 2`. - let s = self.state; + let s = self.state ^ (self.state >> 1); self.state += 1; s } @@ -101,21 +101,49 @@ impl RandomOrder { #[inline(always)] pub fn memeq(lhs: &[u32], rhs: &[u32]) -> bool { - hardened_memeq(lhs, rhs) == HARDENED_BOOL_TRUE + hardened_memeq(lhs, rhs) == true +} + +// Performs constant-time unsigned ascending comparison. +// Returns `a < b` as a constant-time boolean. +#[inline(always)] +fn ct_slt(a: usize, b: usize) -> usize { + ct_sltz(((a & !b) | ((a ^ !b) & (a.wrapping_sub(b)))) as isize) +} + +// Performs constant-time signed comparison to zero. +// Returns whether `a < 0`, as a constant-time boolean. +// In other words, this checks if `a` is negative, i.e., its sign bit is set. +#[inline(always)] +fn ct_sltz(a: isize) -> usize { + // Proof. `a` is negative iff its MSB is set; + // arithmetic-right-shifting by bits(a)-1 smears the sign bit across all + // of `a`. + ((a as isize) >> (usize::BITS - 1)) as usize +} + +// Performs a constant-time select. +// Returns `a` if `c` is true; otherwise, returns `b`. +// This function should be used with one of the comparison functions above; do +// NOT create `c` using an `if` or `?:` operation. +#[inline(always)] +fn ct_cmov(c: usize, a: usize, b: usize) -> usize { + (launders(c) & a) | (launders(!c) & b) } #[inline(never)] -pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> HardenedBool { +pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> bool { let word_len = lhs.len(); //assert_eq!(word_len, rhs.len()); if word_len != rhs.len() { - return HARDENED_BOOL_FALSE; + return false; //HARDENED_BOOL_FALSE; } let mut order = RandomOrder::new(word_len as u32); - let mut count = 0; + let mut count: u32 = 0; let expected_count = order.len(); + //let expected_count = (word_len as u32).next_power_of_two(); let lhs_addr = lhs.as_ptr() as usize; let rhs_addr = rhs.as_ptr() as usize; @@ -143,7 +171,8 @@ pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> HardenedBool { // // The value obtained from `advance()` is laundered to prevent // implementation details from leaking across procedures. - let byte_idx = cfi_launder(order.advance()) as usize * core::mem::size_of::(); + let byte_idx = launder(order.advance()) as usize * core::mem::size_of::(); + //let byte_idx = ((count ^ (count >> 1)) * 4) as usize; // Prevent the compiler from reordering the loop; this ensures a // happens-before among indices consistent with `order`. @@ -165,9 +194,9 @@ pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> HardenedBool { // Pretty much everything needs to be laundered: we need to launder // `byte_idx` for obvious reasons, and we need to launder the result of the // select, so that the compiler cannot delete the resulting loads and - // stores. This is similar to having used `volatile uint32_t *`. - let av = if byte_idx < byte_len { ap } else { decoy1 } as *const u32; - let bv = if byte_idx < byte_len { bp } else { decoy2 } as *const u32; + // stores. This is similar to having used `volatile uint32_t *`.p + let av = ct_cmov(ct_slt(byte_idx, byte_len), ap, decoy1) as *const u32; + let bv = ct_cmov(ct_slt(byte_idx, byte_len), bp, decoy2) as *const u32; let a = unsafe { ptr::read_volatile(av) }; let b = unsafe { ptr::read_volatile(bv) }; @@ -177,36 +206,59 @@ pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> HardenedBool { // // We launder `zeroes` so that compiler cannot learn that `zeroes` has // strictly more bits set at the end of the loop. - zeros = cfi_launder(zeros) | (cfi_launder(a) ^ b); + zeros = launder(zeros) | (launder(a) ^ b); // Same as above. The compiler can cache the value of `a[offset]` but it // has no chance to strength-reduce this operation. - ones = cfi_launder(ones) & (cfi_launder(a) ^ !b); + ones = launder(ones) & (launder(a) ^ !b); // We need to launder `count` so that the SW.LOOP-COMPLETION check is not // deleted by the compiler. - count = cfi_launder(count) + 1; + count = launder(count) + 1; } - if cfi_launder(zeros) == 0 { - cfi_assert_eq(ones, u32::MAX); + if launder(zeros) == 0 { + //cfi_assert_eq(ones, u32::MAX); if ones == u32::MAX { - return HARDENED_BOOL_TRUE; + return true; } } - cfi_assert_ne(ones, u32::MAX); - HARDENED_BOOL_FALSE + //cfi_assert_ne(ones, u32::MAX); + false } #[allow(asm_sub_register)] // otherwise x86 complains about the no-op asm -pub fn barrier(val: u32) { - if cfg!(feature = "cfi") { - unsafe { - core::arch::asm!( - "/* {t} */", - t = in(reg) val, - ); - } +#[inline(always)] +fn launder(mut val: u32) -> u32 { + unsafe { + core::arch::asm!( + "/* {t} */", + t = inout(reg) val, + ); + } + val +} + +#[allow(asm_sub_register)] // otherwise x86 complains about the no-op asm +#[inline(always)] +fn launders(mut val: usize) -> usize { + unsafe { + core::arch::asm!( + "/* {t} */", + t = inout(reg) val, + ); + } + val +} + +#[allow(asm_sub_register)] // otherwise x86 complains about the no-op asm +#[inline(always)] +fn barrier(val: u32) { + unsafe { + core::arch::asm!( + "/* {t} */", + t = in(reg) val, + ); } } From c50153fd52a1c5bd19895cbe9b549834afe88485 Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Thu, 10 Oct 2024 14:36:17 -0700 Subject: [PATCH 11/11] Launder everything we were supposed to --- cfi/lib/src/secmem.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cfi/lib/src/secmem.rs b/cfi/lib/src/secmem.rs index d822167998..c569693039 100644 --- a/cfi/lib/src/secmem.rs +++ b/cfi/lib/src/secmem.rs @@ -91,8 +91,6 @@ impl RandomOrder { /// @param ctx The context to advance. /// @return The next value in the sequence. fn advance(&mut self) -> u32 { - // TODO: The current implementation is just a skeleton, and currently just - // traverses from 0 to `min_len * 2`. let s = self.state ^ (self.state >> 1); self.state += 1; s @@ -195,8 +193,8 @@ pub fn hardened_memeq(lhs: &[u32], rhs: &[u32]) -> bool { // `byte_idx` for obvious reasons, and we need to launder the result of the // select, so that the compiler cannot delete the resulting loads and // stores. This is similar to having used `volatile uint32_t *`.p - let av = ct_cmov(ct_slt(byte_idx, byte_len), ap, decoy1) as *const u32; - let bv = ct_cmov(ct_slt(byte_idx, byte_len), bp, decoy2) as *const u32; + let av = launders(ct_cmov(ct_slt(launders(byte_idx), byte_len), ap, decoy1)) as *const u32; + let bv = launders(ct_cmov(ct_slt(launders(byte_idx), byte_len), bp, decoy2)) as *const u32; let a = unsafe { ptr::read_volatile(av) }; let b = unsafe { ptr::read_volatile(bv) };