RFR: 8328119: Support HKDF in SunPKCS11 (Preview) [v10]

Valerie Peng valeriep at openjdk.org
Sat Jan 11 01:46:56 UTC 2025


On Sat, 11 Jan 2025 01:40:39 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Martin Balao has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Renaming of P11KDF fix.
>>   
>>   Co-authored-by: Martin Balao Alonso <mbalao at redhat.com>
>>   Co-authored-by: Francisco Ferrari Bihurriet <fferrari at redhat.com>
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11HKDF.java line 230:
> 
>> 228:     }
>> 229: 
>> 230:     private abstract sealed class KeyMaterialMerger permits
> 
> Hmm, instead of all these different sub-classes which handle the various combination of key/data when merging the key materials, how about encapsulate the key/data into a record and handling the merging in one class? Something like:
> 
> 
>     record KeyMaterial(boolean isBytes, byte[] data, P11Key.P11SecretKey key) {
>     }
> 
>     private final class KeyMaterialMerger {
> 
>         private KeyMaterial curr;
> 
>         private KeyMaterial toKeyMaterial(SecretKey key) {
>             if (key instanceof SecretKeySpec) {
>                 return new KeyMaterial(true, key.getEncoded(), null);
>             } else {
>                 return new KeyMaterial(false, null,
>                         convertKey(key, "Failure merging key material"));
>             }
>         }
> 
>         KeyMaterialMerger merge(SecretKey nextKey) {
>             if (curr == null) {
>                 curr = toKeyMaterial(nextKey);
>             } else {
>                 KeyMaterial next = toKeyMaterial(nextKey);
>                 int combination = (curr.isBytes ? 2 : 0) |
>                         (next.isBytes ? 1 : 0);
>                 switch (combination) {
>                     case 3: // 'curr' and 'next' both are byte[]
>                         int newLen = curr.data.length + next.data.length;
>                         var resData = Arrays.copyOf(curr.data, newLen);
>                         System.arraycopy(next.data, 0, resData, newLen -
>                                 next.data.length, next.data.length);
>                         curr = new KeyMaterial(true, resData, null);
>                         break;
>                     case 2: // 'curr' is byte[], 'next' is key
>                         var resKey = p11Merge(next.key, new CK_MECHANISM(
>                             CKM_CONCATENATE_DATA_AND_BASE,
>                             new CK_KEY_DERIVATION_STRING_DATA(curr.data)),
>                             curr.data.length + next.key.keyLength);
>                         curr = new KeyMaterial(false, null, resKey);
>                         break;
>                     case 1: // 'curr' is key, 'next' is byte[]
>                         resKey = p11Merge(curr.key, new CK_MECHANISM(
>                             CKM_CONCATENATE_BASE_AND_DATA,
>                             new CK_KEY_DERIVATION_STRING_DATA(next.data)),
>                             curr.key.keyLength + next.data.le...

This way, there is no need for the 3 XXXKeyMaterialMerger (where XXX: Any/Data/Key) and the merging code is in 1 method instead of handled over 4 different methods.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22215#discussion_r1911796182


More information about the security-dev mailing list