Skip to content

CAMEL-24437/24439/24441/24443/24454: secure-default fixes, backport to camel-4.18.x - #25946

Open
oscerd wants to merge 5 commits into
apache:camel-4.18.xfrom
oscerd:backport/glasswing-secure-defaults-4.18.x
Open

CAMEL-24437/24439/24441/24443/24454: secure-default fixes, backport to camel-4.18.x#25946
oscerd wants to merge 5 commits into
apache:camel-4.18.xfrom
oscerd:backport/glasswing-secure-defaults-4.18.x

Conversation

@oscerd

@oscerd oscerd commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Backport to camel-4.18.x of five fixes already reviewed and merged on main. 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.

The same set is going to camel-4.22.x in #25944.

What changes for an existing deployment:

  • camel-mllp stops writing HL7 payload content to the log unless logPhi=true is set explicitly. Previously it defaulted to on, and two paths logged regardless of the setting.
  • camel-knative with SSL enabled but no truststore configured no longer falls back to trusting every certificate. Such a deployment now needs a truststore, or trustAll set deliberately.
  • camel-oauth rejects an authorization-code callback whose state does 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.
  • camel-shiro verifies the presented credentials on every exchange rather than accepting a subject already bound to the thread. alwaysReauthenticate=false keeps the old behaviour.
  • camel-crypto-pgp refuses a decryption whose message has no integrity packet. requireIntegrityProtection=false restores the old acceptance.

Deviations from a straight cherry-pick. This branch has diverged from main in MllpSocketBuffer, in the test layer, and across the generated model files:

  • MllpSocketBuffer keeps this branch's synchronized methods rather than main's ReentrantLock form, and its existing readFrom structure. The substantive change is carried across: convertToLoggableString in the partial-payload warning, plus the new toLoggableStringAndReset.
  • The generated model, catalog, XSD and YAML DSL files for CAMEL-24441 are regenerated from this branch rather than taken from main. camel-java-io and the canonical/model YAML schemas do not exist here, so those files are dropped. camel-spring.xsd gains the new attribute by hand, in the exact wording the generator produced for camel-xml-io.xsd on this branch.
  • OAuthProcessorFailClosedTest uses JUnit assertions (no assertj test dependency here, and the rest of that file is JUnit), and ShiroAuthenticationCredentialAlwaysCheckedTest imports camel-test-junit5. Each is folded into its own commit.

The upgrade-guide entries are not included: the guides for every line live on main.

Built and tested per module on this branch, including the new and touched tests: PGPRequireIntegrityProtectionTest, PGPDataFormatTest, KnativeSslClientOptionsTrustTest, LogPhiTest, Hl7UtilTest, OAuthProcessorFailClosedTest, ShiroAuthenticationCredentialAlwaysCheckedTest.

Claude Code on behalf of oscerd

oscerd and others added 5 commits August 31, 2026 14:41
@oscerd @claude
CAMEL-24454: camel-mllp - default logPhi to false and honour it on th…
62d96f9 
…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)

MllpSocketBuffer keeps this branch's synchronized methods rather than main's
ReentrantLock form, and its existing readFrom structure; the substantive change,
convertToLoggableString in the partial-payload warning plus the new
toLoggableStringAndReset, is carried across.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@oscerd @claude
CAMEL-24443: camel-knative - do not trust every certificate when SSL …
26e2f3f 
…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>
@oscerd @claude
CAMEL-24437: camel-oauth - bind the authorization code callback to th…
7875453 
…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>
@oscerd @claude
CAMEL-24439: camel-shiro - verify the presented credentials on every …
2b171f0 
…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>
@oscerd @claude
CAMEL-24441: camel-crypto-pgp - require integrity protection when dec…
f5a245e 
…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)

The generated model, catalog, XSD and YAML DSL files are regenerated from this
branch rather than taken from main, since the two lines have drifted. camel-java-io
and the canonical/model YAML schemas do not exist here, so those files are dropped.
camel-spring.xsd gains the attribute by hand, in the wording the generator produced
for camel-xml-io.xsd on this branch.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@oscerd
oscerd force-pushed the backport/glasswing-secure-defaults-4.18.x branch from 5af401a to f5a245e Compare August 31, 2026 12:42
@oscerd oscerd mentioned this pull request Aug 31, 2026
Open
gnodet
gnodet approved these changes Aug 31, 2026
View reviewed changes

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean backport of five security-default hardening fixes from main to camel-4.18.x. All changes are functionally identical to the main-branch originals, and each includes new tests validating both the stricter default and the opt-out path.

Highlights:

  • Each change provides a clear backward-compatibility opt-out (logPhi=true, trust.all=true, requireIntegrityProtection=false, alwaysReauthenticate=false).
  • The Shiro fix correctly removes the shortcut where a matching username on a thread-bound Subject skipped credential verification — currentUser.login(token) is now called on every exchange.
  • The OAuth state-parameter binding uses MessageDigest.isEqual for timing-safe comparison, the state is single-use (consumed via removeValue), and error paths stop the route — consistent with RFC 6749 §10.12.
  • The PGP integrity-protection check and the Knative trust-all removal are both clean adaptations of the main versions.

📋 PR Metadata

Aspect Current Suggested
Labels (none) bug
Milestone (none) 4.18.5

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gnodet gnodet gnodet approved these changes
@Croway Croway Croway approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@oscerd @gnodet @Croway