CAMEL-24437/24439/24441/24443/24454: secure-default fixes, backport to camel-4.22.x - #25944
Merged
davsclaus merged 5 commits intoAug 31, 2026
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Croway
approved these changes
Aug 31, 2026
Contributor
|
…e paths that ignored it (apache#25832) MllpComponent.logPhi defaulted to true, so message content reached the log at the default INFO/WARN levels with no configuration at all. For a protocol whose payload is patient data by definition, the safe default is the other way round. Two paths logged content regardless of the flag, because MllpSocketBuffer has no logPhi of its own: the partial-payload warning in readFrom(), which logs the content of a legitimate in-flight message from a slow sender rather than only unexpected bytes, and the bytes-before-START_OF_BLOCK warning in readSocketInputStream(). Both now go through a helper that honours the setting, printing <PHI suppressed> when it is off. The suppression is applied at the log statements, via a new Hl7Util.convertToLoggableString, rather than inside convertToPrintFriendlyString. That method is not a logging helper despite the name: generateAcknowledgementPayload() uses it to extract the MSH-9 field, so redacting inside it corrupts the acknowledgement rather than the log - which is what the MLLP suite showed when it was tried that way. Both methods now carry a javadoc saying so. LogPhiTest.testLogPhiDefault asserted that the default includes the payload; it now asserts the opposite. testLogPhiFalse and testLogPhiTrue are unchanged and still pass, so the flag still works in both directions - only the default moved. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit dd2bf57) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…is enabled without a truststore (apache#25824) KnativeSslClientOptions.configureOptions() installed TrustAllOptions.INSTANCE - a trust manager that accepts every certificate - whenever camel.knative.client.ssl.enabled was true and neither truststore.path nor trust.cert.path was set. No option named trustAll was involved: enabling TLS was itself what turned certificate validation off. Hostname verification in the same method already defaults to true, so the trust decision was the outlier, and KnativeOidcClientOptions extends this class. Leave the trust options unset in that case instead, so the JVM default trust anchors apply - the fallback SSLContextParameters and the rest of Camel use. Accepting any certificate stays available behind the new camel.knative.client.ssl.trust.all property, which defaults to false. KnativeHttpTest.testSecureClientOptionsPropertyConf configures SSL entirely through properties against a self-signed test server, so it relied on the old fallback; it now sets trust.all explicitly, which is the same migration an affected deployment makes. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 26c899d) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e flow with a state parameter (apache#25821) buildCodeFlowAuthRequestUrl() sent only a redirect URI and scopes - no state - and OAuthCodeFlowCallback redeemed whatever code arrived and bound the resulting profile to the caller's session. Nothing tied the callback to a flow that session had started, which is the login CSRF that RFC 6749 section 10.12 and OpenID Connect Core require the state binding to prevent. OAuthCodeFlowParams already carried a state field; no processor set it. The hardcoded SameSite=None; Secure session cookie makes it reachable cross-site. OAuthCodeFlowProcessor now generates a 32-byte random state, stores it in the OAuth session, and passes it through both URL builders - VertxOAuth via OAuth2AuthorizationURL.setState, ServletOAuth as a state query parameter. OAuthCodeFlowCallback removes the stored value, so it is single use, and compares it with the callback's state using MessageDigest.isEqual. A callback with no flow in progress, or with a state that does not match, is answered with 400 and stops the route. Scope: state only. nonce needs ID-token validation to be worth sending, PKCE needs a code_verifier carried through AuthCodeCredentials and both authenticate() implementations, and the session cookie's SameSite is a separate change - all three are noted on the issue. Not verified end to end: OAuthCodeFlowVertxTest and OAuthCodeFlowServletTest are gated on an externally running Keycloak at https://oauth.localtest.me/kc, provisioned by the module's Helm chart, and were skipped here. They are what would confirm the provider echoes state back as a state message header. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 495c5ad) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…exchange (apache#25822) * CAMEL-24439: camel-shiro - verify the presented credentials on every exchange ShiroSecurityProcessor.authenticateUser() called login() only when the thread-bound subject was not already authenticated for the same username as the incoming ShiroSecurityToken: if (!authenticated || !sameUser) { ... currentUser.login(token); } That conflates "same principal name" with "same credentials". Once a user had authenticated on a worker thread, a later exchange presenting that username with any password was accepted for as long as the subject stayed bound, because the password was never checked. The default alwaysReauthenticate=true masks it, since the processor calls logout() in a finally block after each exchange. With alwaysReauthenticate=false the skip is reachable, and that mode deliberately sets rememberMe(true) to keep subjects long-lived on Camel's shared worker threads. Call login() for every exchange with the credentials that exchange presented. Shiro offers no way to compare presented credentials against a bound subject, so the principal-name comparison could not be made sound and is removed rather than narrowed. The added test sends a valid token for ringo, then the same username with a wrong password on the same thread; without the fix both reach mock:success. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com> * CAMEL-24439: Simplify test encryption key --------- Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 6f72ae6) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rypting (apache#25848) unmarshal() verified the modification detection code only when the message was an OpenPGP symmetrically encrypted integrity protected data packet: if (pbe.isIntegrityProtected()) { if (!pbe.verify()) { throw new PGPException("Message failed integrity check"); } } The older symmetrically encrypted data packet carries no such code, and OpenPGP's CFB mode is malleable without one, so a message using that packet skipped the check altogether. The packet type is chosen by whoever produced the message, which left the sender - or anyone able to rewrite the message in transit - deciding whether the check applied. The existing integrity option governs marshalling only and has no decrypt-side counterpart. Add requireIntegrityProtection, defaulting to true, which rejects a message that is not integrity protected. Routes interoperating with a sender that still emits the legacy packet must set it to false. signatureVerificationOption still defaults to optional, so a message carrying no signature is accepted. Flipping that would reject every unsigned message and is a separate decision; the upgrade guide points at it, since the two options together are what give a decrypted message authenticity and not only confidentiality. PGPDataFormatTest sets encryptor.setIntegrity(false) for its whole class, so its decryptor now opts out explicitly - the same change an affected deployment makes. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> (cherry picked from commit 9d3108d) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
oscerd
force-pushed
the
backport/glasswing-secure-defaults-4.22.x
branch
from
August 31, 2026 12:42
8f0cc86 to
600001e
Compare
Contributor
Author
|
Thanks — that was a real gap in the backport, not noise.
Regenerated it from this branch and folded it into the CAMEL-24454 commit. The resulting hunk is the single Claude Code on behalf of oscerd |
gnodet
approved these changes
Aug 31, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
All five security-fix cherry-picks are clean and compatible with the camel-4.22.x branch. The single mechanical adaptation (JUnit assertions instead of AssertJ in OAuthProcessorFailClosedTest) is correct and consistent with the module's test dependencies on this branch.
Verified that prerequisites already exist on the branch: the reject method in AbstractOAuthProcessor (from CAMEL-24411 backport), removeValue in OAuthSession, setState/getState in OAuthCodeFlowParams, and TrustAllOptions in the knative-http package.
Each fix correctly tightens a secure default:
- camel-mllp:
logPhidefaults tofalse - camel-knative: no longer trusts all certificates implicitly when SSL is enabled without a truststore
- camel-oauth: state parameter for CSRF protection on authorization code callback
- camel-shiro: always verifies credentials on every exchange
- camel-crypto-pgp: requires integrity protection when decrypting
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Milestone | (none) | 4.22.1 |
| Labels | components, core, catalog, dsl |
+ backport |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 552 tested, 25 compile-only — current: 548 all testedMaveniverse Scalpel detected 577 affected modules (current approach: 548).
|
davsclaus
approved these changes
Aug 31, 2026
davsclaus
pushed a commit
that referenced
this pull request
Aug 31, 2026
…camel-4.18.x Backports five fixes already reviewed and merged on main (the same set backported to camel-4.22.x in #25944), grouped together because each changes a default or adds a check that was not there before. camel-mllp stops logging HL7 payload content unless logPhi=true is set explicitly. camel-knative with SSL enabled but no truststore configured no longer falls back to trusting every certificate; a deliberate trust.all property restores the old behaviour. camel-oauth now binds the authorization-code callback to the flow that started it via a random state parameter, checked with a constant-time comparison, closing a login-CSRF gap. camel-shiro verifies presented credentials on every exchange instead of accepting a subject already bound to the thread when alwaysReauthenticate=false. camel-crypto-pgp rejects decryption of a message with no integrity-protection packet unless requireIntegrityProtection=false is set. This branch has diverged from main in MllpSocketBuffer, the test layer, and the generated model files, so each commit needed manual adaptation rather than a mechanical cherry-pick: MllpSocketBuffer keeps this branch's synchronized methods rather than main's ReentrantLock form while carrying the substantive logging change across; the generated model/catalog/XSD/YAML-DSL files touched by CAMEL-24441 are regenerated from this branch rather than taken from main, since camel-java-io and the canonical/model YAML schemas don't exist here; and the new oauth and shiro tests use this branch's existing JUnit/camel-test-junit5 style. Upgrade-guide entries are intentionally excluded — those live on main only. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Closes #25946
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport to
camel-4.22.xof five fixes already reviewed and merged onmain. These are grouped together because each changes a default or adds a check that was not there before, so they deserve a closer look than the other backports in this wave before going into a patch release.logPhito false and honour it on the paths that ignored it (CAMEL-24454: camel-mllp - default logPhi to false and honour it on the paths that ignored it #25832,dd2bf571)26c899d8)495c5ad3)6f72ae64)9d3108de)What changes for an existing deployment:
logPhi=trueis set explicitly. Previously it defaulted to on, and two paths logged regardless of the setting.trustAllset deliberately.statedoes not match the one issued for that session. A deployment behind a proxy that drops or rewrites the callback query string would start seeing 400s.alwaysReauthenticate=falsekeeps the old behaviour.requireIntegrityProtection=falserestores the old acceptance.Straight cherry-picks, applied in the order they merged on
main. One mechanical adaptation, folded into the CAMEL-24437 commit: the new cases inOAuthProcessorFailClosedTestuse JUnit assertions, sincecamel-oauthhas no assertj test dependency on this branch and the rest of that file is JUnit.The upgrade-guide entries are not included: the guides for every line live on
main.Built and tested per module on this branch (
core,camel-crypto-pgp,camel-knative-http,camel-mllp,camel-oauth,camel-shiro), including the new and touched tests:PGPRequireIntegrityProtectionTest,PGPDataFormatTest,KnativeSslClientOptionsTrustTest,KnativeHttpTest,LogPhiTest,Hl7UtilTest,OAuthProcessorFailClosedTest,ShiroAuthenticationCredentialAlwaysCheckedTest.Claude Code on behalf of oscerd