RFR: 8301553: Support Password-Based Cryptography in SunPKCS11 [v6]

Francisco Ferrari Bihurriet duke at openjdk.org
Thu Jun 1 23:50:20 UTC 2023


On Tue, 30 May 2023 23:42:24 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Martin Balao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8301553: Support Password-Based Cryptography in SunPKCS11 (iteration #4)
>>   
>>   Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>   Co-authored-by: Martin Balao <mbalao at redhat.com>
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java line 221:
> 
>> 219:         // for the underlying cipher is equal to the PBE service key length.
>> 220:         // Otherwise, initialization fails.
>> 221:         return svcPbeKi.keyLen;
> 
> This method "Returns the key size of the given key object in bits. " 
> How do you ensure that this key is the one used in the initialization? This method may also throw InvalidKeyException though, With this impl, even if passing a null key, this would return an int value and not detecting the key is invalid.

@valeriepeng: the rationale behind this decision is based on the only usage of `engineGetKeySize()`, which corresponds to a [`crypto.policy=limited` _Cryptographic Jurisdiction Policy_](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/conf/security/java.security#L906-L942). Here, `engineGetKeySize()` is employed to check the key during the _Cipher_ initialization (see [`Cipher.java:1463`](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/classes/javax/crypto/Cipher.java#L1463), [`Cipher.java:1110`](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/classes/javax/crypto/Cipher.java#L1110), [`Cipher.java:1139`](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/classes/javax/crypto/Cipher.java#L1139)). So, what we can assert is that if the key size isn't going to be `svcPbeKi.keyLen`, then that very sa
 me _Cipher_ initialization will later fail anyway.

There also is a previous precedent set by _SunJCE_:
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java#L214-L216
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/classes/com/sun/crypto/provider/PBEWithMD5AndTripleDESCipher.java#L377-L379
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e//src/java.base/share/classes/com/sun/crypto/provider/PKCS12PBECipherCore.java#L355-L358
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e//src/java.base/share/classes/com/sun/crypto/provider/PBEWithMD5AndDESCipher.java#L367-L371

NOTE: by passing `-Djava.security.properties=<(echo crypto.policy=limited)` as a JVM argument to [`test/jdk/sun/security/pkcs11/Cipher/PBECipher.java`](https://github.com/openjdk/jdk/blob/80575ccb06f8f1aee26f9cd18b8504211d2f62c7/test/jdk/sun/security/pkcs11/Cipher/PBECipher.java#L191), I could reproduce a failure of the [older implementation using the underlying cipher](https://github.com/openjdk/jdk/blob/80575ccb06f8f1aee26f9cd18b8504211d2f62c7/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java#L210-L215): if we pass a `com.sun.crypto.provider.PBEKey` of the [`PBE` → `PBEWithMD5AndDES`](https://github.com/openjdk/jdk/blob/80575ccb06f8f1aee26f9cd18b8504211d2f62c7/src/java.base/share/classes/sun/security/util/SecurityProviderConstants.java#L279) algorithm, the [underlying `AES` cipher's `P11SecretKeyFactory.convertKey()` call](https://github.com/openjdk/jdk/blob/80575ccb06f8f1aee26f9cd18b8504211d2f62c7/src/jdk.crypto.cryptoki/share/classes/sun/security/pk
 cs11/P11Cipher.java#L1021-L1022) fails because this algorithm is not registered in the `P11SecretKeyFactory.keyInfo` map, so [`ki` is `null` here](https://github.com/openjdk/jdk/blob/80575ccb06f8f1aee26f9cd18b8504211d2f62c7/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java#L292-L295), resulting in `InvalidKeyException: Cannot use a PBEWithMD5AndDES key for a AES service`.

I've also found a failure for the current implementation when using `crypto.policy=limited`, I will investigate this further tomorrow.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1213781875



More information about the security-dev mailing list