Hardening: namespace-unqualified assertion selection in SAMLEndpoint.handleLoginResponse (XML Signature Wrapping defense-in-depth) #22

Summary

While reviewing this Keycloak fork I noticed a defense-in-depth weakness in how the login assertion is selected in SAMLEndpoint.handleLoginResponse. I reported it privately to First8 first. After a joint analysis we concluded it is not exploitable in the current code flow, so this is a code-robustness / bad-practice hardening item, not a known-exploitable vulnerability, and no CVE is being claimed. I am filing it publicly at the maintainers' request so the origin of the hardening is visible.

Where

In SAMLEndpoint.handleLoginResponse the assertion element is selected with a namespace-unqualified lookup (currently around line 476):

assertionElement = DocumentUtil.getElement(
    holder.getSamlDocument(),
    new QName(Objects.requireNonNull(JBossSAMLConstants.ASSERTION.get())));

Because the QName carries no namespace URI, this returns the first <Assertion> element anywhere in the document (for example one nested under samlp:Extensions, or one that is not a direct child of Response). Upstream Keycloak instead uses AssertionUtil.getAssertionElement, which is namespace-qualified and bound to the identity assertion.

This is the classic shape of an XML Signature Wrapping (XSW) weakness: the element whose signature is checked can, in principle, differ from the element the identity is read from (CWE-347 / CWE-290).

Why it is not exploitable in the current flow

handleLoginResponse is only reachable through handleSamlResponse, which runs signature verification first (verifySignature is invoked before the call to handleLoginResponse). Keycloak's XMLSignatureUtil.validate rejects a document that mixes an unsigned assertion among signed ones, which is exactly the wrapping payload (a signed attacker assertion plus an unsigned victim assertion). The wrapping document is therefore rejected before handleLoginResponse is ever reached.

First8 independently tested signature-confusion and signature-wrapping variants against handleSamlResponse and could not exploit the weakness, and I confirm that assessment. So the selection logic is a latent bad practice rather than a reachable bypass in the shipped code.

Suggested hardening (closes the whole class at once)

• Select the assertion via AssertionUtil.getAssertionElement (namespace-qualified, bound to the identity assertion) instead of a namespace-unqualified DocumentUtil.getElement.
• Reject responses that contain more than one assertion.
• Reject assertions that are not a direct child of Response.

Together these remove the gap between the verified element and the consumed element, independent of the order in which signature verification runs.

Attribution

Reported by Ray Sabee (Whitehat Security), https://whitehatsecurity.nl, GitHub @raysabee. Thanks to the First8 team (Akke Toeter) for the quick, open and professional handling, and for independently re-verifying the exploitability assessment.