Skip to content

Commit

Permalink
finagle-core: UsingSslSessionInfo Fails When Local Certificates are Null
Browse files Browse the repository at this point in the history
Problem / Solution

`UsingSslSessionInfo` takes in an `SSLSession` and calls the session's
`getLocalCertificates` method to retrieve the collection of local
certificates sent as part of this `SSLSession`. However, that method
can return 'null', and when it does, construction of `UsingSslSessionInfo`
fails. Add a guard which checks for 'null'.

JIRA Issues: CSL-8262

Differential Revision: https://phabricator.twitter.biz/D324499
  • Loading branch information
ryanoneill authored and jenkins committed Jun 7, 2019
1 parent a799e22 commit 8d98496
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ Runtime Behavior Changes
Bug Fixes
~~~~~~~~~

* finagle-core: `UsingSslSessionInfo` would fail to be constructed properly when
`SSLSession.getLocalCertificates` returns 'null'. ``PHAB_ID=D324499``

* finagle-http: Finagle now properly sets the `Transport.peerCertificate` local context
when using HTTP/2. ``PHAB_ID=D324392``

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ private[finagle] final class UsingSslSessionInfo(val session: SSLSession) extend
*
* @return The sequence of local certificates sent.
*/
val localCertificates: Seq[X509Certificate] =
session.getLocalCertificates.toSeq.collect { case cert: X509Certificate => cert }
val localCertificates: Seq[X509Certificate] = {
val localCerts = session.getLocalCertificates
if (localCerts == null) Nil
else localCerts.toSeq.collect { case cert: X509Certificate => cert }
}

/**
* The `X509Certificate`s that were received from the peer during the SSL/TLS handshake.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,17 @@ class UsingSslSessionInfoTest extends FunSuite with MockitoSugar {
assert(sessionInfo.localCertificates.head.getSerialNumber == localSerialNumber)
}

test("UsingSslSessionInfo works when local certificates are null") {
val nullLocalSession: SSLSession = mock[SSLSession]
when(nullLocalSession.getId).thenReturn(sessionID)
when(nullLocalSession.getLocalCertificates).thenReturn(null)
when(nullLocalSession.getPeerCertificates).thenReturn(Array[Certificate](peerCert))
when(nullLocalSession.getCipherSuite).thenReturn("my-made-up-cipher")

val sessionInfo: SslSessionInfo = new UsingSslSessionInfo(nullLocalSession)
assert(sessionInfo.localCertificates.length == 0)
}

test("UsingSslSessionInfo returns a peer X509Certificate") {
assert(sessionInfo.peerCertificates.length == 1)
assert(sessionInfo.peerCertificates.head.getSerialNumber == peerSerialNumber)
Expand Down

0 comments on commit 8d98496

Please sign in to comment.