From 51f63e4852f14df798a54bab88732e49a819974a Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 29 Sep 2023 20:53:57 -0700 Subject: [PATCH] Upgrade to *ring* 0.17 and untrusted 0.9. untrusted 0.9 is used by *ring*. untrusted stopped providing a `PartialEq` for `Input` in 0.9; this was the driver for all the code changes. --- Cargo.toml | 4 ++-- src/cert.rs | 4 ++-- src/lib.rs | 6 ++++++ src/name/verify.rs | 4 ++-- src/signed_data.rs | 17 ++++++++++------- src/verify_cert.rs | 17 ++++++++++------- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 43b603cb..5fa27cec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,8 +56,8 @@ alloc = ["ring/alloc"] std = ["alloc"] [dependencies] -ring = { version = "0.16.19", default-features = false } -untrusted = "0.7.1" +ring = { version = "0.17.0", default-features = false } +untrusted = "0.9" [dev-dependencies] base64 = "0.9.1" diff --git a/src/cert.rs b/src/cert.rs index 7c76f2e1..792f49fa 100644 --- a/src/cert.rs +++ b/src/cert.rs @@ -12,7 +12,7 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use crate::{der, signed_data, Error}; +use crate::{der, equal, signed_data, Error}; pub enum EndEntityOrCa<'a> { EndEntity, @@ -66,7 +66,7 @@ pub(crate) fn parse_cert_internal<'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 !equal(signature, signed_data.algorithm) { return Err(Error::SignatureAlgorithmMismatch); } diff --git a/src/lib.rs b/src/lib.rs index 32f46bca..d42f57d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,3 +95,9 @@ pub type TLSServerTrustAnchors<'a> = TlsServerTrustAnchors<'a>; #[deprecated(note = "use TlsClientTrustAnchors")] #[allow(unknown_lints, clippy::upper_case_acronyms)] pub type TLSClientTrustAnchors<'a> = TlsClientTrustAnchors<'a>; + +// We don't operate on secret data so a convenient comparison function is warranted. +#[must_use] +fn equal(a: untrusted::Input, b: untrusted::Input) -> bool { + a.as_slice_less_safe() == b.as_slice_less_safe() +} diff --git a/src/name/verify.rs b/src/name/verify.rs index bbe3aec0..699aea21 100644 --- a/src/name/verify.rs +++ b/src/name/verify.rs @@ -18,7 +18,7 @@ use super::{ }; use crate::{ cert::{Cert, EndEntityOrCa}, - der, Error, + der, equal, Error, }; pub fn verify_cert_dns_name( @@ -234,7 +234,7 @@ fn presented_directory_name_matches_constraint( subtrees: Subtrees, ) -> bool { match subtrees { - Subtrees::PermittedSubtrees => name == constraint, + Subtrees::PermittedSubtrees => equal(name, constraint), Subtrees::ExcludedSubtrees => true, } } diff --git a/src/signed_data.rs b/src/signed_data.rs index 8928434f..ccd834e6 100644 --- a/src/signed_data.rs +++ b/src/signed_data.rs @@ -12,7 +12,7 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use crate::{der, Error}; +use crate::{der, equal, Error}; use ring::signature; /// X.509 certificates and related items that are signed are almost always @@ -313,7 +313,7 @@ struct AlgorithmIdentifier { impl AlgorithmIdentifier { fn matches_algorithm_id_value(&self, encoded: untrusted::Input) -> bool { - encoded == self.asn1_id_value + equal(encoded, self.asn1_id_value) } } @@ -461,10 +461,12 @@ mod tests { let tsd = parse_test_signed_data(file_contents); let signature = untrusted::Input::from(&tsd.signature); assert_eq!( - Err(expected_error), - signature.read_all(Error::BadDer, |input| { - der::bit_string_with_no_unused_bits(input) - }) + expected_error, + signature + .read_all(Error::BadDer, |input| { + der::bit_string_with_no_unused_bits(input) + }) + .unwrap_err() ); } @@ -482,10 +484,11 @@ mod tests { let tsd = parse_test_signed_data(file_contents); let spki = untrusted::Input::from(&tsd.spki); assert_eq!( - Err(expected_error), + expected_error, spki.read_all(Error::BadDer, |input| { der::expect_tag_and_get_value(input, der::Tag::Sequence) }) + .unwrap_err() ); } diff --git a/src/verify_cert.rs b/src/verify_cert.rs index db55e4e5..2c243141 100644 --- a/src/verify_cert.rs +++ b/src/verify_cert.rs @@ -17,7 +17,7 @@ use core::default::Default; use crate::{ budget::Budget, cert::{self, Cert, EndEntityOrCa}, - der, + der, equal, error::ErrorOrInternalError, name, signed_data, time, Error, SignatureAlgorithm, TrustAnchor, }; @@ -89,7 +89,7 @@ fn build_chain_inner( match loop_while_non_fatal_error(trust_anchors, |trust_anchor: &TrustAnchor| { let trust_anchor_subject = untrusted::Input::from(trust_anchor.subject); - if cert.issuer != trust_anchor_subject { + if !equal(cert.issuer, trust_anchor_subject) { return Err(Error::UnknownIssuer.into()); } @@ -118,15 +118,15 @@ fn build_chain_inner( let potential_issuer = cert::parse_cert(untrusted::Input::from(cert_der), EndEntityOrCa::Ca(cert))?; - if potential_issuer.subject != cert.issuer { + if !equal(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 equal(potential_issuer.spki.value(), prev.spki.value()) + && equal(potential_issuer.subject, prev.subject) { return Err(Error::UnknownIssuer.into()); } @@ -361,7 +361,7 @@ fn check_eku( Some(input) => { loop { let value = der::expect_tag_and_get_value(input, der::Tag::OID)?; - if value == required_eku_if_present.oid_value { + if equal(value, required_eku_if_present.oid_value) { input.skip_to_end(); break; } @@ -381,7 +381,10 @@ fn check_eku( // important that id-kp-OCSPSigning is explicit so that a normal // end-entity certificate isn't able to sign trusted OCSP responses // for itself or for other certificates issued by its issuing CA. - if required_eku_if_present.oid_value == EKU_OCSP_SIGNING.oid_value { + if equal( + required_eku_if_present.oid_value, + EKU_OCSP_SIGNING.oid_value, + ) { return Err(Error::RequiredEkuNotFound); }