Thank you Xuelei. Please take your time.
-Jaikiran
On Wednesday, December 6, 2017, Xuelei Fan <xuelei....@oracle.com> 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/jav
>> a.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
>>
>>
>>
>>
