diff --git a/openhcl/underhill_attestation/src/jwt.rs b/openhcl/underhill_attestation/src/jwt.rs index 4c51faa61c..8403c7e219 100644 --- a/openhcl/underhill_attestation/src/jwt.rs +++ b/openhcl/underhill_attestation/src/jwt.rs @@ -58,6 +58,8 @@ pub(crate) enum CertificateChainValidationError { VerifyChildSignatureWithParentPublicKey(#[source] crypto::x509::X509Error), #[error("cert chain validation failed -- signature mismatch")] CertChainSignatureMismatch, + #[error("failed to check if the certificate was issued by the issuer")] + CheckCertificateIssuedByIssuer(#[source] crypto::x509::X509Error), #[error("cert chain validation failed -- subject and issuer mismatch")] CertChainSubjectIssuerMismatch, } @@ -251,7 +253,9 @@ fn validate_cert_chain( Err(CertificateChainValidationError::CertChainSignatureMismatch)? } - let issued = parent.issued(child); + let issued = parent + .issued(child) + .map_err(CertificateChainValidationError::CheckCertificateIssuedByIssuer)?; if !issued { Err(CertificateChainValidationError::CertChainSubjectIssuerMismatch)? } diff --git a/support/crypto/src/x509/mod.rs b/support/crypto/src/x509/mod.rs index 659f9b358a..524b417a99 100644 --- a/support/crypto/src/x509/mod.rs +++ b/support/crypto/src/x509/mod.rs @@ -44,7 +44,7 @@ impl X509Certificate { } /// Check if this certificate (acting as issuer) issued `subject`. - pub fn issued(&self, subject: &X509Certificate) -> bool { + pub fn issued(&self, subject: &X509Certificate) -> Result { self.0.issued(&subject.0) } diff --git a/support/crypto/src/x509/ossl.rs b/support/crypto/src/x509/ossl.rs index 0d0d6b65e0..8bb672e280 100644 --- a/support/crypto/src/x509/ossl.rs +++ b/support/crypto/src/x509/ossl.rs @@ -33,9 +33,13 @@ impl X509CertificateInner { .map_err(|e| err(e, "verifying certificate signature")) } - pub fn issued(&self, subject: &X509CertificateInner) -> bool { - let result = self.0.issued(&subject.0); - result == openssl::x509::X509VerifyResult::OK + pub fn issued(&self, subject: &X509CertificateInner) -> Result { + // `X509_check_issued` only performs deterministic comparisons on + // already-parsed data (name, AKID/SKID, serial, KeyUsage) and cannot + // fail with internal errors. Per the OpenSSL docs, every non-OK + // result is an `X509_V_ERR*` constant "indicating why the issuer + // does not match" — i.e., a legitimate `Ok(false)`. + Ok(self.0.issued(&subject.0) == openssl::x509::X509VerifyResult::OK) } pub fn to_der(&self) -> Result, X509Error> { diff --git a/support/crypto/src/x509/symcrypt.rs b/support/crypto/src/x509/symcrypt.rs index 79f73f1be0..74ef7096c7 100644 --- a/support/crypto/src/x509/symcrypt.rs +++ b/support/crypto/src/x509/symcrypt.rs @@ -72,8 +72,71 @@ impl X509CertificateInner { Ok(true) } - pub fn issued(&self, subject: &X509CertificateInner) -> bool { - self.0.tbs_certificate().subject() == subject.0.tbs_certificate().issuer() + pub fn issued(&self, subject: &X509CertificateInner) -> Result { + use x509_cert::ext::pkix::AuthorityKeyIdentifier; + use x509_cert::ext::pkix::KeyUsage; + use x509_cert::ext::pkix::SubjectKeyIdentifier; + use x509_cert::ext::pkix::name::GeneralName; + + let issuer_tbs = self.0.tbs_certificate(); + let subject_tbs = subject.0.tbs_certificate(); + + // The subject's issuer name must match the issuer's subject name. + if subject_tbs.issuer() != issuer_tbs.subject() { + return Ok(false); + } + + // If this certificate has a KeyUsage extension, it must permit + // signing other certificates. + let ku = issuer_tbs + .get_extension::() + .map_err(|e| der_err(e, "parsing KeyUsage extension"))?; + if let Some((_crit, ku)) = ku + && !ku.key_cert_sign() + { + return Ok(false); + } + + // If the subject carries an AuthorityKeyIdentifier, validate its + // populated fields against this certificate (the candidate issuer). + let akid = subject_tbs + .get_extension::() + .map_err(|e| der_err(e, "parsing AuthorityKeyIdentifier extension"))?; + if let Some((_crit, akid)) = akid { + if let Some(akid_key_id) = &akid.key_identifier { + let skid = issuer_tbs + .get_extension::() + .map_err(|e| der_err(e, "parsing SubjectKeyIdentifier extension"))?; + match skid { + Some((_crit, ski)) => { + if akid_key_id != &ski.0 { + return Ok(false); + } + } + None => return Ok(false), + } + } + if let Some(akid_serial) = &akid.authority_cert_serial_number { + if akid_serial != issuer_tbs.serial_number() { + return Ok(false); + } + } + if let Some(gens) = &akid.authority_cert_issuer { + let mut has_dn = false; + let has_matching_dn = gens.iter().any(|g| match g { + GeneralName::DirectoryName(dn) => { + has_dn = true; + dn == issuer_tbs.subject() + } + _ => false, + }); + if has_dn && !has_matching_dn { + return Ok(false); + } + } + } + + Ok(true) } pub fn to_der(&self) -> Result, X509Error> {