Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove IS_CA API surface. #366

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Remove IS_CA API surface. #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ on:
pull_request:
branches:
- main
- feature/gh-issue-caliptra-sw-1807 # TODO(clundin): Remove once feature is complete.

jobs:
ubuntu:
Expand Down
1 change: 0 additions & 1 deletion dpe/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down
134 changes: 2 additions & 132 deletions dpe/src/commands/certify_key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,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)]
Expand All @@ -42,10 +40,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 {
Expand All @@ -59,13 +53,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);
Expand All @@ -84,8 +71,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());
Expand Down Expand Up @@ -154,7 +139,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,
Expand Down Expand Up @@ -385,120 +370,6 @@ mod tests {
};
}

#[test]
fn test_is_ca() {
CfiCounter::reset_for_test();
let mut env = DpeEnv::<TestTypes> {
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();
Expand Down Expand Up @@ -694,7 +565,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) => {
Expand Down
2 changes: 0 additions & 2 deletions dpe/src/commands/derive_context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,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())
{
Expand Down Expand Up @@ -209,7 +208,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());
Expand Down
12 changes: 0 additions & 12 deletions dpe/src/support.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -185,7 +174,6 @@ pub mod test {
| (1 << 24)
| (1 << 22)
| (1 << 21)
| (1 << 20)
| (1 << 19)
);
}
Expand Down
5 changes: 0 additions & 5 deletions simulator/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions verification/sim/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
8 changes: 2 additions & 6 deletions verification/testing/certifyKey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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)
}
}

Expand Down
14 changes: 0 additions & 14 deletions verification/testing/negativeCases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading