Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/end_entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

use crate::{
cert, name, signed_data, verify_cert, DnsNameRef, Error, SignatureAlgorithm,
TLSClientTrustAnchors, TLSServerTrustAnchors, Time,
cert,
name::{self, GeneralDnsNameRef},
signed_data, verify_cert, DnsNameRef, Error, SignatureAlgorithm, TLSClientTrustAnchors,
TLSServerTrustAnchors, Time,
};
use core::convert::TryFrom;

Expand Down Expand Up @@ -181,4 +183,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<Vec<GeneralDnsNameRef<'a>>, Error> {
name::list_cert_dns_names(self)
}
}
5 changes: 4 additions & 1 deletion src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

mod dns_name;
pub use dns_name::{DnsNameRef, InvalidDnsNameError};
pub use dns_name::{DnsNameRef, GeneralDnsNameRef, InvalidDnsNameError, WildcardDnsNameRef};

/// Requires the `alloc` feature.
#[cfg(feature = "alloc")]
Expand All @@ -23,3 +23,6 @@ mod ip_address;

mod verify;
pub(super) use verify::{check_name_constraints, verify_cert_dns_name};

#[cfg(feature = "alloc")]
pub(super) use verify::list_cert_dns_names;
131 changes: 130 additions & 1 deletion src/name/dns_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use alloc::string::String;
/// allowed.
///
/// `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.
/// and so it is only available when the `alloc` 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
Expand Down Expand Up @@ -147,6 +147,131 @@ impl<'a> From<DnsNameRef<'a>> for &'a str {
}
}

/// 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.
pub enum GeneralDnsNameRef<'name> {
/// a valid DNS name
DnsName(DnsNameRef<'name>),
/// a DNS name containing a wildcard
Wildcard(WildcardDnsNameRef<'name>),
}

impl<'a> From<GeneralDnsNameRef<'a>> 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 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. Compared to `DnsName`, this one will store domain names containing
/// a wildcard.
///
/// A `WildcardDnsName` is guaranteed to be syntactically valid. The validity rules are
/// specified in [RFC 5280 Section 7.2], except that underscores are also
/// allowed, and following [RFC 6125].
///
/// `WildcardDnsName` stores a copy of the input it was constructed from in a `String`
/// and so it is only available when the `alloc` default feature is enabled.
///
/// `Eq`, `PartialEq`, etc. are not implemented because name comparison
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by: This comment doesn't match the derive below?

/// frequently should be done case-insensitively and/or with other caveats that
/// depend on the specific circumstances in which the comparison is done.
///
/// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2
/// [RFC 6125]: https://tools.ietf.org/html/rfc6125
#[cfg(feature = "alloc")]
#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub struct WildcardDnsName(String);

#[cfg(feature = "alloc")]
impl WildcardDnsName {
/// Returns a `WildcardDnsNameRef` that refers to this `WildcardDnsName`.
pub fn as_ref(&self) -> WildcardDnsNameRef {
WildcardDnsNameRef(self.0.as_bytes())
}
}

#[cfg(feature = "alloc")]
impl AsRef<str> for WildcardDnsName {
fn as_ref(&self) -> &str {
self.0.as_ref()
}
}

// Deprecated
#[cfg(feature = "alloc")]
impl From<WildcardDnsNameRef<'_>> for WildcardDnsName {
fn from(dns_name: WildcardDnsNameRef) -> Self {
dns_name.to_owned()
}
}

/// A reference to 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.
///
/// 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.
///
/// `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.
///
/// [RFC 5280 Section 7.2]: https://tools.ietf.org/html/rfc5280#section-7.2
#[derive(Clone, Copy)]
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<Self, InvalidDnsNameError> {
if !is_valid_wildcard_dns_id(untrusted::Input::from(dns_name)) {
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, InvalidDnsNameError> {
Self::try_from_ascii(dns_name.as_bytes())
}

/// Constructs a `WildcardDnsName` from this `WildcardDnsNameRef`
#[cfg(feature = "alloc")]
pub fn to_owned(self) -> WildcardDnsName {
// WildcardDnsNameRef is already guaranteed to be valid ASCII, which is a
// subset of UTF-8.
let s: &str = self.into();
WildcardDnsName(s.to_ascii_lowercase())
}
}

#[cfg(feature = "alloc")]
impl core::fmt::Debug for WildcardDnsNameRef<'_> {
fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
let lowercase = self.to_owned();
f.debug_tuple("WildcardDnsNameRef")
.field(&lowercase.0)
.finish()
}
}

impl<'a> From<WildcardDnsNameRef<'a>> for &'a str {
fn from(WildcardDnsNameRef(d): WildcardDnsNameRef<'a>) -> Self {
// The unwrap won't fail because DnsNameRefs are guaranteed to be ASCII
// and ASCII is a subset of UTF-8.
core::str::from_utf8(d).unwrap()
}
}

pub(super) fn presented_id_matches_reference_id(
presented_dns_id: untrusted::Input,
reference_dns_id: untrusted::Input,
Expand Down Expand Up @@ -577,6 +702,10 @@ fn is_valid_dns_id(
true
}

fn is_valid_wildcard_dns_id(hostname: untrusted::Input) -> bool {
is_valid_dns_id(hostname, IdRole::Reference, AllowWildcards::Yes)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
40 changes: 36 additions & 4 deletions src/name/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ use crate::{
cert::{Cert, EndEntityOrCa},
der, Error,
};
#[cfg(feature = "alloc")]
use {
alloc::vec::Vec,
dns_name::{GeneralDnsNameRef, WildcardDnsNameRef},
};

pub fn verify_cert_dns_name(
cert: &crate::EndEntityCert,
Expand Down Expand Up @@ -238,11 +243,11 @@ enum NameIteration {
Stop(Result<(), Error>),
}

fn iterate_names(
subject: untrusted::Input,
subject_alt_name: Option<untrusted::Input>,
fn iterate_names<'names>(
subject: untrusted::Input<'names>,
subject_alt_name: Option<untrusted::Input<'names>>,
result_if_never_stopped_early: Result<(), Error>,
f: &dyn Fn(GeneralName) -> NameIteration,
f: &dyn Fn(GeneralName<'names>) -> NameIteration,
) -> Result<(), Error> {
match subject_alt_name {
Some(subject_alt_name) => {
Expand Down Expand Up @@ -272,6 +277,33 @@ fn iterate_names(
}
}

#[cfg(feature = "alloc")]
pub fn list_cert_dns_names<'names>(
cert: &'names crate::EndEntityCert<'names>,
) -> Result<Vec<GeneralDnsNameRef<'names>>, Error> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How hard would it be to rewire this to return an impl Iterator instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it certainly could be rewritten to return impl Iterator --- I wanted to start by coping the original diff from upstream exactly, but would be happy to revise.

let cert = &cert.inner();
let names = core::cell::RefCell::new(Vec::new());

iterate_names(cert.subject, cert.subject_alt_name, Ok(()), &|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.borrow_mut().push(name)
}
}
NameIteration::KeepGoing
})
.map(|_| names.into_inner())
}

// 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
Expand Down
114 changes: 114 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,117 @@ fn read_root_with_neg_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");

// 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.is_empty());
}
Binary file added tests/misc/dns_names_and_wildcards.der
Binary file not shown.
Binary file added tests/misc/invalid_subject_alternative_name.der
Binary file not shown.
Binary file added tests/misc/no_subject_alternative_name.der
Binary file not shown.