gnodet-bot commented on code in PR #28094:
URL: https://github.com/apache/camel/pull/28094#discussion_r4312457442
##########
components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java:
##########
@@ -776,7 +776,7 @@ private void hybridSignature(Exchange exchange)
exchange.getMessage().setHeader(PQCConstants.PQC_SIGNATURE,
components.pqcSignature());
try {
- persistStatefulKeyStateAfterSign(exchange);
+ recordKeyUsageAfterSign(exchange);
} catch (Exception e) {
throw new RuntimeCamelException("Failed to persist stateful key
state after hybrid signing", e);
Review Comment:
💡 Stale error message — the method was renamed from
`persistStatefulKeyStateAfterSign` to `recordKeyUsageAfterSign` but the catch
block still says "persist stateful key state". Should reflect the generalized
semantics:
```
throw new RuntimeCamelException("Failed to record key usage after hybrid
signing", e);
```
##########
components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java:
##########
@@ -1146,21 +1146,22 @@ 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.
Review Comment:
⚠️ **Inaccurate Javadoc:** The comment claims "KEM keys would otherwise sit
at a usage count of zero forever", but no KEM operation path
(`generateEncapsulation`, `extractEncapsulation`,
`hybridGenerateEncapsulation`, `hybridExtractEncapsulation`) calls
`recordKeyUsageAfterSign`. KEM usage counting remains broken — the fix only
covers the two signing paths. Either add the call to KEM operations, or remove
the KEM claim from the Javadoc to avoid misleading operators who configure
`maxKeyUsage` on KEM keys.
```suggestion
* not only the stateful ones: ML-DSA, SLH-DSA and Falcon would
otherwise sit at a usage count of zero
* forever and never be rotated. KEM operations do not call this method
and do not yet advance the usage count.
```
--
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]
