CAMEL-23843: Camel-PQC - sign and verify String payloads as UTF-8#24716
Conversation
sign/verify (and the hybrid variants) encoded a String message body with String.getBytes(), which uses the JVM default charset. The default charset is platform dependent, so signing on one JVM and verifying on another with a different default could disagree on the bytes of a non-ASCII payload and fail verification. Pin StandardCharsets.UTF_8 in both body-to-bytes fallbacks, so signing and verifying always agree. Binary bodies (byte[], InputStream) are unaffected: they are already signed byte-for-byte (CAMEL-23847). On Java 18 and newer the default charset is already UTF-8 (JEP 400), so this is a no-op there; on Java 17 with a non-UTF-8 default it changes the signature of a non-ASCII String payload, which is documented in the 4.22 upgrade guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Minimal, well-scoped fix for the last remaining piece after CAMEL-23847 handled binary bodies. Both String-to-bytes conversion points (updateSignatureFromBody and bodyToByteArray) are pinned to UTF-8, so sign and verify always agree regardless of the JVM default charset.
The test covers both directions (String ↔ UTF-8 bytes interchangeable, String ↛ ISO-8859-1 bytes) with non-ASCII text where the encodings genuinely differ. Upgrade guide entry is honest about impact (no-op on Java 18+, live fix on Java 17 with non-UTF-8 default). Component docs updated.
LGTM.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Clean fix that pins UTF-8 encoding in both String-to-bytes fallback paths for PQC sign/verify operations. Correct, well-tested, and thoroughly documented.
Key observations:
-
The fix is applied consistently to both conversion points:
updateSignatureFromBody(used bysign/verify) andbodyToByteArray(used byhybridSign/hybridVerify). This symmetry prevents encoding mismatches between standard and hybrid code paths. -
Good use of
StandardCharsets.UTF_8(compile-time constant) rather thanCharset.forName("UTF-8"). The inline comments explaining the platform-dependent charset rationale are helpful for future maintainers. -
The test uses non-ASCII characters (accented Latin, em-dash) which is the right choice for detecting encoding mismatches — these are multi-byte in UTF-8 but single-byte in ISO-8859-1. The explicit cross-check at line 87 that ISO-8859-1 produces different bytes confirms the test's discriminating power.
-
The upgrade guide entry correctly notes this is a no-op on Java 18+ (where
UTF-8is the default charset) and only a live fix on Java 17 with a non-UTF-8 default. This is exactly the kind of precision users need. -
The PR description's honesty about scope and impact ("no-op on Java 18+, live fix on Java 17 with non-UTF-8 default") is a model for security-sensitive changes.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code review on behalf of @gnodet
|
conflict to resolve |
…ng payloads as UTF-8 (#24723) [backport camel-4.18.x] CAMEL-23843: Camel-PQC sign and verify String payloads as UTF-8 sign/verify encoded a String message body with String.getBytes(), which uses the JVM default charset. The default charset is platform dependent, so signing on one JVM and verifying on another with a different default could disagree on the bytes of a non-ASCII payload and fail verification. Pin StandardCharsets.UTF_8. Charset-only backport of #24716. The native byte[]/InputStream handling from CAMEL-23847 is an improvement that stays on main only, so it is intentionally not part of this backport. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
…ng payloads as UTF-8 (#24725) [backport camel-4.14.x] CAMEL-23843: Camel-PQC sign and verify String payloads as UTF-8 sign/verify encoded a String message body with String.getBytes(), which uses the JVM default charset. The default charset is platform dependent, so signing on one JVM and verifying on another with a different default could disagree on the bytes of a non-ASCII payload and fail verification. Pin StandardCharsets.UTF_8. Charset-only backport of #24716. The native byte[]/InputStream handling from CAMEL-23847 is an improvement that stays on main only, so it is intentionally not part of this backport. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 28 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
4 participants
CAMEL-23843: Camel-PQC — sign and verify String payloads as UTF-8
The bug
PQCProducersign/verify (and the hybrid variants) read the body and, when it is not already binary,encode it with
String.getBytes()— i.e. the JVM default charset. The default charset is platformdependent, so signing on one JVM and verifying on another with a different default disagrees on the
bytes of a non-ASCII payload and verification fails.
The issue also called out that binary payloads were forced through a String conversion. That half is
already fixed on
mainby CAMEL-23847 (#24644, merged):byte[]andInputStreambodies are now fedto the signature byte-for-byte. What remained is the charset.
The fix
Pin
StandardCharsets.UTF_8in both body-to-bytes fallbacks (updateSignatureFromBody, used bysign/verify, and
bodyToByteArray, used by the hybrid variants), so sign and verify always agree onthe encoding. Binary bodies are untouched.
Scope / honesty about impact
On Java 18+ the default charset is already UTF-8 (JEP 400), so this is a no-op there — the value
of the change is that the encoding becomes explicit and deterministic rather than
environment-dependent. On Java 17 with a non-UTF-8 default (which Camel still supports) the bug is
live, and this fixes it.
Because it can change the signature of a non-ASCII
Stringpayload on such a JVM, it is documented inthe 4.22 upgrade guide.
Testing
New
PQCSignatureCharsetTest(2 tests):Stringbody and its UTF-8 bytes are interchangeable — signing the String verifies against theUTF-8 bytes and vice versa;
ISO-8859-1.
The existing
PQCStreamingSignatureTest(5) still passes. Module build green, no generated drift.Backport
4.18.x (4.18.4) and 4.14.x (4.14.9) — charset only. Those branches never received the
streaming/binary-payload work (CAMEL-23847, an improvement kept to
main), so the backport pins UTF-8for
Stringbodies but does not add nativebyte[]/InputStreamhandling: that remains themain-only improvement. The determinism bug for
Stringpayloads is fixed on all three lines.Claude Code on behalf of Andrea Cosentino.