[jira] [Commented] (CAMEL-24440) camel-crypto - CryptoDataFormat validates the MAC after decrypting, reuses the cipher key for the HMAC, and has no per-message IV

["Andrea Cosentino (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAMEL-24440?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107256#comment-18107256 ] 

Andrea Cosentino commented on CAMEL-24440:
------------------------------------------

Three of the four sub-issues are fixed; the fourth is format-breaking and is left out deliberately.

*Fixed.*

1. *Per-message initialization vector when inlining.* {{marshal}} threw {{Inlining cannot be performed, as no initialization vector was specified}} whenever {{shouldInlineInitializationVector}} was set without a static vector - which is what pushed routes into configuring one and reusing it for every message. A fresh vector is now generated per message when none is supplied. Format-compatible: the vector is written into the message, so readers take it from the stream unchanged. An explicitly supplied vector (configuration or the {{CamelCryptoInitVector}} header) is still honoured.

2. *Uniform authentication failure.* Verified on JDK 21 that {{CipherInputStream}} does surface bad padding, as {{IOException}} wrapping {{BadPaddingException}} - I checked rather than assuming, since older JDKs swallowed it. The two outcomes were plainly distinguishable, which the red test shows directly:

{noformat}
expected: <Expected mac did not match actual mac
expected:5cc61463cee1bb52d9111feb9a3e25c33862fa10
     actual:bc9749c9a62fb11c7cd38ac55e1b24617da71ff6>
but was: <Given final block not properly padded. Such issues can arise if a bad key is used during decryption.>
{noformat}

Both now report {{Message authentication failed}}.

3. *MAC values out of the message.* Visible in the output above - the failure text carried both MACs in hex, and the computed one is {{HMAC_k}} over the plaintext just produced, so it hands the caller a value they could not otherwise compute. Removed. ({{MessageDigest.isEqual}} was already in place for the comparison itself.)

Also bounded the inlined vector length, which is read from the message and used to size an allocation directly - {{new byte[ivLength]}} with {{ivLength}} up to {{2^31}}. That was reported separately in the same method, and it was a three line fix in code I was already changing.

*Not fixed: separating the HMAC key from the cipher key.*

{{HMACAccumulator}} derives the MAC key from the same {{key.getEncoded()}} as the cipher. Deriving them separately - HKDF or equivalent - changes the MAC written into the message, so data produced by any earlier version could no longer be read, and data produced by a fixed version could not be read by an unfixed one. That needs either an opt-in flag or a version marker in the output format, and it is the one change here that cannot be backported. Worth its own issue rather than being folded in.

Also unchanged: the composition is still MAC-then-encrypt, and {{unmarshal}} still streams plaintext to the output before {{validate()}} runs. Validating first means buffering the whole plaintext, which would break streaming for large payloads - a real trade-off, and a separate decision from the uniform error reporting that removes the distinguisher.

Module suite 75 green; the three new behaviours are covered and verified red without the fix.

_Claude Code on behalf of oscerd_

> camel-crypto - CryptoDataFormat validates the MAC after decrypting, reuses the cipher key for the HMAC, and has no per-message IV
> ---------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-24440
>                 URL: https://issues.apache.org/jira/browse/CAMEL-24440
>             Project: Camel
>          Issue Type: Bug
>          Components:  camel-crypto
>            Reporter: Andrea Cosentino
>            Assignee: Andrea Cosentino
>            Priority: Major
>             Fix For: 4.23.0
>
>
> Four related issues in {{CryptoDataFormat}}:
> 1. *MAC checked after decryption.* {{unmarshal()}} runs the body through {{CipherInputStream}} to completion before {{HMACAccumulator.validate()}} is called. Tampered input therefore surfaces as two distinguishable failures - {{IOException}} (bad padding, from the cipher) versus {{IllegalStateException}} (bad MAC) - rather than one uniform authentication failure.
> 2. *Key reuse.* {{HMACAccumulator}} is constructed with the HMAC key derived from the same {{key.getEncoded()}} used for the cipher. The same key material is used for both confidentiality and integrity.
> 3. *MAC values in the failure message.* {{HMACAccumulator.validate()}} builds its exception message with both the expected and the computed MAC in hex.
> 4. *No per-message IV.* {{getInitializationVector()}} returns the header value or the statically configured {{initVector}}; nothing ever generates a fresh one. {{marshal()}} even throws when {{inlineInitVector=true}} and no static IV was configured, which steers users towards a fixed IV for exactly the feature that exists to carry a per-message one. The javadoc notes the IV need not be secret but never that it must be unique.
> Proposal: derive separate cipher and MAC keys from the configured key (HKDF or equivalent); validate the MAC before releasing plaintext, or move to an AEAD mode; report a single uniform authentication failure without embedding MAC values; and generate a per-message {{SecureRandom}} IV when {{inlineInitVector}} is enabled instead of requiring a static one. Changing key derivation or IV handling breaks compatibility with data produced by earlier versions, so it needs an upgrade-guide entry and probably a version marker in the output format.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)