Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion openhcl/underhill_attestation/src/jwt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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 {
Comment thread
smalis-msft marked this conversation as resolved.
Err(CertificateChainValidationError::CertChainSubjectIssuerMismatch)?
}
Expand Down
2 changes: 1 addition & 1 deletion support/crypto/src/x509/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool, X509Error> {
self.0.issued(&subject.0)
}

Expand Down
10 changes: 7 additions & 3 deletions support/crypto/src/x509/ossl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool, X509Error> {
// `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<Vec<u8>, X509Error> {
Expand Down
67 changes: 65 additions & 2 deletions support/crypto/src/x509/symcrypt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool, X509Error> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this some set of standard checks? Why isn't this logic in symcrypt itself?

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::<KeyUsage>()
.map_err(|e| der_err(e, "parsing KeyUsage extension"))?;
if let Some((_crit, ku)) = ku
&& !ku.key_cert_sign()
{
return Ok(false);
}
Comment thread
smalis-msft marked this conversation as resolved.

// If the subject carries an AuthorityKeyIdentifier, validate its
// populated fields against this certificate (the candidate issuer).
let akid = subject_tbs
.get_extension::<AuthorityKeyIdentifier>()
.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::<SubjectKeyIdentifier>()
.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<Vec<u8>, X509Error> {
Expand Down
Loading