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
