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.

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?

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

Reply via email to