gnodet-bot commented on code in PR #28099:
URL: https://github.com/apache/camel/pull/28099#discussion_r4312627897
##########
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:
⚠️ **Non-stateful keys silently skip `updateLastUsed()` /
`updateKeyMetadata()` when a prerequisite is missing**
The Javadoc just added states:
> *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.*
However, the three early-return guards (`klm == null`, `keyId` empty,
`metadata == null`) exit before `metadata.updateLastUsed()` /
`klm.updateKeyMetadata()` are reached. When `stateful == false`,
`warnStatefulStateNotPersisted` is a no-op (the `compareAndSet` guard is gated
on `stateful`), so the skip is entirely silent. Non-stateful keys (ML-DSA,
SLH-DSA, Falcon, KEM) will have their `usageCount` stuck at zero and will never
be rotated by `KeyRotationScheduler` if any of these conditions apply.
Consider separating the two concerns: if `klm` is `null` (or `keyId` is
missing, or `metadata` is absent), a non-stateful key should still have its
`usageCount` incremented in-memory — only the stateful persist-to-store path
actually requires those prerequisites.
--
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]
