Re: RFR 8205445: Add RSASSA-PSS Signature support to SunMSCAPI

2018-06-22 Thread Xuelei Fan

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

2018-06-21 Thread Xuelei Fan

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



Re: Update Re: new infra? Fwd: Re: Request to build security jars

2013-03-06 Thread Xuelei Fan
Looks fine to me.

Thanks,
Xuelei

On 3/7/2013 10:07 AM, Weijun Wang wrote:
> [ Copying security-dev and build-dev ]
> 
> JDK-8009604
> 
> and please review the fix at
> 
>   http://cr.openjdk.java.net/~weijun/8009604/webrev.00/
> 
> Noreg-build.
> 
> Thanks
> Max
> 



Re: code review request: 7091290: fails to build jdk8 b05 Embedded build

2011-09-19 Thread Xuelei Fan
Looks fine to me.

Xuelei

On Sep 20, 2011, at 11:57 AM, Weijun Wang  wrote:

> Code change
> 
>   http://cr.openjdk.java.net/~weijun/7091290/webrev.00/
> 
> Normally, Oid.java is compiled when compiling SSL (sun/security/ssl) and 
> deprecation warning is not treated as error there. In some increment builds, 
> it might be compiled along with jgss codes, where deprecated warning is an 
> error.
> 
> No regression tests, build issue.
> 
> How to verify: after JDK is built, remove files in classes/org/ietf/jgss/ and 
> classes/sun/security/jgss/, go into make/sun/security and call make.
> 
> Thanks
> Max
> 
>  Original Message 
> *Change Request ID*: 7091290
> *Synopsis*: fails to build jdk8 b05 Embedded build
> 
> 
> === *Description* 
> # Running javac:
> /java/re/jdk/1.7.0/archive/fcs/binaries/linux-i586/bin/java 
> -XX:-PrintVMOptions -XX:+UnlockDiagnosticVMOptions -XX:-LogVMOutput -client 
> -Xmx512m -Xms512m -XX:PermSize=32m -XX:MaxPermSize=160m 
> -Xbootclasspath/p:/HUDSON/workspace/embedded-jdk7/ws/builds/b05/linux-i586-ea/langtools/dist/bootstrap/lib/javac.jar
>  -jar 
> /HUDSON/workspace/embedded-jdk7/ws/builds/b05/linux-i586-ea/langtools/dist/bootstrap/lib/javac.jar
>  -Werror -Xlint:all -Xlint:-path -source 7 -target 7 -encoding ascii 
> -Xbootclasspath:/HUDSON/workspace/embedded-jdk7/ws/builds/b05/linux-i586-ea/classes
>  -sourcepath 
> ../../../../src/closed/solaris/classes:../../../../src/closed/share/classes:/HUDSON/workspace/embedded-jdk7/ws/builds/b05/linux-i586-ea/gensrc:../../../../src/solaris/classes:../../../../src/share/classes
>  -d /HUDSON/workspace/embedded-jdk7/ws/builds/b05/linux-i586-ea/classes 
> @/HUDSON/workspace/embedded-jdk7/ws/builds/b05/linux-i586-ea/tmp/sun/sun.security.jgss/.classes.list.filtered
> ../../../../src/share/classes/org/ietf/jgss/Oid.java:160: warning: 
> [deprecation] equals(ObjectIdentifier) in ObjectIdentifier has been deprecated
>return this.oid.equals(((Oid) other).oid);
>   ^
> error: warnings found and -Werror specified
> 1 error
> 1 warning
> make[5]: *** [.compile.classlist] Error 1
> make[5]: Leaving directory 
> `/HUDSON/workspace/embedded-jdk7/ws/jdk/make/sun/security/jgss'
> make[4]: *** [all] Error 1
> make[4]: Leaving directory 
> `/HUDSON/workspace/embedded-jdk7/ws/jdk/make/sun/security'
> make[3]: *** [all] Error 1
> make[3]: Leaving directory `/HUDSON/workspace/embedded-jdk7/ws/jdk/make/sun'
> make[2]: *** [all] Error 1
> make[2]: Leaving directory `/HUDSON/workspace/embedded-jdk7/ws/jdk/make'
> make[1]: *** [jdk-build] Error 2
> make[1]: Leaving directory `/HUDSON/workspace/embedded-jdk7/ws'
> make: *** [build_product_image] Error 2
> Notifying upstream projects of job completion
> Finished: FAILURE