Proxy: Add parser to distinguish proxy TLS traffic from other traffic. (#1197)

* Proxy: Add parser to distinguish proxy TLS traffic from other traffic. Distinguish incoming TLS traffic intended for the proxy to terminate from TLS traffic intended for the proxied service to terminate and from non-TLS traffic. The new version of `untrusted` is required for this to work. Signed-off-by: Brian Smith <brian@briansmith.org> * More tests Signed-off-by: Brian Smith <brian@briansmith.org> * Stop abusing `futures::Async`. Signed-off-by: Brian Smith <brian@briansmith.org>
2018-06-25 12:12:25 -10:00 · 2018-06-25 12:12:25 -10:00 · 140f246624
parent c204441ecb
commit 140f246624
8 changed files with 273 additions and 17 deletions
--- 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"
--- 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());
        }
    }
 }
--- 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()
--- a/proxy/src/transport/tls/conditional_accept.rs
+++ 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<Option<untrusted::Input<'a>>, 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<u16>.
+    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<u16>
+        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<u16>.
+                    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<Option<T>, untrusted::EndOfInput>
+    where F: Fn(&mut untrusted::Reader<'a>) -> Result<Option<T>, 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<bool, untrusted::EndOfInput> {
+    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<u16, untrusted::EndOfInput> {
+    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))
+        }
+    }
+}
--- 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<Self, <Self as TryFrom<&str>>::Err> {
-        webpki::DNSNameRef::try_from_ascii_str(s)
+    fn try_from(s: &[u8]) -> Result<Self, Self::Err> {
+        webpki::DNSNameRef::try_from_ascii(untrusted::Input::from(s))
            .map(|r| DnsName(r.to_owned()))
            .map_err(|()| InvalidDnsName)
    }
--- 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<Self, ()> {
+        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);
                ()
            })
    }
--- 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;
--- a/proxy/src/transport/tls/testdata/example-com-client-hello.bin
+++ b/proxy/src/transport/tls/testdata/example-com-client-hello.bin