Skip to content

Commit

Permalink
Change SignedTcbInfoVerifier to use an optional key
Browse files Browse the repository at this point in the history 
Previously the `SignedTcbInfoVerifier` required a key to verify the TCB
info. Now the `SignedTcbInfoVerifier` takes an `Option<VerifyingKey>`.
This provides a more ergonomic interface when the verifying key cannot
be determined.
  • Loading branch information
@nick-mobilecoin
nick-mobilecoin committed Jul 12, 2023
1 parent 5fbe343 commit d48fe31
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 64 deletions.
2 changes: 2 additions & 0 deletions verifier/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ pub enum Error {
SignatureDecodeError,
/// Error verifying the signature
SignatureVerification,
/// No public key available for signature verification
MissingPublicKey,
/// TCB info not yet valid
TcbInfoNotYetValid,
/// TCB info expired
Expand Down
54 changes: 6 additions & 48 deletions verifier/src/evidence.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ where
chain: &[Certificate],
crls: impl IntoIterator<Item = &'d CertificateList>,
) -> (
VerifyingKey,
Option<VerifyingKey>,
VerificationOutput<Option<CertificateChainVerifierError>>,
) {
let result = self
Expand All @@ -235,10 +235,7 @@ where
// the signed data. So we try to get the key from the certificate chain, even if the
// verification failed. This handles the most likely failure case of an expired
// certificate, whose key is still the key that signed the data of interest.
let key = chain
.first()
.and_then(key_from_certificate)
.unwrap_or_else(default_verifying_key);
let key = chain.first().and_then(key_from_certificate);

(
key,
Expand All @@ -250,7 +247,7 @@ where
&self,
collateral: &Collateral,
) -> (
VerifyingKey,
Option<VerifyingKey>,
VerificationOutput<Option<CertificateChainVerifierError>>,
) {
let chain = collateral.tcb_issuer_chain();
Expand All @@ -262,7 +259,7 @@ where
&self,
collateral: &Collateral,
) -> (
VerifyingKey,
Option<VerifyingKey>,
VerificationOutput<Option<CertificateChainVerifierError>>,
) {
let chain = collateral.qe_identity_issuer_chain();
Expand All @@ -275,7 +272,7 @@ where
quote: &Quote3<Q>,
collateral: &Collateral,
) -> (
VerifyingKey,
Option<VerifyingKey>,
VerificationOutput<Option<CertificateChainVerifierError>>,
) {
let crls = [collateral.root_ca_crl(), collateral.pck_crl()];
Expand All @@ -287,10 +284,9 @@ where
match certificate_chain_try_from_quote(quote) {
Ok(chain) => self.verify_certificate_chain(&chain, crls),
Err(_) => {
let key = default_verifying_key();
let is_success = 0u8;
(
key,
None,
VerificationOutput::new(
Some(CertificateChainVerifierError::GeneralCertificateError),
is_success.into(),
Expand All @@ -310,36 +306,6 @@ fn key_from_certificate(cert: &Certificate) -> Option<VerifyingKey> {
VerifyingKey::from_sec1_bytes(key_bytes).ok()
}

fn default_verifying_key() -> VerifyingKey {
// This public key is taken from
// <https://csrc.nist.gov/csrc/media/projects/cryptographic-standards-and-guidelines/documents/examples/ecdsa_prime.pdf>
//
// Public Key:
// Q_x is
// B7E08AFD FE94BAD3
// F1DC8C73 4798BA1C 62B3A0AD 1E9EA2A3 8201CD08 89BC7A19
// Q_y is
// 3603F747 959DBF7A
// 4BB226E4 19287290 63ADC7AE 43529E61 B563BBC6 06CC5E09
//
// The `VerifyingKey` type requires the key to be a valid curve point so we can't use all 0s or
// 1s. Since this key is used in a documented example it is not likely to be seen in production
// use.
let mut sec1_key = [0u8; 65];
sec1_key[0] = 0x04;
sec1_key[1..].copy_from_slice(
[
0xB7, 0xE0, 0x8A, 0xFD, 0xFE, 0x94, 0xBA, 0xD3, 0xF1, 0xDC, 0x8C, 0x73, 0x47, 0x98,
0xBA, 0x1C, 0x62, 0xB3, 0xA0, 0xAD, 0x1E, 0x9E, 0xA2, 0xA3, 0x82, 0x01, 0xCD, 0x08,
0x89, 0xBC, 0x7A, 0x19, 0x36, 0x03, 0xF7, 0x47, 0x95, 0x9D, 0xBF, 0x7A, 0x4B, 0xB2,
0x26, 0xE4, 0x19, 0x28, 0x72, 0x90, 0x63, 0xAD, 0xC7, 0xAE, 0x43, 0x52, 0x9E, 0x61,
0xB5, 0x63, 0xBB, 0xC6, 0x06, 0xCC, 0x5E, 0x09,
]
.as_ref(),
);
VerifyingKey::try_from(sec1_key.as_ref()).expect("Failed to create default verifying key")
}

impl<'a, C: CertificateChainVerifier, E: Accessor<Evidence<Vec<u8>>>> Verifier<E>
for EvidenceVerifier<'a, C>
{
Expand Down Expand Up @@ -574,14 +540,6 @@ mod test {
assert_matches!(Evidence::new(quote, collateral), Err(Error::Serde(_)));
}

#[test]
fn default_key_does_not_abort() {
// We aren't concerned with the actual value of the key, just that it doesn't abort being
// created and that it's not the identity
let default_key = default_verifying_key();
assert_ne!(default_key.to_encoded_point(false).as_bytes(), [0u8; 65]);
}

struct TestDoubleChainVerifier {
failed_certificate_subject: String,
error: CertificateChainVerifierError,
Expand Down
55 changes: 39 additions & 16 deletions verifier/src/tcb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,15 @@ impl SignedTcbInfo {
/// let time = DateTime::from_system_time(SystemTime::now()).unwrap();
/// ```
/// or equivalent
pub fn verify(self, key: &VerifyingKey, time: DateTime) -> Result<(), Error> {
pub fn verify(self, key: Option<&VerifyingKey>, time: DateTime) -> Result<(), Error> {
self.verify_signature(key)?;
let tcb_info = TcbInfo::try_from(&self)?;
tcb_info.verify(time)?;
Ok(())
}

fn verify_signature(&self, key: &VerifyingKey) -> Result<(), Error> {
fn verify_signature(&self, key: Option<&VerifyingKey>) -> Result<(), Error> {
let key = key.ok_or(Error::MissingPublicKey)?;
let tcb_info = self.tcb_info.as_ref().get();
let signature =
Signature::try_from(&self.signature[..]).map_err(|_| Error::SignatureDecodeError)?;
Expand All @@ -272,7 +273,7 @@ impl TryFrom<&str> for SignedTcbInfo {
/// Verifier for ensuring a TCB info was signed with the provided key
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct SignedTcbInfoVerifier {
key: VerifyingKey,
key: Option<VerifyingKey>,
time: DateTime,
}

Expand All @@ -288,13 +289,14 @@ impl SignedTcbInfoVerifier {
/// This should be retrieved from the x509 certificate provided in the
/// TCB request from
/// <https://api.trustedservices.intel.com/sgx/certification/v4/tcb?fmspc={}>
/// When this is `None` verification will always fail.
/// - `time` - The current system time
/// This is expected to be generated by:
/// ```ignore
/// let time = DateTime::from_system_time(SystemTime::now()).unwrap();
/// ```
/// or equivalent
pub fn new(key: VerifyingKey, time: DateTime) -> Self {
pub fn new(key: Option<VerifyingKey>, time: DateTime) -> Self {
Self { key, time }
}
}
Expand All @@ -303,7 +305,7 @@ impl<E: Accessor<SignedTcbInfo>> Verifier<E> for SignedTcbInfoVerifier {
type Value = Option<Error>;
fn verify(&self, evidence: &E) -> VerificationOutput<Self::Value> {
let signed_tcb_info = evidence.get();
let result = signed_tcb_info.verify(&self.key, self.time);
let result = signed_tcb_info.verify(self.key.as_ref(), self.time);
let is_success = result.is_ok() as u8;

VerificationOutput::new(result.err(), is_success.into())
Expand Down Expand Up @@ -641,7 +643,7 @@ mod tests {

let time = time.parse::<DateTime>().expect("Failed to parse time");

assert_eq!(signed_tcb_info.verify(&key, time).is_ok(), true);
assert_eq!(signed_tcb_info.verify(Some(&key), time).is_ok(), true);
}

#[test]
Expand All @@ -656,7 +658,7 @@ mod tests {
.expect("Failed to parse time");

assert_matches!(
signed_tcb_info.verify(&key, time),
signed_tcb_info.verify(Some(&key), time),
Err(Error::TcbInfoNotYetValid)
);
}
Expand All @@ -673,7 +675,7 @@ mod tests {
.expect("Failed to parse time");

assert_matches!(
signed_tcb_info.verify(&key, time),
signed_tcb_info.verify(Some(&key), time),
Err(Error::TcbInfoExpired)
);
}
Expand All @@ -692,7 +694,7 @@ mod tests {
signed_tcb_info.signature.truncate(63);

assert_matches!(
signed_tcb_info.verify(&key, time),
signed_tcb_info.verify(Some(&key), time),
Err(Error::SignatureDecodeError)
);
}
Expand All @@ -710,7 +712,7 @@ mod tests {
signed_tcb_info.signature[0] += 1;

assert_matches!(
signed_tcb_info.verify(&key, time),
signed_tcb_info.verify(Some(&key), time),
Err(Error::SignatureVerification)
);
}
Expand All @@ -722,7 +724,7 @@ mod tests {
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(key, time);
let verifier = SignedTcbInfoVerifier::new(Some(key), time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 0);
Expand All @@ -739,7 +741,7 @@ mod tests {
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(key, time);
let verifier = SignedTcbInfoVerifier::new(Some(key), time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 0);
Expand All @@ -753,7 +755,7 @@ mod tests {
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(key, time);
let verifier = SignedTcbInfoVerifier::new(Some(key), time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 0);
Expand Down Expand Up @@ -895,7 +897,7 @@ mod tests {
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(key, time);
let verifier = SignedTcbInfoVerifier::new(Some(key), time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 1);
Expand All @@ -917,7 +919,7 @@ mod tests {
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(key, time);
let verifier = SignedTcbInfoVerifier::new(Some(key), time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 0);
Expand All @@ -939,7 +941,7 @@ mod tests {
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(key, time);
let verifier = SignedTcbInfoVerifier::new(Some(key), time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 0);
Expand All @@ -948,4 +950,25 @@ mod tests {
Error::TcbInfoVersion{expected: TCB_INFO_VERSION, actual} if actual == version
);
}

#[test]
fn signed_tcb_info_verifier_fails_when_no_key() {
let tcb_json = include_str!("../data/tests/fmspc_00906ED50000_2023_05_10.json");
let signed_tcb_info =
SignedTcbInfo::try_from(tcb_json).expect("Failed to parse signed TCB");

let time = "2023-06-08T13:43:27Z"
.parse::<DateTime>()
.expect("Failed to parse time");

let verifier = SignedTcbInfoVerifier::new(None, time);
let verification = verifier.verify(&signed_tcb_info);

assert_eq!(verification.is_success().unwrap_u8(), 0);

let displayable = VerificationTreeDisplay::new(&verifier, verification);
let expected = r#"
- [ ] The TCB info could not be verified: No public key available for signature verification"#;
assert_eq!(format!("\n{displayable}"), textwrap::dedent(expected));
}
}

0 comments on commit d48fe31

Please sign in to comment.