Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a method to collect the DNS names from a certificate (finish work from #91 ) #103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
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 @@
/// 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 @@
}
}

/// 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
/// 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)]

Check warning on line 188 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L188

Added line #L188 was not covered by tests
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())
}

Check warning on line 196 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L194-L196

Added lines #L194 - L196 were not covered by tests
}

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

Check warning on line 203 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L201-L203

Added lines #L201 - L203 were not covered by tests
}

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

Check warning on line 211 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L209-L211

Added lines #L209 - L211 were not covered by tests
}

/// 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)]

Check warning on line 227 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L227

Added line #L227 was not covered by tests
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())
}

Check warning on line 245 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L243-L245

Added lines #L243 - L245 were not covered by tests

/// 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.clone().into();
WildcardDnsName(s.to_ascii_lowercase())
}

Check warning on line 254 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L249-L254

Added lines #L249 - L254 were not covered by tests
}

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

Check warning on line 264 in src/name/dns_name.rs

View check run for this annotation

Codecov / codecov/patch

src/name/dns_name.rs#L259-L264

Added lines #L259 - L264 were not covered by tests
}

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 @@
true
}

fn is_valid_wildcard_dns_id(hostname: untrusted::Input) -> bool {
is_valid_dns_id(hostname, IDRole::ReferenceID, 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 @@ -245,11 +250,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 @@ -279,6 +284,33 @@ fn iterate_names(
}
}

#[cfg(feature = "alloc")]
pub fn list_cert_dns_names<'names>(
cert: &crate::EndEntityCert<'names>,
) -> Result<Vec<GeneralDnsNameRef<'names>>, Error> {
let cert = &cert.inner;
let names = core::cell::RefCell::new(Vec::new());

iterate_names(cert.subject, cert.subject_alt_name, Ok(()), &|name| {
match name {
GeneralName::DnsName(presented_id) => {
match 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)
}) {
Ok(name) => names.borrow_mut().push(name),
Err(_) => { /* keep going */ }
};
}
_ => (),
}
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
15 changes: 14 additions & 1 deletion src/webpki.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub mod trust_anchor_util;
mod verify_cert;

pub use error::Error;
pub use name::{DnsNameRef, InvalidDnsNameError};
pub use name::{DnsNameRef, GeneralDnsNameRef, InvalidDnsNameError, WildcardDnsNameRef};

#[cfg(feature = "alloc")]
pub use name::DnsName;
Expand Down Expand Up @@ -248,6 +248,19 @@ 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(&self) -> Result<Vec<GeneralDnsNameRef<'a>>, Error> {
name::list_cert_dns_names(&self)
}
}

/// A trust anchor (a.k.a. root CA).
Expand Down
114 changes: 114 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,117 @@
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);

Check warning on line 190 in tests/integration.rs

View check run for this annotation

Codecov / codecov/patch

tests/integration.rs#L190

Added line #L190 was not covered by tests
}

#[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.