diff --git a/Cargo.lock b/Cargo.lock index 9327094d8..ef03c41ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -167,7 +167,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)", "try-lock 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", - "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", + "untrusted 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "webpki 0.18.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -894,7 +894,7 @@ dependencies = [ "cc 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.40 (registry+https://github.com/rust-lang/crates.io-index)", - "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", + "untrusted 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -919,7 +919,7 @@ dependencies = [ "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "ring 0.13.0-alpha5 (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)", + "untrusted 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "webpki 0.18.0-alpha4 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -939,7 +939,7 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "ring 0.13.0-alpha5 (registry+https://github.com/rust-lang/crates.io-index)", - "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", + "untrusted 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1413,7 +1413,7 @@ dependencies = [ [[package]] name = "untrusted" -version = "0.6.1" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] @@ -1452,7 +1452,7 @@ version = "0.18.0-alpha4" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "ring 0.13.0-alpha5 (registry+https://github.com/rust-lang/crates.io-index)", - "untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", + "untrusted 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1683,7 +1683,7 @@ dependencies = [ "checksum unicode-xid 0.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "8c1f860d7d29cf02cb2f3f359fd35991af3d30bac52c57d265a3c461074cb4dc" "checksum unicode-xid 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fc72304796d0818e357ead4e000d19c9c174ab23dc11093ac919054d20a6a7fc" "checksum unreachable 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "382810877fe448991dfc7f0dd6e3ae5d58088fd0ea5e35189655f84e6814fa56" -"checksum untrusted 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "70afa43c8c5d23a53a3c39ec9b56232c5badc19f6bb5ad529c1d6448a7241365" +"checksum untrusted 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "55cd1f4b4e96b46aeb8d4855db4a7a9bd96eeeb5c6a1ab54593328761642ce2f" "checksum url 1.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f808aadd8cfec6ef90e4a14eb46f24511824d1ac596b9682703c87056c8678b7" "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" diff --git a/proxy/src/dns.rs b/proxy/src/dns.rs index 85faaed93..fd0f3e40b 100644 --- a/proxy/src/dns.rs +++ b/proxy/src/dns.rs @@ -213,7 +213,7 @@ mod tests { ]; for case in VALID { - let name = Name::try_from(case.input); + let name = Name::try_from(case.input.as_bytes()); assert_eq!(name.as_ref().map(|x| x.as_ref()), Ok(case.output)); } @@ -227,7 +227,7 @@ mod tests { ]; for case in INVALID { - assert!(Name::try_from(case).is_err()); + assert!(Name::try_from(case.as_bytes()).is_err()); } } } diff --git a/proxy/src/transport/connect.rs b/proxy/src/transport/connect.rs index cf19db405..ba7e92247 100644 --- a/proxy/src/transport/connect.rs +++ b/proxy/src/transport/connect.rs @@ -62,7 +62,7 @@ impl HostAndPort { let host = IpAddr::from_str(a.host()) .map(Host::Ip) .or_else(|_| - dns::Name::try_from(a.host()) + dns::Name::try_from(a.host().as_bytes()) .map(Host::DnsName) .map_err(|_| HostAndPortError::InvalidHost))?; let port = a.port() diff --git a/proxy/src/transport/tls/conditional_accept.rs b/proxy/src/transport/tls/conditional_accept.rs new file mode 100644 index 000000000..fb8545cc0 --- /dev/null +++ b/proxy/src/transport/tls/conditional_accept.rs @@ -0,0 +1,248 @@ +#![allow(dead_code)] // TODO: Use this. + +use super::{Identity, untrusted}; + +#[derive(Debug, Eq, PartialEq)] +pub enum Match { + Incomplete, + Matched, + NotMatched, +} + +/// Determintes whether the given `input` looks like the start of a TLS +/// connection that the proxy should terminate. +/// +/// The determination is made based on whether the input looks like (the start +/// of) a valid ClientHello that a reasonable TLS client might send, and the +/// SNI matches the given identity. +/// +/// XXX: Once the TLS record header is matched, the determination won't be +/// made until the entire TLS record including the entire ClientHello handshake +/// message is available. TODO: Reject non-matching inputs earlier. +/// +/// This assumes that the ClientHello is small and is sent in a single TLS +/// record, which is what all reasonable implementations do. (If they were not +/// to, they wouldn't interoperate with picky servers.) +pub fn match_client_hello(input: &[u8], identity: &Identity) -> Match { + let r = untrusted::Input::from(input).read_all(untrusted::EndOfInput, |input| { + let r = extract_sni(input); + input.skip_to_end(); // Ignore anything after what we parsed. + r + }); + match r { + Ok(Some(sni)) => { + let matches = if let Ok(sni) = Identity::from_sni_hostname(sni.as_slice_less_safe()) { + if sni == *identity { + Match::Matched + } else { + Match::NotMatched + } + } else { + Match::NotMatched + }; + trace!("match_client_hello: parsed correctly up to SNI; matches: {:?}", matches); + matches + }, + Ok(None) => { + trace!("match_client_hello: failed to parse up to SNI"); + Match::NotMatched + }, + Err(untrusted::EndOfInput) => { + trace!("match_client_hello: needs more input"); + Match::Incomplete + }, + } +} + +/// The result is `Ok(Some(hostname))` if the SNI extension was found, `Ok(None)` +/// if we affirmatively rejected the input before we found the SNI extension, or +/// `Err(EndOfInput)` if we don't have enough input to continue. +fn extract_sni<'a>(input: &mut untrusted::Reader<'a>) + -> Result>, untrusted::EndOfInput> +{ + // TLS ciphertext record header. + + if input.read_byte()? != 22 { // ContentType::handshake + return Ok(None); + } + if input.read_byte()? != 0x03 { // legacy_record_version.major is always 0x03. + return Ok(None); + } + { + // legacy_record_version.minor may be 0x01 or 0x03 according to + // https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-5.1 + let minor = input.read_byte()?; + if minor != 0x01 && minor != 0x03 { + return Ok(None); + } + } + + // Treat the record length and its body as a vector. + let r = read_vector(input, |input| { + if input.read_byte()? != 1 { // HandshakeType::client_hello + return Ok(None); + } + // The length is a 24-bit big-endian value. Nobody (good) will never + // send a value larger than 0xffff so treat it as a 0x00 followed + // by vector + if input.read_byte()? != 0 { // Most significant byte of the length + return Ok(None); + } + read_vector(input, |input| { + // version{.major,.minor} == {0x3, 0x3} for TLS 1.2 and later. + if input.read_byte()? != 0x03 || + input.read_byte()? != 0x03 { + return Ok(None); + } + + input.skip(32)?; // random + skip_vector_u8(input)?; // session_id + if !skip_vector(input)? { // cipher_suites + return Ok(None); + } + skip_vector_u8(input)?; // compression_methods + + // Look for the SNI extension as specified in + // https://tools.ietf.org/html/rfc6066#section-1.1 + read_vector(input, |input| { + while !input.at_end() { + let extension_type = read_u16(input)?; + if extension_type != 0 { // ExtensionType::server_name + skip_vector(input)?; + continue; + } + + // Treat extension_length followed by extension_value as a + // vector. + let r = read_vector(input, |input| { + // server_name_list + read_vector(input, |input| { + // Nobody sends an SNI extension with anything + // other than a single `host_name` value. + if input.read_byte()? != 0 { // NameType::host_name + return Ok(None); + } + // Return the value of the `HostName`. + read_vector(input, |input| Ok(Some(input.skip_to_end()))) + }) + }); + + input.skip_to_end(); // Ignore stuff after SNI + return r; + } + + Ok(None) // No SNI extension. + }) + }) + }); + + // Ignore anything after the first handshake record. + input.skip_to_end(); + + r +} + +/// Reads a `u16` vector, which is formatted as a big-endian `u16` length +/// followed by that many bytes. +fn read_vector<'a, F, T>(input: &mut untrusted::Reader<'a>, f: F) + -> Result, untrusted::EndOfInput> + where F: Fn(&mut untrusted::Reader<'a>) -> Result, untrusted::EndOfInput>, + T: 'a, +{ + let length = read_u16(input)?; + + // ClientHello has to be small for compatibility with many deployed + // implementations, so if it is (relatively) huge, we might not be looking + // at TLS traffic, and we're definitely not looking at proxy-terminated + // traffic, so bail out early. + if length > 8192 { + return Ok(None); + } + let r = input.skip_and_get_input(usize::from(length))?; + r.read_all(untrusted::EndOfInput, f) +} + +/// Like `read_vector` except the contents are ignored. +fn skip_vector(input: &mut untrusted::Reader) -> Result { + let r = read_vector(input, |input| { + input.skip_to_end(); + Ok(Some(())) + }); + r.map(|r| r.is_some()) +} + +/// Like `skip_vector` for vectors with `u8` lengths. +fn skip_vector_u8(input: &mut untrusted::Reader) -> Result<(), untrusted::EndOfInput> { + let length = input.read_byte()?; + input.skip(usize::from(length)) +} + +/// Read a big-endian-encoded `u16`. +fn read_u16(input: &mut untrusted::Reader) -> Result { + let hi = input.read_byte()?; + let lo = input.read_byte()?; + Ok(u16::from(hi) << 8 | u16::from(lo)) +} + +#[cfg(test)] +mod tests { + use super::*; + use tls; + + /// From `cargo run --example tlsclient -- --http example.com` + static VALID_EXAMPLE_COM: &[u8] = include_bytes!("testdata/example-com-client-hello.bin"); + + #[test] + fn matches() { + check_all_prefixes(Match::Matched, "example.com", VALID_EXAMPLE_COM); + } + + #[test] + fn mismatch_different_sni() { + check_all_prefixes(Match::NotMatched, "example.org", VALID_EXAMPLE_COM); + } + + #[test] + fn mismatch_truncated_sni() { + check_all_prefixes(Match::NotMatched, "example.coma", VALID_EXAMPLE_COM); + } + + #[test] + fn mismatch_appended_sni() { + check_all_prefixes(Match::NotMatched, "example.co", VALID_EXAMPLE_COM); + } + + #[test] + fn mismatch_prepended_sni() { + check_all_prefixes(Match::NotMatched, "aexample.com", VALID_EXAMPLE_COM); + } + + #[test] + fn mismatch_http_1_0_request() { + check_all_prefixes(Match::NotMatched, "example.com", + b"GET /TheProject.html HTTP/1.0\r\n\r\n"); + } + + fn check_all_prefixes(expected_match: Match, identity: &str, input: &[u8]) { + assert!(expected_match == Match::Matched || expected_match == Match::NotMatched); + + let identity = tls::Identity::from_sni_hostname(identity.as_bytes()).unwrap(); + + let mut i = 0; + + // `Async::NotReady` will be returned for some number of prefixes. + loop { + let m = match_client_hello(&input[..i], &identity); + if m != Match::Incomplete { + assert_eq!(m, expected_match); + break; + } + i += 1; + } + + // The same result will be returned for all longer prefixes. + for i in (i + 1)..input.len() { + assert_eq!(expected_match, match_client_hello(&input[..i], &identity)) + } + } +} diff --git a/proxy/src/transport/tls/dns_name.rs b/proxy/src/transport/tls/dns_name.rs index 3a141bc9a..9abc38cdf 100644 --- a/proxy/src/transport/tls/dns_name.rs +++ b/proxy/src/transport/tls/dns_name.rs @@ -1,4 +1,4 @@ -use super::webpki; +use super::{untrusted, webpki}; use std::fmt; use convert::TryFrom; @@ -17,10 +17,10 @@ impl fmt::Display for DnsName { #[derive(Debug, Eq, PartialEq)] pub struct InvalidDnsName; -impl<'a> TryFrom<&'a str> for DnsName { +impl<'a> TryFrom<&'a [u8]> for DnsName { type Err = InvalidDnsName; - fn try_from(s: &str) -> Result>::Err> { - webpki::DNSNameRef::try_from_ascii_str(s) + fn try_from(s: &[u8]) -> Result { + webpki::DNSNameRef::try_from_ascii(untrusted::Input::from(s)) .map(|r| DnsName(r.to_owned())) .map_err(|()| InvalidDnsName) } diff --git a/proxy/src/transport/tls/identity.rs b/proxy/src/transport/tls/identity.rs index b9c523d68..c57bbd808 100644 --- a/proxy/src/transport/tls/identity.rs +++ b/proxy/src/transport/tls/identity.rs @@ -72,16 +72,23 @@ impl Identity { // We reserve all names under a fake "managed-pods" service in // our namespace for identifying pods by name. let name = format!( - "{pod}.{pod_ns}.conduit-managed-pods.{controller_ns}.svc.cluster.local.", + "{pod}.{pod_ns}.conduit-managed-pods.{controller_ns}.svc.cluster.local", pod = pod_name, pod_ns = &namespaces.pod, controller_ns = controller_ns, ); - DnsName::try_from(&name) + Self::from_sni_hostname(name.as_bytes()) + } + + pub fn from_sni_hostname(hostname: &[u8]) -> Result { + if hostname.last() == Some(&b'.') { + return Err(()); // SNI hostnames are implicitly absolute. + } + DnsName::try_from(hostname) .map(|name| Identity(Arc::new(name))) .map_err(|InvalidDnsName| { - error!("Invalid DNS name: {:?}", name); + error!("Invalid DNS name: {:?}", hostname); () }) } diff --git a/proxy/src/transport/tls/mod.rs b/proxy/src/transport/tls/mod.rs index c38daba12..8681e01cf 100755 --- a/proxy/src/transport/tls/mod.rs +++ b/proxy/src/transport/tls/mod.rs @@ -4,6 +4,7 @@ extern crate tokio_rustls; extern crate untrusted; extern crate webpki; +mod conditional_accept; mod config; mod cert_resolver; mod connection; diff --git a/proxy/src/transport/tls/testdata/example-com-client-hello.bin b/proxy/src/transport/tls/testdata/example-com-client-hello.bin new file mode 100644 index 000000000..d5220d632 Binary files /dev/null and b/proxy/src/transport/tls/testdata/example-com-client-hello.bin differ