On Wed, 8 Jan 2025 02:31:38 GMT, Martin Balao <mba...@openjdk.org> wrote:
>> We would like to propose an implementation of the HKDF algorithms for >> SunPKCS11, aligned with the KDF API proposed for JDK 24 (see [JEP 478: Key >> Derivation Function API >> (Preview)](https://bugs.openjdk.org/browse/JDK-8189808)). >> >> This implementation will be under the _Preview_ umbrella until the KDF API >> becomes stable in a future JDK release. The benefit of this early proposal >> is to gather more feedback about the KDF API for future improvements. >> >> The `P11KDF` class has the core implementation and Java calls to the PKCS 11 >> API. Different native mechanism were used to merge key material: >> CKM_CONCATENATE_BASE_AND_DATA (key and data), CKM_CONCATENATE_BASE_AND_KEY >> (key and key) and CKM_CONCATENATE_DATA_AND_BASE (data and key). The >> implementation also supports merging data with data, at the Java level. List >> of HKDF algorithms supported: HKDF-SHA256, HKDF-SHA384, and, HKDF-SHA512. >> >> Derivation modes supported: extract, expand, and, extract-expand. >> >> We further advanced the consolidation of algorithm and key info in the >> P11SecretKeyFactory map —this effort started with the PBE support >> enhancement and has helped to avoid duplication—. The map has now >> information about HMAC (`HMACKeyInfo` class) and HKDF (`HKDFKeyInfo` class) >> algorithms. P11Mac is now aligned to take the information from the map. >> >> Generic keys now supported in SecretKeyFactory. Derived keys could be >> Generic. >> >> Testing >> >> * >> [TestHKDF.java](https://github.com/openjdk/jdk/blob/e87ec99b90ff742f531a5031fdeeb9f2e039856d/test/jdk/sun/security/pkcs11/KDF/TestHKDF.java) >> test added >> * All non-SHA1 & non-SHA224 RFC 5869 test vectors checked >> * Cross-checking against SunJCE's HKDF implementation for every algorithm >> possible >> * Static assertion data for resilience if SunJCE were not available >> * Use of derived key for encryption check >> * Concatenation of input key material and salt checked (multiple >> combinations) >> * Multiple derivation types checked (extract only, expand only, and, >> extract-expand) >> * Derive key and derive data checked >> * All supported HKDF algorithms tested (HKDF-SHA256, HKDF-SHA384, and, >> HKDF-SHA512) >> * DH and ECDH key derivation for TLS checked >> * Informative output for debugging purposes (shown automatically if there >> is a test failure) >> * Note: test failures do not prevent all tests for running >> * Test integrated to the SunPKCS11 tests framework >> >> * No regressions observed in jdk/sun/security/pkcs11 (114 tests pa... > > 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 <mba...@redhat.com> > Co-authored-by: Francisco Ferrari Bihurriet <fferr...@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.length); curr = new KeyMaterial(false, null, resKey); break; case 0: // 'curr' and 'next' both are key resKey = p11Merge(curr.key, new CK_MECHANISM( CKM_CONCATENATE_BASE_AND_KEY, next.key.getKeyID()), curr.key.keyLength + next.key.keyLength); curr = new KeyMaterial(false, null, resKey); break; default: // should not happen throw new ProviderException("internal error"); } } return this; } SecretKey getKeyMaterial() { if (curr == null) { return EMPTY_KEY; } else if (curr.isBytes) { return new SecretKeySpec(curr.data, "Generic"); } else { return curr.key; } } private final P11Key.P11SecretKey p11Merge( P11Key.P11SecretKey baseKey, CK_MECHANISM ckMech, int derivedKeyLen) { .... // same no changes here } The calling code is almost the same, e.g. private SecretKey consolidateKeyMaterial(List<SecretKey> keys) { KeyMaterialMerger keyMerger = new KeyMaterialMerger(); for (SecretKey key : keys) { keyMerger = keyMerger.merge(key); } return keyMerger.getKeyMaterial(); } ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22215#discussion_r1911795253
