Skip to content

Commit

Permalink
finagle-core: Add SSL/TLS Session Information to ClientConnection
Browse files Browse the repository at this point in the history
Problem

We would like to be able to export and monitor more information about
whether particular connections are using SSL/TLS, and if so, the
specifics about that connection. For servers, we track connections
based on information contained within the `ClientConnection` class.

Solution

Add `SslSessionInfo` to `ClientConnection` so that server connections
can track whether connections are using SSL/TLS and the specifics
related to a secure connection.

Differential Revision: https://phabricator.twitter.biz/D323305
  • Loading branch information
ryanoneill authored and jenkins committed Jun 5, 2019
1 parent 03d1ad8 commit 69a28c1
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ Unreleased
New Features
~~~~~~~~~~~~

* finagle-core: SSL/TLS session information has been added to `c.t.f.ClientConnection`.
``PHAB_ID=D323305``

* finagle-http: Added counters for request/response stream as: `stream/request/closed`,
`stream/request/failures`, `stream/request/failures/<exception_name>`, `stream/request/opened`,
`stream/request/pending` and `stream/response/closed`, `stream/response/failures`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.twitter.finagle

import com.twitter.finagle.ssl.session.{NullSslSessionInfo, SslSessionInfo}
import com.twitter.finagle.util.InetSocketAddressUtil.unconnected
import com.twitter.util.{Closable, Future, Time}
import java.net.SocketAddress
Expand Down Expand Up @@ -27,6 +28,11 @@ trait ClientConnection extends Closable {
* Expose a `Future` that is satisfied when the connection is closed.
*/
def onClose: Future[Unit]

/**
* SSL/TLS information associated with a client connection.
*/
def sslSessionInfo: SslSessionInfo
}

object ClientConnection {
Expand All @@ -43,5 +49,6 @@ object ClientConnection {
def localAddress: SocketAddress = unconnected
def close(deadline: Time): Future[Unit] = Future.Done
def onClose: Future[Unit] = Future.never
def sslSessionInfo: SslSessionInfo = NullSslSessionInfo
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.twitter.finagle

import com.twitter.finagle.ssl.session.SslSessionInfo
import com.twitter.util.{Closable, Future, Time}
import java.net.SocketAddress

Expand Down Expand Up @@ -51,4 +52,5 @@ private class ClientConnectionProxy(underlying: ClientConnection) extends Client
def remoteAddress: SocketAddress = underlying.remoteAddress
def localAddress: SocketAddress = underlying.localAddress
def onClose: Future[Unit] = underlying.onClose
def sslSessionInfo: SslSessionInfo = underlying.sslSessionInfo
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.twitter.finagle.{
ServiceFactory
}
import com.twitter.finagle.context.Contexts
import com.twitter.finagle.ssl.session.SslSessionInfo
import com.twitter.finagle.transport.{Transport, TransportContext}
import com.twitter.util.{Closable, Future, Return, Throw, Time}
import java.net.SocketAddress
Expand Down Expand Up @@ -133,5 +134,6 @@ trait StdStackServer[Req, Rep, This <: StdStackServer[Req, Rep, This]]
// they both will complete the transports `onClose` future.
override val onClose: Future[Unit] = t.onClose.unit
override def close(deadline: Time): Future[Unit] = t.close(deadline)
override def sslSessionInfo: SslSessionInfo = t.context.sslSessionInfo
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import javax.net.ssl.SSLSession
* Null object which indicates that the existing connection
* is not using SSL/TLS.
*/
private[finagle] object NullSslSessionInfo extends SslSessionInfo {
object NullSslSessionInfo extends SslSessionInfo {

/**
* Indicates whether the connection is using SSL/TLS.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.twitter.finagle

import com.twitter.finagle.ssl.session.SslSessionInfo
import org.mockito.Mockito.when
import org.scalatest.FunSuite
import org.scalatest.mockito.MockitoSugar

class ClientConnectionProxyTest extends FunSuite with MockitoSugar {

test("ClientConnectionProxy uses underlying SSL/TLS session info") {
val sslSessionInfo: SslSessionInfo = mock[SslSessionInfo]
val underlying: ClientConnection = mock[ClientConnection]
when(underlying.sslSessionInfo).thenReturn(sslSessionInfo)
val conn: ClientConnection = new ClientConnectionProxy(underlying)
assert(conn.sslSessionInfo == sslSessionInfo)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.twitter.finagle

import com.twitter.finagle.ssl.session.NullSslSessionInfo
import org.scalatest.FunSuite

class ClientConnectionTest extends FunSuite {

test("ClientConnection.nil has NullSslSessionInfo") {
val conn: ClientConnection = ClientConnection.nil
assert(conn.sslSessionInfo == NullSslSessionInfo)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.twitter.finagle.filter.{RequestSemaphoreFilter, ServerAdmissionContro
import com.twitter.finagle.param.{Stats, Timer}
import com.twitter.finagle.server.utils.StringServer
import com.twitter.finagle.service.{ExpiringService, TimeoutFilter}
import com.twitter.finagle.ssl.session.{NullSslSessionInfo, SslSessionInfo}
import com.twitter.finagle.stack.Endpoint
import com.twitter.finagle.stats.InMemoryStatsReceiver
import com.twitter.finagle.util.StackRegistry
Expand Down Expand Up @@ -92,6 +93,7 @@ class StackServerTest extends FunSuite with Eventually {
Future.Done
}
def onClose: Future[Unit] = closed
def sslSessionInfo: SslSessionInfo = NullSslSessionInfo
}

val svc = Await.result(factory(conn), 5.seconds)
Expand Down

0 comments on commit 69a28c1

Please sign in to comment.