This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch backport/27504-to-camel-4.22.x in repository https://gitbox.apache.org/repos/asf/camel.git
commit 29a0ef3c431e3caa2035a336abf12d11ae93fca8 Author: Guillaume Nodet <[email protected]> AuthorDate: Thu Oct 8 08:57:52 2026 +0000 [backport camel-4.22.x] CAMEL-24440: camel-crypto - finalize and compare the MAC on a padding failure too --- .../camel/converter/crypto/CryptoDataFormat.java | 22 ++++++++++++++++-- .../camel/converter/crypto/HMACAccumulator.java | 27 ++++++++++++++++++---- .../converter/crypto/HMACAccumulatorTest.java | 15 ++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java b/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java index eb0449959608..b6defc8bc8e3 100644 --- a/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java +++ b/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java @@ -21,6 +21,7 @@ import java.io.DataOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.security.GeneralSecurityException; import java.security.Key; import java.security.spec.AlgorithmParameterSpec; @@ -166,8 +167,25 @@ public class CryptoDataFormat extends ServiceSupport implements DataFormat, Data 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 (shouldAppendHMAC && 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 - but only when a MAC is actually appended: with + // shouldAppendHMAC=false nothing is authenticating, so calling it an authentication failure + // would misdescribe a plain padding error and drop its cause. + LOG.debug("Reporting cipher failure as an authentication failure", e); + // Still finalize and compare the MAC, as a bad MAC does, so the two failures also take the + // same final work and cannot be told apart by timing. This always throws. + hmac.validate(true); + } + throw e; } hmac.validate(); return osb.build(); diff --git a/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/HMACAccumulator.java b/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/HMACAccumulator.java index e1deb80f29f4..27b74e041254 100644 --- a/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/HMACAccumulator.java +++ b/components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/HMACAccumulator.java @@ -96,15 +96,32 @@ public class HMACAccumulator { return appended; } + /** + * The single message reported for every authentication failure. Bad padding and a bad MAC must be indistinguishable + * to a caller who can submit ciphertext and observe the outcome, because telling them apart is what turns a CBC + * decryption into a padding oracle. + */ + static final String AUTHENTICATION_FAILED = "Message authentication failed"; + public void validate() { + validate(false); + } + + /** + * Validates the appended MAC. When the cipher has already failed (bad padding) the MAC is still finalized and + * compared before failing, so that a padding failure costs the same final work as a MAC failure and the two cannot + * be told apart by timing either. + * + * @param cipherFailed whether decryption already failed, in which case this always fails + */ + void validate(boolean cipherFailed) { byte[] actual = getCalculatedMac(); byte[] expected = getAppendedMac(); // Use a constant-time comparison to avoid leaking MAC-match progress through timing (side-channel). - if (!MessageDigest.isEqual(expected, actual)) { - throw new IllegalStateException( - "Expected mac did not match actual mac\nexpected:" - + byteArrayToHexString(expected) + "\n actual:" - + byteArrayToHexString(actual)); + if (!MessageDigest.isEqual(expected, actual) || cipherFailed) { + // The computed MAC is HMAC_k over the plaintext that was just produced, so reporting it hands the + // caller a value they could not otherwise compute. Neither MAC belongs in the message. + throw new IllegalStateException(AUTHENTICATION_FAILED); } } diff --git a/components/camel-crypto/src/test/java/org/apache/camel/converter/crypto/HMACAccumulatorTest.java b/components/camel-crypto/src/test/java/org/apache/camel/converter/crypto/HMACAccumulatorTest.java index 5438704d176d..a45e4ece942e 100644 --- a/components/camel-crypto/src/test/java/org/apache/camel/converter/crypto/HMACAccumulatorTest.java +++ b/components/camel-crypto/src/test/java/org/apache/camel/converter/crypto/HMACAccumulatorTest.java @@ -86,6 +86,21 @@ public class HMACAccumulatorTest { assertThrows(IllegalStateException.class, builder::validate); } + @Test + void testValidateAfterCipherFailureStillComputesMacAndFails() throws Exception { + int buffersize = 256; + byte[] buffer = initializeBuffer(buffersize); + + HMACAccumulator builder = new HMACAccumulator(key, "HmacSHA1", null, buffersize); + builder.decryptUpdate(buffer, 40); + // fails although the appended MAC matches, with the same message as a MAC mismatch + IllegalStateException e = assertThrows(IllegalStateException.class, () -> builder.validate(true)); + assertEquals(HMACAccumulator.AUTHENTICATION_FAILED, e.getMessage()); + // the MAC was still finalized and compared, as on the MAC-mismatch path + assertMacs(expected, builder.getCalculatedMac()); + assertMacs(expected, builder.getAppendedMac()); + } + @Test void testDecryptionWhereMacOverlaps() throws Exception { int buffersize = 32;
