Conversation
…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.
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
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.
From the CodeQL
java/unvalidated-url-redirectiontriage: 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
SAMLAwareServlet.ArtifactHandler,SAMLPOSTProfileServlet.doPost): once the artifact or the POSTed response had been verified and a session created, the browser was sent to theTARGETparameter as it came.SAMLUtils.postYNonly decides between POST and redirect against the POST to target URLs list; it does not restrict the target. Both servlets are mapped in the shippedweb.xml(/SAMLAwareServlet,/SAMLPOSTProfileServlet).RPSigninRequest→RPSigninResponse): thewreplyof an RP-initiated sign-in (thegotoparameter) was stored under thewctxhandle and redirected to after the identity provider round trip. WS-Federation has norelayStateUrlList.The fix
Both consult the realm's Valid goto URL list — the list the login
gotoparameter 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 nameVALIDATION→validationServiceinConfigurationInstanceImpl), becauseopenam-federation-librarycannot reachValidGotoUrlExtractorinopenam-core.RealmGotoUrlExtractorreadsopenam-auth-valid-goto-resourcesfor the realm;nullmeans "no list" (no plugin, no configuration for the realm, no attribute, or a read failure).RealmGotoUrlValidatorwraps it in aRedirectUrlValidator, with a package-private seam for tests.SAMLAwareServletandSAMLPOSTProfileServletcheckTARGETagainst the realm the assertion resolved to (sessionAttr[realm]) beforegenerateSession, answering 400invalidTargetSiteotherwise.RPSigninRequest.processresolves the SP realm first and checkswreplybeforeputReplyURL, throwingWSFederationException("invalidWreply")otherwise. The rejected URL is not echoed into the error page.Verification
RealmGotoUrlValidatorTest(8) andRealmGotoUrlExtractorTest(7), watched failing first on the missing classes;openam-federation-library192 tests, 0 failures;OpenFMcompiles. The servlet andprocess()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