From 81656d5a883b8f861f6c29c711aeb8d503b9e950 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 2 May 2023 14:57:35 -0400 Subject: [PATCH] subject_name: more specific errs for invalid names. Previously the `presented_id_matches_constraint` and `presented_id_matches_reference_id` functions used by `verify_cert_dns_name` would return an `Option`, with the `None` case translated into `Error::BadDer`. This makes it difficult for downstream users to know that the encoding error was specific to an invalid presented DNS ID, or a name constraint. This commit introduces two new error variants: `MalformedDnsIdentifier` and `MalformedNameConstraint`. The `presented_id_matches_constraint` and `presented_id_matches_reference_id` functions are changed to return `Result` using the new error types. --- src/error.rs | 8 + src/subject_name/dns_name.rs | 517 +++++++++++++++++++++++------------ src/subject_name/verify.rs | 8 +- 3 files changed, 361 insertions(+), 172 deletions(-) diff --git a/src/error.rs b/src/error.rs index 47961ab9..9667337a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -101,6 +101,14 @@ pub enum Error { /// - it had a sparse network mask (ie, cannot be written in CIDR form). /// - it was too long or short InvalidNetworkMaskConstraint, + + /// A presented or reference DNS identifier was malformed, potentially + /// containing invalid characters or invalid labels. + MalformedDnsIdentifier, + + /// A name constraint was malformed, potentially containing invalid characters or + /// invalid labels. + MalformedNameConstraint, } impl fmt::Display for Error { diff --git a/src/subject_name/dns_name.rs b/src/subject_name/dns_name.rs index 7294ab9e..d5028770 100644 --- a/src/subject_name/dns_name.rs +++ b/src/subject_name/dns_name.rs @@ -16,6 +16,8 @@ use alloc::string::String; use core::fmt::Write; +use crate::Error; + /// A DNS Name suitable for use in the TLS Server Name Indication (SNI) /// extension and/or for use as the reference hostname for which to verify a /// certificate. @@ -244,7 +246,7 @@ impl AsRef for WildcardDnsNameRef<'_> { pub(super) fn presented_id_matches_reference_id( presented_dns_id: untrusted::Input, reference_dns_id: untrusted::Input, -) -> Option { +) -> Result { presented_id_matches_reference_id_internal( presented_dns_id, IdRole::Reference, @@ -255,7 +257,7 @@ pub(super) fn presented_id_matches_reference_id( pub(super) fn presented_id_matches_constraint( presented_dns_id: untrusted::Input, reference_dns_id: untrusted::Input, -) -> Option { +) -> Result { presented_id_matches_reference_id_internal( presented_dns_id, IdRole::NameConstraint, @@ -263,10 +265,6 @@ pub(super) fn presented_id_matches_constraint( ) } -// We do not distinguish between a syntactically-invalid presented_dns_id and -// one that is syntactically valid but does not match reference_dns_id; in both -// cases, the result is false. -// // We assume that both presented_dns_id and reference_dns_id are encoded in // such a way that US-ASCII (7-bit) characters are encoded in one byte and no // encoding of a non-US-ASCII character contains a code point in the range @@ -387,13 +385,16 @@ fn presented_id_matches_reference_id_internal( presented_dns_id: untrusted::Input, reference_dns_id_role: IdRole, reference_dns_id: untrusted::Input, -) -> Option { +) -> Result { if !is_valid_dns_id(presented_dns_id, IdRole::Presented, AllowWildcards::Yes) { - return None; + return Err(Error::MalformedDnsIdentifier); } if !is_valid_dns_id(reference_dns_id, reference_dns_id_role, AllowWildcards::No) { - return None; + return Err(match reference_dns_id_role { + IdRole::NameConstraint => Error::MalformedNameConstraint, + _ => Error::MalformedDnsIdentifier, + }); } let mut presented = untrusted::Reader::new(presented_dns_id); @@ -405,7 +406,7 @@ fn presented_id_matches_reference_id_internal( IdRole::NameConstraint if presented_dns_id.len() > reference_dns_id.len() => { if reference_dns_id.is_empty() { // An empty constraint matches everything. - return Some(true); + return Ok(true); } // If the reference ID starts with a dot then skip the prefix of @@ -446,7 +447,7 @@ fn presented_id_matches_reference_id_internal( unreachable!(); } if presented.read_byte() != Ok(b'.') { - return Some(false); + return Ok(false); } } } @@ -464,7 +465,7 @@ fn presented_id_matches_reference_id_internal( loop { if reference.read_byte().is_err() { - return Some(false); + return Ok(false); } if reference.peek(b'.') { break; @@ -476,14 +477,14 @@ fn presented_id_matches_reference_id_internal( let presented_byte = match (presented.read_byte(), reference.read_byte()) { (Ok(p), Ok(r)) if ascii_lower(p) == ascii_lower(r) => p, _ => { - return Some(false); + return Ok(false); } }; if presented.at_end() { // Don't allow presented IDs to be absolute. if presented_byte == b'.' { - return None; + return Err(Error::MalformedDnsIdentifier); } break; } @@ -496,19 +497,19 @@ fn presented_id_matches_reference_id_internal( match reference.read_byte() { Ok(b'.') => (), _ => { - return Some(false); + return Ok(false); } }; } if !reference.at_end() { - return Some(false); + return Ok(false); } } assert!(presented.at_end()); assert!(reference.at_end()); - Some(true) + Ok(true) } #[inline] @@ -671,200 +672,364 @@ fn is_valid_dns_id( mod tests { use super::*; - const PRESENTED_MATCHES_REFERENCE: &[(&[u8], &[u8], Option)] = &[ - (b"", b"a", None), - (b"a", b"a", Some(true)), - (b"b", b"a", Some(false)), - (b"*.b.a", b"c.b.a", Some(true)), - (b"*.b.a", b"b.a", Some(false)), - (b"*.b.a", b"b.a.", Some(false)), + const PRESENTED_MATCHES_REFERENCE: &[(&[u8], &[u8], Result)] = &[ + (b"", b"a", Err(Error::MalformedDnsIdentifier)), + (b"a", b"a", Ok(true)), + (b"b", b"a", Ok(false)), + (b"*.b.a", b"c.b.a", Ok(true)), + (b"*.b.a", b"b.a", Ok(false)), + (b"*.b.a", b"b.a.", Ok(false)), // Wildcard not in leftmost label - (b"d.c.b.a", b"d.c.b.a", Some(true)), - (b"d.*.b.a", b"d.c.b.a", None), - (b"d.c*.b.a", b"d.c.b.a", None), - (b"d.c*.b.a", b"d.cc.b.a", None), + (b"d.c.b.a", b"d.c.b.a", Ok(true)), + (b"d.*.b.a", b"d.c.b.a", Err(Error::MalformedDnsIdentifier)), + (b"d.c*.b.a", b"d.c.b.a", Err(Error::MalformedDnsIdentifier)), + (b"d.c*.b.a", b"d.cc.b.a", Err(Error::MalformedDnsIdentifier)), // case sensitivity ( b"abcdefghijklmnopqrstuvwxyz", b"ABCDEFGHIJKLMNOPQRSTUVWXYZ", - Some(true), + Ok(true), ), ( b"ABCDEFGHIJKLMNOPQRSTUVWXYZ", b"abcdefghijklmnopqrstuvwxyz", - Some(true), + Ok(true), ), - (b"aBc", b"Abc", Some(true)), + (b"aBc", b"Abc", Ok(true)), // digits - (b"a1", b"a1", Some(true)), + (b"a1", b"a1", Ok(true)), // A trailing dot indicates an absolute name, and absolute names can match // relative names, and vice-versa. - (b"example", b"example", Some(true)), - (b"example.", b"example.", None), - (b"example", b"example.", Some(true)), - (b"example.", b"example", None), - (b"example.com", b"example.com", Some(true)), - (b"example.com.", b"example.com.", None), - (b"example.com", b"example.com.", Some(true)), - (b"example.com.", b"example.com", None), - (b"example.com..", b"example.com.", None), - (b"example.com..", b"example.com", None), - (b"example.com...", b"example.com.", None), + (b"example", b"example", Ok(true)), + (b"example.", b"example.", Err(Error::MalformedDnsIdentifier)), + (b"example", b"example.", Ok(true)), + (b"example.", b"example", Err(Error::MalformedDnsIdentifier)), + (b"example.com", b"example.com", Ok(true)), + ( + b"example.com.", + b"example.com.", + Err(Error::MalformedDnsIdentifier), + ), + (b"example.com", b"example.com.", Ok(true)), + ( + b"example.com.", + b"example.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"example.com..", + b"example.com.", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"example.com..", + b"example.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"example.com...", + b"example.com.", + Err(Error::MalformedDnsIdentifier), + ), // xn-- IDN prefix - (b"x*.b.a", b"xa.b.a", None), - (b"x*.b.a", b"xna.b.a", None), - (b"x*.b.a", b"xn-a.b.a", None), - (b"x*.b.a", b"xn--a.b.a", None), - (b"xn*.b.a", b"xn--a.b.a", None), - (b"xn-*.b.a", b"xn--a.b.a", None), - (b"xn--*.b.a", b"xn--a.b.a", None), - (b"xn*.b.a", b"xn--a.b.a", None), - (b"xn-*.b.a", b"xn--a.b.a", None), - (b"xn--*.b.a", b"xn--a.b.a", None), - (b"xn---*.b.a", b"xn--a.b.a", None), + (b"x*.b.a", b"xa.b.a", Err(Error::MalformedDnsIdentifier)), + (b"x*.b.a", b"xna.b.a", Err(Error::MalformedDnsIdentifier)), + (b"x*.b.a", b"xn-a.b.a", Err(Error::MalformedDnsIdentifier)), + (b"x*.b.a", b"xn--a.b.a", Err(Error::MalformedDnsIdentifier)), + (b"xn*.b.a", b"xn--a.b.a", Err(Error::MalformedDnsIdentifier)), + ( + b"xn-*.b.a", + b"xn--a.b.a", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"xn--*.b.a", + b"xn--a.b.a", + Err(Error::MalformedDnsIdentifier), + ), + (b"xn*.b.a", b"xn--a.b.a", Err(Error::MalformedDnsIdentifier)), + ( + b"xn-*.b.a", + b"xn--a.b.a", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"xn--*.b.a", + b"xn--a.b.a", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"xn---*.b.a", + b"xn--a.b.a", + Err(Error::MalformedDnsIdentifier), + ), // "*" cannot expand to nothing. - (b"c*.b.a", b"c.b.a", None), + (b"c*.b.a", b"c.b.a", Err(Error::MalformedDnsIdentifier)), // -------------------------------------------------------------------------- // The rest of these are test cases adapted from Chromium's // x509_certificate_unittest.cc. The parameter order is the opposite in - // Chromium's tests. Also, they some tests were modified to fit into this + // Chromium's tests. Also, they Ok tests were modified to fit into this // framework or due to intentional differences between mozilla::pkix and // Chromium. - (b"foo.com", b"foo.com", Some(true)), - (b"f", b"f", Some(true)), - (b"i", b"h", Some(false)), - (b"*.foo.com", b"bar.foo.com", Some(true)), - (b"*.test.fr", b"www.test.fr", Some(true)), - (b"*.test.FR", b"wwW.tESt.fr", Some(true)), - (b".uk", b"f.uk", None), - (b"?.bar.foo.com", b"w.bar.foo.com", None), - (b"(www|ftp).foo.com", b"www.foo.com", None), // regex! - (b"www.foo.com\0", b"www.foo.com", None), - (b"www.foo.com\0*.foo.com", b"www.foo.com", None), - (b"ww.house.example", b"www.house.example", Some(false)), - (b"www.test.org", b"test.org", Some(false)), - (b"*.test.org", b"test.org", Some(false)), - (b"*.org", b"test.org", None), + (b"foo.com", b"foo.com", Ok(true)), + (b"f", b"f", Ok(true)), + (b"i", b"h", Ok(false)), + (b"*.foo.com", b"bar.foo.com", Ok(true)), + (b"*.test.fr", b"www.test.fr", Ok(true)), + (b"*.test.FR", b"wwW.tESt.fr", Ok(true)), + (b".uk", b"f.uk", Err(Error::MalformedDnsIdentifier)), + ( + b"?.bar.foo.com", + b"w.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"(www|ftp).foo.com", + b"www.foo.com", + Err(Error::MalformedDnsIdentifier), + ), // regex! + ( + b"www.foo.com\0", + b"www.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"www.foo.com\0*.foo.com", + b"www.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + (b"ww.house.example", b"www.house.example", Ok(false)), + (b"www.test.org", b"test.org", Ok(false)), + (b"*.test.org", b"test.org", Ok(false)), + (b"*.org", b"test.org", Err(Error::MalformedDnsIdentifier)), // '*' must be the only character in the wildcard label - (b"w*.bar.foo.com", b"w.bar.foo.com", None), - (b"ww*ww.bar.foo.com", b"www.bar.foo.com", None), - (b"ww*ww.bar.foo.com", b"wwww.bar.foo.com", None), - (b"w*w.bar.foo.com", b"wwww.bar.foo.com", None), - (b"w*w.bar.foo.c0m", b"wwww.bar.foo.com", None), - (b"wa*.bar.foo.com", b"WALLY.bar.foo.com", None), - (b"*Ly.bar.foo.com", b"wally.bar.foo.com", None), + ( + b"w*.bar.foo.com", + b"w.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"ww*ww.bar.foo.com", + b"www.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"ww*ww.bar.foo.com", + b"wwww.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"w*w.bar.foo.com", + b"wwww.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"w*w.bar.foo.c0m", + b"wwww.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"wa*.bar.foo.com", + b"WALLY.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*Ly.bar.foo.com", + b"wally.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), // Chromium does URL decoding of the reference ID, but we don't, and we also // require that the reference ID is valid, so we can't test these two. - // (b"www.foo.com", b"ww%57.foo.com", Some(true)), - // (b"www&.foo.com", b"www%26.foo.com", Some(true)), - (b"*.test.de", b"www.test.co.jp", Some(false)), - (b"*.jp", b"www.test.co.jp", None), - (b"www.test.co.uk", b"www.test.co.jp", Some(false)), - (b"www.*.co.jp", b"www.test.co.jp", None), - (b"www.bar.foo.com", b"www.bar.foo.com", Some(true)), - (b"*.foo.com", b"www.bar.foo.com", Some(false)), - (b"*.*.foo.com", b"www.bar.foo.com", None), + // (b"www.foo.com", b"ww%57.foo.com", Ok(true)), + // (b"www&.foo.com", b"www%26.foo.com", Ok(true)), + (b"*.test.de", b"www.test.co.jp", Ok(false)), + ( + b"*.jp", + b"www.test.co.jp", + Err(Error::MalformedDnsIdentifier), + ), + (b"www.test.co.uk", b"www.test.co.jp", Ok(false)), + ( + b"www.*.co.jp", + b"www.test.co.jp", + Err(Error::MalformedDnsIdentifier), + ), + (b"www.bar.foo.com", b"www.bar.foo.com", Ok(true)), + (b"*.foo.com", b"www.bar.foo.com", Ok(false)), + ( + b"*.*.foo.com", + b"www.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), // Our matcher requires the reference ID to be a valid DNS name, so we cannot // test this case. - // (b"*.*.bar.foo.com", b"*..bar.foo.com", Some(false)), - (b"www.bath.org", b"www.bath.org", Some(true)), + // (b"*.*.bar.foo.com", b"*..bar.foo.com", Ok(false)), + (b"www.bath.org", b"www.bath.org", Ok(true)), // Our matcher requires the reference ID to be a valid DNS name, so we cannot // test these cases. // DNS_ID_MISMATCH("www.bath.org", ""), - // (b"www.bath.org", b"20.30.40.50", Some(false)), - // (b"www.bath.org", b"66.77.88.99", Some(false)), + // (b"www.bath.org", b"20.30.40.50", Ok(false)), + // (b"www.bath.org", b"66.77.88.99", Ok(false)), // IDN tests ( b"xn--poema-9qae5a.com.br", b"xn--poema-9qae5a.com.br", - Some(true), + Ok(true), ), ( b"*.xn--poema-9qae5a.com.br", b"www.xn--poema-9qae5a.com.br", - Some(true), + Ok(true), ), ( b"*.xn--poema-9qae5a.com.br", b"xn--poema-9qae5a.com.br", - Some(false), + Ok(false), + ), + ( + b"xn--poema-*.com.br", + b"xn--poema-9qae5a.com.br", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"xn--*-9qae5a.com.br", + b"xn--poema-9qae5a.com.br", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*--poema-9qae5a.com.br", + b"xn--poema-9qae5a.com.br", + Err(Error::MalformedDnsIdentifier), ), - (b"xn--poema-*.com.br", b"xn--poema-9qae5a.com.br", None), - (b"xn--*-9qae5a.com.br", b"xn--poema-9qae5a.com.br", None), - (b"*--poema-9qae5a.com.br", b"xn--poema-9qae5a.com.br", None), // The following are adapted from the examples quoted from // http://tools.ietf.org/html/rfc6125#section-6.4.3 // (e.g., *.example.com would match foo.example.com but // not bar.foo.example.com or example.com). - (b"*.example.com", b"foo.example.com", Some(true)), - (b"*.example.com", b"bar.foo.example.com", Some(false)), - (b"*.example.com", b"example.com", Some(false)), - (b"baz*.example.net", b"baz1.example.net", None), - (b"*baz.example.net", b"foobaz.example.net", None), - (b"b*z.example.net", b"buzz.example.net", None), + (b"*.example.com", b"foo.example.com", Ok(true)), + (b"*.example.com", b"bar.foo.example.com", Ok(false)), + (b"*.example.com", b"example.com", Ok(false)), + ( + b"baz*.example.net", + b"baz1.example.net", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*baz.example.net", + b"foobaz.example.net", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"b*z.example.net", + b"buzz.example.net", + Err(Error::MalformedDnsIdentifier), + ), // Wildcards should not be valid for public registry controlled domains, // and unknown/unrecognized domains, at least three domain components must // be present. For mozilla::pkix and NSS, there must always be at least two // labels after the wildcard label. - (b"*.test.example", b"www.test.example", Some(true)), - (b"*.example.co.uk", b"test.example.co.uk", Some(true)), - (b"*.example", b"test.example", None), + (b"*.test.example", b"www.test.example", Ok(true)), + (b"*.example.co.uk", b"test.example.co.uk", Ok(true)), + ( + b"*.example", + b"test.example", + Err(Error::MalformedDnsIdentifier), + ), // The result is different than Chromium, because Chromium takes into account // the additional knowledge it has that "co.uk" is a TLD. mozilla::pkix does // not know that. - (b"*.co.uk", b"example.co.uk", Some(true)), - (b"*.com", b"foo.com", None), - (b"*.us", b"foo.us", None), - (b"*", b"foo", None), + (b"*.co.uk", b"example.co.uk", Ok(true)), + (b"*.com", b"foo.com", Err(Error::MalformedDnsIdentifier)), + (b"*.us", b"foo.us", Err(Error::MalformedDnsIdentifier)), + (b"*", b"foo", Err(Error::MalformedDnsIdentifier)), // IDN variants of wildcards and registry controlled domains. ( b"*.xn--poema-9qae5a.com.br", b"www.xn--poema-9qae5a.com.br", - Some(true), + Ok(true), ), ( b"*.example.xn--mgbaam7a8h", b"test.example.xn--mgbaam7a8h", - Some(true), + Ok(true), ), // RFC6126 allows this, and NSS accepts it, but Chromium disallows it. // TODO: File bug against Chromium. - (b"*.com.br", b"xn--poema-9qae5a.com.br", Some(true)), - (b"*.xn--mgbaam7a8h", b"example.xn--mgbaam7a8h", None), + (b"*.com.br", b"xn--poema-9qae5a.com.br", Ok(true)), + ( + b"*.xn--mgbaam7a8h", + b"example.xn--mgbaam7a8h", + Err(Error::MalformedDnsIdentifier), + ), // Wildcards should be permissible for 'private' registry-controlled // domains. (In mozilla::pkix, we do not know if it is a private registry- // controlled domain or not.) - (b"*.appspot.com", b"www.appspot.com", Some(true)), - (b"*.s3.amazonaws.com", b"foo.s3.amazonaws.com", Some(true)), + (b"*.appspot.com", b"www.appspot.com", Ok(true)), + (b"*.s3.amazonaws.com", b"foo.s3.amazonaws.com", Ok(true)), // Multiple wildcards are not valid. - (b"*.*.com", b"foo.example.com", None), - (b"*.bar.*.com", b"foo.bar.example.com", None), + ( + b"*.*.com", + b"foo.example.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*.bar.*.com", + b"foo.bar.example.com", + Err(Error::MalformedDnsIdentifier), + ), // Absolute vs relative DNS name tests. Although not explicitly specified // in RFC 6125, absolute reference names (those ending in a .) should // match either absolute or relative presented names. // TODO: File errata against RFC 6125 about this. - (b"foo.com.", b"foo.com", None), - (b"foo.com", b"foo.com.", Some(true)), - (b"foo.com.", b"foo.com.", None), - (b"f.", b"f", None), - (b"f", b"f.", Some(true)), - (b"f.", b"f.", None), - (b"*.bar.foo.com.", b"www-3.bar.foo.com", None), - (b"*.bar.foo.com", b"www-3.bar.foo.com.", Some(true)), - (b"*.bar.foo.com.", b"www-3.bar.foo.com.", None), + (b"foo.com.", b"foo.com", Err(Error::MalformedDnsIdentifier)), + (b"foo.com", b"foo.com.", Ok(true)), + (b"foo.com.", b"foo.com.", Err(Error::MalformedDnsIdentifier)), + (b"f.", b"f", Err(Error::MalformedDnsIdentifier)), + (b"f", b"f.", Ok(true)), + (b"f.", b"f.", Err(Error::MalformedDnsIdentifier)), + ( + b"*.bar.foo.com.", + b"www-3.bar.foo.com", + Err(Error::MalformedDnsIdentifier), + ), + (b"*.bar.foo.com", b"www-3.bar.foo.com.", Ok(true)), + ( + b"*.bar.foo.com.", + b"www-3.bar.foo.com.", + Err(Error::MalformedDnsIdentifier), + ), // We require the reference ID to be a valid DNS name, so we cannot test this // case. - // (b".", b".", Some(false)), - (b"*.com.", b"example.com", None), - (b"*.com", b"example.com.", None), - (b"*.com.", b"example.com.", None), - (b"*.", b"foo.", None), - (b"*.", b"foo", None), + // (b".", b".", Ok(false)), + ( + b"*.com.", + b"example.com", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*.com", + b"example.com.", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*.com.", + b"example.com.", + Err(Error::MalformedDnsIdentifier), + ), + (b"*.", b"foo.", Err(Error::MalformedDnsIdentifier)), + (b"*.", b"foo", Err(Error::MalformedDnsIdentifier)), // The result is different than Chromium because we don't know that co.uk is // a TLD. - (b"*.co.uk.", b"foo.co.uk", None), - (b"*.co.uk.", b"foo.co.uk.", None), + ( + b"*.co.uk.", + b"foo.co.uk", + Err(Error::MalformedDnsIdentifier), + ), + ( + b"*.co.uk.", + b"foo.co.uk.", + Err(Error::MalformedDnsIdentifier), + ), ]; #[test] @@ -883,47 +1048,63 @@ mod tests { } // (presented_name, constraint, expected_matches) - const PRESENTED_MATCHES_CONSTRAINT: &[(&[u8], &[u8], Option)] = &[ + const PRESENTED_MATCHES_CONSTRAINT: &[(&[u8], &[u8], Result)] = &[ // No absolute presented IDs allowed - (b".", b"", None), - (b"www.example.com.", b"", None), - (b"www.example.com.", b"www.example.com.", None), + (b".", b"", Err(Error::MalformedDnsIdentifier)), + (b"www.example.com.", b"", Err(Error::MalformedDnsIdentifier)), + ( + b"www.example.com.", + b"www.example.com.", + Err(Error::MalformedDnsIdentifier), + ), // No absolute constraints allowed - (b"www.example.com", b".", None), - (b"www.example.com", b"www.example.com.", None), + ( + b"www.example.com", + b".", + Err(Error::MalformedNameConstraint), + ), + ( + b"www.example.com", + b"www.example.com.", + Err(Error::MalformedNameConstraint), + ), // No wildcard in constraints allowed - (b"www.example.com", b"*.example.com", None), + ( + b"www.example.com", + b"*.example.com", + Err(Error::MalformedNameConstraint), + ), // No empty presented IDs allowed - (b"", b"", None), + (b"", b"", Err(Error::MalformedDnsIdentifier)), // Empty constraints match everything allowed - (b"example.com", b"", Some(true)), - (b"*.example.com", b"", Some(true)), + (b"example.com", b"", Ok(true)), + (b"*.example.com", b"", Ok(true)), // Constraints that start with a dot - (b"www.example.com", b".example.com", Some(true)), - (b"www.example.com", b".EXAMPLE.COM", Some(true)), - (b"www.example.com", b".axample.com", Some(false)), - (b"www.example.com", b".xample.com", Some(false)), - (b"www.example.com", b".exampl.com", Some(false)), - (b"badexample.com", b".example.com", Some(false)), + (b"www.example.com", b".example.com", Ok(true)), + (b"www.example.com", b".EXAMPLE.COM", Ok(true)), + (b"www.example.com", b".axample.com", Ok(false)), + (b"www.example.com", b".xample.com", Ok(false)), + (b"www.example.com", b".exampl.com", Ok(false)), + (b"badexample.com", b".example.com", Ok(false)), // Constraints that do not start with a dot - (b"www.example.com", b"example.com", Some(true)), - (b"www.example.com", b"EXAMPLE.COM", Some(true)), - (b"www.example.com", b"axample.com", Some(false)), - (b"www.example.com", b"xample.com", Some(false)), - (b"www.example.com", b"exampl.com", Some(false)), - (b"badexample.com", b"example.com", Some(false)), + (b"www.example.com", b"example.com", Ok(true)), + (b"www.example.com", b"EXAMPLE.COM", Ok(true)), + (b"www.example.com", b"axample.com", Ok(false)), + (b"www.example.com", b"xample.com", Ok(false)), + (b"www.example.com", b"exampl.com", Ok(false)), + (b"badexample.com", b"example.com", Ok(false)), // Presented IDs with wildcard - (b"*.example.com", b".example.com", Some(true)), - (b"*.example.com", b"example.com", Some(true)), - (b"*.example.com", b"www.example.com", Some(true)), - (b"*.example.com", b"www.EXAMPLE.COM", Some(true)), - (b"*.example.com", b"www.axample.com", Some(false)), - (b"*.example.com", b".xample.com", Some(false)), - (b"*.example.com", b"xample.com", Some(false)), - (b"*.example.com", b".exampl.com", Some(false)), - (b"*.example.com", b"exampl.com", Some(false)), + (b"*.example.com", b".example.com", Ok(true)), + (b"*.example.com", b"example.com", Ok(true)), + (b"*.example.com", b"www.example.com", Ok(true)), + (b"*.example.com", b"www.EXAMPLE.COM", Ok(true)), + (b"*.example.com", b"www.axample.com", Ok(false)), + (b"*.example.com", b".xample.com", Ok(false)), + (b"*.example.com", b"xample.com", Ok(false)), + (b"*.example.com", b".exampl.com", Ok(false)), + (b"*.example.com", b"exampl.com", Ok(false)), // Matching IDs - (b"www.example.com", b"www.example.com", Some(true)), + (b"www.example.com", b"www.example.com", Ok(true)), ]; #[test] @@ -936,7 +1117,7 @@ mod tests { assert_eq!( actual_result, expected_result, "presented_id_matches_constraint(\"{:?}\", \"{:?}\")", - presented, constraint + presented, constraint, ); } } diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs index a2bfc7b5..489436a1 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -41,9 +41,9 @@ pub(crate) fn verify_cert_dns_name( &mut |name| { if let GeneralName::DnsName(presented_id) = name { match dns_name::presented_id_matches_reference_id(presented_id, dns_name) { - Some(true) => return NameIteration::Stop(Ok(())), - Some(false) => (), - None => return NameIteration::Stop(Err(Error::BadDer)), + Ok(true) => return NameIteration::Stop(Ok(())), + Ok(false) => (), + Err(e) => return NameIteration::Stop(Err(e)), } } NameIteration::KeepGoing @@ -208,7 +208,7 @@ fn check_presented_id_conforms_to_constraints_in_subtree( let matches = match (name, base) { (GeneralName::DnsName(name), GeneralName::DnsName(base)) => { - dns_name::presented_id_matches_constraint(name, base).ok_or(Error::BadDer) + dns_name::presented_id_matches_constraint(name, base) } (GeneralName::DirectoryName(name), GeneralName::DirectoryName(base)) => Ok(