oscerd commented on code in PR #28094:
URL: https://github.com/apache/camel/pull/28094#discussion_r4313365529


##########
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:
   You are right and this was my error — thank you for catching it. No KEM path 
calls `recordKeyUsageAfterSign`, so the claim was false exactly where it would 
mislead: an operator setting `maxKeyUsage` on a KEM key.
   
   Fixed in 24365a4. The javadoc now states which keys are covered (ML-DSA, 
SLH-DSA, Falcon and the stateful ones) and adds an explicit sentence that the 
key encapsulation operations do not call it, so a KEM key does not advance its 
usage count and `maxKeyUsage` does not apply to it.
   
   I chose to correct the documentation rather than extend the hook to the KEM 
operations: that is a behaviour change of its own and belongs in a separate 
issue rather than widening this one.
   
   _Claude Code on behalf of @oscerd_



##########
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:
   Fixed in 24365a4 — the message is now "Failed to record key usage after 
hybrid signing", matching the generalised semantics.
   
   _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]

Reply via email to