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


##########
components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java:
##########
@@ -625,41 +635,75 @@ private static byte[] bodyToByteArray(Message message) 
throws InvalidPayloadExce
         }
     }
 
+    /**
+     * Returns the KEM {@link KeyGenerator} to use for a single exchange.
+     * <p>
+     * A KEM {@code KeyGenerator} keeps the spec passed to {@code init()} 
until {@code generateKey()} reads it and is
+     * not thread-safe, so the two calls of one exchange must not interleave 
with those of another. As with
+     * {@link #signerForExchange()}, an instance this producer created is 
simply created again, while a configured one -
+     * an autowired bean can be shared by every endpoint, and {@code 
PQCDefault*Material.keyGenerator} JVM-wide - is
+     * handed back as-is, and the caller serializes on whatever it gets.
+     */
+    private KeyGenerator keyGeneratorForExchange() throws 
NoSuchAlgorithmException {
+        if (keyGeneratorAlgorithm == null) {
+            return keyGenerator;
+        }
+        return KeyGenerator.getInstance(keyGeneratorAlgorithm, 
keyGeneratorProvider);
+    }
+
+    /**
+     * The {@link KeyAgreement} counterpart of {@link 
#keyGeneratorForExchange()}, used by the hybrid KEM operations.
+     */
+    private KeyAgreement classicalKeyAgreementForExchange() throws 
NoSuchAlgorithmException {
+        if (classicalKeyAgreementAlgorithm == null) {
+            return classicalKeyAgreement;
+        }
+        return KeyAgreement.getInstance(classicalKeyAgreementAlgorithm);
+    }

Review Comment:
   Done in fc0a7f68e9a3: `classicalKeyAgreementForExchange()` now has a 
comment. I worded it a little differently from the suggestion. The instance is 
created the same way as at start, without naming a provider, so the JDK picks a 
provider that accepts the key when `init()` is called. That is why there is no 
`Provider` object to keep, and naming one here would change which provider 
handles the classical keys.
   
   I left out "they come from JDK-bundled providers, not from Bouncy Castle". 
With delayed provider selection the JDK can pick Bouncy Castle when it is 
registered and accepts the key, so that part is not guaranteed.
   
   _Claude Code on behalf of oscerd_
   



##########
components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java:
##########
@@ -625,41 +635,75 @@ private static byte[] bodyToByteArray(Message message) 
throws InvalidPayloadExce
         }
     }
 
+    /**
+     * Returns the KEM {@link KeyGenerator} to use for a single exchange.
+     * <p>
+     * A KEM {@code KeyGenerator} keeps the spec passed to {@code init()} 
until {@code generateKey()} reads it and is
+     * not thread-safe, so the two calls of one exchange must not interleave 
with those of another. As with
+     * {@link #signerForExchange()}, an instance this producer created is 
simply created again, while a configured one -
+     * an autowired bean can be shared by every endpoint, and {@code 
PQCDefault*Material.keyGenerator} JVM-wide - is
+     * handed back as-is, and the caller serializes on whatever it gets.
+     */
+    private KeyGenerator keyGeneratorForExchange() throws 
NoSuchAlgorithmException {
+        if (keyGeneratorAlgorithm == null) {
+            return keyGenerator;
+        }
+        return KeyGenerator.getInstance(keyGeneratorAlgorithm, 
keyGeneratorProvider);
+    }

Review Comment:
   Thanks for checking both paths. I agree it is worth doing, and I have left 
it out of this PR: it is tracked in 
[CAMEL-25624](https://issues.apache.org/jira/browse/CAMEL-25624), for `main` 
only.
   
   - It changes no result. The instances the component wires are only used one 
at a time now.
   - It also covers the `Signature` instances that 
`configureParameterSpecMaterial()` and the default materials wire (from 
CAMEL-24445). The hybrid default materials also create their classical 
`KeyAgreement` and `Signature` with the `BC` provider, while the producer names 
none, so the change needs its own look.
   - Keeping this PR to the fix keeps the 4.22.x and 4.18.x backports the same 
change.
   
   _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