[ot_certs] Parse Authority and Subjecy Key Identifier manually

OpenSSL has an undocumented quirk: if the basic key usage is
empty (and possibly in other circumstances?), it silently
pretend that the certificate has no authority or subject key
identifier. It is not clear whether this is intentional or
a side-effect of a bug but the result is the same: a seemingly
valid (but stupid) certificate will generate an error when parse
with ot_certs.

The solution adopted in this commit is to simply parse those
extensions manually. We already have to do it for a number of
extensions and those are really simple. As a side effect of this
commit, the authority and subject key identifier have become
optional fields of the certificate. It is expected that we will
always populate them of course but there is no harm in handling
this case as well.

Signed-off-by: Amaury Pouly <[email protected]>

sw/host/ot_certs/src/asn1/x509.rs

@@ -153,8 +153,12 @@ impl X509 {
                         if let Some(key_usage) = &cert.key_usage {
                             Self::push_key_usage_ext(builder, key_usage)?;
                         }
-                        Self::push_auth_key_id_ext(builder, &cert.authority_key_identifier)?;
-                        Self::push_subject_key_id_ext(builder, &cert.subject_key_identifier)?;
+                        if let Some(auth_key_id) = &cert.authority_key_identifier {
+                            Self::push_auth_key_id_ext(builder, auth_key_id)?;
+                        }
+                        if let Some(subj_key_id) = &cert.subject_key_identifier {
+                            Self::push_subject_key_id_ext(builder, subj_key_id)?;
+                        }
                         for ext in &cert.private_extensions {
                             Self::push_cert_extension(builder, ext)?
                         }

sw/host/ot_certs/src/template/mod.rs

@@ -70,9 +70,9 @@ pub struct Certificate {
     /// X509 certificate's public key.
     pub subject_public_key_info: SubjectPublicKeyInfo,
     /// X509 certificate's authority key identifier.
-    pub authority_key_identifier: Value<Vec<u8>>,
+    pub authority_key_identifier: Option<Value<Vec<u8>>>,
     /// X509 certificate's public key identifier.
-    pub subject_key_identifier: Value<Vec<u8>>,
+    pub subject_key_identifier: Option<Value<Vec<u8>>>,
     // X509 basic constraints extension, optional.
     pub basic_constraints: Option<BasicConstraints>,
     pub key_usage: Option<KeyUsage>,

sw/host/ot_certs/src/x509.rs

@@ -7,9 +7,7 @@ use indexmap::IndexMap;
 use num_bigint_dig::BigUint;
 
 use foreign_types::ForeignTypeRef;
-use openssl::asn1::{
-    Asn1IntegerRef, Asn1ObjectRef, Asn1OctetStringRef, Asn1StringRef, Asn1TimeRef,
-};
+use openssl::asn1::{Asn1IntegerRef, Asn1ObjectRef, Asn1StringRef, Asn1TimeRef};
 use openssl::bn::{BigNum, BigNumContext, BigNumRef};
 use openssl::ec::{EcGroupRef, EcKey};
 use openssl::ecdsa::EcdsaSig;
@@ -84,10 +82,6 @@ fn asn1str_to_str(field: &str, s: &Asn1StringRef) -> Result<Value<String>> {
     ))
 }
 
-fn asn1octets_to_vec(s: &Asn1OctetStringRef) -> Value<Vec<u8>> {
-    Value::literal(s.as_slice().to_vec())
-}
-
 fn asn1time_to_string(time: &Asn1TimeRef) -> Result<Value<String>> {
     // OpenSSL guarantees that an ASN1_TIME is in fact just a typedef for ASN1_STRING
     // https://www.openssl.org/docs/man1.1.1/man3/ASN1_TIME_to_generalizedtime.html
@@ -222,6 +216,8 @@ pub fn parse_certificate(cert: &[u8]) -> Result<template::Certificate> {
     let mut private_extensions = Vec::new();
     let mut basic_constraints = None;
     let mut key_usage: Option<KeyUsage> = None;
+    let mut auth_key_id = None;
+    let mut subj_key_id = None;
     for ext in raw_extensions {
         match ext.object.nid() {
             // Ignore extensions that are standard and handled by openssl.
@@ -240,9 +236,24 @@ pub fn parse_certificate(cert: &[u8]) -> Result<template::Certificate> {
                     extension::parse_key_usage(&ext).context("could not parse X509 key usage")?,
                 );
             }
-            Nid::AUTHORITY_KEY_IDENTIFIER => (),
+            Nid::AUTHORITY_KEY_IDENTIFIER => {
+                // Although OpenSSL will parse the authority key identifier correctly, it will sometimes
+                // pretend that it doesn't exist if the certificate is unusual (i.e. all key usage bit
+                // set to false) without showing any error. Since this is very confusing, we may as well
+                // parse it ourselves.
+                auth_key_id = Some(Value::Literal(
+                    extension::parse_authority_key_id(&ext)
+                        .context("could not parse authority key identifier")?,
+                ));
+            }
             Nid::SUBJECT_ALT_NAME => (),
-            Nid::SUBJECT_KEY_IDENTIFIER => (),
+            Nid::SUBJECT_KEY_IDENTIFIER => {
+                // Same issue as with authority key identifier.
+                subj_key_id = Some(Value::Literal(
+                    extension::parse_subject_key_id(&ext)
+                        .context("could not parse subject key identifier")?,
+                ));
+            }
             _ => private_extensions
                 .push(extension::parse_extension(&ext).context("could not parse X509 extension")?),
         }
@@ -253,7 +264,6 @@ pub fn parse_certificate(cert: &[u8]) -> Result<template::Certificate> {
             .public_key()
             .context("the X509 does not have a valid public key!")?,
     )?;
-
     Ok(template::Certificate {
         serial_number: asn1int_to_bn("serial number", x509.serial_number())?,
         issuer: asn1name_to_name("issuer", x509.issuer_name())?,
@@ -262,14 +272,8 @@ pub fn parse_certificate(cert: &[u8]) -> Result<template::Certificate> {
             .context("cannot parse not_before time")?,
         not_after: asn1time_to_string(x509.not_after()).context("cannot parse not_after time")?,
         subject_public_key_info,
-        authority_key_identifier: asn1octets_to_vec(
-            x509.authority_key_id()
-                .context("the certificate has not authority key id")?,
-        ),
-        subject_key_identifier: asn1octets_to_vec(
-            x509.subject_key_id()
-                .context("the certificate has not subject key id")?,
-        ),
+        authority_key_identifier: auth_key_id,
+        subject_key_identifier: subj_key_id,
         basic_constraints,
         key_usage,
         subject_alt_name: get_subject_alt_name(&x509)?,

sw/host/ot_certs/src/x509/extension.rs

@@ -266,6 +266,19 @@ impl<'a> asn1::SimpleAsn1Readable<'a> for KeyUsage {
     }
 }
 
+// From https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.1
+// AuthorityKeyIdentifier ::= SEQUENCE {
+//       keyIdentifier             [0] KeyIdentifier           OPTIONAL,
+//       authorityCertIssuer       [1] GeneralNames            OPTIONAL,
+//       authorityCertSerialNumber [2] CertificateSerialNumber OPTIONAL  }
+//
+// KeyIdentifier ::= OCTET STRING
+#[derive(asn1::Asn1Read)]
+struct AuthorityKeyIdentifier<'a> {
+    #[implicit(0)]
+    pub key_id: Option<&'a [u8]>,
+}
+
 /// Try to parse an X509 extension as a DICE TCB info extension.
 pub fn parse_dice_tcb_info_extension(ext: &[u8]) -> Result<DiceTcbInfoExtension> {
     asn1::parse_single::<DiceTcbInfo>(ext)
@@ -300,6 +313,23 @@ pub fn parse_basic_constraints(ext: &X509ExtensionRef) -> Result<BasicConstraint
         .to_basic_constraints()
 }
 
+pub fn parse_authority_key_id(ext: &X509ExtensionRef) -> Result<Vec<u8>> {
+    let auth = asn1::parse_single::<AuthorityKeyIdentifier>(ext.data.as_slice())?;
+    Ok(auth
+        .key_id
+        .context("authority key identifier extension is empty")?
+        .to_vec())
+}
+
+pub fn parse_subject_key_id(ext: &X509ExtensionRef) -> Result<Vec<u8>> {
+    // From https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.2
+    // SubjectKeyIdentifier ::= KeyIdentifier
+    //
+    //  KeyIdentifier ::= OCTET STRING
+    let subj = asn1::parse_single::<&[u8]>(ext.data.as_slice())?;
+    Ok(subj.to_vec())
+}
+
 /// Try to parse an X509 extension.
 pub fn parse_extension(ext: &X509ExtensionRef) -> Result<CertificateExtension> {
     let dice_oid =