RFR: 8301553: Support Password-Based Cryptography in SunPKCS11 [v6]
Francisco Ferrari Bihurriet
duke at openjdk.org
Fri Jun 2 01:17:21 UTC 2023
On Thu, 1 Jun 2023 23:47:46 GMT, Francisco Ferrari Bihurriet <duke at openjdk.org> wrote:
>> 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
same _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...
I can confirm that the failures I was seeing were caused by the `128` bits key size limit here:
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/java.base/share/conf/security/policy/limited/default_local.policy#L13
If I set that to `512`, [`test/jdk/sun/security/pkcs11/Cipher/PBECipher.java`](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/test/jdk/sun/security/pkcs11/Cipher/PBECipher.java) passes with `crypto.policy=limited`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1213815957
More information about the security-dev
mailing list