On Mon, 4 Aug 2025 11:12:11 GMT, Sebastian Stenzel <d...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> combine security properties description; remove one test
>
> src/java.base/share/classes/com/sun/crypto/provider/ML_KEM_Impls.java line
> 132:
>
>> 130: if (nk instanceof NamedPKCS8Key npk) {
>> 131: var type = KeyChoices.getPreferred("mlkem");
>> 132: if (KeyChoices.typeOfChoice(npk.getRawBytes()) != type)
>> {
>
>> ```
>> /// 1. If only `privKeyMaterial` is present, it's also the expanded format.
>> /// 2. If both `privKeyMaterial` and `expanded` are available,
>> `privKeyMaterial`
>> /// is the encoding format, and `expanded` is the expanded format.
>> ```
>
> This in mind, the result of `getRawBytes()` differs. In the first case, it is
> in fact raw bytes, as the method name suggests. (See also usage of
> `privKeyMaterial` in `PKCS8Key.generateEncoding()`).
>
> Therefore, the ASN.1 envelope may be missing, causing
>
>
> java.security.InvalidKeyException: Wrong tag: -39
> at
> java.base/sun.security.util.KeyChoices.typeOfChoice(KeyChoices.java:144)
> at
> java.base/com.sun.crypto.provider.ML_KEM_Impls$KF.engineTranslateKey(ML_KEM_Impls.java:132)
> at java.base/java.security.KeyFactory.translateKey(KeyFactory.java:475)
>
>
> Isn't `getEncoded()` the safer bet?
>
> Suggestion:
>
> if (KeyChoices.typeOfChoice(npk.getEncoded()) != type) {
`getEncoded` is the whole PKCS #8 encoding.
`getRawBytes` returns the content of the `privateKey` field inside the PKCS #8
encoding.
`getExpanded` is not related to encoding. It's what the underlying `NamedKEM`
or `NamedSignature` will use.
For ML-KEM, `getRawBytes` might be any of the 3 CHOICEs: seed, expandedKey, or
both. `getExpanded` is always the expanded format (defined in FIPS 203). There
is a slight difference even if `getRawBytes` is using the expandedKey choice:
it has the OCTET STRING header but `getExpanded` does not.
`engineTranslateKey` can re-encode the key depending on the
"jdk.mlkem.pkcs8.encoding" security property. Therefore it works on the
`getRawBytes` level.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24969#discussion_r2252029114
