This is an automated email from the ASF dual-hosted git repository.
acosentino pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 88a34f9ea6ce CAMEL-PQC: Address Guillaume Nodet's review findings on
PQC stateful … (#22282)
88a34f9ea6ce is described below
commit 88a34f9ea6ce6febca07ec75d454c51dfa0ec1b5
Author: Andrea Cosentino <[email protected]>
AuthorDate: Thu Mar 26 16:40:05 2026 +0100
CAMEL-PQC: Address Guillaume Nodet's review findings on PQC stateful …
(#22282)
* CAMEL-PQC: Address Guillaume Nodet's review findings on PQC stateful key
tracking
Fix all 8 findings from post-merge review on PR #22264:
- Simplify redundant remaining<=0 to remaining==0 after remaining<0 check
- Health check reads runtime keyPair from producer instead of configuration
- Health check reports DOWN with warning detail when capacity below
threshold
- Validate statefulKeyWarningThreshold is between 0.0 and 1.0
- Guard null metadata in persistStatefulKeyStateAfterSign
- Add producer-level exhaustion test exercising the Camel route
- Extract duplicated instanceof dispatch into shared static helpers
- Add checkStatefulKeyBeforeSign to hybridSignature method
Signed-off-by: Andrea Cosentino <[email protected]>
* CAMEL-PQC: Address follow-up review from PR #22282
Fix 4 findings from Guillaume Nodet's review:
- Health check reports UP with warning=true detail (not DOWN) when key
capacity is below threshold but not exhausted
- Javadoc updated to match UP+warning behavior
- Null metadata in persistStatefulKeyStateAfterSign now logs a warning
about potential key reuse risk instead of silently skipping
- hybridSignature throws clause restored to specific checked exceptions;
persistStatefulKeyStateAfterSign wrapped with RuntimeCamelException
Signed-off-by: Andrea Cosentino <[email protected]>
---------
Signed-off-by: Andrea Cosentino <[email protected]>
---
.../camel/component/pqc/PQCConfiguration.java | 4 ++
.../apache/camel/component/pqc/PQCProducer.java | 64 +++++++++++++---------
.../component/pqc/PQCStatefulKeyHealthCheck.java | 54 +++++++++---------
.../component/pqc/PQCStatefulKeyTrackingTest.java | 40 ++++++++++++++
4 files changed, 108 insertions(+), 54 deletions(-)
diff --git
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCConfiguration.java
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCConfiguration.java
index bfac40af1cb2..d54f21e01d9f 100644
---
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCConfiguration.java
+++
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCConfiguration.java
@@ -358,6 +358,10 @@ public class PQCConfiguration implements Cloneable {
* to disable warnings.
*/
public void setStatefulKeyWarningThreshold(double
statefulKeyWarningThreshold) {
+ if (statefulKeyWarningThreshold < 0.0 || statefulKeyWarningThreshold >
1.0) {
+ throw new IllegalArgumentException(
+ "statefulKeyWarningThreshold must be between 0.0 and 1.0,
but was: " + statefulKeyWarningThreshold);
+ }
this.statefulKeyWarningThreshold = statefulKeyWarningThreshold;
}
diff --git
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java
index ed169ae65e7f..f684ad8a359f 100644
---
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java
+++
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java
@@ -31,6 +31,7 @@ import javax.crypto.spec.SecretKeySpec;
import org.apache.camel.Endpoint;
import org.apache.camel.Exchange;
import org.apache.camel.InvalidPayloadException;
+import org.apache.camel.RuntimeCamelException;
import org.apache.camel.component.pqc.crypto.hybrid.HybridKEM;
import org.apache.camel.component.pqc.crypto.hybrid.HybridSignature;
import org.apache.camel.component.pqc.lifecycle.KeyLifecycleManager;
@@ -119,6 +120,14 @@ public class PQCProducer extends DefaultProducer {
super(endpoint);
}
+ /**
+ * Returns the runtime key pair used by this producer. Used by the health
check to read actual key state rather than
+ * the configuration snapshot.
+ */
+ KeyPair getRuntimeKeyPair() {
+ return keyPair;
+ }
+
@Override
public void process(Exchange exchange) throws Exception {
PQCOperations operation = determineOperation(exchange);
@@ -397,7 +406,7 @@ public class PQCProducer extends DefaultProducer {
if (ObjectHelper.isNotEmpty(healthCheckRepository)) {
String id = getEndpoint().getId();
- producerHealthCheck = new PQCStatefulKeyHealthCheck(getEndpoint(),
id);
+ producerHealthCheck = new PQCStatefulKeyHealthCheck(getEndpoint(),
this, id);
producerHealthCheck.setEnabled(getEndpoint().getComponent().isHealthCheckProducerEnabled());
healthCheckRepository.addHealthCheck(producerHealthCheck);
}
@@ -501,6 +510,8 @@ public class PQCProducer extends DefaultProducer {
private void hybridSignature(Exchange exchange)
throws InvalidPayloadException, InvalidKeyException,
SignatureException {
+ checkStatefulKeyBeforeSign();
+
String payload = exchange.getMessage().getMandatoryBody(String.class);
byte[] data = payload.getBytes();
@@ -526,6 +537,12 @@ public class PQCProducer extends DefaultProducer {
exchange.getMessage().setHeader(PQCConstants.HYBRID_SIGNATURE,
hybridSig);
exchange.getMessage().setHeader(PQCConstants.CLASSICAL_SIGNATURE,
components.classicalSignature());
exchange.getMessage().setHeader(PQCConstants.PQC_SIGNATURE,
components.pqcSignature());
+
+ try {
+ persistStatefulKeyStateAfterSign(exchange);
+ } catch (Exception e) {
+ throw new RuntimeCamelException("Failed to persist stateful key
state after hybrid signing", e);
+ }
}
private void hybridVerification(Exchange exchange)
@@ -789,15 +806,9 @@ public class PQCProducer extends DefaultProducer {
}
PrivateKey privateKey = keyPair.getPrivate();
- long remaining;
+ long remaining = getStatefulKeyRemaining(privateKey);
- if (privateKey instanceof XMSSPrivateKey) {
- remaining = ((XMSSPrivateKey) privateKey).getUsagesRemaining();
- } else if (privateKey instanceof XMSSMTPrivateKey) {
- remaining = ((XMSSMTPrivateKey) privateKey).getUsagesRemaining();
- } else if (privateKey instanceof LMSPrivateKey) {
- remaining = ((LMSPrivateKey) privateKey).getUsagesRemaining();
- } else {
+ if (remaining < 0) {
throw new IllegalArgumentException(
"getRemainingSignatures is only supported for stateful
signature schemes (XMSS, XMSSMT, LMS/HSS). "
+ "Key type: " +
privateKey.getClass().getName());
@@ -812,23 +823,17 @@ public class PQCProducer extends DefaultProducer {
}
PrivateKey privateKey = keyPair.getPrivate();
- StatefulKeyState state;
+ long remaining = getStatefulKeyRemaining(privateKey);
- if (privateKey instanceof XMSSPrivateKey) {
- XMSSPrivateKey xmssKey = (XMSSPrivateKey) privateKey;
- state = new StatefulKeyState(privateKey.getAlgorithm(),
xmssKey.getIndex(), xmssKey.getUsagesRemaining());
- } else if (privateKey instanceof XMSSMTPrivateKey) {
- XMSSMTPrivateKey xmssmtKey = (XMSSMTPrivateKey) privateKey;
- state = new StatefulKeyState(privateKey.getAlgorithm(),
xmssmtKey.getIndex(), xmssmtKey.getUsagesRemaining());
- } else if (privateKey instanceof LMSPrivateKey) {
- LMSPrivateKey lmsKey = (LMSPrivateKey) privateKey;
- state = new StatefulKeyState(privateKey.getAlgorithm(),
lmsKey.getIndex(), lmsKey.getUsagesRemaining());
- } else {
+ if (remaining < 0) {
throw new IllegalArgumentException(
"getKeyState is only supported for stateful signature
schemes (XMSS, XMSSMT, LMS/HSS). "
+ "Key type: " +
privateKey.getClass().getName());
}
+ long index = getStatefulKeyIndex(privateKey);
+ StatefulKeyState state = new
StatefulKeyState(privateKey.getAlgorithm(), index, remaining);
+
exchange.getMessage().setHeader(PQCConstants.KEY_STATE, state);
}
@@ -863,7 +868,7 @@ public class PQCProducer extends DefaultProducer {
return;
}
- if (remaining <= 0) {
+ if (remaining == 0) {
throw new IllegalStateException(
"Stateful key (" + privateKey.getAlgorithm() + ") is
exhausted with 0 remaining signatures. "
+ "The key must not be reused —
generate a new key pair.");
@@ -913,11 +918,18 @@ public class PQCProducer extends DefaultProducer {
// Update metadata with current usage
KeyMetadata metadata = klm.getKeyMetadata(keyId);
- if (metadata != null) {
- metadata.updateLastUsed();
- klm.updateKeyMetadata(keyId, metadata);
+ if (metadata == null) {
+ LOG.warn(
+ "No metadata found for stateful key '{}'. The key index
has been advanced by the signing operation "
+ + "but cannot be persisted — on restart this index
advance will be lost, which may lead to key reuse. "
+ + "Ensure a KeyLifecycleManager is properly configured
and the key is stored with metadata.",
+ keyId);
+ return;
}
+ metadata.updateLastUsed();
+ klm.updateKeyMetadata(keyId, metadata);
+
// Persist the updated key (with new index) so state survives restarts
klm.storeKey(keyId, keyPair, metadata);
}
@@ -925,7 +937,7 @@ public class PQCProducer extends DefaultProducer {
/**
* Returns the remaining signatures for a stateful private key, or -1 if
the key is not stateful.
*/
- private long getStatefulKeyRemaining(PrivateKey privateKey) {
+ static long getStatefulKeyRemaining(PrivateKey privateKey) {
if (privateKey instanceof XMSSPrivateKey) {
return ((XMSSPrivateKey) privateKey).getUsagesRemaining();
} else if (privateKey instanceof XMSSMTPrivateKey) {
@@ -940,7 +952,7 @@ public class PQCProducer extends DefaultProducer {
* Returns the current index (number of signatures already produced) for a
stateful private key, or 0 if the key is
* not stateful.
*/
- private long getStatefulKeyIndex(PrivateKey privateKey) {
+ static long getStatefulKeyIndex(PrivateKey privateKey) {
if (privateKey instanceof XMSSPrivateKey) {
return ((XMSSPrivateKey) privateKey).getIndex();
} else if (privateKey instanceof XMSSMTPrivateKey) {
diff --git
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCStatefulKeyHealthCheck.java
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCStatefulKeyHealthCheck.java
index 23acc4e43a6c..5a7284dec2e9 100644
---
a/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCStatefulKeyHealthCheck.java
+++
b/components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCStatefulKeyHealthCheck.java
@@ -22,28 +22,29 @@ import java.util.Map;
import org.apache.camel.health.HealthCheckResultBuilder;
import org.apache.camel.impl.health.AbstractHealthCheck;
-import org.bouncycastle.pqc.jcajce.interfaces.LMSPrivateKey;
-import org.bouncycastle.pqc.jcajce.interfaces.XMSSMTPrivateKey;
-import org.bouncycastle.pqc.jcajce.interfaces.XMSSPrivateKey;
/**
* Health check that reports the state of stateful PQC signature keys (XMSS,
XMSSMT, LMS/HSS). These hash-based
- * signature schemes have a finite number of signatures. This health check
reports DOWN when a key is exhausted and
- * includes remaining signature capacity as a detail.
+ * signature schemes have a finite number of signatures. This health check
reports DOWN when a key is exhausted, UP with
+ * a {@code warning=true} detail when remaining signatures fall below the
warning threshold, and includes remaining
+ * signature capacity as a detail.
*/
public class PQCStatefulKeyHealthCheck extends AbstractHealthCheck {
private final PQCEndpoint endpoint;
+ private final PQCProducer producer;
- public PQCStatefulKeyHealthCheck(PQCEndpoint endpoint, String clientId) {
+ public PQCStatefulKeyHealthCheck(PQCEndpoint endpoint, PQCProducer
producer, String clientId) {
super("camel", "producer:pqc-stateful-key-" + clientId);
this.endpoint = endpoint;
+ this.producer = producer;
}
@Override
protected void doCall(HealthCheckResultBuilder builder, Map<String,
Object> options) {
- PQCConfiguration configuration = endpoint.getConfiguration();
- KeyPair keyPair = configuration.getKeyPair();
+ // Read key state from the producer's runtime keyPair, not from
configuration,
+ // to avoid stale state when the key has been rotated or updated at
runtime
+ KeyPair keyPair = producer.getRuntimeKeyPair();
if (keyPair == null || keyPair.getPrivate() == null) {
builder.detail("stateful_key", false);
@@ -52,23 +53,9 @@ public class PQCStatefulKeyHealthCheck extends
AbstractHealthCheck {
}
PrivateKey privateKey = keyPair.getPrivate();
- long remaining = -1;
- long index = 0;
String algorithm = privateKey.getAlgorithm();
-
- if (privateKey instanceof XMSSPrivateKey) {
- XMSSPrivateKey xmssKey = (XMSSPrivateKey) privateKey;
- remaining = xmssKey.getUsagesRemaining();
- index = xmssKey.getIndex();
- } else if (privateKey instanceof XMSSMTPrivateKey) {
- XMSSMTPrivateKey xmssmtKey = (XMSSMTPrivateKey) privateKey;
- remaining = xmssmtKey.getUsagesRemaining();
- index = xmssmtKey.getIndex();
- } else if (privateKey instanceof LMSPrivateKey) {
- LMSPrivateKey lmsKey = (LMSPrivateKey) privateKey;
- remaining = lmsKey.getUsagesRemaining();
- index = lmsKey.getIndex();
- }
+ long remaining = PQCProducer.getStatefulKeyRemaining(privateKey);
+ long index = PQCProducer.getStatefulKeyIndex(privateKey);
if (remaining < 0) {
// Not a stateful key - always healthy
@@ -78,24 +65,35 @@ public class PQCStatefulKeyHealthCheck extends
AbstractHealthCheck {
return;
}
+ long totalCapacity = index + remaining;
+
builder.detail("stateful_key", true);
builder.detail("algorithm", algorithm);
builder.detail("remaining_signatures", remaining);
builder.detail("signatures_used", index);
- builder.detail("total_capacity", index + remaining);
+ builder.detail("total_capacity", totalCapacity);
- if (remaining <= 0) {
+ if (remaining == 0) {
builder.message("Stateful key (" + algorithm + ") is exhausted
with 0 remaining signatures");
builder.down();
return;
}
- double threshold = configuration.getStatefulKeyWarningThreshold();
- long totalCapacity = index + remaining;
+ double threshold =
endpoint.getConfiguration().getStatefulKeyWarningThreshold();
if (threshold > 0 && totalCapacity > 0) {
double fractionRemaining = (double) remaining / totalCapacity;
builder.detail("fraction_remaining", String.format("%.4f",
fractionRemaining));
builder.detail("warning_threshold", String.valueOf(threshold));
+
+ if (fractionRemaining <= threshold) {
+ builder.message(
+ "Stateful key (" + algorithm + ") is approaching
exhaustion: " + remaining
+ + " signatures remaining out of " +
totalCapacity + " total ("
+ + String.format("%.1f%%", fractionRemaining *
100) + " remaining)");
+ builder.detail("warning", true);
+ builder.up();
+ return;
+ }
}
builder.up();
diff --git
a/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/PQCStatefulKeyTrackingTest.java
b/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/PQCStatefulKeyTrackingTest.java
index fc7c133e96dd..0bd8a1f53186 100644
---
a/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/PQCStatefulKeyTrackingTest.java
+++
b/components/camel-pqc/src/test/java/org/apache/camel/component/pqc/PQCStatefulKeyTrackingTest.java
@@ -25,6 +25,7 @@ import java.security.Security;
import java.security.Signature;
import org.apache.camel.BindToRegistry;
+import org.apache.camel.CamelExecutionException;
import org.apache.camel.EndpointInject;
import org.apache.camel.Produce;
import org.apache.camel.ProducerTemplate;
@@ -191,4 +192,43 @@ public class PQCStatefulKeyTrackingTest extends
CamelTestSupport {
assertEquals(0, state.getUsagesRemaining());
assertEquals(4, state.getIndex());
}
+
+ @Test
+ void testProducerExhaustionThrowsException() throws Exception {
+ // Sign 4 times to exhaust the key (XMSS height=2 allows exactly 4)
+ for (int i = 0; i < 4; i++) {
+ resultSigned.reset();
+ resultSigned.expectedMessageCount(1);
+ templateSign.sendBody("message" + i);
+ resultSigned.assertIsSatisfied();
+ }
+
+ // The 5th sign attempt through the producer should throw
IllegalStateException
+ CamelExecutionException ex =
assertThrows(CamelExecutionException.class,
+ () -> templateSign.sendBody("message4"));
+ assertInstanceOf(IllegalStateException.class, ex.getCause());
+ assertTrue(ex.getCause().getMessage().contains("exhausted"),
+ "Exception message should mention exhaustion");
+ }
+
+ @Test
+ void testWarningThresholdValidation() {
+ PQCConfiguration config = new PQCConfiguration();
+
+ // Valid values
+ config.setStatefulKeyWarningThreshold(0.0);
+ assertEquals(0.0, config.getStatefulKeyWarningThreshold());
+
+ config.setStatefulKeyWarningThreshold(0.5);
+ assertEquals(0.5, config.getStatefulKeyWarningThreshold());
+
+ config.setStatefulKeyWarningThreshold(1.0);
+ assertEquals(1.0, config.getStatefulKeyWarningThreshold());
+
+ // Invalid values
+ assertThrows(IllegalArgumentException.class,
+ () -> config.setStatefulKeyWarningThreshold(-0.1));
+ assertThrows(IllegalArgumentException.class,
+ () -> config.setStatefulKeyWarningThreshold(1.1));
+ }
}