Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
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 wrote:
On 6/22/2018 3:23 PM, Weijun Wang wrote:
On Jun 23, 2018, at 2:30 AM, Valerie Peng 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
wrote:
On Jun 21, 2018, at 11:07 PM, Xuelei Fan
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
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
> On Jun 23, 2018, at 8:35 AM, Valerie Peng wrote:
>
> On 6/22/2018 3:23 PM, Weijun Wang wrote:
>>> On Jun 23, 2018, at 2:30 AM, Valerie Peng 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
> wrote:
>
>
>
>
>> On Jun 21, 2018, at 11:07 PM, Xuelei Fan
>> 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
>
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
On 6/22/2018 3:23 PM, Weijun Wang wrote: On Jun 23, 2018, at 2:30 AM, Valerie Peng 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. 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 wrote: On Jun 21, 2018, at 11:07 PM, Xuelei Fan 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
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
> On Jun 23, 2018, at 2:30 AM, Valerie Peng 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 >>> wrote: >>> >>> >>> >>> On Jun 21, 2018, at 11:07 PM, Xuelei Fan 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 >
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
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 wrote: On Jun 21, 2018, at 11:07 PM, Xuelei Fan 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
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
Looks fine to me. Thanks, Xuelei 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 wrote: On Jun 21, 2018, at 11:07 PM, Xuelei Fan 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
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
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 wrote: > > > >> On Jun 21, 2018, at 11:07 PM, Xuelei Fan 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 >
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
Build change looks good. /Erik On 2018-06-21 04:12, 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
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
> On Jun 21, 2018, at 11:07 PM, Xuelei Fan 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
Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI
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? Did you want to remove the debug code in the security.cpp? It seems that they are not used any more. 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
