diff --git a/CHANGELOG.md b/CHANGELOG.md index 3301c89..d55bb38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ to 0.3.0 are beta releases. ## [Unreleased] +## [0.3.4] - PLANNED +### Fixed +- `age-plugin-yubikey` now completely ignores any identity that has unrecognised + critical extensions in its certificate, to ensure it doesn't misuse a newer + identity type. + ## [0.3.3] - 2023-02-11 ### Fixed - When `age-plugin-yubikey` assists the user in changing their PIN from the diff --git a/src/key.rs b/src/key.rs index 9aa81ed..294f185 100644 --- a/src/key.rs +++ b/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 { + let known_oids = KNOWN_OIDS + .iter() + .map(|oid| Oid::from(oid).unwrap()) + .collect::>(); + + // 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 => {