gnodet-bot commented on code in PR #26725:
URL: https://github.com/apache/camel/pull/26725#discussion_r4091444965
##########
components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java:
##########
@@ -124,6 +136,20 @@ private Cipher initializeCipher(int mode, Key key, byte[]
iv) throws Exception {
@Override
public void marshal(Exchange exchange, Object graph, OutputStream
outputStream) throws Exception {
byte[] iv = getInitializationVector(exchange);
+ if (iv == null && inline) {
+ if (algorithmParameterSpec != null) {
+ // initializeCipher gives algorithmParameterSpec precedence
over the IV, so a generated vector would be
+ // written into the message but never used - every message
would encrypt identically behind a vector
+ // that only looks per-message. Keep failing loudly, as this
configuration did before.
+ throw new IllegalStateException(
+ "Inlining cannot be performed when an
algorithmParameterSpec is configured, as the spec is"
+ + " used instead of the
initialization vector");
+ }
+ // The whole point of inlining is that the IV travels with the
message, so there is no reason to make
+ // the caller supply a fixed one - and requiring it is what used
to push users into reusing a single IV
+ // across every message.
+ iv = generateInitializationVector();
Review Comment:
🔴 **ECB + `inline` (davsclaus item 3) — still broken.**
For `AES/ECB/PKCS5Padding` + `inline`, no static IV, no
`algorithmParameterSpec`:
1. `algorithmParameterSpec != null` is false → falls through to
`generateInitializationVector()`
2. `getCipherBlockSize()` returns 16 (ECB block cipher has a block size)
3. A 16-byte IV is generated
4. `initializeCipher` calls `cipher.init(mode, key, new
IvParameterSpec(iv))` for ECB mode
5. JCE throws `InvalidAlgorithmParameterException: ECB mode cannot use IV`
Before the PR the message was `"Inlining cannot be performed, as no
initialization vector was specified"` — at least honest. The fix must guard
against IV-less modes. ECB's `Cipher.getBlockSize()` still returns 16, so block
size alone cannot distinguish it from CBC. The reliable approach is to probe:
after `cipher.init(mode, key)` (no IV), `cipher.getParameters()` returns null
for ECB — that null result means the mode does not use an IV. If the probe
shows no IV is used, fail loudly:
```suggestion
if (iv == null && inline) {
if (algorithmParameterSpec != null) {
// initializeCipher gives algorithmParameterSpec precedence
over the IV, so a generated vector would be
// written into the message but never used - every message
would encrypt identically behind a vector
// that only looks per-message. Keep failing loudly, as this
configuration did before.
throw new IllegalStateException(
"Inlining cannot be performed when an
algorithmParameterSpec is configured, as the spec is"
+ " used instead of the
initialization vector");
}
// Detect IV-less modes (e.g. ECB): after a no-IV init,
getParameters() is null.
Cipher probe = cryptoProvider == null
? Cipher.getInstance(algorithm)
: Cipher.getInstance(algorithm, cryptoProvider);
try {
probe.init(Cipher.ENCRYPT_MODE, key);
} catch (Exception ignored) {
// probe only; ignore init failures here
}
if (probe.getParameters() == null) {
throw new IllegalStateException(
"Inlining cannot be performed with a mode that does
not use an initialization vector (e.g. ECB)."
+ " Use a mode that supports an IV (e.g.
AES/CBC/PKCS5Padding) or supply a static vector.");
}
// The whole point of inlining is that the IV travels with the
message, so there is no reason to make
// the caller supply a fixed one - and requiring it is what used
to push routes into reusing a single IV
// across every message.
iv = generateInitializationVector();
}
```
##########
components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java:
##########
@@ -247,6 +293,31 @@ public byte[] getCalculatedMac() {
};
}
+ /**
+ * A fresh initialization vector, sized to the cipher's block length. Only
used when the vector is inlined into the
+ * message, so the reader takes it from the stream and nothing needs to be
shared out of band.
+ */
+ private byte[] generateInitializationVector() throws Exception {
+ byte[] iv = new byte[getCipherBlockSize()];
+ SECURE_RANDOM.nextBytes(iv);
+ return iv;
+ }
+
+ private int getCipherBlockSize() throws Exception {
+ int blockSize = cachedBlockSize;
+ if (blockSize == 0) {
+ Cipher cipher
+ = cryptoProvider == null ? Cipher.getInstance(algorithm) :
Cipher.getInstance(algorithm, cryptoProvider);
+ blockSize = cipher.getBlockSize();
+ if (blockSize <= 0) {
+ // A stream cipher reports no block size; 16 bytes is the
usual nonce length
+ blockSize = 16;
+ }
+ cachedBlockSize = blockSize;
Review Comment:
test cache comment
--
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]
