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




Reply via email to