[8u] RFR: 8076190: Customizing the generation of a PKCS12 keystore
Alexey Bakhtin
alexey at azul.com
Wed May 12 17:44:44 UTC 2021
Hello Paul,
Thank you a lot for review.
I have created CSR at : https://bugs.openjdk.java.net/browse/JDK-8267040
For the code changes you mention - the following lines in my patch:
-> keyStore = KeyStore.getInstance(storetype);
-> keyStore.load(ksStream, storePass);
performs the same like
keyStore = KeyStore.getInstance(ksfile, storePass);
in the 11u codebase.
KeyStore.getInstance(File, char[]) was introduced in JDK9 and can not be added into 8u. So, in 8u I have to detect storetype and then open and load it.
Also, we are free to close ksStream in this block:
-> ksStream.close();
because of it is not used below in the code any more. So, it is better to close it similar to the “else” section.
Two terminating semicolons will be removed.
Thank you
Alexey
> On 11 May 2021, at 02:43, Hohensee, Paul <hohensee at amazon.com> wrote:
>
> 8202299: Lgtm.
> 8206189: Lgtm.
> 8202837: Lgtm.
> 8214513: Lgtm.
>
> 8076190:
>
> Will need a CSR.
>
> In Main.java, I'm not an expert, but the 11u code is
>
> // Probe for keystore type when filename is available
> if (ksfile != null && ksStream != null && providerName == null &&
> storetype == null && !inplaceImport) {
> keyStore = KeyStore.getInstance(ksfile, storePass);
> storetype = keyStore.getType();
> if (storetype.equalsIgnoreCase("pkcs12")) {
> isPasswordlessKeyStore = PKCS12KeyStore.isPasswordless(ksfile);
> }
> } else {
> if (storetype == null) {
> storetype = KeyStore.getDefaultType();
> }
> if (providerName == null) {
> keyStore = KeyStore.getInstance(storetype);
> } else {
> keyStore = KeyStore.getInstance(storetype, providerName);
> }
> ...
> if (!nullStream) {
> if (inplaceImport) {
> keyStore.load(null, storePass);
> } else {
> keyStore.load(ksStream, storePass);
> }
> if (ksStream != null) {
> ksStream.close();
> }
> }
>
> If storetype is PKCS12 in 11u, the code marked by "->" by me below in the 8u patch
>
> // Probe for keystore type when filename is available
> if (ksfile != null && ksStream != null && providerName == null &&
> storetype == null && !inplaceImport) {
> String realType = keyStoreType(ksfile);
> // If the magic number does not conform to JKS
> // then it must be PKCS12
> if (!"JKS".equalsIgnoreCase(realType)) {
> storetype = P12KEYSTORE;
> isPasswordlessKeyStore = PKCS12KeyStore.isPasswordless(ksfile);
> } else {
> storetype = KeyStore.getDefaultType();;
> }
> -> keyStore = KeyStore.getInstance(storetype);
> -> keyStore.load(ksStream, storePass);
> -> ksStream.close();
>
> doesn't get executed in 11u. I can see that it should be executed when realType is JKS because that's what happens with the old code, but is the new code benign when realtype is not JKS? Or maybe the 11u code has a bug?
>
> There are two semicolons terminating the non-PKCS12 assignment to storetype.
>
> Otherwise lgtm.
>
> Thanks,
> Paul
>
> -----Original Message-----
> From: jdk8u-dev <jdk8u-dev-retn at openjdk.java.net> on behalf of Alexey Bakhtin <alexey at azul.com>
> Date: Thursday, May 6, 2021 at 2:38 PM
> To: jdk8u-dev <jdk8u-dev at openjdk.java.net>
> Subject: [8u] RFR: 8076190: Customizing the generation of a PKCS12 keystore
>
> Hi,
>
> Please review the backport of JDK-8076190 to 8u for parity with Oracle.
> This feature minimizes behavior differences between JKS and PKCS12 keystores. Also, it fixes the issue with incorrectly decoded KDF algorithm as described in JDK-8245169 [5]
>
> JDK-8076190 depends on the series of PKCS12 related fixes, so this patch should be applied as a series of backports:
>
> 1) JDK-8202299: Java Keystore fails to load PKCS12/PFX certificates created in WindowsServer2016
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8202299
> Original patch :http://hg.openjdk.java.net/jdk/jdk/rev/bba1deda9216
> 8u webrev: http://cr.openjdk.java.net/~abakhtin/8202299/webrev.v0/
>
> Original patch applied almost clean except for copyright year.
> Also, I had to fix EmptyPassword.java test to load keystore using FileInputStream. Original test uses KeyStore.getInstance(File) method introduced in JDK9
>
> 2) JDK-8206189: sun/security/pkcs12/EmptyPassword.java fails with Sequence tag error
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8206189
> Original patch: http://hg.openjdk.java.net/jdk/jdk11/rev/038688fa32d0
> 8u webrev: http://cr.openjdk.java.net/~abakhtin/8206189/webrev.v0/
>
> Original patch applied almost clean except for two differences:
> - one of the block is not applied clean because of JDK-8234042 [1] already applied and JDK8 already verifies certValues length
> - JDK8 has strict class cast to Object which is not necessary in JDK11
>
> 3) JDK-8202837: PBES2 AlgorithmId encoding error in PKCS12 KeyStore
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8202837
> Original patch: http://hg.openjdk.java.net/jdk/jdk11/rev/69dc9ea17b33
> 8u webrev: http://cr.openjdk.java.net/~abakhtin/8202837/webrev.v0/
>
> Original patch applied almost clean except of HexDumpEncoder class name space in the com/sun/crypto/provider/PBES2Parameters.java
>
> 4) JDK-8214513: A PKCS12 keystore from Java 8 using custom PBE parameters cannot be read in Java 11
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8214513
> Original patch: http://hg.openjdk.java.net/jdk/jdk/rev/c0f40bca91a5
> 8u webrev: http://cr.openjdk.java.net/~abakhtin/8214513/webrev.v0/
>
> Patch applied clean. I had to fix WrongPBES2.java test to load keystore using FileInputStream. Original test uses KeyStore.getInstance(File) method introduced in JDK9
>
> 5) JDK-8076190: Customizing the generation of a PKCS12 keystore
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8076190
> Original patch: http://hg.openjdk.java.net/jdk/jdk/rev/2457d862a646
> 8u webrev: http://cr.openjdk.java.net/~abakhtin/8076190_8u/webrev.v0/
>
> Patch does not apply cleanly:
> - src/share/classes/com/sun/crypto/provider/SunJCE.java does not have JDK-7092821 [2] , so Mac.HmacPBESHA algorithms registration is performed in the old 8u style
> - src/java.base/share/classes/java/security/KeyStore.java - javadoc for getProtectionAlgorithm() method is not changed similar to OpenJDK11 changes
> - src/share/classes/sun/security/tools/keytool/Main.java PKCS12 is not default keystore in JDK8u, so I have to modify implementation of PKCS12 password-less detection
> - update all src/share/lib/security/java.security-* property files
> - test/jdk/sun/security/pkcs12/ParamsPreferences.java
> - Path.of replaced by Paths.get
> - added "-storetype PKCS12” options to generate PKCS12 keystore
> - test/sun/security/pkcs12/ParamsTest.java
> - replace missing transferTo() implementation
> - Path.of replaced by Paths.get
> - use “-storetype PKCS12” to force PKCS12 keystore generation
> - load keystore using FileInputStream because of missing KeyStore.getInstance(File) method
> - /test/jdk/sun/security/tools/keytool/ProbingFailure.java - JDK-8214100 [4] is not backported to JDK8u because of no KeyStore.getInstance(File) in the JDK8u. Changes are skipped because of no such test.
> - jdk/test/lib/security/DerUtils.java already added by JDK-8230978 [3]
>
> sun/security/pkcs12 and sun/security/tools/keytool tests passed.
>
> [1] https://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/3054a00b5333
> [2] https://bugs.openjdk.java.net/browse/JDK-7092821
> [3] https://bugs.openjdk.java.net/browse/JDK-8230978
> [4] https://bugs.openjdk.java.net/browse/JDK-8214100
> [5] https://bugs.openjdk.java.net/browse/JDK-8245169
>
>
> Regards
> Alexey
>
More information about the jdk8u-dev
mailing list