<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><tt>Given some recent changes to the class involved in this patch,
        in the jdk repo (<a class="moz-txt-link-freetext" href="http://hg.openjdk.java.net/jdk/jdk">http://hg.openjdk.java.net/jdk/jdk</a>), 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).</tt></p>
    <p><tt>-Jaikiran</tt><br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 06/12/17 9:35 PM, Jaikiran Pai
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CANkmnM53MV2T0-Mq4hfVFa0EjUiPb7nk+pp9MYfXBTnk7gWYgg@mail.gmail.com">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" moz-do-not-send="true">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" moz-do-not-send="true">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" moz-do-not-send="true">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" moz-do-not-send="true">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" moz-do-not-send="true">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>
    </blockquote>
    <br>
  </body>
</html>