You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2017/09/29 12:21:08 UTC
[cxf-fediz] branch master updated: FEDIZ-210 - Limit IdP request
parameter size
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf-fediz.git
The following commit(s) were added to refs/heads/master by this push:
new ba52bf9 FEDIZ-210 - Limit IdP request parameter size
ba52bf9 is described below
commit ba52bf9803355249a08aa6b649b777d3a21801ac
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri Sep 29 13:20:48 2017 +0100
FEDIZ-210 - Limit IdP request parameter size
---
.../service/idp/beans/ParameterSizeChecker.java | 60 ++++++++++++++++++++
.../apache/cxf/fediz/service/idp/domain/Idp.java | 12 +++-
.../service/idp/service/jpa/IdpDAOJPAImpl.java | 2 +
.../fediz/service/idp/service/jpa/IdpEntity.java | 10 ++++
.../WEB-INF/flows/federation-validate-request.xml | 10 ++++
.../webapp/WEB-INF/flows/saml-validate-request.xml | 10 ++++
.../org/apache/cxf/fediz/systests/idp/IdpTest.java | 66 ++++++++++++++++++++++
.../src/test/resources/realma/entities-realma.xml | 1 +
8 files changed, 170 insertions(+), 1 deletion(-)
diff --git a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/beans/ParameterSizeChecker.java b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/beans/ParameterSizeChecker.java
new file mode 100644
index 0000000..d4991c8
--- /dev/null
+++ b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/beans/ParameterSizeChecker.java
@@ -0,0 +1,60 @@
+/**
+ * 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.Map;
+
+import org.apache.cxf.fediz.service.idp.domain.Idp;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.stereotype.Component;
+import org.springframework.webflow.execution.RequestContext;
+
+/**
+ * Check the size of the parameters against the default value. "wresult", "SAMLRequest" and "SAMLResponse"
+ * are excluded from the check.
+ */
+@Component
+public class ParameterSizeChecker {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ParameterSizeChecker.class);
+
+ public boolean submit(Idp idp, RequestContext context) {
+ int maxParameterSize = idp.getMaxParameterSize();
+
+ if (!context.getRequestParameters().isEmpty()) {
+ Map<String, Object> parameters = context.getRequestParameters().asMap();
+ for (Map.Entry<String, Object> param : parameters.entrySet()) {
+ if (!skipCheck(param.getKey()) && param.getValue() != null
+ && param.getValue().toString().length() > maxParameterSize) {
+ LOG.debug("The " + param.getKey() + " parameter size " + param.getValue().toString().length()
+ + " exceeds the maximum allowed value of " + maxParameterSize);
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ private boolean skipCheck(String parameter) {
+ return "wresult".equals(parameter) || "SAMLRequest".equals(parameter)
+ || "SAMLResponse".equals(parameter);
+ }
+}
diff --git a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/domain/Idp.java b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/domain/Idp.java
index c41348c..1515cdd 100644
--- a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/domain/Idp.java
+++ b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/domain/Idp.java
@@ -36,7 +36,7 @@ import javax.xml.bind.annotation.XmlType;
"certificate", "certificatePassword", "provideIdpList", "useCurrentIdp", "hrds",
"rpSingleSignOutConfirmation", "supportedProtocols", "tokenTypesOffered", "claimTypesOffered",
"authenticationURIs", "applications", "trustedIdps", "id", "rpSingleSignOutCleanupConfirmation",
- "automaticRedirectToRpAfterLogout", "disableLogoutAddressValidation"})
+ "automaticRedirectToRpAfterLogout", "disableLogoutAddressValidation", "maxParameterSize"})
public class Idp implements Serializable {
private static final long serialVersionUID = -5570301342547139039L;
@@ -123,6 +123,8 @@ public class Idp implements Serializable {
private boolean disableLogoutAddressValidation;
+ private int maxParameterSize = 500;
+
@XmlAttribute
public int getId() {
return id;
@@ -322,4 +324,12 @@ public class Idp implements Serializable {
this.disableLogoutAddressValidation = disableLogoutAddressValidation;
}
+ public int getMaxParameterSize() {
+ return maxParameterSize;
+ }
+
+ public void setMaxParameterSize(int maxParameterSize) {
+ this.maxParameterSize = maxParameterSize;
+ }
+
}
diff --git a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpDAOJPAImpl.java b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpDAOJPAImpl.java
index 5458d67..360755a 100644
--- a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpDAOJPAImpl.java
+++ b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpDAOJPAImpl.java
@@ -296,6 +296,7 @@ public class IdpDAOJPAImpl implements IdpDAO {
entity.setRpSingleSignOutCleanupConfirmation(idp.isRpSingleSignOutCleanupConfirmation());
entity.setAutomaticRedirectToRpAfterLogout(idp.isAutomaticRedirectToRpAfterLogout());
entity.setDisableLogoutAddressValidation(idp.isDisableLogoutAddressValidation());
+ entity.setMaxParameterSize(idp.getMaxParameterSize());
entity.getAuthenticationURIs().clear();
for (Map.Entry<String, String> item : idp.getAuthenticationURIs().entrySet()) {
@@ -332,6 +333,7 @@ public class IdpDAOJPAImpl implements IdpDAO {
idp.setRpSingleSignOutCleanupConfirmation(entity.isRpSingleSignOutCleanupConfirmation());
idp.setAutomaticRedirectToRpAfterLogout(entity.isAutomaticRedirectToRpAfterLogout());
idp.setDisableLogoutAddressValidation(entity.isDisableLogoutAddressValidation());
+ idp.setMaxParameterSize(entity.getMaxParameterSize());
if (expandList != null && (expandList.contains("all") || expandList.contains("applications"))) {
for (ApplicationEntity item : entity.getApplications()) {
diff --git a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpEntity.java b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpEntity.java
index 60b09d9..27cd33e 100644
--- a/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpEntity.java
+++ b/services/idp-core/src/main/java/org/apache/cxf/fediz/service/idp/service/jpa/IdpEntity.java
@@ -141,6 +141,8 @@ public class IdpEntity {
private boolean disableLogoutAddressValidation;
+ private int maxParameterSize = 500;
+
public int getId() {
return id;
@@ -318,4 +320,12 @@ public class IdpEntity {
this.disableLogoutAddressValidation = disableLogoutAddressValidation;
}
+ public int getMaxParameterSize() {
+ return maxParameterSize;
+ }
+
+ public void setMaxParameterSize(int maxParameterSize) {
+ this.maxParameterSize = maxParameterSize;
+ }
+
}
diff --git a/services/idp/src/main/webapp/WEB-INF/flows/federation-validate-request.xml b/services/idp/src/main/webapp/WEB-INF/flows/federation-validate-request.xml
index 6e7fc80..594e500 100644
--- a/services/idp/src/main/webapp/WEB-INF/flows/federation-validate-request.xml
+++ b/services/idp/src/main/webapp/WEB-INF/flows/federation-validate-request.xml
@@ -22,6 +22,16 @@
xsi:schemaLocation="http://www.springframework.org/schema/webflow
http://www.springframework.org/schema/webflow/spring-webflow-2.0.xsd">
+ <decision-state id="checkParameterSize">
+ <on-entry>
+ <set name="flowScope.idpConfig" value="config.getIDP(fedizEntryPoint.getRealm())" />
+ <evaluate expression="parameterSizeChecker.submit(flowScope.idpConfig, flowRequestContext)"
+ result="flowScope.parametersOK"/>
+ </on-entry>
+ <if test="flowScope.parametersOK "
+ then="evaluateProtocol" else="viewBadRequest" />
+ </decision-state>
+
<decision-state id="evaluateProtocol">
<on-entry>
<set name="flowScope.idpConfig" value="config.getIDP(fedizEntryPoint.getRealm())" />
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 1f12890..00ef583 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
@@ -22,6 +22,16 @@
xsi:schemaLocation="http://www.springframework.org/schema/webflow
http://www.springframework.org/schema/webflow/spring-webflow-2.0.xsd">
+ <decision-state id="checkParameterSize">
+ <on-entry>
+ <set name="flowScope.idpConfig" value="config.getIDP(fedizEntryPoint.getRealm())" />
+ <evaluate expression="parameterSizeChecker.submit(flowScope.idpConfig, flowRequestContext)"
+ result="flowScope.parametersOK"/>
+ </on-entry>
+ <if test="flowScope.parametersOK "
+ then="evaluateProtocol" else="viewBadRequest" />
+ </decision-state>
+
<decision-state id="evaluateProtocol">
<on-entry>
<set name="flowScope.idpConfig" value="config.getIDP(fedizEntryPoint.getRealm())" />
diff --git a/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java b/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
index 4a3bfab..ee585e6 100644
--- a/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
+++ b/systests/idp/src/test/java/org/apache/cxf/fediz/systests/idp/IdpTest.java
@@ -765,6 +765,72 @@ public class IdpTest {
webClient.close();
}
+ // Send a query parameter that's too big
+ @org.junit.Test
+ public void testLargeQueryParameterRejected() throws Exception {
+ String url = "https://localhost:" + getIdpHttpsPort() + "/fediz-idp/federation?";
+ url += "wa=wsignin1.0";
+ url += "&whr=urn:org:apache:cxf:fediz:idp:realm-A";
+ url += "&wtrealm=urn:org:apache:cxf:fediz:fedizhelloworld";
+
+ StringBuilder sb = new StringBuilder("https://localhost:" + getRpHttpsPort() + "/"
+ + getServletContextName() + "/secure/fedservlet");
+ for (int i = 0; i < 100; i++) {
+ sb.append("aaaaaaaaaa");
+ }
+
+ url += "&wreply=" + sb.toString();
+
+ 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 wreply value");
+ } catch (FailingHttpStatusCodeException ex) {
+ Assert.assertEquals(ex.getStatusCode(), 400);
+ }
+
+ webClient.close();
+ }
+
+ // Send a query parameter that's bigger than the accepted default, but is allowed by configuration
+ @org.junit.Test
+ public void testLargeQueryParameterAccepted() throws Exception {
+ String url = "https://localhost:" + getIdpHttpsPort() + "/fediz-idp/federation?";
+ url += "wa=wsignin1.0";
+ url += "&whr=urn:org:apache:cxf:fediz:idp:realm-A";
+ url += "&wtrealm=urn:org:apache:cxf:fediz:fedizhelloworld";
+
+ StringBuilder sb = new StringBuilder("https://localhost:" + getRpHttpsPort()
+ + "/" + getServletContextName() + "/secure/fedservlet");
+ for (int i = 0; i < 50; i++) {
+ sb.append("aaaaaaaaaa");
+ }
+
+ url += "&wreply=" + sb.toString();
+
+ 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);
+ webClient.getPage(url);
+
+ webClient.close();
+ }
@Test
public void testIdPLogout() throws Exception {
diff --git a/systests/idp/src/test/resources/realma/entities-realma.xml b/systests/idp/src/test/resources/realma/entities-realma.xml
index 13269db..27ac5d0 100644
--- a/systests/idp/src/test/resources/realma/entities-realma.xml
+++ b/systests/idp/src/test/resources/realma/entities-realma.xml
@@ -36,6 +36,7 @@
<property name="stsUrl" value="https://localhost:${idp.https.port}/fediz-idp-sts/REALMA" />
<property name="idpUrl" value="https://localhost:${idp.https.port}/fediz-idp/federation" />
<property name="rpSingleSignOutConfirmation" value="true"/>
+ <property name="maxParameterSize" value="600"/>
<property name="supportedProtocols">
<util:list>
<value>http://docs.oasis-open.org/wsfed/federation/200706
--
To stop receiving notification emails like this one, please contact
['"commits@cxf.apache.org" <co...@cxf.apache.org>'].