diff --git a/Cargo.toml b/Cargo.toml index d2d26978..ac5c1665 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,8 +67,8 @@ alloc = ["ring/alloc"] std = ["alloc"] [dependencies] -ring = { version = "0.16.19", default-features = false } -untrusted = "0.7.1" +ring = { version = "0.17", default-features = false } +untrusted = "0.9" [dev-dependencies] base64 = "0.21" diff --git a/src/cert.rs b/src/cert.rs index 3d157545..b2c260d8 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -15,7 +15,7 @@ use crate::der::Tag; use crate::signed_data::SignedData; use crate::x509::{remember_extension, set_extension_once, Extension}; -use crate::{der, Error}; +use crate::{der, public_values_eq, Error}; /// An enumeration indicating whether a [`Cert`] is a leaf end-entity cert, or a linked /// list node from the CA `Cert` to a child `Cert` it issued. @@ -70,7 +70,7 @@ impl<'a> Cert<'a> { // TODO: In mozilla::pkix, the comparison is done based on the // normalized value (ignoring whether or not there is an optional NULL // parameter for RSA-based algorithms), so this may be too strict. - if signature != signed_data.algorithm { + if !public_values_eq(signature, signed_data.algorithm) { return Err(Error::SignatureAlgorithmMismatch); } diff --git a/src/crl.rs b/src/crl.rs index 1111383c..13f300ab 100644 --- a/src/crl.rs +++ b/src/crl.rs @@ -17,7 +17,7 @@ use crate::der::Tag; use crate::signed_data::{self, SignedData}; use crate::verify_cert::Budget; use crate::x509::{remember_extension, set_extension_once, Extension}; -use crate::{der, Error, SignatureAlgorithm, Time}; +use crate::{der, public_values_eq, Error, SignatureAlgorithm, Time}; #[cfg(feature = "alloc")] use std::collections::HashMap; @@ -155,7 +155,7 @@ impl<'a> BorrowedCertRevocationList<'a> { // This field MUST contain the same algorithm identifier as the // signatureAlgorithm field in the sequence CertificateList let signature = der::expect_tag_and_get_value(tbs_cert_list, Tag::Sequence)?; - if signature != signed_data.algorithm { + if !public_values_eq(signature, signed_data.algorithm) { return Err(Error::SignatureAlgorithmMismatch); } diff --git a/src/lib.rs b/src/lib.rs index b6229db9..66811550 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -92,3 +92,7 @@ pub use { }, subject_name::{DnsName, IpAddr}, }; + +fn public_values_eq(a: untrusted::Input<'_>, b: untrusted::Input<'_>) -> bool { + a.as_slice_less_safe() == b.as_slice_less_safe() +} diff --git a/src/signed_data.rs b/src/signed_data.rs index 03fa282d..078b8d0c 100644 --- a/src/signed_data.rs +++ b/src/signed_data.rs @@ -13,7 +13,7 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. use crate::verify_cert::Budget; -use crate::{der, Error}; +use crate::{der, public_values_eq, Error}; use ring::signature; #[cfg(feature = "alloc")] @@ -377,7 +377,7 @@ struct AlgorithmIdentifier { impl AlgorithmIdentifier { fn matches_algorithm_id_value(&self, encoded: untrusted::Input) -> bool { - encoded == self.asn1_id_value + public_values_eq(encoded, self.asn1_id_value) } } diff --git a/src/verify_cert.rs b/src/verify_cert.rs index d188c5e8..c9c65216 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -17,8 +17,8 @@ use core::ops::ControlFlow; use crate::{ cert::{Cert, EndEntityOrCa}, - der, signed_data, subject_name, time, CertRevocationList, Error, SignatureAlgorithm, - TrustAnchor, + der, public_values_eq, signed_data, subject_name, time, CertRevocationList, Error, + SignatureAlgorithm, TrustAnchor, }; pub(crate) struct ChainOptions<'a> { @@ -67,7 +67,7 @@ fn build_chain_inner( opts.trust_anchors, |trust_anchor: &TrustAnchor| { let trust_anchor_subject = untrusted::Input::from(trust_anchor.subject); - if cert.issuer != trust_anchor_subject { + if !public_values_eq(cert.issuer, trust_anchor_subject) { return Err(Error::UnknownIssuer.into()); } @@ -101,15 +101,15 @@ fn build_chain_inner( let potential_issuer = Cert::from_der(untrusted::Input::from(cert_der), EndEntityOrCa::Ca(cert))?; - if potential_issuer.subject != cert.issuer { + if !public_values_eq(potential_issuer.subject, cert.issuer) { return Err(Error::UnknownIssuer.into()); } // Prevent loops; see RFC 4158 section 5.2. let mut prev = cert; loop { - if potential_issuer.spki.value() == prev.spki.value() - && potential_issuer.subject == prev.subject + if public_values_eq(potential_issuer.spki.value(), prev.spki.value()) + && public_values_eq(potential_issuer.subject, prev.subject) { return Err(Error::UnknownIssuer.into()); } @@ -280,7 +280,7 @@ fn check_crls( crls: &[&dyn CertRevocationList], budget: &mut Budget, ) -> Result, Error> { - assert_eq!(cert.issuer, issuer_subject); + assert!(public_values_eq(cert.issuer, issuer_subject)); let crl = match crls .iter() @@ -500,17 +500,19 @@ impl ExtendedKeyUsage { } fn key_purpose_id_equals(&self, value: untrusted::Input<'_>) -> bool { - match self { - ExtendedKeyUsage::Required(eku) => *eku, - ExtendedKeyUsage::RequiredIfPresent(eku) => *eku, - } - .oid_value - == value + public_values_eq( + match self { + ExtendedKeyUsage::Required(eku) => *eku, + ExtendedKeyUsage::RequiredIfPresent(eku) => *eku, + } + .oid_value, + value, + ) } } /// An OID value indicating an Extended Key Usage (EKU) key purpose. -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy)] struct KeyPurposeId { oid_value: untrusted::Input<'static>, } @@ -526,6 +528,14 @@ impl KeyPurposeId { } } +impl PartialEq for KeyPurposeId { + fn eq(&self, other: &Self) -> bool { + public_values_eq(self.oid_value, other.oid_value) + } +} + +impl Eq for KeyPurposeId {} + // id-pkix OBJECT IDENTIFIER ::= { 1 3 6 1 5 5 7 } // id-kp OBJECT IDENTIFIER ::= { id-pkix 3 }