gnodet-bot commented on code in PR #28328:
URL: https://github.com/apache/camel/pull/28328#discussion_r4356509480
##########
components/camel-pqc/src/main/java/org/apache/camel/component/pqc/PQCProducer.java:
##########
@@ -584,43 +589,111 @@ private Signature classicalSignerForExchange() throws
NoSuchAlgorithmException {
/**
* Feeds the message body into the given {@link Signature} without
materialising the whole payload as a String. A
* {@code byte[]} body is used directly, an {@link InputStream} body is
read in chunks (and reset afterwards when it
- * is a re-readable {@link StreamCache} so downstream processors still see
the payload), and any other body type
- * falls back to its String representation, encoded as UTF-8.
+ * is a re-readable {@link StreamCache} so downstream processors still see
the payload), and a file body is read in
+ * chunks from the file (see {@link #fileContent(Object)}). A {@code
String} or {@link CharSequence} body is encoded
+ * as UTF-8, and any other body is read through {@link #rawBytes(Message,
Object)}, which either gets its bytes or
+ * rejects it.
*/
private static void updateSignatureFromBody(Signature signature, Message
message)
throws InvalidPayloadException, SignatureException, IOException {
- Object body = message.getBody();
+ Object body = fileContent(message.getBody());
if (body instanceof byte[] bytes) {
signature.update(bytes);
+ } else if (body instanceof Path path) {
+ try (InputStream stream = Files.newInputStream(path)) {
+ updateSignatureFromStream(signature, stream);
+ }
} else if (body instanceof InputStream stream) {
try {
- byte[] buffer = new byte[STREAM_BUFFER_SIZE];
- int read;
- while ((read = stream.read(buffer)) != -1) {
- signature.update(buffer, 0, read);
- }
+ updateSignatureFromStream(signature, stream);
} finally {
if (body instanceof StreamCache streamCache) {
streamCache.reset();
}
}
+ } else if (body instanceof CharSequence text) {
+ signature.update(textToBytes(text));
} else {
- // Pin UTF-8: the JVM default charset is platform dependent, so
signing and verifying on JVMs with a
- // different default would disagree on the bytes for a non-ASCII
payload
-
signature.update(message.getMandatoryBody(String.class).getBytes(StandardCharsets.UTF_8));
+ signature.update(rawBytes(message, body));
+ }
+ }
+
+ /**
+ * Encodes a text body as UTF-8. The JVM default charset is platform
dependent, so signing and verifying on JVMs
+ * with a different default would disagree on the bytes for a non-ASCII
payload; UTF-8 keeps them in step.
+ */
+ private static byte[] textToBytes(CharSequence text) {
+ return text.toString().getBytes(StandardCharsets.UTF_8);
+ }
+
+ /**
+ * Returns the bytes of a body that is neither a {@code byte[]}, an {@link
InputStream}, a file nor a
+ * {@link CharSequence}, by asking the type converter registry for a
registered converter to {@code byte[]} - which
+ * covers bodies with a binary form such as a {@link java.nio.ByteBuffer},
a {@link java.io.ByteArrayOutputStream},
+ * a Netty {@code ByteBuf} or a Vert.x {@code Buffer}, so they are signed
over their bytes rather than over a text
+ * rendering. A body with no such converter (for example a {@code Map} or
a POJO) has no well-defined bytes to sign,
+ * so it is rejected with {@link InvalidPayloadException} rather than
signed over an incidental text form. Fallback
+ * converters are not consulted, so the result does not depend on which
components are on the classpath.
+ */
+ private static byte[] rawBytes(Message message, Object body) throws
InvalidPayloadException {
+ Exchange exchange = message.getExchange();
+ if (body != null) {
+ // lookup finds only a registered converter, so a body with a
binary form (ByteBuffer, ByteArrayOutputStream,
+ // Netty ByteBuf, Vert.x Buffer, ...) is converted, while a Map or
POJO - which would only convert through a
+ // fallback converter, if any component provides one - is left for
the InvalidPayloadException below, so the
+ // outcome does not depend on the classpath. tryConvertTo returns
null instead of throwing on a miss.
+ TypeConverter converter =
exchange.getContext().getTypeConverterRegistry().lookup(byte[].class,
body.getClass());
+ if (converter != null) {
+ byte[] bytes = converter.tryConvertTo(byte[].class, exchange,
body);
+ if (bytes != null) {
+ return bytes;
+ }
+ }
+ }
+ throw new InvalidPayloadException(exchange, byte[].class, message);
+ }
+
+ private static void updateSignatureFromStream(Signature signature,
InputStream stream)
+ throws SignatureException, IOException {
+ byte[] buffer = new byte[STREAM_BUFFER_SIZE];
+ int read;
+ while ((read = stream.read(buffer)) != -1) {
+ signature.update(buffer, 0, read);
+ }
+ }
+
+ /**
+ * Resolves a file body to the content to sign: a {@link WrappedFile} (the
body of the file based consumers) gives
+ * its content, and a {@link File} is turned into its {@link Path}, so
that the bytes of the file are read as they
+ * are. The content of a {@link WrappedFile} is a {@link File} for the
local file system, otherwise the content the
+ * consumer has already loaded, such as a {@code byte[]} or an {@link
InputStream}. Other bodies are not changed.
+ */
+ private static Object fileContent(Object body) {
+ if (body instanceof WrappedFile<?> wrappedFile) {
+ Object content = wrappedFile.getBody();
+ if (content != null) {
Review Comment:
💡 **Nit:** When `wrappedFile.getBody()` returns `null`, `body` stays as the
`WrappedFile` instance, which has no `byte[]` type converter, so it falls
through `rawBytes()` to `InvalidPayloadException`. That behaviour is
intentional (a `WrappedFile` with null body is a degenerate state), but it's
not obvious from the code. A short comment here would help future readers:
```suggestion
if (content != null) {
body = content;
}
// if content is null the WrappedFile itself falls through to
rawBytes(), which
// throws InvalidPayloadException — a WrappedFile with no body
is a degenerate state
```
##########
components/camel-pqc/src/test/java/org/apache/camel/component/pqc/PQCBinaryBodySignatureTest.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.pqc;
+
+import java.io.ByteArrayOutputStream;
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.Security;
+import java.security.Signature;
+import java.security.spec.ECGenParameterSpec;
+import java.util.Map;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.InvalidPayloadException;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.pqc.crypto.hybrid.HybridSignature;
+import org.apache.camel.spi.Registry;
+import org.apache.camel.test.junit6.CamelTestSupport;
+import org.bouncycastle.jcajce.spec.MLDSAParameterSpec;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.pqc.jcajce.provider.BouncyCastlePQCProvider;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Bodies that are not a {@code byte[]}, an {@code InputStream} or a file are
signed and verified over their bytes when
+ * they have a binary type ({@link ByteBuffer}, {@link ByteArrayOutputStream},
and, through their own type converters,
+ * Netty {@code ByteBuf} and Vert.x {@code Buffer}), a {@code String} is
signed as UTF-8, and a body with no binary
+ * representation (a {@link Map} or a POJO) is rejected rather than signed
over a text rendering.
+ */
+public class PQCBinaryBodySignatureTest extends CamelTestSupport {
+
+ private static final String PQC_KEY =
"&signatureAlgorithm=MLDSA&keyPair=#pqcKeyPair";
+ private static final String CLASSICAL_KEY =
"&classicalSignatureAlgorithm=ECDSA_P256&classicalKeyPair=#classicalKeyPair";
+ private static final String SIGN = "pqc:sign?operation=sign" + PQC_KEY;
+ private static final String VERIFY = "pqc:verify?operation=verify" +
PQC_KEY;
+
+ private static final byte[] CONTENT = binaryContent();
+
+ private static KeyPair pqcKeyPair;
+ private static KeyPair classicalKeyPair;
+
+ @BeforeAll
+ public static void startup() throws Exception {
+ if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
+ Security.addProvider(new BouncyCastleProvider());
+ }
+ if (Security.getProvider(BouncyCastlePQCProvider.PROVIDER_NAME) ==
null) {
+ Security.addProvider(new BouncyCastlePQCProvider());
+ }
+
+ KeyPairGenerator pqc =
KeyPairGenerator.getInstance(PQCSignatureAlgorithms.MLDSA.getAlgorithm(),
+ PQCSignatureAlgorithms.MLDSA.getBcProvider());
+ pqc.initialize(MLDSAParameterSpec.ml_dsa_65);
+ pqcKeyPair = pqc.generateKeyPair();
+
+ KeyPairGenerator classical
+ =
KeyPairGenerator.getInstance(PQCClassicalSignatureAlgorithms.ECDSA_P256.getKeyAlgorithm());
+ classical.initialize(new
ECGenParameterSpec(PQCClassicalSignatureAlgorithms.ECDSA_P256.getParameterSpec()));
+ classicalKeyPair = classical.generateKeyPair();
+ }
+
+ @Override
+ protected void bindToRegistry(Registry registry) {
+ registry.bind("pqcKeyPair", pqcKeyPair);
+ registry.bind("classicalKeyPair", classicalKeyPair);
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() {
+ return new RouteBuilder() {
+ @Override
+ public void configure() {
+ from("direct:sign").to(SIGN);
+ from("direct:verify").to(VERIFY);
+
from("direct:hybridSign").to("pqc:hybridSign?operation=hybridSign" + PQC_KEY +
CLASSICAL_KEY);
+
from("direct:hybridVerify").to("pqc:hybridVerify?operation=hybridVerify" +
PQC_KEY + CLASSICAL_KEY);
+ }
+ };
+ }
+
+ @Test
+ void byteBufferBodyIsSignedOverItsBytes() throws Exception {
+ assertTrue(verifiesContent(sign(ByteBuffer.wrap(CONTENT.clone()))),
+ "the signature must cover the bytes of the ByteBuffer");
+ }
+
+ @Test
+ void byteBufferBodyIsVerifiedOverItsBytes() throws Exception {
+ assertTrue(verify(ByteBuffer.wrap(CONTENT.clone()), signContent()),
+ "a signature over the bytes must verify a ByteBuffer body");
+ }
+
+ @Test
+ void byteArrayOutputStreamBodyIsSignedOverItsBytes() throws Exception {
+ ByteArrayOutputStream body = new ByteArrayOutputStream();
+ body.write(CONTENT);
+
+ assertTrue(verifiesContent(sign(body)), "the signature must cover the
bytes of the ByteArrayOutputStream");
+ }
+
+ @Test
+ void stringBodyIsSignedAsUtf8() throws Exception {
Review Comment:
💡 **Nit:** `stringBodyIsSignedAsUtf8` exercises the `CharSequence` branch
via a `String`, but `String` is the trivial case. A `StringBuilder` or
`StringBuffer` argument would give more confidence that the
`.toString().getBytes(UTF_8)` path works for any `CharSequence` implementation:
```suggestion
void stringBodyIsSignedAsUtf8() throws Exception {
String body = "post-quantum café éè";
Signature verifier = pqcSignature();
verifier.initVerify(pqcKeyPair.getPublic());
verifier.update(body.getBytes(StandardCharsets.UTF_8));
assertTrue(verifier.verify(sign(body)), "a String body must be
signed as UTF-8");
}
@Test
void charSequenceBodyIsSignedAsUtf8() throws Exception {
StringBuilder body = new StringBuilder("post-quantum café éè");
Signature verifier = pqcSignature();
verifier.initVerify(pqcKeyPair.getPublic());
verifier.update(body.toString().getBytes(StandardCharsets.UTF_8));
assertTrue(verifier.verify(sign(body)), "a CharSequence body must be
signed as UTF-8");
}
```
--
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]
