[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
>>
>>
>>
>