From 48ca02921133f94e3aba31e8bdeecd2d87a13ae0 Mon Sep 17 00:00:00 2001 From: Carl Lundin Date: Tue, 17 Dec 2024 15:02:21 -0800 Subject: [PATCH] Remove IS_CA API surface. --- .github/workflows/ci.yml | 1 + dpe/Cargo.toml | 1 - dpe/src/commands/certify_key.rs | 134 +------------------------- dpe/src/commands/derive_context.rs | 2 - dpe/src/support.rs | 12 --- simulator/src/main.rs | 5 - verification/sim/transport.go | 3 - verification/testing/certifyKey.go | 8 +- verification/testing/negativeCases.go | 14 --- 9 files changed, 5 insertions(+), 175 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aaa97be2..25bf1f59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,7 @@ on: pull_request: branches: - main + - feature/gh-issue-caliptra-sw-1807 # TODO(clundin): Remove once feature is complete. jobs: ubuntu: diff --git a/dpe/Cargo.toml b/dpe/Cargo.toml index 4fad35d0..0329a1e4 100644 --- a/dpe/Cargo.toml +++ b/dpe/Cargo.toml @@ -20,7 +20,6 @@ disable_csr = [] disable_is_symmetric = [] disable_internal_info = [] disable_internal_dice = [] -disable_is_ca = [] disable_retain_parent_context = [] no-cfi = ["crypto/no-cfi"] diff --git a/dpe/src/commands/certify_key.rs b/dpe/src/commands/certify_key.rs index 21b78991..9cc0a4c6 100644 --- a/dpe/src/commands/certify_key.rs +++ b/dpe/src/commands/certify_key.rs @@ -33,9 +33,7 @@ use platform::{Platform, PlatformError, MAX_KEY_IDENTIFIER_SIZE}; pub struct CertifyKeyFlags(u32); bitflags! { - impl CertifyKeyFlags: u32 { - const IS_CA = 1u32 << 30; - } + impl CertifyKeyFlags: u32 {} } #[repr(C)] @@ -58,10 +56,6 @@ pub struct CertifyKeyCmd { impl CertifyKeyCmd { pub const FORMAT_X509: u32 = 0; pub const FORMAT_CSR: u32 = 1; - - const fn uses_is_ca(&self) -> bool { - self.flags.contains(CertifyKeyFlags::IS_CA) - } } impl CommandExecution for CertifyKeyCmd { @@ -75,13 +69,6 @@ impl CommandExecution for CertifyKeyCmd { let idx = dpe.get_active_context_pos(&self.handle, locality)?; let context = &dpe.contexts[idx]; - if self.uses_is_ca() && !dpe.support.is_ca() { - return Err(DpeErrorCode::ArgumentNotSupported); - } - if self.uses_is_ca() && !context.allow_ca() { - return Err(DpeErrorCode::InvalidArgument); - } - if self.format == Self::FORMAT_X509 { if !dpe.support.x509() { return Err(DpeErrorCode::ArgumentNotSupported); @@ -100,8 +87,6 @@ impl CommandExecution for CertifyKeyCmd { cfg_if! { if #[cfg(not(feature = "no-cfi"))] { - cfi_assert!(!self.uses_is_ca() || dpe.support.is_ca()); - cfi_assert!(!self.uses_is_ca() || context.allow_ca()); cfi_assert!(self.format != Self::FORMAT_X509 || dpe.support.x509()); cfi_assert!(self.format != Self::FORMAT_X509 || context.allow_x509()); cfi_assert!(self.format != Self::FORMAT_CSR || dpe.support.csr()); @@ -170,7 +155,7 @@ impl CommandExecution for CertifyKeyCmd { let measurements = MeasurementData { label: &self.label, tci_nodes: &nodes[..tcb_count], - is_ca: self.uses_is_ca(), + is_ca: false, supports_recursive: dpe.support.recursive(), subject_key_identifier, authority_key_identifier, @@ -401,120 +386,6 @@ mod tests { }; } - #[test] - fn test_is_ca() { - CfiCounter::reset_for_test(); - let mut env = DpeEnv:: { - crypto: OpensslCrypto::new(), - platform: DefaultPlatform, - }; - let mut dpe = DpeInstance::new(&mut env, Support::X509 | Support::IS_CA).unwrap(); - - let init_resp = match InitCtxCmd::new_use_default() - .execute(&mut dpe, &mut env, TEST_LOCALITIES[0]) - .unwrap() - { - Response::InitCtx(resp) => resp, - _ => panic!("Incorrect return type."), - }; - let certify_cmd_ca = CertifyKeyCmd { - handle: init_resp.handle, - flags: CertifyKeyFlags::IS_CA, - label: [0; DPE_PROFILE.get_hash_size()], - format: CertifyKeyCmd::FORMAT_X509, - }; - - let certify_resp_ca = match certify_cmd_ca - .execute(&mut dpe, &mut env, TEST_LOCALITIES[0]) - .unwrap() - { - Response::CertifyKey(resp) => resp, - _ => panic!("Wrong response type."), - }; - - let mut parser = X509CertificateParser::new().with_deep_parse_extensions(true); - match parser.parse(&certify_resp_ca.cert[..certify_resp_ca.cert_size.try_into().unwrap()]) { - Ok((_, cert)) => { - match cert.basic_constraints() { - Ok(Some(basic_constraints)) => { - assert!(basic_constraints.value.ca); - } - Ok(None) => panic!("basic constraints extension not found"), - Err(_) => panic!("multiple basic constraints extensions found"), - } - - let pub_key = &cert.tbs_certificate.subject_pki.subject_public_key.data; - let mut hasher = match DPE_PROFILE { - DpeProfile::P256Sha256 => Hasher::new(MessageDigest::sha256()).unwrap(), - DpeProfile::P384Sha384 => Hasher::new(MessageDigest::sha384()).unwrap(), - }; - hasher.update(pub_key).unwrap(); - let expected_ski: &[u8] = &hasher.finish().unwrap(); - match cert.get_extension_unique(&oid!(2.5.29 .14)) { - Ok(Some(subject_key_identifier_ext)) => { - if let ParsedExtension::SubjectKeyIdentifier(key_identifier) = - subject_key_identifier_ext.parsed_extension() - { - assert_eq!(key_identifier.0, &expected_ski[..MAX_KEY_IDENTIFIER_SIZE]); - } else { - panic!("Extension has wrong type"); - } - } - Ok(None) => panic!("subject key identifier extension not found"), - Err(_) => panic!("multiple subject key identifier extensions found"), - } - - let mut expected_aki = [0u8; MAX_KEY_IDENTIFIER_SIZE]; - env.platform - .get_issuer_key_identifier(&mut expected_aki) - .unwrap(); - match cert.get_extension_unique(&oid!(2.5.29 .35)) { - Ok(Some(extension)) => { - if let ParsedExtension::AuthorityKeyIdentifier(aki) = - extension.parsed_extension() - { - let key_identifier = aki.key_identifier.clone().unwrap(); - assert_eq!(&key_identifier.0, &expected_aki,); - } else { - panic!("Extension has wrong type"); - } - } - Ok(None) => panic!("authority key identifier extension not found"), - Err(_) => panic!("multiple authority key identifier extensions found"), - } - } - Err(e) => panic!("x509 parsing failed: {:?}", e), - }; - - let certify_cmd_non_ca = CertifyKeyCmd { - handle: init_resp.handle, - flags: CertifyKeyFlags::empty(), - label: [0; DPE_PROFILE.get_hash_size()], - format: CertifyKeyCmd::FORMAT_X509, - }; - - let certify_resp_non_ca = match certify_cmd_non_ca - .execute(&mut dpe, &mut env, TEST_LOCALITIES[0]) - .unwrap() - { - Response::CertifyKey(resp) => resp, - _ => panic!("Wrong response type."), - }; - - match parser - .parse(&certify_resp_non_ca.cert[..certify_resp_non_ca.cert_size.try_into().unwrap()]) - { - Ok((_, cert)) => match cert.basic_constraints() { - Ok(Some(basic_constraints)) => { - assert!(!basic_constraints.value.ca); - } - Ok(None) => panic!("basic constraints extension not found"), - Err(_) => panic!("multiple basic constraints extensions found"), - }, - Err(e) => panic!("x509 parsing failed: {:?}", e), - }; - } - #[test] fn test_certify_key_csr() { CfiCounter::reset_for_test(); @@ -710,7 +581,6 @@ mod tests { for extension in &extension_req.extensions { match extension.parsed_extension() { ParsedExtension::BasicConstraints(basic_constraints) => { - // IS_CA not set so this will be false assert!(!basic_constraints.ca); } ParsedExtension::KeyUsage(key_usage) => { diff --git a/dpe/src/commands/derive_context.rs b/dpe/src/commands/derive_context.rs index 1d7ec874..5dc9e356 100644 --- a/dpe/src/commands/derive_context.rs +++ b/dpe/src/commands/derive_context.rs @@ -195,7 +195,6 @@ impl CommandExecution for DeriveContextCmd { if (!dpe.support.internal_info() && self.uses_internal_info_input()) || (!dpe.support.internal_dice() && self.uses_internal_dice_input()) || (!dpe.support.retain_parent_context() && self.retains_parent()) - || (!dpe.support.is_ca() && self.allows_ca()) || (!dpe.support.x509() && self.allows_x509()) || (!dpe.support.recursive() && self.is_recursive()) { @@ -225,7 +224,6 @@ impl CommandExecution for DeriveContextCmd { cfi_assert!(dpe.support.internal_info() || !self.uses_internal_info_input()); cfi_assert!(dpe.support.internal_dice() || !self.uses_internal_dice_input()); cfi_assert!(dpe.support.retain_parent_context() || !self.retains_parent()); - cfi_assert!(dpe.support.is_ca() || !self.allows_ca()); cfi_assert!(dpe.support.x509() || !self.allows_x509()); cfi_assert!(dpe.contexts[parent_idx].allow_ca() || !self.allows_ca()); cfi_assert!(dpe.contexts[parent_idx].allow_x509() || !self.allows_x509()); diff --git a/dpe/src/support.rs b/dpe/src/support.rs index b06e8697..938f9881 100644 --- a/dpe/src/support.rs +++ b/dpe/src/support.rs @@ -18,7 +18,6 @@ bitflags! { const IS_SYMMETRIC = 1u32 << 24; const INTERNAL_INFO = 1u32 << 22; const INTERNAL_DICE = 1u32 << 21; - const IS_CA = 1u32 << 20; const RETAIN_PARENT_CONTEXT = 1u32 << 19; } } @@ -51,9 +50,6 @@ impl Support { pub fn internal_dice(&self) -> bool { self.contains(Support::INTERNAL_DICE) } - pub fn is_ca(&self) -> bool { - self.contains(Support::IS_CA) - } pub fn retain_parent_context(&self) -> bool { self.contains(Support::RETAIN_PARENT_CONTEXT) } @@ -98,10 +94,6 @@ impl Support { { support.insert(Support::INTERNAL_DICE); } - #[cfg(feature = "disable_is_ca")] - { - support.insert(Support::IS_CA); - } #[cfg(feature = "disable_retain_parent_context")] { support.insert(Support::RETAIN_PARENT_CONTEXT); @@ -152,9 +144,6 @@ pub mod test { // Supports internal DICE. let flags = Support::INTERNAL_DICE.bits(); assert_eq!(flags, 1 << 21); - // Supports is ca. - let flags = Support::IS_CA.bits(); - assert_eq!(flags, 1 << 20); let flags = Support::RETAIN_PARENT_CONTEXT.bits(); assert_eq!(flags, 1 << 19); // Supports a couple combos. @@ -185,7 +174,6 @@ pub mod test { | (1 << 24) | (1 << 22) | (1 << 21) - | (1 << 20) | (1 << 19) ); } diff --git a/simulator/src/main.rs b/simulator/src/main.rs index 919d7d0c..851a48cf 100644 --- a/simulator/src/main.rs +++ b/simulator/src/main.rs @@ -100,10 +100,6 @@ struct Args { #[arg(long)] supports_csr: bool, - // Supports the CertifyKey IS_CA flag - #[arg(long)] - supports_is_ca: bool, - /// Supports symmetric derivation. #[arg(long)] supports_is_symmetric: bool, @@ -159,7 +155,6 @@ fn main() -> std::io::Result<()> { support.set(Support::ROTATE_CONTEXT, args.supports_rotate_context); support.set(Support::INTERNAL_DICE, args.supports_internal_dice); support.set(Support::INTERNAL_INFO, args.supports_internal_info); - support.set(Support::IS_CA, args.supports_is_ca); support.set(Support::IS_SYMMETRIC, args.supports_is_symmetric); support.set( Support::RETAIN_PARENT_CONTEXT, diff --git a/verification/sim/transport.go b/verification/sim/transport.go index 938efcd7..c08cc42b 100644 --- a/verification/sim/transport.go +++ b/verification/sim/transport.go @@ -73,9 +73,6 @@ func (s *DpeSimulator) PowerOn() error { if s.supports.Csr { args = append(args, "--supports-csr") } - if s.supports.IsCA { - args = append(args, "--supports-is-ca") - } if s.supports.IsSymmetric { args = append(args, "--supports-is-symmetric") } diff --git a/verification/testing/certifyKey.go b/verification/testing/certifyKey.go index 7d0a3208..4be0fbab 100644 --- a/verification/testing/certifyKey.go +++ b/verification/testing/certifyKey.go @@ -436,8 +436,6 @@ func checkCertifyKeyAuthorityKeyIdentifierExtension(t *testing.T, extensions []p // Validates basic constraints in certificate returned by CertifyKey command // against the flag set for input parameter. // The BasicConstraints extension MUST be included -// If CertifyKey AddIsCA is set, IsCA MUST be set to true. -// If CertifyKey AddIsCA is NOT set, IsCA MUST be set to false func checkCertifyKeyBasicConstraints(t *testing.T, extensions []pkix.Extension, flags client.CertifyKeyFlags) { t.Helper() @@ -448,10 +446,8 @@ func checkCertifyKeyBasicConstraints(t *testing.T, extensions []pkix.Extension, if err != nil { t.Error(err) } - - flagIsCA := client.CertifyAddIsCA&flags != 0 - if flagIsCA != bc.IsCA { - t.Errorf("[ERROR]: ADD_IS_CA is set to %v but the basic constraint IsCA is set to %v", flagIsCA, bc.IsCA) + if bc.IsCA { + t.Errorf("[ERROR]: basic constraint IsCA is set to %v but it must always be false for CertifyKey.", bc.IsCA) } } diff --git a/verification/testing/negativeCases.go b/verification/testing/negativeCases.go index 08956cc9..5d9d5422 100644 --- a/verification/testing/negativeCases.go +++ b/verification/testing/negativeCases.go @@ -165,13 +165,6 @@ func TestUnsupportedCommandFlag(d client.TestDPEInstance, c client.DPEClient, t t.Errorf("[ERROR]: Incorrect error type. Simulation is not supported by DPE, InitializeContext supported by DPE, should return %q, but returned %q", client.StatusArgumentNotSupported, err) } - // Check whether error is returned since CA certificate request is unsupported by DPE profile - if _, err := c.CertifyKey(handle, make([]byte, digestLen), client.CertifyKeyX509, client.CertifyAddIsCA); err == nil { - t.Errorf("[ERROR]: IS_CA is not supported by DPE, CertifyKey should return %q, but returned no error", client.StatusArgumentNotSupported) - } else if !errors.Is(err, client.StatusArgumentNotSupported) { - t.Errorf("[ERROR]: Incorrect error type. IS_CA is not supported by DPE, CertifyKey should return %q, but returned %q", client.StatusArgumentNotSupported, err) - } - // Check whether error is returned since CSR format is unsupported by DPE profile if _, err := c.CertifyKey(handle, make([]byte, digestLen), client.CertifyKeyCsr, 0); err == nil { t.Errorf("[ERROR]: CSR format is not supported by DPE, CertifyKey should return %q, but returned no error", client.StatusArgumentNotSupported) @@ -207,13 +200,6 @@ func TestUnsupportedCommandFlag(d client.TestDPEInstance, c client.DPEClient, t t.Errorf("[ERROR]: Incorrect error type. InternalDice is not supported by DPE, DeriveContext should return %q, but returned %q", client.StatusArgumentNotSupported, err) } - // Check whether error is returned since InternalInfo usage is unsupported by DPE profile - if _, err := c.DeriveContext(handle, make([]byte, digestLen), client.DeriveContextFlags(client.InputAllowCA), 0, 0); err == nil { - t.Errorf("[ERROR]:IS_CA is not supported by DPE, DeriveContext should return %q, but returned no error", client.StatusArgumentNotSupported) - } else if !errors.Is(err, client.StatusArgumentNotSupported) { - t.Errorf("[ERROR]: Incorrect error type. IS_CA is not supported by DPE, DeriveContext should return %q, but returned %q", client.StatusArgumentNotSupported, err) - } - // Check whether error is returned since InternalDice usgae is unsupported by DPE profile if _, err := c.DeriveContext(handle, make([]byte, digestLen), client.DeriveContextFlags(client.InputAllowX509), 0, 0); err == nil { t.Errorf("[ERROR]:X509 is not supported by DPE, DeriveContext should return %q, but returned no error", client.StatusArgumentNotSupported)