[java-opensaml] branch main updated: OSJ-427: Simple signature verification fails to detect parameter ...
Brent Putman
putmanb at georgetown.edu
Wed Mar 19 22:01:18 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=880f154418d547cf494128c1f25bf967f1764f97
The following commit(s) were added to refs/heads/main by this push:
new 880f15441 OSJ-427: Simple signature verification fails to detect parameter ...
880f15441 is described below
commit 880f154418d547cf494128c1f25bf967f1764f97
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Mar 19 17:05:57 2025 -0400
OSJ-427: Simple signature verification fails to detect parameter ...
Refactor message handler to use new HttpServletRequestValidator.
---
...pServletRequestParametersValidationHandler.java | 173 ----------------
.../impl/HttpServletRequestValidationHandler.java | 82 ++++++++
...vletRequestParametersValidationHandlerTest.java | 223 ---------------------
.../HttpServletRequestValidationHandlerTest.java | 78 +++++++
4 files changed, 160 insertions(+), 396 deletions(-)
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
deleted file mode 100644
index 1c3dc2ce6..000000000
--- a/opensaml-messaging-impl/src/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandler.java
+++ /dev/null
@@ -1,173 +0,0 @@
-/*
- * 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
- */
- 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
- */
- 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
- */
- public void setMutuallyExclusiveParameters(@Nullable 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/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestValidationHandler.java b/opensaml-messaging-impl/src/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestValidationHandler.java
new file mode 100644
index 000000000..5a19d146f
--- /dev/null
+++ b/opensaml-messaging-impl/src/main/java/org/opensaml/messaging/handler/impl/HttpServletRequestValidationHandler.java
@@ -0,0 +1,82 @@
+/*
+ * 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 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.ServletException;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.servlet.HttpServletRequestValidator;
+
+/**
+ * Message handler that validates an HTTP request via an instance of {@link HttpServletRequestValidator}.
+ */
+public class HttpServletRequestValidationHandler extends AbstractHttpServletRequestMessageHandler {
+
+ /** Logger. */
+ @Nonnull private Logger log = LoggerFactory.getLogger(HttpServletRequestValidationHandler.class);
+
+ /** Request validator. */
+ @NonnullAfterInit private HttpServletRequestValidator validator;
+
+ /**
+ * Get the request validator.
+ *
+ * @return the validator
+ */
+ @NonnullAfterInit public HttpServletRequestValidator getValidator() {
+ return validator;
+ }
+
+ /**
+ * Set the request validator.
+ *
+ * @param newValidator the request validator
+ */
+ public void setValidator(final @Nullable HttpServletRequestValidator newValidator) {
+ validator = newValidator;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void doInitialize() throws ComponentInitializationException {
+ super.doInitialize();
+
+ if (getValidator() == null) {
+ throw new ComponentInitializationException("HttpServletRequestValidator was null");
+ }
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void doInvoke(@Nonnull final MessageContext messageContext) throws MessageHandlerException {
+ try {
+ getValidator().validate(getHttpServletRequest());
+ } catch (final ServletException e) {
+ throw new MessageHandlerException("HttpServletRequest was invalid", e);
+ }
+
+ }
+
+}
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
deleted file mode 100644
index 8a808e07c..000000000
--- a/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestParametersValidationHandlerTest.java
+++ /dev/null
@@ -1,223 +0,0 @@
-/*
- * 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 (final Pair<String,String[]> requestParam : requestParams) {
- final String first = requestParam.getFirst();
- final String[] second = requestParam.getSecond();
- assert first != null && second != null;
- request.addParameter(first, second);
- }
-
- 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));
- }
- }
- }
-
-}
diff --git a/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestValidationHandlerTest.java b/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestValidationHandlerTest.java
new file mode 100644
index 000000000..12eb5c1c6
--- /dev/null
+++ b/opensaml-messaging-impl/src/test/java/org/opensaml/messaging/handler/impl/HttpServletRequestValidationHandlerTest.java
@@ -0,0 +1,78 @@
+/*
+ * 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 javax.annotation.Nonnull;
+
+import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.messaging.handler.MessageHandlerException;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.testng.annotations.Test;
+
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import net.shibboleth.shared.component.ComponentInitializationException;
+import net.shibboleth.shared.servlet.HttpServletRequestValidator;
+
+/**
+ * Unit test for {@link HttpServletRequestValidationHandler}.
+ */
+public class HttpServletRequestValidationHandlerTest {
+
+ @Test(expectedExceptions = ComponentInitializationException.class)
+ public void nullValidator() throws Exception {
+ HttpServletRequestValidationHandler handler = new HttpServletRequestValidationHandler();
+ handler.initialize();
+ }
+
+ @Test
+ public void valid() throws Exception {
+ HttpServletRequestValidationHandler handler = new HttpServletRequestValidationHandler();
+ handler.setValidator(new MockValidator(true));
+ handler.setHttpServletRequestSupplier(() -> new MockHttpServletRequest());
+ handler.initialize();
+
+ handler.invoke(new MessageContext());
+ }
+
+ @Test(expectedExceptions = MessageHandlerException.class)
+ public void invalid() throws Exception {
+ HttpServletRequestValidationHandler handler = new HttpServletRequestValidationHandler();
+ handler.setValidator(new MockValidator(false));
+ handler.setHttpServletRequestSupplier(() -> new MockHttpServletRequest());
+ handler.initialize();
+
+ handler.invoke(new MessageContext());
+ }
+
+ public class MockValidator implements HttpServletRequestValidator {
+
+ private boolean valid;
+
+ public MockValidator(boolean result) {
+ valid = result ;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void validate(@Nonnull HttpServletRequest request) throws ServletException {
+ if (!valid) {
+ throw new ServletException("Request was invalid");
+ }
+
+ }
+
+ }
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list