Reject identities with unrecognised critical extensions

We don't know how to correctly use these identities. In particular, some
identities store parts of their private key material in certificate
extensions to work around hardware limitations. Not understanding these
extensions could lead to encrypting with the wrong protocol and
violating security assumptions.

src/key.rs

@@ -15,6 +15,7 @@ use std::io;
 use std::iter;
 use std::thread::sleep;
 use std::time::{Duration, Instant, SystemTime};
+use x509_parser::der_parser::oid::Oid;
 use yubikey::{
     certificate::Certificate,
     piv::{decrypt_data, AlgorithmId, RetiredSlotId, SlotId},
@@ -27,13 +28,16 @@ use crate::{
     fl,
     format::{RecipientLine, STANZA_KEY_LABEL},
     p256::{Recipient, TAG_BYTES},
-    util::{otp_serial_prefix, Metadata},
+    util::{otp_serial_prefix, Metadata, POLICY_EXTENSION_OID},
     IDENTITY_PREFIX,
 };
 
 const ONE_SECOND: Duration = Duration::from_secs(1);
 const FIFTEEN_SECONDS: Duration = Duration::from_secs(15);
 
+/// The set of OIDs that we understand and use when parsing YubiKey slot certificates.
+const KNOWN_OIDS: &[&[u64]] = &[POLICY_EXTENSION_OID];
+
 pub(crate) fn is_connected(reader: Reader) -> bool {
     filter_connected(&reader)
 }
@@ -339,6 +343,30 @@ pub(crate) fn manage(yubikey: &mut YubiKey) -> Result<(), Error> {
     Ok(())
 }
 
+/// Parses the certificate to identify the preferred recipient type it corresponds to.
+pub(crate) fn identify_recipient(cert: &Certificate) -> Option<Recipient> {
+    let known_oids = KNOWN_OIDS
+        .iter()
+        .map(|oid| Oid::from(oid).unwrap())
+        .collect::<Vec<_>>();
+
+    // If the certificate contains any unrecognised critical extensions, reject it: we
+    // don't know how to correctly use the identity. In particular, some identities store
+    // parts of their private key material in certificate extensions to work around
+    // hardware limitations. Not understanding these extensions could lead to encrypting
+    // with the wrong protocol and violating security assumptions.
+    let (_, c) = x509_parser::parse_x509_certificate(cert.as_ref()).ok()?;
+    if c.tbs_certificate
+        .extensions()
+        .iter()
+        .any(|ext| ext.critical && !known_oids.contains(&ext.oid))
+    {
+        return None;
+    }
+
+    Recipient::from_certificate(cert)
+}
+
 /// Returns an iterator of keys that are occupying plugin-compatible slots, along with the
 /// corresponding recipient if the key is compatible with this plugin.
 pub(crate) fn list_slots(
@@ -348,8 +376,7 @@ pub(crate) fn list_slots(
         // We only use the retired slots.
         match key.slot() {
             SlotId::Retired(slot) => {
-                // Only P-256 keys are compatible with us.
-                let recipient = Recipient::from_certificate(key.certificate());
+                let recipient = identify_recipient(key.certificate());
                 Some((key, slot, recipient))
             }
             _ => None,
@@ -556,9 +583,9 @@ impl Stub {
         let (cert, pk) = match Certificate::read(&mut yubikey, SlotId::Retired(self.slot))
             .ok()
             .and_then(|cert| {
-                Recipient::from_certificate(&cert)
-                    .filter(|pk| pk.tag() == self.tag)
-                    .map(|pk| (cert, pk))
+                identify_recipient(&cert)
+                    .filter(|recipient| recipient.tag() == self.tag)
+                    .map(|r| (cert, r))
             }) {
             Some(pk) => pk,
             None => {