diff --git a/src/end_entity.rs b/src/end_entity.rs index fb3a402a..cf74fe8e 100644 --- a/src/end_entity.rs +++ b/src/end_entity.rs @@ -12,6 +12,8 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +#[cfg(feature = "alloc")] +use crate::subject_name::GeneralDnsNameRef; use crate::{ cert, signed_data, subject_name, verify_cert, Error, SignatureAlgorithm, SubjectNameRef, Time, TlsClientTrustAnchors, TlsServerTrustAnchors, @@ -174,4 +176,17 @@ impl<'a> EndEntityCert<'a> { untrusted::Input::from(signature), ) } + + /// Returns a list of the DNS names provided in the subject alternative names extension + /// + /// This function must not be used to implement custom DNS name verification. + /// Verification functions are already provided as `verify_is_valid_for_dns_name` + /// and `verify_is_valid_for_at_least_one_dns_name`. + /// + /// Requires the `alloc` default feature; i.e. this isn't available in + /// `#![no_std]` configurations. + #[cfg(feature = "alloc")] + pub fn dns_names(&'a self) -> Result>, Error> { + subject_name::list_cert_dns_names(self) + } } diff --git a/src/subject_name/dns_name.rs b/src/subject_name/dns_name.rs index 54e3c979..d28c42fe 100644 --- a/src/subject_name/dns_name.rs +++ b/src/subject_name/dns_name.rs @@ -14,6 +14,7 @@ #[cfg(feature = "alloc")] use alloc::string::String; +use core::fmt::Write; /// 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 @@ -21,14 +22,10 @@ use alloc::string::String; /// /// A `DnsName` is guaranteed to be syntactically valid. The validity rules are /// specified in [RFC 5280 Section 7.2], except that underscores are also -/// allowed. +/// allowed. `DnsName`s do not include wildcard labels. /// /// `DnsName` stores a copy of the input it was constructed from in a `String` -/// and so it is only available when the `std` default feature is enabled. -/// -/// `Eq`, `PartialEq`, etc. are not implemented because name comparison -/// frequently should be done case-insensitively and/or with other caveats that -/// depend on the specific circumstances in which the comparison is done. +/// and so it is only available when the `alloc` default feature is enabled. /// /// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2 /// @@ -69,14 +66,10 @@ impl From> for DnsName { /// /// A `DnsNameRef` is guaranteed to be syntactically valid. The validity rules /// are specified in [RFC 5280 Section 7.2], except that underscores are also -/// allowed. -/// -/// `Eq`, `PartialEq`, etc. are not implemented because name comparison -/// frequently should be done case-insensitively and/or with other caveats that -/// depend on the specific circumstances in which the comparison is done. +/// allowed. `DnsNameRef`s do not include wildcard labels. /// /// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2 -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Eq, PartialEq, Hash)] pub struct DnsNameRef<'a>(pub(crate) &'a [u8]); impl AsRef<[u8]> for DnsNameRef<'_> { @@ -105,7 +98,11 @@ impl<'a> DnsNameRef<'a> { /// Constructs a `DnsNameRef` from the given input if the input is a /// syntactically-valid DNS name. pub fn try_from_ascii(dns_name: &'a [u8]) -> Result { - if !is_valid_reference_dns_id(untrusted::Input::from(dns_name)) { + if !is_valid_dns_id( + untrusted::Input::from(dns_name), + IdRole::Reference, + AllowWildcards::No, + ) { return Err(InvalidDnsNameError); } @@ -130,19 +127,18 @@ impl<'a> DnsNameRef<'a> { } } -/// Requires the `alloc` feature. -#[cfg(feature = "alloc")] impl core::fmt::Debug for DnsNameRef<'_> { fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { - let lowercase = self.clone().to_owned(); - f.debug_tuple("DnsNameRef").field(&lowercase.0).finish() - } -} + f.write_str("DnsNameRef(\"")?; -#[cfg(not(feature = "alloc"))] -impl core::fmt::Debug for DnsNameRef<'_> { - fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { - f.debug_tuple("DnsNameRef").field(&self.0).finish() + // Convert each byte of the underlying ASCII string to a `char` and + // downcase it prior to formatting it. We avoid self.clone().to_owned() + // since it requires allocation. + for &ch in self.0 { + f.write_char(char::from(ch).to_ascii_lowercase())?; + } + + f.write_str("\")") } } @@ -154,6 +150,94 @@ impl<'a> From> for &'a str { } } +/// A DNS name that may be either a DNS name identifier presented by a server (which may include +/// wildcards), or a DNS name identifier referenced by a client for matching purposes (wildcards +/// not permitted). +pub enum GeneralDnsNameRef<'name> { + /// a reference to a DNS name that may be used for matching purposes. + DnsName(DnsNameRef<'name>), + /// a reference to a presented DNS name that may include a wildcard. + Wildcard(WildcardDnsNameRef<'name>), +} + +impl<'a> From> for &'a str { + fn from(d: GeneralDnsNameRef<'a>) -> Self { + match d { + GeneralDnsNameRef::DnsName(name) => name.into(), + GeneralDnsNameRef::Wildcard(name) => name.into(), + } + } +} + +/// A reference to a DNS Name presented by a server that may include a wildcard. +/// +/// A `WildcardDnsNameRef` is guaranteed to be syntactically valid. The validity rules +/// are specified in [RFC 5280 Section 7.2], except that underscores are also +/// allowed. +/// +/// Additionally, while [RFC6125 Section 4.1] says that a wildcard label may be of the form +/// `*.`, where `` and/or `` may be empty, we follow a stricter policy common +/// to most validation libraries (e.g. NSS) and only accept wildcard labels that are exactly `*`. +/// +/// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2 +/// [RFC 6125 Section 4.1]: https://www.rfc-editor.org/rfc/rfc6125#section-4.1 +#[derive(Clone, Copy, Eq, PartialEq, Hash)] +pub struct WildcardDnsNameRef<'a>(&'a [u8]); + +impl<'a> WildcardDnsNameRef<'a> { + /// Constructs a `WildcardDnsNameRef` from the given input if the input is a + /// syntactically-valid DNS name. + pub fn try_from_ascii(dns_name: &'a [u8]) -> Result { + if !is_valid_dns_id( + untrusted::Input::from(dns_name), + IdRole::Reference, + AllowWildcards::Yes, + ) { + return Err(InvalidDnsNameError); + } + + Ok(Self(dns_name)) + } + + /// Constructs a `WildcardDnsNameRef` from the given input if the input is a + /// syntactically-valid DNS name. + pub fn try_from_ascii_str(dns_name: &'a str) -> Result { + Self::try_from_ascii(dns_name.as_bytes()) + } +} + +impl core::fmt::Debug for WildcardDnsNameRef<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str("WildcardDnsNameRef(\"")?; + + // Convert each byte of the underlying ASCII string to a `char` and + // downcase it prior to formatting it. We avoid self.to_owned() since + // it requires allocation. + for &ch in self.0 { + f.write_char(char::from(ch).to_ascii_lowercase())?; + } + + f.write_str("\")") + } +} + +impl<'a> From> for &'a str { + fn from(WildcardDnsNameRef(d): WildcardDnsNameRef<'a>) -> Self { + // The unwrap won't fail because WildcardDnsNameRef are guaranteed to be ASCII + // and ASCII is a subset of UTF-8. + core::str::from_utf8(d).unwrap() + } +} + +impl AsRef for WildcardDnsNameRef<'_> { + #[inline] + fn as_ref(&self) -> &str { + // The unwrap won't fail because WildcardDnsNameRef are guaranteed to be ASCII + // and ASCII is a subset of UTF-8. + core::str::from_utf8(self.0).unwrap() + } +} + pub(super) fn presented_id_matches_reference_id( presented_dns_id: untrusted::Input, reference_dns_id: untrusted::Input, @@ -445,10 +529,6 @@ enum IdRole { NameConstraint, } -fn is_valid_reference_dns_id(hostname: untrusted::Input) -> bool { - is_valid_dns_id(hostname, IdRole::Reference, AllowWildcards::No) -} - // https://tools.ietf.org/html/rfc5280#section-4.2.1.6: // // When the subjectAltName extension contains a domain name system diff --git a/src/subject_name/mod.rs b/src/subject_name/mod.rs index a27d5920..33d64c66 100644 --- a/src/subject_name/mod.rs +++ b/src/subject_name/mod.rs @@ -13,6 +13,8 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. mod dns_name; +#[cfg(feature = "alloc")] +pub(crate) use dns_name::GeneralDnsNameRef; pub use dns_name::{DnsNameRef, InvalidDnsNameError}; /// Requires the `alloc` feature. @@ -29,6 +31,8 @@ pub use ip_address::{AddrParseError, IpAddrRef}; pub use ip_address::IpAddr; mod verify; +#[cfg(feature = "alloc")] +pub(super) use verify::list_cert_dns_names; pub(super) use verify::{ check_name_constraints, verify_cert_subject_name, SubjectCommonNameContents, }; diff --git a/src/subject_name/verify.rs b/src/subject_name/verify.rs index 0f50e8a0..a2bfc7b5 100644 --- a/src/subject_name/verify.rs +++ b/src/subject_name/verify.rs @@ -21,6 +21,11 @@ use crate::{ cert::{Cert, EndEntityOrCa}, der, Error, }; +#[cfg(feature = "alloc")] +use { + alloc::vec::Vec, + dns_name::{GeneralDnsNameRef, WildcardDnsNameRef}, +}; pub(crate) fn verify_cert_dns_name( cert: &crate::EndEntityCert, @@ -33,7 +38,7 @@ pub(crate) fn verify_cert_dns_name( cert.subject_alt_name, SubjectCommonNameContents::Ignore, Err(Error::CertNotValidForName), - &|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(())), @@ -67,7 +72,7 @@ pub(crate) fn verify_cert_subject_name( cert.inner().subject_alt_name, SubjectCommonNameContents::Ignore, Err(Error::CertNotValidForName), - &|name| { + &mut |name| { if let GeneralName::IpAddress(presented_id) = name { match ip_address::presented_id_matches_reference_id(presented_id, ip_address) { Ok(true) => return NameIteration::Stop(Ok(())), @@ -115,7 +120,7 @@ pub(crate) fn check_name_constraints( child.subject_alt_name, subject_common_name_contents, Ok(()), - &|name| { + &mut |name| { check_presented_id_conforms_to_constraints( name, permitted_subtrees, @@ -302,12 +307,12 @@ pub(crate) enum SubjectCommonNameContents { Ignore, } -fn iterate_names( - subject: Option, - subject_alt_name: Option, +fn iterate_names<'names>( + subject: Option>, + subject_alt_name: Option>, subject_common_name_contents: SubjectCommonNameContents, result_if_never_stopped_early: Result<(), Error>, - f: &dyn Fn(GeneralName) -> NameIteration, + f: &mut impl FnMut(GeneralName<'names>) -> NameIteration, ) -> Result<(), Error> { if let Some(subject_alt_name) = subject_alt_name { let mut subject_alt_name = untrusted::Reader::new(subject_alt_name); @@ -351,6 +356,39 @@ fn iterate_names( } } +#[cfg(feature = "alloc")] +pub(crate) fn list_cert_dns_names<'names>( + cert: &'names crate::EndEntityCert<'names>, +) -> Result>, Error> { + let cert = &cert.inner(); + let mut names = Vec::new(); + + iterate_names( + Some(cert.subject), + cert.subject_alt_name, + SubjectCommonNameContents::DnsName, + Ok(()), + &mut |name| { + if let GeneralName::DnsName(presented_id) = name { + let dns_name = DnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) + .map(GeneralDnsNameRef::DnsName) + .or_else(|_| { + WildcardDnsNameRef::try_from_ascii(presented_id.as_slice_less_safe()) + .map(GeneralDnsNameRef::Wildcard) + }); + + // if the name could be converted to a DNS name, add it; otherwise, + // keep going. + if let Ok(name) = dns_name { + names.push(name) + } + } + NameIteration::KeepGoing + }, + ) + .map(|_| names.into_iter()) +} + // It is *not* valid to derive `Eq`, `PartialEq, etc. for this type. In // particular, for the types of `GeneralName`s that we don't understand, we // don't even store the value. Also, the meaning of a `GeneralName` in a name diff --git a/tests/integration.rs b/tests/integration.rs index 713cef20..ae88b8c4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -206,3 +206,118 @@ fn read_ee_with_large_pos_serial() { fn time_constructor() { let _ = webpki::Time::try_from(std::time::SystemTime::now()).unwrap(); } + +#[cfg(feature = "alloc")] +#[test] +pub fn list_netflix_names() { + let ee = include_bytes!("netflix/ee.der"); + + expect_cert_dns_names( + ee, + &[ + "account.netflix.com", + "ca.netflix.com", + "netflix.ca", + "netflix.com", + "signup.netflix.com", + "www.netflix.ca", + "www1.netflix.com", + "www2.netflix.com", + "www3.netflix.com", + "develop-stage.netflix.com", + "release-stage.netflix.com", + "www.netflix.com", + ], + ); +} + +#[cfg(feature = "alloc")] +#[test] +pub fn invalid_subject_alt_names() { + // same as netflix ee certificate, but with the last name in the list + // changed to 'www.netflix:com' + let data = include_bytes!("misc/invalid_subject_alternative_name.der"); + + expect_cert_dns_names( + data, + &[ + "account.netflix.com", + "ca.netflix.com", + "netflix.ca", + "netflix.com", + "signup.netflix.com", + "www.netflix.ca", + "www1.netflix.com", + "www2.netflix.com", + "www3.netflix.com", + "develop-stage.netflix.com", + "release-stage.netflix.com", + // NOT 'www.netflix:com' + ], + ); +} + +#[cfg(feature = "alloc")] +#[test] +pub fn wildcard_subject_alternative_names() { + // same as netflix ee certificate, but with the last name in the list + // changed to 'ww*.netflix:com' + let data = include_bytes!("misc/dns_names_and_wildcards.der"); + + expect_cert_dns_names( + data, + &[ + "account.netflix.com", + "*.netflix.com", + "netflix.ca", + "netflix.com", + "signup.netflix.com", + "www.netflix.ca", + "www1.netflix.com", + "www2.netflix.com", + "www3.netflix.com", + "develop-stage.netflix.com", + "release-stage.netflix.com", + "www.netflix.com", + ], + ); +} + +#[cfg(feature = "alloc")] +fn expect_cert_dns_names(data: &[u8], expected_names: &[&str]) { + use std::collections::HashSet; + + let cert = webpki::EndEntityCert::try_from(data) + .expect("should parse end entity certificate correctly"); + + let expected_names: HashSet<_> = expected_names.iter().cloned().collect(); + + let mut actual_names = cert + .dns_names() + .expect("should get all DNS names correctly for end entity cert") + .collect::>(); + + // Ensure that converting the list to a set doesn't throw away + // any duplicates that aren't supposed to be there + assert_eq!(actual_names.len(), expected_names.len()); + + let actual_names: std::collections::HashSet<&str> = + actual_names.drain(..).map(|name| name.into()).collect(); + + assert_eq!(actual_names, expected_names); +} + +#[cfg(feature = "alloc")] +#[test] +pub fn no_subject_alt_names() { + let data = include_bytes!("misc/no_subject_alternative_name.der"); + + let cert = webpki::EndEntityCert::try_from(&data[..]) + .expect("should parse end entity certificate correctly"); + + let names = cert + .dns_names() + .expect("we should get a result even without subjectAltNames"); + + assert!(names.collect::>().is_empty()); +} diff --git a/tests/misc/dns_names_and_wildcards.der b/tests/misc/dns_names_and_wildcards.der new file mode 100644 index 00000000..c41e2d61 Binary files /dev/null and b/tests/misc/dns_names_and_wildcards.der differ diff --git a/tests/misc/invalid_subject_alternative_name.der b/tests/misc/invalid_subject_alternative_name.der new file mode 100644 index 00000000..f81ad7b7 Binary files /dev/null and b/tests/misc/invalid_subject_alternative_name.der differ diff --git a/tests/misc/no_subject_alternative_name.der b/tests/misc/no_subject_alternative_name.der new file mode 100644 index 00000000..8292e358 Binary files /dev/null and b/tests/misc/no_subject_alternative_name.der differ