[java-opensaml] 01/02: OSJ-346: Assertion validator is not checking IssueInstant
Brent Putman
putmanb at georgetown.edu
Wed Mar 2 05:05:43 UTC 2022
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=5efb8cbd5d09295adb49dad65b4ced0775410bb8
commit 5efb8cbd5d09295adb49dad65b4ced0775410bb8
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Tue Mar 1 23:58:56 2022 -0500
OSJ-346: Assertion validator is not checking IssueInstant
---
.../saml2/assertion/SAML20AssertionValidator.java | 111 ++++++++++++++++---
.../SAML2AssertionValidationParameters.java | 5 +
.../DefaultAssertionValidationContextBuilder.java | 42 +++++++
.../tests/SAML20AssertionValidatorTest.java | 123 +++++++++++++++++++++
4 files changed, 267 insertions(+), 14 deletions(-)
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java
index 3a9ce1223..dee4140b6 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java
@@ -38,6 +38,7 @@ import net.shibboleth.utilities.java.support.xml.SerializeSupport;
import org.opensaml.core.criterion.EntityIdCriterion;
import org.opensaml.core.xml.io.MarshallingException;
import org.opensaml.core.xml.util.XMLObjectSupport;
+import org.opensaml.messaging.handler.MessageHandlerException;
import org.opensaml.saml.common.SAMLVersion;
import org.opensaml.saml.common.assertion.AssertionValidationException;
import org.opensaml.saml.common.assertion.ValidationContext;
@@ -125,6 +126,9 @@ public class SAML20AssertionValidator {
/** Default clock skew of 5 minutes. */
@Nonnull public static final Duration DEFAULT_CLOCK_SKEW = Duration.ofMinutes(5);
+ /** Default lifetime for IssueInstant of 5 minutes. */
+ @Nonnull public static final Duration DEFAULT_LIFETIME = Duration.ofMinutes(5);
+
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(SAML20AssertionValidator.class);
@@ -227,6 +231,18 @@ public class SAML20AssertionValidator {
"SAML20AssertionValidator 6 argument constructor");
}
+ /**
+ * Gets the lifetime duration from the {@link ValidationContext#getStaticParameters()} parameters.
+ * If the parameter is not set or is not a non-zero {@link Duration} then the {@link #DEFAULT_LIFETIME} is used.
+ *
+ * @param context current validation context
+ *
+ * @return the lifetime duration
+ */
+ @Nonnull public static Duration getLifetime(@Nonnull final ValidationContext context) {
+ return getDurationParam(context, SAML2AssertionValidationParameters.LIFETIME, DEFAULT_LIFETIME);
+ }
+
/**
* Gets the clock skew from the {@link ValidationContext#getStaticParameters()} parameters. If the parameter is not
* set or is not a non-zero {@link Duration} then the {@link #DEFAULT_CLOCK_SKEW} is used.
@@ -235,32 +251,47 @@ public class SAML20AssertionValidator {
*
* @return the clock skew
*/
- public static Duration getClockSkew(@Nonnull final ValidationContext context) {
- Duration clockSkew = DEFAULT_CLOCK_SKEW;
+ @Nonnull public static Duration getClockSkew(@Nonnull final ValidationContext context) {
+ return getDurationParam(context, SAML2AssertionValidationParameters.CLOCK_SKEW, DEFAULT_CLOCK_SKEW);
+ }
+
+ /**
+ * Gets the clock skew from the {@link ValidationContext#getStaticParameters()} parameters. If the parameter is not
+ * set or is not a non-zero {@link Duration} then the {@link #DEFAULT_CLOCK_SKEW} is used.
+ *
+ * @param context current validation context
+ * @param paramName name of the duration parameter to process
+ * @param defaultDuration the default duration to use if not parameter not present in context
+ *
+ * @return the clock skew
+ */
+ private static Duration getDurationParam(@Nonnull final ValidationContext context, @Nonnull final String paramName,
+ @Nonnull final Duration defaultDuration) {
+
+ Duration duration = defaultDuration;
- if (context.getStaticParameters().containsKey(SAML2AssertionValidationParameters.CLOCK_SKEW)) {
+ if (context.getStaticParameters().containsKey(paramName)) {
try {
- final Object raw = context.getStaticParameters().get(SAML2AssertionValidationParameters.CLOCK_SKEW);
+ final Object raw = context.getStaticParameters().get(paramName);
if (raw instanceof Duration) {
- clockSkew = (Duration) raw;
+ duration = (Duration) raw;
} else if (raw instanceof Long) {
- clockSkew = Duration.ofMillis((Long) raw);
+ duration = Duration.ofMillis((Long) raw);
// This is a V4 deprecation, remove in V5.
- DeprecationSupport.warn(ObjectType.CONFIGURATION, SAML2AssertionValidationParameters.CLOCK_SKEW,
- null, Duration.class.getName());
+ DeprecationSupport.warn(ObjectType.CONFIGURATION, paramName, null, Duration.class.getName());
}
- if (clockSkew.isZero()) {
- clockSkew = DEFAULT_CLOCK_SKEW;
- } else if (clockSkew.isNegative()) {
- clockSkew = clockSkew.abs();
+ if (duration.isZero()) {
+ duration = defaultDuration;
+ } else if (duration.isNegative()) {
+ duration = duration.abs();
}
} catch (final ClassCastException e) {
- clockSkew = DEFAULT_CLOCK_SKEW;
+ duration = defaultDuration;
}
}
- return clockSkew;
+ return duration;
}
/**
@@ -283,6 +314,11 @@ public class SAML20AssertionValidator {
return result;
}
+ result = validateIssueInstant(assertion, context);
+ if (result != ValidationResult.VALID) {
+ return result;
+ }
+
result = validateIssuer(assertion, context);
if (result != ValidationResult.VALID) {
return result;
@@ -355,6 +391,53 @@ public class SAML20AssertionValidator {
return ValidationResult.VALID;
}
+ /**
+ * Validates the Assertion IssueInstant.
+ *
+ * @param assertion the assertion to validate
+ * @param context current validation context
+ *
+ * @return the result of the validation evaluation
+ *
+ * @throws AssertionValidationException if there is a problem validating the IssueInstant
+ */
+ protected ValidationResult validateIssueInstant(@Nonnull final Assertion assertion,
+ @Nonnull final ValidationContext context) throws AssertionValidationException {
+
+ if (assertion.getIssueInstant() == null) {
+ context.setValidationFailureMessage(String.format(
+ "Assertion '%s' did not contain the required IssueInstant", assertion.getID()));
+ return ValidationResult.INVALID;
+ }
+ final Instant issueInstant = assertion.getIssueInstant();
+
+ final Duration clockSkew = getClockSkew(context);
+ final Duration lifetime = getLifetime(context);
+
+ final Instant now = Instant.now();
+ final Instant latestValid = now.plus(clockSkew.abs());
+ final Instant expiration = issueInstant.plus(clockSkew.abs()).plus(lifetime.abs());
+
+ // Check assertion wasn't issued in the future
+ if (issueInstant.isAfter(latestValid)) {
+ log.warn("Assertion was not yet valid: IssueInstant: '{}', latest valid: '{}'", issueInstant, latestValid);
+ context.setValidationFailureMessage("Assertion IssueInstant was invalid, issued in future");
+ return ValidationResult.INVALID;
+
+ }
+
+ // Check assertion has not expired
+ if (expiration.isBefore(now)) {
+ log.warn("Assertion IssueInstant was expired: IssueInstant: '{}', expiration: '{}', now: '{}'",
+ issueInstant, expiration, now);
+ context.setValidationFailureMessage("Assertion IssueInstant was invalid, expired");
+ return ValidationResult.INVALID;
+ }
+
+
+ return ValidationResult.VALID;
+ }
+
/**
* Validates the Assertion {@link Issuer}.
*
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java
index d3a0e9a73..ec7d7e81b 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java
@@ -43,6 +43,11 @@ public final class SAML2AssertionValidationParameters {
*/
public static final String CLOCK_SKEW = STD_PREFIX + ".ClockSkew";
+ /**
+ * Carries a {@link java.time.Duration} specifying a lifetime from 'now' for IssueInstant.
+ */
+ public static final String LIFETIME = STD_PREFIX + ".Lifetime";
+
/**
* Carries the {@link org.opensaml.saml.saml2.core.SubjectConfirmation} that confirmed the subject.
*/
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/DefaultAssertionValidationContextBuilder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/DefaultAssertionValidationContextBuilder.java
index 2c51c7e4a..825c897e0 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/DefaultAssertionValidationContextBuilder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/DefaultAssertionValidationContextBuilder.java
@@ -92,6 +92,9 @@ public class DefaultAssertionValidationContextBuilder
/** A function for resolving the clock skew to apply. */
@Nullable private Function<ProfileRequestContext, Duration> clockSkew;
+ /** A function for resolving the lifetime to apply. */
+ @Nullable private Function<ProfileRequestContext, Duration> lifetime;
+
/** A function for resolving the signature validation CriteriaSet for a particular function. */
@Nullable private Function<Pair<ProfileRequestContext, Assertion>, CriteriaSet> signatureCriteriaSetFunction;
@@ -190,6 +193,39 @@ public class DefaultAssertionValidationContextBuilder
clockSkew = strategy;
}
+ /**
+ * Get the strategy by which to resolve the lifetime.
+ *
+ * @return lookup strategy
+ *
+ * @since 4.2.0
+ */
+ @Nullable public Function<ProfileRequestContext, Duration> getLifetime() {
+ return lifetime;
+ }
+
+ /**
+ * Set the lifetime.
+ *
+ * @param duration lifetime
+ *
+ * @since 4.2.0
+ */
+ public void setLifetime(@Nullable final Duration duration) {
+ lifetime = FunctionSupport.constant(duration);
+ }
+
+ /**
+ * Set the strategy by which to resolve the lifetime.
+ *
+ * @param strategy lookup strategy
+ *
+ * @since 4.2.0
+ */
+ public void setLifetimeLookupStrategy(@Nullable final Function<ProfileRequestContext, Duration> strategy) {
+ lifetime = strategy;
+ }
+
/**
* Get the strategy by which to resolve a {@link SecurityParametersContext}.
*
@@ -598,6 +634,12 @@ public class DefaultAssertionValidationContextBuilder
getClockSkew().apply(input.getProfileRequestContext()));
}
+ // Lifetime (for IssueInstant)
+ if (getLifetime() != null) {
+ staticParams.put(SAML2AssertionValidationParameters.LIFETIME,
+ getLifetime().apply(input.getProfileRequestContext()));
+ }
+
// Issuer
staticParams.put(SAML2AssertionValidationParameters.VALID_ISSUERS,
getValidIssuers().apply(input.getProfileRequestContext()));
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/tests/SAML20AssertionValidatorTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/tests/SAML20AssertionValidatorTest.java
index e79e60e44..0945fa52d 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/tests/SAML20AssertionValidatorTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/tests/SAML20AssertionValidatorTest.java
@@ -25,10 +25,12 @@ import java.security.PrivateKey;
import java.security.PublicKey;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
+import java.time.Duration;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -590,6 +592,127 @@ public class SAML20AssertionValidatorTest extends BaseAssertionValidationTest {
}
+ @Test
+ public void testNoIssueInstant() throws AssertionValidationException {
+ getAssertion().setIssueInstant(null);
+
+ validator = getCurrentValidator();
+
+ Map<String,Object> staticParams = buildBasicStaticParameters();
+ staticParams.put(SAML2AssertionValidationParameters.SIGNATURE_REQUIRED, false);
+
+ ValidationContext validationContext = new ValidationContext(staticParams);
+
+ Assertion assertion = getAssertion();
+
+ Assert.assertEquals(validator.validate(assertion, validationContext), ValidationResult.INVALID);
+ }
+
+ @Test
+ public void testGetLifetime() {
+ ValidationContext validationContext;
+ Map<String,Object> staticParams = new HashMap<>();
+
+ // Default
+ validationContext = new ValidationContext(staticParams);
+ Assert.assertEquals(SAML20AssertionValidator.getLifetime(validationContext), Duration.ofMinutes(5));
+
+ staticParams.put(SAML2AssertionValidationParameters.LIFETIME, Duration.ofMinutes(10));
+ validationContext = new ValidationContext(staticParams);
+ Assert.assertEquals(SAML20AssertionValidator.getLifetime(validationContext), Duration.ofMinutes(10));
+
+ staticParams.put(SAML2AssertionValidationParameters.LIFETIME, Duration.ofMinutes(8).negated());
+ validationContext = new ValidationContext(staticParams);
+ Assert.assertEquals(SAML20AssertionValidator.getLifetime(validationContext), Duration.ofMinutes(8));
+
+ staticParams.put(SAML2AssertionValidationParameters.LIFETIME, 7*60*1000l);
+ validationContext = new ValidationContext(staticParams);
+ Assert.assertEquals(SAML20AssertionValidator.getLifetime(validationContext), Duration.ofMinutes(7));
+
+ staticParams.put(SAML2AssertionValidationParameters.LIFETIME, -9*60*1000l);
+ validationContext = new ValidationContext(staticParams);
+ Assert.assertEquals(SAML20AssertionValidator.getLifetime(validationContext), Duration.ofMinutes(9));
+
+ // Duration==0 means use default
+ staticParams.put(SAML2AssertionValidationParameters.LIFETIME, Duration.ofSeconds(0));
+ validationContext = new ValidationContext(staticParams);
+ Assert.assertEquals(SAML20AssertionValidator.getLifetime(validationContext), Duration.ofMinutes(5));
+ }
+
+ @Test
+ public void testIssueInstantInFuture() throws AssertionValidationException {
+ validator = getCurrentValidator();
+
+ Map<String,Object> staticParams = buildBasicStaticParameters();
+ staticParams.put(SAML2AssertionValidationParameters.SIGNATURE_REQUIRED, false);
+
+ ValidationContext validationContext = new ValidationContext(staticParams);
+
+ Instant now = Instant.now();
+ Duration clockSkew = SAML20AssertionValidator.getClockSkew(validationContext);
+ getAssertion().setIssueInstant(now.plus(clockSkew).plusSeconds(5));
+
+ Assertion assertion = getAssertion();
+
+ Assert.assertEquals(validator.validate(assertion, validationContext), ValidationResult.INVALID);
+ }
+
+ @Test
+ public void testIssueInstantInFutureWithinClockSkew() throws AssertionValidationException {
+ validator = getCurrentValidator();
+
+ Map<String,Object> staticParams = buildBasicStaticParameters();
+ staticParams.put(SAML2AssertionValidationParameters.SIGNATURE_REQUIRED, false);
+
+ ValidationContext validationContext = new ValidationContext(staticParams);
+
+ Instant now = Instant.now();
+ Duration clockSkew = SAML20AssertionValidator.getClockSkew(validationContext);
+ getAssertion().setIssueInstant(now.plus(clockSkew).minusSeconds(5));
+
+ Assertion assertion = getAssertion();
+
+ Assert.assertEquals(validator.validate(assertion, validationContext), ValidationResult.VALID);
+ }
+
+ @Test
+ public void testIssueInstantExpired() throws AssertionValidationException {
+ validator = getCurrentValidator();
+
+ Map<String,Object> staticParams = buildBasicStaticParameters();
+ staticParams.put(SAML2AssertionValidationParameters.SIGNATURE_REQUIRED, false);
+
+ ValidationContext validationContext = new ValidationContext(staticParams);
+
+ Instant now = Instant.now();
+ Duration clockSkew = SAML20AssertionValidator.getClockSkew(validationContext);
+ Duration lifetime = SAML20AssertionValidator.getLifetime(validationContext);
+ getAssertion().setIssueInstant(now.minus(lifetime.plus(clockSkew).plusSeconds(5)));
+
+ Assertion assertion = getAssertion();
+
+ Assert.assertEquals(validator.validate(assertion, validationContext), ValidationResult.INVALID);
+ }
+
+ @Test
+ public void testIssueInstantExpiredWithinClockSkew() throws AssertionValidationException {
+ validator = getCurrentValidator();
+
+ Map<String,Object> staticParams = buildBasicStaticParameters();
+ staticParams.put(SAML2AssertionValidationParameters.SIGNATURE_REQUIRED, false);
+
+ ValidationContext validationContext = new ValidationContext(staticParams);
+
+ Instant now = Instant.now();
+ Duration clockSkew = SAML20AssertionValidator.getClockSkew(validationContext);
+ Duration lifetime = SAML20AssertionValidator.getLifetime(validationContext);
+ getAssertion().setIssueInstant(now.minus(lifetime.plus(clockSkew).minusSeconds(5)));
+
+ Assertion assertion = getAssertion();
+
+ Assert.assertEquals(validator.validate(assertion, validationContext), ValidationResult.VALID);
+ }
+
// Helper methods
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list