On Mon, 4 Aug 2025 16:46:51 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> 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.
In X-Wing, the expanded decapsulation key is referred to as `sk_m`. I.e.
*without* the CHOICE structure. So in order to have
`NamedPKCS8Key.privKeyMaterial` contain the choice, I would have to do this:
byte[] keyBytesWrappedInChoice =
KeyChoices.writeToChoice(KeyChoices.Type.EXPANDED_KEY, null, sk_m);
PrivateKey key = NamedPKCS8Key.internalCreate("ML-KEM", "ML-KEM-768",
keyBytesWrappedInChoice, null);
However, if I now call `keyFactory.translateKey(key)`, it'll throw:
java.security.InvalidKeyException: key contains not enough info to translate
at
java.base/sun.security.util.KeyChoices.choiceToChoice(KeyChoices.java:220)
at
java.base/com.sun.crypto.provider.ML_KEM_Impls$KF.engineTranslateKey(ML_KEM_Impls.java:133)
at java.base/java.security.KeyFactory.translateKey(KeyFactory.java:475)
in base64 `sk_m` has this value (test case 1 from [X-Wing test
vectors](https://datatracker.ietf.org/doc/html/draft-connolly-cfrg-xwing-kem-08#appendix-C)):
IDjHPToyMBF3joBR7joyxgHNZdrAqPmpQIEj8GRIi+thihFD1Fqc+BbMlZuDnLsPH5y7DJWqXHsqhQNeiMRkT1VZjVF7psO12woyNCIYSHFQe8O/BspLfeOW/MFH4tONheMqoIKMUHvC28nEk4Q57YciB7y//iNoevxrdhG4/XrCetNairsm2RB0AQtsc1WhUUyluoExawtc3bmm2BmiGocA2PmbB4hBPVw8fXyhm4ACCJqKjKJ7BZBelLxh49fPLbaXxZYrC0s5O7Cbv/oujGxuk8K8NOpIFxZVSdota6ikQDe4ELd8Dhk0ppU3FjE0JhsjQshoNGtB2AkDgcDADDMb44giv+KWv8o1khgg52Q76KgrzlnMZdUn+ujHA1lK2FcYT+R52uyrJJec8WKCVtSPJdp+ajBrOaCoUlQMrRgMUMC0xnegLoubeWQBxdwUpSgloIjDG0RxVQRPCwgYczWfClLEaOaK/TlOiUisecKTeECk38gfr1IUvdocAtTO62klYLa4Z9fKO/uvXxwzHjap6uaWsJJeZ/GUMrCX+Helmdiu3DArkTgQl9kNIYgkapcOw1gKqxVDCtdHcntCp/fDy3sSDKaM1mZHdtmdZfMIbKW8sZiPHbyZqsJtZwEL0wjAzpeVnouGSCEvPSYuTfW3ThDLsKs+oFxI32S4EvgQNUd2vFd2kuUhmct6YDTLA9hh+Vs7jlEUYjtx1fAUIiO6F9tR1KWRskqkY8qWrWpJYBg61ySdKUq/kHEvhcieVqMGcId8DFpIUDhsCoIjmAQrabZyvTWb9ndKaboaa5kOFNGrEoA1iHovnOZgxhSQf/IXWncIZOJ0B+MSIYJ/OBgU2OrGvmq/28pxtHK1iSnBN5c2JXK/qhZdhkqdzHFhDbOyM7QOOyVnJuMXO3x+fXXPlCgrpVYivOZ5lhfJEuUQMPgm0abCeHScyGRSw2KXkXpg7icb/gWsTSedtEpGcTMie4BqeZaUZRYlxQ
oGeRCeQPZvXIS7ajlPmBxge1S5DDCCw4Q9gUgCAXo6DNQ10ABGXTmba6kkUwFz+xREY1sapcykudONJ7eo8NUkerE0I/pGUErLZrBulhERusBpPuB9FIKXNdY6hEei9OxgfwKWfGMeUrtoUDOrv9hvViBycHXNP0UroNeTU2au3ycSwtkquVNHYtG94DOi8/Bu59ueXwEx4TZNrXNyW9NwknYF7TgZQBm55PVHurYtq1Cy4Wq2r5TLbFUe5DearAdyKZy746WLCOm29wyp7HRvIsSpTqlyYAla5PymyQov0lqbJexd9bAnXUhOByJqHXM0oou+JIpcVrJSQMuE39soR6iFHhB9icVUxjOKjlWr1zeu30EEzAhVD3JKk9gNrImWY+UJkXuWvfFcowE/EmIDDckeqHqrx8iVDsF0g0pLNYgTI6okoHMLccBHolui9abEBPcswpSRDuDOkDUbuESkSAqiXGRS9NClUTkKLToMyByJshhOHEhONaNrxlAxd4psk+aNscF84sAfABBjd/R84gUj4iNrNajCSzmxCqEyOpapGaLO2IQAYzp7BxMXE/wUsrWxnPw9pfoaksSfJVE+D9MNaxYRyauWNdcIZyekt9IdNCROZpac8Vs7KnhTKfYbCWsnfqA3ODR5prVW3nIx/kt/qcmsJMBpmgAYpSU0AbrPqQXKgWVz5WotLgZ+m3KHUzuhOpN97bMfpEus7UB2mSNhADSuMeYZoXAkUZmzxcOYZIWf4bTJcXoHwwSVvfuYoKACzPVsEobO9QQd7ePETPFr9WLHRIUYAms9i5lAaAq9OKFXX9J7WNoGO/rDLDnDCGk3TAXBrrGJi2swPMaL5FU0buCvaZY2IkoUjKKuoQRjERxwn2m2nHDOhTh0ZpjExgqa7wAwx5JM7sQqXTaBb1RerhMpNGCzrLN+oOE9cOSqeGhto5ioOXwI6vloghE/5Pe61NpAsFAeHHU+/nMFPIcBToZhwzCZr+i
+3kFKWxqifYOSs+Ex6acMEFWHgkDK0PQNX+PN+FI26tl+KpdEg2OygIyq/VFs0lBSxcNiVDwlF+Ss0OYOwHFjAJtkJfwyJ3rO5xwkurU+2fKedMZqCjVklVmY12uWqai1DRY1pNemfrQt9WRNMwRXKTqAQvU8x6aSiPF+1Vgn6Cso6CZlqGoU+9lmReyoFywET4O8DYwLTIYmmFYxyoevgpBo8TWJY8szNmTKSCdjujs7sghXf5umrGLCX3ZZJ0O2S+UZMXcUy0ECy3svmiWytPBhXeMd7NnKVQJtbaC2URGxb+Uv7tikh+FERiptupNyj1ALb/xJ5RVWnvJf7Rev9SBQc2glNSWGD1i+O+YclkYEpqyBTmk1WWQCpSCkZws9KEMYhmWT0VpLsBw14+WH7gxn0ogNbyQH+3pwcSuDjeuWxde/K0S89gOMy+M/vPUaVKWE/pAIPJHHptQ9T7FfSMYML9ZuCoqtStZOXEK7iHfA6+wrXjh8ipiP3CO+ueFsh1d4HgoUmcYeE4wh8hbCnQdpeYccqmlCuvwJBUS+6ZtUsWy5qaNk1iRtn0LM5TxmtZxFyPmukpmnXRUYDDyVIVGpG3oQdyQp3Ey65vzGIvqAGMY0OfiQYwuZKNtrt/lDiuQGXtNNc9SG8/UvkPCAfciN/djHKOlU8aw1wGwADOQaBYJYDju1e2cpcokKxeeYjnhQZXEW8bV9CAmq7ewL7eGuFIFIMRxvfjFzRuUYn7jNY1uYb4wL3SdkHFhLd4s6kRqAvhyWkquOG7sSg5VzzOGd8YO0WDW7tVBS+fxmoWeO8qNt6nhBHmyNYFAbTmBZLRNpipQ7UJGF25EuLqEL4GFxI2syfHFxYJTJZKaLAzd/UToFvNmcHzRlg7sFKXehChKt/HWANOVhfaTBJ2WF5XdOHzuZeLCdDpxE07yGFRxDqtGFcScXNAIjrDgdIRUKBClOl7sTu9ohtaGCttqWnhmn/QcnN/qOiApT
wkKOPQSbfSGXQFKW3bNhkSp7z0gnztYR0Men2hBN3kMiCVM59kph1bsQj/C/TXgMrlCfsiwlaRQZP/c0kEJYEjfVIoKIJO4739B/sD8flC0uoXn+ci8GzAPeW2nL1+07ipDytAYD2J8QzfWFiXGJ+R/FH7jN0lxsff7gw/D8b6TkgnUxFoCH7yI+mwcMWninif1G1MYE1psROdTa
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24969#discussion_r2252458776
