oscerd commented on code in PR #28099:
URL: https://github.com/apache/camel/pull/28099#discussion_r4313366067
##########
components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java:
##########
@@ -1146,47 +1155,67 @@ private void checkStatefulKeyBeforeSign() {
}
/**
- * Persists stateful key state after a signing operation through the
KeyLifecycleManager, if configured. This
- * ensures the key index is tracked across restarts.
+ * Records the use of the signing key through the KeyLifecycleManager, if
one is configured and the exchange names a
+ * key.
+ * <p/>
+ * This advances {@code usageCount}, which is what the {@code maxKeyUsage}
rotation signal of
+ * {@link org.apache.camel.component.pqc.lifecycle.KeyRotationScheduler}
reads. It must therefore run for every key,
+ * not only the stateful ones: ML-DSA, SLH-DSA, Falcon and the KEM keys
would otherwise sit at a usage count of zero
+ * forever and never be rotated.
+ * <p/>
+ * For a stateful signature key the advanced private key is stored as
well, so the one-time index survives a
+ * restart.
*/
- private void persistStatefulKeyStateAfterSign(Exchange exchange) throws
Exception {
+ private void recordKeyUsageAfterSign(Exchange exchange) throws Exception {
if (keyPair == null || keyPair.getPrivate() == null) {
return;
}
PrivateKey privateKey = keyPair.getPrivate();
- long remaining = getStatefulKeyRemaining(privateKey);
- if (remaining < 0) {
- // Not a stateful key
- return;
- }
+ boolean stateful = getStatefulKeyRemaining(privateKey) >= 0;
KeyLifecycleManager klm = getConfiguration().getKeyLifecycleManager();
if (klm == null) {
+ warnStatefulStateNotPersisted(stateful, "no keyLifecycleManager is
configured on the endpoint");
return;
}
String keyId = exchange.getMessage().getHeader(PQCConstants.KEY_ID,
String.class);
if (ObjectHelper.isEmpty(keyId)) {
+ warnStatefulStateNotPersisted(stateful,
+ "the exchange carries no " + PQCConstants.KEY_ID + "
header identifying the key");
return;
}
- // Update metadata with current usage
KeyMetadata metadata = klm.getKeyMetadata(keyId);
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);
+ warnStatefulStateNotPersisted(stateful, "the keyLifecycleManager
holds no metadata for key '" + keyId + "'");
return;
}
metadata.updateLastUsed();
Review Comment:
Good catch on the silence — fixed in 1b7f1bc.
`warnStatefulStateNotPersisted` is now `warnKeyUsageNotRecorded` and fires
for every key, not only the stateful ones, with the wording matching the real
consequence in each case: index reuse for a stateful key, and for any other key
a `usageCount` that never advances so a configured `maxKeyUsage` threshold is
never reached. Still once per producer.
On the in-memory increment specifically: I could not make that part work
usefully. `usageCount` lives on the `KeyMetadata` owned by the
`KeyLifecycleManager`, and `KeyRotationScheduler` reads it back from the
manager — so with no manager there is no metadata object to increment, and a
producer-local counter would never reach the scheduler. It would be state
nothing consults. I have recorded that constraint in the javadoc so the
limitation is explicit rather than implied. If you would rather the producer
kept its own counter and pushed it on the next successful `updateKeyMetadata`,
say so and I will add it.
_Claude Code on behalf of @oscerd_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
