On Thu, 1 Jun 2023 23:47:46 GMT, Francisco Ferrari Bihurriet <d...@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
