diff --git a/Cargo.lock b/Cargo.lock index da7f14076..ebb9586bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -164,7 +164,7 @@ dependencies = [ "tower-util 0.1.0 (git+https://github.com/tower-rs/tower)", "trust-dns-resolver 0.9.0 (git+https://github.com/bluejekyll/trust-dns)", "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", - "webpki 0.18.0-alpha3 (registry+https://github.com/rust-lang/crates.io-index)", + "webpki 0.18.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -877,7 +877,7 @@ dependencies = [ "ring 0.13.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", "sct 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", - "webpki 0.18.0-alpha3 (registry+https://github.com/rust-lang/crates.io-index)", + "webpki 0.18.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1081,7 +1081,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "rustls 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "tokio 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", - "webpki 0.18.0-alpha3 (registry+https://github.com/rust-lang/crates.io-index)", + "webpki 0.18.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1378,7 +1378,7 @@ dependencies = [ [[package]] name = "webpki" -version = "0.18.0-alpha3" +version = "0.18.0-alpha4" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "ring 0.13.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1601,7 +1601,7 @@ dependencies = [ "checksum utf8-ranges 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "662fab6525a98beff2921d7f61a39e7d59e0b425ebc7d0d9e66d316e55124122" "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" "checksum want 0.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "a05d9d966753fa4b5c8db73fcab5eed4549cfe0e1e4e66911e5564a0085c35d1" -"checksum webpki 0.18.0-alpha3 (registry+https://github.com/rust-lang/crates.io-index)" = "30cf7434bea34e094993720093b0f0ef4117d3edd977e5bd234de72e6d4c354e" +"checksum webpki 0.18.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)" = "724897af4bb44f3e0142b9cca300eb15f61b9b34fa559440bed8c43f2ff7afc0" "checksum widestring 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7157704c2e12e3d2189c507b7482c52820a16dfa4465ba91add92f266667cadb" "checksum winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "167dc9d6949a9b857f3451275e911c3f44255842c1f7a76f33c55103a909087a" "checksum winapi 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "04e3bd221fcbe8a271359c04f21a76db7d0c6028862d1bb5512d85e1e2eb5bb3" diff --git a/proxy/Cargo.toml b/proxy/Cargo.toml index 575757754..1b631b47a 100644 --- a/proxy/Cargo.toml +++ b/proxy/Cargo.toml @@ -52,7 +52,7 @@ tower-in-flight-limit = { git = "https://github.com/tower-rs/tower" } tower-util = { git = "https://github.com/tower-rs/tower" } ring = "0.13.0-alpha4" -webpki = "0.18.0-alpha3" +webpki = "0.18.0-alpha4" rustls = "0.12.0" tokio-rustls = "0.6.0" untrusted = "0.6.1" diff --git a/proxy/src/dns.rs b/proxy/src/dns.rs index 1e2f6aeda..df39d7d33 100644 --- a/proxy/src/dns.rs +++ b/proxy/src/dns.rs @@ -11,6 +11,7 @@ use trust_dns_resolver::{ lookup_ip::LookupIp, AsyncResolver, }; +use tls; use config::Config; @@ -37,30 +38,12 @@ pub enum Response { // `Box` implements `Future` so it doesn't need to be implemented manually. pub type IpAddrListFuture = Box + Send>; -/// A DNS name. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -pub struct Name(String); - -impl fmt::Display for Name { - fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - self.0.fmt(f) - } -} - -impl<'a> From<&'a str> for Name { - fn from(s: &str) -> Self { - // TODO: Verify the name is a valid DNS name. - // TODO: Avoid this extra allocation. - Name(s.to_ascii_lowercase()) - } -} - -impl AsRef for Name { - fn as_ref(&self) -> &str { - self.0.as_ref() - } -} - +/// A valid DNS name. +/// +/// This is an alias of the strictly-validated `tls::DnsName` based on the +/// premise that we only need to support DNS names for which one could get a +/// valid certificate. +pub type Name = tls::DnsName; impl Resolver { @@ -137,10 +120,10 @@ impl Resolver { // `ResolverFuture` can only be used for one lookup, so we have to clone all // the state during each resolution. - fn lookup_ip(self, &Name(ref name): &Name) + fn lookup_ip(self, name: &Name) -> impl Future { - self.resolver.lookup_ip(name.as_str()) + self.resolver.lookup_ip(name.as_ref()) } } @@ -187,6 +170,11 @@ mod tests { #[test] fn test_dns_name_parsing() { + // Make sure `dns::Name`'s validation isn't too strict. It is + // implemented in terms of `webpki::DNSName` which has many more tests + // at https://github.com/briansmith/webpki/blob/master/tests/dns_name_tests.rs. + use convert::TryFrom; + struct Case { input: &'static str, output: &'static str, @@ -197,19 +185,33 @@ mod tests { Case { input: "1.2.3.x", output: "1.2.3.x", }, Case { input: "1.2.3.x", output: "1.2.3.x", }, Case { input: "1.2.3.4A", output: "1.2.3.4a", }, - Case { input: "a.1.2.3", output: "a.1.2.3", }, - Case { input: "1.2.x.3", output: "1.2.x.3", }, Case { input: "a.b.c.d", output: "a.b.c.d", }, // Uppercase letters in labels Case { input: "A.b.c.d", output: "a.b.c.d", }, Case { input: "a.mIddle.c", output: "a.middle.c", }, Case { input: "a.b.c.D", output: "a.b.c.d", }, + + // Absolute + Case { input: "a.b.c.d.", output: "a.b.c.d.", }, ]; for case in VALID { - let name = Name::from(case.input); - assert_eq!(name.as_ref(), case.output); + let name = Name::try_from(case.input); + assert_eq!(name.as_ref().map(|x| x.as_ref()), Ok(case.output)); + } + + static INVALID: &[&str] = &[ + // These are not in the "preferred name syntax" as defined by + // https://tools.ietf.org/html/rfc1123#section-2.1. In particular + // the last label only has digits. + "1.2.3.4", + "a.1.2.3", + "1.2.x.3", + ]; + + for case in INVALID { + assert!(Name::try_from(case).is_err()); } } } diff --git a/proxy/src/transport/connect.rs b/proxy/src/transport/connect.rs index 1ebc1c9f8..53c28b21f 100644 --- a/proxy/src/transport/connect.rs +++ b/proxy/src/transport/connect.rs @@ -9,6 +9,7 @@ use http; use connection; use control::destination; +use convert::TryFrom; use dns; #[derive(Debug, Clone)] @@ -58,7 +59,10 @@ impl HostAndPort { { let host = IpAddr::from_str(a.host()) .map(Host::Ip) - .unwrap_or_else(|_| Host::DnsName(dns::Name::from(a.host()))); + .or_else(|_| + dns::Name::try_from(a.host()) + .map(Host::DnsName) + .map_err(|_| HostAndPortError::InvalidHost))?; let port = a.port() .or(default_port) .ok_or_else(|| HostAndPortError::MissingPort)?; diff --git a/proxy/src/transport/tls/dns_name.rs b/proxy/src/transport/tls/dns_name.rs new file mode 100644 index 000000000..a82cae59d --- /dev/null +++ b/proxy/src/transport/tls/dns_name.rs @@ -0,0 +1,33 @@ +use super::webpki; +use std::fmt; +use convert::TryFrom; + +/// 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. +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +pub struct DnsName(webpki::DNSName); + +impl fmt::Display for DnsName { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + self.as_ref().fmt(f) + } +} + +#[derive(Debug, Eq, PartialEq)] +pub struct InvalidDnsName; + +impl<'a> TryFrom<&'a str> for DnsName { + type Err = InvalidDnsName; + fn try_from(s: &str) -> Result>::Err> { + webpki::DNSNameRef::try_from_ascii_str(s) + .map(|r| DnsName(r.to_owned())) + .map_err(|()| InvalidDnsName) + } +} + +impl AsRef for DnsName { + fn as_ref(&self) -> &str { + >::as_ref(&self.0) + } +} diff --git a/proxy/src/transport/tls/mod.rs b/proxy/src/transport/tls/mod.rs index 7909d5d19..2fbc5e301 100755 --- a/proxy/src/transport/tls/mod.rs +++ b/proxy/src/transport/tls/mod.rs @@ -8,8 +8,10 @@ extern crate webpki; mod config; mod cert_resolver; mod connection; +mod dns_name; pub use self::{ config::{CommonSettings, CommonConfig, Error, ServerConfig, ServerConfigWatch}, connection::Connection, + dns_name::DnsName, };