Repository: cxf-fediz Updated Branches: refs/heads/master d1c0d7e23 -> a1111af73
Check the SAML SSO RACS URL against a regular expression constraint Project: http://git-wip-us.apache.org/repos/asf/cxf-fediz/repo Commit: http://git-wip-us.apache.org/repos/asf/cxf-fediz/commit/a1111af7 Tree: http://git-wip-us.apache.org/repos/asf/cxf-fediz/tree/a1111af7 Diff: http://git-wip-us.apache.org/repos/asf/cxf-fediz/diff/a1111af7 Branch: refs/heads/master Commit: a1111af73ce75b93f846b01a1e68521b4b1bb1df Parents: d1c0d7e Author: Colm O hEigeartaigh <[email protected]> Authored: Sat Mar 26 17:46:06 2016 +0000 Committer: Colm O hEigeartaigh <[email protected]> Committed: Sat Mar 26 17:46:06 2016 +0000 ---------------------------------------------------------------------- .../idp/beans/PassiveRequestorValidator.java | 82 ++++++++++++++++++++ .../idp/beans/wsfed/WreplyValidator.java | 79 ------------------- .../WEB-INF/flows/federation-signin-request.xml | 2 +- .../WEB-INF/flows/saml-signin-request.xml | 28 ++++++- .../WEB-INF/flows/saml-validate-request.xml | 23 ++---- .../apache/cxf/fediz/systests/idp/IdpTest.java | 65 ++++++++++++++-- 6 files changed, 176 insertions(+), 103 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a1111af7/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/PassiveRequestorValidator.java ---------------------------------------------------------------------- diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/PassiveRequestorValidator.java b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/PassiveRequestorValidator.java new file mode 100644 index 0000000..d7e5bbc --- /dev/null +++ b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/PassiveRequestorValidator.java @@ -0,0 +1,82 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.fediz.service.idp.beans; + +import java.util.regex.Matcher; + +import org.apache.commons.validator.routines.UrlValidator; +import org.apache.cxf.fediz.service.idp.domain.Application; +import org.apache.cxf.fediz.service.idp.domain.Idp; +import org.apache.cxf.fediz.service.idp.util.WebUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; +import org.springframework.webflow.execution.RequestContext; + +/** + * This class is responsible to validate the 'wreply' parameter for WS-Federation, or else the + * AssertionConsumer URL address for SAML SSO, by comparing it to a regular expression. + */ +@Component +public class PassiveRequestorValidator { + + private static final Logger LOG = LoggerFactory.getLogger(PassiveRequestorValidator.class); + + public boolean isValid(RequestContext context, String endpointAddress, String realm) + throws Exception { + if (endpointAddress == null) { + return true; + } + + Idp idpConfig = (Idp) WebUtils.getAttributeFromFlowScope(context, "idpConfig"); + Application serviceConfig = idpConfig.findApplication(realm); + if (serviceConfig == null) { + LOG.warn("No service config found for " + realm); + return true; + } + + // The endpointAddress address must match the passive endpoint requestor constraint + // (if it is specified) + // Also, it must be a valid URL + start with https + // Validate it first using commons-validator + UrlValidator urlValidator = new UrlValidator(UrlValidator.ALLOW_LOCAL_URLS + + UrlValidator.ALLOW_ALL_SCHEMES); + if (!urlValidator.isValid(endpointAddress)) { + LOG.warn("The given endpointAddress parameter {} is not a valid URL", endpointAddress); + return false; + } + + if (serviceConfig.getCompiledPassiveRequestorEndpointConstraint() == null) { + LOG.warn("No passive requestor endpoint constraint is configured for the application. " + + "This could lead to a malicious redirection attack"); + return true; + } + + Matcher matcher = + serviceConfig.getCompiledPassiveRequestorEndpointConstraint().matcher(endpointAddress); + if (!matcher.matches()) { + LOG.error("The endpointAddress value of {} does not match any of the passive requestor values", + endpointAddress); + return false; + } + + return true; + } + +} http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a1111af7/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/wsfed/WreplyValidator.java ---------------------------------------------------------------------- diff --git a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/wsfed/WreplyValidator.java b/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/wsfed/WreplyValidator.java deleted file mode 100644 index afc8607..0000000 --- a/services/idp/src/main/java/org/apache/cxf/fediz/service/idp/beans/wsfed/WreplyValidator.java +++ /dev/null @@ -1,79 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.cxf.fediz.service.idp.beans.wsfed; - -import java.util.regex.Matcher; - -import org.apache.commons.validator.routines.UrlValidator; -import org.apache.cxf.fediz.service.idp.domain.Application; -import org.apache.cxf.fediz.service.idp.domain.Idp; -import org.apache.cxf.fediz.service.idp.util.WebUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Component; -import org.springframework.webflow.execution.RequestContext; - -/** - * This class is responsible to validate the 'wreply' parameter - */ -@Component -public class WreplyValidator { - - private static final Logger LOG = LoggerFactory.getLogger(WreplyValidator.class); - - public boolean isValid(RequestContext context, String wreply, String realm) - throws Exception { - if (wreply == null) { - return true; - } - - Idp idpConfig = (Idp) WebUtils.getAttributeFromFlowScope(context, "idpConfig"); - Application serviceConfig = idpConfig.findApplication(realm); - if (serviceConfig == null) { - LOG.warn("No service config found for " + realm); - return true; - } - - // The wreply address must match the passive endpoint requestor constraint (if it is specified) - // Also, it must be a valid URL + start with https - // Validate it first using commons-validator - UrlValidator urlValidator = new UrlValidator(UrlValidator.ALLOW_LOCAL_URLS - + UrlValidator.ALLOW_ALL_SCHEMES); - if (!urlValidator.isValid(wreply)) { - LOG.warn("The given wreply parameter {} is not a valid URL", wreply); - return false; - } - - if (serviceConfig.getCompiledPassiveRequestorEndpointConstraint() == null) { - LOG.warn("No passive requestor endpoint constraint is configured for the application. " - + "This could lead to a malicious redirection attack"); - return true; - } - - Matcher matcher = serviceConfig.getCompiledPassiveRequestorEndpointConstraint().matcher(wreply); - if (!matcher.matches()) { - LOG.error("The wreply value of {} does not match any of the passive requestor values", - wreply); - return false; - } - - return true; - } - -} http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a1111af7/services/idp/src/main/webapp/WEB-INF/flows/federation-signin-request.xml ---------------------------------------------------------------------- diff --git a/services/idp/src/main/webapp/WEB-INF/flows/federation-signin-request.xml b/services/idp/src/main/webapp/WEB-INF/flows/federation-signin-request.xml index 4280edc..5137047 100644 --- a/services/idp/src/main/webapp/WEB-INF/flows/federation-signin-request.xml +++ b/services/idp/src/main/webapp/WEB-INF/flows/federation-signin-request.xml @@ -102,7 +102,7 @@ </action-state> <action-state id="validateWReply"> - <evaluate expression="wreplyValidator.isValid(flowRequestContext, flowScope.wreply, flowScope.wtrealm)"/> + <evaluate expression="passiveRequestorValidator.isValid(flowRequestContext, flowScope.wreply, flowScope.wtrealm)"/> <transition on="yes" to="requestRpToken" /> <transition on="no" to="viewBadRequest" /> </action-state> http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a1111af7/services/idp/src/main/webapp/WEB-INF/flows/saml-signin-request.xml ---------------------------------------------------------------------- diff --git a/services/idp/src/main/webapp/WEB-INF/flows/saml-signin-request.xml b/services/idp/src/main/webapp/WEB-INF/flows/saml-signin-request.xml index cc7b0f6..a5c16f1 100644 --- a/services/idp/src/main/webapp/WEB-INF/flows/saml-signin-request.xml +++ b/services/idp/src/main/webapp/WEB-INF/flows/saml-signin-request.xml @@ -26,6 +26,7 @@ <input name="idpConfig" /> <input name="SAMLRequest" /> <input name="RelayState" /> + <input name="Signature" /> <decision-state id="signInEntryPoint"> <on-entry> @@ -63,13 +64,38 @@ <set name="flowScope.idpToken" value="externalContext.sessionMap[flowScope.homerealm]" /> </transition> </action-state> + + <action-state id="parseAndValidateSAMLRequest"> + <on-entry> + <evaluate expression="authnRequestParser.parseSAMLRequest(flowRequestContext, flowScope.idpConfig, + flowScope.SAMLRequest)" /> + <evaluate expression="authnRequestParser.retrieveRealm(flowRequestContext)" + result="flowScope.realm"/> + </on-entry> + <evaluate expression="authnRequestValidator.validateAuthnRequest(flowRequestContext, flowScope.idpConfig, + flowScope.Signature, flowScope.RelayState, + flowScope.SAMLRequest, flowScope.realm)" /> + <transition to="validateEndpointAddress"/> + <transition on-exception="org.apache.cxf.fediz.core.exception.ProcessingException" to="viewBadRequest" /> + </action-state> + + <action-state id="validateEndpointAddress"> + <on-entry> + <evaluate expression="authnRequestParser.retrieveConsumerURL(flowRequestContext)" + result="flowScope.consumerURL"/> + </on-entry> + <evaluate expression="passiveRequestorValidator.isValid(flowRequestContext, flowScope.consumerURL, flowScope.realm)"/> + <transition on="yes" to="requestRpToken" /> + <transition on="no" to="viewBadRequest" /> + </action-state> <!-- ============================================================================================================= --> <!-- normal exit point --> - <end-state id="parseAndValidateSAMLRequest"> + <end-state id="requestRpToken"> <output name="homerealm" value="flowScope.homerealm" /> <output name="idpToken" value="flowScope.idpToken" /> + <output name="saml_authn_request" value="flowScope.saml_authn_request" /> </end-state> <!-- abnormal exit point : Http 400 Bad Request --> http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a1111af7/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml ---------------------------------------------------------------------- diff --git a/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml b/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml index 4e008eb..841a5d5 100644 --- a/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml +++ b/services/idp/src/main/webapp/WEB-INF/flows/saml-validate-request.xml @@ -40,34 +40,23 @@ <input name="idpConfig" value="flowScope.idpConfig" /> <input name="SAMLRequest" value="flowScope.SAMLRequest" /> <input name="RelayState" value="flowScope.RelayState" /> + <input name="Signature" value="flowScope.Signature" /> <output name="homerealm" /> <output name="idpToken" /> <output name="trusted_idp_context" /> + <output name="saml_authn_request" /> - <transition on="parseAndValidateSAMLRequest" to="parseAndValidateSAMLRequest"> + <transition on="requestRpToken" to="requestRpToken"> <set name="flowScope.homerealm" value="currentEvent.attributes.homerealm" /> <set name="flowScope.idpToken" value="currentEvent.attributes.idpToken" /> + <set name="flowScope.saml_authn_request" value="currentEvent.attributes.saml_authn_request" /> </transition> <transition on="viewBadRequest" to="viewBadRequest" /> <transition on="scInternalServerError" to="scInternalServerError" /> <transition on="redirectToLocalIDP" to="redirectToLocalIDP" /> </subflow-state> - <action-state id="parseAndValidateSAMLRequest"> - <on-entry> - <evaluate expression="authnRequestParser.parseSAMLRequest(flowRequestContext, flowScope.idpConfig, - flowScope.SAMLRequest)" /> - <evaluate expression="authnRequestParser.retrieveRealm(flowRequestContext)" - result="flowScope.realm"/> - </on-entry> - <evaluate expression="authnRequestValidator.validateAuthnRequest(flowRequestContext, flowScope.idpConfig, - flowScope.Signature, flowScope.RelayState, - flowScope.SAMLRequest, flowScope.realm)" /> - <transition to="requestRpToken"/> - <transition on-exception="org.apache.cxf.fediz.core.exception.ProcessingException" to="viewBadRequest" /> - </action-state> - <!-- produce RP security token (as String type) --> <action-state id="requestRpToken"> <on-entry> @@ -84,12 +73,12 @@ <action-state id="produceSAMLResponse"> <on-entry> + <evaluate expression="authnRequestParser.retrieveConsumerURL(flowRequestContext)" + result="flowScope.consumerURL"/> <evaluate expression="authnRequestParser.retrieveRequestId(flowRequestContext)" result="flowScope.requestId"/> <evaluate expression="authnRequestParser.retrieveRequestIssuer(flowRequestContext)" result="flowScope.requestIssuer"/> - <evaluate expression="authnRequestParser.retrieveConsumerURL(flowRequestContext)" - result="flowScope.consumerURL"/> </on-entry> <evaluate expression="samlResponseCreator.createSAMLResponse(flowRequestContext, flowScope.idpConfig, flowScope.rpTokenElement, flowScope.consumerURL, flowScope.requestId, flowScope.requestIssuer)" http://git-wip-us.apache.org/repos/asf/cxf-fediz/blob/a1111af7/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java ---------------------------------------------------------------------- diff --git a/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java b/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java index 0a165c5..cfd7caa 100644 --- a/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java +++ b/systests/samlsso/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java @@ -172,7 +172,8 @@ public class IdpTest { Document doc = DOMUtils.createDocument(); doc.appendChild(doc.createElement("root")); // Create the AuthnRequest - String consumerURL = "https://localhost/acsa";; + String consumerURL = "https://localhost:"; + getRpHttpsPort() + "/" + + getServletContextName() + "/secure/fedservlet"; AuthnRequest authnRequest = new DefaultAuthnRequestBuilder().createAuthnRequest( null, "urn:org:apache:cxf:fediz:fedizhelloworld", consumerURL @@ -262,9 +263,11 @@ public class IdpTest { Document doc = DOMUtils.createDocument(); doc.appendChild(doc.createElement("root")); // Create the AuthnRequest + String consumerURL = "https://localhost:"; + getRpHttpsPort() + "/" + + getServletContextName() + "/secure/fedservlet"; AuthnRequest authnRequest = new DefaultAuthnRequestBuilder().createAuthnRequest( - null, "urn:org:apache:cxf:fediz:fedizhelloworld-xyz", "https://localhost/acsa"; + null, "urn:org:apache:cxf:fediz:fedizhelloworld-xyz", consumerURL ); authnRequest.setDestination("https://localhost:"; + getIdpHttpsPort() + "/fediz-idp/saml"); signAuthnRequest(authnRequest); @@ -307,9 +310,11 @@ public class IdpTest { Document doc = DOMUtils.createDocument(); doc.appendChild(doc.createElement("root")); // Create the AuthnRequest + String consumerURL = "https://localhost:"; + getRpHttpsPort() + "/" + + getServletContextName() + "/secure/fedservlet"; AuthnRequest authnRequest = new DefaultAuthnRequestBuilder().createAuthnRequest( - null, "urn:org:apache:cxf:fediz:fedizhelloworld", "https://localhost/acsa"; + null, "urn:org:apache:cxf:fediz:fedizhelloworld", consumerURL ); signAuthnRequest(authnRequest); @@ -351,9 +356,11 @@ public class IdpTest { Document doc = DOMUtils.createDocument(); doc.appendChild(doc.createElement("root")); // Create the AuthnRequest + String consumerURL = "https://localhost:"; + getRpHttpsPort() + "/" + + getServletContextName() + "/secure/fedservlet"; AuthnRequest authnRequest = new DefaultAuthnRequestBuilder().createAuthnRequest( - null, "urn:org:apache:cxf:fediz:fedizhelloworld", "https://localhost/acsa"; + null, "urn:org:apache:cxf:fediz:fedizhelloworld", consumerURL ); authnRequest.setDestination("https://localhost:"; + getIdpHttpsPort() + "/fediz-idp/saml"); @@ -395,7 +402,8 @@ public class IdpTest { Document doc = DOMUtils.createDocument(); doc.appendChild(doc.createElement("root")); // Create the AuthnRequest - String consumerURL = "https://localhost/acsa";; + String consumerURL = "https://localhost:"; + getRpHttpsPort() + "/" + + getServletContextName() + "/secure/fedservlet"; AuthnRequest authnRequest = new DefaultAuthnRequestBuilder().createAuthnRequest( null, "urn:org:apache:cxf:fediz:fedizhelloworld", consumerURL @@ -500,6 +508,53 @@ public class IdpTest { webClient.close(); } + @org.junit.Test + public void testUnknownRACS() throws Exception { + OpenSAMLUtil.initSamlEngine(); + + // Create SAML AuthnRequest + Document doc = DOMUtils.createDocument(); + doc.appendChild(doc.createElement("root")); + // Create the AuthnRequest + String consumerURL = "https://localhost:"; + getRpHttpsPort() + "/" + + getServletContextName() + "/insecure/fedservlet"; + AuthnRequest authnRequest = + new DefaultAuthnRequestBuilder().createAuthnRequest( + null, "urn:org:apache:cxf:fediz:fedizhelloworld", consumerURL + ); + authnRequest.setDestination("https://localhost:"; + getIdpHttpsPort() + "/fediz-idp/saml"); + signAuthnRequest(authnRequest); + + Element authnRequestElement = OpenSAMLUtil.toDom(authnRequest, doc); + String authnRequestEncoded = encodeAuthnRequest(authnRequestElement); + + String urlEncodedRequest = URLEncoder.encode(authnRequestEncoded, "UTF-8"); + + String relayState = UUID.randomUUID().toString(); + String url = "https://localhost:"; + getIdpHttpsPort() + "/fediz-idp/saml?"; + url += SSOConstants.RELAY_STATE + "=" + relayState; + url += "&" + SSOConstants.SAML_REQUEST + "=" + urlEncodedRequest; + + String user = "alice"; + String password = "ecila"; + + final WebClient webClient = new WebClient(); + webClient.getOptions().setUseInsecureSSL(true); + webClient.getCredentialsProvider().setCredentials( + new AuthScope("localhost", Integer.parseInt(getIdpHttpsPort())), + new UsernamePasswordCredentials(user, password)); + + webClient.getOptions().setJavaScriptEnabled(false); + try { + webClient.getPage(url); + Assert.fail("Failure expected on a bad RACS URL"); + } catch (FailingHttpStatusCodeException ex) { + Assert.assertEquals(ex.getStatusCode(), 400); + } + + webClient.close(); + } + private String encodeAuthnRequest(Element authnRequest) throws IOException { String requestMessage = DOM2Writer.nodeToString(authnRequest);
