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