(camel) 01/01: CAMEL-24440: camel-crypto - finalize and compare the MAC on a padding failure too

[da...@apache.org]

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 <cl...@gmail.com>
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) <no...@anthropic.com>
    Signed-off-by: Claus Ibsen <cl...@gmail.com>
---
 .../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;