RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI

[Valerie Peng]

Great, that's good then.

Valerie


On 6/22/2018 5:40 PM, Weijun Wang wrote:
>
>> On Jun 23, 2018, at 8:35 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>
>> On 6/22/2018 3:23 PM, Weijun Wang wrote:
>>>> On Jun 23, 2018, at 2:30 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>>>
>>>> Max,
>>>>
>>>> Good catch on the SunRsaSign provider bug.
>>>>
>>>> Looking at the changes, I think we may have to fine-grain the check on the ensureInit() call, i.e.
>>>>
>>>> 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.
>>> Could anything go wrong? This method just ensures one of initSign() or initVerify() is called.
>> Only when the initSign()/initVerify() does not match the subsequent calls of sign()/verify() I suppose.
> I see what you mean.
>
> The Signature class takes care of it:
>
>      public final byte[] sign() throws SignatureException {
>          if (state == SIGN) {
>              return engineSign();
>          }
>          throw new SignatureException("object not initialized for " +
>                                       "signing");
>      }
>
> Thanks
> Max
>
>> Valerie
>>>> In the PSS class engineInitVerify(...) method if the specified key is a MSCAPI public key, then fallbackSignature is set to null and the native verifyPssSignedHash method is used, right?
>>> Yes. The native method only fails when trying to import from a blob.
>>>
>>> Thanks
>>> Max
>>>
>>>> Thanks,
>>>>
>>>> Valerie
>>>>
>>>> On 6/21/2018 10:39 PM, Weijun Wang wrote:
>>>>> Webrev updated at
>>>>>
>>>>>    http://cr.openjdk.java.net/~weijun/8205445/webrev.01
>>>>>
>>>>>
>>>>> 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
>>>>>
>>>>>
>>>>>> On Jun 21, 2018, at 11:23 PM, Weijun Wang <weijun.wang at oracle.com>
>>>>>>   wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Jun 21, 2018, at 11:07 PM, Xuelei Fan <xuelei.fan at oracle.com>
>>>>>>>   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?
>>>>>>>
>>>>>> Sounds good. I'll try it.
>>>>>>
>>>>>>
>>>>>>> Did you want to remove the debug code in the security.cpp?  It seems that they are not used any more.
>>>>>>>
>>>>>> Sure I can.
>>>>>>
>>>>>> Thanks
>>>>>> Max
>>>>>>
>>>>>>
>>>>>>> Xuelei
>>>>>>>
>>>>>>> On 6/21/2018 4:12 AM, Weijun Wang wrote:
>>>>>>>
>>>>>>>> Please take a review on this change
>>>>>>>>   http://cr.openjdk.java.net/~weijun/8205445/webrev.00/
>>>>>>>>
>>>>>>>>    and the release note at
>>>>>>>>   https://bugs.openjdk.java.net/browse/JDK-8205471
>>>>>>>>
>>>>>>>> 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]
>>>>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa376210(v=vs.85).aspx
>>>>>>>>
>>>>>>>> [2]
>>>>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa375529(v=vs.85).aspx
>>>>>>>>
>>>>>>>> [3]
>>>>>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/aa375534(v=vs.85).aspx