From cea4307f5e60e7b8ca32339c82cd4f27d6291386 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Tue, 26 Aug 2025 22:43:05 +0200 Subject: [PATCH 1/4] Simplify KeyPurposeId comparison --- src/verify_cert.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 656bce3a..89eb2008 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -575,14 +575,17 @@ impl ExtendedKeyUsage { #[cfg(feature = "alloc")] let mut present = Vec::new(); loop { - let value = der::expect_tag(input, der::Tag::OID)?; - if self.key_purpose_id_equals(value) { + let id = KeyPurposeId { + oid_value: der::expect_tag(input, der::Tag::OID)?, + }; + + if self.id() == id { input.skip_to_end(); break; } #[cfg(feature = "alloc")] - present.push(OidDecoder::new(value.as_slice_less_safe()).collect()); + present.push(OidDecoder::new(id.oid_value.as_slice_less_safe()).collect()); if input.at_end() { return Err(Error::RequiredEkuNotFoundContext( RequiredEkuNotFoundContext { @@ -598,15 +601,11 @@ impl ExtendedKeyUsage { Ok(()) } - fn key_purpose_id_equals(&self, value: untrusted::Input<'_>) -> bool { - public_values_eq( - match self { - Self::Required(eku) => *eku, - Self::RequiredIfPresent(eku) => *eku, - } - .oid_value, - value, - ) + fn id(&self) -> KeyPurposeId<'static> { + match self { + Self::Required(id) => *id, + Self::RequiredIfPresent(id) => *id, + } } } @@ -905,8 +904,8 @@ mod tests { #[test] fn eku_key_purpose_id() { assert!( - ExtendedKeyUsage::RequiredIfPresent(KeyPurposeId::new(EKU_SERVER_AUTH)) - .key_purpose_id_equals(KeyPurposeId::new(EKU_SERVER_AUTH).oid_value) + ExtendedKeyUsage::RequiredIfPresent(KeyPurposeId::new(EKU_SERVER_AUTH)).id() + == KeyPurposeId::new(EKU_SERVER_AUTH) ) } From 5655b28aea18076ea36e16098da6bb57e1f92433 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 31 Aug 2025 11:26:01 +0200 Subject: [PATCH 2/4] Extract KeyPurposeId iteration from ExtendedKeyUsage::check() --- src/verify_cert.rs | 86 +++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 89eb2008..6c0b4875 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -367,7 +367,12 @@ fn check_issuer_independent_properties( untrusted::read_all_optional(cert.basic_constraints, Error::BadDer, |value| { check_basic_constraints(value, role, sub_ca_count) })?; - untrusted::read_all_optional(cert.eku, Error::BadDer, |value| eku.check(value))?; + untrusted::read_all_optional(cert.eku, Error::BadDer, |value| match value { + Some(input) => eku.check(KeyPurposeIdIter { input }), + None => eku.check(KeyPurposeIdIter { + input: &mut untrusted::Reader::new(untrusted::Input::from(&[])), + }), + })?; Ok(()) } @@ -556,49 +561,36 @@ enum ExtendedKeyUsage { impl ExtendedKeyUsage { // https://tools.ietf.org/html/rfc5280#section-4.2.1.12 - fn check(&self, input: Option<&mut untrusted::Reader<'_>>) -> Result<(), Error> { - let input = match (input, self) { - (Some(input), _) => input, - (None, Self::RequiredIfPresent(_)) => return Ok(()), - (None, Self::Required(_)) => { - return Err(Error::RequiredEkuNotFoundContext( - RequiredEkuNotFoundContext { - #[cfg(feature = "alloc")] - required: KeyUsage { inner: *self }, - #[cfg(feature = "alloc")] - present: Vec::new(), - }, - )); - } - }; - + fn check<'a>( + &self, + iter: impl Iterator, Error>>, + ) -> Result<(), Error> { + let mut empty = true; #[cfg(feature = "alloc")] let mut present = Vec::new(); - loop { - let id = KeyPurposeId { - oid_value: der::expect_tag(input, der::Tag::OID)?, - }; + for id in iter { + empty = false; + let id = id?; if self.id() == id { - input.skip_to_end(); - break; + return Ok(()); } #[cfg(feature = "alloc")] present.push(OidDecoder::new(id.oid_value.as_slice_less_safe()).collect()); - if input.at_end() { - return Err(Error::RequiredEkuNotFoundContext( - RequiredEkuNotFoundContext { - #[cfg(feature = "alloc")] - required: KeyUsage { inner: *self }, - #[cfg(feature = "alloc")] - present, - }, - )); - } } - Ok(()) + match (empty, self) { + (true, ExtendedKeyUsage::RequiredIfPresent(_)) => Ok(()), + _ => Err(Error::RequiredEkuNotFoundContext( + RequiredEkuNotFoundContext { + #[cfg(feature = "alloc")] + required: KeyUsage { inner: *self }, + #[cfg(feature = "alloc")] + present, + }, + )), + } } fn id(&self) -> KeyPurposeId<'static> { @@ -609,6 +601,28 @@ impl ExtendedKeyUsage { } } +struct KeyPurposeIdIter<'a, 'r> { + input: &'r mut untrusted::Reader<'a>, +} + +impl<'a, 'r> Iterator for KeyPurposeIdIter<'a, 'r> { + type Item = Result, Error>; + + fn next(&mut self) -> Option { + if self.input.at_end() { + return None; + } + + Some(der::expect_tag(self.input, der::Tag::OID).map(|oid_value| KeyPurposeId { oid_value })) + } +} + +impl Drop for KeyPurposeIdIter<'_, '_> { + fn drop(&mut self) { + self.input.skip_to_end(); + } +} + /// An OID value indicating an Extended Key Usage (EKU) key purpose. #[derive(Clone, Copy)] struct KeyPurposeId<'a> { @@ -888,7 +902,9 @@ mod tests { #[test] fn eku_fail_empty() { let err = ExtendedKeyUsage::Required(KeyPurposeId::new(EKU_SERVER_AUTH)) - .check(None) + .check(KeyPurposeIdIter { + input: &mut untrusted::Reader::new(untrusted::Input::from(&[])), + }) .unwrap_err(); assert_eq!( err, From afdf6812c9ea78bcb18a41c62818bc39c65f966a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 31 Aug 2025 11:29:39 +0200 Subject: [PATCH 3/4] Move ExtendedKeyUsage::check() to KeyUsage --- src/verify_cert.rs | 70 +++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 6c0b4875..527a57a8 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -60,7 +60,7 @@ impl<'a, 'p: 'a> ChainOptions<'a, 'p> { ) -> Result<&'p TrustAnchor<'p>, ControlFlow> { let role = path.node().role(); - check_issuer_independent_properties(path.head(), time, role, sub_ca_count, self.eku.inner)?; + check_issuer_independent_properties(path.head(), time, role, sub_ca_count, self.eku)?; // TODO: HPKP checks. @@ -349,7 +349,7 @@ fn check_issuer_independent_properties( time: UnixTime, role: Role, sub_ca_count: usize, - eku: ExtendedKeyUsage, + eku: KeyUsage, ) -> Result<(), Error> { // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; // TODO: Check signature algorithm like mozilla::pkix. @@ -531,35 +531,6 @@ impl KeyUsage { } } - /// Yield the OID values of the required extended key usage. - pub fn oid_values(&self) -> impl Iterator + '_ { - OidDecoder::new( - match &self.inner { - ExtendedKeyUsage::Required(eku) => eku, - ExtendedKeyUsage::RequiredIfPresent(eku) => eku, - } - .oid_value - .as_slice_less_safe(), - ) - } - - /// Human-readable representation of the server authentication OID. - pub const SERVER_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 1]; - /// Human-readable representation of the client authentication OID. - pub const CLIENT_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 2]; -} - -/// Extended Key Usage (EKU) of a certificate. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum ExtendedKeyUsage { - /// The certificate must contain the specified [`KeyPurposeId`] as EKU. - Required(KeyPurposeId<'static>), - - /// If the certificate has EKUs, then the specified [`KeyPurposeId`] must be included. - RequiredIfPresent(KeyPurposeId<'static>), -} - -impl ExtendedKeyUsage { // https://tools.ietf.org/html/rfc5280#section-4.2.1.12 fn check<'a>( &self, @@ -572,7 +543,7 @@ impl ExtendedKeyUsage { for id in iter { empty = false; let id = id?; - if self.id() == id { + if self.inner.id() == id { return Ok(()); } @@ -580,12 +551,12 @@ impl ExtendedKeyUsage { present.push(OidDecoder::new(id.oid_value.as_slice_less_safe()).collect()); } - match (empty, self) { + match (empty, self.inner) { (true, ExtendedKeyUsage::RequiredIfPresent(_)) => Ok(()), _ => Err(Error::RequiredEkuNotFoundContext( RequiredEkuNotFoundContext { #[cfg(feature = "alloc")] - required: KeyUsage { inner: *self }, + required: KeyUsage { inner: self.inner }, #[cfg(feature = "alloc")] present, }, @@ -593,6 +564,35 @@ impl ExtendedKeyUsage { } } + /// Yield the OID values of the required extended key usage. + pub fn oid_values(&self) -> impl Iterator + '_ { + OidDecoder::new( + match &self.inner { + ExtendedKeyUsage::Required(eku) => eku, + ExtendedKeyUsage::RequiredIfPresent(eku) => eku, + } + .oid_value + .as_slice_less_safe(), + ) + } + + /// Human-readable representation of the server authentication OID. + pub const SERVER_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 1]; + /// Human-readable representation of the client authentication OID. + pub const CLIENT_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 2]; +} + +/// Extended Key Usage (EKU) of a certificate. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ExtendedKeyUsage { + /// The certificate must contain the specified [`KeyPurposeId`] as EKU. + Required(KeyPurposeId<'static>), + + /// If the certificate has EKUs, then the specified [`KeyPurposeId`] must be included. + RequiredIfPresent(KeyPurposeId<'static>), +} + +impl ExtendedKeyUsage { fn id(&self) -> KeyPurposeId<'static> { match self { Self::Required(id) => *id, @@ -901,7 +901,7 @@ mod tests { #[test] fn eku_fail_empty() { - let err = ExtendedKeyUsage::Required(KeyPurposeId::new(EKU_SERVER_AUTH)) + let err = KeyUsage::required(EKU_SERVER_AUTH) .check(KeyPurposeIdIter { input: &mut untrusted::Reader::new(untrusted::Input::from(&[])), }) From db64a7a0e43f3b8576b2cd42f1b0ea02e13f568c Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Sun, 31 Aug 2025 11:39:07 +0200 Subject: [PATCH 4/4] Extract trait for ExtendedKeyUsage validation --- src/end_entity.rs | 11 ++++--- src/lib.rs | 2 +- src/verify_cert.rs | 76 ++++++++++++++++++++++++++++------------------ 3 files changed, 53 insertions(+), 36 deletions(-) diff --git a/src/end_entity.rs b/src/end_entity.rs index b513b7b7..3073a383 100644 --- a/src/end_entity.rs +++ b/src/end_entity.rs @@ -21,7 +21,7 @@ use pki_types::{ use crate::crl::RevocationOptions; use crate::error::Error; use crate::subject_name::{verify_dns_names, verify_ip_address_names}; -use crate::verify_cert::{self, KeyUsage, VerifiedPath}; +use crate::verify_cert::{self, ExtendedKeyUsageValidator, VerifiedPath}; use crate::{cert, signed_data}; /// An end-entity certificate. @@ -31,7 +31,7 @@ use crate::{cert, signed_data}; /// /// * [`EndEntityCert::verify_for_usage()`]: Verify that the peer's certificate /// is valid for the current usage scenario. For server authentication, use -/// [`KeyUsage::server_auth()`]. +/// [`crate::KeyUsage::server_auth()`]. /// * [`EndEntityCert::verify_is_valid_for_subject_name()`]: Verify that the server's /// certificate is valid for the host or IP address that is being connected to. /// * [`EndEntityCert::verify_signature()`]: Verify that the signature of server's @@ -42,7 +42,7 @@ use crate::{cert, signed_data}; /// /// * [`EndEntityCert::verify_for_usage()`]: Verify that the peer's certificate /// is valid for the current usage scenario. For client authentication, use -/// [`KeyUsage::client_auth()`]. +/// [`crate::KeyUsage::client_auth()`]. /// * [`EndEntityCert::verify_signature()`]: Verify that the signature of client's /// `CertificateVerify` message is valid using the public key from the /// client's certificate. @@ -85,7 +85,8 @@ impl EndEntityCert<'_> { /// * `time` is the time for which the validation is effective (usually the /// current time). /// * `usage` is the intended usage of the certificate, indicating what kind - /// of usage we're verifying the certificate for. + /// of usage we're verifying the certificate for. The default [`ExtendedKeyUsageValidator`] + /// implementation is [`KeyUsage`](crate::KeyUsage). /// * `crls` is the list of certificate revocation lists to check /// the certificate against. /// * `verify_path` is an optional verification function for path candidates. @@ -105,7 +106,7 @@ impl EndEntityCert<'_> { trust_anchors: &'p [TrustAnchor<'_>], intermediate_certs: &'p [CertificateDer<'p>], time: UnixTime, - usage: KeyUsage, + usage: impl ExtendedKeyUsageValidator, revocation: Option>, verify_path: Option<&dyn Fn(&VerifiedPath<'_>) -> Result<(), Error>>, ) -> Result, Error> { diff --git a/src/lib.rs b/src/lib.rs index 74c7923b..d080527d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -85,7 +85,7 @@ pub use { }, rpk_entity::RawPublicKeyEntity, trust_anchor::anchor_from_trusted_cert, - verify_cert::{KeyUsage, RequiredEkuNotFoundContext, VerifiedPath}, + verify_cert::{ExtendedKeyUsageValidator, KeyUsage, RequiredEkuNotFoundContext, VerifiedPath}, }; #[cfg(feature = "alloc")] diff --git a/src/verify_cert.rs b/src/verify_cert.rs index 527a57a8..25f8e8ac 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -28,15 +28,15 @@ use crate::{public_values_eq, signed_data, subject_name}; // Use `'a` for lifetimes that we don't care about, `'p` for lifetimes that become a part of // the `VerifiedPath`. -pub(crate) struct ChainOptions<'a, 'p> { - pub(crate) eku: KeyUsage, +pub(crate) struct ChainOptions<'a, 'p, V> { + pub(crate) eku: V, pub(crate) supported_sig_algs: &'a [&'a dyn SignatureVerificationAlgorithm], pub(crate) trust_anchors: &'p [TrustAnchor<'p>], pub(crate) intermediate_certs: &'p [CertificateDer<'p>], pub(crate) revocation: Option>, } -impl<'a, 'p: 'a> ChainOptions<'a, 'p> { +impl<'a, 'p: 'a, V: ExtendedKeyUsageValidator> ChainOptions<'a, 'p, V> { pub(crate) fn build_chain( &self, end_entity: &'p EndEntityCert<'p>, @@ -60,7 +60,7 @@ impl<'a, 'p: 'a> ChainOptions<'a, 'p> { ) -> Result<&'p TrustAnchor<'p>, ControlFlow> { let role = path.node().role(); - check_issuer_independent_properties(path.head(), time, role, sub_ca_count, self.eku)?; + check_issuer_independent_properties(path.head(), time, role, sub_ca_count, &self.eku)?; // TODO: HPKP checks. @@ -349,7 +349,7 @@ fn check_issuer_independent_properties( time: UnixTime, role: Role, sub_ca_count: usize, - eku: KeyUsage, + eku: &impl ExtendedKeyUsageValidator, ) -> Result<(), Error> { // TODO: check_distrust(trust_anchor_subject, trust_anchor_spki)?; // TODO: Check signature algorithm like mozilla::pkix. @@ -368,8 +368,8 @@ fn check_issuer_independent_properties( check_basic_constraints(value, role, sub_ca_count) })?; untrusted::read_all_optional(cert.eku, Error::BadDer, |value| match value { - Some(input) => eku.check(KeyPurposeIdIter { input }), - None => eku.check(KeyPurposeIdIter { + Some(input) => eku.validate(KeyPurposeIdIter { input }), + None => eku.validate(KeyPurposeIdIter { input: &mut untrusted::Reader::new(untrusted::Input::from(&[])), }), })?; @@ -531,11 +531,27 @@ impl KeyUsage { } } + /// Yield the OID values of the required extended key usage. + pub fn oid_values(&self) -> impl Iterator + '_ { + OidDecoder::new( + match &self.inner { + ExtendedKeyUsage::Required(eku) => eku, + ExtendedKeyUsage::RequiredIfPresent(eku) => eku, + } + .oid_value + .as_slice_less_safe(), + ) + } + + /// Human-readable representation of the server authentication OID. + pub const SERVER_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 1]; + /// Human-readable representation of the client authentication OID. + pub const CLIENT_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 2]; +} + +impl ExtendedKeyUsageValidator for KeyUsage { // https://tools.ietf.org/html/rfc5280#section-4.2.1.12 - fn check<'a>( - &self, - iter: impl Iterator, Error>>, - ) -> Result<(), Error> { + fn validate(&self, iter: KeyPurposeIdIter<'_, '_>) -> Result<(), Error> { let mut empty = true; #[cfg(feature = "alloc")] let mut present = Vec::new(); @@ -556,30 +572,30 @@ impl KeyUsage { _ => Err(Error::RequiredEkuNotFoundContext( RequiredEkuNotFoundContext { #[cfg(feature = "alloc")] - required: KeyUsage { inner: self.inner }, + required: Self { inner: self.inner }, #[cfg(feature = "alloc")] present, }, )), } } +} - /// Yield the OID values of the required extended key usage. - pub fn oid_values(&self) -> impl Iterator + '_ { - OidDecoder::new( - match &self.inner { - ExtendedKeyUsage::Required(eku) => eku, - ExtendedKeyUsage::RequiredIfPresent(eku) => eku, - } - .oid_value - .as_slice_less_safe(), - ) +impl ExtendedKeyUsageValidator for &V { + fn validate(&self, iter: KeyPurposeIdIter<'_, '_>) -> Result<(), Error> { + (*self).validate(iter) } +} - /// Human-readable representation of the server authentication OID. - pub const SERVER_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 1]; - /// Human-readable representation of the client authentication OID. - pub const CLIENT_AUTH_REPR: &[usize] = &[1, 3, 6, 1, 5, 5, 7, 3, 2]; +/// A trait for validating the Extended Key Usage (EKU) extensions of a certificate. +pub trait ExtendedKeyUsageValidator { + /// Validate the EKU values in a certificate. + /// + /// `iter` yields the EKU OIDs in the certificate, or an error if the EKU extension + /// is malformed. `validate()` should yield `Ok(())` if the EKU values match the + /// required policy, or an `Error` if they do not. Ideally the `Error` should be + /// `Error::RequiredEkuNotFoundContext` if the policy is not met. + fn validate(&self, iter: KeyPurposeIdIter<'_, '_>) -> Result<(), Error>; } /// Extended Key Usage (EKU) of a certificate. @@ -601,7 +617,7 @@ impl ExtendedKeyUsage { } } -struct KeyPurposeIdIter<'a, 'r> { +pub struct KeyPurposeIdIter<'a, 'r> { input: &'r mut untrusted::Reader<'a>, } @@ -625,7 +641,7 @@ impl Drop for KeyPurposeIdIter<'_, '_> { /// An OID value indicating an Extended Key Usage (EKU) key purpose. #[derive(Clone, Copy)] -struct KeyPurposeId<'a> { +pub struct KeyPurposeId<'a> { oid_value: untrusted::Input<'a>, } @@ -633,7 +649,7 @@ impl<'a> KeyPurposeId<'a> { /// Construct a new [`KeyPurposeId`]. /// /// `oid` is the OBJECT IDENTIFIER in bytes. - const fn new(oid: &'a [u8]) -> Self { + pub const fn new(oid: &'a [u8]) -> Self { Self { oid_value: untrusted::Input::from(oid), } @@ -902,7 +918,7 @@ mod tests { #[test] fn eku_fail_empty() { let err = KeyUsage::required(EKU_SERVER_AUTH) - .check(KeyPurposeIdIter { + .validate(KeyPurposeIdIter { input: &mut untrusted::Reader::new(untrusted::Input::from(&[])), }) .unwrap_err();