diff --git a/CLAUDE.md b/CLAUDE.md index e419c0d..0fc72be 100644 --- a/CLAUDE.md +++ b/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 diff --git a/README.md b/README.md index 8b4140b..9b2dd04 100644 --- a/README.md +++ b/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()) { diff --git a/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java b/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java index f2c8486..88f276d 100644 --- a/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java +++ b/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 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 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 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); } diff --git a/core/src/main/java/org/codelibs/saml2/core/exception/SAMLSignatureException.java b/core/src/main/java/org/codelibs/saml2/core/exception/SAMLSignatureException.java index f73780e..1d691d0 100644 --- a/core/src/main/java/org/codelibs/saml2/core/exception/SAMLSignatureException.java +++ b/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); + } + } diff --git a/core/src/main/java/org/codelibs/saml2/core/exception/ValidationException.java b/core/src/main/java/org/codelibs/saml2/core/exception/ValidationException.java index b7055a8..b5b856e 100644 --- a/core/src/main/java/org/codelibs/saml2/core/exception/ValidationException.java +++ b/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; diff --git a/core/src/main/java/org/codelibs/saml2/core/logout/LogoutRequest.java b/core/src/main/java/org/codelibs/saml2/core/logout/LogoutRequest.java index efbd361..a70ce9e 100644 --- a/core/src/main/java/org/codelibs/saml2/core/logout/LogoutRequest.java +++ b/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 getNameIdData(final Document samlLogoutRequest */ public static Map 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 getNameIdData(final Document samlLogoutRequestDocument, final PrivateKey key, + final boolean trimValue, final Collection allowedKeyTransportAlgorithms) { try { final NodeList encryptedIDNodes = Util.query(samlLogoutRequestDocument, "/samlp:LogoutRequest/saml:EncryptedID"); NodeList nameIdNodes; @@ -653,7 +688,7 @@ public static Map 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 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 allowedKeyTransportAlgorithms) { + final Map nameIdData = getNameIdData(samlLogoutRequestDocument, key, trimValue, allowedKeyTransportAlgorithms); if (LOGGER.isDebugEnabled()) { LOGGER.debug("LogoutRequest has NameID --> {}", nameIdData.get("Value")); } diff --git a/core/src/main/java/org/codelibs/saml2/core/logout/LogoutResponse.java b/core/src/main/java/org/codelibs/saml2/core/logout/LogoutResponse.java index 80c5a4b..e41a87d 100644 --- a/core/src/main/java/org/codelibs/saml2/core/logout/LogoutResponse.java +++ b/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) { diff --git a/core/src/main/java/org/codelibs/saml2/core/replay/InMemoryReplayCache.java b/core/src/main/java/org/codelibs/saml2/core/replay/InMemoryReplayCache.java new file mode 100644 index 0000000..1c8071c --- /dev/null +++ b/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 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)); + } + +} diff --git a/core/src/main/java/org/codelibs/saml2/core/replay/ReplayCache.java b/core/src/main/java/org/codelibs/saml2/core/replay/ReplayCache.java new file mode 100644 index 0000000..642de75 --- /dev/null +++ b/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); + +} diff --git a/core/src/main/java/org/codelibs/saml2/core/settings/IdPMetadataParser.java b/core/src/main/java/org/codelibs/saml2/core/settings/IdPMetadataParser.java index de52775..6f994dc 100644 --- a/core/src/main/java/org/codelibs/saml2/core/settings/IdPMetadataParser.java +++ b/core/src/main/java/org/codelibs/saml2/core/settings/IdPMetadataParser.java @@ -3,10 +3,12 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; +import java.security.cert.X509Certificate; import java.util.LinkedHashMap; import java.util.Map; import org.codelibs.saml2.core.exception.SAMLSevereException; +import org.codelibs.saml2.core.exception.SAMLSignatureException; import org.codelibs.saml2.core.exception.XMLParsingException; import org.codelibs.saml2.core.util.Constants; import org.codelibs.saml2.core.util.Util; @@ -24,6 +26,17 @@ * * This class does not validate in any way the URL that is introduced, * make sure to validate it properly before use it in a get_metadata method. + * + *

Security note on {@code trustedSigningCert} overloads: the overloads of + * {@code parseXML}/{@code parseFileXML}/{@code parseRemoteXML} that accept a trailing + * {@link X509Certificate} verify that the fetched metadata document is signed by that + * certificate before parsing it. That certificate MUST be provisioned out-of-band + * (e.g. from pinned configuration or a local keystore) and MUST NOT be extracted from + * the very same metadata document being validated. Extracting the trust anchor from the + * document under validation only protects against transport-level corruption; it does + * nothing to detect a MITM attacker or a compromised source serving a self-consistent + * forged document, since the attacker would simply embed their own certificate alongside + * their own signature. */ public class IdPMetadataParser { @@ -153,6 +166,42 @@ public static Map parseXML(final Document xmlDocument, String en return metadataInfo; } + /** + * Get IdP Metadata Info from XML Document, verifying beforehand that the document is signed + * by the given trusted certificate. + * + *

Security note: {@code trustedSigningCert} MUST be provisioned out-of-band (e.g. + * pinned configuration or a local keystore entry), NOT extracted from the same metadata + * document being validated here. Using a certificate embedded in the document under + * validation only detects transport corruption, not a MITM attacker or a compromised source + * serving a self-consistent forged document (which would simply embed its own certificate + * alongside its own signature). + * + * @param xmlDocument + * XML document hat contains IdP metadata + * @param entityId + * Entity Id of the desired IdP, if no entity Id is provided and the XML metadata contains more than one IDPSSODescriptor, the first is returned + * @param desiredNameIdFormat + * If available on IdP metadata, use that nameIdFormat + * @param desiredSSOBinding + * Parse specific binding SSO endpoint. + * @param desiredSLOBinding + * Parse specific binding SLO endpoint. + * @param trustedSigningCert + * the out-of-band trusted certificate the metadata document must be signed with; if {@code null} no signature check is performed + * + * @return Mapped values with metadata info in Saml2Settings format + * + * @throws SAMLSignatureException if {@code trustedSigningCert} is not {@code null} and the metadata document signature does not validate against it + */ + public static Map parseXML(final Document xmlDocument, final String entityId, final String desiredNameIdFormat, + final String desiredSSOBinding, final String desiredSLOBinding, final X509Certificate trustedSigningCert) { + if (trustedSigningCert != null && !Util.validateMetadataSign(xmlDocument, trustedSigningCert, null, null)) { + throw new SAMLSignatureException("Metadata signature validation failed (entityId=" + entityId + ")"); + } + return parseXML(xmlDocument, entityId, desiredNameIdFormat, desiredSSOBinding, desiredSLOBinding); + } + /** * Get IdP Metadata Info from XML Document * @@ -214,6 +263,75 @@ public static Map parseFileXML(final String xmlFileName, final S SAMLSevereException.SETTINGS_FILE_NOT_FOUND); } + /** + * Get IdP Metadata Info from XML file, verifying beforehand that the document is signed by + * the given trusted certificate. + * + *

Security note: {@code trustedSigningCert} MUST be provisioned out-of-band (e.g. + * pinned configuration or a local keystore entry), NOT extracted from the same metadata + * document being validated here. See {@link #parseXML(Document, String, String, String, String, X509Certificate)} + * for details. + * + * @param xmlFileName + * Filename of the XML filename that contains IdP metadata + * @param entityId + * Entity Id of the desired IdP, if no entity Id is provided and the XML metadata contains more than one IDPSSODescriptor, the first is returned + * @param desiredNameIdFormat + * If available on IdP metadata, use that nameIdFormat + * @param desiredSSOBinding + * Parse specific binding SSO endpoint. + * @param desiredSLOBinding + * Parse specific binding SLO endpoint. + * @param trustedSigningCert + * the out-of-band trusted certificate the metadata document must be signed with; if {@code null} no signature check is performed + * + * @return Mapped values with metadata info in Saml2Settings format + * + * @throws SAMLSignatureException if {@code trustedSigningCert} is not {@code null} and the metadata document signature does not validate against it + */ + public static Map parseFileXML(final String xmlFileName, final String entityId, final String desiredNameIdFormat, + final String desiredSSOBinding, final String desiredSLOBinding, final X509Certificate trustedSigningCert) { + final ClassLoader classLoader = IdPMetadataParser.class.getClassLoader(); + try (InputStream inputStream = classLoader.getResourceAsStream(xmlFileName)) { + if (inputStream != null) { + final Document xmlDocument = Util.parseXML(new InputSource(inputStream)); + return parseXML(xmlDocument, entityId, desiredNameIdFormat, desiredSSOBinding, desiredSLOBinding, trustedSigningCert); + } + } catch (final SAMLSignatureException e) { + throw e; + } catch (final Exception e) { + final String errorMsg = "XML file'" + xmlFileName + "' cannot be loaded." + e.getMessage(); + throw new SAMLSevereException(errorMsg, SAMLSevereException.SETTINGS_FILE_NOT_FOUND, e); + } + throw new SAMLSevereException("XML file '" + xmlFileName + "' not found in the classpath", + SAMLSevereException.SETTINGS_FILE_NOT_FOUND); + } + + /** + * Get IdP Metadata Info from XML file, verifying beforehand that the document is signed by + * the given trusted certificate. Shorthand for + * {@link #parseFileXML(String, String, String, String, String, X509Certificate)} with + * {@code entityId=null}, {@code desiredNameIdFormat=null} and both bindings set to + * {@link Constants#BINDING_HTTP_REDIRECT}. + * + *

Security note: {@code trustedSigningCert} MUST be provisioned out-of-band, NOT + * extracted from the same metadata document being validated here. See + * {@link #parseXML(Document, String, String, String, String, X509Certificate)} for details. + * + * @param xmlFileName + * Filename of the XML filename that contains IdP metadata + * @param trustedSigningCert + * the out-of-band trusted certificate the metadata document must be signed with; if {@code null} no signature check is performed + * + * @return Mapped values with metadata info in Saml2Settings format + * + * @throws SAMLSignatureException if {@code trustedSigningCert} is not {@code null} and the metadata document signature does not validate against it + */ + public static Map parseFileXML(final String xmlFileName, final X509Certificate trustedSigningCert) { + return parseFileXML(xmlFileName, null, null, Constants.BINDING_HTTP_REDIRECT, Constants.BINDING_HTTP_REDIRECT, + trustedSigningCert); + } + /** * Get IdP Metadata Info from XML file * @@ -239,7 +357,7 @@ public static Map parseFileXML(final String xmlFileName, final S * */ public static Map parseFileXML(final String xmlFileName) { - return parseFileXML(xmlFileName, null); + return parseFileXML(xmlFileName, (String) null); } /** @@ -269,6 +387,66 @@ public static Map parseRemoteXML(final URL xmlURL, final String } } + /** + * Get IdP Metadata Info from XML file, verifying beforehand that the document is signed by + * the given trusted certificate. + * + *

Security note: {@code trustedSigningCert} MUST be provisioned out-of-band (e.g. + * pinned configuration or a local keystore entry), NOT extracted from the same metadata + * document being validated here. See {@link #parseXML(Document, String, String, String, String, X509Certificate)} + * for details. + * + * @param xmlURL + * URL to the XML document that contains IdP metadata + * @param entityId + * Entity Id of the desired IdP, if no entity Id is provided and the XML metadata contains more than one IDPSSODescriptor, the first is returned + * @param desiredNameIdFormat + * If available on IdP metadata, use that nameIdFormat + * @param desiredSSOBinding + * Parse specific binding SSO endpoint. + * @param desiredSLOBinding + * Parse specific binding SLO endpoint. + * @param trustedSigningCert + * the out-of-band trusted certificate the metadata document must be signed with; if {@code null} no signature check is performed + * + * @return Mapped values with metadata info in Saml2Settings format + * + * @throws SAMLSignatureException if {@code trustedSigningCert} is not {@code null} and the metadata document signature does not validate against it + */ + public static Map parseRemoteXML(final URL xmlURL, final String entityId, final String desiredNameIdFormat, + final String desiredSSOBinding, final String desiredSLOBinding, final X509Certificate trustedSigningCert) { + try (InputStream is = xmlURL.openStream()) { + final Document xmlDocument = Util.parseXML(new InputSource(is)); + return parseXML(xmlDocument, entityId, desiredNameIdFormat, desiredSSOBinding, desiredSLOBinding, trustedSigningCert); + } catch (IOException e) { + throw new XMLParsingException("Failed to parse a remote XML: " + xmlURL, e); + } + } + + /** + * Get IdP Metadata Info from XML file, verifying beforehand that the document is signed by + * the given trusted certificate. Shorthand for + * {@link #parseRemoteXML(URL, String, String, String, String, X509Certificate)} with + * {@code entityId=null}, {@code desiredNameIdFormat=null} and both bindings set to + * {@link Constants#BINDING_HTTP_REDIRECT}. + * + *

Security note: {@code trustedSigningCert} MUST be provisioned out-of-band, NOT + * extracted from the same metadata document being validated here. See + * {@link #parseXML(Document, String, String, String, String, X509Certificate)} for details. + * + * @param xmlURL + * URL to the XML document that contains IdP metadata + * @param trustedSigningCert + * the out-of-band trusted certificate the metadata document must be signed with; if {@code null} no signature check is performed + * + * @return Mapped values with metadata info in Saml2Settings format + * + * @throws SAMLSignatureException if {@code trustedSigningCert} is not {@code null} and the metadata document signature does not validate against it + */ + public static Map parseRemoteXML(final URL xmlURL, final X509Certificate trustedSigningCert) { + return parseRemoteXML(xmlURL, null, null, Constants.BINDING_HTTP_REDIRECT, Constants.BINDING_HTTP_REDIRECT, trustedSigningCert); + } + /** * Get IdP Metadata Info from XML file * @@ -294,7 +472,7 @@ public static Map parseRemoteXML(final URL xmlURL, final String * */ public static Map parseRemoteXML(final URL xmlURL) { - return parseRemoteXML(xmlURL, null); + return parseRemoteXML(xmlURL, (String) null); } /** diff --git a/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java b/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java index 159ee9e..0d938bf 100644 --- a/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java +++ b/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java @@ -13,6 +13,7 @@ import org.codelibs.saml2.core.model.Contact; import org.codelibs.saml2.core.model.Organization; import org.codelibs.saml2.core.model.hsm.HSM; +import org.codelibs.saml2.core.replay.ReplayCache; import org.codelibs.saml2.core.util.Constants; import org.codelibs.saml2.core.util.SchemaFactory; import org.codelibs.saml2.core.util.Util; @@ -54,6 +55,7 @@ public Saml2Settings() { private X509Certificate spX509certNew = null; private PrivateKey spPrivateKey = null; private HSM hsm = null; + private ReplayCache replayCache = null; // IdP private String idpEntityId = ""; @@ -83,6 +85,8 @@ public Saml2Settings() { private boolean wantXMLValidation = true; private String signatureAlgorithm = Constants.RSA_SHA256; private String digestAlgorithm = Constants.SHA256; + private String nameIdEncryptionAlgorithm = Constants.RSA_1_5; + private Set allowedKeyTransportAlgorithms = null; private boolean rejectUnsolicitedResponsesWithInResponseTo = false; private boolean allowRepeatAttributeName = false; private boolean rejectDeprecatedAlg = false; @@ -320,6 +324,15 @@ public boolean getNameIdEncrypted() { return nameIdEncrypted; } + /** + * Returns the key transport (key wrapping) algorithm used to encrypt the NameID. + * + * @return the nameIdEncryptionAlgorithm setting value + */ + public String getNameIdEncryptionAlgorithm() { + return nameIdEncryptionAlgorithm; + } + /** * Returns whether authentication requests should be signed. * @@ -446,6 +459,17 @@ public String getDigestAlgorithm() { return digestAlgorithm; } + /** + * Returns the allow-list of key transport (key wrapping) algorithm URIs accepted when + * decrypting an inbound {@code xenc:EncryptedKey}. + * + * @return the allowedKeyTransportAlgorithms setting value, or {@code null} if every + * algorithm is accepted (permissive default) + */ + public Set getAllowedKeyTransportAlgorithms() { + return allowedKeyTransportAlgorithms; + } + /** * Returns the Service Provider contact information. * @@ -482,6 +506,17 @@ public HSM getHsm() { return this.hsm; } + /** + * Returns the configured replay cache used to detect replayed SAML assertions + * and logout messages. + * + * @return the ReplayCache setting value, or {@code null} if replay checking is + * not configured (the default). + */ + public ReplayCache getReplayCache() { + return this.replayCache; + } + /** * Returns whether debug mode is active. * @@ -539,6 +574,16 @@ public void setHsm(final HSM hsm) { this.hsm = hsm; } + /** + * Sets the replay cache used to detect replayed SAML assertions and logout + * messages. When {@code null} (the default), no replay checking is performed. + * + * @param replayCache The ReplayCache object to be set. + */ + public void setReplayCache(final ReplayCache replayCache) { + this.replayCache = replayCache; + } + /** * Set the spEntityId setting value * @@ -921,6 +966,29 @@ public void setDigestAlgorithm(final String digestAlgorithm) { this.digestAlgorithm = digestAlgorithm; } + /** + * Set the nameIdEncryptionAlgorithm setting value + * + * @param nameIdEncryptionAlgorithm + * the nameIdEncryptionAlgorithm value to be set. The key transport (key wrapping) + * algorithm used when encrypting the NameID. + */ + public void setNameIdEncryptionAlgorithm(final String nameIdEncryptionAlgorithm) { + this.nameIdEncryptionAlgorithm = nameIdEncryptionAlgorithm; + } + + /** + * Set the allowedKeyTransportAlgorithms setting value + * + * @param allowedKeyTransportAlgorithms + * the allow-list of key transport (key wrapping) algorithm URIs accepted when + * decrypting an inbound {@code xenc:EncryptedKey}. {@code null} accepts every + * algorithm (permissive default). + */ + public void setAllowedKeyTransportAlgorithms(final Set allowedKeyTransportAlgorithms) { + this.allowedKeyTransportAlgorithms = allowedKeyTransportAlgorithms; + } + /** * Controls if unsolicited Responses are rejected if they contain an InResponseTo value. * @@ -1074,6 +1142,31 @@ public List checkSettings() { return errors; } + /** + * Returns a list of non-fatal security warnings about the current configuration. Unlike + * {@link #checkSettings()}, these are not configuration errors, but weak or insecure + * settings that should be reviewed before running in production. + * + * @return the list of security warning codes found on the settings data + */ + public List getSecurityWarnings() { + final List warnings = new ArrayList<>(); + if (!strict) { + warnings.add("strict_mode_disabled"); + } + if (idpCertFingerprint != null && !"sha256".equalsIgnoreCase(idpCertFingerprintAlgorithm) + && !"sha384".equalsIgnoreCase(idpCertFingerprintAlgorithm) && !"sha512".equalsIgnoreCase(idpCertFingerprintAlgorithm)) { + warnings.add("weak_fingerprint_algorithm"); + } + if (!rejectDeprecatedAlg) { + warnings.add("deprecated_signature_algorithms_not_rejected"); + } + if (!wantAssertionsSigned && !wantMessagesSigned) { + warnings.add("assertions_and_messages_not_required_signed"); + } + return warnings; + } + /** * Checks the IdP settings . * @@ -1275,12 +1368,52 @@ public String getSPMetadata() { /** * Validates an XML SP Metadata. * + *

This overload never performs metadata signature validation; it is preserved + * unchanged for backward compatibility. Use {@link #validateMetadata(String, X509Certificate)} + * or {@link #validateMetadata(String, X509Certificate, String, String, boolean)} to additionally + * validate the metadata signature. + * + * @param metadataString Metadata's XML that will be validate + * + * @return Array The list of found errors + * + */ + public static List validateMetadata(final String metadataString) { + return validateMetadata(metadataString, null, null, null, false); + } + + /** + * Validates an XML SP Metadata, additionally checking that it is signed by the given certificate. + * * @param metadataString Metadata's XML that will be validate + * @param cert the certificate whose public key is used to validate the metadata's signature * * @return Array The list of found errors * */ - public static List validateMetadata(String metadataString) { + public static List validateMetadata(final String metadataString, final X509Certificate cert) { + return validateMetadata(metadataString, cert, null, null, false); + } + + /** + * Validates an XML SP Metadata, additionally checking that it is signed either by the given + * certificate or by a certificate matching the given fingerprint. + * + *

Signature validation is opt-in: it is only performed when {@code cert} is non-null or + * {@code fingerprint} is non-null/non-empty. When neither is provided this method behaves + * exactly like {@link #validateMetadata(String)}. + * + * @param metadataString Metadata's XML that will be validate + * @param cert the certificate whose public key is used to validate the metadata's signature, or {@code null} + * @param fingerprint the fingerprint of the certificate used to validate the metadata's signature, or {@code null} + * @param alg the fingerprint algorithm + * @param rejectDeprecatedAlg whether to reject signatures using deprecated algorithms + * + * @return Array The list of found errors + * + */ + public static List validateMetadata(String metadataString, final X509Certificate cert, final String fingerprint, + final String alg, final boolean rejectDeprecatedAlg) { metadataString = metadataString.replace("", ""); @@ -1316,14 +1449,11 @@ public static List validateMetadata(String metadataString) { } } - // Note: Metadata signature validation is not currently implemented. - // Future enhancement: Add signature validation for SP metadata to ensure integrity. - // Implementation considerations: - // - Need to determine which certificate to use for validation (SP cert or dedicated metadata cert) - // - Should validation be mandatory or optional based on configuration? - // - Use Util.validateMetadataSign() or similar validation method - // - Add appropriate error messages to the errors list if validation fails - // Security recommendation: Metadata signatures should be validated when received from external sources. + if (cert != null || (fingerprint != null && !fingerprint.isEmpty())) { + if (!Util.validateMetadataSign(metadataDocument, cert, fingerprint, alg, rejectDeprecatedAlg)) { + errors.add("metadata_signature_invalid"); + } + } return errors; } diff --git a/core/src/main/java/org/codelibs/saml2/core/settings/SettingsBuilder.java b/core/src/main/java/org/codelibs/saml2/core/settings/SettingsBuilder.java index 17452fb..b3d317a 100644 --- a/core/src/main/java/org/codelibs/saml2/core/settings/SettingsBuilder.java +++ b/core/src/main/java/org/codelibs/saml2/core/settings/SettingsBuilder.java @@ -15,12 +15,14 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; import java.util.Properties; +import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; import java.util.regex.Matcher; @@ -169,6 +171,10 @@ public SettingsBuilder() { public final static String SECURITY_SIGNATURE_ALGORITHM = "onelogin.saml2.security.signature_algorithm"; /** Property key for the digest algorithm used when signing ({@code onelogin.saml2.security.digest_algorithm}). */ public final static String SECURITY_DIGEST_ALGORITHM = "onelogin.saml2.security.digest_algorithm"; + /** Property key for the key transport algorithm used to encrypt the NameID ({@code onelogin.saml2.security.nameid_encryption_algorithm}). */ + public final static String SECURITY_NAMEID_ENCRYPTION_ALGORITHM = "onelogin.saml2.security.nameid_encryption_algorithm"; + /** Property key for the comma-separated allow-list of key transport algorithms accepted on decrypt ({@code onelogin.saml2.security.allowed_key_transport_algorithms}). */ + public final static String SECURITY_ALLOWED_KEY_TRANSPORT_ALGORITHMS = "onelogin.saml2.security.allowed_key_transport_algorithms"; /** Property key controlling whether unsolicited responses with an InResponseTo attribute are rejected ({@code onelogin.saml2.security.reject_unsolicited_responses_with_inresponseto}). */ public final static String SECURITY_REJECT_UNSOLICITED_RESPONSES_WITH_INRESPONSETO = "onelogin.saml2.security.reject_unsolicited_responses_with_inresponseto"; @@ -379,6 +385,12 @@ public Saml2Settings build(final Saml2Settings saml2Setting) { saml2Setting.setUniqueIDPrefix(Util.UNIQUE_ID_PREFIX); } + if (LOGGER.isWarnEnabled()) { + for (final String warning : saml2Setting.getSecurityWarnings()) { + LOGGER.warn("SAML security warning: {}", warning); + } + } + return saml2Setting; } @@ -516,6 +528,22 @@ private void loadSecuritySetting() { saml2Setting.setDigestAlgorithm(digestAlgorithm); } + final String nameIdEncryptionAlgorithm = loadStringProperty(SECURITY_NAMEID_ENCRYPTION_ALGORITHM); + if (nameIdEncryptionAlgorithm != null && !nameIdEncryptionAlgorithm.isEmpty()) { + saml2Setting.setNameIdEncryptionAlgorithm(nameIdEncryptionAlgorithm); + } + + final List allowedKeyTransportAlgorithms = loadListProperty(SECURITY_ALLOWED_KEY_TRANSPORT_ALGORITHMS); + if (allowedKeyTransportAlgorithms != null) { + final Set allowedKeyTransportAlgorithmsSet = new HashSet<>(); + for (final String allowedKeyTransportAlgorithm : allowedKeyTransportAlgorithms) { + if (allowedKeyTransportAlgorithm != null && !allowedKeyTransportAlgorithm.trim().isEmpty()) { + allowedKeyTransportAlgorithmsSet.add(allowedKeyTransportAlgorithm.trim()); + } + } + saml2Setting.setAllowedKeyTransportAlgorithms(allowedKeyTransportAlgorithmsSet); + } + final Boolean rejectUnsolicitedResponsesWithInResponseTo = loadBooleanProperty(SECURITY_REJECT_UNSOLICITED_RESPONSES_WITH_INRESPONSETO); if (rejectUnsolicitedResponsesWithInResponseTo != null) { diff --git a/core/src/main/java/org/codelibs/saml2/core/util/Util.java b/core/src/main/java/org/codelibs/saml2/core/util/Util.java index fb37b50..6c615f6 100644 --- a/core/src/main/java/org/codelibs/saml2/core/util/Util.java +++ b/core/src/main/java/org/codelibs/saml2/core/util/Util.java @@ -37,6 +37,7 @@ import java.time.temporal.TemporalAmount; import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -1242,12 +1243,37 @@ public static boolean isAlgorithmWhitelisted(final String alg) { * The private key to decrypt. */ public static void decryptElement(final Element encryptedDataElement, final PrivateKey inputKey) { + decryptElement(encryptedDataElement, inputKey, null); + } + + /** + * Decrypt an encrypted element, optionally restricting the accepted key transport + * (key wrapping) algorithm of the inbound {@code xenc:EncryptedKey} to an allow-list. + * + * @param encryptedDataElement + * The encrypted element. + * @param inputKey + * The private key to decrypt. + * @param allowedKeyTransportAlgorithms + * The set of allowed key transport algorithm URIs. If {@code null} or empty, every + * algorithm is accepted, preserving the historical permissive behavior. + */ + public static void decryptElement(final Element encryptedDataElement, final PrivateKey inputKey, + final Collection allowedKeyTransportAlgorithms) { try { final XMLCipher xmlCipher = XMLCipher.getInstance(); xmlCipher.init(XMLCipher.DECRYPT_MODE, null); validateEncryptedData(encryptedDataElement); + final String keyTransportAlgorithm = findKeyTransportAlgorithm(encryptedDataElement); + if (keyTransportAlgorithm != null && !isKeyTransportAlgorithmAllowed(keyTransportAlgorithm, allowedKeyTransportAlgorithms)) { + // Do not throw a distinct exception: an algorithm rejection must be indistinguishable + // from any other decryption failure (e.g. a wrong key) to avoid a new side channel. + LOGGER.warn("Failed to decrypt an element."); + return; + } + xmlCipher.setKEK(inputKey); xmlCipher.doFinal(encryptedDataElement.getOwnerDocument(), encryptedDataElement, false); } catch (final Exception e) { @@ -1263,6 +1289,22 @@ public static void decryptElement(final Element encryptedDataElement, final Priv * */ public static void decryptUsingHsm(final Element encryptedDataElement, final HSM hsm) { + decryptUsingHsm(encryptedDataElement, hsm, null); + } + + /** + * Decrypts the encrypted element using an HSM, optionally restricting the accepted key + * transport (key wrapping) algorithm of the inbound {@code xenc:EncryptedKey} to an allow-list. + * + * @param encryptedDataElement The encrypted element. + * @param hsm The HSM object. + * @param allowedKeyTransportAlgorithms + * The set of allowed key transport algorithm URIs. If {@code null} or empty, every + * algorithm is accepted, preserving the historical permissive behavior. + * + */ + public static void decryptUsingHsm(final Element encryptedDataElement, final HSM hsm, + final Collection allowedKeyTransportAlgorithms) { try { validateEncryptedData(encryptedDataElement); @@ -1274,9 +1316,18 @@ public static void decryptUsingHsm(final Element encryptedDataElement, final HSM final NodeList encryptedKeyNodes = ((Element) encryptedDataElement.getParentNode()).getElementsByTagNameNS(Constants.NS_XENC, "EncryptedKey"); final EncryptedKey encryptedKey = xmlCipher.loadEncryptedKey((Element) encryptedKeyNodes.item(0)); + + final String keyTransportAlgorithm = encryptedKey.getEncryptionMethod().getAlgorithm(); + if (keyTransportAlgorithm != null && !isKeyTransportAlgorithmAllowed(keyTransportAlgorithm, allowedKeyTransportAlgorithms)) { + // Do not throw a distinct exception: an algorithm rejection must be indistinguishable + // from any other decryption failure (e.g. a wrong key) to avoid a new side channel. + LOGGER.warn("Failed to decrypt an element using HSM."); + return; + } + final byte[] encryptedBytes = base64decoder(encryptedKey.getCipherData().getCipherValue().getValue()); - final byte[] decryptedKey = hsm.unwrapKey(encryptedKey.getEncryptionMethod().getAlgorithm(), encryptedBytes); + final byte[] decryptedKey = hsm.unwrapKey(keyTransportAlgorithm, encryptedBytes); final SecretKey encryptionKey = new SecretKeySpec(decryptedKey, 0, decryptedKey.length, "AES"); @@ -1288,6 +1339,57 @@ public static void decryptUsingHsm(final Element encryptedDataElement, final HSM } } + /** + * Locates the key transport (key wrapping) algorithm URI declared on the {@code xenc:EncryptedKey} + * associated with the given encrypted data element, looking first inside the element itself + * (e.g. inside its {@code KeyInfo}) and, if not found there, inside its parent node - mirroring + * how {@link #decryptUsingHsm(Element, HSM, Collection)} locates the {@code EncryptedKey}. + * + * @param encryptedDataElement The encrypted element. + * + * @return the key transport algorithm URI, or {@code null} if it could not be determined. + */ + private static String findKeyTransportAlgorithm(final Element encryptedDataElement) { + try { + NodeList encryptedKeyNodes = encryptedDataElement.getElementsByTagNameNS(Constants.NS_XENC, "EncryptedKey"); + if (encryptedKeyNodes.getLength() == 0 && encryptedDataElement.getParentNode() instanceof Element) { + encryptedKeyNodes = + ((Element) encryptedDataElement.getParentNode()).getElementsByTagNameNS(Constants.NS_XENC, "EncryptedKey"); + } + if (encryptedKeyNodes.getLength() > 0) { + final Element encryptedKeyElem = (Element) encryptedKeyNodes.item(0); + final NodeList encryptionMethodNodes = encryptedKeyElem.getElementsByTagNameNS(Constants.NS_XENC, "EncryptionMethod"); + if (encryptionMethodNodes.getLength() > 0) { + final String alg = ((Element) encryptionMethodNodes.item(0)).getAttribute("Algorithm"); + if (alg != null && !alg.isEmpty()) { + return alg; + } + } + } + } catch (final Exception e) { + // Best effort only; if the algorithm cannot be determined, the caller proceeds permissively. + } + return null; + } + + /** + * Checks whether a key transport (key wrapping) algorithm URI is allowed by an optional allow-list. + * + * @param algorithmUri + * The key transport algorithm URI to check. + * @param allowedAlgorithms + * The set of allowed key transport algorithm URIs. If {@code null} or empty, every + * algorithm is considered allowed (permissive default). + * + * @return {@code true} if the algorithm is allowed, {@code false} otherwise. + */ + public static boolean isKeyTransportAlgorithmAllowed(final String algorithmUri, final Collection allowedAlgorithms) { + if (allowedAlgorithms == null || allowedAlgorithms.isEmpty()) { + return true; + } + return algorithmUri != null && allowedAlgorithms.contains(algorithmUri); + } + /** * Validates the encrypted data and checks whether it contains a retrieval * method to obtain the encrypted key or not. @@ -1668,6 +1770,30 @@ public static SamlResponseStatus getStatus(final String statusXpath, final Docum */ public static String generateNameId(final String value, final String spnq, final String format, final String nq, final X509Certificate cert) { + return generateNameId(value, spnq, format, nq, cert, null); + } + + /** + * Generates a nameID. + * + * @param value + * The value + * @param spnq + * SP Name Qualifier + * @param format + * SP Format + * @param nq + * Name Qualifier + * @param cert + * IdP Public certificate to encrypt the nameID + * @param keyTransportAlgorithm + * The key transport (key wrapping) algorithm URI used to encrypt the symmetric key. + * If {@code null} or empty, {@link Constants#RSA_1_5} is used, preserving the historical default. + * + * @return Xml contained in the document. + */ + public static String generateNameId(final String value, final String spnq, final String format, final String nq, + final X509Certificate cert, final String keyTransportAlgorithm) { String res = null; try { final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); @@ -1697,7 +1823,9 @@ public static String generateNameId(final String value, final String spnq, final xmlCipher.init(XMLCipher.ENCRYPT_MODE, symmetricKey); // cipher for encrypt the symmetric key - final XMLCipher keyCipher = XMLCipher.getInstance(Constants.RSA_1_5); + final String keyTransportAlg = + (keyTransportAlgorithm != null && !keyTransportAlgorithm.isEmpty()) ? keyTransportAlgorithm : Constants.RSA_1_5; + final XMLCipher keyCipher = XMLCipher.getInstance(keyTransportAlg); keyCipher.init(XMLCipher.WRAP_MODE, cert.getPublicKey()); // encrypt the symmetric key diff --git a/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java b/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java index a6ecaa6..76e2d18 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java @@ -16,8 +16,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -28,11 +30,13 @@ import javax.xml.xpath.XPathExpressionException; import org.codelibs.saml2.core.authn.SamlResponse; +import org.codelibs.saml2.core.exception.SAMLException; import org.codelibs.saml2.core.exception.SAMLSevereException; import org.codelibs.saml2.core.exception.SettingsException; 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.InMemoryReplayCache; import org.codelibs.saml2.core.settings.Saml2Settings; import org.codelibs.saml2.core.settings.SettingsBuilder; import org.codelibs.saml2.core.util.Constants; @@ -166,6 +170,33 @@ public void testEncryptedAssertionNokey() throws IOException, SAMLSevereExceptio new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); } + /** + * Tests the constructor of SamlResponse + * Case: EncryptedAssertion whose decryption fails because the configured SP + * private key does not match the key the assertion was encrypted with. + * Util.decryptElement() swallows the underlying decryption exception, so + * decryptAssertion() must detect the missing decrypted saml:Assertion node + * and raise a typed ValidationException(MISSING_ENCRYPTED_ELEMENT) instead + * of letting a NullPointerException escape the constructor. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#decryptAssertion + */ + @Test + public void testEncryptedAssertionWrongKeyThrowsValidationException() throws Exception { + // config.different.properties carries a SP private key that does NOT match + // the key used to encrypt data/responses/valid_encrypted_assertion.xml.base64 + // (see UtilsTest#testDecryptElementAssertionWrongKey which uses the same key + // and proves decryption of this exact fixture fails). + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.different.properties").build(); + String samlResponseEncoded = Util.getFileAsString("data/responses/valid_encrypted_assertion.xml.base64"); + + expectedEx.expect(ValidationException.class); + expectedEx.expectMessage("No /samlp:Response/saml:EncryptedAssertion/saml:Assertion element found"); + new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + } + @Test public void testTextWithCommentAttack() throws Exception { Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); @@ -503,6 +534,74 @@ public void testGetNameIdData() throws Exception { assertTrue(samlResponse.getNameIdData().isEmpty()); } + /** + * Tests the getNameIdData method of SamlResponse with an allow-list of key transport algorithms. + *

+ * Case: by default (no allow-list configured) decryption of a RSA_1_5-encrypted NameID is + * unaffected, matching the historical permissive behavior. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#getNameIdData + */ + @Test + public void testGetNameIdDataDefaultAllowsAllKeyTransportAlgorithms() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + assertNull(settings.getAllowedKeyTransportAlgorithms()); + + String samlResponseEncoded = Util.getFileAsString("data/responses/response_encrypted_nameid.xml.base64"); + SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + String nameIdDataStr = samlResponse.getNameIdData().toString(); + assertThat(nameIdDataStr, containsString("Value=2de11defd199f8d5bb63f9b7deb265ba5c675c10")); + } + + /** + * Tests the getNameIdData method of SamlResponse with an allow-list of key transport algorithms. + *

+ * Case: the fixture is encrypted with RSA_1_5, but the allow-list only contains RSA_OAEP_MGF1P, so + * the NameID decryption must be rejected - surfacing as the same generic failure as any other + * decryption problem (e.g. a wrong key), not a new distinct exception type. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#getNameIdData + */ + @Test + public void testGetNameIdDataRejectsDisallowedKeyTransportAlgorithm() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setAllowedKeyTransportAlgorithms(new HashSet<>(Arrays.asList(Constants.RSA_OAEP_MGF1P))); + + String samlResponseEncoded = Util.getFileAsString("data/responses/response_encrypted_nameid.xml.base64"); + SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + + expectedEx.expect(SAMLException.class); + expectedEx.expectMessage("Not able to decrypt the EncryptedID and get a NameID"); + samlResponse.getNameIdData(); + } + + /** + * Tests the decryptAssertion method of SamlResponse with an allow-list of key transport algorithms. + *

+ * Case: the fixture is encrypted with RSA_1_5, but the allow-list only contains RSA_OAEP_MGF1P, so + * decryption of the EncryptedAssertion must be rejected the same way a wrong key would be - a + * generic ValidationException(MISSING_ENCRYPTED_ELEMENT), not a new distinct exception type. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#decryptAssertion + */ + @Test + public void testDecryptAssertionRejectsDisallowedKeyTransportAlgorithm() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setAllowedKeyTransportAlgorithms(new HashSet<>(Arrays.asList(Constants.RSA_OAEP_MGF1P))); + + String samlResponseEncoded = Util.getFileAsString("data/responses/valid_encrypted_assertion.xml.base64"); + + expectedEx.expect(ValidationException.class); + expectedEx.expectMessage("No /samlp:Response/saml:EncryptedAssertion/saml:Assertion element found"); + new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + } + /** * Tests the getIssueInstant method of SamlResponse * @@ -1540,6 +1639,36 @@ public void testValidateTimestamps() throws ValidationException, IOException, XP assertTrue(samlResponse.validateTimestamps()); } + /** + * Tests the validateTimestamps method of SamlResponse + * Case: Encrypted assertion whose (decrypted) Conditions element has expired. + * The raw pre-decryption document has no Conditions element at all (it is inside + * the ciphertext, see saml:Conditions NotOnOrAfter="2055-06-07T20:17:08Z" in + * data/responses/valid_encrypted_assertion.xml.base64), so validateTimestamps() + * must inspect getSAMLResponseDocument() (the decrypted document) instead of the + * raw samlResponseDocument. Moving "now" past that NotOnOrAfter proves the + * encrypted Conditions are actually being checked: before the fix this method + * always returned true regardless of the clock, because the raw document has no + * Conditions element to find. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#validateTimestamps + */ + @Test + public void testValidateTimestampsEncryptedAssertionExpired() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + String samlResponseEncoded = Util.getFileAsString("data/responses/valid_encrypted_assertion.xml.base64"); + SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + + // Move "now" past the decrypted assertion's Conditions/@NotOnOrAfter (2055-06-07T20:17:08Z). + DateTimeTestUtils.setFixedDateTime("2056-01-01T00:00:00Z"); + + expectedEx.expect(ValidationException.class); + expectedEx.expectMessage("Could not validate timestamp: expired. Check system clock."); + samlResponse.validateTimestamps(); + } + /** * Tests the validateTimestamps method of SamlResponse * @@ -2420,6 +2549,54 @@ public void testIsInValidRequestId() throws IOException, SAMLSevereException, XP assertThat(samlResponse.getError(), containsString("The InResponseTo of the Response")); } + /** + * Tests the isValid method of SamlResponse + * Case: a ReplayCache is configured and the same Assertion is validated twice + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#isValid + */ + @Test + public void testIsInValidReplayedAssertionWithCache() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setStrict(true); + settings.setReplayCache(new InMemoryReplayCache()); + String samlResponseEncoded = Util.getFileAsString("data/responses/valid_response.xml.base64"); + + SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertTrue(samlResponse.isValid()); + + SamlResponse replayedResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertFalse(replayedResponse.isValid()); + assertTrue(replayedResponse.getValidationException() instanceof ValidationException); + assertEquals(ValidationException.ASSERTION_REPLAYED, + ((ValidationException) replayedResponse.getValidationException()).getErrorCode()); + } + + /** + * Tests the isValid method of SamlResponse + * Case: no ReplayCache is configured (default) - validating the same Assertion + * multiple times must keep succeeding, proving default behavior is unchanged. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.authn.SamlResponse#isValid + */ + @Test + public void testIsValidReplayedAssertionWithoutCacheIsUnaffected() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setStrict(true); + assertNull(settings.getReplayCache()); + String samlResponseEncoded = Util.getFileAsString("data/responses/valid_response.xml.base64"); + + SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertTrue(samlResponse.isValid()); + + SamlResponse sameResponseAgain = new SamlResponse(settings, newHttpRequest(samlResponseEncoded)); + assertTrue(sameResponseAgain.isValid()); + } + @Test public void testUnexpectedRequestId() throws Exception { Saml2Settings acceptingUnexpectedInResponseTo = new SettingsBuilder().fromFile("config/config.my.properties").build(); diff --git a/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutRequestTest.java b/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutRequestTest.java index 677b7ee..4dc96f7 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutRequestTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutRequestTest.java @@ -17,18 +17,22 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import javax.xml.xpath.XPathExpressionException; import org.apache.xml.security.exceptions.XMLSecurityException; +import org.codelibs.saml2.core.exception.SAMLException; import org.codelibs.saml2.core.exception.SAMLSevereException; import org.codelibs.saml2.core.exception.SettingsException; import org.codelibs.saml2.core.exception.ValidationException; import org.codelibs.saml2.core.http.HttpRequest; import org.codelibs.saml2.core.logout.LogoutRequest; import org.codelibs.saml2.core.logout.LogoutRequestParams; +import org.codelibs.saml2.core.replay.InMemoryReplayCache; import org.codelibs.saml2.core.settings.Saml2Settings; import org.codelibs.saml2.core.settings.SettingsBuilder; import org.codelibs.saml2.core.test.NaiveUrlEncoder; @@ -343,6 +347,36 @@ public void testGetNameIdData() throws Exception { assertThat(nameIdDataStr, containsString("SPNameQualifier=" + settings.getSpEntityId())); } + /** + * Tests the getNameIdData method of LogoutRequest with an allow-list of key transport algorithms. + *

+ * Case: the fixture is encrypted with RSA_1_5. An allow-list containing RSA_1_5 still decrypts + * successfully (equivalent to the permissive default), while an OAEP-only allow-list rejects it + * the same way a wrong private key would - a generic "Not able to decrypt" failure, not a new + * distinct exception type. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.logout.LogoutRequest#getNameIdData + */ + @Test + public void testGetNameIdDataWithAllowedKeyTransportAlgorithms() throws Exception { + String keyString = Util.getFileAsString("data/customPath/certs/sp.pem"); + PrivateKey key = Util.loadPrivateKey(keyString); + String logoutRequestStr = Util.getFileAsString("data/logout_requests/logout_request_encrypted_nameid.xml"); + + // decryptElement mutates the DOM in place, so each attempt needs its own fresh Document. + Set allowedKeyTransportAlgorithms = new HashSet<>(Arrays.asList(Constants.RSA_1_5)); + String nameIdDataStr = + LogoutRequest.getNameIdData(Util.loadXML(logoutRequestStr), key, false, allowedKeyTransportAlgorithms).toString(); + assertThat(nameIdDataStr, containsString("Value=ONELOGIN_9c86c4542ab9d6fce07f2f7fd335287b9b3cdf69")); + + Set oaepOnly = new HashSet<>(Arrays.asList(Constants.RSA_OAEP_MGF1P)); + expectedEx.expect(SAMLException.class); + expectedEx.expectMessage("Not able to decrypt the EncryptedID and get a NameID"); + LogoutRequest.getNameIdData(Util.loadXML(logoutRequestStr), key, false, oaepOnly); + } + /** * Tests the getNameIdData method of LogoutRequest *

@@ -977,6 +1011,76 @@ public void testIsInValidSign() throws Exception { assertEquals("In order to validate the sign on the Logout Request, the x509cert of the IdP is required", logoutRequest.getError()); } + /** + * Tests the isValid method of LogoutRequest + * Case: a ReplayCache is configured and the same LogoutRequest is validated twice + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.logout.LogoutRequest#isValid + */ + @Test + public void testIsInValidReplayedWithCache() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setStrict(true); + settings.setWantMessagesSigned(true); + settings.setReplayCache(new InMemoryReplayCache()); + + final String requestURL = "https://pitbulk.no-ip.org/newonelogin/demo1/index.php?sls"; + String samlRequestEncoded = + "lVLBitswEP0Vo7tjWbJkSyReFkIhsN1tm6WHvQTZHmdFbUmVZLqfXzlpIS10oZdhGM17b96MtkHNk5MP9myX+AW+LxBi9jZPJsjLyw4t3kirgg7SqBmCjL083n98kGSDpfM22t5O6AbyPkKFAD5qa1B22O/QSWA+EFWPjCtaM6gBugrXHCo6Ut6UgvTV2DSkBoKyr+BDQu5QIkrwEBY4mBCViamEyyrHNCf4ueSScMnIC8r2yY02Kl5QrzG6IIvC6dgt07eNsbl2G+vPhYEf1sBkz9oUA8y2LLQZ4G3jXt1dmALKHm18Mk/+fozgk5YQNMciJ+UzKWV11Wq3q3l5mcq3/9YKenYTrL3FGkihB1fMENWgoloVt8Ut0ZX1Me3xsM+On9bk86ImPep1kv+xdKuBsg/Wzyq+f6u1ood8vLTK6JUJGkxE7WnsSDcQRirOKMc97TtWCgqU1ZyJBvM+RZbSrv/l5mrg6sbJI4T1kId1ye0JhoaQgYg+XT1dnilMSZO4uko1jPSYVF0luqQjrmR/4X8X//jC7U8="; + String relayState = "_1037fbc88ec82ce8e770b2bed1119747bb812a07e6"; + String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; + String signature = + "j/qDRTzgQw3cMDkkSkBOShqxi3t9qJxYnrADqwAECnJ3Y+iYgT33C0l/Vy3+ooQkFRyObYJqg9o7iIcMdgV6CXxpa6itVIUAI2VJewsMjzvJ4OdpePeSx7+/umVPKCfMvffsELlqo/UgxsyRZh8NMLej0ojCB7bUfIMKsiU7e0c="; + + HttpRequest httpRequest = new HttpRequest(requestURL, (String) null).addParameter("SAMLRequest", samlRequestEncoded) + .addParameter("RelayState", relayState).addParameter("SigAlg", sigAlg).addParameter("Signature", signature); + + LogoutRequest logoutRequest = new LogoutRequest(settings, httpRequest); + assertTrue(logoutRequest.isValid()); + + LogoutRequest replayedRequest = new LogoutRequest(settings, httpRequest); + assertFalse(replayedRequest.isValid()); + assertTrue(replayedRequest.getValidationException() instanceof ValidationException); + assertEquals(ValidationException.MESSAGE_REPLAYED, ((ValidationException) replayedRequest.getValidationException()).getErrorCode()); + } + + /** + * Tests the isValid method of LogoutRequest + * Case: no ReplayCache is configured (default) - validating the same + * LogoutRequest multiple times must keep succeeding, proving default + * behavior is unchanged. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.logout.LogoutRequest#isValid + */ + @Test + public void testIsValidReplayedWithoutCacheIsUnaffected() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build(); + settings.setStrict(true); + settings.setWantMessagesSigned(true); + assertNull(settings.getReplayCache()); + + final String requestURL = "https://pitbulk.no-ip.org/newonelogin/demo1/index.php?sls"; + String samlRequestEncoded = + "lVLBitswEP0Vo7tjWbJkSyReFkIhsN1tm6WHvQTZHmdFbUmVZLqfXzlpIS10oZdhGM17b96MtkHNk5MP9myX+AW+LxBi9jZPJsjLyw4t3kirgg7SqBmCjL083n98kGSDpfM22t5O6AbyPkKFAD5qa1B22O/QSWA+EFWPjCtaM6gBugrXHCo6Ut6UgvTV2DSkBoKyr+BDQu5QIkrwEBY4mBCViamEyyrHNCf4ueSScMnIC8r2yY02Kl5QrzG6IIvC6dgt07eNsbl2G+vPhYEf1sBkz9oUA8y2LLQZ4G3jXt1dmALKHm18Mk/+fozgk5YQNMciJ+UzKWV11Wq3q3l5mcq3/9YKenYTrL3FGkihB1fMENWgoloVt8Ut0ZX1Me3xsM+On9bk86ImPep1kv+xdKuBsg/Wzyq+f6u1ood8vLTK6JUJGkxE7WnsSDcQRirOKMc97TtWCgqU1ZyJBvM+RZbSrv/l5mrg6sbJI4T1kId1ye0JhoaQgYg+XT1dnilMSZO4uko1jPSYVF0luqQjrmR/4X8X//jC7U8="; + String relayState = "_1037fbc88ec82ce8e770b2bed1119747bb812a07e6"; + String sigAlg = "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; + String signature = + "j/qDRTzgQw3cMDkkSkBOShqxi3t9qJxYnrADqwAECnJ3Y+iYgT33C0l/Vy3+ooQkFRyObYJqg9o7iIcMdgV6CXxpa6itVIUAI2VJewsMjzvJ4OdpePeSx7+/umVPKCfMvffsELlqo/UgxsyRZh8NMLej0ojCB7bUfIMKsiU7e0c="; + + HttpRequest httpRequest = new HttpRequest(requestURL, (String) null).addParameter("SAMLRequest", samlRequestEncoded) + .addParameter("RelayState", relayState).addParameter("SigAlg", sigAlg).addParameter("Signature", signature); + + LogoutRequest logoutRequest = new LogoutRequest(settings, httpRequest); + assertTrue(logoutRequest.isValid()); + + LogoutRequest sameRequestAgain = new LogoutRequest(settings, httpRequest); + assertTrue(sameRequestAgain.isValid()); + } + /** * Tests the isValid method of LogoutRequest * Case: Signed with deprecated method and flag enabled diff --git a/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutResponseTest.java b/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutResponseTest.java index 1dfdcfc..89806f8 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutResponseTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/logout/LogoutResponseTest.java @@ -24,6 +24,7 @@ import org.codelibs.saml2.core.logout.LogoutResponse; import org.codelibs.saml2.core.logout.LogoutResponseParams; import org.codelibs.saml2.core.model.SamlResponseStatus; +import org.codelibs.saml2.core.replay.InMemoryReplayCache; import org.codelibs.saml2.core.settings.Saml2Settings; import org.codelibs.saml2.core.settings.SettingsBuilder; import org.codelibs.saml2.core.test.NaiveUrlEncoder; @@ -700,6 +701,59 @@ public void testIsValid() throws URISyntaxException, IOException, XMLSecurityExc assertTrue(logoutResponse.isValid()); } + /** + * Tests the isValid method of LogoutResponse + * Case: a ReplayCache is configured and the same LogoutResponse is validated twice + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.logout.LogoutResponse#isValid + */ + @Test + public void testIsInValidReplayedWithCache() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + settings.setStrict(true); + settings.setReplayCache(new InMemoryReplayCache()); + String samlResponseEncoded = Util.getFileAsString("data/logout_responses/logout_response_deflated.xml.base64"); + String requestURL = "http://stuff.com/endpoints/endpoints/sls.php"; + HttpRequest httpRequest = newHttpRequest(requestURL, samlResponseEncoded); + + LogoutResponse logoutResponse = new LogoutResponse(settings, httpRequest); + assertTrue(logoutResponse.isValid()); + + LogoutResponse replayedResponse = new LogoutResponse(settings, httpRequest); + assertFalse(replayedResponse.isValid()); + assertTrue(replayedResponse.getValidationException() instanceof ValidationException); + assertEquals(ValidationException.MESSAGE_REPLAYED, + ((ValidationException) replayedResponse.getValidationException()).getErrorCode()); + } + + /** + * Tests the isValid method of LogoutResponse + * Case: no ReplayCache is configured (default) - validating the same + * LogoutResponse multiple times must keep succeeding, proving default + * behavior is unchanged. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.logout.LogoutResponse#isValid + */ + @Test + public void testIsValidReplayedWithoutCacheIsUnaffected() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + settings.setStrict(true); + assertNull(settings.getReplayCache()); + String samlResponseEncoded = Util.getFileAsString("data/logout_responses/logout_response_deflated.xml.base64"); + String requestURL = "http://stuff.com/endpoints/endpoints/sls.php"; + HttpRequest httpRequest = newHttpRequest(requestURL, samlResponseEncoded); + + LogoutResponse logoutResponse = new LogoutResponse(settings, httpRequest); + assertTrue(logoutResponse.isValid()); + + LogoutResponse sameResponseAgain = new LogoutResponse(settings, httpRequest); + assertTrue(sameResponseAgain.isValid()); + } + @Test public void testIsInValidSign_defaultUrlEncode() throws Exception { Saml2Settings settings = new SettingsBuilder().fromFile("config/config.knownIdpPrivateKey.properties").build(); diff --git a/core/src/test/java/org/codelibs/saml2/core/test/replay/InMemoryReplayCacheTest.java b/core/src/test/java/org/codelibs/saml2/core/test/replay/InMemoryReplayCacheTest.java new file mode 100644 index 0000000..d26af82 --- /dev/null +++ b/core/src/test/java/org/codelibs/saml2/core/test/replay/InMemoryReplayCacheTest.java @@ -0,0 +1,112 @@ +package org.codelibs.saml2.core.test.replay; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.time.Instant; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.codelibs.saml2.core.replay.InMemoryReplayCache; +import org.codelibs.saml2.core.replay.ReplayCache; +import org.junit.Test; + +/** + * Tests for {@link InMemoryReplayCache}. + */ +public class InMemoryReplayCacheTest { + + /** + * Tests the registerAndCheck method of InMemoryReplayCache + * Case: first registration of an id is not a replay + * + * @see org.codelibs.saml2.core.replay.InMemoryReplayCache#registerAndCheck + */ + @Test + public void testFirstRegistrationIsNotReplay() { + final ReplayCache cache = new InMemoryReplayCache(); + final boolean replay = cache.registerAndCheck("id-1", Instant.now().plusSeconds(300)); + assertFalse(replay); + } + + /** + * Tests the registerAndCheck method of InMemoryReplayCache + * Case: second registration of the same, still-valid id is a replay + * + * @see org.codelibs.saml2.core.replay.InMemoryReplayCache#registerAndCheck + */ + @Test + public void testSecondRegistrationIsReplay() { + final ReplayCache cache = new InMemoryReplayCache(); + final Instant expiresAt = Instant.now().plusSeconds(300); + + assertFalse(cache.registerAndCheck("id-2", expiresAt)); + assertTrue(cache.registerAndCheck("id-2", expiresAt)); + } + + /** + * Tests the registerAndCheck method of InMemoryReplayCache + * Case: an id whose previous entry already expired is treated as fresh + * + * @see org.codelibs.saml2.core.replay.InMemoryReplayCache#registerAndCheck + */ + @Test + public void testExpiredEntryIsTreatedAsFresh() { + final ReplayCache cache = new InMemoryReplayCache(); + final Instant pastExpiry = Instant.now().minusSeconds(10); + + assertFalse(cache.registerAndCheck("id-3", pastExpiry)); + // Previous entry already expired, so this is treated as a fresh registration, not a replay. + assertFalse(cache.registerAndCheck("id-3", Instant.now().plusSeconds(300))); + // Now that it has been freshly re-registered with a future expiry, a further call is a replay. + assertTrue(cache.registerAndCheck("id-3", Instant.now().plusSeconds(300))); + } + + /** + * Tests the registerAndCheck method of InMemoryReplayCache + * Case: concurrent registration of the same id only allows a single caller to see "not a replay" + * + * @throws InterruptedException + * + * @see org.codelibs.saml2.core.replay.InMemoryReplayCache#registerAndCheck + */ + @Test + public void testConcurrentRegistrationSmoke() throws InterruptedException { + final ReplayCache cache = new InMemoryReplayCache(); + final int threadCount = 16; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch ready = new CountDownLatch(threadCount); + final CountDownLatch start = new CountDownLatch(1); + final AtomicInteger firstUseCount = new AtomicInteger(); + final Instant expiresAt = Instant.now().plusSeconds(300); + + try { + for (int i = 0; i < threadCount; i++) { + executor.submit(() -> { + ready.countDown(); + try { + start.await(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + if (!cache.registerAndCheck("concurrent-id", expiresAt)) { + firstUseCount.incrementAndGet(); + } + }); + } + ready.await(); + start.countDown(); + executor.shutdown(); + assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } finally { + executor.shutdownNow(); + } + + assertEquals(1, firstUseCount.get()); + } +} diff --git a/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java b/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java index 3a60c09..a2231f3 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java @@ -1,12 +1,16 @@ package org.codelibs.saml2.core.test.settings; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.net.URL; +import java.security.cert.X509Certificate; import java.util.Map; +import org.codelibs.saml2.core.exception.SAMLSignatureException; import org.codelibs.saml2.core.settings.IdPMetadataParser; import org.codelibs.saml2.core.settings.Saml2Settings; import org.codelibs.saml2.core.settings.SettingsBuilder; @@ -182,6 +186,66 @@ public void testParseRemoteXML() throws Exception { assertEquals("urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress", idpInfo.get(SettingsBuilder.SP_NAMEIDFORMAT_PROPERTY_KEY)); } + /** + * Tests the parseXML(Document, String, String, String, String, X509Certificate) overload. + * Case Valid: the metadata is signed with the certificate passed in as the trusted signing cert. + * + * @see org.codelibs.saml2.core.core.settings.IdPMetadataParser#parseXML(Document, String, String, String, String, X509Certificate) + */ + @Test + public void testParseXMLWithTrustedSigningCertValid() throws Exception { + X509Certificate correctCert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + Document signedMetadataDocument = Util + .parseXML(new InputSource(getClass().getClassLoader().getResourceAsStream("data/metadata/signed_metadata_settings1.xml"))); + + Map idpInfo = IdPMetadataParser.parseXML(signedMetadataDocument, null, null, Constants.BINDING_HTTP_REDIRECT, + Constants.BINDING_HTTP_REDIRECT, correctCert); + assertNotNull(idpInfo); + } + + /** + * Tests the parseXML(Document, String, String, String, String, X509Certificate) overload. + * Case Invalid: the metadata is signed with a certificate different from the trusted signing cert + * passed in, so a SAMLSignatureException must be thrown. + * + * @see org.codelibs.saml2.core.core.settings.IdPMetadataParser#parseXML(Document, String, String, String, String, X509Certificate) + */ + @Test + public void testParseXMLWithTrustedSigningCertInvalid() throws Exception { + X509Certificate wrongCert = Util.loadCert(Util.getFileAsString("certs/certificate1")); + Document signedMetadataDocument = Util + .parseXML(new InputSource(getClass().getClassLoader().getResourceAsStream("data/metadata/signed_metadata_settings1.xml"))); + + try { + IdPMetadataParser.parseXML(signedMetadataDocument, null, null, Constants.BINDING_HTTP_REDIRECT, Constants.BINDING_HTTP_REDIRECT, + wrongCert); + fail("Expected a SAMLSignatureException to be thrown"); + } catch (SAMLSignatureException e) { + // expected + } + } + + /** + * Tests the parseFileXML(String, X509Certificate) shorthand overload. + * Case Valid/Invalid: valid with the correct trusted cert, throws with a wrong cert. + * + * @see org.codelibs.saml2.core.core.settings.IdPMetadataParser#parseFileXML(String, X509Certificate) + */ + @Test + public void testParseFileXMLWithTrustedSigningCert() throws Exception { + X509Certificate correctCert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + Map idpInfo = IdPMetadataParser.parseFileXML("data/metadata/signed_metadata_settings1.xml", correctCert); + assertNotNull(idpInfo); + + X509Certificate wrongCert = Util.loadCert(Util.getFileAsString("certs/certificate1")); + try { + IdPMetadataParser.parseFileXML("data/metadata/signed_metadata_settings1.xml", wrongCert); + fail("Expected a SAMLSignatureException to be thrown"); + } catch (SAMLSignatureException e) { + // expected + } + } + @Test public void testParseMultiCerts() throws Exception { Map idpInfo = IdPMetadataParser.parseFileXML("data/metadata/idp/idp_metadata_multi_certs.xml"); diff --git a/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java b/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java index db007cb..77179b5 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java @@ -9,8 +9,11 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.security.cert.X509Certificate; import java.util.Calendar; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.codelibs.saml2.core.exception.SAMLSevereException; import org.codelibs.saml2.core.model.hsm.AzureKeyVault; @@ -505,6 +508,58 @@ public void testValidateMetadataExpired() throws Exception { assertTrue(errors.contains("expired_xml")); } + /** + * Tests the validateMetadata(String, X509Certificate) overload of the Saml2Settings + * Case Valid: metadata signed with the certificate that is passed in for validation + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#validateMetadata(String, X509Certificate) + */ + @Test + public void testValidateMetadataSignatureValid() throws Exception { + String certString = Util.getFileAsString("data/customPath/certs/sp.crt"); + X509Certificate cert = Util.loadCert(certString); + String signedMetadataStr = Util.getFileAsString("data/metadata/signed_metadata_settings1.xml"); + + List errors = Saml2Settings.validateMetadata(signedMetadataStr, cert); + assertFalse(errors.contains("metadata_signature_invalid")); + } + + /** + * Tests the validateMetadata(String, X509Certificate) overload of the Saml2Settings + * Case Invalid: metadata signed with a different certificate than the one passed in for validation + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#validateMetadata(String, X509Certificate) + */ + @Test + public void testValidateMetadataSignatureInvalid() throws Exception { + X509Certificate wrongCert = Util.loadCert(Util.getFileAsString("certs/certificate1")); + String signedMetadataStr = Util.getFileAsString("data/metadata/signed_metadata_settings1.xml"); + + List errors = Saml2Settings.validateMetadata(signedMetadataStr, wrongCert); + assertTrue(errors.contains("metadata_signature_invalid")); + } + + /** + * Tests that the single-argument validateMetadata(String) overload never performs signature + * validation and remains unchanged when fed unsigned metadata. + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#validateMetadata(String) + */ + @Test + public void testValidateMetadataValidUnsignedSingleArgUnchanged() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.all.properties").build(); + String metadataStr = settings.getSPMetadata(); + + List errors = Saml2Settings.validateMetadata(metadataStr); + assertTrue(errors.isEmpty()); + } + /** * Tests that default signature algorithm is SHA-256 instead of deprecated SHA-1 * @@ -591,4 +646,43 @@ public void testClockDriftConfiguration() throws Exception { settings.setClockDrift(180L); assertEquals(180L, settings.getClockDrift()); } + + /** + * Tests the getSecurityWarnings method of the Saml2Settings + * Case: weak security posture, all warnings expected + * + * @throws Exception + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getSecurityWarnings + */ + @Test + public void testGetSecurityWarningsWeakPosture() throws Exception { + Map samlData = new HashMap<>(); + samlData.put(SettingsBuilder.STRICT_PROPERTY_KEY, false); + samlData.put(SettingsBuilder.CERTFINGERPRINT_PROPERTY_KEY, "afe71c28ef740bc87425be13a2263d37971da1f9"); + Saml2Settings settings = new SettingsBuilder().fromValues(samlData).build(); + + List warnings = settings.getSecurityWarnings(); + assertThat(warnings, hasItem("strict_mode_disabled")); + assertThat(warnings, hasItem("weak_fingerprint_algorithm")); + assertThat(warnings, hasItem("deprecated_signature_algorithms_not_rejected")); + assertThat(warnings, hasItem("assertions_and_messages_not_required_signed")); + } + + /** + * Tests the getSecurityWarnings method of the Saml2Settings + * Case: hardened security posture, no warnings expected + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getSecurityWarnings + */ + @Test + public void testGetSecurityWarningsHardenedPosture() { + Saml2Settings settings = new Saml2Settings(); + settings.setStrict(true); + settings.setRejectDeprecatedAlg(true); + settings.setWantAssertionsSigned(true); + + List warnings = settings.getSecurityWarnings(); + assertTrue(warnings.isEmpty()); + } } diff --git a/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java b/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java index b86050b..ac7d2b4 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java @@ -30,8 +30,12 @@ import java.time.Instant; import java.time.format.DateTimeParseException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -1476,6 +1480,83 @@ public void testDecryptElementNoKeyinfo() throws URISyntaxException, IOException assertNotEquals("saml:NameID", EncryptedIdNodes.item(0).getFirstChild().getNodeName()); } + /** + * Tests the decryptElement method with an allow-list of key transport algorithms. + * Case: the fixture is encrypted with RSA_1_5 and the allow-list contains RSA_1_5, so + * decryption proceeds as if no allow-list had been provided. + * + * @throws IOException + * @throws URISyntaxException + * @throws GeneralSecurityException + * @throws XPathExpressionException + * + * @see org.codelibs.saml2.core.core.util.Util#decryptElement + */ + @Test + public void testDecryptElementNameIdWithAllowedKeyTransportAlgorithm() + throws URISyntaxException, IOException, GeneralSecurityException, XPathExpressionException { + String keyString = Util.getFileAsString("data/customPath/certs/sp.pem"); + PrivateKey key = Util.loadPrivateKey(keyString); + + String responseNameIdEnc = Util.base64decodedInflated(Util.getFileAsString("data/responses/response_encrypted_nameid.xml.base64")); + Document responseNameIdEncDoc = Util.loadXML(responseNameIdEnc); + NodeList EncryptedNameIdNodes = Util.query(responseNameIdEncDoc, ".//saml:EncryptedID"); + NodeList EncryptedDataNodes = Util.query(responseNameIdEncDoc, "./xenc:EncryptedData", EncryptedNameIdNodes.item(0)); + Element encryptedData = (Element) EncryptedDataNodes.item(0); + assertEquals("xenc:EncryptedData", encryptedData.getNodeName()); + + Set allowedKeyTransportAlgorithms = new HashSet<>(Arrays.asList(Constants.RSA_1_5)); + Util.decryptElement(encryptedData, key, allowedKeyTransportAlgorithms); + assertEquals("saml:NameID", EncryptedNameIdNodes.item(0).getFirstChild().getNodeName()); + } + + /** + * Tests the decryptElement method with an allow-list of key transport algorithms. + * Case: the fixture is encrypted with RSA_1_5 but the allow-list only contains + * RSA_OAEP_MGF1P, so decryption must be rejected the same way a wrong key would be: + * no exception is thrown, the element is simply left encrypted. + * + * @throws IOException + * @throws URISyntaxException + * @throws GeneralSecurityException + * @throws XPathExpressionException + * + * @see org.codelibs.saml2.core.core.util.Util#decryptElement + */ + @Test + public void testDecryptElementNameIdWithDisallowedKeyTransportAlgorithm() + throws URISyntaxException, IOException, GeneralSecurityException, XPathExpressionException { + String keyString = Util.getFileAsString("data/customPath/certs/sp.pem"); + PrivateKey key = Util.loadPrivateKey(keyString); + + String responseNameIdEnc = Util.base64decodedInflated(Util.getFileAsString("data/responses/response_encrypted_nameid.xml.base64")); + Document responseNameIdEncDoc = Util.loadXML(responseNameIdEnc); + NodeList EncryptedNameIdNodes = Util.query(responseNameIdEncDoc, ".//saml:EncryptedID"); + NodeList EncryptedDataNodes = Util.query(responseNameIdEncDoc, "./xenc:EncryptedData", EncryptedNameIdNodes.item(0)); + Element encryptedData = (Element) EncryptedDataNodes.item(0); + assertEquals("xenc:EncryptedData", encryptedData.getNodeName()); + + Set allowedKeyTransportAlgorithms = new HashSet<>(Arrays.asList(Constants.RSA_OAEP_MGF1P)); + Util.decryptElement(encryptedData, key, allowedKeyTransportAlgorithms); + assertNotEquals("saml:NameID", EncryptedNameIdNodes.item(0).getFirstChild().getNodeName()); + assertEquals("xenc:EncryptedData", EncryptedNameIdNodes.item(0).getFirstChild().getNodeName()); + } + + /** + * Tests the isKeyTransportAlgorithmAllowed method + * + * @see org.codelibs.saml2.core.core.util.Util#isKeyTransportAlgorithmAllowed + */ + @Test + public void testIsKeyTransportAlgorithmAllowed() { + assertTrue(Util.isKeyTransportAlgorithmAllowed(Constants.RSA_1_5, null)); + assertTrue(Util.isKeyTransportAlgorithmAllowed(Constants.RSA_1_5, Collections.emptySet())); + + Set allowedKeyTransportAlgorithms = new HashSet<>(Arrays.asList(Constants.RSA_OAEP_MGF1P)); + assertTrue(Util.isKeyTransportAlgorithmAllowed(Constants.RSA_OAEP_MGF1P, allowedKeyTransportAlgorithms)); + assertFalse(Util.isKeyTransportAlgorithmAllowed(Constants.RSA_1_5, allowedKeyTransportAlgorithms)); + } + /** * Tests the copyDocument method * @@ -1937,12 +2018,47 @@ public void testGenerateNameId() throws URISyntaxException, IOException, Certifi assertThat(nameIdEnc, containsString("http://www.w3.org/2001/04/xmlenc#rsa-1_5")); } + /** + * Tests the generateNameId method with an explicit key transport algorithm. + * Case: default (5-arg overload) still uses RSA_1_5; explicit RSA_OAEP_MGF1P is honored. + * + * @throws IOException + * @throws URISyntaxException + * @throws CertificateException + * + * @see org.codelibs.saml2.core.core.util.Util#generateNameId + */ + @Test + public void testGenerateNameIdWithKeyTransportAlgorithm() throws URISyntaxException, IOException, CertificateException { + String nameIdValue = "ONELOGIN_ce998811003f4e60f8b07a311dc641621379cfde"; + String entityId = "http://stuff.com/endpoints/metadata.php"; + String nameIDFormat = "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"; + + String certString = Util.getFileAsString("data/customPath/certs/sp.crt"); + X509Certificate cert = Util.loadCert(certString); + + // Existing 5-arg overload keeps the default RSA_1_5 key transport algorithm. + String nameIdDefault = Util.generateNameId(nameIdValue, entityId, nameIDFormat, cert); + assertThat(nameIdDefault, containsString(Constants.RSA_1_5)); + assertThat(nameIdDefault, not(containsString(Constants.RSA_OAEP_MGF1P))); + + // New 6-arg overload with a null algorithm behaves exactly like the default. + String nameIdNullAlg = Util.generateNameId(nameIdValue, entityId, nameIDFormat, null, cert, null); + assertThat(nameIdNullAlg, containsString(Constants.RSA_1_5)); + + // New 6-arg overload honors an explicit key transport algorithm. + String nameIdOaep = Util.generateNameId(nameIdValue, entityId, nameIDFormat, null, cert, Constants.RSA_OAEP_MGF1P); + assertThat(nameIdOaep, containsString("