[PATCH] JDK-8190917 : SSL session resumption, through handshake, in SSLEngine is broken for any protocols lesser than TLSv1.2

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] JDK-8190917 : SSL session resumption, through handshake, in SSLEngine is broken for any protocols lesser than TLSv1.2

Jaikiran Pai
As noted in [1], there's a regression in Java 9, where SSL session
resumption no longer works for SSL protocols other than TLSv1.2. The
code which is responsible for session resumption resides in the
ServerHandshaker[2], in the clientHello method. This method, in its
logic to decide whether or not to resume a (previously) cached session,
checks if the client hello message has a session id associated. If it
does, it then fetches a session from the cache. If it does find a
previously cached session for that id, it then goes ahead to check if
the SSL protocol associated with the cached session matches with the
protocol version that's "applicable/selected" of the incoming client
hello message. However, a relatively recent change[3] has, IMO,
unintentionally caused the protocol version check logic to fail for
protocols other than TLSv1.2. The protocol version check looks like:


// cannot resume session with different

if (oldVersion != protocolVersion) {
resumingSession = false;
}

The `protocolVersion` variable in this case happens to be a _default
initialized value_ (TLSv1.2) instead of the one that's "selected" based
on the incoming client hello message. As a result the `protocolVersion`
value is always TLSv1.2 and thus for any other protocols, this
comparison fails, even when the client hello message uses the right
session id and the right protocol that matches the previous session.

The attached patch, includes a potential fix to this issue. The change
makes sure that the protocol selection, based on the client hello
message, is done before this session resumption check and also makes
sure it uses that "selected" protocol in the version comparison check
instead of the class member `protocolVersion` (which gets set when the
server hello message is finally being sent).

The attached patch also contains a (jtreg) test case which reproduces
this bug and verifies this fix. The test case tests the session
resumption against TLSv1, TLSv1.1 and TLSv1.2. The test code itself is a
minor modification of the SSL demo that's available here [4].

This is my first OpenJDK patch and my OCA got approved a few days back.
Could someone please help with the review of this patch?

P.S: I noticed that the JIRA got assigned a few days back. I am not sure
if that means the fix is already being worked upon by someone else from
the team. If that's the case, then let me know and I'll let it be
handled by them.

[1] https://bugs.openjdk.java.net/browse/JDK-8190917

[2]
http://hg.openjdk.java.net/jdk/jdk/file/b7ae1437111b/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java

[3] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/42268eb6e04e

[4]
https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java

-Jaikiran




jdk-8190917.patch (23K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] JDK-8190917 : SSL session resumption, through handshake, in SSLEngine is broken for any protocols lesser than TLSv1.2

Jaikiran Pai
Anyone willing to sponsor/review the patch please?

-Jaikiran


On 24/11/17 10:52 AM, Jaikiran Pai wrote:

> As noted in [1], there's a regression in Java 9, where SSL session
> resumption no longer works for SSL protocols other than TLSv1.2. The
> code which is responsible for session resumption resides in the
> ServerHandshaker[2], in the clientHello method. This method, in its
> logic to decide whether or not to resume a (previously) cached
> session, checks if the client hello message has a session id
> associated. If it does, it then fetches a session from the cache. If
> it does find a previously cached session for that id, it then goes
> ahead to check if the SSL protocol associated with the cached session
> matches with the protocol version that's "applicable/selected" of the
> incoming client hello message. However, a relatively recent change[3]
> has, IMO, unintentionally caused the protocol version check logic to
> fail for protocols other than TLSv1.2. The protocol version check
> looks like:
>
>
> // cannot resume session with different
>
> if (oldVersion != protocolVersion) {
> resumingSession = false;
> }
>
> The `protocolVersion` variable in this case happens to be a _default
> initialized value_ (TLSv1.2) instead of the one that's "selected"
> based on the incoming client hello message. As a result the
> `protocolVersion` value is always TLSv1.2 and thus for any other
> protocols, this comparison fails, even when the client hello message
> uses the right session id and the right protocol that matches the
> previous session.
>
> The attached patch, includes a potential fix to this issue. The change
> makes sure that the protocol selection, based on the client hello
> message, is done before this session resumption check and also makes
> sure it uses that "selected" protocol in the version comparison check
> instead of the class member `protocolVersion` (which gets set when the
> server hello message is finally being sent).
>
> The attached patch also contains a (jtreg) test case which reproduces
> this bug and verifies this fix. The test case tests the session
> resumption against TLSv1, TLSv1.1 and TLSv1.2. The test code itself is
> a minor modification of the SSL demo that's available here [4].
>
> This is my first OpenJDK patch and my OCA got approved a few days
> back. Could someone please help with the review of this patch?
>
> P.S: I noticed that the JIRA got assigned a few days back. I am not
> sure if that means the fix is already being worked upon by someone
> else from the team. If that's the case, then let me know and I'll let
> it be handled by them.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8190917
>
> [2]
> http://hg.openjdk.java.net/jdk/jdk/file/b7ae1437111b/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java
>
> [3] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/42268eb6e04e
>
> [4]
> https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java
>
> -Jaikiran
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] JDK-8190917 : SSL session resumption, through handshake, in SSLEngine is broken for any protocols lesser than TLSv1.2

Jaikiran Pai
Last attempt - if there's no interest in this patch I'll stop pestering.

-Jaikiran


On 30/11/17 7:34 AM, Jaikiran Pai wrote:

> Anyone willing to sponsor/review the patch please?
>
> -Jaikiran
>
>
> On 24/11/17 10:52 AM, Jaikiran Pai wrote:
>> As noted in [1], there's a regression in Java 9, where SSL session
>> resumption no longer works for SSL protocols other than TLSv1.2. The
>> code which is responsible for session resumption resides in the
>> ServerHandshaker[2], in the clientHello method. This method, in its
>> logic to decide whether or not to resume a (previously) cached
>> session, checks if the client hello message has a session id
>> associated. If it does, it then fetches a session from the cache. If
>> it does find a previously cached session for that id, it then goes
>> ahead to check if the SSL protocol associated with the cached session
>> matches with the protocol version that's "applicable/selected" of the
>> incoming client hello message. However, a relatively recent change[3]
>> has, IMO, unintentionally caused the protocol version check logic to
>> fail for protocols other than TLSv1.2. The protocol version check
>> looks like:
>>
>>
>> // cannot resume session with different
>>
>> if (oldVersion != protocolVersion) {
>> resumingSession = false;
>> }
>>
>> The `protocolVersion` variable in this case happens to be a _default
>> initialized value_ (TLSv1.2) instead of the one that's "selected"
>> based on the incoming client hello message. As a result the
>> `protocolVersion` value is always TLSv1.2 and thus for any other
>> protocols, this comparison fails, even when the client hello message
>> uses the right session id and the right protocol that matches the
>> previous session.
>>
>> The attached patch, includes a potential fix to this issue. The
>> change makes sure that the protocol selection, based on the client
>> hello message, is done before this session resumption check and also
>> makes sure it uses that "selected" protocol in the version comparison
>> check instead of the class member `protocolVersion` (which gets set
>> when the server hello message is finally being sent).
>>
>> The attached patch also contains a (jtreg) test case which reproduces
>> this bug and verifies this fix. The test case tests the session
>> resumption against TLSv1, TLSv1.1 and TLSv1.2. The test code itself
>> is a minor modification of the SSL demo that's available here [4].
>>
>> This is my first OpenJDK patch and my OCA got approved a few days
>> back. Could someone please help with the review of this patch?
>>
>> P.S: I noticed that the JIRA got assigned a few days back. I am not
>> sure if that means the fix is already being worked upon by someone
>> else from the team. If that's the case, then let me know and I'll let
>> it be handled by them.
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8190917
>>
>> [2]
>> http://hg.openjdk.java.net/jdk/jdk/file/b7ae1437111b/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java
>>
>> [3] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/42268eb6e04e
>>
>> [4]
>> https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java
>>
>> -Jaikiran
>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] JDK-8190917 : SSL session resumption, through handshake, in SSLEngine is broken for any protocols lesser than TLSv1.2

Xuelei Fan-2
In reply to this post by Jaikiran Pai
Hi Jaikiran,

I will sponsor this contribution.  I need more time for the review and
testing.

Thanks,
Xuelei

On 11/23/2017 9:22 PM, Jaikiran Pai wrote:

> As noted in [1], there's a regression in Java 9, where SSL session
> resumption no longer works for SSL protocols other than TLSv1.2. The
> code which is responsible for session resumption resides in the
> ServerHandshaker[2], in the clientHello method. This method, in its
> logic to decide whether or not to resume a (previously) cached session,
> checks if the client hello message has a session id associated. If it
> does, it then fetches a session from the cache. If it does find a
> previously cached session for that id, it then goes ahead to check if
> the SSL protocol associated with the cached session matches with the
> protocol version that's "applicable/selected" of the incoming client
> hello message. However, a relatively recent change[3] has, IMO,
> unintentionally caused the protocol version check logic to fail for
> protocols other than TLSv1.2. The protocol version check looks like:
>
>
> // cannot resume session with different
>
> if (oldVersion != protocolVersion) {
> resumingSession = false;
> }
>
> The `protocolVersion` variable in this case happens to be a _default
> initialized value_ (TLSv1.2) instead of the one that's "selected" based
> on the incoming client hello message. As a result the `protocolVersion`
> value is always TLSv1.2 and thus for any other protocols, this
> comparison fails, even when the client hello message uses the right
> session id and the right protocol that matches the previous session.
>
> The attached patch, includes a potential fix to this issue. The change
> makes sure that the protocol selection, based on the client hello
> message, is done before this session resumption check and also makes
> sure it uses that "selected" protocol in the version comparison check
> instead of the class member `protocolVersion` (which gets set when the
> server hello message is finally being sent).
>
> The attached patch also contains a (jtreg) test case which reproduces
> this bug and verifies this fix. The test case tests the session
> resumption against TLSv1, TLSv1.1 and TLSv1.2. The test code itself is a
> minor modification of the SSL demo that's available here [4].
>
> This is my first OpenJDK patch and my OCA got approved a few days back.
> Could someone please help with the review of this patch?
>
> P.S: I noticed that the JIRA got assigned a few days back. I am not sure
> if that means the fix is already being worked upon by someone else from
> the team. If that's the case, then let me know and I'll let it be
> handled by them.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8190917
>
> [2]
> http://hg.openjdk.java.net/jdk/jdk/file/b7ae1437111b/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java 
>
>
> [3] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/42268eb6e04e
>
> [4]
> https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java 
>
>
> -Jaikiran
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] JDK-8190917 : SSL session resumption, through handshake, in SSLEngine is broken for any protocols lesser than TLSv1.2

Jaikiran Pai
Thank you Xuelei. Please take your time.

-Jaikiran



On Wednesday, December 6, 2017, Xuelei Fan <[hidden email]> wrote:
Hi Jaikiran,

I will sponsor this contribution.  I need more time for the review and testing.

Thanks,
Xuelei

On 11/23/2017 9:22 PM, Jaikiran Pai wrote:
As noted in [1], there's a regression in Java 9, where SSL session resumption no longer works for SSL protocols other than TLSv1.2. The code which is responsible for session resumption resides in the ServerHandshaker[2], in the clientHello method. This method, in its logic to decide whether or not to resume a (previously) cached session, checks if the client hello message has a session id associated. If it does, it then fetches a session from the cache. If it does find a previously cached session for that id, it then goes ahead to check if the SSL protocol associated with the cached session matches with the protocol version that's "applicable/selected" of the incoming client hello message. However, a relatively recent change[3] has, IMO, unintentionally caused the protocol version check logic to fail for protocols other than TLSv1.2. The protocol version check looks like:


// cannot resume session with different

if (oldVersion != protocolVersion) {
resumingSession = false;
}

The `protocolVersion` variable in this case happens to be a _default initialized value_ (TLSv1.2) instead of the one that's "selected" based on the incoming client hello message. As a result the `protocolVersion` value is always TLSv1.2 and thus for any other protocols, this comparison fails, even when the client hello message uses the right session id and the right protocol that matches the previous session.

The attached patch, includes a potential fix to this issue. The change makes sure that the protocol selection, based on the client hello message, is done before this session resumption check and also makes sure it uses that "selected" protocol in the version comparison check instead of the class member `protocolVersion` (which gets set when the server hello message is finally being sent).

The attached patch also contains a (jtreg) test case which reproduces this bug and verifies this fix. The test case tests the session resumption against TLSv1, TLSv1.1 and TLSv1.2. The test code itself is a minor modification of the SSL demo that's available here [4].

This is my first OpenJDK patch and my OCA got approved a few days back. Could someone please help with the review of this patch?

P.S: I noticed that the JIRA got assigned a few days back. I am not sure if that means the fix is already being worked upon by someone else from the team. If that's the case, then let me know and I'll let it be handled by them.

[1] https://bugs.openjdk.java.net/browse/JDK-8190917

[2] http://hg.openjdk.java.net/jdk/jdk/file/b7ae1437111b/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java

[3] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/42268eb6e04e

[4] https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java

-Jaikiran