Proxy: Skip TLS for control plane loopback connections. (#1229)

If the controller address has a loopback host then don't use TLS to connect
to it. TLS isn't needed for security in that case. In mormal configurations
the proxy isn't terminating TLS for loopback connections anyway.

Signed-off-by: Brian Smith <brian@briansmith.org>

proxy/src/config.rs

@@ -299,28 +299,41 @@ impl<'a> TryFrom<&'a Strings> for Config {
             tls_controller: controller_namespace?,
         };
 
+        let tls_controller_identity = tls_controller_identity?;
+        let control_host_and_port = control_host_and_port?;
+
         let tls_settings = match (tls_trust_anchors?,
                                   tls_end_entity_cert?,
                                   tls_private_key?,
-                                  tls_pod_identity_template?.as_ref(),
-                                  tls_controller_identity?)
+                                  tls_pod_identity_template?.as_ref())
         {
             (Some(trust_anchors),
              Some(end_entity_cert),
              Some(private_key),
-             Some(tls_pod_identity_template),
-             controller_identity) => {
+             Some(tls_pod_identity_template)) => {
                 let pod_identity =
                     tls_pod_identity_template.replace(VAR_POD_NAMESPACE, &namespaces.pod);
                 let pod_identity = tls::Identity::from_sni_hostname(pod_identity.as_bytes())
                     .map_err(|_| Error::InvalidEnvVar)?; // Already logged.
-                let controller_identity = if let Some(controller_identity) = &controller_identity {
-                    let identity = tls::Identity::from_sni_hostname(controller_identity.as_bytes())
-                        .map_err(|_| Error::InvalidEnvVar)?; // Already logged.
-                    Conditional::Some(identity)
+
+                // Avoid setting the controller identity if it is going to be
+                // a loopback connection since TLS isn't needed or supported in
+                // that case.
+                let controller_identity = if let Some(identity) = &tls_controller_identity {
+                    match &control_host_and_port {
+                        Some(hp) if hp.is_loopback() =>
+                            Conditional::None(tls::ReasonForNoIdentity::Loopback),
+                        Some(_) => {
+                            let identity = tls::Identity::from_sni_hostname(identity.as_bytes())
+                                .map_err(|_| Error::InvalidEnvVar)?; // Already logged.
+                            Conditional::Some(identity)
+                        },
+                        None => Conditional::None(tls::ReasonForNoIdentity::NotConfigured),
+                    }
                 } else {
                     Conditional::None(tls::ReasonForNoIdentity::NotConfigured)
                 };
+
                 Ok(Conditional::Some(tls::CommonSettings {
                     trust_anchors,
                     end_entity_cert,
@@ -329,8 +342,8 @@ impl<'a> TryFrom<&'a Strings> for Config {
                     controller_identity,
                 }))
             },
-            (None, None, None, _, _) => Ok(Conditional::None(tls::ReasonForNoTls::Disabled)),
-            (trust_anchors, end_entity_cert, private_key, pod_identity, _) => {
+            (None, None, None, _) => Ok(Conditional::None(tls::ReasonForNoTls::Disabled)),
+            (trust_anchors, end_entity_cert, private_key, pod_identity) => {
                 if trust_anchors.is_none() {
                     error!("{} is not set; it is required when {} and {} are set.",
                            ENV_TLS_TRUST_ANCHORS, ENV_TLS_CERT, ENV_TLS_PRIVATE_KEY);
@@ -395,7 +408,7 @@ impl<'a> TryFrom<&'a Strings> for Config {
             resolv_conf_path: resolv_conf_path?
                 .unwrap_or(DEFAULT_RESOLV_CONF.into())
                 .into(),
-            control_host_and_port: control_host_and_port?,
+            control_host_and_port,
 
             event_buffer_capacity: event_buffer_capacity?.unwrap_or(DEFAULT_EVENT_BUFFER_CAPACITY),
             metrics_retain_idle: metrics_retain_idle?.unwrap_or(DEFAULT_METRICS_RETAIN_IDLE),

proxy/src/transport/connect.rs

@@ -73,6 +73,13 @@ impl HostAndPort {
             port
         })
     }
+
+    pub fn is_loopback(&self) -> bool {
+        match &self.host {
+            Host::DnsName(dns_name) => dns_name.is_localhost(),
+            Host::Ip(ip) => ip.is_loopback(),
+        }
+    }
 }
 
 impl<'a> From<&'a HostAndPort> for http::uri::Authority {
@@ -162,3 +169,27 @@ impl tokio_connect::Connect for LookupAddressAndConnect {
         Box::new(c)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use http::uri::Authority;
+    use super::*;
+
+    #[test]
+    fn test_is_loopback() {
+        let cases = &[
+            ("localhost", false), // Not absolute
+            ("localhost.", true),
+            ("LocalhOsT.", true), // Case-insensitive
+            ("mlocalhost.", false), // prefixed
+            ("localhost1.", false), // suffixed
+            ("127.0.0.1", true), // IPv4
+            ("[::1]", true), // IPv6
+        ];
+        for (host, expected_result) in cases {
+            let authority = Authority::from_static(host);
+            let hp = HostAndPort::normalize(&authority, Some(80)).unwrap();
+            assert_eq!(hp.is_loopback(), *expected_result, "{:?}", host)
+        }
+    }
+}

proxy/src/transport/tls/config.rs

@@ -119,6 +119,10 @@ pub enum ReasonForNoIdentity {
     /// of telling us that we shouldn't do TLS for this endpoint.
     NotProvidedByServiceDiscovery,
 
+    /// No TLS is wanted because the connection is a loopback connection which
+    /// doesn't need or support TLS.
+    Loopback,
+
     /// The proxy wasn't configured with the identity.
     NotConfigured,
 

proxy/src/transport/tls/dns_name.rs

@@ -8,6 +8,12 @@ use convert::TryFrom;
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct DnsName(pub(super) webpki::DNSName);
 
+impl DnsName {
+    pub fn is_localhost(&self) -> bool {
+        *self == DnsName::try_from("localhost.".as_bytes()).unwrap()
+    }
+}
+
 impl fmt::Display for DnsName {
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
         self.as_ref().fmt(f)
@@ -31,3 +37,23 @@ impl AsRef<str> for DnsName {
         <webpki::DNSName as AsRef<str>>::as_ref(&self.0)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_localhost() {
+        let cases = &[
+            ("localhost", false), // Not absolute
+            ("localhost.", true),
+            ("LocalhOsT.", true), // Case-insensitive
+            ("mlocalhost.", false), // prefixed
+            ("localhost1.", false), // suffixed
+        ];
+        for (host, expected_result) in cases {
+            let dns_name = DnsName::try_from(host.as_bytes()).unwrap();
+            assert_eq!(dns_name.is_localhost(), *expected_result, "{:?}", dns_name)
+        }
+    }
+}