On Tue, 18 Mar 2025 23:20:39 GMT, Francisco Ferrari Bihurriet <fferr...@openjdk.org> wrote:
>> As part of [https://bugs.openjdk.org/browse/JDK-8301553](JDK-8301553), >> SunPKCS11 provider added support for PBE SecretKeyFactories for >> `HmacPBESHAxxx` and `PBEWithHmacSHAxxxAndAES_yyy`. These impls produce keys >> whose encoding contains the PBKDF2 derived bytes. Given that SunJCE provider >> have supported `PBEWithHmacSHAxxxAndAES_yyy` SecretKeyFactories whose key >> encoding is the password bytes for long time. Such difference may be very >> confusing, e.g. using the same KeySpec and same-name SecretKeyFactory (from >> different providers), the resulting keys have same algorithm and format but >> different encodings. >> >> Given that the `P11Mac` and `P11PBECipher` classes already do key derivation >> internally, these PKCS11 SecretKeyFactories aren't a must-have and are >> proposed to be removed. I've also aligned the com.sun.crypto.provider.PBEKey >> class with com.sun.crypto.provider.PPBKDF2KeyImpl class to switch to "UTF-8" >> when converting the char[] to byte[]. This is to accomodate unicode >> passwords and given that "UTF-8" encoding is same for ASCII characters, this >> change should not affect backward compatibility. > > test/jdk/sun/security/pkcs11/Mac/PBAMac.java line 1: > >> 1: /* > > I suggest changing this test's password to contain non-ASCII characters, so > we have a better coverage in both _SunJCE_ (when checking the assertion data) > and _SunPKCS11_ (when doing the actual test): > > diff --git a/test/jdk/sun/security/pkcs11/Mac/PBAMac.java > b/test/jdk/sun/security/pkcs11/Mac/PBAMac.java > index b70a0a6d618..0baf85bb5de 100644 > --- a/test/jdk/sun/security/pkcs11/Mac/PBAMac.java > +++ b/test/jdk/sun/security/pkcs11/Mac/PBAMac.java > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2023, Red Hat, Inc. > + * Copyright (c) 2023, 2025, Red Hat, Inc. > * > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. > * > @@ -44,7 +44,7 @@ > */ > > public final class PBAMac extends PKCS11Test { > - private static final char[] password = "123456".toCharArray(); > + private static final char[] password = "123456\uA4F7".toCharArray(); > private static final byte[] salt = "abcdefgh".getBytes( > StandardCharsets.UTF_8); > private static final int iterations = 1000; > @@ -102,19 +102,19 @@ private static void checkAssertionValues(BigInteger > expectedValue, > // Generated with SunJCE. > private static final AssertionData[] assertionData = new AssertionData[]{ > macAssertionData("HmacPBESHA1", "HmacSHA1", > - "707606929395e4297adc63d520ac7d22f3f5fa66"), > + "8611414ddb1875d9f576282199ab492a802b7d49"), > macAssertionData("HmacPBESHA224", "HmacSHA224", > - > "4ffb5ad4974a7a9fca5a36ebe3e34dd443c07fb68c392f8b611657e6"), > + > "cebb12b48eb90c07336c695f771d1d0ef4ccf5b9524fc0ab6fb9813a"), > macAssertionData("HmacPBESHA256", "HmacSHA256", > - > "9e8c102c212d2fd1334dc497acb4e002b04e84713b7eda5a63807af2" + > - "989d3e50"), > + > "d83a6a4e8b0e1ec939d05790f385dd774bd2b7c17cfa2dd004efc894" + > + "e5d53f51"), > macAssertionData("HmacPBESHA384", "HmacSHA384", > - > "77f31a785d4f2220251143a4ba80f5610d9d0aeaebb4a278b8a7535c" + > - "8cea8e8211809ba450458e351c5b66d691839c23"), > + > "ae6b69cf9edfd9cd8c3b51cdf2b0243502f35a3e6007f33b1ab73568" + > + "2ea81ea562f4383bb9512ff70752367b7259b16f"), > macAssertionData("HmacPBESHA512", "HmacSHA512", > - > "a53f942a844b234a69c1f92cba20ef272c4394a3cf4024dc16d9dbac" + > - "1969870b1c2b28b897149a1a... Even when the suggested `PBECipher` and `PBAMac` test changes would improve the confidence, validating _SunJCE_ against _SunPKCS11_ and viceversa is not a completely independent test, specially given both providers share some common code in `PBEUtil`. For this reason, I've just also did the following cross-check with OpenSSL: # Non-ASCII password password='Th1s is a Bullet: •' # Create a PKCS #12 keystore with a certificate and a key pair openssl req -x509 -nodes -newkey rsa:4096 -subj /C=TT/ST=TT/L=TT/O=Test/CN=test.com/ -keyout key.pem -out cert.pem openssl pkcs12 -export -inkey key.pem -in cert.pem -passout "pass:$password" -out ks.p12 # Read the keystore with keytool build/*/images/jdk/bin/keytool -v -list -storetype pkcs12 -keystore ks.p12 -storepass "$password" # Cleanup unset password && rm -f key.pem cert.pem ks.p12 <details> <summary><code>keytool</code> output from JDK 23:</summary> keytool error: java.io.IOException: keystore password was incorrect java.io.IOException: keystore password was incorrect at java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2112) at java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:228) at java.base/java.security.KeyStore.load(KeyStore.java:1499) at java.base/sun.security.tools.keytool.Main.doCommands(Main.java:961) at java.base/sun.security.tools.keytool.Main.run(Main.java:429) at java.base/sun.security.tools.keytool.Main.main(Main.java:410) Caused by: java.security.UnrecoverableKeyException: failed to decrypt safe contents entry: java.io.IOException: getSecretKey failed: Password is not ASCII ... 6 more </details> <details> <summary><code>keytool</code> output from a build of this PR's branch:</summary> keytool error: java.io.IOException: keystore password was incorrect java.io.IOException: keystore password was incorrect at java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2109) at java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:226) at java.base/java.security.KeyStore.load(KeyStore.java:1502) at java.base/sun.security.tools.keytool.Main.doCommands(Main.java:951) at java.base/sun.security.tools.keytool.Main.run(Main.java:419) at java.base/sun.security.tools.keytool.Main.main(Main.java:400) Caused by: java.security.UnrecoverableKeyException: failed to decrypt safe contents entry: javax.crypto.BadPaddingException: Given final block not properly padded. Such issues can arise if a bad key is used during decryption. ... 6 more </details> <details> <summary><code>keytool</code> output from a build of this PR's branch, including my previous <code>PBEUtil</code> suggestion:</summary> Keystore type: PKCS12 Keystore provider: SUN Your keystore contains 1 entry Alias name: 1 Creation date: Mar 19, 2025 Entry type: PrivateKeyEntry Certificate chain length: 1 Certificate[1]: Owner: CN=test.com, O=Test, L=TT, ST=TT, C=TT Issuer: CN=test.com, O=Test, L=TT, ST=TT, C=TT Serial number: 52dbdcf60a04c79cb1c2375502f0f8f026e65724 Valid from: Wed Mar 19 18:22:27 CET 2025 until: Fri Apr 18 19:22:27 CEST 2025 Certificate fingerprints: SHA1: 4D:79:59:2D:92:F6:83:40:0B:29:38:B4:79:96:39:2D:43:04:8B:FB SHA256: 4A:20:0E:94:D2:F0:5E:D8:A9:7F:D5:D3:C7:35:D4:DA:B0:BB:F4:14:E6:5D:E8:48:5E:87:99:A7:C5:F3:4A:FB Signature algorithm name: SHA256withRSA Subject Public Key Algorithm: 4096-bit RSA key Version: 3 Extensions: #1: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: AC E0 8F C9 E3 81 63 6B 4E F8 2B C1 35 41 AA CC ......ckN.+.5A.. 0010: 8C C8 DB 16 .... ] ] #2: ObjectId: 2.5.29.19 Criticality=true BasicConstraints:[ CA:true PathLen: no limit ] #3: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: AC E0 8F C9 E3 81 63 6B 4E F8 2B C1 35 41 AA CC ......ckN.+.5A.. 0010: 8C C8 DB 16 .... ] ] ******************************************* ******************************************* </details> ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24068#discussion_r2003907172
