<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <p>Thanks for taking this on Valerie. It's a shame to see such
      confusion arise from the new PKCS11 spec. <br>
      <br>
      The changes look fine to me. It'll certainly benefit from
      widespread interoperability testing.<br>
    </p>
    <pre class="moz-signature" cols="72">Regards,
Sean.</pre>
    <div class="moz-cite-prefix">On 16/08/19 00:49, Valerie Peng wrote:<br>
    </div>
    <blockquote
      cite="mid:608300e7-cc6e-6f73-ba93-eae8c7904340@oracle.com"
      type="cite">
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      <p>Anyone has time to help review this fix? PKCS#11 v2.40 has
        inconsistent definition for CK_GCM_PARAMS struct. The mechanism
        spec (sec 2..12.3) has: <br>
      </p>
      <blockquote>
        <p><tt>typedef struct CK_GCM_PARAMS {</tt><tt><br>
          </tt><tt>    CK_BYTE_PTR       pIv;</tt><tt><br>
          </tt><tt>    CK_ULONG          ulIvLen;</tt><tt><br>
          </tt><tt>    CK_BYTE_PTR       pAAD;</tt><tt><br>
          </tt><tt>    CK_ULONG          ulAADLen;</tt><tt><br>
          </tt><tt>    CK_ULONG          ulTagBits;</tt><tt><br>
          </tt><tt>} CK_GCM_PARAMS;</tt><br>
        </p>
      </blockquote>
      <p>However, the header file pkcs11t.h has an extra "ulIvBits"
        field: <br>
      </p>
      <blockquote>
        <p><tt>typedef struct CK_GCM_PARAMS {</tt><br>
          <tt>    CK_BYTE_PTR       pIv;</tt><br>
          <tt>    CK_ULONG          ulIvLen;</tt><br>
          <b><font color="#000099"><tt>    CK_ULONG          ulIvBits;</tt></font></b><br>
          <tt>    CK_BYTE_PTR       pAAD;</tt><br>
          <tt>    CK_ULONG          ulAADLen;</tt><br>
          <tt>    CK_ULONG          ulTagBits;</tt><br>
          <tt>} CK_GCM_PARAMS;</tt><br>
        </p>
      </blockquote>
      <p>Some vendors such OpenHSM2 and Solaris go with the header file
        while some vendor such as NSS go with the mech spec. Current
        SunPKCS11 provider impl works with NSS but not with other
        vendors whose CK_GCM_PARAMS struct contains ulIvBits field.
        Based on testing results, OpenHSM2 and Solaris error out at init
        time when the parameter length does not have their expected
        value. Thus, one way to workaround this inconsistency is to
        re-try with a different structure for AES GCM when the init
        failed. In addition, given the parameters contains Iv and AAD
        which some vendor may use during subsequent enc/dec operations,
        I changed to use the same model as RSASSA-PSS to defer the
        freeing after the enc/dec operation is finished. Verified the
        changes on Solaris 11.4 against existing GCM regression tests.<br>
      </p>
      <p>Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="https://bugs.openjdk.java.net/browse/JDK-8229243">https://bugs.openjdk.java.net/browse/JDK-8229243</a></p>
      <p>Webrev: <a moz-do-not-send="true"
          class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Evaleriep/8229243/webrev.00/">http://cr.openjdk.java.net/~valeriep/8229243/webrev.00/</a></p>
      Thanks,<br>
      Valerie<br>
    </blockquote>
    <br>
  </body>
</html>