> On Jun 23, 2018, at 2:30 AM, Valerie Peng <valerie.p...@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.
>
> 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.w...@oracle.com>
>>> wrote:
>>>
>>>
>>>
>>>
>>>> On Jun 21, 2018, at 11:07 PM, Xuelei Fan <xuelei....@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
>
