Harden SAML validation and add opt-in security defenses

CLAUDE.md

@@ -48,7 +48,7 @@ Settings loaded from `onelogin.saml.properties` on classpath or programmatically
 
 ### Security Defaults
 
-SHA-256 digest, RSA-SHA256 signatures, 120s clock drift tolerance. `strict` mode must be TRUE in production. Deprecated algorithms (SHA-1) rejected by default.
+SHA-256 digest, RSA-SHA256 signatures, 120s clock drift tolerance. `strict` mode must be TRUE in production. By default, deprecated algorithms are NOT rejected (`rejectDeprecatedAlg` defaults to `false`) and the IdP cert fingerprint algorithm defaults to `sha1`. For production, explicitly set `rejectDeprecatedAlg=true`, `idpCertFingerprintAlgorithm=sha256`, and `wantAssertionsSigned`/`wantMessagesSigned=true`.
 
 ## Test Resources
 

README.md

@@ -568,7 +568,11 @@ if (!errors.isEmpty()) {
 
     String relayState = request.getParameter("RelayState");
 
-    if (relayState != null && relayState != ServletUtils.getSelfRoutedURLNoQuery(request)) {
+    // SECURITY WARNING: RelayState is attacker-controlled (it comes from an HTTP parameter).
+    // Redirecting to it verbatim is an open-redirect vulnerability. Validate it against an
+    // application-controlled allowlist, or restrict it to a known-safe relative path, before
+    // calling sendRedirect.
+    if (relayState != null && !relayState.equals(ServletUtils.getSelfRoutedURLNoQuery(request))) {
         response.sendRedirect(request.getParameter("RelayState"));
     } else {
         if (attributes.isEmpty()) {

core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java

@@ -22,6 +22,7 @@
 import org.codelibs.saml2.core.model.SamlResponseStatus;
 import org.codelibs.saml2.core.model.SubjectConfirmationIssue;
 import org.codelibs.saml2.core.model.hsm.HSM;
+import org.codelibs.saml2.core.replay.ReplayCache;
 import org.codelibs.saml2.core.settings.Saml2Settings;
 import org.codelibs.saml2.core.util.Constants;
 import org.codelibs.saml2.core.util.SchemaFactory;
@@ -342,6 +343,21 @@ public boolean isValid(final String requestId) {
                 throw new ValidationException("Signature validation failed. SAML Response rejected", ValidationException.INVALID_SIGNATURE);
             }
 
+            final ReplayCache replayCache = settings.getReplayCache();
+            if (replayCache != null) {
+                final String assertionId = getAssertionId();
+                Instant expiresAt = getSessionNotOnOrAfter();
+                if (expiresAt == null) {
+                    final List<Instant> notOnOrAfters = getAssertionNotOnOrAfter();
+                    expiresAt = notOnOrAfters.isEmpty() ? Instant.now().plusSeconds(settings.getClockDrift() + 300)
+                            : java.util.Collections.min(notOnOrAfters);
+                }
+                if (replayCache.registerAndCheck(assertionId, expiresAt)) {
+                    throw new ValidationException("The Assertion was already processed (replay detected): " + assertionId,
+                            ValidationException.ASSERTION_REPLAYED);
+                }
+            }
+
             LOGGER.debug("SAMLResponse validated --> {}", samlResponseString);
             return true;
         } catch (final Exception e) {
@@ -459,7 +475,7 @@ public Map<String, String> getNameIdData() {
                                 SettingsException.PRIVATE_KEY_NOT_FOUND);
                     }
 
-                    Util.decryptElement(encryptedData, key);
+                    Util.decryptElement(encryptedData, key, settings.getAllowedKeyTransportAlgorithms());
                 }
                 nameIdNodes = this.queryAssertion("/saml:Subject/saml:EncryptedID/saml:NameID|/saml:Subject/saml:NameID");
 
@@ -1038,7 +1054,7 @@ public boolean validateSignedElements(final ArrayList<String> signedElements) {
      *
      */
     public boolean validateTimestamps() {
-        final NodeList timestampNodes = samlResponseDocument.getElementsByTagNameNS("*", "Conditions");
+        final NodeList timestampNodes = getSAMLResponseDocument().getElementsByTagNameNS("*", "Conditions");
         if (timestampNodes.getLength() != 0) {
             for (int i = 0; i < timestampNodes.getLength(); i++) {
                 final NamedNodeMap attrName = timestampNodes.item(i).getAttributes();
@@ -1203,14 +1219,14 @@ private Document decryptAssertion(final Document dom) {
         final Element encryptedData = (Element) encryptedDataNodes.item(0);
 
         if (hsm != null) {
-            Util.decryptUsingHsm(encryptedData, hsm);
+            Util.decryptUsingHsm(encryptedData, hsm, settings.getAllowedKeyTransportAlgorithms());
         } else {
-            Util.decryptElement(encryptedData, key);
+            Util.decryptElement(encryptedData, key, settings.getAllowedKeyTransportAlgorithms());
         }
 
         // We need to Remove the saml:EncryptedAssertion Node
         final NodeList AssertionDataNodes = Util.query(dom, "/samlp:Response/saml:EncryptedAssertion/saml:Assertion");
-        if (encryptedDataNodes.getLength() == 0) {
+        if (AssertionDataNodes.getLength() == 0) {
             throw new ValidationException("No /samlp:Response/saml:EncryptedAssertion/saml:Assertion element found",
                     ValidationException.MISSING_ENCRYPTED_ELEMENT);
         }

core/src/main/java/org/codelibs/saml2/core/exception/SAMLSignatureException.java

@@ -17,4 +17,13 @@ public SAMLSignatureException(Exception e) {
         super(e);
     }
 
+    /**
+     * Constructs a new {@code SAMLSignatureException} with the given message.
+     *
+     * @param message the detail message
+     */
+    public SAMLSignatureException(final String message) {
+        super(message);
+    }
+
 }

core/src/main/java/org/codelibs/saml2/core/exception/ValidationException.java

@@ -108,6 +108,10 @@ public class ValidationException extends SAMLException {
     public static final int MISSING_ENCRYPTED_ELEMENT = 48;
     /** Error code indicating an invalid IssueInstant format. */
     public static final int INVALID_ISSUE_INSTANT_FORMAT = 49;
+    /** Error code indicating the assertion was already processed (replay detected). */
+    public static final int ASSERTION_REPLAYED = 50;
+    /** Error code indicating the logout message was already processed (replay detected). */
+    public static final int MESSAGE_REPLAYED = 51;
 
     /** The error code associated with this validation failure. */
     private final int errorCode;

core/src/main/java/org/codelibs/saml2/core/logout/LogoutRequest.java

@@ -6,6 +6,7 @@
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -16,6 +17,7 @@
 import org.codelibs.saml2.core.exception.ValidationException;
 import org.codelibs.saml2.core.exception.XMLParsingException;
 import org.codelibs.saml2.core.http.HttpRequest;
+import org.codelibs.saml2.core.replay.ReplayCache;
 import org.codelibs.saml2.core.settings.Saml2Settings;
 import org.codelibs.saml2.core.util.Constants;
 import org.codelibs.saml2.core.util.SchemaFactory;
@@ -389,7 +391,8 @@ private StringSubstitutor generateSubstitutor(final LogoutRequestParams params,
             }
         }
 
-        final String nameIdStr = Util.generateNameId(nameId, spNameQualifier, nameIdFormat, nameQualifier, cert);
+        final String nameIdStr =
+                Util.generateNameId(nameId, spNameQualifier, nameIdFormat, nameQualifier, cert, settings.getNameIdEncryptionAlgorithm());
         valueMap.put("nameIdStr", nameIdStr);
 
         String sessionIndexStr = "";
@@ -470,7 +473,7 @@ public boolean isValid() {
                     }
                 }
 
-                getNameId(logoutRequestDocument, settings.getSPkey(), settings.isTrimNameIds());
+                getNameId(logoutRequestDocument, settings.getSPkey(), settings.isTrimNameIds(), settings.getAllowedKeyTransportAlgorithms());
 
                 // Check the issuer
                 final String issuer = getIssuer(logoutRequestDocument, settings.isTrimNameIds());
@@ -530,6 +533,16 @@ public boolean isValid() {
                 }
             }
 
+            final ReplayCache replayCache = settings.getReplayCache();
+            if (replayCache != null) {
+                final String messageId = getId();
+                final Instant expiresAt = Instant.now().plusSeconds(settings.getClockDrift() + 300);
+                if (replayCache.registerAndCheck(messageId, expiresAt)) {
+                    throw new ValidationException("The LogoutRequest was already processed (replay detected): " + messageId,
+                            ValidationException.MESSAGE_REPLAYED);
+                }
+            }
+
             LOGGER.debug("LogoutRequest validated --> {}", logoutRequestString);
             return true;
         } catch (final Exception e) {
@@ -642,6 +655,28 @@ public static Map<String, String> getNameIdData(final Document samlLogoutRequest
      */
     public static Map<String, String> getNameIdData(final Document samlLogoutRequestDocument, final PrivateKey key,
             final boolean trimValue) {
+        return getNameIdData(samlLogoutRequestDocument, key, trimValue, null);
+    }
+
+    /**
+     * Gets the NameID Data from the the Logout Request Document, optionally restricting the accepted
+     * key transport (key wrapping) algorithm of an encrypted NameID to an allow-list.
+     *
+     * @param samlLogoutRequestDocument
+     *              A DOMDocument object loaded from the SAML Logout Request.
+     * @param key
+     *              The SP key to decrypt the NameID if encrypted
+     * @param trimValue
+     *              whether the extracted Name ID value should be trimmed
+     * @param allowedKeyTransportAlgorithms
+     *              The set of allowed key transport algorithm URIs. If {@code null} or empty, every
+     *              algorithm is accepted, preserving the historical permissive behavior.
+     *
+     * @return the Name ID Data (Value, Format, NameQualifier, SPNameQualifier)
+     *
+     */
+    public static Map<String, String> getNameIdData(final Document samlLogoutRequestDocument, final PrivateKey key,
+            final boolean trimValue, final Collection<String> allowedKeyTransportAlgorithms) {
         try {
             final NodeList encryptedIDNodes = Util.query(samlLogoutRequestDocument, "/samlp:LogoutRequest/saml:EncryptedID");
             NodeList nameIdNodes;
@@ -653,7 +688,7 @@ public static Map<String, String> getNameIdData(final Document samlLogoutRequest
                 }
 
                 final Element encryptedData = (Element) encryptedIDNodes.item(0);
-                Util.decryptElement(encryptedData, key);
+                Util.decryptElement(encryptedData, key, allowedKeyTransportAlgorithms);
                 nameIdNodes = Util.query(samlLogoutRequestDocument, "/samlp:LogoutRequest/saml:NameID");
 
                 if (nameIdNodes == null || nameIdNodes.getLength() != 1) {
@@ -760,7 +795,32 @@ public static String getNameId(final Document samlLogoutRequestDocument, final P
      *
      */
     public static String getNameId(final Document samlLogoutRequestDocument, final PrivateKey key, final boolean trimValue) {
-        final Map<String, String> nameIdData = getNameIdData(samlLogoutRequestDocument, key, trimValue);
+        return getNameId(samlLogoutRequestDocument, key, trimValue, null);
+    }
+
+    /**
+     * Gets the NameID value provided from the SAML Logout Request Document, optionally restricting the
+     * accepted key transport (key wrapping) algorithm of an encrypted NameID to an allow-list.
+     *
+     * @param samlLogoutRequestDocument
+     *              A DOMDocument object loaded from the SAML Logout Request.
+     *
+     * @param key
+     *              The SP key to decrypt the NameID if encrypted
+     *
+     * @param trimValue
+     *              whether the extracted Name ID value should be trimmed
+     *
+     * @param allowedKeyTransportAlgorithms
+     *              The set of allowed key transport algorithm URIs. If {@code null} or empty, every
+     *              algorithm is accepted, preserving the historical permissive behavior.
+     *
+     * @return the Name ID value
+     *
+     */
+    public static String getNameId(final Document samlLogoutRequestDocument, final PrivateKey key, final boolean trimValue,
+            final Collection<String> allowedKeyTransportAlgorithms) {
+        final Map<String, String> nameIdData = getNameIdData(samlLogoutRequestDocument, key, trimValue, allowedKeyTransportAlgorithms);
         if (LOGGER.isDebugEnabled()) {
             LOGGER.debug("LogoutRequest has NameID --> {}", nameIdData.get("Value"));
         }

core/src/main/java/org/codelibs/saml2/core/logout/LogoutResponse.java

@@ -2,6 +2,7 @@
 
 import java.net.URL;
 import java.security.cert.X509Certificate;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.HashMap;
@@ -14,6 +15,7 @@
 import org.codelibs.saml2.core.exception.ValidationException;
 import org.codelibs.saml2.core.http.HttpRequest;
 import org.codelibs.saml2.core.model.SamlResponseStatus;
+import org.codelibs.saml2.core.replay.ReplayCache;
 import org.codelibs.saml2.core.settings.Saml2Settings;
 import org.codelibs.saml2.core.util.Constants;
 import org.codelibs.saml2.core.util.SchemaFactory;
@@ -292,6 +294,16 @@ public boolean isValid(final String requestId) {
                 }
             }
 
+            final ReplayCache replayCache = settings.getReplayCache();
+            if (replayCache != null) {
+                final String messageId = getId();
+                final Instant expiresAt = Instant.now().plusSeconds(settings.getClockDrift() + 300);
+                if (replayCache.registerAndCheck(messageId, expiresAt)) {
+                    throw new ValidationException("The LogoutResponse was already processed (replay detected): " + messageId,
+                            ValidationException.MESSAGE_REPLAYED);
+                }
+            }
+
             LOGGER.debug("LogoutRequest validated --> {}", logoutResponseString);
             return true;
         } catch (final Exception e) {

core/src/main/java/org/codelibs/saml2/core/replay/InMemoryReplayCache.java

@@ -0,0 +1,64 @@
+package org.codelibs.saml2.core.replay;
+
+import java.time.Instant;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+
+/**
+ * Default in-memory, thread-safe {@link ReplayCache} implementation backed by a
+ * {@link ConcurrentHashMap}. Expired entries are evicted opportunistically (no
+ * background threads are started by this class).
+ */
+public class InMemoryReplayCache implements ReplayCache {
+
+    /** Number of registrations between opportunistic sweeps of expired entries. */
+    private static final long SWEEP_INTERVAL = 1000L;
+
+    /** Map of id to the instant after which the entry may be evicted. */
+    private final ConcurrentHashMap<String, Instant> entries = new ConcurrentHashMap<>();
+
+    /** Counts registrations to decide when to opportunistically sweep expired entries. */
+    private final AtomicLong registrationCount = new AtomicLong();
+
+    /**
+     * Constructs a new, empty {@code InMemoryReplayCache}.
+     */
+    public InMemoryReplayCache() {
+        // No-op: entries map starts empty.
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public boolean registerAndCheck(final String id, final Instant expiresAt) {
+        final Instant now = Instant.now();
+        final AtomicBoolean replay = new AtomicBoolean(false);
+
+        entries.compute(id, (key, previousExpiresAt) -> {
+            if (previousExpiresAt != null && previousExpiresAt.isAfter(now)) {
+                // Still-valid previous entry: this is a replay, keep the existing expiry.
+                replay.set(true);
+                return previousExpiresAt;
+            }
+            // No previous entry, or the previous entry already expired: treat as a first use.
+            replay.set(false);
+            return expiresAt;
+        });
+
+        if (registrationCount.incrementAndGet() % SWEEP_INTERVAL == 0) {
+            sweep(now);
+        }
+
+        return replay.get();
+    }
+
+    /**
+     * Opportunistically removes entries whose expiry is not after the given instant.
+     *
+     * @param now the instant to compare entry expiries against
+     */
+    private void sweep(final Instant now) {
+        entries.entrySet().removeIf(entry -> !entry.getValue().isAfter(now));
+    }
+
+}

core/src/main/java/org/codelibs/saml2/core/replay/ReplayCache.java

@@ -0,0 +1,20 @@
+package org.codelibs.saml2.core.replay;
+
+import java.time.Instant;
+
+/**
+ * Thread-safe store for detecting replayed SAML message/assertion IDs.
+ * Implementations MUST be thread-safe.
+ */
+public interface ReplayCache {
+
+    /**
+     * Atomically records the id if unseen and reports whether it was already present.
+     *
+     * @param id the assertion or message ID
+     * @param expiresAt instant after which the entry may be evicted (never null)
+     * @return true if id was already registered (=> replay); false if newly registered (first use)
+     */
+    boolean registerAndCheck(String id, Instant expiresAt);
+
+}