[java-plugin-shibd] branch main updated: Add optional address checking to state managers.
Codeberg
noreply at shibboleth.net
Tue Apr 14 14:05:15 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository java-plugin-shibd.
View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd/commit/764b0b5cc97f3c64924375d1cce7ded33f818c1e
The following commit(s) were added to refs/heads/main by this push:
new 764b0b5 Add optional address checking to state managers.
764b0b5 is described below
commit 764b0b5cc97f3c64924375d1cce7ded33f818c1e
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Tue Apr 14 10:04:52 2026 -0400
Add optional address checking to state managers.
---
.../shibboleth/sp/state/AbstractStateManager.java | 42 ++++++++++++++-
.../java/net/shibboleth/sp/state/StateData.java | 43 ++++++++++++---
.../sp/state/impl/CookieStateManagerTest.java | 63 +++++++++++++++++++++-
3 files changed, 138 insertions(+), 10 deletions(-)
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/state/AbstractStateManager.java b/sp-server-api/src/main/java/net/shibboleth/sp/state/AbstractStateManager.java
index 9dae5d8..13cd872 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/state/AbstractStateManager.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/state/AbstractStateManager.java
@@ -27,12 +27,14 @@ import org.slf4j.Logger;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
+import jakarta.servlet.http.HttpServletRequest;
import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponent;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.primitive.NonnullSupplier;
import net.shibboleth.shared.security.DataSealer;
import net.shibboleth.shared.security.DataSealerException;
import net.shibboleth.shared.security.IdentifierGenerationStrategy;
@@ -61,6 +63,9 @@ public abstract class AbstractStateManager extends AbstractIdentifiableInitializ
/** Optional component to prevent replay of state. */
@Nullable private ReplayCache replayCache;
+ /** Supplier for the servlet request to read from. */
+ @NonnullAfterInit private NonnullSupplier<HttpServletRequest> httpRequestSupplier;
+
/** Expiration for state tokens. */
@Nonnull private Duration expiration;
@@ -148,6 +153,16 @@ public abstract class AbstractStateManager extends AbstractIdentifiableInitializ
replayCache = cache;
}
+ /**
+ * Set the Supplier for the servlet request to read from.
+ *
+ * @param requestSupplier servlet request supplier
+ */
+ public void setHttpServletRequestSupplier(@Nonnull final NonnullSupplier<HttpServletRequest> requestSupplier) {
+ checkSetterPreconditions();
+ httpRequestSupplier = Constraint.isNotNull(requestSupplier, "HttpServletRequest cannot be null");
+ }
+
/**
* Get the expiration limit for state tokens.
*
@@ -236,11 +251,22 @@ public abstract class AbstractStateManager extends AbstractIdentifiableInitializ
}
final T stateData = objectMapper.readValue(data, type);
+
+ // Enforcement checks based on the information available.
+
final Instant issued = stateData.getRequestTime();
if (issued != null && issued.plus(expiration).isBefore(Instant.now())) {
log.warn("State data for token '{}' has expired", token);
return null;
}
+
+ final String addr1 = stateData.getClientAddress();
+ if (addr1 != null && !addr1.equals(getClientAddress())) {
+ log.warn("Client address mismatch for token '{}', issued to '{}', attempt by '{}' ", token, addr1,
+ getClientAddress());
+ return null;
+ }
+
return stateData;
} catch (final DataSealerException|JsonProcessingException e) {
@@ -267,8 +293,10 @@ public abstract class AbstractStateManager extends AbstractIdentifiableInitializ
* Subclasses implement this method to recover the stored data in whatever way is necessary and return
* the supplied data string as a successful result.
*
+ * <p>The implementation should ensure when possible that this method works only once for a given state token.</p>
+ *
* <p>Subclasses may assume that the state token inputs they receive will have been returned by them
- * via the {@link #doPreserve(Agent, Application, String)} method.</p>
+ * via the {@link #doPreserve(Agent, Application, String)} method.</p>
*
* @param agent agent owning the state
* @param application application owning the state
@@ -290,4 +318,16 @@ public abstract class AbstractStateManager extends AbstractIdentifiableInitializ
return identifierStrategy.generateIdentifier(false);
}
+ /**
+ * Get the current client address if available.
+ *
+ * @return client address or null
+ */
+ @Nullable protected String getClientAddress() {
+ if (httpRequestSupplier == null) {
+ return null;
+ }
+ return httpRequestSupplier.get().getRemoteAddr();
+ }
+
}
\ No newline at end of file
diff --git a/sp-server-api/src/main/java/net/shibboleth/sp/state/StateData.java b/sp-server-api/src/main/java/net/shibboleth/sp/state/StateData.java
index aa7b6d3..ee27333 100644
--- a/sp-server-api/src/main/java/net/shibboleth/sp/state/StateData.java
+++ b/sp-server-api/src/main/java/net/shibboleth/sp/state/StateData.java
@@ -33,7 +33,7 @@ import net.shibboleth.shared.collection.CollectionSupport;
/**
* A DTO class that carries protocol state information that needs to be recovered to validate a protocol
- * response. This class is designed for JSON serialization and deserialization for storage e.g. in a cookie
+ * response; this class is designed for JSON serialization and deserialization for storage e.g. in a cookie
* or database.
*
* <p>This class is designed to be extended to support non-generic protocol state as required.</p>
@@ -46,6 +46,9 @@ import net.shibboleth.shared.collection.CollectionSupport;
@NotThreadSafe
public class StateData {
+ /** The client's address for optional enforcement. */
+ @Nullable private String clientAddress;
+
/** The identifier of the party that is making a request, i.e., us. */
@Nullable private String issuer;
@@ -69,6 +72,31 @@ public class StateData {
acrs = CollectionSupport.emptyList();
}
+ /**
+ * Get the IP address of the client associated with the state.
+ *
+ * <p>Can be used to optionally enforce the address when recovering the state.</p>
+ *
+ * @return the client's address
+ */
+ @JsonProperty("address")
+ @Nullable public String getClientAddress() {
+ return clientAddress;
+ }
+
+ /**
+ * Set the identifier of the client that is making the authentication request. Can be used to ensure the audience
+ * of the response matches the client that made the request.
+ *
+ * @param address client address
+ *
+ * @return the updated object
+ */
+ @Nonnull public StateData setClientAddress(@Nullable final String address) {
+ clientAddress = address;
+ return this;
+ }
+
/**
* Get the identifier of the party that is making the request, i.e., us.
*
@@ -82,10 +110,9 @@ public class StateData {
}
/**
- * Set the identifier of the client that is making the authentication request. Can be used to ensure the audience
- * of the response matches the client that made the request.
+ * Set the identifier of the party that is making the request, i.e., us.
*
- * @param id The client ID to set.
+ * @param id the issuer to set
*
* @return the updated object
*/
@@ -95,7 +122,7 @@ public class StateData {
}
/**
- * Get the expected issuer of the authentication response.
+ * Get the expected issuer of a response to the message.
*
* @return the authentication authority
*/
@@ -105,7 +132,7 @@ public class StateData {
}
/**
- * Set the expected issuer of the authentication response.
+ * Set the expected issuer of a response to the message.
*
* @param authority the authentication authority to set
*
@@ -139,7 +166,7 @@ public class StateData {
}
/**
- * Get the context classes requested.
+ * Get the authentication context classes requested.
*
* @return the context classes
*/
@@ -149,7 +176,7 @@ public class StateData {
}
/**
- * Set the context classes requested.
+ * Set the authentication context classes requested.
*
* @param refs context class references
*
diff --git a/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/CookieStateManagerTest.java b/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/CookieStateManagerTest.java
index a670fdc..d177711 100644
--- a/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/CookieStateManagerTest.java
+++ b/sp-server-impl/src/test/java/net/shibboleth/sp/state/impl/CookieStateManagerTest.java
@@ -82,9 +82,18 @@ public class CookieStateManagerTest extends BaseApplicationActionTest {
stateManager = new CookieStateManager();
stateManager.setId("test");
+
final ObjectMapper mapper = new ObjectMapper();
mapper.registerModule(new JavaTimeModule());
stateManager.setObjectMapper(mapper);
+
+ stateManager.setHttpServletRequestSupplier(new NonnullSupplier<HttpServletRequest>() {
+ @Nonnull public HttpServletRequest get() {
+ assert request != null;
+ return request;
+ }
+ });
+
stateManager.setCookieManager(cookieManager);
stateManager.initialize();
@@ -132,7 +141,7 @@ public class CookieStateManagerTest extends BaseApplicationActionTest {
}
@Test
- public void testMapRecover() throws IOException {
+ public void testSuccess() throws IOException {
final StateData source = buildStateData();
final String token = stateManager.preserveToStateToken(agent, application, source);
@@ -154,6 +163,58 @@ public class CookieStateManagerTest extends BaseApplicationActionTest {
Assert.assertEquals(cookies[0].getMaxAge(), 0);
Assert.assertEquals(cookies[0].getAttribute("SameSite"), SameSiteValue.None.getValue());
}
+
+ @Test
+ public void testSuccessWithAddress() throws IOException {
+
+ final StateData source = buildStateData();
+ source.setClientAddress("192.168.1.1");
+ final String token = stateManager.preserveToStateToken(agent, application, source);
+ assert token != null;
+
+ // Move token set on response to request.
+ request = new MockHttpServletRequest();
+ request.setRemoteAddr("192.168.1.1");
+ request.setCookies(response.getCookies());
+ response = new MockHttpServletResponse();
+
+ final StateData recovered = stateManager.recoverFromStateToken(agent, application, token, StateData.class);
+ Assert.assertEquals(source, recovered);
+
+ // Check that old token is unset.
+ final Cookie[] cookies = response.getCookies();
+ Assert.assertEquals(cookies.length, 1);
+ Assert.assertEquals(cookies[0].getName(), CookieStateManager.DEFAULT_PREFIX + '_' + "test" + '_' + token);
+ Assert.assertEquals(cookies[0].getValue(), null);
+ Assert.assertEquals(cookies[0].getMaxAge(), 0);
+ Assert.assertEquals(cookies[0].getAttribute("SameSite"), SameSiteValue.None.getValue());
+ }
+
+ @Test
+ public void testFailureWithAddress() throws IOException {
+
+ final StateData source = buildStateData();
+ source.setClientAddress("192.168.1.1");
+ final String token = stateManager.preserveToStateToken(agent, application, source);
+ assert token != null;
+
+ // Move token set on response to request.
+ request = new MockHttpServletRequest();
+ request.setRemoteAddr("192.168.1.2");
+ request.setCookies(response.getCookies());
+ response = new MockHttpServletResponse();
+
+ final StateData recovered = stateManager.recoverFromStateToken(agent, application, token, StateData.class);
+ Assert.assertNull(recovered);
+
+ // Check that old token is unset.
+ final Cookie[] cookies = response.getCookies();
+ Assert.assertEquals(cookies.length, 1);
+ Assert.assertEquals(cookies[0].getName(), CookieStateManager.DEFAULT_PREFIX + '_' + "test" + '_' + token);
+ Assert.assertEquals(cookies[0].getValue(), null);
+ Assert.assertEquals(cookies[0].getMaxAge(), 0);
+ Assert.assertEquals(cookies[0].getAttribute("SameSite"), SameSiteValue.None.getValue());
+ }
@Nonnull private String getCookieName() {
final Instant now = Instant.now();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list