Skip to content

Conversation

@jaymode
Copy link
Member

@jaymode jaymode commented Sep 28, 2018

The SSLService invalidates SSLSessions when there is a change to any of
the underlying key or trust material. However, this invalidation code
did not check for a null SSLSession being returned from the context and
assumed that the context would always return a non-null object. The
return of a null object is possible in all versions, but JDK11 seems to
return them more often due to changes for TLS 1.3. There are a number
of reasons that we get a id of a session but the context returns null
when the session with that id is requested. Some of the reasons for
this are:

  • Session was evicted by session cache
  • Session has timed out
  • Session has been invalidated by another caller

To handle this, the SSLService now checks if the value is null before
calling invalidate on the SSLSession.

Closes #32124

The SSLService invalidates SSLSessions when there is a change to any of
the underlying key or trust material. However, this invalidation code
did not check for a null SSLSession being returned from the context and
assumed that the context would always return a non-null object. The
return of a null object is possible in all versions, but JDK11 seems to
return them more often due to changes for TLS 1.3. There are a number
of reasons that we get a id of a session but the context returns null
when the session with that id is requested. Some of the reasons for
this are:

* Session was evicted by session cache
* Session has timed out
* Session has been invalidated by another caller

To handle this, the SSLService now checks if the value is null before
calling invalidate on the SSLSession.

Closes elastic#32124
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

Copy link
Contributor

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for dealing with this.

Since this is fixed we can go ahead and unpin the HttpServer and HttpClient from TLS1.2 ( i.e. revert 214652d and d9f5e4f ) in SSLConfigurationRelaoderTests

@jaymode jaymode merged commit 14d841e into elastic:master Sep 28, 2018
@jaymode jaymode deleted the fix_jdk11_ssl_session_invalidate branch September 28, 2018 15:03
jaymode added a commit that referenced this pull request Sep 28, 2018
The SSLService invalidates SSLSessions when there is a change to any of
the underlying key or trust material. However, this invalidation code
did not check for a null SSLSession being returned from the context and
assumed that the context would always return a non-null object. The
return of a null object is possible in all versions, but JDK11 seems to
return them more often due to changes for TLS 1.3. There are a number
of reasons that we get a id of a session but the context returns null
when the session with that id is requested. Some of the reasons for
this are:

* Session was evicted by session cache
* Session has timed out
* Session has been invalidated by another caller

To handle this, the SSLService now checks if the value is null before
calling invalidate on the SSLSession.

Closes #32124
jaymode added a commit to jaymode/elasticsearch that referenced this pull request Sep 28, 2018
Revert "[TESTS] Pin MockWebServer to TLS1.2 (elastic#33127)" (commit
214652d) and "Pin TLS1.2 in
SSLConfigurationReloaderTests" (commit
d9f5e4f), which pinned the
MockWebServer used in the SSLConfigurationReloaderTests to TLSv1.2 in
order to prevent failures with JDK 11 related to ssl session
invalidation. We no longer need this pinning as the problematic code
was fixed in elastic#34130.
jaymode added a commit that referenced this pull request Oct 2, 2018
Revert "[TESTS] Pin MockWebServer to TLS1.2 (#33127)" (commit
214652d) and "Pin TLS1.2 in
SSLConfigurationReloaderTests" (commit
d9f5e4f), which pinned the
MockWebServer used in the SSLConfigurationReloaderTests to TLSv1.2 in
order to prevent failures with JDK 11 related to ssl session
invalidation. We no longer need this pinning as the problematic code
was fixed in #34130.
kcm pushed a commit that referenced this pull request Oct 30, 2018
The SSLService invalidates SSLSessions when there is a change to any of
the underlying key or trust material. However, this invalidation code
did not check for a null SSLSession being returned from the context and
assumed that the context would always return a non-null object. The
return of a null object is possible in all versions, but JDK11 seems to
return them more often due to changes for TLS 1.3. There are a number
of reasons that we get a id of a session but the context returns null
when the session with that id is requested. Some of the reasons for
this are:

* Session was evicted by session cache
* Session has timed out
* Session has been invalidated by another caller

To handle this, the SSLService now checks if the value is null before
calling invalidate on the SSLSession.

Closes #32124
kcm pushed a commit that referenced this pull request Oct 30, 2018
Revert "[TESTS] Pin MockWebServer to TLS1.2 (#33127)" (commit
214652d) and "Pin TLS1.2 in
SSLConfigurationReloaderTests" (commit
d9f5e4f), which pinned the
MockWebServer used in the SSLConfigurationReloaderTests to TLSv1.2 in
order to prevent failures with JDK 11 related to ssl session
invalidation. We no longer need this pinning as the problematic code
was fixed in #34130.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants