[java-opensaml] branch main updated: OSJ-427: Simple signature verification fails to detect parameter ...
Brent Putman
putmanb at georgetown.edu
Thu Mar 13 18:44:45 UTC 2025
This is an automated email from the git hooks/post-receive script.
putmanb pushed a commit to branch main
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=4794522242dd9a6b7d3f425f7bda5d49b42327b7
The following commit(s) were added to refs/heads/main by this push:
new 479452224 OSJ-427: Simple signature verification fails to detect parameter ...
479452224 is described below
commit 4794522242dd9a6b7d3f425f7bda5d49b42327b7
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Mar 12 20:59:29 2025 -0400
OSJ-427: Simple signature verification fails to detect parameter ...
Simple signature verification fails to detect parameter smuggling.
This implements a MessageHandler that validates an HttpServletRequest
against specified requirements.
---
opensaml-messaging-impl/pom.xml | 13 ++
...pServletRequestParametersValidationHandler.java | 173 ++++++++++++++++
...vletRequestParametersValidationHandlerTest.java | 220 +++++++++++++++++++++
3 files changed, 406 insertions(+)
diff --git a/opensaml-messaging-impl/pom.xml b/opensaml-messaging-impl/pom.xml
index e176dd050..ca2c4e343 100644
--- a/opensaml-messaging-impl/pom.xml
+++ b/opensaml-messaging-impl/pom.xml
@@ -35,8 +35,16 @@
<groupId>${shib-shared.groupId}</groupId>
<artifactId>shib-networking</artifactId>
</dependency>
+ <dependency>
+ <groupId>${shib-shared.groupId}</groupId>
+ <artifactId>shib-support</artifactId>
+ </dependency>
<!-- Provided Dependencies -->
+ <dependency>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
+ </dependency>
<!-- Runtime Dependencies -->
@@ -61,6 +69,11 @@
<artifactId>spring-core</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>${spring.groupId}</groupId>
+ <artifactId>spring-test</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
diff --git a/opensaml-messaging-impl/src/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandler.java b/opensaml-messaging-impl/src/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandler.java
new file mode 100644
index 000000000..442497144
--- /dev/null
+++ b/opensaml-messaging-impl/src/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandler.java
@@ -0,0 +1,173 @@
+/*
+ * Licensed 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.opensaml.messaging.handler.impl;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.messaging.handler.AbstractHttpServletRequestMessageHandler;
+import org.opensaml.messaging.handler.MessageHandlerException;
+import org.slf4j.Logger;
+
+import jakarta.servlet.http.HttpServletRequest;
+import net.shibboleth.shared.collection.CollectionSupport;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.StringSupport;
+
+/**
+ * Message handler that validates HTTP request parameters for required presence, uniqueness and mutual exclusivity.
+ */
+public class HttpServletRequestParametersValidationHandler extends AbstractHttpServletRequestMessageHandler {
+
+ /** Logger. */
+ @Nonnull private Logger log = LoggerFactory.getLogger(HttpServletRequestParametersValidationHandler.class);
+
+ /** Required parameters. */
+ @Nonnull private Set<String> requiredParameters = CollectionSupport.emptySet();
+
+ /** Unique parameters. */
+ @Nonnull private Set<String> uniqueParameters = CollectionSupport.emptySet();
+
+ /** Mutually exclusive parameters */
+ @Nonnull private Set<Set<String>> mutuallyExclusiveParameters = CollectionSupport.emptySet();
+
+ /**
+ * Get the required parameters.
+ *
+ * <p>A required parameter must be present in the request.</p>
+ *
+ * @return the required parameters
+ */
+ @Nonnull public Set<String> getRequiredParameters() {
+ return requiredParameters;
+ }
+
+ /**
+ * Set the required parameters.
+ *
+ * <p>A required parameter must be present in the request.</p>
+ *
+ * @param params the required parameters
+ */
+ @Nonnull public void setRequiredParameters(@Nullable final Set<String> params) {
+ checkSetterPreconditions();
+ requiredParameters = CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(params));
+ }
+
+ /**
+ * Get the unique parameters.
+ *
+ * <p>A unique parameter must have at most 1 value.</p>
+ *
+ * @return the unique parameters
+ */
+ @Nonnull public Set<String> getUniqueParameters() {
+ return uniqueParameters;
+ }
+
+ /**
+ * Set the unique parameters.
+ *
+ * <p>A unique parameter must have at most 1 value.</p>
+ *
+ * @param params the unique parameters
+ */
+ @Nonnull public void setUniqueParameters(@Nullable final Set<String> params) {
+ checkSetterPreconditions();
+ uniqueParameters = CollectionSupport.copyToSet(StringSupport.normalizeStringCollection(params));
+ }
+
+ /**
+ * Get the mutually exclusive parameters.
+ *
+ * <p>A request may not contain more then 1 parameter from an exclusivity set (the "inner" set(s) configured).
+ * Multiple exclusivity sets may be specified.</p>
+ *
+ * @return the mutually exclusive parameters
+ */
+ @Nonnull public Set<Set<String>> getMutuallyExclusiveParameters() {
+ return mutuallyExclusiveParameters;
+ }
+
+ /**
+ * Set the mutually exclusive parameters.
+ *
+ * <p>A request may not contain more then 1 parameter from an exclusivity set (the "inner" set(s) configured).
+ * Multiple exclusivity sets may be specified.</p>
+ *
+ * @param params the mutually exclusive parameters
+ */
+ @Nonnull public void setMutuallyExclusiveParameters(@Nonnull final Set<Set<String>> params) {
+ checkSetterPreconditions();
+ if (params == null) {
+ mutuallyExclusiveParameters = CollectionSupport.emptySet();
+ } else {
+ mutuallyExclusiveParameters = params.stream()
+ .map(StringSupport::normalizeStringCollection)
+ .map(CollectionSupport::copyToSet)
+ .collect(CollectionSupport.nonnullCollector(Collectors.toSet())).get();
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void doInvoke(@Nonnull final MessageContext messageContext) throws MessageHandlerException {
+ final HttpServletRequest request = getHttpServletRequest();
+ final Set<String> requestParams = request.getParameterMap().keySet();
+
+ log.debug("{} Evaluating request for required parameters: {}", getLogPrefix(), getRequiredParameters());
+ for (final String param : getRequiredParameters()) {
+ if (!requestParams.contains(param)) {
+ log.warn("{} HTTP request did not contain required parameter: {}", getLogPrefix(), param);
+ throw new MessageHandlerException("HTTP request did not contain required parameter: " + param);
+ }
+ }
+
+ log.debug("{} Evaluating request for unique parameters: {}", getLogPrefix(), getUniqueParameters());
+ for (final String param : getUniqueParameters()) {
+ final String[] values = request.getParameterValues(param);
+ if (values != null && values.length > 1) {
+ log.warn("{} HTTP request contained {} values for parameter: {}", getLogPrefix(), values.length, param);
+ throw new MessageHandlerException("HTTP request contained multiple values for parameter: " + param);
+ }
+ }
+
+ log.debug("{} Evaluating request for mutually exclusive parameters: {}", getLogPrefix(),
+ getMutuallyExclusiveParameters());
+ for (final Set<String> group : getMutuallyExclusiveParameters()) {
+ if (group.size() < 2) {
+ log.debug("{} Exclusivity group had < 2 members, skipping evaluation: ", getLogPrefix(), group);
+ continue;
+ }
+
+ final Set<String> groupIntersection = requestParams.stream()
+ .filter(p -> group.contains(p))
+ .collect(Collectors.toSet());
+
+ if (groupIntersection.size() > 1) {
+ log.warn("{} HTTP request contained mutuallly exclusive parameters: {}", getLogPrefix(),
+ groupIntersection);
+ throw new MessageHandlerException("HTTP request contained mutually exclusivity parameters: "
+ + groupIntersection);
+ }
+ }
+
+ }
+
+}
diff --git a/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandlerTest.java b/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandlerTest.java
new file mode 100644
index 000000000..ad8dc64ac
--- /dev/null
+++ b/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandlerTest.java
@@ -0,0 +1,220 @@
+/*
+ * Licensed 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.opensaml.messaging.handler.impl;
+
+import java.util.List;
+import java.util.Set;
+
+import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.messaging.handler.MessageHandlerException;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import net.shibboleth.shared.collection.Pair;
+
+/**
+ * Unit test for {@link HttpServletRequestParametersValidationHandler}.
+ */
+public class HttpServletRequestParametersValidationHandlerTest {
+
+ @DataProvider
+ Object[][] requiredParamsSetterData() {
+ return new Object[][] {
+ new Object[] { Set.of(),
+ Set.of()},
+ new Object[] { Set.of(" Foo ", " Bar ", " "),
+ Set.of("Foo", "Bar")},
+ };
+ }
+
+ @Test(dataProvider="requiredParamsSetterData")
+ public void requiredSetter(Set<String> params, Set<String> expected) throws Exception {
+ HttpServletRequestParametersValidationHandler handler = new HttpServletRequestParametersValidationHandler();
+ handler.setRequiredParameters(params);
+ handler.setHttpServletRequestSupplier(() -> new MockHttpServletRequest());
+ handler.initialize();
+
+ Assert.assertEquals(handler.getRequiredParameters(), expected);
+ }
+
+ @DataProvider
+ Object[][] uniqueParamsSetterData() {
+ return new Object[][] {
+ new Object[] { Set.of(),
+ Set.of()},
+ new Object[] { Set.of(" Foo ", " Bar ", " "),
+ Set.of("Foo", "Bar")},
+ };
+ }
+
+ @Test(dataProvider="uniqueParamsSetterData")
+ public void uniqueSetter(Set<String> params, Set<String> expected) throws Exception {
+ HttpServletRequestParametersValidationHandler handler = new HttpServletRequestParametersValidationHandler();
+ handler.setUniqueParameters(params);
+ handler.setHttpServletRequestSupplier(() -> new MockHttpServletRequest());
+ handler.initialize();
+
+ Assert.assertEquals(handler.getUniqueParameters(), expected);
+ }
+
+ @DataProvider
+ Object[][] exclusiveParamsSetterData() {
+ return new Object[][] {
+ new Object[] { Set.of(Set.of()),
+ Set.of(Set.of())},
+ new Object[] { Set.of(Set.of(" Foo ", " Bar ", " ")),
+ Set.of(Set.of("Foo", "Bar"))},
+ new Object[] { Set.of(Set.of(" Foo ", " Bar ", " "), Set.of(" Baz ", " ", " ABC ")),
+ Set.of(Set.of("Foo", "Bar"), Set.of("Baz", "ABC"))},
+ };
+ }
+
+ @Test(dataProvider="exclusiveParamsSetterData")
+ public void exclusiveSetter(Set<Set<String>> params, Set<Set<String>> expected) throws Exception {
+ HttpServletRequestParametersValidationHandler handler = new HttpServletRequestParametersValidationHandler();
+ handler.setMutuallyExclusiveParameters(params);
+ handler.setHttpServletRequestSupplier(() -> new MockHttpServletRequest());
+ handler.initialize();
+
+ Assert.assertEquals(handler.getMutuallyExclusiveParameters(), expected);
+ }
+
+ @DataProvider
+ Object[][] requiredParamsEvalData() {
+ return new Object[][] {
+ new Object[] { List.of(),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc"})),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc"})),
+ Set.of("Foo"),
+ true},
+ new Object[] { List.of(new Pair<>("Bar", new String[]{"def"})),
+ Set.of("Foo"),
+ false},
+ new Object[] { List.of(),
+ Set.of("Foo"),
+ false},
+ };
+ }
+
+ @Test(dataProvider="requiredParamsEvalData")
+ public void requiredEval(List<Pair<String, String[]>> requestParams, Set<String> requiredParams, boolean valid) throws Exception {
+ HttpServletRequestParametersValidationHandler handler = new HttpServletRequestParametersValidationHandler();
+ handler.setRequiredParameters(requiredParams);
+ evaluateRequest(handler, "required", requestParams, valid);
+ }
+
+ @DataProvider
+ Object[][] uniqueParamsEvalData() {
+ return new Object[][] {
+ new Object[] { List.of(),
+ Set.of(),
+ true},
+ new Object[] { List.of(),
+ Set.of("Foo"),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc"})),
+ Set.of("Foo"),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc"})),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc", "def"})),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Bar", new String[]{"abc", "def"})),
+ Set.of("Foo"),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc", "def"})),
+ Set.of("Foo"),
+ false},
+ };
+ }
+
+ @Test(dataProvider="uniqueParamsEvalData")
+ public void uniqueEval(List<Pair<String, String[]>> requestParams, Set<String> uniqueParams, boolean valid) throws Exception {
+ HttpServletRequestParametersValidationHandler handler = new HttpServletRequestParametersValidationHandler();
+ handler.setUniqueParameters(uniqueParams);
+ evaluateRequest(handler, "unique", requestParams, valid);
+ }
+
+ @DataProvider
+ Object[][] mutuallyExclusiveParamsEvalData() {
+ return new Object[][] {
+ new Object[] { List.of(),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc"})),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc",}), new Pair<>("Bar", new String[] {"def"})),
+ Set.of(),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc",}), new Pair<>("Bar", new String[] {"def"})),
+ Set.of(Set.of("Foo")),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc"})),
+ Set.of(Set.of("Foo", "Bar")),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc",}), new Pair<>("Baz", new String[] {"def"})),
+ Set.of(Set.of("Foo", "Bar")),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc",}), new Pair<>("Baz", new String[] {"def"})),
+ Set.of(Set.of("Foo", "Bar"), Set.of("Bar", "Baz")),
+ true},
+ new Object[] { List.of(new Pair<>("Foo", new String[]{"abc",}), new Pair<>("Bar", new String[] {"def"})),
+ Set.of(Set.of("Foo", "Bar")),
+ false},
+ };
+ }
+
+ @Test(dataProvider="mutuallyExclusiveParamsEvalData")
+ public void mutuallyExclusiveEval(List<Pair<String, String[]>> requestParams, Set<Set<String>> exclusiveParams, boolean valid) throws Exception {
+ HttpServletRequestParametersValidationHandler handler = new HttpServletRequestParametersValidationHandler();
+ handler.setMutuallyExclusiveParameters(exclusiveParams);
+ evaluateRequest(handler, "exclusive", requestParams, valid);
+ }
+
+ private void evaluateRequest(HttpServletRequestParametersValidationHandler handler, String desc,
+ List<Pair<String, String[]>> requestParams, boolean valid) throws Exception{
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ for (Pair<String,String[]> requestParam : requestParams) {
+ request.addParameter(requestParam.getFirst(), requestParam.getSecond());
+ }
+
+ handler.setHttpServletRequestSupplier(() -> request);
+ handler.initialize();
+
+ MessageContext messageContext = new MessageContext();
+
+ try {
+ handler.invoke(messageContext);
+ if (!valid) {
+ Assert.fail(String.format("Request evaled to valid on invalid %s params", desc));
+ }
+ } catch (MessageHandlerException e) {
+ if (valid) {
+ Assert.fail(String.format("Request evaled to invaid on valid %s params", desc));
+ }
+ }
+ }
+
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list