This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24440-mac-on-padding-failure in repository https://gitbox.apache.org/repos/asf/camel.git
commit f32a7312ee6867305297e96a93a92ffd287c94e3 Author: Claus Ibsen <[email protected]> AuthorDate: Wed Oct 7 15:09:14 2026 +0200 CAMEL-24440: camel-crypto - finalize and compare the MAC on a padding failure too A message with bad padding is now reported as the same authentication failure as a bad MAC, but it skipped the MAC finalization and constant-time compare that the bad-MAC path runs. The padding path now runs that same final step before failing, so the two failures also take the same final work. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../apache/camel/converter/crypto/CryptoDataFormat.java | 4 +++- .../apache/camel/converter/crypto/HMACAccumulator.java | 13 ++++++++++++- .../camel/converter/crypto/HMACAccumulatorTest.java | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 2 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 09a0764b5a1e..ae5052b18057 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 @@ -206,7 +206,9 @@ public class CryptoDataFormat extends ServiceSupport implements DataFormat, Data // 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); - throw new IllegalStateException(HMACAccumulator.AUTHENTICATION_FAILED); + // 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; } 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 e0b7181fffa4..9cade39ef0ec 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 @@ -102,10 +102,21 @@ public class HMACAccumulator { 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)) { + 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;
