From f7b394f8b300b13a17933f42d27e29241cfea223 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Fri, 31 Jul 2026 17:18:07 -0400 Subject: [PATCH 1/6] feat(sdk): support ML-KEM rewrap session keys Part of DSPX-4221 (Session Keys should support ML-KEM). KASClient.unwrap() only branched EC vs RSA for the client's ephemeral rewrap "session key", so requesting an ML-KEM session key would fail. KeyType already had MLKEM768Key/MLKEM1024Key entries and sdk-pqc-bc already had the BouncyCastle KEM primitives (used for KAS-managed mechanism keys), but nothing generated a fresh ML-KEM keypair for the session-key role. - spi/KemProvider: add generateKeyPair(KeyType), returning a new KeyPairPem (PEM-encoded SPKI/PKCS#8 pair). The core sdk module has no compile-time dependency on BouncyCastle, so the actual keypair generation lives in the optional sdk-pqc-bc module and is reached via the existing ServiceLoader-based KemProviders registry. - sdk-pqc-bc/HybridCrypto: add a generateKeyPair dispatch mirroring the existing wrapDEK/unwrapDEK switch, covering all five supported PQC key types (X-Wing, both NIST hybrids, ML-KEM-768/1024). - sdk-pqc-bc/BouncyCastleKemProvider: implement the new SPI method. - KASClient.unwrap(): branch on sessionKeyType.isMLKEM(), generating a keypair via KemProviders.get(...).generateKeyPair(...) and decrypting the response with KemProviders.get(...).unwrapDEK(...). - New unit test (HybridCryptoGenerateKeyPairTest) round-trips generateKeyPair -> wrapDEK -> unwrapDEK for all five PQC key types. - cmdline: add "session-key-mlkem" to the `supports` subcommand's feature list, gated on KemProviders.registered() actually containing an ML-KEM provider rather than being unconditionally true (so a FIPS-profile build, which excludes sdk-pqc-bc, correctly reports it as unsupported instead of claiming a capability that would throw at runtime). Addresses CodeRabbit review feedback: - unwrap() previously wrote the generated session public key to the shared clientPublicKey instance field before serializing the request. A concurrent unwrap() call on the same KASClient could overwrite that field first, causing this call to send the wrong public key while decrypting with its own private key. Refactored so each call's session-key material (public PEM plus whichever private key it needs for the response) is a call-scoped SessionKeyMaterial value, threaded explicitly through generateSessionKeyMaterial()/unwrapResponseKey() rather than shared mutable fields. This split also resolves a SonarCloud cognitive-complexity (java:S3776) failure on unwrap() from combining request-build and response-decrypt branching in one method. - Follow-up on the same class of bug: the RSA branch's lazy keypair-initialization was still a check-then-act race on non-volatile fields (decryptor/clientPublicKey), so two concurrent first callers could generate distinct keypairs and race to overwrite each other's fields. Fixed with double-checked locking and volatile fields. - Command.Supports' FEATURES list unconditionally claimed "session-key-mlkem" regardless of whether an ML-KEM KemProvider is actually registered (e.g. under the fips Maven profile, which excludes sdk-pqc-bc). Now computed dynamically via KemProviders.registered(). Ref: DSPX-4221 Signed-off-by: Dave Mihalcik --- .../java/io/opentdf/platform/Command.java | 22 +++- .../java/io/opentdf/platform/CommandTest.java | 4 +- .../sdk/pqc/bc/BouncyCastleKemProvider.java | 5 + .../platform/sdk/pqc/bc/HybridCrypto.java | 32 +++++ .../bc/HybridCryptoGenerateKeyPairTest.java | 43 +++++++ .../io/opentdf/platform/sdk/KASClient.java | 115 ++++++++++++------ .../opentdf/platform/sdk/spi/KemProvider.java | 22 ++++ 7 files changed, 200 insertions(+), 43 deletions(-) create mode 100644 sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java diff --git a/cmdline/src/main/java/io/opentdf/platform/Command.java b/cmdline/src/main/java/io/opentdf/platform/Command.java index 936a6ea7a..350fd72fe 100644 --- a/cmdline/src/main/java/io/opentdf/platform/Command.java +++ b/cmdline/src/main/java/io/opentdf/platform/Command.java @@ -47,6 +47,7 @@ import io.opentdf.platform.sdk.KeyType; import io.opentdf.platform.sdk.SDK; import io.opentdf.platform.sdk.SDKBuilder; +import io.opentdf.platform.sdk.spi.KemProviders; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.config.Configurator; import picocli.CommandLine; @@ -79,11 +80,23 @@ class Command { @CommandLine.Command(name = "supports", description = "Check if a feature is supported, or list all supported features") static class Supports implements Callable { - // Static, always-on capabilities of this build. There is no "known but + // Always-on capabilities of this build. There is no "known but // unsupported" feature here: anything not in this list is simply unrecognized. - private static final List FEATURES = List.of("dpop", "dpop_nonce_challenge", + private static final List STATIC_FEATURES = List.of("dpop", "dpop_nonce_challenge", "partial_decrypt", "http_range_source"); + // session-key-mlkem additionally requires an ML-KEM KemProvider on the + // classpath (e.g. sdk-pqc-bc), which the fips Maven profile excludes. + // Check the live registry rather than assuming build-profile wiring, so + // this doesn't claim support on a classpath that would actually throw. + private static List features() { + List features = new ArrayList<>(STATIC_FEATURES); + if (KemProviders.registered().contains(KeyType.MLKEM768Key)) { + features.add("session-key-mlkem"); + } + return features; + } + @CommandLine.Parameters(index = "0", arity = "0..1", description = "Feature to check (e.g., dpop). Omit to list all supported features.") private String feature; @@ -92,12 +105,13 @@ static class Supports implements Callable { @Override public Integer call() { + List features = features(); if (feature == null) { - printFeatures(FEATURES); + printFeatures(features); return 0; } - Optional canonical = FEATURES.stream().filter(f -> f.equalsIgnoreCase(feature)).findFirst(); + Optional canonical = features.stream().filter(f -> f.equalsIgnoreCase(feature)).findFirst(); if (json) { Map result = new LinkedHashMap<>(); // Emit the canonical feature name for recognized features so casing is stable diff --git a/cmdline/src/test/java/io/opentdf/platform/CommandTest.java b/cmdline/src/test/java/io/opentdf/platform/CommandTest.java index 9bb01a1f9..d58539ee8 100644 --- a/cmdline/src/test/java/io/opentdf/platform/CommandTest.java +++ b/cmdline/src/test/java/io/opentdf/platform/CommandTest.java @@ -108,7 +108,7 @@ void supports_noArgs_listsFeatures_exits0() { assertThat(code[0]).isEqualTo(0); assertThat(out.lines()).containsExactlyInAnyOrder("dpop", "dpop_nonce_challenge", - "partial_decrypt", "http_range_source"); + "partial_decrypt", "http_range_source", "session-key-mlkem"); } @Test @@ -118,7 +118,7 @@ void supports_noArgs_json_listsFeatures() { assertThat(code[0]).isEqualTo(0); assertThat(out.trim()).isEqualTo("{\"dpop\":true,\"dpop_nonce_challenge\":true," - + "\"partial_decrypt\":true,\"http_range_source\":true}"); + + "\"partial_decrypt\":true,\"http_range_source\":true,\"session-key-mlkem\":true}"); } @Test diff --git a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java index cccad4619..1ca268b96 100644 --- a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java +++ b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java @@ -51,4 +51,9 @@ public byte[] wrapDEK(KeyType keyType, String publicKeyPEM, byte[] dek) { public byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped) { return HybridCrypto.unwrapDEK(keyType, privateKeyPEM, wrapped); } + + @Override + public KeyPairPem generateKeyPair(KeyType keyType) { + return HybridCrypto.generateKeyPair(keyType); + } } diff --git a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java index 361ec2d3f..cbdf1c53e 100644 --- a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java +++ b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java @@ -3,6 +3,7 @@ import io.opentdf.platform.sdk.ECKeyPair; import io.opentdf.platform.sdk.KeyType; import io.opentdf.platform.sdk.SDKException; +import io.opentdf.platform.sdk.spi.KemProvider; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; @@ -96,6 +97,37 @@ static byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped) { } } + /** + * Generate a fresh ephemeral keypair for {@code keyType}. Same dispatch + * table as {@link #wrapDEK} / {@link #unwrapDEK}. + */ + static KemProvider.KeyPairPem generateKeyPair(KeyType keyType) { + switch (keyType) { + case HybridXWingKey: { + XWingKeyPair kp = XWingKeyPair.generate(); + return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + } + case HybridSecp256r1MLKEM768Key: { + HybridNISTKeyPair kp = HybridNISTAlgorithm.P256_MLKEM768.generate(); + return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + } + case HybridSecp384r1MLKEM1024Key: { + HybridNISTKeyPair kp = HybridNISTAlgorithm.P384_MLKEM1024.generate(); + return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + } + case MLKEM768Key: { + MLKEMKeyPair kp = MLKEMAlgorithm.MLKEM_768.generate(); + return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + } + case MLKEM1024Key: { + MLKEMKeyPair kp = MLKEMAlgorithm.MLKEM_1024.generate(); + return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + } + default: + throw new SDKException("unsupported PQC key type: " + keyType); + } + } + /** * Build the ASN.1 envelope from a hybrid KEM ciphertext and the AES-GCM(iv||ct) encrypted DEK. */ diff --git a/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java new file mode 100644 index 000000000..52740be43 --- /dev/null +++ b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java @@ -0,0 +1,43 @@ +package io.opentdf.platform.sdk.pqc.bc; + +import io.opentdf.platform.sdk.KeyType; +import io.opentdf.platform.sdk.spi.KemProvider; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Unit tests for {@link HybridCrypto#generateKeyPair}, the dispatcher backing + * {@link BouncyCastleKemProvider#generateKeyPair} and (in the core {@code sdk} + * module) {@code KASClient}'s KEM branch for the rewrap client "session key". + * + *

Each variant is exercised end-to-end through the same PEM-based + * {@link KemProvider} contract KASClient uses: generate a fresh keypair, wrap + * a DEK against its public PEM, unwrap against its private PEM, assert equal. + */ +class HybridCryptoGenerateKeyPairTest { + + private static final byte[] DEK = "0123456789abcdef0123456789abcdef".getBytes(StandardCharsets.UTF_8); + + @ParameterizedTest + @EnumSource(value = KeyType.class, names = { + "HybridXWingKey", + "HybridSecp256r1MLKEM768Key", + "HybridSecp384r1MLKEM1024Key", + "MLKEM768Key", + "MLKEM1024Key"}) + void generatedKeyPairRoundTrips(KeyType keyType) { + KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); + + assertTrue(kp.publicKeyPEM.startsWith("-----BEGIN PUBLIC KEY-----"), "public PEM header"); + assertTrue(kp.privateKeyPEM.startsWith("-----BEGIN PRIVATE KEY-----"), "private PEM header"); + + byte[] wrapped = HybridCrypto.wrapDEK(keyType, kp.publicKeyPEM, DEK); + byte[] unwrapped = HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, wrapped); + assertArrayEquals(DEK, unwrapped, "DEK round-trip via generated keypair"); + } +} diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java index dec68a333..3e3b2fa9c 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java @@ -18,6 +18,8 @@ import io.opentdf.platform.kas.RewrapRequest; import io.opentdf.platform.kas.RewrapResponse; import io.opentdf.platform.sdk.SDK.KasBadRequestException; +import io.opentdf.platform.sdk.spi.KemProvider; +import io.opentdf.platform.sdk.spi.KemProviders; import okhttp3.OkHttpClient; import org.slf4j.Logger; @@ -47,8 +49,8 @@ class KASClient implements SDK.KAS { private final BiFunction protocolClientFactory; private final boolean usePlaintext; private final JWSSigner signer; - private AsymDecryption decryptor; - private String clientPublicKey; + private volatile AsymDecryption decryptor; + private volatile String clientPublicKey; private KASKeyCache kasKeyCache; private static final Logger log = LoggerFactory.getLogger(KASClient.class); @@ -118,25 +120,86 @@ static class RewrapRequestBody { private static final Gson gson = new Gson(); - @Override - public byte[] unwrap(Manifest.KeyAccess keyAccess, String policy, KeyType sessionKeyType) { - ECKeyPair ecKeyPair = null; + /** Session-key material generated for a single unwrap() call: the PEM to send to KAS, plus whichever private key is needed to process the response. */ + private static final class SessionKeyMaterial { + final String publicKeyPem; + final ECKeyPair ecKeyPair; + final KemProvider.KeyPairPem kemKeyPair; + + SessionKeyMaterial(String publicKeyPem, ECKeyPair ecKeyPair, KemProvider.KeyPairPem kemKeyPair) { + this.publicKeyPem = publicKeyPem; + this.ecKeyPair = ecKeyPair; + this.kemKeyPair = kemKeyPair; + } + } + + private SessionKeyMaterial generateSessionKeyMaterial(KeyType sessionKeyType) { if (sessionKeyType.isEc()) { var curve = sessionKeyType.getECCurve(); - ecKeyPair = new ECKeyPair(curve); - clientPublicKey = ecKeyPair.publicKeyInPEMFormat(); - } else { - // Initialize the RSA key pair only once and reuse it for future unwrap operations - if (decryptor == null) { - var encryptionKeypair = CryptoUtils.generateRSAKeypair(); - decryptor = new AsymDecryption(encryptionKeypair.getPrivate()); - clientPublicKey = CryptoUtils.getRSAPublicKeyPEM(encryptionKeypair.getPublic()); + ECKeyPair ecKeyPair = new ECKeyPair(curve); + return new SessionKeyMaterial(ecKeyPair.publicKeyInPEMFormat(), ecKeyPair, null); + } + if (sessionKeyType.isMLKEM()) { + KemProvider.KeyPairPem kemKeyPair = KemProviders.get(sessionKeyType).generateKeyPair(sessionKeyType); + return new SessionKeyMaterial(kemKeyPair.publicKeyPEM, null, kemKeyPair); + } + // Initialize the RSA key pair only once and reuse it for future unwrap + // operations. Double-checked locking: decryptor/clientPublicKey are + // volatile so the initializing thread's write is visible to others, + // and the synchronized block keeps two concurrent first-callers from + // generating distinct keypairs and racing to overwrite each other's + // fields (which would send one generation's public key while + // decrypting with another's private key). + if (decryptor == null) { + synchronized (this) { + if (decryptor == null) { + var encryptionKeypair = CryptoUtils.generateRSAKeypair(); + decryptor = new AsymDecryption(encryptionKeypair.getPrivate()); + clientPublicKey = CryptoUtils.getRSAPublicKeyPEM(encryptionKeypair.getPublic()); + } } } + return new SessionKeyMaterial(clientPublicKey, null, null); + } + + private byte[] unwrapResponseKey( + KeyType sessionKeyType, SessionKeyMaterial keyMaterial, RewrapResponse response, byte[] wrappedKey) { + if (sessionKeyType.isEc()) { + if (keyMaterial.ecKeyPair == null) { + throw new SDKException("ECKeyPair is null. Unable to proceed with the unwrap operation."); + } + + var kasEphemeralPublicKey = response.getSessionPublicKey(); + ECPublicKey publicKey; + try { + publicKey = ECKeyPair.publicKeyFromPem(kasEphemeralPublicKey); + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new SDKException("error decoding KAS session public key", e); + } + byte[] symKey = ECKeyPair.computeECDHKey(publicKey, keyMaterial.ecKeyPair.getPrivateKey()); + + var sessionKey = ECKeyPair.calculateHKDF(GLOBAL_KEY_SALT, symKey); + + AesGcm gcm = new AesGcm(sessionKey); + AesGcm.Encrypted encrypted = new AesGcm.Encrypted(wrappedKey); + return gcm.decrypt(encrypted); + } + if (sessionKeyType.isMLKEM()) { + if (keyMaterial.kemKeyPair == null) { + throw new SDKException("KEM keypair is null. Unable to proceed with the unwrap operation."); + } + return KemProviders.get(sessionKeyType).unwrapDEK(sessionKeyType, keyMaterial.kemKeyPair.privateKeyPEM, wrappedKey); + } + return decryptor.decrypt(wrappedKey); + } + + @Override + public byte[] unwrap(Manifest.KeyAccess keyAccess, String policy, KeyType sessionKeyType) { + SessionKeyMaterial keyMaterial = generateSessionKeyMaterial(sessionKeyType); RewrapRequestBody body = new RewrapRequestBody(); body.policy = policy; - body.clientPublicKey = clientPublicKey; + body.clientPublicKey = keyMaterial.publicKeyPem; body.keyAccess = keyAccess; var requestBody = gson.toJson(body); @@ -171,29 +234,7 @@ public byte[] unwrap(Manifest.KeyAccess keyAccess, String policy, KeyType sessi } var wrappedKey = response.getEntityWrappedKey().toByteArray(); - if (sessionKeyType.isEc()) { - - if (ecKeyPair == null) { - throw new SDKException("ECKeyPair is null. Unable to proceed with the unwrap operation."); - } - - var kasEphemeralPublicKey = response.getSessionPublicKey(); - ECPublicKey publicKey; - try { - publicKey = ECKeyPair.publicKeyFromPem(kasEphemeralPublicKey); - } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { - throw new SDKException("error decoding KAS session public key", e); - } - byte[] symKey = ECKeyPair.computeECDHKey(publicKey, ecKeyPair.getPrivateKey()); - - var sessionKey = ECKeyPair.calculateHKDF(GLOBAL_KEY_SALT, symKey); - - AesGcm gcm = new AesGcm(sessionKey); - AesGcm.Encrypted encrypted = new AesGcm.Encrypted(wrappedKey); - return gcm.decrypt(encrypted); - } else { - return decryptor.decrypt(wrappedKey); - } + return unwrapResponseKey(sessionKeyType, keyMaterial, response, wrappedKey); } private final HashMap stubs = new HashMap<>(); diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java b/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java index aa90c0da2..06772c1c7 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java @@ -47,4 +47,26 @@ public interface KemProvider { * decap path; the production decrypt flow defers unwrap to the KAS. */ byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped); + + /** + * Generate a fresh ephemeral keypair for {@code keyType}. Used for the + * client's rewrap "session key" — the ephemeral key a client generates + * and sends with a rewrap request so KAS can wrap the response DEK back + * to it — as opposed to a KAS-managed wrapping key (see {@link #wrapDEK}). + * + * @param keyType hybrid or pure-PQC algorithm; must be a member of {@link #supportedKeyTypes()} + * @return the generated keypair, PEM-encoded + */ + KeyPairPem generateKeyPair(KeyType keyType); + + /** A generated KEM keypair: SPKI public key PEM and PKCS#8 private key PEM. */ + final class KeyPairPem { + public final String publicKeyPEM; + public final String privateKeyPEM; + + public KeyPairPem(String publicKeyPEM, String privateKeyPEM) { + this.publicKeyPEM = publicKeyPEM; + this.privateKeyPEM = privateKeyPEM; + } + } } From 0a6f852323fd61b15f82071377746a7cb291e0ef Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 26 Oct 2026 13:24:10 -0400 Subject: [PATCH 2/6] fix(cmdline): expect session-key-mlkem only when an ML-KEM provider is present The mavenverify job runs the cmdline tests only in its fips step (`mvn test -P 'fips,!non-fips'`); the non-fips coverage step builds just the sdk module. The fips profile leaves sdk-pqc-bc off cmdline's classpath, so `supports` correctly omits session-key-mlkem there, but CommandTest expected it unconditionally and failed supports_noArgs_listsFeatures_exits0 and supports_noArgs_json_listsFeatures. Make the expectation depend on whether sdk-pqc-bc's BouncyCastleKemProvider class is on the test classpath. The probe is a plain class lookup, not KemProviders, so a provider jar whose ServiceLoader registration is broken still fails the test. Also add a direct `supports session-key-mlkem` exit-code test, the probe the opentdf/tests xtest suite relies on. Signed-off-by: Dave Mihalcik --- .../java/io/opentdf/platform/CommandTest.java | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/cmdline/src/test/java/io/opentdf/platform/CommandTest.java b/cmdline/src/test/java/io/opentdf/platform/CommandTest.java index d58539ee8..8820bf328 100644 --- a/cmdline/src/test/java/io/opentdf/platform/CommandTest.java +++ b/cmdline/src/test/java/io/opentdf/platform/CommandTest.java @@ -14,6 +14,8 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; @@ -38,6 +40,33 @@ private String captureStdout(Runnable action) { return captured.toString(StandardCharsets.UTF_8); } + // session-key-mlkem is reported only when an ML-KEM KemProvider is on the + // classpath: the default (non-fips) profile puts sdk-pqc-bc on cmdline's + // runtime classpath, the fips profile leaves it off, and CI runs these + // tests under fips. Probe for the provider class directly rather than via + // KemProviders, so the expectation doesn't just mirror Supports' own check + // (a provider jar whose ServiceLoader registration broke would still fail). + private static final boolean MLKEM_PROVIDER_ON_CLASSPATH = + isClassPresent("io.opentdf.platform.sdk.pqc.bc.BouncyCastleKemProvider"); + + private static boolean isClassPresent(String className) { + try { + Class.forName(className, false, CommandTest.class.getClassLoader()); + return true; + } catch (ClassNotFoundException e) { + return false; + } + } + + private static List expectedFeatures() { + List features = new ArrayList<>( + List.of("dpop", "dpop_nonce_challenge", "partial_decrypt", "http_range_source")); + if (MLKEM_PROVIDER_ON_CLASSPATH) { + features.add("session-key-mlkem"); + } + return features; + } + @Test void supports_dpop_exits_0() { int code = new CommandLine(new Command()).execute("supports", "dpop"); @@ -56,6 +85,12 @@ void supports_partial_decrypt_and_http_range_source_exit_0() { assertThat(new CommandLine(new Command()).execute("supports", "http_range_source")).isEqualTo(0); } + @Test + void supports_session_key_mlkem_exits_0_only_with_mlkem_provider() { + int code = new CommandLine(new Command()).execute("supports", "session-key-mlkem"); + assertThat(code).isEqualTo(MLKEM_PROVIDER_ON_CLASSPATH ? 0 : 1); + } + @Test void supports_unknown_feature_exits_1() { int code = new CommandLine(new Command()).execute("supports", "unknown_feature"); @@ -107,8 +142,7 @@ void supports_noArgs_listsFeatures_exits0() { String out = captureStdout(() -> code[0] = new CommandLine(new Command()).execute("supports")); assertThat(code[0]).isEqualTo(0); - assertThat(out.lines()).containsExactlyInAnyOrder("dpop", "dpop_nonce_challenge", - "partial_decrypt", "http_range_source", "session-key-mlkem"); + assertThat(out.lines()).containsExactlyInAnyOrderElementsOf(expectedFeatures()); } @Test @@ -117,8 +151,11 @@ void supports_noArgs_json_listsFeatures() { String out = captureStdout(() -> code[0] = new CommandLine(new Command()).execute("supports", "--json")); assertThat(code[0]).isEqualTo(0); - assertThat(out.trim()).isEqualTo("{\"dpop\":true,\"dpop_nonce_challenge\":true," - + "\"partial_decrypt\":true,\"http_range_source\":true,\"session-key-mlkem\":true}"); + String expected = "{\"dpop\":true,\"dpop_nonce_challenge\":true," + + "\"partial_decrypt\":true,\"http_range_source\":true" + + (MLKEM_PROVIDER_ON_CLASSPATH ? ",\"session-key-mlkem\":true" : "") + + "}"; + assertThat(out.trim()).isEqualTo(expected); } @Test From 11a0d2d451a72f4f3f0cc8d8e2f111c3f5a84732 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 26 Oct 2026 13:25:45 -0400 Subject: [PATCH 3/6] fix(sdk): publish the RSA session keypair through one reference Review follow-up (Copilot; Arpan0995's issue 1). The double-checked locking added for the lazily generated RSA session keypair wrote the volatile `decryptor` before `clientPublicKey`. A thread arriving between the two writes saw a non-null decryptor, skipped the lock, and built a rewrap request with a null client public key. The window spans the PEM encoding of the public key, and only exists around first RSA use. Keep the decryptor and its public PEM in one immutable RsaSessionKey, published through a single AtomicReference, so any thread that sees the keypair sees both halves. Initialization still double-checks, but now on a dedicated lock instead of `this`, so a first-use RSA keygen no longer blocks getStub() and close(). The decryptor is carried in the per-call SessionKeyMaterial with the PEM it matches, rather than re-read from a field when the response is processed. Add a KASClient test that releases eight concurrent first-time RSA unwraps at once and asserts they all succeed and send one non-null public key. Without the lock it fails with eight different keys. It cannot reliably hit the old write-order window; the single-reference publish rules that out by construction. Signed-off-by: Dave Mihalcik --- .../io/opentdf/platform/sdk/KASClient.java | 69 ++++++++++++------ .../opentdf/platform/sdk/KASClientTest.java | 71 +++++++++++++++++++ 2 files changed, 117 insertions(+), 23 deletions(-) diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java index 3e3b2fa9c..f0caf67b6 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java @@ -34,6 +34,7 @@ import java.util.Date; import java.util.HashMap; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; import static io.opentdf.platform.sdk.TDF.GLOBAL_KEY_SALT; @@ -49,8 +50,11 @@ class KASClient implements SDK.KAS { private final BiFunction protocolClientFactory; private final boolean usePlaintext; private final JWSSigner signer; - private volatile AsymDecryption decryptor; - private volatile String clientPublicKey; + // RSA session keypair, generated on the first RSA unwrap() and reused after + // that. Both halves are published together through this one reference, so a + // thread can never see the private key without its matching public PEM. + private final AtomicReference rsaSessionKey = new AtomicReference<>(); + private final Object rsaSessionKeyLock = new Object(); private KASKeyCache kasKeyCache; private static final Logger log = LoggerFactory.getLogger(KASClient.class); @@ -125,41 +129,60 @@ private static final class SessionKeyMaterial { final String publicKeyPem; final ECKeyPair ecKeyPair; final KemProvider.KeyPairPem kemKeyPair; + final AsymDecryption rsaDecryptor; - SessionKeyMaterial(String publicKeyPem, ECKeyPair ecKeyPair, KemProvider.KeyPairPem kemKeyPair) { + SessionKeyMaterial(String publicKeyPem, ECKeyPair ecKeyPair, KemProvider.KeyPairPem kemKeyPair, + AsymDecryption rsaDecryptor) { this.publicKeyPem = publicKeyPem; this.ecKeyPair = ecKeyPair; this.kemKeyPair = kemKeyPair; + this.rsaDecryptor = rsaDecryptor; } } + /** The long-lived RSA session keypair: the decryptor for its private key and its public key as PEM. */ + private static final class RsaSessionKey { + final AsymDecryption decryptor; + final String publicKeyPem; + + RsaSessionKey(AsymDecryption decryptor, String publicKeyPem) { + this.decryptor = decryptor; + this.publicKeyPem = publicKeyPem; + } + } + + // Double-checked locking on a single reference: the fast path returns an + // already-published keypair; the lock keeps two concurrent first callers + // from generating different keypairs. + private RsaSessionKey getOrCreateRsaSessionKey() { + RsaSessionKey key = rsaSessionKey.get(); + if (key == null) { + synchronized (rsaSessionKeyLock) { + key = rsaSessionKey.get(); + if (key == null) { + var encryptionKeypair = CryptoUtils.generateRSAKeypair(); + key = new RsaSessionKey( + new AsymDecryption(encryptionKeypair.getPrivate()), + CryptoUtils.getRSAPublicKeyPEM(encryptionKeypair.getPublic())); + rsaSessionKey.set(key); + } + } + } + return key; + } + private SessionKeyMaterial generateSessionKeyMaterial(KeyType sessionKeyType) { if (sessionKeyType.isEc()) { var curve = sessionKeyType.getECCurve(); ECKeyPair ecKeyPair = new ECKeyPair(curve); - return new SessionKeyMaterial(ecKeyPair.publicKeyInPEMFormat(), ecKeyPair, null); + return new SessionKeyMaterial(ecKeyPair.publicKeyInPEMFormat(), ecKeyPair, null, null); } if (sessionKeyType.isMLKEM()) { KemProvider.KeyPairPem kemKeyPair = KemProviders.get(sessionKeyType).generateKeyPair(sessionKeyType); - return new SessionKeyMaterial(kemKeyPair.publicKeyPEM, null, kemKeyPair); - } - // Initialize the RSA key pair only once and reuse it for future unwrap - // operations. Double-checked locking: decryptor/clientPublicKey are - // volatile so the initializing thread's write is visible to others, - // and the synchronized block keeps two concurrent first-callers from - // generating distinct keypairs and racing to overwrite each other's - // fields (which would send one generation's public key while - // decrypting with another's private key). - if (decryptor == null) { - synchronized (this) { - if (decryptor == null) { - var encryptionKeypair = CryptoUtils.generateRSAKeypair(); - decryptor = new AsymDecryption(encryptionKeypair.getPrivate()); - clientPublicKey = CryptoUtils.getRSAPublicKeyPEM(encryptionKeypair.getPublic()); - } - } + return new SessionKeyMaterial(kemKeyPair.publicKeyPEM, null, kemKeyPair, null); } - return new SessionKeyMaterial(clientPublicKey, null, null); + RsaSessionKey rsaKey = getOrCreateRsaSessionKey(); + return new SessionKeyMaterial(rsaKey.publicKeyPem, null, null, rsaKey.decryptor); } private byte[] unwrapResponseKey( @@ -190,7 +213,7 @@ private byte[] unwrapResponseKey( } return KemProviders.get(sessionKeyType).unwrapDEK(sessionKeyType, keyMaterial.kemKeyPair.privateKeyPEM, wrappedKey); } - return decryptor.decrypt(wrappedKey); + return keyMaterial.rsaDecryptor.decrypt(wrappedKey); } @Override diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java index 013f3bc4d..f9a24a8ab 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java @@ -31,9 +31,18 @@ import java.io.IOException; import java.security.interfaces.RSAPublicKey; import java.text.ParseException; +import java.util.ArrayList; import java.util.Base64; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Random; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiFunction; import java.nio.charset.StandardCharsets; @@ -187,6 +196,68 @@ public void rewrap(RewrapRequest request, StreamObserver respons } } + @Test + void testConcurrentFirstRsaUnwrapsShareOneSessionKey() throws Exception { + var dpopKeypair = CryptoUtils.generateRSAKeypair(); + var dpopKey = new RSAKey.Builder((RSAPublicKey) dpopKeypair.getPublic()).privateKey(dpopKeypair.getPrivate()) + .build(); + var serverKeypair = CryptoUtils.generateRSAKeypair(); + List receivedClientKeys = Collections.synchronizedList(new ArrayList<>()); + AccessServiceGrpc.AccessServiceImplBase accessService = new AccessServiceGrpc.AccessServiceImplBase() { + @Override + public void rewrap(RewrapRequest request, StreamObserver responseObserver) { + try { + var requestBodyJson = SignedJWT.parse(request.getSignedRequestToken()) + .getJWTClaimsSet().getStringClaim("requestBody"); + var req = new Gson().fromJson(requestBodyJson, KASClient.RewrapRequestBody.class); + receivedClientKeys.add(req.clientPublicKey); + var dek = new AsymDecryption(serverKeypair.getPrivate()) + .decrypt(Base64.getDecoder().decode(req.keyAccess.wrappedKey)); + var rewrapped = new AsymEncryption(req.clientPublicKey).encrypt(dek); + responseObserver.onNext( + RewrapResponse.newBuilder().setEntityWrappedKey(ByteString.copyFrom(rewrapped)).build()); + responseObserver.onCompleted(); + } catch (Exception e) { + responseObserver.onError(e); + } + } + }; + + int callers = 8; + Server rewrapServer = startServer(accessService); + ExecutorService pool = Executors.newFixedThreadPool(callers); + try (var kas = new KASClient(httpClient, aclientFactory, new DefaultSrtSigner(dpopKey), true)) { + Manifest.KeyAccess keyAccess = new Manifest.KeyAccess(); + keyAccess.url = "http://localhost:" + rewrapServer.getPort(); + byte[] plaintextKey = new byte[32]; + new Random().nextBytes(plaintextKey); + keyAccess.wrappedKey = Base64.getEncoder().encodeToString( + new AsymEncryption(serverKeypair.getPublic()).encrypt(plaintextKey)); + + // Release all callers together so they race the lazy RSA keypair initialization. + CountDownLatch start = new CountDownLatch(1); + List> results = new ArrayList<>(); + for (int i = 0; i < callers; i++) { + results.add(pool.submit(() -> { + start.await(); + return kas.unwrap(keyAccess, "the policy", KeyType.RSA2048Key); + })); + } + start.countDown(); + for (Future result : results) { + assertThat(result.get(30, TimeUnit.SECONDS)).containsExactly(plaintextKey); + } + } finally { + pool.shutdownNow(); + rewrapServer.shutdownNow(); + } + + // Every request carried a public key, and all of them the same one. + assertThat(receivedClientKeys.size()).isEqualTo(callers); + assertThat(receivedClientKeys.stream().allMatch(Objects::nonNull)).isTrue(); + assertThat(new HashSet<>(receivedClientKeys).size()).isEqualTo(1); + } + @Test void testCustomSrtSignerIsUsed() throws IOException { var serverKeypair = CryptoUtils.generateRSAKeypair(); From a5e0a40c7f16aae076a5e34ec25e6e1c0fc38408 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 26 Oct 2026 13:27:48 -0400 Subject: [PATCH 4/6] fix(sdk): give KemProvider.generateKeyPair a default implementation Review follow-up (Copilot). KemProvider is a public SPI published in the sdk artifact, so a new abstract method breaks source compatibility for any external provider and leaves already-compiled ones open to AbstractMethodError. Make generateKeyPair a default method that throws UnsupportedOperationException naming the provider and key type. BouncyCastleKemProvider already overrides it. KASClient wraps that exception in an SDKException, so callers that request an ML-KEM session key see the SDK's usual exception type whether the provider is missing (KemProviders.get) or can't generate keypairs. Signed-off-by: Dave Mihalcik --- .../io/opentdf/platform/sdk/KASClient.java | 7 +++- .../opentdf/platform/sdk/spi/KemProvider.java | 12 +++++- .../platform/sdk/spi/KemProviderTest.java | 38 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java index f0caf67b6..2b679ab17 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java @@ -178,7 +178,12 @@ private SessionKeyMaterial generateSessionKeyMaterial(KeyType sessionKeyType) { return new SessionKeyMaterial(ecKeyPair.publicKeyInPEMFormat(), ecKeyPair, null, null); } if (sessionKeyType.isMLKEM()) { - KemProvider.KeyPairPem kemKeyPair = KemProviders.get(sessionKeyType).generateKeyPair(sessionKeyType); + KemProvider.KeyPairPem kemKeyPair; + try { + kemKeyPair = KemProviders.get(sessionKeyType).generateKeyPair(sessionKeyType); + } catch (UnsupportedOperationException e) { + throw new SDKException("cannot generate a " + sessionKeyType + " rewrap session key", e); + } return new SessionKeyMaterial(kemKeyPair.publicKeyPEM, null, kemKeyPair, null); } RsaSessionKey rsaKey = getOrCreateRsaSessionKey(); diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java b/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java index 06772c1c7..6eb807565 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java @@ -54,10 +54,20 @@ public interface KemProvider { * and sends with a rewrap request so KAS can wrap the response DEK back * to it — as opposed to a KAS-managed wrapping key (see {@link #wrapDEK}). * + *

The default implementation throws {@link UnsupportedOperationException}, + * so providers written before this method existed keep compiling and + * loading. They still serve {@link #wrapDEK} / {@link #unwrapDEK}; only a + * rewrap that asks for a session key of their type fails. Providers that + * can generate keypairs should override it. + * * @param keyType hybrid or pure-PQC algorithm; must be a member of {@link #supportedKeyTypes()} * @return the generated keypair, PEM-encoded + * @throws UnsupportedOperationException if this provider cannot generate keypairs */ - KeyPairPem generateKeyPair(KeyType keyType); + default KeyPairPem generateKeyPair(KeyType keyType) { + throw new UnsupportedOperationException( + getClass().getName() + " does not support generating " + keyType + " keypairs"); + } /** A generated KEM keypair: SPKI public key PEM and PKCS#8 private key PEM. */ final class KeyPairPem { diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java new file mode 100644 index 000000000..bab7969da --- /dev/null +++ b/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java @@ -0,0 +1,38 @@ +package io.opentdf.platform.sdk.spi; + +import io.opentdf.platform.sdk.KeyType; +import org.junit.jupiter.api.Test; + +import java.util.EnumSet; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class KemProviderTest { + + /** A provider written against the SPI before generateKeyPair existed: it implements only the original methods. */ + private static final class WrapOnlyProvider implements KemProvider { + @Override + public Set supportedKeyTypes() { + return EnumSet.of(KeyType.MLKEM768Key); + } + + @Override + public byte[] wrapDEK(KeyType keyType, String publicKeyPEM, byte[] dek) { + return dek; + } + + @Override + public byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped) { + return wrapped; + } + } + + @Test + void generateKeyPairDefaultsToUnsupported() { + assertThatThrownBy(() -> new WrapOnlyProvider().generateKeyPair(KeyType.MLKEM768Key)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining(WrapOnlyProvider.class.getName()) + .hasMessageContaining(KeyType.MLKEM768Key.toString()); + } +} From 6fab5007f63c0efae41cef90dd2146462c470fff Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 26 Oct 2026 13:30:10 -0400 Subject: [PATCH 5/6] chore(tests): add fail-closed tests for generated KEM session keypairs Review follow-up (Arpan0995's note 3). HybridCryptoGenerateKeyPairTest covered only the happy path. For every hybrid and pure ML-KEM key type, add three cases that must throw instead of returning a DEK: - unwrapping with a different generated keypair of the same type - a flipped byte inside the KEM ciphertext (ML-KEM implicit rejection gives a wrong wrap key, so AES-GCM fails) - a flipped byte in the AES-GCM sealed DEK The tampered envelopes are re-marshalled so they still parse, which exercises the decapsulation and AEAD paths, not the ASN.1 parser. The key-type list now comes from KeyType.isHybrid()/isMLKEM(), so a new PQC type can't skip these tests. Also add a KASClient test: with no KemProvider on the classpath (the fips deployment shape), requesting an ML-KEM session key fails with the actionable SDKException before any rewrap request is sent. Signed-off-by: Dave Mihalcik --- .../bc/HybridCryptoGenerateKeyPairTest.java | 63 ++++++++++++++++--- .../opentdf/platform/sdk/KASClientTest.java | 17 +++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java index 52740be43..7e54ce3ad 100644 --- a/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java +++ b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java @@ -1,13 +1,17 @@ package io.opentdf.platform.sdk.pqc.bc; import io.opentdf.platform.sdk.KeyType; +import io.opentdf.platform.sdk.SDKException; import io.opentdf.platform.sdk.spi.KemProvider; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.MethodSource; import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -18,18 +22,20 @@ *

Each variant is exercised end-to-end through the same PEM-based * {@link KemProvider} contract KASClient uses: generate a fresh keypair, wrap * a DEK against its public PEM, unwrap against its private PEM, assert equal. + * The negative cases check that the generated keypair and envelope fail + * closed: unwrapping must throw rather than return a DEK. */ class HybridCryptoGenerateKeyPairTest { private static final byte[] DEK = "0123456789abcdef0123456789abcdef".getBytes(StandardCharsets.UTF_8); + /** Every hybrid and pure ML-KEM key type, so a newly added PQC type can't skip these tests. */ + static Stream pqcKeyTypes() { + return Arrays.stream(KeyType.values()).filter(kt -> kt.isHybrid() || kt.isMLKEM()); + } + @ParameterizedTest - @EnumSource(value = KeyType.class, names = { - "HybridXWingKey", - "HybridSecp256r1MLKEM768Key", - "HybridSecp384r1MLKEM1024Key", - "MLKEM768Key", - "MLKEM1024Key"}) + @MethodSource("pqcKeyTypes") void generatedKeyPairRoundTrips(KeyType keyType) { KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); @@ -40,4 +46,47 @@ void generatedKeyPairRoundTrips(KeyType keyType) { byte[] unwrapped = HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, wrapped); assertArrayEquals(DEK, unwrapped, "DEK round-trip via generated keypair"); } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void otherKeyPairOfSameTypeCannotUnwrap(KeyType keyType) { + KemProvider.KeyPairPem recipient = HybridCrypto.generateKeyPair(keyType); + KemProvider.KeyPairPem other = HybridCrypto.generateKeyPair(keyType); + + byte[] wrapped = HybridCrypto.wrapDEK(keyType, recipient.publicKeyPEM, DEK); + assertThrows(SDKException.class, + () -> HybridCrypto.unwrapDEK(keyType, other.privateKeyPEM, wrapped), + "a different keypair of the same type must not unwrap the DEK"); + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void tamperedKemCiphertextCannotUnwrap(KeyType keyType) { + KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); + byte[][] parts = HybridCrypto.unmarshalEnvelope(HybridCrypto.wrapDEK(keyType, kp.publicKeyPEM, DEK)); + + // The midpoint of the KEM ciphertext is inside its ML-KEM part for every type + // (the EC / X25519 share, where present, is appended after it). Decapsulation + // implicitly rejects and yields a wrong wrap key, so AES-GCM must fail. + parts[0][parts[0].length / 2] ^= 0x01; + byte[] tampered = HybridCrypto.marshalEnvelope(parts[0], parts[1]); + + assertThrows(SDKException.class, + () -> HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, tampered), + "a flipped byte in the KEM ciphertext must not unwrap the DEK"); + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void tamperedEncryptedDekCannotUnwrap(KeyType keyType) { + KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); + byte[][] parts = HybridCrypto.unmarshalEnvelope(HybridCrypto.wrapDEK(keyType, kp.publicKeyPEM, DEK)); + + parts[1][parts[1].length / 2] ^= 0x01; + byte[] tampered = HybridCrypto.marshalEnvelope(parts[0], parts[1]); + + assertThrows(SDKException.class, + () -> HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, tampered), + "a flipped byte in the AES-GCM sealed DEK must not unwrap"); + } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java index f9a24a8ab..107dc608a 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/KASClientTest.java @@ -258,6 +258,23 @@ public void rewrap(RewrapRequest request, StreamObserver respons assertThat(new HashSet<>(receivedClientKeys).size()).isEqualTo(1); } + @Test + void testMlkemSessionKeyWithoutProviderFailsBeforeRewrap() { + // sdk's own test classpath has no KemProvider (see KemProvidersTest), which is + // the fips deployment shape. Asking for an ML-KEM session key must fail with the + // actionable SDKException before any request is sent; the URL is never contacted. + var dpopKeypair = CryptoUtils.generateRSAKeypair(); + var dpopKey = new RSAKey.Builder((RSAPublicKey) dpopKeypair.getPublic()).privateKey(dpopKeypair.getPrivate()) + .build(); + try (var kas = new KASClient(httpClient, aclientFactory, new DefaultSrtSigner(dpopKey), true)) { + Manifest.KeyAccess keyAccess = new Manifest.KeyAccess(); + keyAccess.url = "http://localhost:1"; + var thrown = assertThrows(SDKException.class, + () -> kas.unwrap(keyAccess, "the policy", KeyType.MLKEM768Key)); + assertThat(thrown.getMessage()).contains("sdk-pqc-bc"); + } + } + @Test void testCustomSrtSignerIsUsed() throws IOException { var serverKeypair = CryptoUtils.generateRSAKeypair(); From 2454b823e93364825c6ae097a2fdc4497c6469ca Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 26 Oct 2026 17:31:21 -0400 Subject: [PATCH 6/6] refactor(sdk): keep KEM session private keys inside the provider Replace KemProvider.generateKeyPair / KeyPairPem with generateSessionKey, which returns a KemSessionKey. It exposes only the public key PEM, unwraps the rewrap response itself, and erases its private key on close(). KASClient closes it once the rewrap finishes. The private key used to round-trip through a PKCS#8 PEM String only to be parsed back into raw bytes for the unwrap. It now stays as raw bytes owned by the provider and is zeroed after use. generateKeyPair and KeyPairPem were added by this PR and never released, so nothing else depends on them. unwrapDEK(String) is unchanged; it has no production callers left. `supports session-key-mlkem` now probes by generating ML-KEM-768 and ML-KEM-1024 session keys, so a provider that is registered for those types but cannot generate session keys no longer makes it report support. Signed-off-by: Dave Mihalcik --- .../java/io/opentdf/platform/Command.java | 25 +++- .../sdk/pqc/bc/BouncyCastleKemProvider.java | 4 +- .../sdk/pqc/bc/BouncyCastleKemSessionKey.java | 55 +++++++++ .../platform/sdk/pqc/bc/HybridCrypto.java | 23 ++-- .../platform/sdk/pqc/bc/XWingKeyPair.java | 5 + .../bc/HybridCryptoGenerateKeyPairTest.java | 92 -------------- .../pqc/bc/HybridCryptoSessionKeyTest.java | 116 ++++++++++++++++++ .../io/opentdf/platform/sdk/KASClient.java | 39 ++++-- .../opentdf/platform/sdk/spi/KemProvider.java | 48 +++++--- .../platform/sdk/spi/KemProviderTest.java | 6 +- 10 files changed, 274 insertions(+), 139 deletions(-) create mode 100644 sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemSessionKey.java delete mode 100644 sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java create mode 100644 sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoSessionKeyTest.java diff --git a/cmdline/src/main/java/io/opentdf/platform/Command.java b/cmdline/src/main/java/io/opentdf/platform/Command.java index 350fd72fe..c4953843e 100644 --- a/cmdline/src/main/java/io/opentdf/platform/Command.java +++ b/cmdline/src/main/java/io/opentdf/platform/Command.java @@ -47,6 +47,8 @@ import io.opentdf.platform.sdk.KeyType; import io.opentdf.platform.sdk.SDK; import io.opentdf.platform.sdk.SDKBuilder; +import io.opentdf.platform.sdk.SDKException; +import io.opentdf.platform.sdk.spi.KemProvider; import io.opentdf.platform.sdk.spi.KemProviders; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.config.Configurator; @@ -85,18 +87,31 @@ static class Supports implements Callable { private static final List STATIC_FEATURES = List.of("dpop", "dpop_nonce_challenge", "partial_decrypt", "http_range_source"); - // session-key-mlkem additionally requires an ML-KEM KemProvider on the - // classpath (e.g. sdk-pqc-bc), which the fips Maven profile excludes. - // Check the live registry rather than assuming build-profile wiring, so - // this doesn't claim support on a classpath that would actually throw. + // session-key-mlkem additionally requires a KemProvider that can generate + // ML-KEM-768 and ML-KEM-1024 session keys (e.g. sdk-pqc-bc), which the + // fips Maven profile excludes. Probe by generating one of each rather + // than checking the registry: a provider can be registered for these + // types yet only wrap and unwrap, since generateSessionKey defaults to + // unsupported. private static List features() { List features = new ArrayList<>(STATIC_FEATURES); - if (KemProviders.registered().contains(KeyType.MLKEM768Key)) { + if (canGenerateSessionKey(KeyType.MLKEM768Key) && canGenerateSessionKey(KeyType.MLKEM1024Key)) { features.add("session-key-mlkem"); } return features; } + private static boolean canGenerateSessionKey(KeyType keyType) { + KemProvider.KemSessionKey probe; + try { + probe = KemProviders.get(keyType).generateSessionKey(keyType); + } catch (SDKException | UnsupportedOperationException e) { + return false; + } + probe.close(); + return true; + } + @CommandLine.Parameters(index = "0", arity = "0..1", description = "Feature to check (e.g., dpop). Omit to list all supported features.") private String feature; diff --git a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java index 1ca268b96..68195e8be 100644 --- a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java +++ b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemProvider.java @@ -53,7 +53,7 @@ public byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped) { } @Override - public KeyPairPem generateKeyPair(KeyType keyType) { - return HybridCrypto.generateKeyPair(keyType); + public KemSessionKey generateSessionKey(KeyType keyType) { + return HybridCrypto.generateSessionKey(keyType); } } diff --git a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemSessionKey.java b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemSessionKey.java new file mode 100644 index 000000000..bc4395e47 --- /dev/null +++ b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/BouncyCastleKemSessionKey.java @@ -0,0 +1,55 @@ +package io.opentdf.platform.sdk.pqc.bc; + +import io.opentdf.platform.sdk.spi.KemProvider; + +import java.util.Arrays; + +/** + * {@link KemProvider.KemSessionKey} returned by {@link BouncyCastleKemProvider}. + * Owns the raw private key bytes (the same encoding the algorithm classes' + * {@code unwrapDEK(byte[] rawPriv, ...)} take) and zeroes them on {@link #close()}. + * The private key is never PEM-encoded or turned into a {@code String}. + * + *

BouncyCastle's key parameter objects, built inside each unwrap, keep + * their own copies that cannot be wiped; they are unreachable once the + * unwrap returns. + */ +final class BouncyCastleKemSessionKey implements KemProvider.KemSessionKey { + + /** One algorithm's raw-key unwrap, e.g. {@code MLKEMAlgorithm.MLKEM_768::unwrapDEK}. */ + @FunctionalInterface + interface RawUnwrap { + byte[] unwrapDEK(byte[] rawPrivateKey, byte[] wrapped); + } + + private final String publicKeyPEM; + private final byte[] privateKey; + private final RawUnwrap unwrap; + private boolean closed; + + /** Takes ownership of {@code privateKey}: the caller must not keep or reuse the array. */ + BouncyCastleKemSessionKey(String publicKeyPEM, byte[] privateKey, RawUnwrap unwrap) { + this.publicKeyPEM = publicKeyPEM; + this.privateKey = privateKey; + this.unwrap = unwrap; + } + + @Override + public String publicKeyPEM() { + return publicKeyPEM; + } + + @Override + public synchronized byte[] unwrapDEK(byte[] wrapped) { + if (closed) { + throw new IllegalStateException("KEM session key has been closed"); + } + return unwrap.unwrapDEK(privateKey, wrapped); + } + + @Override + public synchronized void close() { + Arrays.fill(privateKey, (byte) 0); + closed = true; + } +} diff --git a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java index cbdf1c53e..7f092a217 100644 --- a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java +++ b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/HybridCrypto.java @@ -98,30 +98,37 @@ static byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped) { } /** - * Generate a fresh ephemeral keypair for {@code keyType}. Same dispatch - * table as {@link #wrapDEK} / {@link #unwrapDEK}. + * Generate a fresh ephemeral rewrap session key for {@code keyType}. Same + * dispatch table as {@link #wrapDEK} / {@link #unwrapDEK}. The raw private + * key goes straight into the returned {@link BouncyCastleKemSessionKey}; + * it is never PEM-encoded. */ - static KemProvider.KeyPairPem generateKeyPair(KeyType keyType) { + static KemProvider.KemSessionKey generateSessionKey(KeyType keyType) { switch (keyType) { case HybridXWingKey: { XWingKeyPair kp = XWingKeyPair.generate(); - return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + return new BouncyCastleKemSessionKey(kp.publicKeyInPemFormat(), kp.getPrivateKey(), + XWingKeyPair::unwrapDEK); } case HybridSecp256r1MLKEM768Key: { HybridNISTKeyPair kp = HybridNISTAlgorithm.P256_MLKEM768.generate(); - return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + return new BouncyCastleKemSessionKey(kp.publicKeyInPemFormat(), kp.getPrivateKey(), + HybridNISTAlgorithm.P256_MLKEM768::unwrapDEK); } case HybridSecp384r1MLKEM1024Key: { HybridNISTKeyPair kp = HybridNISTAlgorithm.P384_MLKEM1024.generate(); - return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + return new BouncyCastleKemSessionKey(kp.publicKeyInPemFormat(), kp.getPrivateKey(), + HybridNISTAlgorithm.P384_MLKEM1024::unwrapDEK); } case MLKEM768Key: { MLKEMKeyPair kp = MLKEMAlgorithm.MLKEM_768.generate(); - return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + return new BouncyCastleKemSessionKey(kp.publicKeyInPemFormat(), kp.getPrivateKey(), + MLKEMAlgorithm.MLKEM_768::unwrapDEK); } case MLKEM1024Key: { MLKEMKeyPair kp = MLKEMAlgorithm.MLKEM_1024.generate(); - return new KemProvider.KeyPairPem(kp.publicKeyInPemFormat(), kp.privateKeyInPemFormat()); + return new BouncyCastleKemSessionKey(kp.publicKeyInPemFormat(), kp.getPrivateKey(), + MLKEMAlgorithm.MLKEM_1024::unwrapDEK); } default: throw new SDKException("unsupported PQC key type: " + keyType); diff --git a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/XWingKeyPair.java b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/XWingKeyPair.java index c870cd7c0..b95316cc5 100644 --- a/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/XWingKeyPair.java +++ b/sdk-pqc-bc/src/main/java/io/opentdf/platform/sdk/pqc/bc/XWingKeyPair.java @@ -62,6 +62,11 @@ public String privateKeyInPemFormat() { return HybridSpki.encodePkcs8Pem(HybridSpki.OID_XWING, privateKey); } + /** A copy of the raw 32-byte private key seed, as {@link #unwrapDEK(byte[], byte[])} takes it. */ + byte[] getPrivateKey() { + return privateKey.clone(); + } + public static byte[] pubKeyFromPem(String pem) { byte[] raw = HybridSpki.decodeSpkiPem(pem, HybridSpki.OID_XWING); if (raw.length != PUBLIC_KEY_SIZE) { diff --git a/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java deleted file mode 100644 index 7e54ce3ad..000000000 --- a/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoGenerateKeyPairTest.java +++ /dev/null @@ -1,92 +0,0 @@ -package io.opentdf.platform.sdk.pqc.bc; - -import io.opentdf.platform.sdk.KeyType; -import io.opentdf.platform.sdk.SDKException; -import io.opentdf.platform.sdk.spi.KemProvider; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; - -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.stream.Stream; - -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -/** - * Unit tests for {@link HybridCrypto#generateKeyPair}, the dispatcher backing - * {@link BouncyCastleKemProvider#generateKeyPair} and (in the core {@code sdk} - * module) {@code KASClient}'s KEM branch for the rewrap client "session key". - * - *

Each variant is exercised end-to-end through the same PEM-based - * {@link KemProvider} contract KASClient uses: generate a fresh keypair, wrap - * a DEK against its public PEM, unwrap against its private PEM, assert equal. - * The negative cases check that the generated keypair and envelope fail - * closed: unwrapping must throw rather than return a DEK. - */ -class HybridCryptoGenerateKeyPairTest { - - private static final byte[] DEK = "0123456789abcdef0123456789abcdef".getBytes(StandardCharsets.UTF_8); - - /** Every hybrid and pure ML-KEM key type, so a newly added PQC type can't skip these tests. */ - static Stream pqcKeyTypes() { - return Arrays.stream(KeyType.values()).filter(kt -> kt.isHybrid() || kt.isMLKEM()); - } - - @ParameterizedTest - @MethodSource("pqcKeyTypes") - void generatedKeyPairRoundTrips(KeyType keyType) { - KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); - - assertTrue(kp.publicKeyPEM.startsWith("-----BEGIN PUBLIC KEY-----"), "public PEM header"); - assertTrue(kp.privateKeyPEM.startsWith("-----BEGIN PRIVATE KEY-----"), "private PEM header"); - - byte[] wrapped = HybridCrypto.wrapDEK(keyType, kp.publicKeyPEM, DEK); - byte[] unwrapped = HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, wrapped); - assertArrayEquals(DEK, unwrapped, "DEK round-trip via generated keypair"); - } - - @ParameterizedTest - @MethodSource("pqcKeyTypes") - void otherKeyPairOfSameTypeCannotUnwrap(KeyType keyType) { - KemProvider.KeyPairPem recipient = HybridCrypto.generateKeyPair(keyType); - KemProvider.KeyPairPem other = HybridCrypto.generateKeyPair(keyType); - - byte[] wrapped = HybridCrypto.wrapDEK(keyType, recipient.publicKeyPEM, DEK); - assertThrows(SDKException.class, - () -> HybridCrypto.unwrapDEK(keyType, other.privateKeyPEM, wrapped), - "a different keypair of the same type must not unwrap the DEK"); - } - - @ParameterizedTest - @MethodSource("pqcKeyTypes") - void tamperedKemCiphertextCannotUnwrap(KeyType keyType) { - KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); - byte[][] parts = HybridCrypto.unmarshalEnvelope(HybridCrypto.wrapDEK(keyType, kp.publicKeyPEM, DEK)); - - // The midpoint of the KEM ciphertext is inside its ML-KEM part for every type - // (the EC / X25519 share, where present, is appended after it). Decapsulation - // implicitly rejects and yields a wrong wrap key, so AES-GCM must fail. - parts[0][parts[0].length / 2] ^= 0x01; - byte[] tampered = HybridCrypto.marshalEnvelope(parts[0], parts[1]); - - assertThrows(SDKException.class, - () -> HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, tampered), - "a flipped byte in the KEM ciphertext must not unwrap the DEK"); - } - - @ParameterizedTest - @MethodSource("pqcKeyTypes") - void tamperedEncryptedDekCannotUnwrap(KeyType keyType) { - KemProvider.KeyPairPem kp = HybridCrypto.generateKeyPair(keyType); - byte[][] parts = HybridCrypto.unmarshalEnvelope(HybridCrypto.wrapDEK(keyType, kp.publicKeyPEM, DEK)); - - parts[1][parts[1].length / 2] ^= 0x01; - byte[] tampered = HybridCrypto.marshalEnvelope(parts[0], parts[1]); - - assertThrows(SDKException.class, - () -> HybridCrypto.unwrapDEK(keyType, kp.privateKeyPEM, tampered), - "a flipped byte in the AES-GCM sealed DEK must not unwrap"); - } -} diff --git a/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoSessionKeyTest.java b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoSessionKeyTest.java new file mode 100644 index 000000000..1390651c4 --- /dev/null +++ b/sdk-pqc-bc/src/test/java/io/opentdf/platform/sdk/pqc/bc/HybridCryptoSessionKeyTest.java @@ -0,0 +1,116 @@ +package io.opentdf.platform.sdk.pqc.bc; + +import io.opentdf.platform.sdk.KeyType; +import io.opentdf.platform.sdk.SDKException; +import io.opentdf.platform.sdk.spi.KemProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Unit tests for {@link HybridCrypto#generateSessionKey}, the dispatcher backing + * {@link BouncyCastleKemProvider#generateSessionKey} and (in the core {@code sdk} + * module) {@code KASClient}'s KEM branch for the rewrap client "session key". + * + *

Each variant is exercised end-to-end the way KAS and KASClient use it: + * generate a session key, wrap a DEK against its public PEM, unwrap through + * the session key, assert equal. The negative cases check that the session + * key and envelope fail closed: unwrapping must throw rather than return a DEK. + */ +class HybridCryptoSessionKeyTest { + + private static final byte[] DEK = "0123456789abcdef0123456789abcdef".getBytes(StandardCharsets.UTF_8); + + /** Every hybrid and pure ML-KEM key type, so a newly added PQC type can't skip these tests. */ + static Stream pqcKeyTypes() { + return Arrays.stream(KeyType.values()).filter(kt -> kt.isHybrid() || kt.isMLKEM()); + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void generatedSessionKeyRoundTrips(KeyType keyType) { + try (KemProvider.KemSessionKey sessionKey = HybridCrypto.generateSessionKey(keyType)) { + assertTrue(sessionKey.publicKeyPEM().startsWith("-----BEGIN PUBLIC KEY-----"), "public PEM header"); + + byte[] wrapped = HybridCrypto.wrapDEK(keyType, sessionKey.publicKeyPEM(), DEK); + assertArrayEquals(DEK, sessionKey.unwrapDEK(wrapped), "DEK round-trip via generated session key"); + } + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void otherSessionKeyOfSameTypeCannotUnwrap(KeyType keyType) { + try (KemProvider.KemSessionKey recipient = HybridCrypto.generateSessionKey(keyType); + KemProvider.KemSessionKey other = HybridCrypto.generateSessionKey(keyType)) { + byte[] wrapped = HybridCrypto.wrapDEK(keyType, recipient.publicKeyPEM(), DEK); + assertThrows(SDKException.class, () -> other.unwrapDEK(wrapped), + "a different session key of the same type must not unwrap the DEK"); + } + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void tamperedKemCiphertextCannotUnwrap(KeyType keyType) { + try (KemProvider.KemSessionKey sessionKey = HybridCrypto.generateSessionKey(keyType)) { + byte[][] parts = HybridCrypto.unmarshalEnvelope( + HybridCrypto.wrapDEK(keyType, sessionKey.publicKeyPEM(), DEK)); + + // The midpoint of the KEM ciphertext is inside its ML-KEM part for every type + // (the EC / X25519 share, where present, is appended after it). Decapsulation + // implicitly rejects and yields a wrong wrap key, so AES-GCM must fail. + parts[0][parts[0].length / 2] ^= 0x01; + byte[] tampered = HybridCrypto.marshalEnvelope(parts[0], parts[1]); + + assertThrows(SDKException.class, () -> sessionKey.unwrapDEK(tampered), + "a flipped byte in the KEM ciphertext must not unwrap the DEK"); + } + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void tamperedEncryptedDekCannotUnwrap(KeyType keyType) { + try (KemProvider.KemSessionKey sessionKey = HybridCrypto.generateSessionKey(keyType)) { + byte[][] parts = HybridCrypto.unmarshalEnvelope( + HybridCrypto.wrapDEK(keyType, sessionKey.publicKeyPEM(), DEK)); + + parts[1][parts[1].length / 2] ^= 0x01; + byte[] tampered = HybridCrypto.marshalEnvelope(parts[0], parts[1]); + + assertThrows(SDKException.class, () -> sessionKey.unwrapDEK(tampered), + "a flipped byte in the AES-GCM sealed DEK must not unwrap"); + } + } + + @ParameterizedTest + @MethodSource("pqcKeyTypes") + void closedSessionKeyCannotUnwrap(KeyType keyType) { + KemProvider.KemSessionKey sessionKey = HybridCrypto.generateSessionKey(keyType); + byte[] wrapped = HybridCrypto.wrapDEK(keyType, sessionKey.publicKeyPEM(), DEK); + + sessionKey.close(); + + assertThrows(IllegalStateException.class, () -> sessionKey.unwrapDEK(wrapped), + "a closed session key must not unwrap"); + } + + @Test + void closeZeroesThePrivateKey() { + byte[] privateKey = {1, 2, 3, 4}; + BouncyCastleKemSessionKey sessionKey = + new BouncyCastleKemSessionKey("unused", privateKey, (priv, wrapped) -> priv.clone()); + assertArrayEquals(new byte[] {1, 2, 3, 4}, sessionKey.unwrapDEK(new byte[0]), + "unwrap is handed the private key"); + + sessionKey.close(); + + assertArrayEquals(new byte[4], privateKey, "close must zero the private key"); + } +} diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java index 2b679ab17..60fe9b305 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/KASClient.java @@ -124,20 +124,30 @@ static class RewrapRequestBody { private static final Gson gson = new Gson(); - /** Session-key material generated for a single unwrap() call: the PEM to send to KAS, plus whichever private key is needed to process the response. */ - private static final class SessionKeyMaterial { + /** + * Session-key material generated for a single unwrap() call: the PEM to send to KAS, plus whichever private key + * is needed to process the response. Closing it erases a KEM session key; the long-lived RSA key is untouched. + */ + private static final class SessionKeyMaterial implements AutoCloseable { final String publicKeyPem; final ECKeyPair ecKeyPair; - final KemProvider.KeyPairPem kemKeyPair; + final KemProvider.KemSessionKey kemSessionKey; final AsymDecryption rsaDecryptor; - SessionKeyMaterial(String publicKeyPem, ECKeyPair ecKeyPair, KemProvider.KeyPairPem kemKeyPair, + SessionKeyMaterial(String publicKeyPem, ECKeyPair ecKeyPair, KemProvider.KemSessionKey kemSessionKey, AsymDecryption rsaDecryptor) { this.publicKeyPem = publicKeyPem; this.ecKeyPair = ecKeyPair; - this.kemKeyPair = kemKeyPair; + this.kemSessionKey = kemSessionKey; this.rsaDecryptor = rsaDecryptor; } + + @Override + public void close() { + if (kemSessionKey != null) { + kemSessionKey.close(); + } + } } /** The long-lived RSA session keypair: the decryptor for its private key and its public key as PEM. */ @@ -178,13 +188,13 @@ private SessionKeyMaterial generateSessionKeyMaterial(KeyType sessionKeyType) { return new SessionKeyMaterial(ecKeyPair.publicKeyInPEMFormat(), ecKeyPair, null, null); } if (sessionKeyType.isMLKEM()) { - KemProvider.KeyPairPem kemKeyPair; + KemProvider.KemSessionKey kemSessionKey; try { - kemKeyPair = KemProviders.get(sessionKeyType).generateKeyPair(sessionKeyType); + kemSessionKey = KemProviders.get(sessionKeyType).generateSessionKey(sessionKeyType); } catch (UnsupportedOperationException e) { throw new SDKException("cannot generate a " + sessionKeyType + " rewrap session key", e); } - return new SessionKeyMaterial(kemKeyPair.publicKeyPEM, null, kemKeyPair, null); + return new SessionKeyMaterial(kemSessionKey.publicKeyPEM(), null, kemSessionKey, null); } RsaSessionKey rsaKey = getOrCreateRsaSessionKey(); return new SessionKeyMaterial(rsaKey.publicKeyPem, null, null, rsaKey.decryptor); @@ -213,18 +223,23 @@ private byte[] unwrapResponseKey( return gcm.decrypt(encrypted); } if (sessionKeyType.isMLKEM()) { - if (keyMaterial.kemKeyPair == null) { - throw new SDKException("KEM keypair is null. Unable to proceed with the unwrap operation."); + if (keyMaterial.kemSessionKey == null) { + throw new SDKException("KEM session key is null. Unable to proceed with the unwrap operation."); } - return KemProviders.get(sessionKeyType).unwrapDEK(sessionKeyType, keyMaterial.kemKeyPair.privateKeyPEM, wrappedKey); + return keyMaterial.kemSessionKey.unwrapDEK(wrappedKey); } return keyMaterial.rsaDecryptor.decrypt(wrappedKey); } @Override public byte[] unwrap(Manifest.KeyAccess keyAccess, String policy, KeyType sessionKeyType) { - SessionKeyMaterial keyMaterial = generateSessionKeyMaterial(sessionKeyType); + try (SessionKeyMaterial keyMaterial = generateSessionKeyMaterial(sessionKeyType)) { + return rewrap(keyAccess, policy, sessionKeyType, keyMaterial); + } + } + private byte[] rewrap(Manifest.KeyAccess keyAccess, String policy, KeyType sessionKeyType, + SessionKeyMaterial keyMaterial) { RewrapRequestBody body = new RewrapRequestBody(); body.policy = policy; body.clientPublicKey = keyMaterial.publicKeyPem; diff --git a/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java b/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java index 6eb807565..96729a982 100644 --- a/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java +++ b/sdk/src/main/java/io/opentdf/platform/sdk/spi/KemProvider.java @@ -49,34 +49,48 @@ public interface KemProvider { byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped); /** - * Generate a fresh ephemeral keypair for {@code keyType}. Used for the - * client's rewrap "session key" — the ephemeral key a client generates - * and sends with a rewrap request so KAS can wrap the response DEK back - * to it — as opposed to a KAS-managed wrapping key (see {@link #wrapDEK}). + * Generate a fresh ephemeral rewrap "session key" for {@code keyType} — the + * key a client generates and sends with a rewrap request so KAS can wrap + * the response DEK back to it — as opposed to a KAS-managed wrapping key + * (see {@link #wrapDEK}). * *

The default implementation throws {@link UnsupportedOperationException}, * so providers written before this method existed keep compiling and * loading. They still serve {@link #wrapDEK} / {@link #unwrapDEK}; only a * rewrap that asks for a session key of their type fails. Providers that - * can generate keypairs should override it. + * can generate session keys should override it. * * @param keyType hybrid or pure-PQC algorithm; must be a member of {@link #supportedKeyTypes()} - * @return the generated keypair, PEM-encoded - * @throws UnsupportedOperationException if this provider cannot generate keypairs + * @return the generated session key; the caller must {@link KemSessionKey#close() close} it + * @throws UnsupportedOperationException if this provider cannot generate session keys */ - default KeyPairPem generateKeyPair(KeyType keyType) { + default KemSessionKey generateSessionKey(KeyType keyType) { throw new UnsupportedOperationException( - getClass().getName() + " does not support generating " + keyType + " keypairs"); + getClass().getName() + " does not support generating " + keyType + " session keys"); } - /** A generated KEM keypair: SPKI public key PEM and PKCS#8 private key PEM. */ - final class KeyPairPem { - public final String publicKeyPEM; - public final String privateKeyPEM; + /** + * An ephemeral rewrap session key. Only the public half is exposed; the + * private half stays inside the provider, which uses it to unwrap the + * rewrap response and erases it on {@link #close()}. + * + *

Erasure covers the key material the provider holds. Copies made + * inside a crypto library while unwrapping may not be erasable; they + * become unreachable once {@link #unwrapDEK} returns. + */ + interface KemSessionKey extends AutoCloseable { + /** @return the public key to send to KAS, as an SPKI PEM */ + String publicKeyPEM(); + + /** + * Unwrap a DEK that KAS wrapped to {@link #publicKeyPEM()}. + * + * @throws IllegalStateException if this key has been closed + */ + byte[] unwrapDEK(byte[] wrapped); - public KeyPairPem(String publicKeyPEM, String privateKeyPEM) { - this.publicKeyPEM = publicKeyPEM; - this.privateKeyPEM = privateKeyPEM; - } + /** Erase the private key. Further {@link #unwrapDEK} calls fail. */ + @Override + void close(); } } diff --git a/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java b/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java index bab7969da..e043d4fee 100644 --- a/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java +++ b/sdk/src/test/java/io/opentdf/platform/sdk/spi/KemProviderTest.java @@ -10,7 +10,7 @@ class KemProviderTest { - /** A provider written against the SPI before generateKeyPair existed: it implements only the original methods. */ + /** A provider written against the SPI before generateSessionKey existed: it implements only the original methods. */ private static final class WrapOnlyProvider implements KemProvider { @Override public Set supportedKeyTypes() { @@ -29,8 +29,8 @@ public byte[] unwrapDEK(KeyType keyType, String privateKeyPEM, byte[] wrapped) { } @Test - void generateKeyPairDefaultsToUnsupported() { - assertThatThrownBy(() -> new WrapOnlyProvider().generateKeyPair(KeyType.MLKEM768Key)) + void generateSessionKeyDefaultsToUnsupported() { + assertThatThrownBy(() -> new WrapOnlyProvider().generateSessionKey(KeyType.MLKEM768Key)) .isInstanceOf(UnsupportedOperationException.class) .hasMessageContaining(WrapOnlyProvider.class.getName()) .hasMessageContaining(KeyType.MLKEM768Key.toString());