[8u] RFR 8080462: Update SunPKCS11 provider with PKCS11 v2.40 support

[Andrew John Hughes]

On 19/11/2019 19:55, Martin Balao wrote:
> Hi,
> 
> I'd like to request a review for the 8u backport of 8080462 [1].
> 
> Webrev.00:
> 
>  *
> http://cr.openjdk.java.net/~mbalao/webrevs/8080462/8080462.8u.jdk.webrev.00/
> 
> Differences from 11u patch [2]:
> 
>  * src/share/legal/pkcs11cryptotoken.md
>   * Does not apply because "8169925: Organize licenses by module in
> source, JMOD file, and run-time image" [3] is not in 8u.
> 
>  * src/share/classes/sun/security/pkcs11/SunPKCS11.java
>   * 6th and 11th hook do not apply cleanly because ECParameters location
> is "sun.security.ec.ECParameters" in 8u instead of
> "sun.security.util.ECParameters"
>   * 8th hook does not apply cleanly because 8042967 [4] is not in 8u.
> 
>  * src/share/classes/sun/security/pkcs11/wrapper/CK_MECHANISM.java
>   * 5th hook does not apply cleanly because toString method uses a
> StringBuffer instead of a StringBuilder (8041679 [5] is not in 8u).
> 
>  * src/share/classes/sun/security/pkcs11/wrapper/CK_RSA_PKCS_PSS_PARAMS.java
>   * 1st hook does not apply cleanly because toString method uses a
> StringBuffer instead of a StringBuilder (8041679 [5] is not in 8u).
> 
>  * src/share/native/sun/security/pkcs11/wrapper/p11_keymgmt.c
>   * 13th hook does no apply cleanly because 8074580 [6] is not in 8u.
> Manually applied change.
> 
>  * src/share/native/sun/security/pkcs11/wrapper/p11_mutex.c
>   * Copyright date.
> 
>  * src/share/native/sun/security/pkcs11/wrapper/p11_util.c
>   * Copyright date.
> 
>  * src/share/native/sun/security/pkcs11/wrapper/pkcs11wrapper.h
>   * 4th hook does not apply cleanly because 6913047 was backported to 8u
> without the "//#define P11_DEBUG" line.
> 
>  * test/sun/security/pkcs11/MessageDigest/ByteBuffers.java
>   * 1th hook does not apply cleanly because of copyright date.
>   * 2nd hook do not apply cleanly because 8164639 [7], 8078334 [8],
> 8172527 [9], 8144539 [10] are not in 8u. Manually applied changes.
> 
>  * src/share/classes/sun/security/util/GCMParameters.java
>   * HexDumpEncoder is sun.misc.HexDumpEncoder in 8u (instead of
> sun.security.util.HexDumpEncoder)
> 
>  * src/share/classes/sun/security/pkcs11/P11PSSSignature.java
>   * PSSParameterSpec.TRAILER_FIELD_BC does not exist in 8u because
> 8146293 [11] has not been backported. Added a private field in
> P11PSSSignature with the constant.
> 
>  * test/sun/security/pkcs11/Cipher/TestKATForGCM.java
>  * test/sun/security/pkcs11/Cipher/Test4512704.java
>  * test/sun/security/pkcs11/Cipher/TestCICOWithGCM.java
>  * test/sun/security/pkcs11/Cipher/TestCICOWithGCMAndAAD.java
>  * test/sun/security/pkcs11/Cipher/TestGCMKeyAndIvCheck.java
>  * test/sun/security/pkcs11/Signature/InitAgainPSS.java
>  * test/sun/security/pkcs11/Signature/KeyAndParamCheckForPSS.java
>  * test/sun/security/pkcs11/Signature/SigInteropPSS.java
>  * test/sun/security/pkcs11/Signature/SignatureTestPSS.java
>  * test/sun/security/pkcs11/Signature/TestDSA2.java
>   * @library jtreg header modified to remove "/test/lib"
>   * 8144539 [12] is not in 8u. Given that the test uses no arguments, I
> discarded the parameter when calling PKCS11Test::main method.
> 
>  * test/sun/security/pkcs11/Signature/InitAgainPSS.java
>   * PSSParameterSpec.TRAILER_FIELD_BC does not exist in 8u because
> 8146293 [11] has not been backported. Added a private field in
> InitAgainPSS with the constant.
> 
>  * make/mapfiles/libj2pkcs11/mapfile-vers
>   * Added Java_sun_security_pkcs11_wrapper_PKCS11_freeMechanism native
> method
> 
>  * test/sun/security/pkcs11/Signature/SigInteropPSS.java
>   * "java.security.NoSuchAlgorithmException: no such algorithm:
> RSASSA-PSS for provider SunRsaSign" error.
>    * This test cannot properly execute because 8146293 [11] is not in
> 8u. Manually modified to skip unless 8146293 [11] is available.
> 
> 
> Testing
> 
>  * No regressions have been observed in sun/security/pkcs11 category
> 
>  * All new tests (introduced by this enhancement) pass
>   * Note: SigInteropPSS is skipped for the reasons previously stated
> 
> Thanks,
> Martin.-
> 
> --
> [1] - https://bugs.openjdk.java.net/browse/JDK-8080462
> [2] - https://hg.openjdk.java.net/jdk-updates/jdk11u-dev/rev/8bac0ba1d5ce
> [3] - https://bugs.openjdk.java.net/browse/JDK-8169925
> [4] - https://bugs.openjdk.java.net/browse/JDK-8042967
> [5] - https://bugs.openjdk.java.net/browse/JDK-8041679
> [6] - https://bugs.openjdk.java.net/browse/JDK-8074580
> [7] - https://bugs.openjdk.java.net/browse/JDK-8164639
> [8] - https://bugs.openjdk.java.net/browse/JDK-8078334
> [9] - https://bugs.openjdk.java.net/browse/JDK-8172527
> [10] - https://bugs.openjdk.java.net/browse/JDK-8144539
> [11] - https://bugs.openjdk.java.net/browse/JDK-8146293
> [12] - https://bugs.openjdk.java.net/browse/JDK-8144539
> 

* The home of legal notices in 8u is the THIRD_PARTY_README file in each
repository, so the changes should go there, not simply be dropped. This
one actually seems to be missing at present (it suddenly appears with
JDK-8169925), so even more reason to add it above the section for the
PKCS#11 wrapper.
* The original patch removes pkcs-11v2-20a3.h while the 8u backport doesn't.
* The change to TRAILER_FIELD_BC is correct, as altering
PSSParameterSpec would alter the Java 8 API. However, it may be worth
waiting for JDK-8146293, which has now been proposed for backport via
JDK-8230978, which will also initiate a spec change via the reference
implementation, jdk8u41.
* I'm seeing a lot of noise when comparing the 8u and 11u patches for
CK_RSA_PKCS_PSS_PARAMS.java, PKCS11Constants.java, p11_convert.c,
p11_digest.c, p11_sign.c & pkcs11t.h, though the changes look the same.
Have you checked that the patched files in 8u compare to those in 8u as
expected?
* Please don't alter the SigInteropPSS.java test without comments
explaining why. As JDK-8146293 is imminent, I'd say just leave this as
it is and it will be automatically resolved.
* Why is the args argument dropped from the tests? Is this another
missing fix?

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew