Thank you Xuelei. Please take your time.<div><br></div><div>-Jaikiran<br><div><br></div><div><br><br>On Wednesday, December 6, 2017, Xuelei Fan <<a href="mailto:xuelei.fan@oracle.com">xuelei.fan@oracle.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Jaikiran,<br>
<br>
I will sponsor this contribution.  I need more time for the review and testing.<br>
<br>
Thanks,<br>
Xuelei<br>
<br>
On 11/23/2017 9:22 PM, Jaikiran Pai wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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:<br>
<br>
<br>
// cannot resume session with different<br>
<br>
if (oldVersion != protocolVersion) {<br>
resumingSession = false;<br>
}<br>
<br>
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.<br>
<br>
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).<br>
<br>
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].<br>
<br>
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?<br>
<br>
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.<br>
<br>
[1] <a href="https://bugs.openjdk.java.net/browse/JDK-8190917" target="_blank">https://bugs.openjdk.java.net/<wbr>browse/JDK-8190917</a><br>
<br>
[2] <a href="http://hg.openjdk.java.net/jdk/jdk/file/b7ae1437111b/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java" target="_blank">http://hg.openjdk.java.net/jdk<wbr>/jdk/file/b7ae1437111b/src/jav<wbr>a.base/share/classes/sun/secur<wbr>ity/ssl/ServerHandshaker.java</a> <br>
<br>
[3] <a href="http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/42268eb6e04e" target="_blank">http://hg.openjdk.java.net/jdk<wbr>9/jdk9/jdk/rev/42268eb6e04e</a><br>
<br>
[4] <a href="https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java" target="_blank">https://docs.oracle.com/javase<wbr>/8/docs/technotes/guides/<wbr>security/jsse/samples/sslengin<wbr>e/SSLEngineSimpleDemo.java</a> <br>
<br>
-Jaikiran<br>
<br>
<br>
<br>
</blockquote>
</blockquote></div></div>