Skip to content

Check the SAML 1.x TARGET and the WS-Federation wreply against the realm's valid goto URLs - #1135

Merged
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:saml1-wsfed-redirect-target
Sep 29, 2026
Merged

vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:saml1-wsfed-redirect-target

Conversation

@vharseko

Copy link
Copy Markdown
Member

From the CodeQL java/unvalidated-url-redirection triage: of the redirects driven by a request parameter, these four are the ones with no allow-list anywhere on their path (#82, #83, #84, #95).

What was open

  • SAML 1.x destination site (SAMLAwareServlet.ArtifactHandler, SAMLPOSTProfileServlet.doPost): once the artifact or the POSTed response had been verified and a session created, the browser was sent to the TARGET parameter as it came. SAMLUtils.postYN only decides between POST and redirect against the POST to target URLs list; it does not restrict the target. Both servlets are mapped in the shipped web.xml (/SAMLAwareServlet, /SAMLPOSTProfileServlet).
  • WS-Federation service provider (RPSigninRequest → RPSigninResponse): the wreply of an RP-initiated sign-in (the goto parameter) was stored under the wctx handle and redirected to after the identity provider round trip. WS-Federation has no relayStateUrlList.

The fix

Both consult the realm's Valid goto URL list — the list the login goto parameter is checked against — with the same rules: a relative URL passes, a scheme other than http(s) does not, and a realm that configures no list restricts nothing. The list is read through the federation configuration plugin (ConfigurationManager, new component name VALIDATION → validationService in ConfigurationInstanceImpl), because openam-federation-library cannot reach ValidGotoUrlExtractor in openam-core.

  • RealmGotoUrlExtractor reads openam-auth-valid-goto-resources for the realm; null means "no list" (no plugin, no configuration for the realm, no attribute, or a read failure).
  • RealmGotoUrlValidator wraps it in a RedirectUrlValidator, with a package-private seam for tests.
  • SAMLAwareServlet and SAMLPOSTProfileServlet check TARGET against the realm the assertion resolved to (sessionAttr[realm]) before generateSession, answering 400 invalidTargetSite otherwise.
  • RPSigninRequest.process resolves the SP realm first and checks wreply before putReplyURL, throwing WSFederationException("invalidWreply") otherwise. The rejected URL is not echoed into the error page.

Verification

RealmGotoUrlValidatorTest (8) and RealmGotoUrlExtractorTest (7), watched failing first on the missing classes; openam-federation-library 192 tests, 0 failures; OpenFM compiles. The servlet and process() wiring has no unit test: exercising them needs a SAML 1.x / WS-Federation harness with metadata that the module does not have.

Notes for review

  • The extractor does not cache; SMS caches the service configuration itself and SAML 1.x / WS-Federation sign-ins are not hot paths.
  • Permit-on-empty is deliberate: it is the login goto's behaviour, and a deployment that never configured the realm list keeps working. Protection is switched on by populating Valid goto URL for the realm, as for the login page.

…alm's valid goto URLs

After a successful SAML 1.x artifact or POST profile exchange the destination
site redirected to the TARGET parameter as it came, and the WS-Federation
service provider redirected to the wreply it had stored at sign-in; neither
protocol carries an allow-list of its own. Both now consult the realm's
Valid goto URL list - the one the login goto parameter is checked against -
through the federation configuration plugin, before a session is created
for the TARGET and before the wreply is stored. A realm that configures no
list restricts nothing, as for the login goto.
@vharseko vharseko added security Security fix or hardening (CVE, GHSA, XSS/CSRF/SSRF) java Pull requests that update java code tests Test suite: coverage, fixtures, or test infrastructure saml SAML / SAML2 federation labels Sep 18, 2026

@maximthomas maximthomas 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.

praise: the SAML 1.x checks refuse before anything is created for the request.

  • SAMLAwareServlet.ArtifactHandler and SAMLPOSTProfileServlet.doPost call RealmGotoUrlValidator.isValid before SAMLUtils.generateSession (SAMLAwareServlet.java:348 vs :358, SAMLPOSTProfileServlet.java:443 vs :453), so a refused TARGET never gets a session.
  • The list is read through the federation ConfigurationManager plugin (VALIDATION → validationService) rather than a new dependency on openam-core, and the WS-Fed refusal does not echo the rejected URL.

@vharseko
vharseko merged commit 09a2e12 into OpenIdentityPlatform:master Sep 29, 2026
15 checks passed
@vharseko
vharseko deleted the saml1-wsfed-redirect-target branch September 29, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java Pull requests that update java code saml SAML / SAML2 federation security Security fix or hardening (CVE, GHSA, XSS/CSRF/SSRF) tests Test suite: coverage, fixtures, or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants