[java-identity-provider] branch master updated: IDP-1507 - Increase flexibility around session address checking
Scott Cantor
cantor.2 at osu.edu
Mon Oct 7 20:46:13 EDT 2019
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch master
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=12e9c375caf83476358e84aa7de03b6dafe9a8ef
The following commit(s) were added to refs/heads/master by this push:
new 12e9c37 IDP-1507 - Increase flexibility around session address checking
12e9c37 is described below
commit 12e9c375caf83476358e84aa7de03b6dafe9a8ef
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Oct 7 20:45:15 2019 -0400
IDP-1507 - Increase flexibility around session address checking
https://issues.shibboleth.net/jira/browse/IDP-1507
Replace original support added in 3.4.
Add BiPredicate supporting unreliable network ranges.
---
.../resources/system/flows/authn/authn-beans.xml | 3 +-
.../resources/system/flows/logout/logout-beans.xml | 3 +-
.../idp/session/logic/IPRangeBiPredicate.java | 73 ++++++++++++++++++++++
.../idp/session/impl/PopulateSessionContext.java | 36 ++---------
.../shibboleth/idp/session/impl/ProcessLogout.java | 48 ++++----------
.../idp/session/impl/StorageBackedIdPSession.java | 2 +-
6 files changed, 94 insertions(+), 71 deletions(-)
diff --git a/idp-conf/src/main/resources/system/flows/authn/authn-beans.xml b/idp-conf/src/main/resources/system/flows/authn/authn-beans.xml
index 38881ef..2949144 100644
--- a/idp-conf/src/main/resources/system/flows/authn/authn-beans.xml
+++ b/idp-conf/src/main/resources/system/flows/authn/authn-beans.xml
@@ -29,8 +29,7 @@
class="net.shibboleth.idp.session.impl.PopulateSessionContext" scope="prototype"
p:activationCondition="%{idp.session.enabled:true}"
p:httpServletRequest-ref="shibboleth.HttpServletRequest"
- p:sessionResolver-ref="shibboleth.SessionManager"
- p:checkAddressCondition="#{getObject('%{idp.session.consistentAddressCondition:null}'.trim()) ?: %{idp.session.consistentAddress:true}}" />
+ p:sessionResolver-ref="shibboleth.SessionManager" />
<bean id="SetRPUIInformation"
class="net.shibboleth.idp.ui.impl.SetRPUIInformation" scope="prototype"
diff --git a/idp-conf/src/main/resources/system/flows/logout/logout-beans.xml b/idp-conf/src/main/resources/system/flows/logout/logout-beans.xml
index a4f6a78..c056b14 100644
--- a/idp-conf/src/main/resources/system/flows/logout/logout-beans.xml
+++ b/idp-conf/src/main/resources/system/flows/logout/logout-beans.xml
@@ -47,8 +47,7 @@
<bean id="ProcessLogout"
class="net.shibboleth.idp.session.impl.ProcessLogout" scope="prototype"
p:httpServletRequest-ref="shibboleth.HttpServletRequest"
- p:sessionResolver-ref="shibboleth.SessionManager"
- p:checkAddressCondition="#{getObject('%{idp.session.consistentAddressCondition:null}'.trim()) ?: %{idp.session.consistentAddress:true}}" />
+ p:sessionResolver-ref="shibboleth.SessionManager" />
<bean id="DestroySessions"
class="net.shibboleth.idp.session.impl.DestroySessions" scope="prototype"
diff --git a/idp-session-api/src/main/java/net/shibboleth/idp/session/logic/IPRangeBiPredicate.java b/idp-session-api/src/main/java/net/shibboleth/idp/session/logic/IPRangeBiPredicate.java
new file mode 100644
index 0000000..0c0b653
--- /dev/null
+++ b/idp-session-api/src/main/java/net/shibboleth/idp/session/logic/IPRangeBiPredicate.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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 net.shibboleth.idp.session.logic;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.function.BiPredicate;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import com.google.common.net.InetAddresses;
+
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.net.IPRange;
+
+/** A {@link BiPredicate} that checks if a pair of addresses are either equal, or share an {@link IPRange}. */
+public class IPRangeBiPredicate implements BiPredicate<String,String> {
+
+ /** IP ranges to match against. */
+ @Nonnull @NonnullElements private Collection<IPRange> addressRanges;
+
+ /** Constructor. */
+ IPRangeBiPredicate() {
+ addressRanges = List.of();
+ }
+
+ /**
+ * Set the address ranges to check against.
+ *
+ * @param ranges address ranges to check against
+ */
+ public void setRanges(@Nonnull @NonnullElements final Collection<IPRange> ranges) {
+ Constraint.isNotNull(ranges, "Address range collection cannot be null");
+
+ addressRanges = List.copyOf(ranges);
+ }
+
+ /** {@inheritDoc} */
+ public boolean test(@Nullable final String input1, @Nullable final String input2) {
+
+ if (input1 == null || input2 == null) {
+ return false;
+ } else if (input1.equals(input2)) {
+ return true;
+ }
+
+ for (final IPRange range : addressRanges) {
+ if (range.contains(InetAddresses.forString(input1)) && range.contains(InetAddresses.forString(input2))) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
+}
\ No newline at end of file
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateSessionContext.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateSessionContext.java
index 7144f5f..0d38b44 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateSessionContext.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/PopulateSessionContext.java
@@ -18,7 +18,6 @@
package net.shibboleth.idp.session.impl;
import java.util.function.Function;
-import java.util.function.Predicate;
import javax.annotation.Nonnull;
import javax.servlet.http.HttpServletRequest;
@@ -60,9 +59,6 @@ public class PopulateSessionContext extends AbstractProfileAction {
/** Session resolver. */
@NonnullAfterInit private SessionResolver sessionResolver;
-
- /** Condition to determine whether to enforce address binding on the session. */
- @Nonnull private Predicate<ProfileRequestContext> checkAddressCondition;
/** Creation/lookup function for SessionContext. */
@Nonnull private Function<ProfileRequestContext,SessionContext> sessionContextCreationStrategy;
@@ -72,7 +68,6 @@ public class PopulateSessionContext extends AbstractProfileAction {
/** Constructor. */
public PopulateSessionContext() {
- checkAddressCondition = Predicates.alwaysTrue();
sessionContextCreationStrategy = new ChildContextLookup<>(SessionContext.class, true);
@@ -80,21 +75,6 @@ public class PopulateSessionContext extends AbstractProfileAction {
}
/**
- * Set condition to determine whether to perform address binding check before use of session.
- *
- * <p>Defaults to true insofar as the decision is then delegated back to the resolver.</p>
- *
- * @param condition condition to apply
- *
- * @since 3.4.0
- */
- public void setCheckAddressCondition(@Nonnull final Predicate<ProfileRequestContext> condition) {
- ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- checkAddressCondition = Constraint.isNotNull(condition, "Address checking condition cannot be null");
- }
-
- /**
* Set the {@link SessionResolver} to use.
*
* @param resolver session resolver to use
@@ -154,18 +134,14 @@ public class PopulateSessionContext extends AbstractProfileAction {
return;
}
- if (checkAddressCondition.test(profileRequestContext)) {
- final HttpServletRequest request = getHttpServletRequest();
- if (request != null && request.getRemoteAddr() != null) {
- if (!session.checkAddress(request.getRemoteAddr())) {
- return;
- }
- } else {
- log.info("{} No servlet request or client address available, skipping address check for session {}",
- getLogPrefix(), session.getId());
+ final HttpServletRequest request = getHttpServletRequest();
+ if (request != null && request.getRemoteAddr() != null) {
+ if (!session.checkAddress(request.getRemoteAddr())) {
+ return;
}
} else {
- log.debug("{} Bypassing address check for session {}", getLogPrefix(), session.getId());
+ log.info("{} No servlet request or client address available, skipping address check for session {}",
+ getLogPrefix(), session.getId());
}
final SessionContext sessionCtx = sessionContextCreationStrategy.apply(profileRequestContext);
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ProcessLogout.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ProcessLogout.java
index 73f997d..279c5b9 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ProcessLogout.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/ProcessLogout.java
@@ -18,7 +18,6 @@
package net.shibboleth.idp.session.impl;
import java.util.function.Function;
-import java.util.function.Predicate;
import javax.annotation.Nonnull;
import javax.servlet.http.HttpServletRequest;
@@ -70,9 +69,6 @@ public class ProcessLogout extends AbstractProfileAction {
/** Session resolver. */
@NonnullAfterInit private SessionResolver sessionResolver;
- /** Condition to determine whether to enforce address binding on the session. */
- @Nonnull private Predicate<ProfileRequestContext> checkAddressCondition;
-
/** Creation/lookup function for SubjectContext. */
@Nonnull private Function<ProfileRequestContext,SubjectContext> subjectContextCreationStrategy;
@@ -87,7 +83,6 @@ public class ProcessLogout extends AbstractProfileAction {
/** Constructor. */
public ProcessLogout() {
- checkAddressCondition = Predicates.alwaysTrue();
subjectContextCreationStrategy = new ChildContextLookup<>(SubjectContext.class, true);
sessionContextCreationStrategy = new ChildContextLookup<>(SessionContext.class, true);
logoutContextCreationStrategy = new ChildContextLookup<>(LogoutContext.class, true);
@@ -96,21 +91,6 @@ public class ProcessLogout extends AbstractProfileAction {
}
/**
- * Set condition to determine whether to perform address binding check before use of session.
- *
- * <p>Defaults to true insofar as the decision is then delegated back to the resolver.</p>
- *
- * @param condition condition to apply
- *
- * @since 3.4.0
- */
- public void setCheckAddressCondition(@Nonnull final Predicate<ProfileRequestContext> condition) {
- ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-
- checkAddressCondition = Constraint.isNotNull(condition, "Address checking condition cannot be null");
- }
-
- /**
* Set the {@link SessionResolver} to use.
*
* @param resolver session resolver to use
@@ -195,24 +175,20 @@ public class ProcessLogout extends AbstractProfileAction {
return;
}
- if (checkAddressCondition.test(profileRequestContext)) {
- final HttpServletRequest request = getHttpServletRequest();
- if (request != null && request.getRemoteAddr() != null) {
- try {
- if (!session.checkAddress(request.getRemoteAddr())) {
- return;
- }
- } catch (final SessionException e) {
- log.error("{} Error binding session to client address", getLogPrefix(), e);
- ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
+ final HttpServletRequest request = getHttpServletRequest();
+ if (request != null && request.getRemoteAddr() != null) {
+ try {
+ if (!session.checkAddress(request.getRemoteAddr())) {
return;
- }
- } else {
- log.info("{} No servlet request or client address available, skipping address check for sessions",
- getLogPrefix());
- }
+ }
+ } catch (final SessionException e) {
+ log.error("{} Error binding session to client address", getLogPrefix(), e);
+ ActionSupport.buildEvent(profileRequestContext, EventIds.IO_ERROR);
+ return;
+ }
} else {
- log.debug("{} Bypassing address check for session {}", getLogPrefix(), session.getId());
+ log.info("{} No servlet request or client address available, skipping address check for sessions",
+ getLogPrefix());
}
final SubjectContext subjectCtx = subjectContextCreationStrategy.apply(profileRequestContext);
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
index ef9ecd0..e3c25b1 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
@@ -117,7 +117,7 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
return false;
}
} else {
- log.info("Session {} not yet locked to a {} address, locking it to {}", getId(), family, address);
+ log.info("Session {} not yet bound to a {} address, binding to {}", getId(), family, address);
try {
bindToAddress(address);
} catch (final SessionException e) {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list