finagle: Support TLS Snooping in Mux

Problem

We use thriftmux all over the place and it supports downgrading to vanilla
thrift so that non-thriftmux clients can still play in our ecosystem. However,
after we added opportunistic tls and turned it on everywhere that is no
longer true since we need to make sure the connection is secure and we
previously only allowed that through a cleartext-upgrade, only supported
in mux.

Solution

Support TLS snooping in finagle mux. If a client starts a connection as TLS
we consider that the same as the client saying TLS is required. This allows
us to downgrade to vanilla thrift via the same path we'd normally follow with
cleartext and still allowing the server to require a secure connection.

Differential Revision: https://phabricator.twitter.biz/D584638

CHANGELOG.rst

@@ -15,6 +15,11 @@ New Features
   limit rather than rejecting them.  A `buffered_streams` gauge has been added to track the
   current number of buffered streams.  ``PHAB_ID=D643138``
 
+* finagle-mux: Added support for TLS snooping to the mux protocol. This allows a thriftmux
+  server to start a connection as TLS or follow the existing upgrade pathway at the leisure of
+  the client. This also allows the server to support opportunistic TLS and still downgrade to
+  vanilla thrift. ``PHAB_ID=D584638``
+
 * finagle-netty4: Added a new counter to keep track of the number of TLS connections that were
   started via snooping. ``PHAB_ID=D667652``
 

finagle-core/src/main/scala/com/twitter/finagle/ssl/OpportunisticTls.scala

@@ -1,16 +1,9 @@
 package com.twitter.finagle.ssl
 
-import com.twitter.finagle.Stack
 import com.twitter.io.Buf
 
 object OpportunisticTls {
 
-  case class Param(level: Level)
-
-  object Param {
-    implicit val param = Stack.Param(Param(Off))
-  }
-
   /**
    * Configures the level of TLS that the client or server can support or must
    * support.

finagle-core/src/main/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreter.scala

@@ -1,18 +1,35 @@
 package com.twitter.finagle.ssl
 
 import com.twitter.finagle.Stack
+import com.twitter.finagle.param.OppTls
 import com.twitter.finagle.ssl.server.SslServerConfiguration
 import com.twitter.finagle.transport.Transport
 import com.twitter.logging.Logger
 
-private[finagle] object SnoopingLevelInterpreter {
+/**
+ * Stack params for configuring TLS snooping
+ *
+ * TLS snooping is a server side feature which allows servers to use both cleartext
+ * and TLS socket connections on the same port. This should only be used if it is
+ * desirable but not required that traffic be encrypted.
+ *
+ * There are two classes of protocols considered: protocols that support TLS
+ * negotiation such as Mux and those that don't such as the framed thrift transport.
+ * The two classes of protocols have a different truth matrix for determining
+ * whether to enable TLS negotiation (see below). For example, a negotiating TLS
+ * connection that _requires_ TLS may enable snooping because if the connection
+ * starts cleartext the session itself may upgrade to a secure connection later.
+ * In contrast, a cleartext HTTP connection has no way to upgrade to a secure
+ * connection and thus if TLS is required snooping must be disabled.
+ */
+object SnoopingLevelInterpreter {
 
   private val logger = Logger.get()
 
   case class Param(interpreter: Interpreter)
 
   object Param {
-    implicit lazy val param: Stack.Param[Param] = Stack.Param(Param(Disabled))
+    implicit lazy val param: Stack.Param[Param] = Stack.Param(Off)
   }
 
   sealed abstract class Interpreter
@@ -24,6 +41,11 @@ private[finagle] object SnoopingLevelInterpreter {
   case class Enabled(predicate: (OpportunisticTls.Level, SslServerConfiguration) => Boolean)
       extends Interpreter
 
+  /**
+   * Tls snooping configuration that disables TLS snooping.
+   */
+  val Off: Param = Param(Disabled)
+
   /**
    * TLS Snooping configuration for protocols that don't support negotiation.
    *
@@ -74,7 +96,8 @@ private[finagle] object SnoopingLevelInterpreter {
     params[SnoopingLevelInterpreter.Param].interpreter match {
       case Disabled => false
       case Enabled(enableSnooping) =>
-        val level = params[OpportunisticTls.Param].level
+        // If the opportunistic level is not configured we default to desired.
+        val level = params[OppTls].level.getOrElse(OpportunisticTls.Desired)
         params[Transport.ServerSsl].sslServerConfiguration match {
           case Some(config) =>
             enableSnooping(level, config)

finagle-core/src/test/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreterTest.scala

@@ -1,6 +1,7 @@
 package com.twitter.finagle.ssl
 
 import com.twitter.finagle.Stack
+import com.twitter.finagle.param.OppTls
 import com.twitter.finagle.ssl.server.SslServerConfiguration
 import com.twitter.finagle.transport.Transport
 import org.scalatest.FunSuite
@@ -32,7 +33,7 @@ class SnoopingLevelInterpreterTest extends FunSuite {
       c <- clientAuthServerConfigs
       l <- optTlsOptions
     } {
-      val params = base + OpportunisticTls.Param(l) + Transport.ServerSsl(Some(c))
+      val params = base + OppTls(Some(l)) + Transport.ServerSsl(Some(c))
       assert(!SnoopingLevelInterpreter.shouldEnableSnooping(params))
     }
   }
@@ -43,7 +44,7 @@ class SnoopingLevelInterpreterTest extends FunSuite {
       l <- optTlsOptions
       i <- interpreters
     } {
-      val params = base + OpportunisticTls.Param(l) + i
+      val params = base + OppTls(Some(l)) + i
       assert(!SnoopingLevelInterpreter.shouldEnableSnooping(params))
     }
   }
@@ -56,7 +57,7 @@ class SnoopingLevelInterpreterTest extends FunSuite {
     for {
       l <- optTlsOptions
     } {
-      val params = base + OpportunisticTls.Param(l)
+      val params = base + OppTls(Some(l))
       assert(!SnoopingLevelInterpreter.shouldEnableSnooping(params))
     }
   }
@@ -69,7 +70,7 @@ class SnoopingLevelInterpreterTest extends FunSuite {
       c <- clientAuthServerConfigs if c.clientAuth != ClientAuth.Needed
       l <- optTlsOptions
     } {
-      val params = base + Transport.ServerSsl(Some(c)) + OpportunisticTls.Param(l)
+      val params = base + Transport.ServerSsl(Some(c)) + OppTls(Some(l))
       assert(
         SnoopingLevelInterpreter.shouldEnableSnooping(params) == (l == OpportunisticTls.Desired))
     }
@@ -82,7 +83,7 @@ class SnoopingLevelInterpreterTest extends FunSuite {
       c <- clientAuthServerConfigs
       l <- optTlsOptions
     } {
-      val params = base + Transport.ServerSsl(Some(c)) + OpportunisticTls.Param(l)
+      val params = base + Transport.ServerSsl(Some(c)) + OppTls(Some(l))
       assert(SnoopingLevelInterpreter.shouldEnableSnooping(params) == (l != OpportunisticTls.Off))
     }
   }

finagle-http/src/test/scala/com/twitter/finagle/TlsSnoopingEndToEndTest.scala

@@ -4,6 +4,7 @@ import com.twitter.conversions.DurationOps._
 import com.twitter.finagle
 import com.twitter.finagle.http.{Request, Response}
 import com.twitter.finagle.http.ssl.HttpSslTestComponents
+import com.twitter.finagle.param.OppTls
 import com.twitter.finagle.ssl.{ClientAuth, OpportunisticTls, SnoopingLevelInterpreter, TlsSnooping}
 import com.twitter.finagle.transport.Transport
 import com.twitter.util.{Await, Awaitable, Future}
@@ -25,7 +26,7 @@ class TlsSnoopingEndToEndTest extends FunSuite {
     (if (useH2) base.withHttp2 else base.withNoHttp2)
       .configured(Transport.ServerSsl(
         Some(HttpSslTestComponents.unauthenticatedServerConfig.copy(clientAuth = clientAuth))))
-      .configured(OpportunisticTls.Param(level))
+      .configured(OppTls(Some(level)))
       .configured(TlsSnooping.Param(snooper))
   }
 

finagle-http/src/test/scala/com/twitter/finagle/http/AlpnToTlsSnoopingEndToEndTest.scala

@@ -2,6 +2,7 @@ package com.twitter.finagle.http
 
 import com.twitter.finagle.Http
 import com.twitter.finagle.http.ssl.HttpSslTestComponents
+import com.twitter.finagle.param.OppTls
 import com.twitter.finagle.ssl.OpportunisticTls
 import com.twitter.finagle.ssl.server.SslServerConfiguration
 
@@ -14,5 +15,5 @@ class AlpnToTlsSnoopingEndToEndTest extends Http2AlpnTest {
   override def serverImpl(): Http.Server =
     super
       .serverImpl()
-      .configured(OpportunisticTls.Param(OpportunisticTls.Desired))
+      .configured(OppTls(Some(OpportunisticTls.Desired)))
 }

finagle-http/src/test/scala/com/twitter/finagle/http/H2CToTlsSnoopingEndToEndTest.scala

@@ -2,6 +2,7 @@ package com.twitter.finagle.http
 
 import com.twitter.finagle
 import com.twitter.finagle.http.ssl.HttpSslTestComponents
+import com.twitter.finagle.param.OppTls
 import com.twitter.finagle.ssl.{OpportunisticTls, SnoopingLevelInterpreter}
 import com.twitter.finagle.transport.Transport
 
@@ -15,5 +16,5 @@ class H2CToTlsSnoopingEndToEndTest extends H2CEndToEndTest {
       .serverImpl()
       .configured(SnoopingLevelInterpreter.EnabledForNonNegotiatingProtocols)
       .configured(Transport.ServerSsl(Some(HttpSslTestComponents.unauthenticatedServerConfig)))
-      .configured(OpportunisticTls.Param(OpportunisticTls.Desired))
+      .configured(OppTls(Some(OpportunisticTls.Desired)))
 }

finagle-mux/src/main/scala/com/twitter/finagle/Mux.scala

@@ -1,14 +1,14 @@
 package com.twitter.finagle
 
 import com.twitter.conversions.StorageUnitOps._
-import com.twitter.finagle.Mux.param.{CompressionPreferences, MaxFrameSize, OppTls}
+import com.twitter.finagle.Mux.param.{CompressionPreferences, MaxFrameSize}
 import com.twitter.finagle.client._
 import com.twitter.finagle.factory.TimeoutFactory
 import com.twitter.finagle.naming.BindingFactory
 import com.twitter.finagle.filter.{NackAdmissionFilter, PayloadSizeFilter}
 import com.twitter.finagle.mux.Handshake.Headers
 import com.twitter.finagle.mux.pushsession._
-import com.twitter.finagle.mux.transport._
+import com.twitter.finagle.mux.transport.{OpportunisticTls => MuxOpportunisticTls, _}
 import com.twitter.finagle.mux.{
   ExportCompressionUsage,
   Handshake,
@@ -20,10 +20,18 @@ import com.twitter.finagle.mux.{
 import com.twitter.finagle.netty4.pushsession.{Netty4PushListener, Netty4PushTransporter}
 import com.twitter.finagle.netty4.ssl.server.Netty4ServerSslChannelInitializer
 import com.twitter.finagle.netty4.ssl.client.Netty4ClientSslChannelInitializer
-import com.twitter.finagle.param.{Label, ProtocolLibrary, Stats, Timer, WithDefaultLoadBalancer}
+import com.twitter.finagle.param.{
+  Label,
+  OppTls,
+  ProtocolLibrary,
+  Stats,
+  Timer,
+  WithDefaultLoadBalancer
+}
 import com.twitter.finagle.pool.BalancingPool
 import com.twitter.finagle.pushsession._
 import com.twitter.finagle.server._
+import com.twitter.finagle.ssl.{OpportunisticTls, SnoopingLevelInterpreter}
 import com.twitter.finagle.tracing._
 import com.twitter.finagle.transport.Transport.{ClientSsl, ServerSsl}
 import com.twitter.finagle.transport.Transport
@@ -103,17 +111,22 @@ object Mux extends Client[mux.Request, mux.Response] with Server[mux.Request, mu
 
     // tells the Netty4Transporter not to turn on TLS so we can turn it on later
     private[finagle] def removeTlsIfOpportunisticClient(params: Stack.Params): Stack.Params = {
-      params[param.OppTls].level match {
+      params[OppTls].level match {
         case None => params
         case _ => params + Transport.ClientSsl(None)
       }
     }
 
     // tells the Netty4Listener not to turn on TLS so we can turn it on later
     private[finagle] def removeTlsIfOpportunisticServer(params: Stack.Params): Stack.Params = {
-      params[param.OppTls].level match {
-        case None => params
-        case _ => params + Transport.ServerSsl(None)
+      // If we have snooping enabled we're going to leave the params as is.
+      // The snooper will decided whether to add the TLS handler.
+      if (SnoopingLevelInterpreter.shouldEnableSnooping(params)) params
+      else {
+        params[OppTls].level match {
+          case None => params
+          case _ => params + Transport.ServerSsl(None)
+        }
       }
     }
 
@@ -201,7 +214,7 @@ object Mux extends Client[mux.Request, mux.Response] with Server[mux.Request, mu
     ): Handshake.Headers = {
       val buffer = ArrayBuffer(
         MuxFramer.Header.KeyBuf -> MuxFramer.Header.encodeFrameSize(maxFrameSize.inBytes.toInt),
-        OpportunisticTls.Header.KeyBuf -> tlsLevel.buf
+        MuxOpportunisticTls.Header.KeyBuf -> tlsLevel.buf
       )
 
       if (!compressionPreferences.isDisabled) {
@@ -215,8 +228,8 @@ object Mux extends Client[mux.Request, mux.Response] with Server[mux.Request, mu
      * Check the opportunistic TLS configuration to ensure it's in a consistent state
      */
     private[finagle] def validateTlsParamConsistency(params: Stack.Params): Unit = {
-      if (param.OppTls.enabled(params) && params[ClientSsl].sslClientConfiguration.isEmpty) {
-        val level = params[param.OppTls].level
+      if (OppTls.enabled(params) && params[ClientSsl].sslClientConfiguration.isEmpty) {
+        val level = params[OppTls].level
         throw new IllegalStateException(
           s"Client desired opportunistic TLS ($level) but ClientSsl param is empty."
         )
@@ -395,7 +408,7 @@ object Mux extends Client[mux.Request, mux.Response] with Server[mux.Request, mu
 
       val withoutCompression = Seq(
         MuxFramer.Header.KeyBuf -> MuxFramer.Header.encodeFrameSize(maxFrameSize.inBytes.toInt),
-        OpportunisticTls.Header.KeyBuf -> tlsLevel.buf
+        MuxOpportunisticTls.Header.KeyBuf -> tlsLevel.buf
       )
 
       if (compressionFormats.isDisabled) {
@@ -411,8 +424,8 @@ object Mux extends Client[mux.Request, mux.Response] with Server[mux.Request, mu
      */
     private[finagle] def validateTlsParamConsistency(params: Stack.Params): Unit = {
       // We need to make sure
-      if (param.OppTls.enabled(params) && params[ServerSsl].sslServerConfiguration.isEmpty) {
-        val level = params[param.OppTls].level
+      if (OppTls.enabled(params) && params[ServerSsl].sslServerConfiguration.isEmpty) {
+        val level = params[OppTls].level
         throw new IllegalStateException(
           s"Server desired opportunistic TLS ($level) but ServerSsl param is empty."
         )

finagle-mux/src/main/scala/com/twitter/finagle/mux/package.scala

@@ -144,6 +144,33 @@ import com.twitter.finagle.toggle.{StandardToggleMap, ToggleMap}
  *  Rejected      1 << 1    Request was rejected/Nacked by the server
  *  NonRetryable  1 << 2    Request should not be retried
  *
+ * =Security=
+ *
+ * TLS is supported via three mechanisms:
+ * - Explicit and exclusive TLS. This pathway involves requiring the establishment of TLS
+ *   immediately after establishing the socket connection. This is configured by adding TLS
+ *   configuration to the client or server and __not__ configuring opportunistic TLS or
+ *   TLS snooping (see below).
+ *
+ * - Negotiated Opportunistic TLS. This pathway involves starting the connection as cleartext
+ *   and the client and server subsequently negotiate a TLS level via the handshake. Based on
+ *   that handshake the connection is either left as cleartext or upgraded to TLS. This is
+ *   configured by adding TLS configuration and also configuring an opportunistic TLS level
+ *   but __not__ configuring TLS snooping.
+ *
+ *   In this pathway there are three configuration options:
+ *   - `Off` signals that TLS is not supported by this peer
+ *   - `Desired` signals that TLS is preferred but not required by this peer
+ *   - `Required` signals that this peer will only allow the session to continue over TLS
+ *
+ * - TLS snooping. This pathway allows a server to use TLS either by performing a TLS
+ *   handshake immediately after the socket is established or by starting the session as
+ *   cleartext or using the negotiated pathway described above. If the session is started as
+ *   a TLS session the headers that drive the opportunistic TLS pathway are ignored.
+ *
+ *   Note that the server may still require TLS but leaves the option to start TLS immediately
+ *   after establishing the socket or starting cleartext and requiring TLS via the opportunistic
+ *   TLS pathway described above.
  */
 package object mux {
 

finagle-mux/src/main/scala/com/twitter/finagle/mux/pushsession/Negotiation.scala

@@ -1,6 +1,6 @@
 package com.twitter.finagle.mux.pushsession
 
-import com.twitter.finagle.Mux.param.{CompressionPreferences, OppTls}
+import com.twitter.finagle.Mux.param.CompressionPreferences
 import com.twitter.finagle.pushsession.{PushChannelHandle, PushSession}
 import com.twitter.finagle.liveness.FailureDetector
 import com.twitter.finagle.mux.Handshake.Headers
@@ -9,9 +9,11 @@ import com.twitter.finagle.mux.transport.{
   CompressionNegotiation,
   IncompatibleNegotiationException,
   MuxFramer,
-  OpportunisticTls
+  OpportunisticTls => MuxOpportunisticTls
 }
 import com.twitter.finagle.mux.{Handshake, Request, Response}
+import com.twitter.finagle.param.OppTls
+import com.twitter.finagle.ssl.OpportunisticTls
 import com.twitter.finagle.{Service, Stack, param}
 import com.twitter.io.{Buf, ByteReader}
 import com.twitter.logging.{Level, Logger}
@@ -65,11 +67,24 @@ private[finagle] abstract class Negotiation(
     handle: PushChannelHandle[ByteReader, Buf],
     peerHeaders: Option[Headers],
     onTlsHandshakeComplete: Try[Unit] => Unit
+  ): Unit =
+    if (handle.sslSessionInfo.usingSsl) {
+      // If we're already encrypted this is already decided and we're at max security.
+      log.debug("Session already encrypted. Skipping OppTls header check.")
+      onTlsHandshakeComplete(Try.Unit)
+    } else {
+      negotiateOppTlsViaHeaders(handle, peerHeaders, onTlsHandshakeComplete)
+    }
+
+  private[this] def negotiateOppTlsViaHeaders(
+    handle: PushChannelHandle[ByteReader, Buf],
+    peerHeaders: Option[Headers],
+    onTlsHandshakeComplete: Try[Unit] => Unit
   ): Unit = {
     val localEncryptLevel = params[OppTls].level.getOrElse(OpportunisticTls.Off)
     val remoteEncryptLevel = peerHeaders
-      .flatMap(Handshake.valueOf(OpportunisticTls.Header.KeyBuf, _)) match {
-      case Some(buf) => OpportunisticTls.Header.decodeLevel(buf)
+      .flatMap(Handshake.valueOf(MuxOpportunisticTls.Header.KeyBuf, _)) match {
+      case Some(buf) => MuxOpportunisticTls.Header.decodeLevel(buf)
       case None =>
         log.debug(
           "Peer either didn't negotiate or didn't send an Opportunistic Tls preference: " +
@@ -79,7 +94,7 @@ private[finagle] abstract class Negotiation(
     }
 
     try {
-      val useTls = OpportunisticTls.negotiate(localEncryptLevel, remoteEncryptLevel)
+      val useTls = MuxOpportunisticTls.negotiate(localEncryptLevel, remoteEncryptLevel)
       if (log.isLoggable(Level.DEBUG)) {
         log.debug(
           s"Successfully negotiated TLS with remote peer. Using TLS: $useTls local level: " +