This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 3.6.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/3.6.x-fixes by this push:
new 4c328257fd8 only follow same-origin RelayState in SamlFormInHandler
(#3654)
4c328257fd8 is described below
commit 4c328257fd8692241f05e7581eac9ee920ed628f
Author: Naveed Khan <[email protected]>
AuthorDate: Mon Nov 2 13:05:32 2026 +0530
only follow same-origin RelayState in SamlFormInHandler (#3654)
* only follow same-origin RelayState in SamlFormInHandler
* add allowed RelayState origins to SamlFormInHandler
Compare origins by scheme, host and effective port, make the check
protected and log an ignored RelayState.
* validate allowed RelayState origins in SamlFormInHandler
Reject an entry without a scheme or host, or with user info, a path, a
query or a fragment, treat a null list as the empty default and log an ignored
RelayState at FINE.
(cherry picked from commit f442c6563c037e0dde9747408adcfde378ce5ec3)
---
.../cxf/rs/security/saml/SamlFormInHandler.java | 98 +++++++++++-
.../rs/security/saml/SamlFormInHandlerTest.java | 170 +++++++++++++++++++++
2 files changed, 265 insertions(+), 3 deletions(-)
diff --git
a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlFormInHandler.java
b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlFormInHandler.java
index 3e2eb5da342..e7908aefd44 100644
---
a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlFormInHandler.java
+++
b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/SamlFormInHandler.java
@@ -20,11 +20,18 @@
package org.apache.cxf.rs.security.saml;
import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Collections;
+import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.core.Form;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.Response;
+import org.apache.cxf.common.logging.LogUtils;
import org.apache.cxf.jaxrs.impl.UriInfoImpl;
import org.apache.cxf.jaxrs.provider.FormEncodingProvider;
@@ -34,14 +41,48 @@ import org.apache.cxf.message.Message;
public class SamlFormInHandler extends AbstractSamlBase64InHandler {
+ private static final Logger LOG =
LogUtils.getL7dLogger(SamlFormInHandler.class);
private static final String SAML_ELEMENT = "SAMLToken";
private static final String SAML_RELAY_STATE = "RelayState";
private FormEncodingProvider<Form> provider = new
FormEncodingProvider<>(true);
+ private List<URI> allowedRelayStateOrigins = Collections.emptyList();
public SamlFormInHandler() {
}
+ /**
+ * Set the origins a RelayState may redirect to in addition to the origin
of the service,
+ * for example https://app.example.com. Each entry is compared with the
RelayState as an
+ * exact scheme, host and port, no entries are allowed by default and null
resets to that.
+ *
+ * @throws IllegalArgumentException if an entry is null, has no scheme or
host, or has
+ * user info, a path, a query or a fragment
+ */
+ public void setAllowedRelayStateOrigins(List<String>
allowedRelayStateOrigins) {
+ this.allowedRelayStateOrigins = allowedRelayStateOrigins == null
+ ? Collections.emptyList()
+ :
allowedRelayStateOrigins.stream().map(SamlFormInHandler::parseOrigin).collect(Collectors.toUnmodifiableList());
+ }
+
+ private static URI parseOrigin(String value) {
+ URI origin = null;
+ if (value != null) {
+ try {
+ origin = new URI(value);
+ } catch (URISyntaxException ex) {
+ // reported below
+ }
+ }
+ if (origin == null || origin.getScheme() == null || origin.getHost()
== null
+ || origin.getRawUserInfo() != null ||
!origin.getRawPath().isEmpty()
+ || origin.getRawQuery() != null || origin.getRawFragment() !=
null) {
+ throw new IllegalArgumentException("Invalid RelayState origin " +
value
+ + ": it must have a scheme and a host and no user info, path,
query or fragment");
+ }
+ return origin;
+ }
+
@Override
public void filter(ContainerRequestContext context) {
Message message = JAXRSUtils.getCurrentMessage();
@@ -59,9 +100,18 @@ public class SamlFormInHandler extends
AbstractSamlBase64InHandler {
// containing the actual REQUEST URI client was using before being
redirected
// back to IDP - at the moment assume it's URI
UriInfoImpl ui = new UriInfoImpl(message);
- if (!samlRequestURI.startsWith(ui.getBaseUri().toString())) {
-
context.abortWith(Response.status(302).location(URI.create(samlRequestURI)).build());
- return;
+ URI baseUri = ui.getBaseUri();
+ if (!samlRequestURI.startsWith(baseUri.toString())) {
+ URI location = getRelayStateLocation(samlRequestURI, baseUri);
+ if (location != null) {
+
context.abortWith(Response.status(302).location(location).build());
+ return;
+ }
+ if (LOG.isLoggable(Level.FINE)) {
+ LOG.fine("Ignoring RelayState " +
samlRequestURI.replaceAll("\\p{Cntrl}", "_")
+ + ": it is not relative and its origin matches neither
" + baseUri
+ + " nor the allowed RelayState origins");
+ }
}
}
formData.remove(SAML_ELEMENT);
@@ -75,6 +125,48 @@ public class SamlFormInHandler extends
AbstractSamlBase64InHandler {
}
}
+ /**
+ * Get the location to redirect to, null means the RelayState is ignored
and the request
+ * continues. RelayState is not covered by the assertion signature, so
only a relative location
+ * or one on the origin of the service or on an allowed origin is
returned. Origins are compared
+ * by scheme, host and effective port, a URI whose host java.net.URI can't
parse never matches.
+ */
+ protected URI getRelayStateLocation(String relayState, URI baseUri) {
+ try {
+ URI location = new URI(relayState);
+ if ((location.getScheme() == null && location.getRawAuthority() ==
null)
+ || isSameOrigin(location, baseUri)
+ || allowedRelayStateOrigins.stream().anyMatch(origin ->
isSameOrigin(location, origin))) {
+ return location;
+ }
+ } catch (URISyntaxException ex) {
+ // not a valid URI
+ }
+ return null;
+ }
+
+ private static boolean isSameOrigin(URI location, URI origin) {
+ return location.getScheme() != null
+ && location.getHost() != null
+ && location.getRawUserInfo() == null
+ && location.getScheme().equalsIgnoreCase(origin.getScheme())
+ && location.getHost().equalsIgnoreCase(origin.getHost())
+ && getEffectivePort(location) == getEffectivePort(origin);
+ }
+
+ private static int getEffectivePort(URI uri) {
+ if (uri.getPort() != -1) {
+ return uri.getPort();
+ }
+ if ("http".equalsIgnoreCase(uri.getScheme())) {
+ return 80;
+ }
+ if ("https".equalsIgnoreCase(uri.getScheme())) {
+ return 443;
+ }
+ return -1;
+ }
+
private Form readFormData(Message message) {
try {
return FormUtils.readForm(provider, message);
diff --git
a/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/saml/SamlFormInHandlerTest.java
b/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/saml/SamlFormInHandlerTest.java
new file mode 100644
index 00000000000..ef730847b9b
--- /dev/null
+++
b/rt/rs/security/xml/src/test/java/org/apache/cxf/rs/security/saml/SamlFormInHandlerTest.java
@@ -0,0 +1,170 @@
+/**
+ * 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.rs.security.saml;
+
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.lang.reflect.Field;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+
+import javax.ws.rs.core.Response;
+import org.apache.cxf.jaxrs.impl.ContainerRequestContextImpl;
+import org.apache.cxf.message.ExchangeImpl;
+import org.apache.cxf.message.Message;
+import org.apache.cxf.message.MessageImpl;
+import org.apache.cxf.phase.PhaseInterceptorChain;
+
+import org.junit.After;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
+
+public class SamlFormInHandlerTest {
+
+ private static final String BASE_ADDRESS =
"https://sp.example.com/services";;
+ private static final String ALLOWED_ORIGIN = "https://app.example.com";;
+
+ @After
+ public void tearDown() throws Exception {
+ setThreadLocalMessage(null);
+ }
+
+ @Test
+ public void testRelayStateOnAnotherHostIsNotFollowed() throws Exception {
+ assertNull(filter("https://evil.example.com/";));
+ }
+
+ @Test
+ public void testRelayStateWithUserInfoIsNotFollowed() throws Exception {
+ assertNull(filter("https://[email protected]/";));
+ assertNull(filter("https://[email protected]/app/page";));
+ }
+
+ @Test
+ public void testProtocolRelativeRelayStateIsNotFollowed() throws Exception
{
+ assertNull(filter("//evil.example.com/"));
+ }
+
+ @Test
+ public void testRelayStateOnAnotherPortIsNotFollowed() throws Exception {
+ assertNull(filter("https://sp.example.com:8443/app/page";));
+ }
+
+ @Test
+ public void testRelayStateWithExplicitDefaultPortIsFollowed() throws
Exception {
+ Response r = filter("https://sp.example.com:443/app/page";);
+ assertNotNull(r);
+ assertEquals(302, r.getStatus());
+ assertEquals("https://sp.example.com:443/app/page";,
r.getLocation().toString());
+ }
+
+ @Test
+ public void testRelayStateOnAllowedOriginIsFollowed() throws Exception {
+ Response r = filter(ALLOWED_ORIGIN + "/app/page", ALLOWED_ORIGIN);
+ assertNotNull(r);
+ assertEquals(302, r.getStatus());
+ assertEquals(ALLOWED_ORIGIN + "/app/page", r.getLocation().toString());
+ }
+
+ @Test
+ public void testRelayStateOnLookalikeOriginIsNotFollowed() throws
Exception {
+ assertNull(filter("https://[email protected]/";,
ALLOWED_ORIGIN));
+ assertNull(filter("https://app.example.com.evil.example.com/";,
ALLOWED_ORIGIN));
+ assertNull(filter("http://app.example.com/";, ALLOWED_ORIGIN));
+ assertNull(filter("http://app.example.com:443/";, ALLOWED_ORIGIN));
+ assertNull(filter("https://app.example.com:8443/";, ALLOWED_ORIGIN));
+ }
+
+ @Test
+ public void testInvalidAllowedOriginIsRejected() {
+ SamlFormInHandler handler = new SamlFormInHandler();
+ for (String origin : Arrays.asList(null, "app.example.com",
"https://app.example.com/";,
+ "https://app.example.com/app";, "https://[email protected]";,
"https://app.example.com?a=b";,
+ "https://app.example.com#a";, "https://my_service";,
"mailto:[email protected]";, "https://[";)) {
+ assertThrows(origin, IllegalArgumentException.class,
+ () ->
handler.setAllowedRelayStateOrigins(Arrays.asList(origin)));
+ }
+ }
+
+ @Test
+ public void testValidAllowedOriginsAreAccepted() {
+ SamlFormInHandler handler = new SamlFormInHandler();
+ handler.setAllowedRelayStateOrigins(null);
+ handler.setAllowedRelayStateOrigins(Arrays.asList(ALLOWED_ORIGIN,
"https://app.example.com:8443";));
+ }
+
+ @Test
+ public void testSameOriginRelayStateIsFollowed() throws Exception {
+ Response r = filter("https://sp.example.com/app/page";);
+ assertNotNull(r);
+ assertEquals(302, r.getStatus());
+ assertEquals("https://sp.example.com/app/page";,
r.getLocation().toString());
+ }
+
+ @Test
+ public void testRelativeRelayStateIsFollowed() throws Exception {
+ Response r = filter("/app/page");
+ assertNotNull(r);
+ assertEquals(302, r.getStatus());
+ assertEquals(BASE_ADDRESS + "/app/page", r.getLocation().toString());
+ }
+
+ @Test
+ public void testRelayStateForThisServiceIsNotRedirected() throws Exception
{
+ assertNull(filter(BASE_ADDRESS + "/resource"));
+ }
+
+ private static Response filter(String relayState, String...
allowedOrigins) throws Exception {
+ Message message = new MessageImpl();
+ message.setExchange(new ExchangeImpl());
+ message.getExchange().setInMessage(message);
+ message.put(Message.ENDPOINT_ADDRESS, BASE_ADDRESS);
+ // the handler keeps form values encoded, so the RelayState is sent as
is
+ String form = "SAMLToken=token&RelayState=" + relayState;
+ message.setContent(InputStream.class, new
ByteArrayInputStream(form.getBytes(StandardCharsets.UTF_8)));
+ setThreadLocalMessage(message);
+
+ SamlFormInHandler handler = new SamlFormInHandler() {
+ @Override
+ protected void handleToken(Message m, String assertion) {
+ // the assertion is assumed to be valid
+ }
+ };
+ handler.setAllowedRelayStateOrigins(Arrays.asList(allowedOrigins));
+ handler.filter(new ContainerRequestContextImpl(message, false, false));
+ return message.getExchange().get(Response.class);
+ }
+
+ private static void setThreadLocalMessage(Message message) throws
Exception {
+ Field f =
PhaseInterceptorChain.class.getDeclaredField("CURRENT_MESSAGE");
+ f.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ ThreadLocal<Message> tl = (ThreadLocal<Message>) f.get(null);
+ if (message == null) {
+ tl.remove();
+ } else {
+ tl.set(message);
+ }
+ }
+}
