On Mon, 14 Apr 2025 17:44:53 GMT, Francisco Ferrari Bihurriet 
<fferr...@openjdk.org> wrote:

>> Martin Balao has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - Algorithm and key size checking before derivation. Mechanism 
>> normalization for TLS.
>>  - Minor import adjustment.
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java
>  line 240:
> 
>> 238:         putKeyInfo(new KeyInfo("TlsPremasterSecret", 
>> PCKK_TLSPREMASTER));
>> 239:         putKeyInfo(new KeyInfo("TlsRsaPremasterSecret", 
>> PCKK_TLSRSAPREMASTER));
>> 240:         putKeyInfo(new KeyInfo("TlsMasterSecret", PCKK_TLSMASTER));
> 
> Have you considered removing `PCKK_TLSPREMASTER`, `PCKK_TLSRSAPREMASTER` and 
> `PCKK_TLSMASTER` from everywhere? We could just use `CKK_GENERIC_SECRET` for 
> the `TlsPremasterSecret`, `TlsRsaPremasterSecret` and `TlsMasterSecret` key 
> info entries.
> 
> Unlike `PCKK_ANY`, which is used for the `TemplateManager` to map `*` entries 
> in _SunPKCS11_ configuration files [1], these other 3 pseudo key types are 
> only used here in `P11SecretKeyFactory`. Additionally, any time these 3 
> pseudo key types are used, it is to map to `CKK_GENERIC_SECRET`.
> 
> [1] _SunPKCS11_ configuration files can't contain `PCKK_TLS*MASTER` 
> attributes entries, only `*` is parsable, and corresponds with `PCKK_ANY`:
> https://github.com/openjdk/jdk/blob/6ddbcc34c019d780fc12d8f636e3aa3de33ecaaa/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/Functions.java#L1258

I like this idea but the downside I see is that we would need string comparison 
in `P11KDF::getDerivedKeyType` to allow TLS keys. What if we merge all 
`PCKK_TLSPREMASTER`, `PCKK_TLSRSAPREMASTER` and `PCKK_TLSMASTER` into 
`PCKK_TLSKEY` and then do the translation to `CKK_GENERIC_SECRET` as needed? 
This will also help with the new Tls* keys that I am planning to add to the map.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24526#discussion_r2044527101

Reply via email to