oscerd commented on code in PR #26725:
URL: https://github.com/apache/camel/pull/26725#discussion_r4091330509
##########
components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java:
##########
@@ -166,8 +186,20 @@ public Object unmarshal(final Exchange exchange, final
InputStream encryptedStre
byte[] buffer = new byte[bufferSize];
hmac.attachStream(osb);
int read;
- while ((read = cipherStream.read(buffer)) >= 0) {
- hmac.decryptUpdate(buffer, read);
+ try {
+ while ((read = cipherStream.read(buffer)) >= 0) {
+ hmac.decryptUpdate(buffer, read);
+ }
+ } catch (IOException e) {
+ if (e.getCause() instanceof GeneralSecurityException) {
+ // CipherInputStream surfaces bad padding as an
IOException wrapping
+ // BadPaddingException, while a bad MAC surfaces from
validate() below. Reporting the two
+ // differently is exactly what lets a caller who can
submit ciphertext and watch the
+ // outcome tell them apart, which is the
padding-oracle distinguisher. Report the same
+ // authentication failure for both.
+ throw new
IllegalStateException(HMACAccumulator.AUTHENTICATION_FAILED);
Review Comment:
Fixed in bdd8116 — the `AUTHENTICATION_FAILED` rewrite is now gated on
`shouldAppendHMAC`; with the MAC off the real cipher cause is surfaced (at
debug) rather than relabelled. Test added.
_Claude Code on behalf of oscerd_
##########
components/camel-crypto/src/test/java/org/apache/camel/converter/crypto/CryptoDataFormatIvAndFailureTest.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * 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.converter.crypto;
+
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+import java.security.Key;
+
+import javax.crypto.KeyGenerator;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.impl.DefaultCamelContext;
+import org.apache.camel.support.DefaultExchange;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class CryptoDataFormatIvAndFailureTest {
+
+ private static final String PAYLOAD = "the quick brown fox jumps over the
lazy dog";
+
+ /**
+ * Inlining exists so the initialization vector travels with the message.
Requiring a statically configured one as
+ * well is what pushed routes into reusing a single vector for every
message.
+ */
+ @Test
+ void inliningGeneratesAFreshInitializationVectorPerMessage() throws
Exception {
+ Key key = key();
+ try (DefaultCamelContext context = new DefaultCamelContext()) {
+ context.start();
+ CryptoDataFormat encryptor = new
CryptoDataFormat("AES/CBC/PKCS5Padding", key);
+ encryptor.setShouldInlineInitializationVector(true);
+
+ byte[] first = marshal(context, encryptor, PAYLOAD);
+ byte[] second = marshal(context, encryptor, PAYLOAD);
+
+ assertFalse(java.util.Arrays.equals(first, second),
Review Comment:
Fixed in bdd8116 — imported `java.util.Arrays` and use the simple name.
_Claude Code on behalf of oscerd_
##########
components/camel-crypto/src/test/java/org/apache/camel/converter/crypto/CryptoDataFormatIvAndFailureTest.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * 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.converter.crypto;
+
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+import java.security.Key;
+
+import javax.crypto.KeyGenerator;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.impl.DefaultCamelContext;
+import org.apache.camel.support.DefaultExchange;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class CryptoDataFormatIvAndFailureTest {
+
+ private static final String PAYLOAD = "the quick brown fox jumps over the
lazy dog";
+
+ /**
+ * Inlining exists so the initialization vector travels with the message.
Requiring a statically configured one as
+ * well is what pushed routes into reusing a single vector for every
message.
+ */
+ @Test
+ void inliningGeneratesAFreshInitializationVectorPerMessage() throws
Exception {
+ Key key = key();
+ try (DefaultCamelContext context = new DefaultCamelContext()) {
+ context.start();
+ CryptoDataFormat encryptor = new
CryptoDataFormat("AES/CBC/PKCS5Padding", key);
+ encryptor.setShouldInlineInitializationVector(true);
+
+ byte[] first = marshal(context, encryptor, PAYLOAD);
+ byte[] second = marshal(context, encryptor, PAYLOAD);
+
+ assertFalse(java.util.Arrays.equals(first, second),
+ "the same plaintext must not produce identical ciphertext
twice");
+
+ CryptoDataFormat decryptor = new
CryptoDataFormat("AES/CBC/PKCS5Padding", key);
+ decryptor.setShouldInlineInitializationVector(true);
+ assertEquals(PAYLOAD, unmarshal(context, decryptor, first));
+ assertEquals(PAYLOAD, unmarshal(context, decryptor, second));
+ }
+ }
+
+ /**
+ * A caller who can submit ciphertext and observe the outcome must not be
able to tell a padding failure from a MAC
+ * failure - telling them apart is what turns CBC decryption into a
padding oracle.
+ */
+ @Test
+ void badPaddingAndBadMacAreReportedIdentically() throws Exception {
+ Key key = key();
+ try (DefaultCamelContext context = new DefaultCamelContext()) {
+ context.start();
+ // a static vector, not inlining, so this exercises the failure
reporting and nothing else
+ CryptoDataFormat format = new
CryptoDataFormat("AES/CBC/PKCS5Padding", key);
+ format.setInitVector(new byte[16]);
+
+ byte[] ciphertext = marshal(context, format, PAYLOAD);
+
+ // The MAC is written through the CipherOutputStream during
marshal, so it is encrypted inside the
+ // ciphertext, not appended in the clear. The last wire byte is
therefore the final ciphertext block, and
+ // flipping it makes that block no longer decrypt to valid PKCS5
padding - the BadPaddingException path,
+ // distinct from the bad-MAC path exercised just below.
+ byte[] badPadding = ciphertext.clone();
+ badPadding[badPadding.length - 1] ^= 0x01;
+
+ // corrupt a byte in the middle: padding still validates, the
appended MAC does not
+ byte[] badMac = ciphertext.clone();
+ badMac[badMac.length / 2] ^= 0x01;
+
+ String paddingFailure = failureMessage(context, format,
badPadding);
+ String macFailure = failureMessage(context, format, badMac);
+
+ assertEquals(macFailure, paddingFailure, "the two failures must be
indistinguishable");
+ assertTrue(paddingFailure.contains("authentication failed"),
"unexpected message: " + paddingFailure);
+ }
+ }
+
+ /**
+ * The inlined length is read from the message and used to size an
allocation, so it has to be bounded.
+ */
+ @Test
+ void anOversizedInlinedInitializationVectorLengthIsRejected() throws
Exception {
+ Key key = key();
+ try (DefaultCamelContext context = new DefaultCamelContext()) {
+ context.start();
+ CryptoDataFormat decryptor = new
CryptoDataFormat("AES/CBC/PKCS5Padding", key);
+ decryptor.setShouldInlineInitializationVector(true);
+
+ // a four byte length of 0x7FFFFFFF followed by nothing
+ byte[] hostile = { 0x7F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF };
+
+ Exception e = assertThrows(Exception.class, () ->
unmarshal(context, decryptor, hostile));
+ assertTrue(rootMessage(e).contains("is not between 0 and"),
"unexpected message: " + rootMessage(e));
+ }
+ }
+
+ @Test
+ void aRoundTripWithAStaticVectorStillWorks() throws Exception {
+ Key key = key();
+ byte[] iv = new byte[16];
+ try (DefaultCamelContext context = new DefaultCamelContext()) {
+ context.start();
+ CryptoDataFormat format = new
CryptoDataFormat("AES/CBC/PKCS5Padding", key);
+ format.setInitVector(iv);
+
+ byte[] ciphertext = marshal(context, format, PAYLOAD);
+ assertEquals(PAYLOAD, unmarshal(context, format, ciphertext));
+ assertArrayEquals(iv, format.getInitVector());
+ }
+ }
+
+ private static String failureMessage(DefaultCamelContext context,
CryptoDataFormat format, byte[] body) {
+ Exception e = assertThrows(Exception.class, () -> unmarshal(context,
format, body));
+ return rootMessage(e);
+ }
+
+ private static String rootMessage(Throwable t) {
+ while (t.getCause() != null) {
+ t = t.getCause();
+ }
+ return String.valueOf(t.getMessage());
+ }
+
+ private static byte[] marshal(DefaultCamelContext context,
CryptoDataFormat format, String payload)
+ throws Exception {
+ Exchange exchange = new DefaultExchange(context);
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ format.marshal(exchange, payload.getBytes(StandardCharsets.UTF_8),
out);
+ return out.toByteArray();
+ }
+
+ private static String unmarshal(DefaultCamelContext context,
CryptoDataFormat format, byte[] body)
+ throws Exception {
+ Exchange exchange = new DefaultExchange(context);
+ Object result = format.unmarshal(exchange, new
java.io.ByteArrayInputStream(body));
Review Comment:
Fixed in bdd8116 — imported `java.io.ByteArrayInputStream` and use the
simple name.
_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]
