Skip to content

Commit

Permalink
feat: check CRLs during attestation
Browse files Browse the repository at this point in the history 
Signed-off-by: Richard Zak <[email protected]>
  • Loading branch information
@rjzak
rjzak committed Feb 1, 2023
1 parent bde9e8e commit 37420a9
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 63 deletions.
119 changes: 69 additions & 50 deletions crates/attestation/src/crypto/crl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
// SPDX-License-Identifier: AGPL-3.0-only

use crate::crypto::{SubjectPublicKeyInfoExt, TbsCertificateExt};
use std::collections::HashSet;

use anyhow::{bail, ensure, Context, Result};
use anyhow::{bail, Context, Result};
use der::{Encode, Sequence};
use tracing::debug;
use x509::crl::CertificateList;
use x509::name::Name;
use x509::PkiPath;

#[derive(Clone, Debug, Eq, PartialEq, Sequence)]
Expand All @@ -29,15 +30,6 @@ impl<'a> CrlList<'a> {
}
None
}

fn get_crl_by_issuer(&self, issuer: &Name) -> Option<&CertificateList> {
for pair in &self.crls {
if &pair.crl.tbs_cert_list.issuer == issuer {
return Some(&pair.crl);
}
}
None
}
}

pub trait PkiPathCRLCheck<'a> {
Expand All @@ -48,6 +40,7 @@ impl<'a> PkiPathCRLCheck<'a> for PkiPath<'a> {
fn check_crl(&self, pairs: &CrlList<'a>) -> Result<()> {
// We want to ensure that a valid CRL was passed, so we make sure at least one of the CRLs
// is valid for this `PkiPath`, otherwise, no valid CRLs were received, and that's not okay.
let mut validated_crls = HashSet::new();
let mut found = false;
let mut iter = self.iter().peekable();
loop {
Expand All @@ -57,56 +50,67 @@ impl<'a> PkiPathCRLCheck<'a> for PkiPath<'a> {
break;
}
let cert = cert.unwrap();
let next = next.unwrap();
let next = *next.unwrap();

let crls = {
let mut crls = vec![];
if let Some(crl) = pairs.get_crl_by_issuer(&cert.tbs_certificate.subject) {
crls.push(crl);
} else {
let urls = cert.tbs_certificate.get_crl_urls()?;
for url in urls {
if let Some(crl) = pairs.get_crl_by_url(&url) {
crls.push(crl);
}
}
let (url, crl) = {
let mut urls = next.tbs_certificate.get_crl_urls()?;
// Some end certs use the parent cert CRL
if urls.is_empty() {
urls = cert.tbs_certificate.get_crl_urls()?;
}

crls
if let Some(crl) = pairs.get_crl_by_url(&urls[0]) {
(urls[0].clone(), crl)
} else {
debug!("No CRLS");
continue;
}
};

ensure!(!crls.is_empty(), "Certificate had no CRL URLs");
if let Some(next_update) = crl.tbs_cert_list.next_update {
if next_update.to_system_time() <= std::time::SystemTime::now() {
// Don't error on CRL expiration in unit tests
#[cfg(not(debug_assertions))]
bail!("CRL expired");
#[cfg(debug_assertions)]
debug!("CRL {} expired.", crl.tbs_cert_list.issuer);
}
}

for crl in crls {
if let Some(next_update) = crl.tbs_cert_list.next_update {
if next_update.to_system_time() <= std::time::SystemTime::now() {
// Don't error on CRL expiration in unit tests
#[cfg(not(debug_assertions))]
bail!("CRL expired");
#[cfg(debug_assertions)]
eprintln!("CRL {} expired.", crl.tbs_cert_list.issuer);
if validated_crls.contains(&url) {
// Using the parent cert CRL
// Should only be for AMD.
debug_assert!(!crl.tbs_cert_list.issuer.to_string().contains("AMD"));
if let Some(revocations) = crl.tbs_cert_list.revoked_certificates.as_ref() {
for revoked in revocations {
if revoked.serial_number == next.tbs_certificate.serial_number {
bail!("revoked!");
}
}
}
found = true;
continue;
}

let raw_bytes = crl.tbs_cert_list.to_vec().unwrap();
match cert.tbs_certificate.subject_public_key_info.verify(
&raw_bytes,
cert.signature_algorithm,
crl.signature.raw_bytes(),
) {
Ok(_) => {
if let Some(revocations) = crl.tbs_cert_list.revoked_certificates.as_ref() {
for revoked in revocations {
if revoked.serial_number == next.tbs_certificate.serial_number {
bail!("revoked!");
}
let raw_bytes = crl.tbs_cert_list.to_vec().unwrap();
match cert.tbs_certificate.subject_public_key_info.verify(
&raw_bytes,
cert.signature_algorithm,
crl.signature.raw_bytes(),
) {
Ok(_) => {
if let Some(revocations) = crl.tbs_cert_list.revoked_certificates.as_ref() {
for revoked in revocations {
if revoked.serial_number == next.tbs_certificate.serial_number {
bail!("revoked!");
}
}
found = true;
}
Err(e) => {
return Err(e).context("CRL validation error");
}
found = true;
validated_crls.insert(url);
}
Err(e) => {
return Err(e).context("CRL validation error");
}
}
}
Expand Down Expand Up @@ -204,7 +208,7 @@ mod tests {
},
x509::ext::Extension {
extn_id: const_oid::db::rfc5912::ID_CE_CRL_DISTRIBUTION_POINTS,
critical: true,
critical: false,
extn_value: &crl_dist,
},
]),
Expand Down Expand Up @@ -250,6 +254,16 @@ mod tests {
not_after: Time::GeneralTime(GeneralizedTime::from_system_time(now + dur).unwrap()),
};

let crl_dist = CrlDistributionPoints(vec![DistributionPoint {
distribution_point: Some(DistributionPointName::FullName(vec![
GeneralName::UniformResourceIdentifier(Ia5StringRef::new(TEST_URL).unwrap()),
])),
reasons: None,
crl_issuer: None,
}]);

let crl_dist = crl_dist.to_vec().unwrap();

// Create the certificate body.
let tbs = TbsCertificate {
version: x509::Version::V3,
Expand All @@ -272,6 +286,11 @@ mod tests {
critical: true,
extn_value: &bc,
},
x509::ext::Extension {
extn_id: const_oid::db::rfc5912::ID_CE_CRL_DISTRIBUTION_POINTS,
critical: false,
extn_value: &crl_dist,
},
]),
};

Expand Down
7 changes: 3 additions & 4 deletions crates/attestation/src/sgx/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// SPDX-FileCopyrightText: 2022 Profian Inc. <[email protected]>
// SPDX-License-Identifier: AGPL-3.0-only

#![allow(unused_variables, unused_imports)] // temporary until CRL validation enabled

pub mod config;
pub mod quote;

Expand All @@ -17,6 +15,7 @@ use const_oid::ObjectIdentifier;
use der::{Decode, Encode};
use sgx::pck::SgxExtension;
use sha2::{Digest, Sha256};
#[cfg(debug_assertions)]
use tracing::debug;
use x509::{ext::Extension, request::CertReqInfo, Certificate, PkiPath, TbsCertificate};

Expand Down Expand Up @@ -44,7 +43,7 @@ impl Sgx {
signer = signer.verify_crt(cert)?;
}

//PkiPath::from(chain).check_crl(crls)?;
PkiPath::from(chain).check_crl(crls)?;

Ok(signer)
}
Expand Down Expand Up @@ -96,7 +95,7 @@ impl Sgx {
// Parse the TCB certificate chain
let tcb_chain = &quote.tcb.certs;
ensure!(chain[0] == tcb_chain[0], "sgx TCB root not SGX root");
let tcb_signer = self.trusted(tcb_chain, &quote.crls)?;
self.trusted(tcb_chain, &quote.crls)?;

// Validate TCB report
tcb_chain[1]
Expand Down
1 change: 0 additions & 1 deletion crates/attestation/src/sgx/quote/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use traits::{FromBytes, ParseBytes, Steal};
use anyhow::anyhow;
use der::{Decode, Encode, Sequence};
use p256::ecdsa::signature::Verifier;
use rustls_pemfile::ec_private_keys;
use sgx::ReportBody;
use sha2::{digest::DynDigest, Sha256};
use tcb::{TcbInfo, TcbRoot};
Expand Down
3 changes: 0 additions & 3 deletions crates/attestation/src/sgx/quote/tcb.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// SPDX-FileCopyrightText: 2022 Profian Inc. <[email protected]>
// SPDX-License-Identifier: AGPL-3.0-only

use std::cmp::Ordering;
use std::time::SystemTime;

use chrono::DateTime;
use serde::{Deserialize, Serialize};

Expand Down
8 changes: 3 additions & 5 deletions crates/attestation/src/snp/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// SPDX-FileCopyrightText: 2022 Profian Inc. <[email protected]>
// SPDX-License-Identifier: AGPL-3.0-only

#![allow(unused_variables, unused_imports)] // temporary until CRL validation enabled

pub mod config;

use self::config::Config;
Expand Down Expand Up @@ -254,9 +252,9 @@ impl Snp {

if let Some(signer) = signer {
if signer == &vcek.tbs_certificate {
//let mut path = path;
//path.push(vcek.clone());
//path.check_crl(&certs.crl)?;
let mut path = path;
path.push(vcek.clone());
path.check_crl(&certs.crl)?;
return Ok(&vcek.tbs_certificate);
}
}
Expand Down

0 comments on commit 37420a9

Please sign in to comment.