gnodet-bot commented on code in PR #27490:
URL: https://github.com/apache/camel/pull/27490#discussion_r4206663265
##########
components/camel-crypto/src/main/java/org/apache/camel/converter/crypto/CryptoDataFormat.java:
##########
@@ -166,8 +192,23 @@ public Object unmarshal(final Exchange exchange, final
InputStream encryptedStre
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);
+ throw new
IllegalStateException(HMACAccumulator.AUTHENTICATION_FAILED);
Review Comment:
**Informational (not blocking):** This catch block eliminates the
*message-level* padding-oracle distinguisher, which is the important fix. Worth
noting for the commit record: a *timing* side-channel remains because the
bad-padding path exits the read loop early while the bad-MAC path completes the
full stream + constant-time compare. This is inherent to `CipherInputStream`
and existed before this PR — just calling it out so nobody reads this catch and
assumes the oracle is fully closed. Fully closing it would require a custom
decryptor that buffers/processes all bytes before checking padding, which is a
much larger change.
--
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]
