proxy: Add `tls="true"` metric label to connections accepted with TLS (#1050)

Depends on #1047.

This PR adds a `tls="true"` label to metrics produced by TLS connections and
requests/responses on those connections, and a `tls="no_config"` label on 
connections where TLS was enabled but the proxy has not been able to load
a valid TLS configuration.

Currently, these labels are only set on accepted connections, as we are not yet
opening encrypted connections, but I wired through the `tls_status` field on 
the `Client` transport context as well, so when we start opening client 
connections with TLS, the label will be applied to their metrics as well.

Closes #1046

Signed-off-by: Eliza Weisman <eliza@buoyanbt.io>

proxy/benches/record.rs

@@ -36,7 +36,10 @@ fn process() -> Arc<ctx::Process> {
 }
 
 fn server(proxy: &Arc<ctx::Proxy>) -> Arc<ctx::transport::Server> {
-    ctx::transport::Server::new(&proxy, &addr(), &addr(), &Some(addr()))
+    ctx::transport::Server::new(
+        &proxy, &addr(), &addr(), &Some(addr()),
+        ctx::transport::TlsStatus::Disabled,
+    )
 }
 
 fn client<L, S>(proxy: &Arc<ctx::Proxy>, labels: L) -> Arc<ctx::transport::Client>
@@ -48,6 +51,7 @@ where
         &proxy,
         &addr(),
         destination::Metadata::new(metrics::DstLabels::new(labels), None),
+        ctx::transport::TlsStatus::Disabled,
     )
 }
 

proxy/src/bind.rs

@@ -209,6 +209,9 @@ where
             &self.ctx,
             &addr,
             ep.metadata().clone(),
+            // TODO: when we can use TLS for client connections, indicate
+            //       whether or not the connection was TLS here.
+            ctx::transport::TlsStatus::Disabled,
         );
 
         // Map a socket address to a connection.

proxy/src/connection.rs

@@ -10,6 +10,7 @@ use tokio::{
     reactor::Handle,
 };
 
+use ctx::transport::TlsStatus;
 use config::Addr;
 use transport::{GetOriginalDst, Io, tls};
 
@@ -22,11 +23,19 @@ pub struct BoundPort {
 
 /// Initiates a client connection to the given address.
 pub fn connect(addr: &SocketAddr) -> Connecting {
-    Connecting(PlaintextSocket::connect(addr))
+    Connecting {
+        inner: PlaintextSocket::connect(addr),
+        // TODO: when we can open TLS client connections, this is where we will
+        //       indicate that for telemetry.
+        tls_status: TlsStatus::Disabled,
+    }
 }
 
 /// A socket that is in the process of connecting.
-pub struct Connecting(ConnectFuture);
+pub struct Connecting {
+    inner: ConnectFuture,
+    tls_status: TlsStatus,
+}
 
 /// Abstracts a plaintext socket vs. a TLS decorated one.
 ///
@@ -44,6 +53,9 @@ pub struct Connection {
     /// When calling `read`, it's important to consume bytes from this buffer
     /// before calling `io.read`.
     peek_buf: BytesMut,
+
+    /// Whether or not the connection is secured with TLS.
+    tls_status: TlsStatus,
 }
 
 /// A trait describing that a type can peek bytes.
@@ -126,18 +138,25 @@ impl BoundPort {
                     // libraries don't have the necessary API for that, so just
                     // do it here.
                     set_nodelay_or_warn(&socket);
-                    if let Some((_identity, config_watch)) = &tls {
+                    let tls_status = if let Some((_identity, config_watch)) = &tls {
                         // TODO: use `identity` to differentiate between TLS
                         // that the proxy should terminate vs. TLS that should
                         // be passed through.
                         if let Some(config) = &*config_watch.borrow() {
-                            return Either::A(
+                            let f = tls::Connection::accept(socket, config.clone())
-                                tls::Connection::accept(socket, config.clone())
+                                .map(move |tls| {
-                                    .map(move |tls| (Connection::new(Box::new(tls)), remote_addr)));
+                                    (Connection::tls(tls), remote_addr)
+                                });
+                            return Either::A(f);
+                        } else {
+                            // No valid TLS configuration.
+                            TlsStatus::NoConfig
                         }
-                    }
+                    } else {
-
+                        TlsStatus::Disabled
-                    Either::B(future::ok((Connection::new(Box::new(socket)), remote_addr)))
+                    };
+                    let conn = Connection::new(socket, tls_status);
+                    Either::B(future::ok((conn, remote_addr)))
                 })
                 .then(|r| {
                     future::ok(match r {
@@ -162,20 +181,28 @@ impl Future for Connecting {
     type Error = io::Error;
 
     fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
-        let socket = try_ready!(self.0.poll());
+        let socket = try_ready!(self.inner.poll());
         set_nodelay_or_warn(&socket);
-        Ok(Async::Ready(Connection::new(Box::new(socket))))
+        Ok(Async::Ready(Connection::new(socket, self.tls_status)))
     }
 }
 
 // ===== impl Connection =====
 
 impl Connection {
-    /// A constructor of `Connection` with a plain text TCP socket.
+    fn new<I: Io + 'static>(io: I, tls_status: TlsStatus) -> Self {
-    fn new(io: Box<Io>) -> Self {
         Connection {
-            io,
+            io: Box::new(io),
             peek_buf: BytesMut::new(),
+            tls_status,
+        }
+    }
+
+    fn tls(tls: tls::Connection) -> Self {
+            Connection {
+            io: Box::new(tls),
+            peek_buf: BytesMut::new(),
+            tls_status: TlsStatus::Success,
         }
     }
 
@@ -186,6 +213,10 @@ impl Connection {
     pub fn local_addr(&self) -> Result<SocketAddr, std::io::Error> {
         self.io.local_addr()
     }
+
+    pub fn tls_status(&self) -> TlsStatus {
+        self.tls_status
+    }
 }
 
 impl io::Read for Connection {

proxy/src/ctx/http.rs

@@ -80,6 +80,19 @@ impl Request {
         self.client.tls_identity()
     }
 
+    /// Returns a `TlsStatus` indicating if the request was sent was over TLS.
+    pub fn tls_status(&self) -> ctx::transport::TlsStatus {
+        if self.server.proxy.is_outbound() {
+            // If the request is in the outbound direction, then we opened the
+            // client connection, so check if it was secured.
+            self.client.tls_status
+        } else {
+            // Otherwise, the request is inbound, so check if we accepted it
+            // over TLS.
+            self.server.tls_status
+        }
+    }
+
     pub fn dst_labels(&self) -> Option<&DstLabels> {
         self.client.dst_labels()
     }
@@ -95,6 +108,11 @@ impl Response {
         Arc::new(r)
     }
 
+    /// Returns a `TlsStatus` indicating if the response was sent was over TLS.
+    pub fn tls_status(&self) -> ctx::transport::TlsStatus {
+        self.request.tls_status()
+    }
+
     pub fn dst_labels(&self) -> Option<&DstLabels> {
         self.request.dst_labels()
     }

proxy/src/ctx/mod.rs

@@ -100,17 +100,24 @@ pub mod test_util {
         })
     }
 
-    pub fn server(proxy: &Arc<ctx::Proxy>) -> Arc<ctx::transport::Server> {
+    pub fn server(
-        ctx::transport::Server::new(&proxy, &addr(), &addr(), &Some(addr()))
+        proxy: &Arc<ctx::Proxy>,
+        tls: ctx::transport::TlsStatus
+    ) -> Arc<ctx::transport::Server> {
+        ctx::transport::Server::new(&proxy, &addr(), &addr(), &Some(addr()), tls)
     }
 
-    pub fn client<L, S>(proxy: &Arc<ctx::Proxy>, labels: L) -> Arc<ctx::transport::Client>
+    pub fn client<L, S>(
+        proxy: &Arc<ctx::Proxy>,
+        labels: L,
+        tls: ctx::transport::TlsStatus,
+    ) -> Arc<ctx::transport::Client>
     where
         L: IntoIterator<Item=(S, S)>,
         S: fmt::Display,
     {
         let meta = destination::Metadata::new(DstLabels::new(labels), None);
-        ctx::transport::Client::new(&proxy, &addr(), meta)
+        ctx::transport::Client::new(&proxy, &addr(), meta, tls)
     }
 
     pub fn request(

proxy/src/ctx/transport.rs

@@ -19,6 +19,7 @@ pub struct Server {
     pub remote: SocketAddr,
     pub local: SocketAddr,
     pub orig_dst: Option<SocketAddr>,
+    pub tls_status: TlsStatus,
 }
 
 /// Identifies a connection from the proxy to another process.
@@ -27,6 +28,22 @@ pub struct Client {
     pub proxy: Arc<ctx::Proxy>,
     pub remote: SocketAddr,
     pub metadata: destination::Metadata,
+    pub tls_status: TlsStatus,
+}
+
+/// Identifies whether or not a connection was secured with TLS,
+/// and, if it was not, the reason why.
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
+pub enum TlsStatus {
+    /// The TLS handshake was successful.
+    Success,
+    /// TLS was not enabled for this connection.
+    Disabled,
+    /// TLS was enabled for this connection, but we have no valid
+    /// config.
+    NoConfig,
+    // TODO: When the proxy falls back to plaintext on handshake
+    // failures, we'll want to add a variant for that here as well.
 }
 
 impl Ctx {
@@ -36,6 +53,13 @@ impl Ctx {
             Ctx::Server(ref ctx) => &ctx.proxy,
         }
     }
+
+    pub fn tls_status(&self) -> TlsStatus {
+        match self {
+            Ctx::Client(ctx)  => ctx.tls_status,
+            Ctx::Server(ctx) => ctx.tls_status,
+        }
+    }
 }
 
 impl Server {
@@ -44,12 +68,14 @@ impl Server {
         local: &SocketAddr,
         remote: &SocketAddr,
         orig_dst: &Option<SocketAddr>,
+        tls_status: TlsStatus,
     ) -> Arc<Server> {
         let s = Server {
             proxy: Arc::clone(proxy),
             local: *local,
             remote: *remote,
             orig_dst: *orig_dst,
+            tls_status,
         };
 
         Arc::new(s)
@@ -84,11 +110,13 @@ impl Client {
         proxy: &Arc<ctx::Proxy>,
         remote: &SocketAddr,
         metadata: destination::Metadata,
+        tls_status: TlsStatus,
     ) -> Arc<Client> {
         let c = Client {
             proxy: Arc::clone(proxy),
             remote: *remote,
             metadata,
+            tls_status,
         };
 
         Arc::new(c)
@@ -102,7 +130,6 @@ impl Client {
         self.metadata.dst_labels()
     }
 }
-
 impl From<Arc<Client>> for Ctx {
     fn from(c: Arc<Client>) -> Self {
         Ctx::Client(c)

proxy/src/inbound.rs

@@ -133,7 +133,10 @@ mod tests {
 
             let inbound = new_inbound(None, &ctx);
 
-            let srv_ctx = ctx::transport::Server::new(&ctx, &local, &remote, &Some(orig_dst));
+            let srv_ctx = ctx::transport::Server::new(
+                &ctx, &local, &remote, &Some(orig_dst),
+                ctx::transport::TlsStatus::Disabled,
+            );
 
             let rec = srv_ctx.orig_dst_if_not_local().map(make_key_http1);
 
@@ -160,6 +163,7 @@ mod tests {
                     &local,
                     &remote,
                     &None,
+                    ctx::transport::TlsStatus::Disabled,
                 ));
 
             inbound.recognize(&req) == default.map(make_key_http1)
@@ -191,6 +195,7 @@ mod tests {
                     &local,
                     &remote,
                     &Some(local),
+                    ctx::transport::TlsStatus::Disabled,
                 ));
 
             inbound.recognize(&req) == default.map(make_key_http1)

proxy/src/telemetry/metrics/labels.rs

@@ -21,6 +21,9 @@ pub struct RequestLabels {
     /// The value of the `:authority` (HTTP/2) or `Host` (HTTP/1.1) header of
     /// the request.
     authority: Option<http::uri::Authority>,
+
+    /// Whether or not the request was made over TLS.
+    tls_status: ctx::transport::TlsStatus,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
@@ -46,6 +49,9 @@ pub struct TransportLabels {
     direction: Direction,
 
     peer: Peer,
+
+    /// Was the transport secured with TLS?
+    tls_status: ctx::transport::TlsStatus,
 }
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
@@ -95,8 +101,14 @@ impl RequestLabels {
             direction,
             outbound_labels,
             authority,
+            tls_status: req.tls_status(),
         }
     }
+
+    #[cfg(test)]
+    pub fn tls_status(&self) -> ctx::transport::TlsStatus {
+        self.tls_status
+    }
 }
 
 impl fmt::Display for RequestLabels {
@@ -114,6 +126,8 @@ impl fmt::Display for RequestLabels {
             write!(f, ",{}", outbound)?;
         }
 
+        write!(f, "{}", self.tls_status)?;
+
         Ok(())
     }
 }
@@ -145,6 +159,11 @@ impl ResponseLabels {
             classification: Classification::Failure,
         }
     }
+
+    #[cfg(test)]
+    pub fn tls_status(&self) -> ctx::transport::TlsStatus {
+        self.request_labels.tls_status
+    }
 }
 
 impl fmt::Display for ResponseLabels {
@@ -301,17 +320,28 @@ impl TransportLabels {
                 ctx::transport::Ctx::Server(_) => Peer::Src,
                 ctx::transport::Ctx::Client(_) => Peer::Dst,
             },
+            tls_status: ctx.tls_status(),
         }
     }
+
+    #[cfg(test)]
+    pub fn tls_status(&self) -> ctx::transport::TlsStatus {
+        self.tls_status
+    }
 }
 
 impl fmt::Display for TransportLabels {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Display::fmt(&self.direction, f)?;
+        write!(f, "{},{}{}", self.direction, self.peer, self.tls_status)
-        f.pad(match self.peer {
+    }
-            Peer::Src => ",peer=\"src\"",
+}
-            Peer::Dst => ",peer=\"dst\"",
+
-        })
+impl fmt::Display for Peer {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            Peer::Src => f.pad("peer=\"src\""),
+            Peer::Dst => f.pad("peer=\"dst\""),
+        }
     }
 }
 
@@ -326,6 +356,11 @@ impl TransportCloseLabels {
             classification: Classification::transport_close(close),
         }
     }
+
+    #[cfg(test)]
+    pub fn tls_status(&self) -> ctx::transport::TlsStatus  {
+        self.transport.tls_status()
+    }
 }
 
 impl fmt::Display for TransportCloseLabels {
@@ -334,3 +369,18 @@ impl fmt::Display for TransportCloseLabels {
     }
 }
 
+// TLS status is the only label that prints its own preceding comma, because
+// there is a case when we don't print a label. If the comma was added by
+// whatever owns a TlsStatus, and the status is Disabled, we might sometimes
+// get double commas.
+// TODO: There's got to be a nicer way to handle this.
+impl fmt::Display for ctx::transport::TlsStatus {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use ctx::transport::TlsStatus;
+        match *self {
+            TlsStatus::Disabled => Ok(()),
+            TlsStatus::NoConfig => f.pad(",tls=\"no_config\""),
+            TlsStatus::Success  => f.pad(",tls=\"true\""),
+        }
+    }
+}

proxy/src/telemetry/metrics/mod.rs

@@ -285,7 +285,7 @@ mod tests {
         server: &Arc<ctx::transport::Server>,
         team: &str
     ) {
-        let client = client(&proxy, vec![("team", team)]);
+        let client = client(&proxy, vec![("team", team)], ctx::transport::TlsStatus::Disabled);
         let (req, rsp) = request("http://nba.com", &server, &client);
 
         let client_transport = Arc::new(ctx::transport::Ctx::Client(client));
@@ -310,7 +310,7 @@ mod tests {
         let process = process();
         let proxy = ctx::Proxy::outbound(&process);
 
-        let server = server(&proxy);
+        let server = server(&proxy, ctx::transport::TlsStatus::Disabled);
         let server_transport = Arc::new(ctx::transport::Ctx::Server(server.clone()));
 
         let mut root = Root::default();

proxy/src/telemetry/metrics/record.rs

@@ -93,20 +93,20 @@ mod test {
         metrics::{self, labels},
         Event,
     };
-    use ctx::{self, test_util::* };
+    use ctx::{self, test_util::*, transport::TlsStatus};
     use std::time::{Duration, Instant};
 
-    #[test]
+
-    fn record_response_end() {
+    fn test_record_response_end_outbound(client_tls: TlsStatus, server_tls: TlsStatus) {
         let process = process();
         let proxy = ctx::Proxy::outbound(&process);
-        let server = server(&proxy);
+        let server = server(&proxy, server_tls);
 
         let client = client(&proxy, vec![
             ("service", "draymond"),
             ("deployment", "durant"),
             ("pod", "klay"),
-        ]);
+        ], client_tls);
 
         let (_, rsp) = request("http://buoyant.io", &server, &client);
 
@@ -128,6 +128,8 @@ mod test {
         let ev = Event::StreamResponseEnd(rsp.clone(), end.clone());
         let labels = labels::ResponseLabels::new(&rsp, None);
 
+        assert_eq!(labels.tls_status(), client_tls);
+
         assert!(r.metrics.lock()
             .expect("lock")
             .responses.scopes
@@ -152,21 +154,20 @@ mod test {
 
     }
 
-    #[test]
+    fn test_record_one_conn_request_outbound(client_tls: TlsStatus, server_tls: TlsStatus) {
-    fn record_one_conn_request() {
         use self::Event::*;
         use self::labels::*;
         use std::sync::Arc;
 
         let process = process();
         let proxy = ctx::Proxy::outbound(&process);
-        let server = server(&proxy);
+        let server = server(&proxy, server_tls);
 
         let client = client(&proxy, vec![
             ("service", "draymond"),
             ("deployment", "durant"),
             ("pod", "klay"),
-        ]);
+        ], client_tls);
 
         let (req, rsp) = request("http://buoyant.io", &server, &client);
         let server_transport =
@@ -232,6 +233,13 @@ mod test {
             &transport_close,
         );
 
+        assert_eq!(client_tls, req_labels.tls_status());
+        assert_eq!(client_tls, rsp_labels.tls_status());
+        assert_eq!(client_tls, client_open_labels.tls_status());
+        assert_eq!(client_tls, client_close_labels.tls_status());
+        assert_eq!(server_tls, srv_open_labels.tls_status());
+        assert_eq!(server_tls, srv_close_labels.tls_status());
+
         {
             let lock = r.metrics.lock()
                 .expect("lock");
@@ -315,4 +323,43 @@ mod test {
         }
     }
 
+    #[test]
+    fn record_one_conn_request_outbound_client_tls() {
+        test_record_one_conn_request_outbound(TlsStatus::Success, TlsStatus::Disabled)
+    }
+
+    #[test]
+    fn record_one_conn_request_outbound_server_tls() {
+        test_record_one_conn_request_outbound(TlsStatus::Disabled, TlsStatus::Success)
+    }
+
+    #[test]
+    fn record_one_conn_request_outbound_both_tls() {
+        test_record_one_conn_request_outbound(TlsStatus::Success, TlsStatus::Success)
+    }
+
+    #[test]
+    fn record_one_conn_request_outbound_no_tls() {
+        test_record_one_conn_request_outbound(TlsStatus::Disabled, TlsStatus::Disabled)
+    }
+
+    #[test]
+    fn record_response_end_outbound_client_tls() {
+        test_record_response_end_outbound(TlsStatus::Success, TlsStatus::Disabled)
+    }
+
+    #[test]
+    fn record_response_end_outbound_server_tls() {
+        test_record_response_end_outbound(TlsStatus::Disabled, TlsStatus::Success)
+    }
+
+    #[test]
+    fn record_response_end_outbound_both_tls() {
+        test_record_response_end_outbound(TlsStatus::Success, TlsStatus::Success)
+    }
+
+    #[test]
+    fn record_response_end_outbound_no_tls() {
+        test_record_response_end_outbound(TlsStatus::Disabled, TlsStatus::Disabled)
+    }
 }

proxy/src/transparency/server.rs

@@ -126,6 +126,7 @@ where
             &local_addr,
             &remote_addr,
             &orig_dst,
+            connection.tls_status(),
         );
         let log = self.log.clone()
             .with_remote(remote_addr);

proxy/src/transparency/tcp.rs

@@ -8,7 +8,11 @@ use tokio_connect::Connect;
 use tokio::io::{AsyncRead, AsyncWrite};
 
 use control::destination;
-use ctx::transport::{Client as ClientCtx, Server as ServerCtx};
+use ctx::transport::{
+    Client as ClientCtx,
+    Server as ServerCtx,
+    TlsStatus,
+};
 use telemetry::Sensors;
 use timeout::Timeout;
 use transport;
@@ -59,6 +63,12 @@ impl Proxy {
             &srv_ctx.proxy,
             &orig_dst,
             destination::Metadata::no_metadata(),
+            // A raw TCP client connection may be or may not be TLS traffic,
+            // but the `TlsStatus` field indicates whether _the proxy_ is
+            // responsible for the encryption, so set this to "Disabled".
+            // XXX: Should raw TCP connections have a different TLS status
+            // from HTTP connections for which TLS is disabled?
+            TlsStatus::Disabled,
         );
         let c = Timeout::new(
             transport::Connect::new(orig_dst, None), // No TLS.