This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit b0c2630caaecfd205130d8c4f3bc13e706be4944 Author: James Bognar <[email protected]> AuthorDate: Sun Aug 16 13:40:18 2026 -0400 READY-378: Fail closed when SAML assertions lack a NotOnOrAfter bound An assertion with no NotOnOrAfter had no replay-expiry bound, so its replay-cache record could be evicted early and the assertion replayed. Reject such unbounded assertions and retain replay records until the resolved NotOnOrAfter + clock skew. --- .../server/auth/saml/SamlAssertionValidator.java | 51 ++++-- .../saml/SamlAssertionValidator_Branches_Test.java | 12 +- .../SamlAssertionValidator_ReplayExpiry_Test.java | 202 +++++++++++++++++++++ 3 files changed, 247 insertions(+), 18 deletions(-) diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java index 70cef30352..62044f1d9c 100644 --- a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java +++ b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java @@ -79,7 +79,10 @@ import net.shibboleth.shared.resolver.*; * {@code InResponseTo}/{@code Address} expectations. * <li><b>One-time use</b> — each assertion ID is recorded in a {@link ReplayCache} and a second * presentation of the same ID is rejected. The check is fail-closed: if the cache cannot answer, the - * assertion is rejected. + * assertion is rejected. An assertion whose validity window is unbounded — carrying neither a + * {@code Conditions/NotOnOrAfter} nor a bearer {@code SubjectConfirmationData/NotOnOrAfter} — is + * rejected outright, and the cache record is retained until the resolved {@code NotOnOrAfter} (plus clock + * skew) rather than a short default window that would later expire and re-open replay. * <li><b>Encrypted assertions</b> — if the response contains * {@code <EncryptedAssertion>}, a {@code decryptionCredential} must be configured. * </ul> @@ -134,12 +137,6 @@ public class SamlAssertionValidator { /** XML-DSig namespace URI, used when scanning the signature DOM for reference digest methods. */ private static final String XMLSIG_NS = "http://www.w3.org/2000/09/xmldsig#";; - /** - * Retention window applied to a one-time-use cache entry when the assertion carries no - * {@code Conditions/NotOnOrAfter} to bound it (5 minutes). - */ - private static final Duration DEFAULT_REPLAY_RETENTION = Duration.ofMinutes(5); - /** Default allowlist of XML signature algorithms. */ private static final Set<String> DEFAULT_ALGORITHMS = Set.of( SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256, @@ -814,7 +811,10 @@ public class SamlAssertionValidator { var id = assertion.getID(); if (id == null || id.isEmpty()) throw rejectAssertion("assertion has no ID for single-use enforcement"); - var expiresAtMs = singleUseExpiry(assertion); + var notOnOrAfter = resolveNotOnOrAfter(assertion); + if (notOnOrAfter == null) + throw rejectAssertion("assertion has no NotOnOrAfter to bound one-time use"); + var expiresAtMs = notOnOrAfter.plus(clockSkew).toEpochMilli(); var firstSeen = replayCache.checkAndRecord(id, expiresAtMs, ReplayCache.FailMode.FAIL_CLOSED, e -> LOG.log(Level.WARNING, e, () -> "SAML assertion single-use check failed; rejecting (fail-closed).")); if (! firstSeen) @@ -822,11 +822,36 @@ public class SamlAssertionValidator { .wwwAuthenticate("SAML error=\"assertion_replayed\""); } - private long singleUseExpiry(Assertion assertion) { - var conditions = assertion.getConditions(); - if (conditions != null && conditions.getNotOnOrAfter() != null) - return conditions.getNotOnOrAfter().plus(clockSkew).toEpochMilli(); - return clock.instant().plus(DEFAULT_REPLAY_RETENTION).toEpochMilli(); + /** + * Resolves the effective {@code NotOnOrAfter} that bounds one-time use of the assertion. + * + * <p> + * The resolved value is the latest of the assertion's {@code Conditions/NotOnOrAfter} and any bearer + * {@code SubjectConfirmationData/NotOnOrAfter}. Returns <jk>null</jk> when the assertion carries neither + * — in that case its validity window is unbounded, so single-use cannot be enforced past a cache + * eviction and the caller rejects the assertion rather than inventing a short fallback that later re-opens + * replay. {@code <Conditions>} itself is guaranteed non-<jk>null</jk> here (a null {@code <Conditions>} is + * rejected earlier by {@link #validateConditions(Assertion)}). + * + * @param assertion The assertion. + * @return The latest bounding instant, or <jk>null</jk> if the assertion carries no {@code NotOnOrAfter}. + */ + private static Instant resolveNotOnOrAfter(Assertion assertion) { + var result = assertion.getConditions().getNotOnOrAfter(); + var subject = assertion.getSubject(); + if (subject != null) { + for (var sc : subject.getSubjectConfirmations()) { + if (! SubjectConfirmation.METHOD_BEARER.equals(sc.getMethod())) + continue; + var data = sc.getSubjectConfirmationData(); + if (data == null) + continue; + var noa = data.getNotOnOrAfter(); + if (noa != null && (result == null || noa.isAfter(result))) + result = noa; + } + } + return result; } private static String extractStringValue(org.opensaml.core.xml.XMLObject av) { diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java index fc45f2ce69..2a04593d3b 100644 --- a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java +++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_Branches_Test.java @@ -73,17 +73,19 @@ class SamlAssertionValidator_Branches_Test extends TestBase { } // ----------------------------------------------------------------------------------------------------------------- - // B: nbf == null and noa == null → timestamps not checked (lines 561–564 both false branches) + // B: nbf == null and noa == null → the NotBefore/NotOnOrAfter checks are skipped in validateConditions, but + // an assertion with no NotOnOrAfter anywhere has an unbounded validity window and is rejected at the + // one-time-use stage (its lifetime cannot be bounded, so single-use cannot be enforced past eviction). // ----------------------------------------------------------------------------------------------------------------- - @Test void b01_noNbf_noNoa_accepted() throws Exception { + @Test void b01_noNbf_noNoa_rejected() throws Exception { var pair = SamlTestSupport.generateRsaKeyPair(); var cred = SamlTestSupport.credential(pair); - // conditions() with null notBefore and null notOnOrAfter → neither timestamp check fires + // conditions() with null notBefore and null notOnOrAfter → neither timestamp check fires, but the missing + // NotOnOrAfter leaves the assertion unbounded → rejected (fail-closed) rather than replayable. var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", null, null); var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); - var principal = validator(cred).validate(xml); - assertEquals("alice", principal.getName()); + assertThrows(AuthenticationException.class, () -> validator(cred).validate(xml)); } // ----------------------------------------------------------------------------------------------------------------- diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java new file mode 100644 index 0000000000..67f12bfc6b --- /dev/null +++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_ReplayExpiry_Test.java @@ -0,0 +1,202 @@ +/* + * 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.juneau.rest.server.auth.saml; + +import static org.junit.jupiter.api.Assertions.*; + +import java.time.*; +import java.util.concurrent.atomic.*; + +import org.apache.juneau.*; +import org.apache.juneau.commons.concurrent.*; +import org.apache.juneau.rest.server.auth.*; +import org.junit.jupiter.api.*; +import org.opensaml.saml.common.*; +import org.opensaml.saml.saml2.core.*; +import org.opensaml.security.credential.*; + +/** + * Tests that one-time-use enforcement in {@link SamlAssertionValidator} holds for the assertion's full validity + * window, including when {@code Conditions/NotOnOrAfter} is absent. + * + * <p> + * An assertion with no {@code NotOnOrAfter} anywhere (neither {@code Conditions/NotOnOrAfter} nor a bearer + * {@code SubjectConfirmationData/NotOnOrAfter}) has an unbounded validity window and must be rejected rather than + * pinned to a short replay-cache fallback that later expires and re-opens replay. When a bounding + * {@code NotOnOrAfter} <i>is</i> present, the replay-cache record must be retained until that resolved instant + * (plus clock skew), not a fixed default. + * + * @since 10.0.0 + */ +class SamlAssertionValidator_ReplayExpiry_Test extends TestBase { + + private static final String ISSUER = "https://idp.example.com";; + private static final String AUDIENCE = "https://sp.example.com";; + private static final String ACS = "https://sp.example.com/saml/acs";; + private static final Instant NOW = Instant.parse("2026-01-01T00:00:00Z"); + private static final Clock CLOCK = Clock.fixed(NOW, ZoneOffset.UTC); + private static final Duration SKEW = Duration.ofSeconds(60); + private static final Instant NBF = NOW.minusSeconds(60); + + private static SamlAssertionValidator.Builder base(Credential cred) { + return SamlAssertionValidator.create() + .spEntityId(AUDIENCE) + .expectedIssuer(ISSUER) + .signingCredential(cred) + .clock(CLOCK); + } + + /** A replay cache that always reports first-seen and records the expiry it was handed. */ + private static final class CapturingCache implements ReplayCache { + private final AtomicLong lastExpiry = new AtomicLong(Long.MIN_VALUE); + + @Override /* ReplayCache */ + public boolean checkAndRecord(String id, long expiresAtMs) { + lastExpiry.set(expiresAtMs); + return true; + } + } + + // ----------------------------------------------------------------------------------------------------------------- + // A: Missing NotOnOrAfter everywhere → rejected (unbounded validity window). + // ----------------------------------------------------------------------------------------------------------------- + + @Test void a01_noNotOnOrAfterAnywhere_rejected() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + // Conditions carries an audience but no NotOnOrAfter; no bearer SubjectConfirmation at all. + var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", NBF, null); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var validator = base(cred).build(); + assertThrows(AuthenticationException.class, () -> validator.validate(xml)); + } + + @Test void a02_conditionsNotOnOrAfterPresent_accepted() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", NBF, NOW.plusSeconds(300)); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + assertEquals("alice", base(cred).build().validate(xml).getName()); + } + + // ----------------------------------------------------------------------------------------------------------------- + // B: Replay-cache retention uses the resolved NotOnOrAfter (plus skew), not a fixed default. + // ----------------------------------------------------------------------------------------------------------------- + + @Test void b01_missingConditionsNoa_fallsBackToBearerConfirmationNoa() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var confNoa = NOW.plusSeconds(600); + // Conditions has no NotOnOrAfter, but a bearer confirmation supplies one; recipient(...) is NOT configured, + // so the confirmation is not enforced during validation, yet must still bound the replay-cache retention. + var sc = SamlTestSupport.bearerConfirmation(ACS, null, confNoa, null, null); + var sub = SamlTestSupport.subjectWithConfirmations("alice", sc); + var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, null); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + assertEquals("alice", base(cred).replayCache(cache).build().validate(xml).getName()); + assertEquals(confNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + @Test void b02_conditionsNoa_usedForRetention() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var condNoa = NOW.plusSeconds(300); + var assertion = SamlTestSupport.buildMinimalAssertion(ISSUER, AUDIENCE, "alice", NBF, condNoa); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + base(cred).replayCache(cache).build().validate(xml); + assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + @Test void b03_laterBearerConfirmationNoa_wins() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var condNoa = NOW.plusSeconds(300); + var confNoa = NOW.plusSeconds(900); // later than the Conditions window + var sc = SamlTestSupport.bearerConfirmation(ACS, null, confNoa, null, null); + var sub = SamlTestSupport.subjectWithConfirmations("alice", sc); + var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + base(cred).replayCache(cache).build().validate(xml); + assertEquals(confNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + @Test void b04_laterConditionsNoa_wins() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var condNoa = NOW.plusSeconds(900); // later than the confirmation window + var confNoa = NOW.plusSeconds(300); + var sc = SamlTestSupport.bearerConfirmation(ACS, null, confNoa, null, null); + var sub = SamlTestSupport.subjectWithConfirmations("alice", sc); + var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + base(cred).replayCache(cache).build().validate(xml); + assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + // ----------------------------------------------------------------------------------------------------------------- + // C: Confirmations that do not contribute a NotOnOrAfter fall through to the Conditions value. + // ----------------------------------------------------------------------------------------------------------------- + + @Test void c01_nonBearerConfirmation_ignoredForRetention() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var condNoa = NOW.plusSeconds(300); + var sub = SamlTestSupport.subjectWithConfirmations("alice", holderOfKeyConfirmation()); + var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + base(cred).replayCache(cache).build().validate(xml); + assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + @Test void c02_bearerConfirmationWithNoData_ignoredForRetention() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var condNoa = NOW.plusSeconds(300); + var sub = SamlTestSupport.subjectWithConfirmations("alice", bearerConfirmationNoData()); + var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + base(cred).replayCache(cache).build().validate(xml); + assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + @Test void c03_bearerConfirmationWithNullNoa_ignoredForRetention() throws Exception { + var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair()); + var condNoa = NOW.plusSeconds(300); + // Bearer confirmation carries a SubjectConfirmationData but no NotOnOrAfter → contributes nothing. + var sc = SamlTestSupport.bearerConfirmation(null, null, null, null, null); + var sub = SamlTestSupport.subjectWithConfirmations("alice", sc); + var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, condNoa); + var xml = SamlTestSupport.signAndBuildResponse(cred, ISSUER, assertion); + var cache = new CapturingCache(); + base(cred).replayCache(cache).build().validate(xml); + assertEquals(condNoa.plus(SKEW).toEpochMilli(), cache.lastExpiry.get()); + } + + @SuppressWarnings("unchecked") + private static SubjectConfirmation holderOfKeyConfirmation() { + var scb = (SAMLObjectBuilder<SubjectConfirmation>) SamlTestSupport.bf().getBuilder(SubjectConfirmation.DEFAULT_ELEMENT_NAME); + var sc = scb.buildObject(); + sc.setMethod(SubjectConfirmation.METHOD_HOLDER_OF_KEY); + return sc; + } + + @SuppressWarnings("unchecked") + private static SubjectConfirmation bearerConfirmationNoData() { + var scb = (SAMLObjectBuilder<SubjectConfirmation>) SamlTestSupport.bf().getBuilder(SubjectConfirmation.DEFAULT_ELEMENT_NAME); + var sc = scb.buildObject(); + sc.setMethod(SubjectConfirmation.METHOD_BEARER); + return sc; // no SubjectConfirmationData + } +}
