From 60705fd270a3ef85c2d31ae09626971cb12b77a8 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Thu, 20 May 2021 16:14:26 +0000 Subject: [PATCH] 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 | 5 + .../finagle/ssl/OpportunisticTls.scala | 7 - .../ssl/SnoopingLevelInterpreter.scala | 29 +- .../ssl/SnoopingLevelInterpreterTest.scala | 11 +- .../finagle/TlsSnoopingEndToEndTest.scala | 3 +- .../http/AlpnToTlsSnoopingEndToEndTest.scala | 3 +- .../http/H2CToTlsSnoopingEndToEndTest.scala | 3 +- .../main/scala/com/twitter/finagle/Mux.scala | 39 ++- .../com/twitter/finagle/mux/package.scala | 27 ++ .../finagle/mux/pushsession/Negotiation.scala | 25 +- .../MuxClientNegotiatingSessionTest.scala | 6 +- .../server/Netty4TlsSnoopingHandlerTest.scala | 4 +- .../scala/com/twitter/finagle/ThriftMux.scala | 3 +- .../MuxDowngradingNegotiatorTest.scala | 61 ++++- .../ssl/AbstractThriftSmuxSslTest.scala | 247 ++++++++++++++++++ .../ssl/PriorEncryptionSslTest.scala | 26 ++ .../thriftmux/ssl/SnoopingSmuxSslTest.scala | 33 +++ .../thriftmux/ssl/ThriftSmuxSslTest.scala | 245 ++--------------- .../ssl/ThriftSmuxSslTestComponents.scala | 82 ++++-- .../ssl/VanillaThriftSmuxSslTest.scala | 30 +++ 20 files changed, 599 insertions(+), 290 deletions(-) create mode 100644 finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/AbstractThriftSmuxSslTest.scala create mode 100644 finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/PriorEncryptionSslTest.scala create mode 100644 finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/SnoopingSmuxSslTest.scala create mode 100644 finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/VanillaThriftSmuxSslTest.scala diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e49222e8cc..419b200faa 100644 --- a/CHANGELOG.rst +++ b/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`` diff --git a/finagle-core/src/main/scala/com/twitter/finagle/ssl/OpportunisticTls.scala b/finagle-core/src/main/scala/com/twitter/finagle/ssl/OpportunisticTls.scala index 9a64014cf6..5e70ed5eff 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/ssl/OpportunisticTls.scala +++ b/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. diff --git a/finagle-core/src/main/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreter.scala b/finagle-core/src/main/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreter.scala index 678c6e44ce..f4478d0db2 100644 --- a/finagle-core/src/main/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreter.scala +++ b/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) diff --git a/finagle-core/src/test/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreterTest.scala b/finagle-core/src/test/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreterTest.scala index 664b5f47de..35264c8e95 100644 --- a/finagle-core/src/test/scala/com/twitter/finagle/ssl/SnoopingLevelInterpreterTest.scala +++ b/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)) } } diff --git a/finagle-http/src/test/scala/com/twitter/finagle/TlsSnoopingEndToEndTest.scala b/finagle-http/src/test/scala/com/twitter/finagle/TlsSnoopingEndToEndTest.scala index 66f031c8b8..0d3d70924f 100644 --- a/finagle-http/src/test/scala/com/twitter/finagle/TlsSnoopingEndToEndTest.scala +++ b/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)) } diff --git a/finagle-http/src/test/scala/com/twitter/finagle/http/AlpnToTlsSnoopingEndToEndTest.scala b/finagle-http/src/test/scala/com/twitter/finagle/http/AlpnToTlsSnoopingEndToEndTest.scala index 9075c27640..b916444639 100644 --- a/finagle-http/src/test/scala/com/twitter/finagle/http/AlpnToTlsSnoopingEndToEndTest.scala +++ b/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))) } diff --git a/finagle-http/src/test/scala/com/twitter/finagle/http/H2CToTlsSnoopingEndToEndTest.scala b/finagle-http/src/test/scala/com/twitter/finagle/http/H2CToTlsSnoopingEndToEndTest.scala index 0dc977eb04..1e4bf88af3 100644 --- a/finagle-http/src/test/scala/com/twitter/finagle/http/H2CToTlsSnoopingEndToEndTest.scala +++ b/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))) } diff --git a/finagle-mux/src/main/scala/com/twitter/finagle/Mux.scala b/finagle-mux/src/main/scala/com/twitter/finagle/Mux.scala index 553273f220..919823a759 100644 --- a/finagle-mux/src/main/scala/com/twitter/finagle/Mux.scala +++ b/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,7 +111,7 @@ 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) } @@ -111,9 +119,14 @@ object Mux extends Client[mux.Request, mux.Response] with Server[mux.Request, mu // 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." ) diff --git a/finagle-mux/src/main/scala/com/twitter/finagle/mux/package.scala b/finagle-mux/src/main/scala/com/twitter/finagle/mux/package.scala index c8c8b191ea..af18db9c75 100644 --- a/finagle-mux/src/main/scala/com/twitter/finagle/mux/package.scala +++ b/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 { diff --git a/finagle-mux/src/main/scala/com/twitter/finagle/mux/pushsession/Negotiation.scala b/finagle-mux/src/main/scala/com/twitter/finagle/mux/pushsession/Negotiation.scala index b21d7c90be..5dc35ed5c5 100644 --- a/finagle-mux/src/main/scala/com/twitter/finagle/mux/pushsession/Negotiation.scala +++ b/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: " + diff --git a/finagle-mux/src/test/scala/com/twitter/finagle/mux/pushsession/MuxClientNegotiatingSessionTest.scala b/finagle-mux/src/test/scala/com/twitter/finagle/mux/pushsession/MuxClientNegotiatingSessionTest.scala index c7a4c2511d..644b9bd0b2 100644 --- a/finagle-mux/src/test/scala/com/twitter/finagle/mux/pushsession/MuxClientNegotiatingSessionTest.scala +++ b/finagle-mux/src/test/scala/com/twitter/finagle/mux/pushsession/MuxClientNegotiatingSessionTest.scala @@ -2,7 +2,7 @@ package com.twitter.finagle.mux.pushsession import com.twitter.conversions.StorageUnitOps._ import com.twitter.conversions.DurationOps._ -import com.twitter.finagle.Mux.param.{CompressionPreferences, MaxFrameSize, OppTls} +import com.twitter.finagle.Mux.param.{CompressionPreferences, MaxFrameSize} import com.twitter.finagle.mux.transport.CompressionNegotiation import com.twitter.finagle.Stack.Params import com.twitter.finagle.pushsession.PushChannelHandle @@ -10,7 +10,9 @@ import com.twitter.finagle.pushsession.utils.MockChannelHandle import com.twitter.finagle.mux.Handshake.{CanTinitMsg, Headers, TinitTag} import com.twitter.finagle.mux.Request import com.twitter.finagle.mux.transport.Message.Tdispatch -import com.twitter.finagle.mux.transport.{Message, MuxFramer, OpportunisticTls} +import com.twitter.finagle.mux.transport.{Message, MuxFramer} +import com.twitter.finagle.param.OppTls +import com.twitter.finagle.ssl.OpportunisticTls import com.twitter.finagle.stats.{InMemoryStatsReceiver, NullStatsReceiver} import com.twitter.finagle.{ChannelClosedException, Failure, FailureFlags, Mux, Path, liveness} import com.twitter.io.{Buf, ByteReader} diff --git a/finagle-netty4/src/test/scala/com/twitter/finagle/netty4/ssl/server/Netty4TlsSnoopingHandlerTest.scala b/finagle-netty4/src/test/scala/com/twitter/finagle/netty4/ssl/server/Netty4TlsSnoopingHandlerTest.scala index e466b0e4d5..516563b626 100644 --- a/finagle-netty4/src/test/scala/com/twitter/finagle/netty4/ssl/server/Netty4TlsSnoopingHandlerTest.scala +++ b/finagle-netty4/src/test/scala/com/twitter/finagle/netty4/ssl/server/Netty4TlsSnoopingHandlerTest.scala @@ -2,7 +2,7 @@ package com.twitter.finagle.netty4.ssl.server import com.twitter.finagle.Stack import com.twitter.finagle.netty4.ssl.Netty4SslTestComponents -import com.twitter.finagle.param.Stats +import com.twitter.finagle.param.{OppTls, Stats} import com.twitter.finagle.ssl.OpportunisticTls import com.twitter.finagle.stats.{InMemoryStatsReceiver, StatsReceiver} import com.twitter.finagle.transport.Transport @@ -59,7 +59,7 @@ class Netty4TlsSnoopingHandlerTest extends FunSuite with ScalaCheckDrivenPropert private[this] val params: Stack.Params = { Stack.Params.empty + Transport.ServerSsl(Some(Netty4SslTestComponents.serverConfig)) + - OpportunisticTls.Param(OpportunisticTls.Desired) + OppTls(Some(OpportunisticTls.Desired)) } private[this] def arrayToBuf(bytes: Array[Byte]): ByteBuf = { diff --git a/finagle-thriftmux/src/main/scala/com/twitter/finagle/ThriftMux.scala b/finagle-thriftmux/src/main/scala/com/twitter/finagle/ThriftMux.scala index 2e94a44666..0bb846b240 100644 --- a/finagle-thriftmux/src/main/scala/com/twitter/finagle/ThriftMux.scala +++ b/finagle-thriftmux/src/main/scala/com/twitter/finagle/ThriftMux.scala @@ -9,7 +9,7 @@ import com.twitter.finagle.client.{ import com.twitter.finagle.context.Contexts import com.twitter.finagle.context.RemoteInfo.Upstream import com.twitter.finagle.filter.{ClientExceptionTracingFilter => ExceptionTracingFilter} -import com.twitter.finagle.mux.transport.{MuxFailure, OpportunisticTls} +import com.twitter.finagle.mux.transport.MuxFailure import com.twitter.finagle.mux.{OpportunisticTlsParams, WithCompressionPreferences} import com.twitter.finagle.naming.BindingFactory import com.twitter.finagle.param.{ @@ -21,6 +21,7 @@ import com.twitter.finagle.param.{ } import com.twitter.finagle.server.{BackupRequest, StackBasedServer, StackServer} import com.twitter.finagle.service._ +import com.twitter.finagle.ssl.OpportunisticTls import com.twitter.finagle.stats.{ ClientStatsReceiver, ExceptionStatsHandler, diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/pushsession/MuxDowngradingNegotiatorTest.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/pushsession/MuxDowngradingNegotiatorTest.scala index c84fcb3bb9..d7182bcffd 100644 --- a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/pushsession/MuxDowngradingNegotiatorTest.scala +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/pushsession/MuxDowngradingNegotiatorTest.scala @@ -1,22 +1,26 @@ package com.twitter.finagle.thriftmux.pushsession import com.twitter.conversions.DurationOps._ -import com.twitter.finagle.Mux.param.OppTls import com.twitter.finagle.pushsession.RefPushSession import com.twitter.finagle.pushsession.utils.MockChannelHandle import com.twitter.finagle.{param => fparam} import com.twitter.finagle.mux.{Request, Response} -import com.twitter.finagle.mux.transport.{Message, OpportunisticTls} +import com.twitter.finagle.mux.transport.Message import com.twitter.finagle.{Service, Stack, ThriftMux} import com.twitter.finagle.mux.pushsession.{ MuxChannelHandle, MuxClientNegotiatingSession, SharedNegotiationStats } +import com.twitter.finagle.param.OppTls import com.twitter.finagle.stats.InMemoryStatsReceiver +import com.twitter.finagle.ssl.OpportunisticTls +import com.twitter.finagle.ssl.session.SslSessionInfo import com.twitter.finagle.thriftmux.thriftscala.TestService import com.twitter.io.{Buf, ByteReader} -import com.twitter.util.Future +import com.twitter.util.{Future, Try} +import java.security.cert.X509Certificate +import javax.net.ssl.SSLSession import org.apache.thrift.protocol.TBinaryProtocol import org.apache.thrift.transport.{TFramedTransport, TMemoryBuffer} import org.scalatest.FunSuite @@ -24,7 +28,7 @@ import org.scalatest.FunSuite class MuxDowngradingNegotiatorTest extends FunSuite { private class Ctx { - private class MockMuxChannelHandle + class MockMuxChannelHandle extends MuxChannelHandle( underlying = handle, ch = null, // only used in overridden method `sendAndForgetNow` @@ -33,6 +37,24 @@ class MuxDowngradingNegotiatorTest extends FunSuite { override def sendNowAndForget(buf: Buf): Unit = sendAndForget(buf) } + class TlsMockMuxChannelHandle extends MockMuxChannelHandle { + + var tlsActivated: Boolean = false + + override def turnOnTls(onHandshakeComplete: Try[Unit] => Unit): Unit = { + tlsActivated = true + } + + override val sslSessionInfo: SslSessionInfo = new SslSessionInfo { + def usingSsl: Boolean = true + def session: SSLSession = ??? + def sessionId: String = ??? + def cipherSuite: String = ??? + def localCertificates: Seq[X509Certificate] = ??? + def peerCertificates: Seq[X509Certificate] = ??? + } + } + lazy val statsReceiver: InMemoryStatsReceiver = new InMemoryStatsReceiver lazy val sharedStats = new SharedNegotiationStats(statsReceiver) @@ -123,6 +145,36 @@ class MuxDowngradingNegotiatorTest extends FunSuite { } } + test( + "allows downgraded session if opportunistic TLS is required AND the transport is already encrypted") { + new Ctx { + override lazy val params: Stack.Params = + ThriftMux.BaseServerParams + fparam.Stats(statsReceiver) + + OppTls(Some(OpportunisticTls.Required)) + + override lazy val muxChannelHandle: TlsMockMuxChannelHandle = new TlsMockMuxChannelHandle + + val thriftMsg = { + val buffer = new TMemoryBuffer(1) + val framed = new TFramedTransport(buffer) + val proto = new TBinaryProtocol(framed) + val arg = TestService.Query.Args("hi") + TestService.Query.Args.encode(arg, proto) + framed.flush() + val bytes = buffer.getArray + Buf.ByteArray.Owned(bytes) + } + + refSession.receive(ByteReader(thriftMsg)) + + // We don't need to initiate TLS if it's already started + assert(!muxChannelHandle.tlsActivated) + + assert(!handle.closedCalled) + assert(statsReceiver.counters(Seq("thriftmux", "downgraded_connects")) == 1L) + } + } + test("no infinite loops if we close before the handshake is complete and fail negotiation") { new Ctx { override lazy val params: Stack.Params = @@ -143,7 +195,6 @@ class MuxDowngradingNegotiatorTest extends FunSuite { val closeF = refSession.close(60.seconds) handle.serialExecutor.executeAll() refSession.receive(ByteReader(thriftMsg)) - assert(handle.closedCalled) handle.onClosePromise.setDone() assert(closeF.isDefined) diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/AbstractThriftSmuxSslTest.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/AbstractThriftSmuxSslTest.scala new file mode 100644 index 0000000000..bff5b7b622 --- /dev/null +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/AbstractThriftSmuxSslTest.scala @@ -0,0 +1,247 @@ +package com.twitter.finagle.thriftmux.ssl + +import com.twitter.conversions.DurationOps._ +import com.twitter.finagle.ssl.client.SslClientSessionVerifier +import com.twitter.finagle.ssl.server.SslServerSessionVerifier +import com.twitter.finagle.{ChannelClosedException, ListeningServer, SslVerificationFailedException} +import com.twitter.finagle.stats.{InMemoryStatsReceiver, NullStatsReceiver, StatsReceiver} +import com.twitter.finagle.thriftmux.ssl.ThriftSmuxSslTestComponents._ +import com.twitter.finagle.thriftmux.thriftscala._ +import com.twitter.util.{Await, Awaitable, Closable, Duration} +import org.scalatest.FunSuite +import org.scalatest.concurrent.Eventually + +abstract class AbstractThriftSmuxSslTest extends FunSuite with Eventually { + + protected def doMkTlsServer( + label: String = "server", + statsReceiver: StatsReceiver = NullStatsReceiver, + sessionVerifier: SslServerSessionVerifier = SslServerSessionVerifier.AlwaysValid + ): ListeningServer + + protected def doMkTlsClient( + port: Int, + label: String = "client", + statsReceiver: StatsReceiver = NullStatsReceiver, + sessionVerifier: SslClientSessionVerifier = SslClientSessionVerifier.AlwaysValid + ): TestService.MethodPerEndpoint + + // this method is a safety-net to ensure that resources always get cleaned up, even if + // an unexpected exception gets thrown + private[this] def tryWithResources[T](closables: Closable*)(fn: => T): T = + try { + fn + } finally { + await(Closable.all(closables: _*).close()) + } + + private[this] def assertGaugeIsZero( + statsReceiver: InMemoryStatsReceiver, + name: Array[String] + ): Unit = statsReceiver.gauges.get(name) match { + case Some(f) => assert(f() == 0.0) + case None => // all good + } + + private[this] def assertGaugeIsNonZero( + value: Float + )( + statsReceiver: InMemoryStatsReceiver, + name: Array[String] + ): Unit = statsReceiver.gauges.get(name) match { + case Some(f) => assert(f() == value) + case None => fail() + } + + def await[T](a: Awaitable[T], d: Duration = 2.seconds): T = + Await.result(a, d) + + private[this] val assertGaugeIsOne = assertGaugeIsNonZero(1.0f) _ + private[this] val assertGaugeIsTwo = assertGaugeIsNonZero(2.0f) _ + + private[this] def mkSuccessfulHelloRequest(client: TestService.MethodPerEndpoint): Unit = { + val response = await(client.query("hello")) + assert(response == "hellohello") + } + + test("Single client and server results in server TLS connections incremented and decremented") { + val serverStats = new InMemoryStatsReceiver + val serverTlsConnections = Array("server", "tls", "connections") + + val server = doMkTlsServer("server", serverStats) + val client = doMkTlsClient(getPort(server)) + + tryWithResources(server, client.asClosable) { + assertGaugeIsZero(serverStats, serverTlsConnections) + mkSuccessfulHelloRequest(client) + assertGaugeIsOne(serverStats, serverTlsConnections) + + await(client.asClosable.close()) + eventually { assertGaugeIsZero(serverStats, serverTlsConnections) } + await(server.close()) + } + } + + test("Single client and server results in client TLS connections incremented and decremented") { + val clientStats = new InMemoryStatsReceiver + val clientTlsConnections = Array("client", "tls", "connections") + + val server = doMkTlsServer() + val client = doMkTlsClient(getPort(server), "client", clientStats) + + tryWithResources(server, client.asClosable) { + assertGaugeIsZero(clientStats, clientTlsConnections) + mkSuccessfulHelloRequest(client) + assertGaugeIsOne(clientStats, clientTlsConnections) + + await(client.asClosable.close()) + eventually { assertGaugeIsZero(clientStats, clientTlsConnections) } + await(server.close()) + } + + } + + test( + "Multiple clients and server results in server TLS connections incremented and decremented") { + val serverStats = new InMemoryStatsReceiver + + val server = doMkTlsServer("server", serverStats) + val serverPort = getPort(server) + val client1 = doMkTlsClient(serverPort, "client1") + val client2 = doMkTlsClient(serverPort, "client2") + + tryWithResources(server, client1.asClosable, client2.asClosable) { + val serverTlsConnections = Array("server", "tls", "connections") + + assertGaugeIsZero(serverStats, serverTlsConnections) + mkSuccessfulHelloRequest(client1) + assertGaugeIsOne(serverStats, serverTlsConnections) + + mkSuccessfulHelloRequest(client2) + assertGaugeIsTwo(serverStats, serverTlsConnections) + + await(client1.asClosable.close()) + eventually { assertGaugeIsOne(serverStats, serverTlsConnections) } + + await(client2.asClosable.close()) + await(server.close()) + eventually { assertGaugeIsZero(serverStats, serverTlsConnections) } + } + + } + + test( + "Multiple clients and server results in separate client TLS connections incremented and decremented" + ) { + val client1Stats = new InMemoryStatsReceiver + val client2Stats = new InMemoryStatsReceiver + + val server = doMkTlsServer("server") + val serverPort = getPort(server) + val client1 = doMkTlsClient(serverPort, "client1", client1Stats) + val client2 = doMkTlsClient(serverPort, "client2", client2Stats) + + tryWithResources(server, client1.asClosable, client2.asClosable) { + val client1TlsConnections = Array("client1", "tls", "connections") + val client2TlsConnections = Array("client2", "tls", "connections") + + assertGaugeIsZero(client1Stats, client1TlsConnections) + assertGaugeIsZero(client2Stats, client2TlsConnections) + + mkSuccessfulHelloRequest(client1) + assertGaugeIsOne(client1Stats, client1TlsConnections) + assertGaugeIsZero(client2Stats, client2TlsConnections) + + mkSuccessfulHelloRequest(client2) + assertGaugeIsOne(client1Stats, client1TlsConnections) + assertGaugeIsOne(client2Stats, client2TlsConnections) + + await(client1.asClosable.close()) + eventually { + assertGaugeIsZero(client1Stats, client1TlsConnections) + assertGaugeIsOne(client2Stats, client2TlsConnections) + } + + await(client2.asClosable.close()) + await(server.close()) + eventually { + assertGaugeIsZero(client1Stats, client1TlsConnections) + assertGaugeIsZero(client2Stats, client2TlsConnections) + } + } + } + + test("Single client and rejecting server results in both sides TLS connections at 0") { + val serverStats = new InMemoryStatsReceiver + val serverTlsConnections = Array("server", "tls", "connections") + + val clientStats = new InMemoryStatsReceiver + val clientTlsConnections = Array("client", "tls", "connections") + + val server = doMkTlsServer("server", serverStats, NeverValidServerSide) + val client = doMkTlsClient(getPort(server), "client", clientStats) + + tryWithResources(server, client.asClosable) { + assertGaugeIsZero(serverStats, serverTlsConnections) + assertGaugeIsZero(clientStats, clientTlsConnections) + + // If the server rejects the handshake, it just hangs up. Therefore, + // we expect to get a ChannelClosedException here. + intercept[ChannelClosedException] { + await(client.query("hello")) + } + + eventually { assertGaugeIsZero(serverStats, serverTlsConnections) } + eventually { assertGaugeIsZero(clientStats, clientTlsConnections) } + + await(client.asClosable.close()) + await(server.close()) + + eventually { + assertGaugeIsZero(serverStats, serverTlsConnections) + assertGaugeIsZero(clientStats, clientTlsConnections) + } + } + } + + test("Single rejecting client and server results in both sides TLS connections at 0") { + val serverStats = new InMemoryStatsReceiver + val serverTlsConnections = Array("server", "tls", "connections") + + val clientStats = new InMemoryStatsReceiver + val clientTlsConnections = Array("client", "tls", "connections") + + val server = doMkTlsServer("server", serverStats) + val client = doMkTlsClient(getPort(server), "client", clientStats, NeverValidClientSide) + + tryWithResources(server, client.asClosable) { + assertGaugeIsZero(serverStats, serverTlsConnections) + assertGaugeIsZero(clientStats, clientTlsConnections) + + val exn = intercept[Throwable] { + await(client.query("hello")) + } + + // We need to make sure we failed due to the ssl verification + def findSslVerificationFailedException(ex: Throwable): Unit = ex match { + case null => throw new Exception("Failed to find SslVerficationFailure", exn) + case _: SslVerificationFailedException => // ok + case ex => findSslVerificationFailedException(ex.getCause) + } + findSslVerificationFailedException(exn) + + eventually { + assertGaugeIsZero(serverStats, serverTlsConnections) + assertGaugeIsZero(clientStats, clientTlsConnections) + } + + await(client.asClosable.close()) + await(server.close()) + + eventually { + assertGaugeIsZero(serverStats, serverTlsConnections) + assertGaugeIsZero(clientStats, clientTlsConnections) + } + } + } +} diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/PriorEncryptionSslTest.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/PriorEncryptionSslTest.scala new file mode 100644 index 0000000000..9fd7c4bc81 --- /dev/null +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/PriorEncryptionSslTest.scala @@ -0,0 +1,26 @@ +package com.twitter.finagle.thriftmux.ssl + +import com.twitter.finagle.ListeningServer +import com.twitter.finagle.ssl.client.SslClientSessionVerifier +import com.twitter.finagle.ssl.server.SslServerSessionVerifier +import com.twitter.finagle.stats.StatsReceiver +import com.twitter.finagle.thriftmux.ssl.ThriftSmuxSslTestComponents.{mkTlsClient, mkTlsServer} +import com.twitter.finagle.thriftmux.thriftscala.TestService + +// Tests the case where both client and server want TLS but not opportunistically +class PriorEncryptionSslTest extends AbstractThriftSmuxSslTest { + protected def doMkTlsClient( + port: Int, + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslClientSessionVerifier + ): TestService.MethodPerEndpoint = + mkTlsClient(port, label, statsReceiver, sessionVerifier, None) + + protected def doMkTlsServer( + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslServerSessionVerifier + ): ListeningServer = + mkTlsServer(label, statsReceiver, sessionVerifier, false, None) +} diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/SnoopingSmuxSslTest.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/SnoopingSmuxSslTest.scala new file mode 100644 index 0000000000..df46589c5c --- /dev/null +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/SnoopingSmuxSslTest.scala @@ -0,0 +1,33 @@ +package com.twitter.finagle.thriftmux.ssl + +import com.twitter.finagle.ListeningServer +import com.twitter.finagle.ssl.OpportunisticTls +import com.twitter.finagle.ssl.client.SslClientSessionVerifier +import com.twitter.finagle.ssl.server.SslServerSessionVerifier +import com.twitter.finagle.stats.StatsReceiver +import com.twitter.finagle.thriftmux.ssl.ThriftSmuxSslTestComponents.{mkTlsClient, mkTlsServer} +import com.twitter.finagle.thriftmux.thriftscala.TestService + +class TlsRequiredSnoopingSmuxSslTest extends AbstractSnoopingSmuxSslTest(OpportunisticTls.Required) +class TlsDesiredSnoopingSmuxSslTest extends AbstractSnoopingSmuxSslTest(OpportunisticTls.Desired) + +// Tests for supporting a prior-knowledge TLS client against a snooping server. +abstract class AbstractSnoopingSmuxSslTest(level: OpportunisticTls.Level) + extends AbstractThriftSmuxSslTest { + + protected def doMkTlsClient( + port: Int, + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslClientSessionVerifier + ): TestService.MethodPerEndpoint = + mkTlsClient(port, label, statsReceiver, sessionVerifier, None) + + protected def doMkTlsServer( + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslServerSessionVerifier + ): ListeningServer = + mkTlsServer(label, statsReceiver, sessionVerifier, true, Some(level)) + +} diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTest.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTest.scala index 8e2e531078..02e064d198 100644 --- a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTest.scala +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTest.scala @@ -1,225 +1,26 @@ package com.twitter.finagle.thriftmux.ssl -import com.twitter.conversions.DurationOps._ -import com.twitter.finagle.{ChannelClosedException, SslVerificationFailedException} -import com.twitter.finagle.stats.InMemoryStatsReceiver -import com.twitter.finagle.thriftmux.ssl.ThriftSmuxSslTestComponents._ -import com.twitter.finagle.thriftmux.thriftscala._ -import com.twitter.util.{Await, Awaitable, Closable, Duration} -import org.scalatest.FunSuite -import org.scalatest.concurrent.Eventually - -class ThriftSmuxSslTest extends FunSuite with Eventually { - - // this method is a safety-net to ensure that resources always get cleaned up, even if - // an unexpected exception gets thrown - private[this] def tryWithResources[T](closables: Closable*)(fn: => T): T = - try { - fn - } finally { - await(Closable.all(closables: _*).close()) - } - - private[this] def assertGaugeIsZero( - statsReceiver: InMemoryStatsReceiver, - name: Array[String] - ): Unit = statsReceiver.gauges.get(name) match { - case Some(f) => assert(f() == 0.0) - case None => // all good - } - - private[this] def assertGaugeIsNonZero( - value: Float - )( - statsReceiver: InMemoryStatsReceiver, - name: Array[String] - ): Unit = statsReceiver.gauges.get(name) match { - case Some(f) => assert(f() == value) - case None => fail() - } - - def await[T](a: Awaitable[T], d: Duration = 2.seconds): T = - Await.result(a, d) - - private[this] val assertGaugeIsOne = assertGaugeIsNonZero(1.0f) _ - private[this] val assertGaugeIsTwo = assertGaugeIsNonZero(2.0f) _ - - private[this] def mkSuccessfulHelloRequest(client: TestService.MethodPerEndpoint): Unit = { - val response = await(client.query("hello")) - assert(response == "hellohello") - } - - test("Single client and server results in server TLS connections incremented and decremented") { - val serverStats = new InMemoryStatsReceiver - val serverTlsConnections = Array("server", "tls", "connections") - - val server = mkTlsServer("server", serverStats) - val client = mkTlsClient(getPort(server)) - - tryWithResources(server, client.asClosable) { - assertGaugeIsZero(serverStats, serverTlsConnections) - mkSuccessfulHelloRequest(client) - assertGaugeIsOne(serverStats, serverTlsConnections) - - await(client.asClosable.close()) - eventually { assertGaugeIsZero(serverStats, serverTlsConnections) } - await(server.close()) - } - - } - - test("Single client and server results in client TLS connections incremented and decremented") { - val clientStats = new InMemoryStatsReceiver - val clientTlsConnections = Array("client", "tls", "connections") - - val server = mkTlsServer() - val client = mkTlsClient(getPort(server), "client", clientStats) - - tryWithResources(server, client.asClosable) { - assertGaugeIsZero(clientStats, clientTlsConnections) - mkSuccessfulHelloRequest(client) - assertGaugeIsOne(clientStats, clientTlsConnections) - - await(client.asClosable.close()) - eventually { assertGaugeIsZero(clientStats, clientTlsConnections) } - await(server.close()) - } - - } - - test( - "Multiple clients and server results in server TLS connections incremented and decremented") { - val serverStats = new InMemoryStatsReceiver - - val server = mkTlsServer("server", serverStats) - val serverPort = getPort(server) - val client1 = mkTlsClient(serverPort, "client1") - val client2 = mkTlsClient(serverPort, "client2") - - tryWithResources(server, client1.asClosable, client2.asClosable) { - val serverTlsConnections = Array("server", "tls", "connections") - - assertGaugeIsZero(serverStats, serverTlsConnections) - mkSuccessfulHelloRequest(client1) - assertGaugeIsOne(serverStats, serverTlsConnections) - - mkSuccessfulHelloRequest(client2) - assertGaugeIsTwo(serverStats, serverTlsConnections) - - await(client1.asClosable.close()) - eventually { assertGaugeIsOne(serverStats, serverTlsConnections) } - - await(client2.asClosable.close()) - await(server.close()) - eventually { assertGaugeIsZero(serverStats, serverTlsConnections) } - } - - } - - test( - "Multiple clients and server results in separate client TLS connections incremented and decremented" - ) { - val client1Stats = new InMemoryStatsReceiver - val client2Stats = new InMemoryStatsReceiver - - val server = mkTlsServer("server") - val serverPort = getPort(server) - val client1 = mkTlsClient(serverPort, "client1", client1Stats) - val client2 = mkTlsClient(serverPort, "client2", client2Stats) - - tryWithResources(server, client1.asClosable, client2.asClosable) { - val client1TlsConnections = Array("client1", "tls", "connections") - val client2TlsConnections = Array("client2", "tls", "connections") - - assertGaugeIsZero(client1Stats, client1TlsConnections) - assertGaugeIsZero(client2Stats, client2TlsConnections) - - mkSuccessfulHelloRequest(client1) - assertGaugeIsOne(client1Stats, client1TlsConnections) - assertGaugeIsZero(client2Stats, client2TlsConnections) - - mkSuccessfulHelloRequest(client2) - assertGaugeIsOne(client1Stats, client1TlsConnections) - assertGaugeIsOne(client2Stats, client2TlsConnections) - - await(client1.asClosable.close()) - eventually { - assertGaugeIsZero(client1Stats, client1TlsConnections) - assertGaugeIsOne(client2Stats, client2TlsConnections) - } - - await(client2.asClosable.close()) - await(server.close()) - eventually { - assertGaugeIsZero(client1Stats, client1TlsConnections) - assertGaugeIsZero(client2Stats, client2TlsConnections) - } - } - } - - test("Single client and rejecting server results in both sides TLS connections at 0") { - val serverStats = new InMemoryStatsReceiver - val serverTlsConnections = Array("server", "tls", "connections") - - val clientStats = new InMemoryStatsReceiver - val clientTlsConnections = Array("client", "tls", "connections") - - val server = mkTlsServer("server", serverStats, NeverValidServerSide) - val client = mkTlsClient(getPort(server), "client", clientStats) - - tryWithResources(server, client.asClosable) { - assertGaugeIsZero(serverStats, serverTlsConnections) - assertGaugeIsZero(clientStats, clientTlsConnections) - - // If the server rejects the handshake, it just hangs up. Therefore, - // we expect to get a ChannelClosedException here. - intercept[ChannelClosedException] { - await(client.query("hello")) - } - - eventually { assertGaugeIsZero(serverStats, serverTlsConnections) } - eventually { assertGaugeIsZero(clientStats, clientTlsConnections) } - - await(client.asClosable.close()) - await(server.close()) - - eventually { - assertGaugeIsZero(serverStats, serverTlsConnections) - assertGaugeIsZero(clientStats, clientTlsConnections) - } - } - } - - test("Single rejecting client and server results in both sides TLS connections at 0") { - val serverStats = new InMemoryStatsReceiver - val serverTlsConnections = Array("server", "tls", "connections") - - val clientStats = new InMemoryStatsReceiver - val clientTlsConnections = Array("client", "tls", "connections") - - val server = mkTlsServer("server", serverStats) - val client = mkTlsClient(getPort(server), "client", clientStats, NeverValidClientSide) - - tryWithResources(server, client.asClosable) { - assertGaugeIsZero(serverStats, serverTlsConnections) - assertGaugeIsZero(clientStats, clientTlsConnections) - - intercept[SslVerificationFailedException] { - await(client.query("hello")) - } - - eventually { - assertGaugeIsZero(serverStats, serverTlsConnections) - assertGaugeIsZero(clientStats, clientTlsConnections) - } - - await(client.asClosable.close()) - await(server.close()) - - eventually { - assertGaugeIsZero(serverStats, serverTlsConnections) - assertGaugeIsZero(clientStats, clientTlsConnections) - } - } - } +import com.twitter.finagle.ListeningServer +import com.twitter.finagle.ssl.OpportunisticTls +import com.twitter.finagle.ssl.client.SslClientSessionVerifier +import com.twitter.finagle.ssl.server.SslServerSessionVerifier +import com.twitter.finagle.stats.StatsReceiver +import com.twitter.finagle.thriftmux.ssl.ThriftSmuxSslTestComponents.{mkTlsClient, mkTlsServer} +import com.twitter.finagle.thriftmux.thriftscala.TestService + +class ThriftSmuxSslTest extends AbstractThriftSmuxSslTest { + protected def doMkTlsClient( + port: Int, + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslClientSessionVerifier + ): TestService.MethodPerEndpoint = + mkTlsClient(port, label, statsReceiver, sessionVerifier, Some(OpportunisticTls.Required)) + + protected def doMkTlsServer( + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslServerSessionVerifier + ): ListeningServer = + mkTlsServer(label, statsReceiver, sessionVerifier, false, Some(OpportunisticTls.Required)) } diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTestComponents.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTestComponents.scala index ab9ddf107c..9f90526411 100644 --- a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTestComponents.scala +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/ThriftSmuxSslTestComponents.scala @@ -1,11 +1,16 @@ package com.twitter.finagle.thriftmux.ssl -import com.twitter.finagle.{Address, ListeningServer, ThriftMux} -import com.twitter.finagle.mux.transport.OpportunisticTls -import com.twitter.finagle.ssl.{ClientAuth, KeyCredentials, TrustCredentials} +import com.twitter.finagle.{Address, ListeningServer, Thrift, ThriftMux} +import com.twitter.finagle.ssl.{ + ClientAuth, + KeyCredentials, + OpportunisticTls, + SnoopingLevelInterpreter, + TrustCredentials +} import com.twitter.finagle.ssl.client.{SslClientConfiguration, SslClientSessionVerifier} import com.twitter.finagle.ssl.server.{SslServerConfiguration, SslServerSessionVerifier} -import com.twitter.finagle.stats.{NullStatsReceiver, StatsReceiver} +import com.twitter.finagle.stats.StatsReceiver import com.twitter.finagle.thriftmux.thriftscala._ import com.twitter.io.TempFile import com.twitter.util.Future @@ -45,32 +50,55 @@ object ThriftSmuxSslTestComponents { false } + val sslClientConfiguration = SslClientConfiguration( + keyCredentials = KeyCredentials.CertAndKey(clientCert, clientKey), + trustCredentials = TrustCredentials.CertCollection(chainCert) + ) + def getPort(server: ListeningServer): Int = server.boundAddress.asInstanceOf[InetSocketAddress].getPort - def mkTlsClient( + def mkTlsVanillaThriftClient( port: Int, - label: String = "client", - statsReceiver: StatsReceiver = NullStatsReceiver, - sessionVerifier: SslClientSessionVerifier = SslClientSessionVerifier.AlwaysValid + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslClientSessionVerifier ): TestService.MethodPerEndpoint = { - val clientConfig = SslClientConfiguration( - keyCredentials = KeyCredentials.CertAndKey(clientCert, clientKey), - trustCredentials = TrustCredentials.CertCollection(chainCert) - ) - - ThriftMux.client.withTransport - .tls(clientConfig, sessionVerifier) - .withOpportunisticTls(OpportunisticTls.Required) + Thrift.client.withTransport + .tls(sslClientConfiguration, sessionVerifier) .withStatsReceiver(statsReceiver) .withLabel(label) .build[TestService.MethodPerEndpoint]("localhost:" + port) } + def mkTlsClient( + port: Int, + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslClientSessionVerifier, + oppTlsLevel: Option[OpportunisticTls.Level] + ): TestService.MethodPerEndpoint = { + + var client = + ThriftMux.client.withTransport + .tls(sslClientConfiguration, sessionVerifier) + .withStatsReceiver(statsReceiver) + .withLabel(label) + + client = oppTlsLevel match { + case Some(level) => client.withOpportunisticTls(level) + case None => client.withNoOpportunisticTls + } + + client.build[TestService.MethodPerEndpoint]("localhost:" + port) + } + def mkTlsServer( - label: String = "server", - statsReceiver: StatsReceiver = NullStatsReceiver, - sessionVerifier: SslServerSessionVerifier = SslServerSessionVerifier.AlwaysValid + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslServerSessionVerifier, + snoopingEnabled: Boolean, + oppTlsLevel: Option[OpportunisticTls.Level] ): ListeningServer = { val serverConfig = SslServerConfiguration( keyCredentials = KeyCredentials.CertAndKey(serverCert, serverKey), @@ -78,11 +106,21 @@ object ThriftSmuxSslTestComponents { clientAuth = ClientAuth.Needed ) - ThriftMux.server.withTransport + val snoopingLevel = + if (snoopingEnabled) SnoopingLevelInterpreter.EnabledForNegotiatingProtocols + else SnoopingLevelInterpreter.Off + + var server = ThriftMux.server.withTransport .tls(serverConfig, sessionVerifier) - .withOpportunisticTls(OpportunisticTls.Required) + .configured(snoopingLevel) .withStatsReceiver(statsReceiver) .withLabel(label) - .serveIface("localhost:*", concatService) + + server = oppTlsLevel match { + case Some(level) => server.withOpportunisticTls(level) + case None => server.withNoOpportunisticTls + } + + server.serveIface("localhost:*", concatService) } } diff --git a/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/VanillaThriftSmuxSslTest.scala b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/VanillaThriftSmuxSslTest.scala new file mode 100644 index 0000000000..f1773cdb76 --- /dev/null +++ b/finagle-thriftmux/src/test/scala/com/twitter/finagle/thriftmux/ssl/VanillaThriftSmuxSslTest.scala @@ -0,0 +1,30 @@ +package com.twitter.finagle.thriftmux.ssl + +import com.twitter.finagle.ListeningServer +import com.twitter.finagle.ssl.OpportunisticTls +import com.twitter.finagle.ssl.client.SslClientSessionVerifier +import com.twitter.finagle.ssl.server.SslServerSessionVerifier +import com.twitter.finagle.stats.StatsReceiver +import com.twitter.finagle.thriftmux.ssl.ThriftSmuxSslTestComponents.{ + mkTlsServer, + mkTlsVanillaThriftClient +} +import com.twitter.finagle.thriftmux.thriftscala.TestService + +// Tests that we can use a vanilla thrift client to talk to a thriftmux server that can do snooping. +class VanillaThriftSmuxSslTest extends AbstractThriftSmuxSslTest { + protected def doMkTlsClient( + port: Int, + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslClientSessionVerifier + ): TestService.MethodPerEndpoint = + mkTlsVanillaThriftClient(port, label, statsReceiver, sessionVerifier) + + protected def doMkTlsServer( + label: String, + statsReceiver: StatsReceiver, + sessionVerifier: SslServerSessionVerifier + ): ListeningServer = + mkTlsServer(label, statsReceiver, sessionVerifier, true, Some(OpportunisticTls.Required)) +}