<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p>Max,</p>
    <p>Good catch on the SunRsaSign provider bug.</p>
    <p>Looking at the changes, I think we may have to fine-grain the
      check on the ensureInit() call, i.e.</p>
    <p>use ensureInit(boolean sign) instead of ensureInit(), as the
      current method only ensures that at least one of the privKey,
      pubKey or fallbackSignature is non-null, I think it should check
      the right one is non-null, i.e. sign -> privKey, verify ->
      pubKey/fallbackSignature.</p>
    <p>In the PSS class engineInitVerify(...) method if the specified
      key is a MSCAPI public key, then fallbackSignature is set to null
      and the native <span class="new">verifyPssSignedHash method is
        used, right?</span></p>
    <span class="new">Thanks,</span><br>
    <span class="new"></span><br>
    <span class="new">Valerie</span><br>
    <br>
    <div class="moz-cite-prefix">On 6/21/2018 10:39 PM, Weijun Wang
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:6C5264A2-3651-4832-AA96-E6B9838CCC67@oracle.com">
      <pre wrap="">Webrev updated at

  <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8205445/webrev.01">http://cr.openjdk.java.net/~weijun/8205445/webrev.01</a>

I think I found a bug in SunRsaSign of the RSASSA-PSS signature. Fixed and added a test.

BTW, I commented out the debug code in security.cpp. Once there is a bug I can use it.

Thanks
Max

</pre>
      <blockquote type="cite">
        <pre wrap="">On Jun 21, 2018, at 11:23 PM, Weijun Wang <a class="moz-txt-link-rfc2396E" href="mailto:weijun.wang@oracle.com"><weijun.wang@oracle.com></a> wrote:



</pre>
        <blockquote type="cite">
          <pre wrap="">On Jun 21, 2018, at 11:07 PM, Xuelei Fan <a class="moz-txt-link-rfc2396E" href="mailto:xuelei.fan@oracle.com"><xuelei.fan@oracle.com></a> wrote:

Hi Weijun,

The release note and the following notes look reasonable to me.

For the implementation part, could it be a little bit more straightforward if wrapping the new attributes (pss/pssParams/fallbackSignature) and codes (if pss/fallbackSignature, etc) in the PSS subclass?
</pre>
        </blockquote>
        <pre wrap="">
Sounds good. I'll try it.

</pre>
        <blockquote type="cite">
          <pre wrap="">
Did you want to remove the debug code in the security.cpp?  It seems that they are not used any more.
</pre>
        </blockquote>
        <pre wrap="">
Sure I can.

Thanks
Max

</pre>
        <blockquote type="cite">
          <pre wrap="">
Xuelei

On 6/21/2018 4:12 AM, Weijun Wang wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="">Please take a review on this change
 <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8205445/webrev.00/">http://cr.openjdk.java.net/~weijun/8205445/webrev.00/</a>
  and the release note at
 <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8205471">https://bugs.openjdk.java.net/browse/JDK-8205471</a>
The code change adds RSASSA-PSS signature support to the SunMSCAPI provider.
Several notes:
1. CryptoAPI (which SunMSCAPI is based on and now a deprecated technology) does not support RSASSA-PSS. In fact, CNG [1] is used to perform the signing and verification. This is certainly not a perfect solution and we are thinking of support CNG in a more sophisticated way in future releases of JDK.
2. For unknown reason, the newly added verification code for RSASSA-PSS does not work correctly (precisely, ::NCryptTranslateHandle returns NTE_INVALID_PARAMETER). A fallback mechanism is added into mscapi/RSASignature.java. A SunRsaSign Signature object is actually used when a SunMSCAPI Signature is initialized to verify an RSASSA-PSS signature.
3. It looks like CNG only supports PSSParamterSpec with the same message hash algorithm and MGF1 hash algorithm, because there is only one algorithm field in BCRYPT_PSS_PADDING_INFO [2]. This is checked when setting the parameter.
4. It looks like CNG only supports RSASSA-PSS using these hash algorithms: SHA-1, SHA-256, SHA-384, and SHA-512. This is not checked at parameter setting but sign() will throw a SignatureException saying "Unrecognised hash algorithm". Since the verify() side uses a fallback SunRsaSign signature, other hash algorithms are supported.
Thanks
Max
[1] <a class="moz-txt-link-freetext" href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa376210(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/windows/desktop/aa376210(v=vs.85).aspx</a>
[2] <a class="moz-txt-link-freetext" href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa375529(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/windows/desktop/aa375529(v=vs.85).aspx</a>
[3] <a class="moz-txt-link-freetext" href="https://msdn.microsoft.com/en-us/library/windows/desktop/aa375534(v=vs.85).aspx">https://msdn.microsoft.com/en-us/library/windows/desktop/aa375534(v=vs.85).aspx</a>
</pre>
          </blockquote>
        </blockquote>
        <pre wrap="">
</pre>
      </blockquote>
      <pre wrap="">
</pre>
    </blockquote>
    <br>
  </body>
</html>