Hi Jaikiran,
Thanks a lot for the update. Your code looks fine to me.
As we are working on the re-org of the implementation[1] now, I may
integrate your contribution shortly after the re-org changes.
Thanks,
Xuelei
[1]:
http://mail.openjdk.java.net/pipermail/security-dev/2018-February/016815.html
On 2/22/2018 9:58 PM, Jaikiran Pai wrote:
Given some recent changes to the class involved in this patch, in the
jdk repo (http://hg.openjdk.java.net/jdk/jdk), I noticed some merge
conflicts to this patch today. So I've now attached an updated patch
which resolves those merge issues. This has been tested with latest
jtreg (tip).
-Jaikiran
On 06/12/17 9:35 PM, Jaikiran Pai wrote:
Thank you Xuelei. Please take your time.
-Jaikiran
On Wednesday, December 6, 2017, Xuelei Fan <xuelei....@oracle.com
<mailto: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
<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
<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
<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
<https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/samples/sslengine/SSLEngineSimpleDemo.java>
-Jaikiran
