diff --git a/README.md b/README.md index ef5b9ca..c2da4c0 100755 --- a/README.md +++ b/README.md @@ -182,6 +182,7 @@ In core the SAML code is mostly related to: - **SP Endpoint Hostname/Port**: `localhost:8443` - **Validation Type**: Select `Only Assertion` if Azure is configured to sign assertions only (default). If your Entra application is configured to sign the entire SAML **Response** (not just the assertion), select `Both Response and Assertion` or `Only Response` accordingly. + A response is rejected unless the element(s) selected here are signed by the IdP; any other signature present is verified too. A missing or unrecognized value is treated as `Both Response and Assertion`. - **Public Certificate / Private Key**: Generate a public/private key pair for dotCMS SAML. This key pair is used by the SP (dotCMS) to sign authentication requests and decrypt encrypted assertions. You can generate one using: ```bash openssl req -x509 -newkey rsa:2048 -keyout saml-key.pem -out saml-cert.pem -days 365 -nodes @@ -378,8 +379,11 @@ Once your changes are tested and working, create the release version: - authn.protocol.binding: The binding for the auth request XML, such as Http-Redirect, Http-POST, Http-POST-Raw. Based on this value will use Redirect or Post to do the authentication request from dotCMS to the IDP - logout.service.endpoint.url: this is a callback to be called by the IDP when the logout happens on the IDP and needs to get back to dotCMS, it is usually set to "/dotAdmin/show-logout" - logout.okta.url: okta needs a special url instead of the IDP metadata XML to get logout, this is the url used in that particular case. -- clock.skew: this is the clock skew in seconds, it is used to allow some time difference between the IDP and dotCMS servers, so if the IDP sends a SAML Assertion with a timestamp that is 5 seconds in the past, it will still be valid. -- message.life.time: this is the time in seconds that the SAML message will be valid, after this time it will be considered expired. +- clock.skew: this is the clock skew in milliseconds (default 10000), it is used to allow some time difference between the IDP and dotCMS servers when checking the Response IssueInstant and the assertion Conditions, SubjectConfirmationData and AuthnStatement validity times. +- message.life.time: this is the time in milliseconds (default 20000) that the SAML Response is valid after its IssueInstant, after this time it will be considered expired. +- allow.unsolicited.responses: boolean, false by default. By default dotCMS only accepts a SAML Response that answers an authentication request started from the same browser (InResponseTo). Set it to true to also accept IdP-initiated login (for example an app tile on the IdP portal); such responses still have to pass all the assertion checks. +- authn.request.max.age: seconds (default 900, between 60 and 3600) an authentication request started from dotCMS stays valid while the user signs in at the IdP. +- allow.sha1.signatures: boolean, false by default. IdP signatures must use RSA or ECDSA with SHA-256, SHA-384 or SHA-512, and SHA-256 or stronger digests. Set it to true only for an IdP that still signs with RSA-SHA1 or SHA-1 digests; each such response is accepted with a warning in the log. - auth.sign.request: this is a boolean that indicates if the SAML request should be signed or not, it is usually set to false. - auth.signature.reference.digestmethod.algorithm: this is the algorithm used to sign the SAML request, it is usually set to "http://www.w3.org/2001/04/xmlenc#sha256". - auth.sign.params: this is a boolean that indicates if the SAML request parameters should be signed or not, it is usually set to true. @@ -390,8 +394,8 @@ Once your changes are tested and working, create the release version: - logout.signature.algorithm: this is the algorithm used to sign the SAML logout request, it is usually set to "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256". - location.cleanqueryparams: this is a boolean that indicates if the query parameters should be cleaned from the location URL, it is usually set to true. - logout.protocol.binding: this is the binding for the logout request XML, such as Http-Redirect, Http-POST, Http-Okta. Based on this value will use Redirect or Post to do the logout request from dotCMS to the IDP -- verify.signature.credentials: this is a boolean that indicates if the SAML response signature should be verified or not, it is usually set to true. -- verify.signature.profile: this is the profile used to verify the SAML response signature, it is usually set to "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256". +- verify.signature.credentials: no longer configurable. Signatures are always verified against the IdP credentials; a value of false is ignored and logged as a warning. +- verify.signature.profile: no longer configurable. The SAML signature profile check is always applied; a value of false is ignored and logged as a warning. - idp.metadata.protocol: this is the protocol used to retrieve the IDP metadata, it is usually set to "urn:oasis:names:tc:SAML:2.0:protocol". - use.encrypted.descriptor: When setting the Key Descriptor in the IDP metadata, this property indicates if the Key Descriptor should be encrypted or not, it is usually set to false. - access.filter.values: Is a comma separeted list with exceptional cases to avoid SAML evaluation @@ -415,6 +419,41 @@ Once your changes are tested and working, create the release version: - authn.comparisontype: This is the comparison type of the AuthnContext element in the SAML request, it is usually set to "minimum". This value can be changed to "exact" or "maximum" or "better" depending on the IDP configuration. - isassertion.encrypted: This is a boolean that indicates if the SAML assertion should be encrypted or not, it is usually set to false. If set to true, the SAML assertion will be encrypted using the public key of the SP, which can be useful for some IDPs that require it. +### How a SAML Response is validated + +On the assertion consumer endpoint (`/dotsaml/login/{idpConfigId}`), after decoding, a response is accepted only when all of the following hold (SAML 2.0 Web Browser SSO profile): + +- It contains exactly one assertion, encrypted or not as configured. +- The signatures required by the Validation Type are present and valid, and any other signature present is valid. The signature profile check binds each signature to the element it covers. +- Every signature uses an allowed algorithm: RSA or ECDSA with SHA-256/384/512 and SHA-256/384/512 digests (RSA-SHA1 and SHA-1 digests only with `allow.sha1.signatures`). +- The status is Success and the Response IssueInstant is within `message.life.time`. +- The Response Destination, when present (always, for a signed Response), is this site's assertion consumer URL. +- The Response Issuer (when present) and the Assertion Issuer are the IdP entityID from the IdP metadata. +- The assertion has Conditions whose NotBefore/NotOnOrAfter include the current time, and every AudienceRestriction includes the Service Provider Issuer ID. +- It has a bearer SubjectConfirmation whose SubjectConfirmationData has a NotOnOrAfter in the future, a Recipient equal to the assertion consumer URL, and an InResponseTo equal to the Response's. +- The Response InResponseTo matches an authentication request started from the same browser, or it is absent and `allow.unsolicited.responses` is true. A login started on a host other than the Service Provider Endpoint Hostname (a site alias, or a site using the System Host configuration) is first redirected to `https:///dotsaml/login/` and starts from there, so the browser sends the request cookie back with the response. +- It has an AuthnStatement whose SessionNotOnOrAfter, if present, is in the future. +- The assertion ID has not been used before on any node. Used IDs are recorded in the `dotsaml_assertion_replay` table, which the bundle creates in the dotCMS database on first use, until the confirmation expires. + +### Troubleshooting rejected SAML logins + +Each rejection is logged with the IdP name and the reason. The usual causes: + +| Log message contains | Cause | Fix | +|---|---|---| +| `The SAML Response ... must be signed` / `The SAML Assertion ... must be signed` | The IdP doesn't sign what Validation Type requires. | Turn signing on at the IdP, or select the Validation Type that matches exactly what it signs. | +| `Signature cannot be validated` | The signature doesn't match the IdP certificate in the metadata. | Re-import the IdP metadata (the IdP may have rotated its certificate). | +| `signature algorithm ... is not allowed` / `digest algorithm ... is not allowed` | The IdP signs with SHA-1. | Switch the IdP to SHA-256. As a stopgap, set `allow.sha1.signatures=true`. | +| `Unsolicited SAML Responses (IdP-initiated login) are not accepted` | The login was started from the IdP (for example an app tile). | Start it from dotCMS, or set `allow.unsolicited.responses=true`. | +| `does not match an authentication request started from this browser` | The login took longer than `authn.request.max.age`, the browser didn't send the request cookie (it needs HTTPS), or the response was posted again. | Start the login again from dotCMS. | +| `Destination ... is not this service provider's assertion consumer URL` / `Recipient ...` | The IdP posts to a different URL. | Set the IdP's ACS URL to `https:///dotsaml/login/`, or correct the Endpoint Hostname. | +| `AudienceRestriction does not include this service provider` | The IdP's audience (SP entity ID) differs from the Service Provider Issuer ID. | Make the two match. | +| `Issuer ... is not the configured IdP` | The metadata belongs to another IdP or tenant. | Re-import the right IdP metadata. | +| `expired at` / `not valid before` | Clock difference between the IdP and dotCMS. | Check NTP on both sides, or raise `clock.skew`. | +| `has already been used` | The same response was posted twice (back button, refresh). | Start the login again. | +| `Could not check whether the SAML Assertion ... has already been used` | The replay table couldn't be read or written. | Check database connectivity and that the dotCMS user can create `dotsaml_assertion_replay`. | +| `must contain exactly one ... assertion` | Several assertions, or encryption that doesn't match `isassertion.encrypted`. | Configure the IdP to send one assertion, and set `isassertion.encrypted` to match. | + ### Why SAML evolution is organic, and has been reactive The evolution of our SAML implementation has been organic because it started from a baseline that was designed based on best practices and common SAML flows described in the literature (e.g., handling GET/POST authentication, single logout, assertion consumption endpoints, and metadata generation/parsing). diff --git a/src/main/java/com/dotcms/saml/service/external/SamlConstants.java b/src/main/java/com/dotcms/saml/service/external/SamlConstants.java index add8802..6b75164 100644 --- a/src/main/java/com/dotcms/saml/service/external/SamlConstants.java +++ b/src/main/java/com/dotcms/saml/service/external/SamlConstants.java @@ -18,6 +18,26 @@ public class SamlConstants { public static final String RESPONSE_AND_ASSERTION = "responseandassertion"; public static final String RESPONSE = "response"; public static final String ASSERTION = "assertion"; + + /** + * Optional IdP property. When "true", responses without InResponseTo (IdP-initiated SSO) are accepted. + * By default only responses to an authentication request started from the same browser are accepted. + */ + public static final String ALLOW_UNSOLICITED_RESPONSES = "allow.unsolicited.responses"; + + /** + * Optional IdP property. Seconds an outstanding authentication request stays valid while the user + * authenticates at the IdP. + */ + public static final String AUTHN_REQUEST_MAX_AGE = "authn.request.max.age"; + public static final int AUTHN_REQUEST_MAX_AGE_DEFAULT_VALUE = 900; + + /** + * Optional IdP property. When "true", RSA-SHA1 signatures and SHA-1 digests are accepted (with a warning) + * for IdPs that can not sign with SHA-256 or stronger yet. + */ + public static final String ALLOW_SHA1_SIGNATURES = "allow.sha1.signatures"; + public static final String SAML_USER_ID = "SAMLUserId"; public static final String DEFAULT_LOGIN_PATH = "/dotAdmin"; diff --git a/src/main/java/com/dotcms/saml/service/handler/HttpPOSTAuthenticationHandler.java b/src/main/java/com/dotcms/saml/service/handler/HttpPOSTAuthenticationHandler.java index 79dc0e9..2649a90 100644 --- a/src/main/java/com/dotcms/saml/service/handler/HttpPOSTAuthenticationHandler.java +++ b/src/main/java/com/dotcms/saml/service/handler/HttpPOSTAuthenticationHandler.java @@ -5,6 +5,7 @@ import com.dotcms.saml.service.external.SamlException; import com.dotcms.saml.service.impl.DotHTTPPOSTDeflateEncoder; import com.dotcms.saml.service.internal.SamlCoreService; +import com.dotcms.saml.utils.AuthnRequestStateCookie; import com.dotcms.saml.utils.SamlUtils; import com.dotcms.saml.utils.SignatureUtils; import com.dotmarketing.util.UtilMethods; @@ -53,6 +54,8 @@ public void handle(final HttpServletRequest request, final HttpServletResponse r final MessageContext context = new MessageContext(); // main context final AuthnRequest authnRequest = this.samlCoreService.buildAuthnRequest(request, identityProviderConfiguration, SAMLConstants.SAML2_POST_BINDING_URI); + // lets the SAML Response be bound to this request (InResponseTo) when the IdP posts it back + AuthnRequestStateCookie.remember(request, response, identityProviderConfiguration, authnRequest.getID()); // the client can ask to sign or not the request final boolean needSign = identityProviderConfiguration.containsOptionalProperty("auth.sign.request")? diff --git a/src/main/java/com/dotcms/saml/service/handler/HttpPOSTRawAuthenticationHandler.java b/src/main/java/com/dotcms/saml/service/handler/HttpPOSTRawAuthenticationHandler.java index 1a7d3ed..3653f4f 100644 --- a/src/main/java/com/dotcms/saml/service/handler/HttpPOSTRawAuthenticationHandler.java +++ b/src/main/java/com/dotcms/saml/service/handler/HttpPOSTRawAuthenticationHandler.java @@ -5,6 +5,7 @@ import com.dotcms.saml.service.external.SamlException; import com.dotcms.saml.service.impl.DotHTTPPOSTRawDeflateEncoder; import com.dotcms.saml.service.internal.SamlCoreService; +import com.dotcms.saml.utils.AuthnRequestStateCookie; import com.dotcms.saml.utils.SamlUtils; import com.dotmarketing.util.UtilMethods; import net.shibboleth.utilities.java.support.component.ComponentInitializationException; @@ -47,6 +48,8 @@ public void handle(final HttpServletRequest request, final HttpServletResponse r final MessageContext context = new MessageContext(); // main context final AuthnRequest authnRequest = this.samlCoreService.buildAuthnRequest(request, identityProviderConfiguration, SAMLConstants.SAML2_POST_BINDING_URI); + // lets the SAML Response be bound to this request (InResponseTo) when the IdP posts it back + AuthnRequestStateCookie.remember(request, response, identityProviderConfiguration, authnRequest.getID()); context.setMessage(authnRequest); diff --git a/src/main/java/com/dotcms/saml/service/handler/HttpPostAssertionResolverHandlerImpl.java b/src/main/java/com/dotcms/saml/service/handler/HttpPostAssertionResolverHandlerImpl.java index c26caa1..e7b6428 100755 --- a/src/main/java/com/dotcms/saml/service/handler/HttpPostAssertionResolverHandlerImpl.java +++ b/src/main/java/com/dotcms/saml/service/handler/HttpPostAssertionResolverHandlerImpl.java @@ -4,8 +4,10 @@ import com.dotcms.saml.MessageObserver; import com.dotcms.saml.SamlConfigurationService; import com.dotcms.saml.SamlName; +import com.dotcms.saml.service.impl.SamlResponseValidator; import com.dotcms.saml.service.internal.SamlCoreService; import com.dotcms.saml.service.external.SamlException; +import com.dotcms.saml.utils.InstanceUtil; import com.dotcms.saml.utils.SamlUtils; import net.shibboleth.utilities.java.support.component.ComponentInitializationException; import org.apache.commons.lang.StringUtils; @@ -44,14 +46,25 @@ public class HttpPostAssertionResolverHandlerImpl implements AssertionResolverHa private final MessageObserver messageObserver; private final SamlCoreService samlCoreService; private final SamlConfigurationService samlConfigurationService; + private final SamlResponseValidator samlResponseValidator; public HttpPostAssertionResolverHandlerImpl(final MessageObserver messageObserver, final SamlCoreService samlCoreService, final SamlConfigurationService samlConfigurationService) { + this(messageObserver, samlCoreService, samlConfigurationService, + InstanceUtil.getInstance(SamlResponseValidator.class)); + } + + public HttpPostAssertionResolverHandlerImpl(final MessageObserver messageObserver, + final SamlCoreService samlCoreService, + final SamlConfigurationService samlConfigurationService, + final SamlResponseValidator samlResponseValidator) { + this.messageObserver = messageObserver; this.samlCoreService = samlCoreService; this.samlConfigurationService = samlConfigurationService; + this.samlResponseValidator = samlResponseValidator; } @Override @@ -85,6 +98,13 @@ public Assertion resolveAssertion(final HttpServletRequest request, final HttpSe decoder.decode(); messageContext = decoder.getMessageContext(); + if (!(messageContext.getMessage() instanceof Response)) { + + this.messageObserver.updateError(this.getClass().getName(), "The SAML message posted for IdP '" + + identityProviderConfiguration.getIdpName() + "' is not a Response"); + throw new SamlException("The SAML message posted to the assertion consumer URL is not a Response"); + } + samlResponse = (Response) messageContext.getMessage(); this.messageObserver.updateDebug(this.getClass().getName(),"Post message context decoded:"); @@ -100,7 +120,13 @@ public Assertion resolveAssertion(final HttpServletRequest request, final HttpSe decoder.destroy(); } - this.validateDestinationAndLifetime(messageContext, request, identityProviderConfiguration); + if (null == this.samlResponseValidator) { + + // fail closed: never resolve an assertion without the profile checks + throw new SamlException("The SAML Response validator is not available"); + } + + this.validateMessageLifetime(messageContext, identityProviderConfiguration); assertion = this.samlCoreService.getAssertion(samlResponse, identityProviderConfiguration); @@ -113,26 +139,35 @@ public Assertion resolveAssertion(final HttpServletRequest request, final HttpSe this.verifyStatus(samlResponse); + // Destination, Issuer, Audience, Conditions, SubjectConfirmation, InResponseTo and replay. + this.samlResponseValidator.validate(samlResponse, assertion, request, response, identityProviderConfiguration); + return assertion; } private void verifyStatus(final Response response) { final Status status = response.getStatus(); - final StatusCode statusCode = status.getStatusCode(); - final String statusCodeURI = statusCode.getValue(); + final StatusCode statusCode = null != status ? status.getStatusCode() : null; + final String statusCodeURI = null != statusCode ? statusCode.getValue() : null; - if (!statusCodeURI.equals(StatusCode.SUCCESS)) { + if (!StatusCode.SUCCESS.equals(statusCodeURI)) { this.messageObserver.updateError(this.getClass().getName(), - "SAML status code was NOT successful: " + statusCode.getStatusCode().getValue()); - throw new SamlException("SAML status code was NOT successful: " + statusCode.getValue()); + "SAML status code was NOT successful: " + statusCodeURI); + throw new SamlException("SAML status code was NOT successful: " + statusCodeURI); } } + /** + * Checks the Response IssueInstant against the configured message lifetime. The Destination and the + * assertion validity window are checked by {@link SamlResponseValidator}, against the configured assertion + * consumer URL rather than the servlet request URL, which differs behind a TLS-terminating proxy and after the + * /dotsaml/login to /api/v1/dotsaml/login rewrite. + */ @SuppressWarnings("unchecked") - private void validateDestinationAndLifetime(final MessageContext context, - final HttpServletRequest request, final IdentityProviderConfiguration identityProviderConfiguration) { + private void validateMessageLifetime(final MessageContext context, + final IdentityProviderConfiguration identityProviderConfiguration) { // Just setting it to a value in case of exception. long clockSkew = DOT_SAML_CLOCK_SKEW_DEFAULT_VALUE; @@ -180,7 +215,6 @@ private void validateDestinationAndLifetime(final MessageContext con lifetimeSecurityHandler.setMessageLifetime(lifeTime); lifetimeSecurityHandler.setRequiredRule(true); - // validation of message destination. handlers.add(lifetimeSecurityHandler); handlerChain.setHandlers(handlers); diff --git a/src/main/java/com/dotcms/saml/service/handler/HttpRedirectAuthenticationHandler.java b/src/main/java/com/dotcms/saml/service/handler/HttpRedirectAuthenticationHandler.java index 8d29fe0..170b397 100644 --- a/src/main/java/com/dotcms/saml/service/handler/HttpRedirectAuthenticationHandler.java +++ b/src/main/java/com/dotcms/saml/service/handler/HttpRedirectAuthenticationHandler.java @@ -7,6 +7,7 @@ import com.dotcms.saml.service.external.SamlException; import com.dotcms.saml.service.impl.DotHTTPRedirectDeflateEncoder; import com.dotcms.saml.service.internal.SamlCoreService; +import com.dotcms.saml.utils.AuthnRequestStateCookie; import com.dotcms.saml.utils.SamlUtils; import com.dotcms.saml.utils.SignatureUtils; import com.dotmarketing.util.Logger; @@ -57,6 +58,8 @@ public void handle(final HttpServletRequest request, final HttpServletResponse r final MessageContext context = new MessageContext(); // main context final AuthnRequest authnRequest = this.samlCoreService.buildAuthnRequest(request, identityProviderConfiguration); + // lets the SAML Response be bound to this request (InResponseTo) when the IdP posts it back + AuthnRequestStateCookie.remember(request, response, identityProviderConfiguration, authnRequest.getID()); // in case sign is need on request final boolean needSign = identityProviderConfiguration.containsOptionalProperty("auth.sign.request")? diff --git a/src/main/java/com/dotcms/saml/service/impl/CredentialServiceImpl.java b/src/main/java/com/dotcms/saml/service/impl/CredentialServiceImpl.java index aa8c1fe..0205c65 100644 --- a/src/main/java/com/dotcms/saml/service/impl/CredentialServiceImpl.java +++ b/src/main/java/com/dotcms/saml/service/impl/CredentialServiceImpl.java @@ -7,6 +7,10 @@ import com.dotcms.saml.service.internal.CredentialService; import com.dotcms.saml.utils.InstanceUtil; import com.dotcms.saml.service.external.SamlConstants; +import com.dotmarketing.util.Logger; +import org.apache.commons.lang.StringUtils; + +import java.util.Locale; public class CredentialServiceImpl implements CredentialService { @@ -57,10 +61,8 @@ public CredentialProvider getServiceProviderCustomCredentialProvider(final Iden } /** - * If the user wants to do a verifyAssertionSignature, by default true. - * There are some testing or diagnostic scenarios where you want to avoid - * the validation to identified issues, but in general on production this - * must be true. + * True when the assertion must carry a valid signature: validation type "assertion" or + * "responseandassertion", or any value that is not recognized (see {@link #resolveSignatureValidationType}). * * @param identityProviderConfiguration {@link IdentityProviderConfiguration} * @return boolean @@ -68,15 +70,13 @@ public CredentialProvider getServiceProviderCustomCredentialProvider(final Iden @Override public boolean isVerifyAssertionSignatureNeeded(final IdentityProviderConfiguration identityProviderConfiguration) { - return identityProviderConfiguration.getSignatureValidationType().equals(SamlConstants.RESPONSE_AND_ASSERTION) || - identityProviderConfiguration.getSignatureValidationType().equals(SamlConstants.ASSERTION); + final String validationType = resolveSignatureValidationType(identityProviderConfiguration); + return SamlConstants.RESPONSE_AND_ASSERTION.equals(validationType) || SamlConstants.ASSERTION.equals(validationType); } /** - * If the user wants to do a verifyResponseSignature, by default true. - * There are some testing or diagnostic scenarios where you want to avoid - * the validation to identified issues, but in general on production this - * must be true. + * True when the response must carry a valid signature: validation type "response" or + * "responseandassertion", or any value that is not recognized (see {@link #resolveSignatureValidationType}). * * @param identityProviderConfiguration identityProviderConfiguration * @return boolean @@ -84,40 +84,70 @@ public boolean isVerifyAssertionSignatureNeeded(final IdentityProviderConfigurat @Override public boolean isVerifyResponseSignatureNeeded(final IdentityProviderConfiguration identityProviderConfiguration) { - return identityProviderConfiguration.getSignatureValidationType().equals( SamlConstants.RESPONSE_AND_ASSERTION ) || - identityProviderConfiguration.getSignatureValidationType().equals( SamlConstants.RESPONSE ); + final String validationType = resolveSignatureValidationType(identityProviderConfiguration); + return SamlConstants.RESPONSE_AND_ASSERTION.equals(validationType) || SamlConstants.RESPONSE.equals(validationType); } /** - * If the user wants to do a verifySignatureCredentials, by default true - * There are some testing or diagnostic scenarios where you want to avoid - * the validation to identified issues, but in general on production this - * must be true. Note: if isVerifyAssertionSignatureNeeded is true, this is - * also skipped. - * + * Always true. Cryptographic verification of a signature cannot be turned off; a configured + * "false" value is ignored and logged. + * * @param identityProviderConfiguration IdentityProviderConfiguration * @return boolean */ @Override public boolean isVerifySignatureCredentialsNeeded(final IdentityProviderConfiguration identityProviderConfiguration) { - return this.samlConfigurationService.getConfigAsBoolean(identityProviderConfiguration, SamlName.DOT_SAML_VERIFY_SIGNATURE_CREDENTIALS); - + warnIfDisabled(identityProviderConfiguration, SamlName.DOT_SAML_VERIFY_SIGNATURE_CREDENTIALS); + return true; } /** - * If the user wants to do a verifySignatureProfile, by default true There - * are some testing or diagnostic scenarios where you want to avoid the - * validation to identified issues, but in general on production this must - * be true. Note: if isVerifyAssertionSignatureNeeded is true, this is also - * skipped. - * + * Always true. The signature profile check binds a signature to the element it covers; a configured + * "false" value is ignored and logged. + * * @param identityProviderConfiguration IdentityProviderConfiguration * @return boolean */ @Override public boolean isVerifySignatureProfileNeeded(final IdentityProviderConfiguration identityProviderConfiguration) { - return this.samlConfigurationService.getConfigAsBoolean(identityProviderConfiguration, SamlName.DOT_SAML_VERIFY_SIGNATURE_PROFILE); + warnIfDisabled(identityProviderConfiguration, SamlName.DOT_SAML_VERIFY_SIGNATURE_PROFILE); + return true; + } + + /** + * Returns the configured signature validation type when it is one of "response", "assertion" or + * "responseandassertion". Anything else (blank, "none", a typo) fails closed to "responseandassertion", + * so an unrecognized value can never turn signature verification off. + * + * @param identityProviderConfiguration IdentityProviderConfiguration + * @return String one of the {@link SamlConstants} validation types + */ + public static String resolveSignatureValidationType(final IdentityProviderConfiguration identityProviderConfiguration) { + + final String configured = identityProviderConfiguration.getSignatureValidationType(); + final String normalized = null == configured ? StringUtils.EMPTY : configured.trim().toLowerCase(Locale.ROOT); + switch (normalized) { + case SamlConstants.RESPONSE: + case SamlConstants.ASSERTION: + case SamlConstants.RESPONSE_AND_ASSERTION: + return normalized; + default: + Logger.warn(CredentialServiceImpl.class, "Unsupported signature validation type '" + configured + + "' for IdP '" + identityProviderConfiguration.getIdpName() + + "'. Requiring a signed response and a signed assertion."); + return SamlConstants.RESPONSE_AND_ASSERTION; + } + } + + private void warnIfDisabled(final IdentityProviderConfiguration identityProviderConfiguration, final SamlName samlName) { + + final Boolean configured = this.samlConfigurationService.getConfigAsBoolean(identityProviderConfiguration, samlName); + if (Boolean.FALSE.equals(configured)) { + + Logger.warn(CredentialServiceImpl.class, "Ignoring '" + samlName.getPropertyName() + "=false' for IdP '" + + identityProviderConfiguration.getIdpName() + "': SAML signatures are always verified."); + } } } diff --git a/src/main/java/com/dotcms/saml/service/impl/DatabaseAssertionReplayStore.java b/src/main/java/com/dotcms/saml/service/impl/DatabaseAssertionReplayStore.java new file mode 100644 index 0000000..580a5eb --- /dev/null +++ b/src/main/java/com/dotcms/saml/service/impl/DatabaseAssertionReplayStore.java @@ -0,0 +1,140 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.service.external.SamlException; +import com.dotcms.saml.service.internal.AssertionReplayStore; +import com.dotmarketing.db.DbConnectionFactory; +import com.dotmarketing.util.Logger; + +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.LongSupplier; + +/** + * {@link AssertionReplayStore} backed by a table in the dotCMS database, so a used assertion is rejected by + * every node of a cluster and after a restart. + * + * The bundle creates the table itself on first use. Each check deletes expired rows, then inserts the + * assertion key with {@code ON CONFLICT DO NOTHING}: one inserted row means first use, none means replay. + * It runs on its own auto-commit connection, so the record is committed straight away and two nodes + * checking the same assertion at once can't both succeed. Any database error is reported as a + * {@link SamlException}, so the caller rejects the response rather than skipping the check. + * + * @author dotCMS + */ +public class DatabaseAssertionReplayStore implements AssertionReplayStore { + + static final String TABLE = "dotsaml_assertion_replay"; + + static final String CREATE_TABLE = "CREATE TABLE IF NOT EXISTS " + TABLE + + " (assertion_key VARCHAR(64) NOT NULL PRIMARY KEY, expires_at BIGINT NOT NULL)"; + static final String CREATE_INDEX = "CREATE INDEX IF NOT EXISTS idx_" + TABLE + "_expires_at ON " + + TABLE + " (expires_at)"; + static final String DELETE_EXPIRED = "DELETE FROM " + TABLE + " WHERE expires_at < ?"; + static final String INSERT = "INSERT INTO " + TABLE + " (assertion_key, expires_at) VALUES (?, ?)" + + " ON CONFLICT (assertion_key) DO NOTHING"; + + /** The table only has to be created once per JVM. */ + private static final AtomicBoolean TABLE_READY = new AtomicBoolean(false); + + /** Opens a connection; closing it is the store's job. */ + @FunctionalInterface + interface ConnectionSource { + Connection open() throws SQLException; + } + + private final ConnectionSource connectionSource; + private final LongSupplier clock; + private final AtomicBoolean tableReady; + + public DatabaseAssertionReplayStore() { + + this(() -> DbConnectionFactory.getDataSource().getConnection(), System::currentTimeMillis, TABLE_READY); + } + + DatabaseAssertionReplayStore(final ConnectionSource connectionSource, final LongSupplier clock, + final AtomicBoolean tableReady) { + + this.connectionSource = connectionSource; + this.clock = clock; + this.tableReady = tableReady; + } + + @Override + public boolean markUsed(final String key, final long expiresAt) { + + try (Connection connection = this.connectionSource.open()) { + + connection.setAutoCommit(true); + this.ensureTable(connection); + + try (PreparedStatement delete = connection.prepareStatement(DELETE_EXPIRED)) { + + delete.setLong(1, this.clock.getAsLong()); + delete.executeUpdate(); + } + + try (PreparedStatement insert = connection.prepareStatement(INSERT)) { + + insert.setString(1, hash(key)); + insert.setLong(2, expiresAt); + return insert.executeUpdate() == 1; + } + } catch (SQLException e) { + + Logger.error(DatabaseAssertionReplayStore.class, "Could not check the SAML assertion replay table: " + + e.getMessage(), e); + throw new SamlException("Could not check whether the SAML assertion has already been used", e); + } + } + + private void ensureTable(final Connection connection) throws SQLException { + + if (this.tableReady.get()) { + return; + } + + synchronized (this.tableReady) { + + if (this.tableReady.get()) { + return; + } + + try (Statement statement = connection.createStatement()) { + + statement.execute(CREATE_TABLE); + statement.execute(CREATE_INDEX); + } catch (SQLException e) { + + // another node may be creating it at the same moment; if the table really is missing, + // the insert below fails and the response is rejected + Logger.warn(DatabaseAssertionReplayStore.class, "Could not create " + TABLE + ": " + e.getMessage()); + return; + } + + this.tableReady.set(true); + } + } + + /** Fixed-length key, whatever the length of the assertion ID. */ + static String hash(final String key) { + + try { + + final byte[] digest = MessageDigest.getInstance("SHA-256").digest(key.getBytes(StandardCharsets.UTF_8)); + final StringBuilder hex = new StringBuilder(digest.length * 2); + for (final byte b : digest) { + hex.append(Character.forDigit((b >> 4) & 0xF, 16)).append(Character.forDigit(b & 0xF, 16)); + } + return hex.toString(); + } catch (NoSuchAlgorithmException e) { + + throw new SamlException("SHA-256 is not available", e); + } + } +} diff --git a/src/main/java/com/dotcms/saml/service/impl/OpenSamlAuthenticationServiceImpl.java b/src/main/java/com/dotcms/saml/service/impl/OpenSamlAuthenticationServiceImpl.java index 7e7bc7f..28fefa4 100644 --- a/src/main/java/com/dotcms/saml/service/impl/OpenSamlAuthenticationServiceImpl.java +++ b/src/main/java/com/dotcms/saml/service/impl/OpenSamlAuthenticationServiceImpl.java @@ -11,6 +11,7 @@ import com.dotcms.saml.service.external.AdditionalInformationType; import com.dotcms.saml.service.external.AttributesNotFoundException; import com.dotcms.saml.service.external.NotNullEmailAllowedException; +import com.dotcms.saml.service.external.SamlException; import com.dotcms.saml.service.handler.AssertionResolverHandler; import com.dotcms.saml.service.handler.AssertionResolverHandlerFactory; import com.dotcms.saml.service.handler.AuthenticationHandler; @@ -20,6 +21,7 @@ import com.dotcms.saml.service.init.Initializer; import com.dotcms.saml.service.internal.MetaDescriptorService; import com.dotcms.saml.service.internal.SamlCoreService; +import com.dotcms.saml.utils.EndpointHostRedirect; import com.dotcms.saml.utils.MetaDataXMLPrinter; import com.dotcms.saml.utils.SamlUtils; import com.dotmarketing.exception.DotRuntimeException; @@ -37,6 +39,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; import java.io.Writer; import java.util.ArrayList; import java.util.HashMap; @@ -137,6 +140,20 @@ public void authentication(final HttpServletRequest request, final HttpServletResponse response, final IdentityProviderConfiguration identityProviderConfiguration, final String relayState) { + // start the login on the host the IdP posts back to, so the request-binding cookie comes back with it + try { + + if (EndpointHostRedirect.redirectIfNeeded(request, response, + this.getAssertionConsumerUrl(request, identityProviderConfiguration))) { + + this.messageObserver.updateDebug(this.getClass().getName(), "Login for IdP '" + + identityProviderConfiguration.getIdpName() + "' started on another host; redirected to the endpoint host"); + return; + } + } catch (IOException e) { + + throw new SamlException("Could not redirect the login to the Service Provider Endpoint Hostname", e); + } final AuthenticationHandler authenticationHandler = this.authenticationResolverHandlerFactory.getAuthenticationHandlerForSite(identityProviderConfiguration); @@ -144,6 +161,17 @@ public void authentication(final HttpServletRequest request, authenticationHandler.handle(request, response, identityProviderConfiguration, relayState); } + /** The configured assertion consumer URL, or null when it can't be built (the handler then reports it). */ + private String getAssertionConsumerUrl(final HttpServletRequest request, + final IdentityProviderConfiguration identityProviderConfiguration) { + + try { + return this.samlCoreService.getAssertionConsumerEndpoint(request, identityProviderConfiguration); + } catch (RuntimeException e) { + return null; + } + } + /** * This method creates the logout requet based on the site configuration and the IDP metadata, sends the request to the IDP to ask for the logout * @param request diff --git a/src/main/java/com/dotcms/saml/service/impl/SamlCoreServiceImpl.java b/src/main/java/com/dotcms/saml/service/impl/SamlCoreServiceImpl.java index fdca359..3a8fa87 100644 --- a/src/main/java/com/dotcms/saml/service/impl/SamlCoreServiceImpl.java +++ b/src/main/java/com/dotcms/saml/service/impl/SamlCoreServiceImpl.java @@ -8,6 +8,7 @@ import com.dotcms.saml.service.internal.CredentialProvider; import com.dotcms.saml.service.internal.CredentialService; import com.dotcms.saml.service.internal.EndpointService; +import com.dotcms.saml.service.external.SamlConstants; import com.dotcms.saml.service.external.SamlException; import com.dotcms.saml.service.internal.MetaDataService; import com.dotcms.saml.service.internal.SamlCoreService; @@ -15,15 +16,20 @@ import com.dotcms.saml.utils.IdpConfigCredentialResolver; import com.dotcms.saml.utils.SamlUtils; import com.dotcms.saml.utils.SignatureUtils; +import com.dotmarketing.util.Logger; import net.shibboleth.utilities.java.support.resolver.CriteriaSet; import net.shibboleth.utilities.java.support.resolver.Criterion; import net.shibboleth.utilities.java.support.resolver.ResolverException; import org.apache.commons.lang.StringUtils; +import org.apache.xml.security.exceptions.XMLSecurityException; +import org.apache.xml.security.signature.SignedInfo; +import org.apache.xml.security.signature.XMLSignature; import org.joda.time.DateTime; import org.opensaml.core.criterion.EntityIdCriterion; import org.opensaml.core.xml.XMLObjectBuilderFactory; import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; import org.opensaml.saml.common.SAMLVersion; +import org.opensaml.saml.common.SignableSAMLObject; import org.opensaml.saml.common.xml.SAMLConstants; import org.opensaml.saml.saml2.core.ArtifactResponse; import org.opensaml.saml.saml2.core.Assertion; @@ -52,6 +58,9 @@ import org.opensaml.xmlsec.encryption.support.DecryptionException; import org.opensaml.xmlsec.encryption.support.InlineEncryptedKeyResolver; import org.opensaml.xmlsec.keyinfo.impl.StaticKeyInfoCredentialResolver; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.impl.SignatureImpl; +import org.opensaml.xmlsec.signature.support.SignatureConstants; import org.opensaml.xmlsec.signature.support.SignatureException; import org.opensaml.xmlsec.signature.support.SignatureValidator; import org.w3c.dom.DOMException; @@ -63,9 +72,12 @@ import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.PrivateKey; +import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import static com.dotmarketing.util.UtilMethods.isSet; @@ -86,6 +98,19 @@ public class SamlCoreServiceImpl implements SamlCoreService { private static final Map idpCredentialMap = new ConcurrentHashMap<>(); public static final String SKIP_REQUEST_AUTHN_CONTEXT = "skip.request.authn.context"; + private static final Set ALLOWED_SIGNATURE_ALGORITHMS = Set.of( + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA384, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA512, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA256, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA384, + SignatureConstants.ALGO_ID_SIGNATURE_ECDSA_SHA512); + + private static final Set ALLOWED_DIGEST_ALGORITHMS = Set.of( + SignatureConstants.ALGO_ID_DIGEST_SHA256, + SignatureConstants.ALGO_ID_DIGEST_SHA384, + SignatureConstants.ALGO_ID_DIGEST_SHA512); + private final CredentialService credentialService; private final EndpointService endpointService; private final MetaDataService metaDataService; @@ -517,7 +542,8 @@ public Assertion getAssertion(final ArtifactResponse artifactResponse, } /** - * Get the Assertion decrypted + * Get the Assertion decrypted. The response must carry exactly one assertion, in the form + * (encrypted or not) the configuration expects; anything else is rejected rather than picking one. * * @param response {@link Response} * @param identityProviderConfiguration {@link IdentityProviderConfiguration} @@ -528,20 +554,23 @@ public Assertion getAssertion(final ArtifactResponse artifactResponse, public Assertion getAssertion(final Response response, final IdentityProviderConfiguration identityProviderConfiguration) { - final EncryptedAssertion encryptedAssertion; - Assertion assertion = null; - - if (this.samlConfigurationService.getConfigAsBoolean(identityProviderConfiguration, - SamlName.DOTCMS_SAML_IS_ASSERTION_ENCRYPTED)) { + final int plainAssertions = response.getAssertions().size(); + final int encryptedAssertions = response.getEncryptedAssertions().size(); + final boolean expectEncrypted = this.samlConfigurationService.getConfigAsBoolean(identityProviderConfiguration, + SamlName.DOTCMS_SAML_IS_ASSERTION_ENCRYPTED); - encryptedAssertion = response.getEncryptedAssertions().get(0); - assertion = this.decryptAssertion(encryptedAssertion, identityProviderConfiguration); - } else { + if (plainAssertions + encryptedAssertions != 1 || (expectEncrypted ? encryptedAssertions : plainAssertions) != 1) { - assertion = response.getAssertions().get(0); + final String message = "The SAML Response for IdP '" + identityProviderConfiguration.getIdpName() + + "' must contain exactly one " + (expectEncrypted ? "encrypted" : "unencrypted") + + " assertion, found " + plainAssertions + " unencrypted and " + encryptedAssertions + " encrypted"; + this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), message); + throw new SamlException(message); } - return assertion; + return expectEncrypted? + this.decryptAssertion(response.getEncryptedAssertions().get(0), identityProviderConfiguration): + response.getAssertions().get(0); } /** @@ -586,10 +615,9 @@ public Assertion decryptAssertion(final EncryptedAssertion encryptedAssertion, final EncryptedKey key = this.findEncryptedKey(encryptedAssertion); final Decrypter keyDecrypter = new Decrypter(null, keyInfoCredentialResolver, null); - this.messageObserver.updateInfo(this.getClass().getName(), "Credential: " + credential - + ", key: " + key + ", Algorithm" + encryptedAssertion.getEncryptedData(). - getEncryptionMethod().getAlgorithm() + ", credential.getPrivateKey(): " + credential.getPrivateKey() + - ", encryptedAssertion: " + encryptedAssertion); + // never log the credential's private key + this.messageObserver.updateDebug(this.getClass().getName(), "Decrypting assertion, algorithm: " + + encryptedAssertion.getEncryptedData().getEncryptionMethod().getAlgorithm()); try { @@ -597,7 +625,7 @@ public Assertion decryptAssertion(final EncryptedAssertion encryptedAssertion, final SecretKey decryptKey = (SecretKey) keyDecrypter.decryptKey(key, encryptedAssertion.getEncryptedData(). getEncryptionMethod().getAlgorithm()); - this.messageObserver.updateInfo(this.getClass().getName(), "decryptKey: " + decryptKey); + this.messageObserver.updateDebug(this.getClass().getName(), "Assertion key decrypted"); assertion = new EncryptedAssertionDecrypter(this.messageObserver).decrypt(encryptedAssertion, decryptKey); } catch (DecryptionException | IllegalArgumentException | DOMException e) { @@ -674,146 +702,169 @@ private Assertion decryptAssertionUsingInlineEncrypter (final EncryptedAssertion return assertion; }*/ - private void validateSignature(final Assertion assertion, final Collection credentials) - throws SignatureException { - + /** + * Validates the signature against the IdP signing credentials from the metadata, or the configured IdP + * credential when the metadata has none. Throws when no credential validates it. + */ + private void validateSignature(final Signature signature, final String elementName, + final IdentityProviderConfiguration identityProviderConfiguration) throws SignatureException { + + final Collection signingCredentials = this.metaDataService.getSigningCredentials(identityProviderConfiguration); + final Collection credentials = null != signingCredentials && !signingCredentials.isEmpty()? + signingCredentials: Collections.singletonList(getIdPCredentials(identityProviderConfiguration)); + for (final Credential credential : credentials) { + + if (null == credential) { + continue; + } + try { - SignatureValidator.validate(assertion.getSignature(), credential); + SignatureValidator.validate(signature, credential); return; } catch (SignatureException ignore) { try { - SignatureUtils.validate(assertion.getSignature(), credential); + SignatureUtils.validate(signature, credential); return; } catch (SignatureException ignoreToo) { this.messageObserver.updateInfo(SamlCoreServiceImpl.class.getName(), - "Signature Validation failed with provided credential (ignore?): " + - ignore.getMessage()); + "Signature Validation failed with provided credential: " + ignore.getMessage()); } } } + this.messageObserver.updateInfo(SamlCoreServiceImpl.class.getName(), + "Couldn't find any valid credential to validate the " + elementName + " signature"); + throw new SignatureException(elementName + " Signature cannot be validated"); + } + /** + * Accepts only RSA or ECDSA signatures with SHA-256 or stronger, and SHA-256 or stronger reference digests. + * RSA-SHA1 signatures and SHA-1 digests are accepted only when {@link SamlConstants#ALLOW_SHA1_SIGNATURES} is + * on, with a warning. Anything else (HMAC, MD5, DSA, unknown algorithms) is rejected. The algorithms are read + * from the SignedInfo that is actually verified. + */ + private void validateSignatureAlgorithms(final Signature signature, final String elementName, + final IdentityProviderConfiguration identityProviderConfiguration) { - this.messageObserver.updateInfo(SamlCoreServiceImpl.class.getName(), "Couldn't find any valid credential to validate the assertion signature"); - throw new SignatureException("Assertion Signature cannot be validated"); - } + final XMLSignature xmlSignature = signature instanceof SignatureImpl ? + SignatureImpl.class.cast(signature).getXMLSignature() : null; + if (null == xmlSignature || null == xmlSignature.getSignedInfo()) { - private void validateSignature(final Response response, final Collection credentials) - throws SignatureException { + throw this.rejectAlgorithm(identityProviderConfiguration, "The SAML " + elementName + + " signature algorithms can not be determined"); + } - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), - "Validating Signature - Credentials " + ((credentials != null) ? "are present" : "are null")); - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), - "Validating Signature - Credentials " + ((response != null) ? "are present" : "are null")); + final SignedInfo signedInfo = xmlSignature.getSignedInfo(); + final boolean sha1Allowed = this.isSha1Allowed(identityProviderConfiguration); + final List sha1Usage = new ArrayList<>(); - for (final Credential credential : credentials) { + final String signatureAlgorithm = signedInfo.getSignatureMethodURI(); + if (!ALLOWED_SIGNATURE_ALGORITHMS.contains(signatureAlgorithm)) { - try { + if (!sha1Allowed || !SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1.equals(signatureAlgorithm)) { + throw this.rejectAlgorithm(identityProviderConfiguration, "The SAML " + elementName + + " signature algorithm '" + signatureAlgorithm + "' is not allowed"); + } + sha1Usage.add("signature " + signatureAlgorithm); + } - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Validating Signature - Credential " + - ((credential != null) ? "is present" : "is null")); - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Validating Signature - response.getSignature " + - ((response.getSignature() != null)?"is present" : "is null")); + try { - SignatureValidator.validate(response.getSignature(), credential); - return; - } catch (SignatureException ignore) { + for (int i = 0; i < signedInfo.getLength(); i++) { - try { + final String digestAlgorithm = signedInfo.item(i).getMessageDigestAlgorithm().getAlgorithmURI(); + if (!ALLOWED_DIGEST_ALGORITHMS.contains(digestAlgorithm)) { - SignatureUtils.validate(response.getSignature(), credential); - return; - } catch (SignatureException ignoreToo) { - this.messageObserver.updateInfo(SamlCoreServiceImpl.class.getName(), - "Signature Validation failed with provided credential(s): " + ignore.getMessage()); + if (!sha1Allowed || !SignatureConstants.ALGO_ID_DIGEST_SHA1.equals(digestAlgorithm)) { + throw this.rejectAlgorithm(identityProviderConfiguration, "The SAML " + elementName + + " signature digest algorithm '" + digestAlgorithm + "' is not allowed"); + } + sha1Usage.add("digest " + digestAlgorithm); } } + } catch (XMLSecurityException e) { + + throw this.rejectAlgorithm(identityProviderConfiguration, "The SAML " + elementName + + " signature references can not be read: " + e.getMessage()); + } + + if (!sha1Usage.isEmpty()) { + + Logger.warn(SamlCoreServiceImpl.class, "The SAML " + elementName + " for IdP '" + + identityProviderConfiguration.getIdpName() + "' uses SHA-1 (" + String.join(", ", sha1Usage) + + "), accepted because " + SamlConstants.ALLOW_SHA1_SIGNATURES + + " is on. Configure the IdP to sign with SHA-256 or stronger."); } + } + + private boolean isSha1Allowed(final IdentityProviderConfiguration identityProviderConfiguration) { - this.messageObserver.updateInfo(SamlCoreServiceImpl.class.getName(), "Couldn't find any valid credential to validate the response signature"); - throw new SignatureException("Response Signature cannot be validated"); + return identityProviderConfiguration.containsOptionalProperty(SamlConstants.ALLOW_SHA1_SIGNATURES) + && Boolean.parseBoolean(String.valueOf(identityProviderConfiguration + .getOptionalProperty(SamlConstants.ALLOW_SHA1_SIGNATURES)).trim()); + } + + private SamlException rejectAlgorithm(final IdentityProviderConfiguration identityProviderConfiguration, + final String reason) { + + final String message = reason + " for IdP '" + identityProviderConfiguration.getIdpName() + "'"; + this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), message); + return new SamlException(message); } /** - * Does the verification of the assertion - * - * @param assertion {@link Assertion} - * @param identityProviderConfiguration {@link IdentityProviderConfiguration} + * Verifies a signature on a SAML element. A required signature must be present, and a signature that is + * present is always verified: its profile (which binds it to the element it covers) and its value against + * the IdP credentials. An unsigned element is accepted only when its signature is not required, i.e. when the + * other element of the response carries the required signature. */ - @Override - public void verifyAssertionSignature(final Assertion assertion, - final IdentityProviderConfiguration identityProviderConfiguration) { + private void verifySignature(final SignableSAMLObject signableObject, final String elementName, + final boolean signatureRequired, + final IdentityProviderConfiguration identityProviderConfiguration) { - if (this.credentialService.isVerifyAssertionSignatureNeeded(identityProviderConfiguration) != assertion.isSigned()) { + if (!signableObject.isSigned()) { - this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), "Assertion Signatures for IdP '" + - identityProviderConfiguration.getIdpName() + "' do not match."); - throw new SamlException("The SAML Assertion for IdP '" + - identityProviderConfiguration.getIdpName() + "' does not match"); - } + if (signatureRequired) { - // If unsigned, No need to go further. - if (!this.credentialService.isVerifyAssertionSignatureNeeded(identityProviderConfiguration)) { + this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), "The SAML " + elementName + + " for IdP '" + identityProviderConfiguration.getIdpName() + "' is not signed, but a signature is required."); + throw new SamlException("The SAML " + elementName + " for IdP '" + + identityProviderConfiguration.getIdpName() + "' must be signed"); + } - this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), "The verification assertion signature and status code for IdP '" + - identityProviderConfiguration.getIdpName() + "' was skipped."); - return; // Exit + this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "The SAML " + elementName + + " for IdP '" + identityProviderConfiguration.getIdpName() + "' is not signed and its signature is not required."); + return; } - // Here on out we are checking signature try { // this avoids the: Apache xmlsec IdResolver could not resolve the Element for id reference: xxx - assertion.getDOM().setIdAttribute("ID", true); + if (null != signableObject.getDOM()) { + signableObject.getDOM().setIdAttribute("ID", true); + } + + this.validateSignatureAlgorithms(signableObject.getSignature(), elementName, identityProviderConfiguration); if (this.credentialService.isVerifySignatureProfileNeeded(identityProviderConfiguration)) { this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Executing Profile Validation..."); - - new SAMLSignatureProfileValidator().validate(assertion.getSignature()); - + new SAMLSignatureProfileValidator().validate(signableObject.getSignature()); this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Profile Validation finished"); - } else { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Skipping the Verify Signature Profile check"); } - // Ask on the config if the app wants signature validator if (this.credentialService.isVerifySignatureCredentialsNeeded(identityProviderConfiguration)) { - if (null != this.metaDataService.getSigningCredentials(identityProviderConfiguration)) { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), - "Validating the signatures: " + this.metaDataService.getSigningCredentials(identityProviderConfiguration)); - - this.validateSignature(assertion, this.metaDataService.getSigningCredentials(identityProviderConfiguration)); - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Executing signatures validation..."); - } else { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Validating the signature with a IdP Credentials..."); - - final Credential credential = getIdPCredentials(identityProviderConfiguration); - try { - SignatureValidator.validate(assertion.getSignature(), credential); - } catch (SignatureException e) { - SignatureUtils.validate(assertion.getSignature(), credential); - } - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Validation of the signature with a IdP Credentials finished"); - } - } else { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Skipping the Verify Signature Profile check"); + this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Executing signature validation..."); + this.validateSignature(signableObject.getSignature(), elementName, identityProviderConfiguration); } - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "SAML Assertion signature verified"); - + this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "SAML " + elementName + " signature verified"); } catch (SignatureException e) { this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), e.getMessage(), e); @@ -822,79 +873,30 @@ public void verifyAssertionSignature(final Assertion assertion, } /** - * Does the verification of the assertion + * Does the verification of the assertion signature * - * @param response {@link Assertion} + * @param assertion {@link Assertion} * @param identityProviderConfiguration {@link IdentityProviderConfiguration} */ @Override - public void verifyResponseSignature(final Response response, final IdentityProviderConfiguration identityProviderConfiguration) { - - // The check signature in dotCMS and IdP must match - if (this.credentialService.isVerifyResponseSignatureNeeded(identityProviderConfiguration) != response.isSigned()) { - - this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), "The response signatures for IdP '" + identityProviderConfiguration.getIdpName() + "' do not match."); - throw new SamlException("The SAML Response for IdP '" + identityProviderConfiguration.getIdpName() + "' does not match"); - } - - // If unsigned, No need to go further. - if (!this.credentialService.isVerifyResponseSignatureNeeded(identityProviderConfiguration)) { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "The verification response signature and status code for IdP '" + - identityProviderConfiguration.getIdpName() + "' was skipped."); - return; // Exit - } - - // Here on out we are checking signature - try { - if (this.credentialService.isVerifySignatureProfileNeeded(identityProviderConfiguration)) { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Executing Profile Validation..."); - - new SAMLSignatureProfileValidator().validate(response.getSignature()); - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Profile Validation finished"); - } else { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Skipping verification of Signature Profile"); - } - - // Ask on the config if the app wants signature validator - if (this.credentialService.isVerifySignatureCredentialsNeeded(identityProviderConfiguration)) { - - if (null != this.metaDataService.getSigningCredentials(identityProviderConfiguration)) { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), - "Validating the signatures: " + this.metaDataService.getSigningCredentials(identityProviderConfiguration)); - - this.validateSignature(response, this.metaDataService.getSigningCredentials(identityProviderConfiguration)); - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Executing signature validation..."); - } else { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Validating the signature with a IdP Credentials..."); - - final Credential credential = getIdPCredentials(identityProviderConfiguration); - try { - SignatureValidator.validate(response.getSignature(), credential); - } catch (SignatureException e) { - SignatureUtils.validate(response.getSignature(), credential); - } - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Validation of the signature with a IdP Credentials finished"); - } - } else { - - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "Skipping the Verify Signature Profile check"); - } + public void verifyAssertionSignature(final Assertion assertion, + final IdentityProviderConfiguration identityProviderConfiguration) { - this.messageObserver.updateDebug(SamlCoreServiceImpl.class.getName(), "SAML Response signature verified"); + this.verifySignature(assertion, "Assertion", + this.credentialService.isVerifyAssertionSignatureNeeded(identityProviderConfiguration), identityProviderConfiguration); + } - } catch (SignatureException e) { + /** + * Does the verification of the response signature + * + * @param response {@link Response} + * @param identityProviderConfiguration {@link IdentityProviderConfiguration} + */ + @Override + public void verifyResponseSignature(final Response response, final IdentityProviderConfiguration identityProviderConfiguration) { - this.messageObserver.updateError(SamlCoreServiceImpl.class.getName(), e.getMessage(), e); - throw new SamlException(e.getMessage(), e); - } + this.verifySignature(response, "Response", + this.credentialService.isVerifyResponseSignatureNeeded(identityProviderConfiguration), identityProviderConfiguration); } /** diff --git a/src/main/java/com/dotcms/saml/service/impl/SamlResponseValidator.java b/src/main/java/com/dotcms/saml/service/impl/SamlResponseValidator.java new file mode 100644 index 0000000..985f2d1 --- /dev/null +++ b/src/main/java/com/dotcms/saml/service/impl/SamlResponseValidator.java @@ -0,0 +1,492 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.IdentityProviderConfiguration; +import com.dotcms.saml.MessageObserver; +import com.dotcms.saml.SamlConfigurationService; +import com.dotcms.saml.SamlName; +import com.dotcms.saml.service.external.MetaData; +import com.dotcms.saml.service.external.SamlConstants; +import com.dotcms.saml.service.external.SamlException; +import com.dotcms.saml.service.handler.AssertionResolverHandler; +import com.dotcms.saml.service.internal.AssertionReplayStore; +import com.dotcms.saml.service.internal.EndpointService; +import com.dotcms.saml.service.internal.MetaDataService; +import com.dotcms.saml.utils.AuthnRequestStateCookie; +import org.apache.commons.lang.StringUtils; +import org.joda.time.DateTime; +import org.opensaml.saml.saml2.core.Assertion; +import org.opensaml.saml.saml2.core.Audience; +import org.opensaml.saml.saml2.core.AudienceRestriction; +import org.opensaml.saml.saml2.core.AuthnStatement; +import org.opensaml.saml.saml2.core.Conditions; +import org.opensaml.saml.saml2.core.Issuer; +import org.opensaml.saml.saml2.core.Response; +import org.opensaml.saml.saml2.core.Subject; +import org.opensaml.saml.saml2.core.SubjectConfirmation; +import org.opensaml.saml.saml2.core.SubjectConfirmationData; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.net.URI; +import java.net.URISyntaxException; +import java.time.Clock; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +/** + * Validates a decoded SAML Response and its assertion against the SAML 2.0 Web Browser SSO profile + * (Profiles §4.1.4.2-4.1.4.3, Core §2.5.1, Bindings §3.5.5.2). It runs after the signatures have been + * verified and checks everything a valid signature does not prove on its own: + *
    + *
  • the Response Destination and the bearer SubjectConfirmationData Recipient are this service + * provider's assertion consumer URL;
    • + *
  • the Response and Assertion Issuer are the configured IdP;
    • + *
  • the Assertion is addressed to this service provider (AudienceRestriction) and is inside its + * Conditions NotBefore / NotOnOrAfter window;
    • + *
  • there is a bearer SubjectConfirmation with an unexpired NotOnOrAfter whose InResponseTo matches + * the Response;
    • + *
  • a solicited Response answers an authentication request started from this browser, and an + * unsolicited one is only accepted when {@link SamlConstants#ALLOW_UNSOLICITED_RESPONSES} is on;
    • + *
  • the Assertion has an AuthnStatement whose IdP session has not ended;
    • + *
  • the Assertion has not been used before, on any node ({@link AssertionReplayStore}, until its + * confirmation expires).
    • + *
+ * + * OpenSAML 3.3.1 does ship {@code SAML20AssertionValidator}, but not the parameters for valid issuers or the + * expected InResponseTo ({@code VALID_ISSUERS}, {@code SC_VALID_IN_RESPONSE_TO}, both added in 4.0). It also + * treats a missing Conditions, AudienceRestriction, Recipient or NotOnOrAfter as valid, and resolves + * {@code SubjectConfirmationData@Address} through DNS. So the checks are implemented here explicitly. + * + * @author dotCMS + */ +public class SamlResponseValidator { + + private static final String API_PATH_PREFIX = "/api/v1"; + private static final int MAX_LOGGED_VALUE_LENGTH = 256; + + private final EndpointService endpointService; + private final MetaDataService metaDataService; + private final SamlConfigurationService samlConfigurationService; + private final MessageObserver messageObserver; + private final AssertionReplayStore replayStore; + private final Clock clock; + + public SamlResponseValidator(final EndpointService endpointService, + final MetaDataService metaDataService, + final SamlConfigurationService samlConfigurationService, + final MessageObserver messageObserver) { + + this(endpointService, metaDataService, samlConfigurationService, messageObserver, + new DatabaseAssertionReplayStore(), Clock.systemUTC()); + } + + /** + * For tests and custom deployments: a specific replay store and clock. + */ + public SamlResponseValidator(final EndpointService endpointService, + final MetaDataService metaDataService, + final SamlConfigurationService samlConfigurationService, + final MessageObserver messageObserver, + final AssertionReplayStore replayStore, + final Clock clock) { + + this.endpointService = endpointService; + this.metaDataService = metaDataService; + this.samlConfigurationService = samlConfigurationService; + this.messageObserver = messageObserver; + this.replayStore = replayStore; + this.clock = clock; + } + + /** + * Validates the response and its (already decrypted and signature-verified) assertion. Throws a + * {@link SamlException} on the first check that fails. + * + * @param response {@link Response} + * @param assertion {@link Assertion} taken from the response + * @param request {@link HttpServletRequest} carrying the response + * @param httpServletResponse {@link HttpServletResponse} + * @param identityProviderConfiguration {@link IdentityProviderConfiguration} + */ + public void validate(final Response response, final Assertion assertion, + final HttpServletRequest request, final HttpServletResponse httpServletResponse, + final IdentityProviderConfiguration identityProviderConfiguration) { + + final long now = this.clock.millis(); + final long clockSkew = this.getClockSkew(identityProviderConfiguration); + final Set acsUrls = this.getAssertionConsumerUrls(identityProviderConfiguration); + final String idpEntityId = this.getIdentityProviderEntityId(identityProviderConfiguration); + final String requestId = StringUtils.trimToNull(response.getInResponseTo()); + + this.validateDestination(response, acsUrls, identityProviderConfiguration); + this.validateIssuer(response.getIssuer(), idpEntityId, "Response", + response.isSigned() || !response.getEncryptedAssertions().isEmpty(), identityProviderConfiguration); + this.validateIssuer(assertion.getIssuer(), idpEntityId, "Assertion", true, identityProviderConfiguration); + this.validateConditions(assertion, now, clockSkew, identityProviderConfiguration); + final long confirmationExpiry = this.validateSubjectConfirmation(assertion, acsUrls, requestId, now, + clockSkew, identityProviderConfiguration); + this.validateAuthnStatements(assertion, now, clockSkew, identityProviderConfiguration); + this.validateInResponseTo(requestId, request, httpServletResponse, identityProviderConfiguration); + this.checkReplay(assertion, idpEntityId, confirmationExpiry + clockSkew, identityProviderConfiguration); + } + + private void validateDestination(final Response response, final Set acsUrls, + final IdentityProviderConfiguration identityProviderConfiguration) { + + final String destination = StringUtils.trimToNull(response.getDestination()); + if (null == destination) { + + // Bindings §3.5.5.2: a signed message MUST carry the Destination it was issued for. + if (response.isSigned()) { + throw this.fail(identityProviderConfiguration, "The signed SAML Response has no Destination"); + } + return; + } + + if (!matchesAny(destination, acsUrls)) { + + throw this.fail(identityProviderConfiguration, "The SAML Response Destination '" + sanitize(destination) + + "' is not this service provider's assertion consumer URL " + acsUrls); + } + } + + private void validateIssuer(final Issuer issuer, final String idpEntityId, final String elementName, + final boolean required, + final IdentityProviderConfiguration identityProviderConfiguration) { + + final String issuerValue = null != issuer ? StringUtils.trimToNull(issuer.getValue()) : null; + if (null == issuerValue) { + + if (required) { + throw this.fail(identityProviderConfiguration, "The SAML " + elementName + " has no Issuer"); + } + return; + } + + if (!idpEntityId.equals(issuerValue)) { + + throw this.fail(identityProviderConfiguration, "The SAML " + elementName + " Issuer '" + sanitize(issuerValue) + + "' is not the configured IdP '" + idpEntityId + "'"); + } + } + + private void validateConditions(final Assertion assertion, final long now, final long clockSkew, + final IdentityProviderConfiguration identityProviderConfiguration) { + + final Conditions conditions = assertion.getConditions(); + if (null == conditions) { + + throw this.fail(identityProviderConfiguration, + "The SAML Assertion has no Conditions, so it is not restricted to this service provider"); + } + + final DateTime notBefore = conditions.getNotBefore(); + if (null != notBefore && now + clockSkew < notBefore.getMillis()) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion is not valid before " + notBefore); + } + + final DateTime notOnOrAfter = conditions.getNotOnOrAfter(); + if (null != notOnOrAfter && now - clockSkew >= notOnOrAfter.getMillis()) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion expired at " + notOnOrAfter); + } + + final String spEntityId = StringUtils.trimToNull(identityProviderConfiguration.getSpIssuerURL()); + if (null == spEntityId) { + + throw this.fail(identityProviderConfiguration, "The Service Provider Issuer ID is not configured"); + } + + // Profiles §4.1.4.2: the assertion MUST be restricted to the service provider, and every + // AudienceRestriction present must include it (Core §2.5.1.4). + final List audienceRestrictions = conditions.getAudienceRestrictions(); + if (audienceRestrictions.isEmpty()) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion has no AudienceRestriction"); + } + + for (final AudienceRestriction audienceRestriction : audienceRestrictions) { + + boolean addressedToUs = false; + for (final Audience audience : audienceRestriction.getAudiences()) { + + if (sameEntityId(audience.getAudienceURI(), spEntityId)) { + addressedToUs = true; + break; + } + } + + if (!addressedToUs) { + + throw this.fail(identityProviderConfiguration, + "The SAML Assertion AudienceRestriction does not include this service provider '" + spEntityId + "'"); + } + } + } + + /** + * Returns the NotOnOrAfter (epoch millis) of the first valid bearer confirmation. + */ + private long validateSubjectConfirmation(final Assertion assertion, final Set acsUrls, + final String requestId, final long now, final long clockSkew, + final IdentityProviderConfiguration identityProviderConfiguration) { + + final Subject subject = assertion.getSubject(); + if (null == subject || subject.getSubjectConfirmations().isEmpty()) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion has no SubjectConfirmation"); + } + + final List problems = new ArrayList<>(); + for (final SubjectConfirmation subjectConfirmation : subject.getSubjectConfirmations()) { + + if (!SubjectConfirmation.METHOD_BEARER.equals(subjectConfirmation.getMethod())) { + problems.add("method '" + sanitize(subjectConfirmation.getMethod()) + "' is not bearer"); + continue; + } + + final SubjectConfirmationData data = subjectConfirmation.getSubjectConfirmationData(); + if (null == data) { + problems.add("no SubjectConfirmationData"); + continue; + } + + // Profiles §4.1.4.2 says a bearer confirmation must not carry NotBefore. Like OpenSAML, this + // tolerates one and only checks it is not in the future, for IdPs that send it anyway. + if (null != data.getNotBefore() && now + clockSkew < data.getNotBefore().getMillis()) { + problems.add("not valid before " + data.getNotBefore()); + continue; + } + + if (null == data.getNotOnOrAfter()) { + problems.add("no NotOnOrAfter"); + continue; + } + + if (now - clockSkew >= data.getNotOnOrAfter().getMillis()) { + problems.add("expired at " + data.getNotOnOrAfter()); + continue; + } + + final String recipient = StringUtils.trimToNull(data.getRecipient()); + if (null == recipient || !matchesAny(recipient, acsUrls)) { + problems.add("Recipient '" + sanitize(recipient) + "' is not this service provider's assertion consumer URL"); + continue; + } + + // Profiles §4.1.4.2: the confirmation's InResponseTo must agree with the Response's. + if (!StringUtils.equals(requestId, StringUtils.trimToNull(data.getInResponseTo()))) { + problems.add("InResponseTo does not match the SAML Response"); + continue; + } + + return data.getNotOnOrAfter().getMillis(); + } + + throw this.fail(identityProviderConfiguration, + "The SAML Assertion has no valid bearer SubjectConfirmation: " + String.join("; ", problems)); + } + + private void validateAuthnStatements(final Assertion assertion, final long now, final long clockSkew, + final IdentityProviderConfiguration identityProviderConfiguration) { + + if (assertion.getAuthnStatements().isEmpty()) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion has no AuthnStatement"); + } + + for (final AuthnStatement authnStatement : assertion.getAuthnStatements()) { + + final DateTime sessionNotOnOrAfter = authnStatement.getSessionNotOnOrAfter(); + if (null != sessionNotOnOrAfter && now - clockSkew >= sessionNotOnOrAfter.getMillis()) { + + throw this.fail(identityProviderConfiguration, "The IdP session in the SAML Assertion ended at " + sessionNotOnOrAfter); + } + } + } + + private void validateInResponseTo(final String requestId, final HttpServletRequest request, + final HttpServletResponse httpServletResponse, + final IdentityProviderConfiguration identityProviderConfiguration) { + + if (null == requestId) { + + if (!this.isUnsolicitedResponseAllowed(identityProviderConfiguration)) { + + throw this.fail(identityProviderConfiguration, "Unsolicited SAML Responses (IdP-initiated login) are not accepted. " + + "Start the login from dotCMS, or set '" + SamlConstants.ALLOW_UNSOLICITED_RESPONSES + + "=true' on the SAML configuration to allow IdP-initiated login"); + } + return; + } + + if (!AuthnRequestStateCookie.consume(request, httpServletResponse, identityProviderConfiguration, requestId)) { + + throw this.fail(identityProviderConfiguration, "The SAML Response InResponseTo '" + sanitize(requestId) + + "' does not match an authentication request started from this browser"); + } + } + + private void checkReplay(final Assertion assertion, final String idpEntityId, final long expiresAt, + final IdentityProviderConfiguration identityProviderConfiguration) { + + final String assertionId = StringUtils.trimToNull(assertion.getID()); + if (null == assertionId) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion has no ID"); + } + + final boolean firstUse; + try { + + firstUse = this.replayStore.markUsed(idpEntityId + '|' + assertionId, expiresAt); + } catch (SamlException e) { + + throw this.fail(identityProviderConfiguration, "Could not check whether the SAML Assertion '" + + sanitize(assertionId) + "' has already been used"); + } + + if (!firstUse) { + + throw this.fail(identityProviderConfiguration, "The SAML Assertion '" + sanitize(assertionId) + "' has already been used"); + } + } + + private boolean isUnsolicitedResponseAllowed(final IdentityProviderConfiguration identityProviderConfiguration) { + + return identityProviderConfiguration.containsOptionalProperty(SamlConstants.ALLOW_UNSOLICITED_RESPONSES) + && Boolean.parseBoolean(String.valueOf(identityProviderConfiguration + .getOptionalProperty(SamlConstants.ALLOW_UNSOLICITED_RESPONSES)).trim()); + } + + private long getClockSkew(final IdentityProviderConfiguration identityProviderConfiguration) { + + try { + + final Integer clockSkew = this.samlConfigurationService.getConfigAsInteger(identityProviderConfiguration, + SamlName.DOT_SAML_CLOCK_SKEW); + if (null != clockSkew && clockSkew >= 0) { + return clockSkew.longValue(); + } + } catch (Exception e) { + + this.messageObserver.updateInfo(this.getClass().getName(), + "Optional property not set: " + SamlName.DOT_SAML_CLOCK_SKEW.getPropertyName() + ". Using default."); + } + + return AssertionResolverHandler.DOT_SAML_CLOCK_SKEW_DEFAULT_VALUE; + } + + /** + * The assertion consumer URL published in the SP metadata and AuthnRequest, plus its /api/v1 form + * (the REST endpoint the public URL is rewritten to), which some IdPs are configured with directly. + */ + private Set getAssertionConsumerUrls(final IdentityProviderConfiguration identityProviderConfiguration) { + + final Set acsUrls = new LinkedHashSet<>(); + final String acsUrl = this.endpointService.getAssertionConsumerEndpoint(identityProviderConfiguration); + if (StringUtils.isBlank(acsUrl)) { + + throw this.fail(identityProviderConfiguration, "The assertion consumer URL can not be determined; " + + "check the Service Provider Endpoint Hostname"); + } + + acsUrls.add(acsUrl); + final int pathStart = acsUrl.indexOf(SamlConstants.ASSERTION_CONSUMER_ENDPOINT_DOTSAML3SP); + if (pathStart > 0) { + acsUrls.add(acsUrl.substring(0, pathStart) + API_PATH_PREFIX + acsUrl.substring(pathStart)); + } + + return acsUrls; + } + + private String getIdentityProviderEntityId(final IdentityProviderConfiguration identityProviderConfiguration) { + + final MetaData metaData = this.metaDataService.getMetaData(identityProviderConfiguration); + final String entityId = null != metaData ? StringUtils.trimToNull(metaData.getEntityId()) : null; + if (null == entityId) { + + throw this.fail(identityProviderConfiguration, "The IdP entityID can not be read from the IdP metadata"); + } + + return entityId; + } + + private SamlException fail(final IdentityProviderConfiguration identityProviderConfiguration, final String reason) { + + final String message = "Rejected SAML Response for IdP '" + identityProviderConfiguration.getIdpName() + "': " + reason; + this.messageObserver.updateError(this.getClass().getName(), message); + return new SamlException(message); + } + + private static boolean matchesAny(final String url, final Set expectedUrls) { + + final String normalized = normalizeUrl(url); + if (null == normalized) { + return false; + } + + for (final String expectedUrl : expectedUrls) { + if (normalized.equals(normalizeUrl(expectedUrl))) { + return true; + } + } + + return false; + } + + /** + * Normalizes a URL for comparison: lower-case scheme and host, default port removed, no trailing slash. + * The path is compared as is. Returns null for anything that is not an absolute http(s) URL. + */ + static String normalizeUrl(final String url) { + + if (StringUtils.isBlank(url)) { + return null; + } + + try { + + final URI uri = new URI(url.trim()); + if (null == uri.getScheme() || null == uri.getHost()) { + return null; + } + + final String scheme = uri.getScheme().toLowerCase(Locale.ROOT); + if (!"https".equals(scheme) && !"http".equals(scheme)) { + return null; + } + + final int port = uri.getPort(); + final boolean defaultPort = -1 == port || ("https".equals(scheme) && 443 == port) || ("http".equals(scheme) && 80 == port); + final String path = StringUtils.removeEnd(StringUtils.defaultString(uri.getRawPath()), "/"); + final String query = null != uri.getRawQuery() ? "?" + uri.getRawQuery() : StringUtils.EMPTY; + + return scheme + "://" + uri.getHost().toLowerCase(Locale.ROOT) + (defaultPort ? StringUtils.EMPTY : ":" + port) + + path + query; + } catch (URISyntaxException e) { + return null; + } + } + + private static boolean sameEntityId(final String audience, final String spEntityId) { + + final String trimmedAudience = StringUtils.trimToNull(audience); + return null != trimmedAudience + && StringUtils.removeEnd(trimmedAudience, "/").equals(StringUtils.removeEnd(spEntityId, "/")); + } + + private static String sanitize(final String value) { + + if (null == value) { + return "null"; + } + + final String singleLine = value.replaceAll("[\\p{Cntrl}]", "_"); + return singleLine.length() > MAX_LOGGED_VALUE_LENGTH ? singleLine.substring(0, MAX_LOGGED_VALUE_LENGTH) + "..." : singleLine; + } +} diff --git a/src/main/java/com/dotcms/saml/service/impl/SamlServiceBuilderImpl.java b/src/main/java/com/dotcms/saml/service/impl/SamlServiceBuilderImpl.java index f41ed2d..145c95c 100644 --- a/src/main/java/com/dotcms/saml/service/impl/SamlServiceBuilderImpl.java +++ b/src/main/java/com/dotcms/saml/service/impl/SamlServiceBuilderImpl.java @@ -59,10 +59,14 @@ public SamlAuthenticationService buildAuthenticationService(final IdentityProvid final MetaDescriptorService metaDescriptorService = new DefaultMetaDescriptorServiceImpl(samlConfigurationService, messageObserver, samlCoreService, credentialService, endpointService); + final SamlResponseValidator samlResponseValidator = + new SamlResponseValidator(endpointService, metaDataService, samlConfigurationService, messageObserver); InstanceUtil.putInstance(MetaDescriptorService.class, metaDescriptorService); InstanceUtil.putInstance(SamlCoreService.class, samlCoreService); + InstanceUtil.putInstance(SamlResponseValidator.class, samlResponseValidator); assertionResolverHandlerFactory.addAssertionResolverHandler(HttpPostAssertionResolverHandlerImpl.class.getName(), - new HttpPostAssertionResolverHandlerImpl(messageObserver, samlCoreService, samlConfigurationService)); + new HttpPostAssertionResolverHandlerImpl(messageObserver, samlCoreService, samlConfigurationService, + samlResponseValidator)); final AuthenticationResolverHandlerFactory authenticationResolverHandlerFactory = new AuthenticationResolverHandlerFactory(samlConfigurationService, samlCoreService, velocityEngine, messageObserver); final LogoutResolverHandlerFactory logoutResolverHandlerFactory = diff --git a/src/main/java/com/dotcms/saml/service/internal/AssertionReplayStore.java b/src/main/java/com/dotcms/saml/service/internal/AssertionReplayStore.java new file mode 100644 index 0000000..6aefddd --- /dev/null +++ b/src/main/java/com/dotcms/saml/service/internal/AssertionReplayStore.java @@ -0,0 +1,19 @@ +package com.dotcms.saml.service.internal; + +/** + * Remembers the SAML assertions that have been accepted, so each one is accepted only once. + * + * @author dotCMS + */ +public interface AssertionReplayStore { + + /** + * Records an assertion as used. + * + * @param key identifies the assertion (IdP and assertion ID) + * @param expiresAt epoch millis after which the assertion is no longer valid, so it can be forgotten + * @return true the first time the key is recorded, false when it was already used (a replay) + * @throws com.dotcms.saml.service.external.SamlException when the store can't tell; callers must reject + */ + boolean markUsed(String key, long expiresAt); +} diff --git a/src/main/java/com/dotcms/saml/utils/AuthnRequestStateCookie.java b/src/main/java/com/dotcms/saml/utils/AuthnRequestStateCookie.java new file mode 100644 index 0000000..be78fad --- /dev/null +++ b/src/main/java/com/dotcms/saml/utils/AuthnRequestStateCookie.java @@ -0,0 +1,209 @@ +package com.dotcms.saml.utils; + +import com.dotcms.saml.IdentityProviderConfiguration; +import com.dotcms.saml.service.external.SamlConstants; +import com.dotcms.saml.service.external.SamlException; +import org.apache.commons.lang.StringUtils; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.util.ArrayList; +import java.util.List; +import java.util.function.LongSupplier; +import java.util.regex.Pattern; + +/** + * Remembers, in the user's browser, the authentication requests dotCMS sent to the IdP, so the + * SAML Response that comes back can be bound to the request that caused it (InResponseTo). + * + * The IdP returns the user with a cross-site POST, and the session cookie is SameSite=Lax by default, so + * the HTTP session is not available on that request. A short-lived, HttpOnly, SameSite=None cookie is sent + * on that POST instead. There is one cookie per IdP configuration, holding the most recent request IDs + * (several, so logins started in more than one tab still work), each with the time it was issued. The + * {@code __Host-} prefix keeps a sibling subdomain from setting it. A response whose InResponseTo isn't in + * the cookie was not started from this browser and is rejected. + * + * @author dotCMS + */ +public final class AuthnRequestStateCookie { + + public static final String COOKIE_PREFIX = "__Host-dotsaml_req_"; + + /** How many outstanding requests are kept per IdP configuration. */ + static final int MAX_OUTSTANDING_REQUESTS = 5; + + private static final Pattern SAFE_TOKEN = Pattern.compile("[A-Za-z0-9_-]{1,128}"); + private static final String ENTRY_SEPARATOR = "."; + private static final String TIME_SEPARATOR = ":"; + private static final int MIN_MAX_AGE_SECONDS = 60; + private static final int MAX_MAX_AGE_SECONDS = 3600; + + private static final LongSupplier SYSTEM_CLOCK = System::currentTimeMillis; + + private AuthnRequestStateCookie() { + // utility class + } + + /** + * Records an outstanding authentication request for the IdP configuration. + * + * @param request {@link HttpServletRequest} that starts the login + * @param response {@link HttpServletResponse} that redirects or posts to the IdP + * @param identityProviderConfiguration {@link IdentityProviderConfiguration} + * @param requestId the AuthnRequest ID + */ + public static void remember(final HttpServletRequest request, final HttpServletResponse response, + final IdentityProviderConfiguration identityProviderConfiguration, + final String requestId) { + + remember(request, response, identityProviderConfiguration, requestId, SYSTEM_CLOCK); + } + + static void remember(final HttpServletRequest request, final HttpServletResponse response, + final IdentityProviderConfiguration identityProviderConfiguration, + final String requestId, final LongSupplier clock) { + + if (!isSafeToken(requestId) || !isSafeToken(identityProviderConfiguration.getId())) { + + throw new SamlException("Can not record the authentication request for IdP '" + + identityProviderConfiguration.getIdpName() + "'"); + } + + final int maxAgeSeconds = getMaxAgeSeconds(identityProviderConfiguration); + final List entries = new ArrayList<>(); + entries.add(requestId + TIME_SEPARATOR + nowSeconds(clock)); + for (final String entry : readEntries(request, identityProviderConfiguration, maxAgeSeconds, clock)) { + + if (entries.size() < MAX_OUTSTANDING_REQUESTS && !requestId.equals(idOf(entry))) { + entries.add(entry); + } + } + + writeCookie(response, identityProviderConfiguration, entries, maxAgeSeconds); + } + + /** + * Returns true when this browser has an unexpired outstanding authentication request with the given ID + * for the IdP configuration, and removes it so the same request can not be answered twice. + * + * @param request {@link HttpServletRequest} carrying the SAML Response + * @param response {@link HttpServletResponse} + * @param identityProviderConfiguration {@link IdentityProviderConfiguration} + * @param requestId the InResponseTo value of the SAML Response + * @return boolean + */ + public static boolean consume(final HttpServletRequest request, final HttpServletResponse response, + final IdentityProviderConfiguration identityProviderConfiguration, + final String requestId) { + + return consume(request, response, identityProviderConfiguration, requestId, SYSTEM_CLOCK); + } + + static boolean consume(final HttpServletRequest request, final HttpServletResponse response, + final IdentityProviderConfiguration identityProviderConfiguration, + final String requestId, final LongSupplier clock) { + + if (!isSafeToken(requestId) || !isSafeToken(identityProviderConfiguration.getId())) { + + return false; + } + + final int maxAgeSeconds = getMaxAgeSeconds(identityProviderConfiguration); + final List entries = readEntries(request, identityProviderConfiguration, maxAgeSeconds, clock); + final boolean found = entries.removeIf(entry -> requestId.equals(idOf(entry))); + if (found) { + + writeCookie(response, identityProviderConfiguration, entries, maxAgeSeconds); + } + + return found; + } + + static String cookieName(final IdentityProviderConfiguration identityProviderConfiguration) { + + return COOKIE_PREFIX + identityProviderConfiguration.getId(); + } + + static int getMaxAgeSeconds(final IdentityProviderConfiguration identityProviderConfiguration) { + + int maxAge = SamlConstants.AUTHN_REQUEST_MAX_AGE_DEFAULT_VALUE; + if (identityProviderConfiguration.containsOptionalProperty(SamlConstants.AUTHN_REQUEST_MAX_AGE)) { + + try { + maxAge = Integer.parseInt(String.valueOf( + identityProviderConfiguration.getOptionalProperty(SamlConstants.AUTHN_REQUEST_MAX_AGE)).trim()); + } catch (NumberFormatException e) { + maxAge = SamlConstants.AUTHN_REQUEST_MAX_AGE_DEFAULT_VALUE; + } + } + + return Math.max(MIN_MAX_AGE_SECONDS, Math.min(MAX_MAX_AGE_SECONDS, maxAge)); + } + + /** Unexpired, well-formed entries from the cookie, newest first. */ + private static List readEntries(final HttpServletRequest request, + final IdentityProviderConfiguration identityProviderConfiguration, + final int maxAgeSeconds, final LongSupplier clock) { + + final List entries = new ArrayList<>(); + final Cookie[] cookies = request.getCookies(); + if (null == cookies) { + return entries; + } + + final String name = cookieName(identityProviderConfiguration); + final long oldest = nowSeconds(clock) - maxAgeSeconds; + for (final Cookie cookie : cookies) { + + if (!name.equals(cookie.getName()) || StringUtils.isBlank(cookie.getValue())) { + continue; + } + + for (final String entry : StringUtils.split(cookie.getValue(), ENTRY_SEPARATOR)) { + + final String id = idOf(entry); + final long issuedAt = issuedAtOf(entry); + if (isSafeToken(id) && issuedAt >= oldest && entries.size() < MAX_OUTSTANDING_REQUESTS) { + entries.add(entry); + } + } + } + + return entries; + } + + private static void writeCookie(final HttpServletResponse response, + final IdentityProviderConfiguration identityProviderConfiguration, + final List entries, final int maxAgeSeconds) { + + final String value = String.join(ENTRY_SEPARATOR, entries); + response.addHeader("Set-Cookie", cookieName(identityProviderConfiguration) + "=" + value + + "; Max-Age=" + (entries.isEmpty() ? 0 : maxAgeSeconds) + + "; Path=/; Secure; HttpOnly; SameSite=None"); + } + + private static String idOf(final String entry) { + + return StringUtils.substringBefore(entry, TIME_SEPARATOR); + } + + private static long issuedAtOf(final String entry) { + + try { + return Long.parseLong(StringUtils.substringAfter(entry, TIME_SEPARATOR)); + } catch (NumberFormatException e) { + return -1L; + } + } + + private static long nowSeconds(final LongSupplier clock) { + + return clock.getAsLong() / 1000L; + } + + private static boolean isSafeToken(final String value) { + + return null != value && SAFE_TOKEN.matcher(value).matches(); + } +} diff --git a/src/main/java/com/dotcms/saml/utils/EndpointHostRedirect.java b/src/main/java/com/dotcms/saml/utils/EndpointHostRedirect.java new file mode 100644 index 0000000..eedb662 --- /dev/null +++ b/src/main/java/com/dotcms/saml/utils/EndpointHostRedirect.java @@ -0,0 +1,78 @@ +package com.dotcms.saml.utils; + +import org.apache.commons.lang.StringUtils; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Locale; + +/** + * Makes an SP-initiated login start on the host the IdP posts back to. + * + * The AuthnRequest always names the assertion consumer URL on the Service Provider Endpoint Hostname, and the + * request-binding cookie ({@link AuthnRequestStateCookie}) is set on the host that starts the login. When the + * user starts on another host (a site alias, or a site using the System Host configuration under a different + * hostname), the browser would not send that cookie back with the response. So the browser is first sent to + * the assertion consumer URL on the endpoint host, where a GET starts the login again. Users already end up on + * the endpoint host after login, so the flow looks the same to them. + * + * The target is always the configured assertion consumer URL, never a value from the request. A marker + * parameter stops a second redirect, so a proxy that rewrites the Host header can't cause a loop. Hosts are + * compared without ports, because cookies are shared across ports. + * + * @author dotCMS + */ +public final class EndpointHostRedirect { + + public static final String MARKER_PARAMETER = "dotsaml_host_redirect"; + + private EndpointHostRedirect() { + // utility class + } + + /** + * Redirects the browser to the assertion consumer URL when the login started on another host. + * + * @param request {@link HttpServletRequest} starting the login + * @param response {@link HttpServletResponse} + * @param assertionConsumerUrl the configured assertion consumer URL + * @return true when the browser was redirected and the login must not continue on this request + * @throws IOException when the redirect can't be sent + */ + public static boolean redirectIfNeeded(final HttpServletRequest request, final HttpServletResponse response, + final String assertionConsumerUrl) throws IOException { + + if (null != request.getParameter(MARKER_PARAMETER)) { + return false; + } + + final String endpointHost = hostOf(assertionConsumerUrl); + final String serverName = StringUtils.trimToNull(request.getServerName()); + final String requestHost = null == serverName ? null : serverName.toLowerCase(Locale.ROOT); + if (null == endpointHost || null == requestHost || endpointHost.equals(requestHost)) { + return false; + } + + response.sendRedirect(assertionConsumerUrl + (assertionConsumerUrl.contains("?") ? "&" : "?") + + MARKER_PARAMETER + "=1"); + return true; + } + + /** The lower-case host of an absolute URL, or null. */ + static String hostOf(final String url) { + + if (StringUtils.isBlank(url)) { + return null; + } + + try { + final String host = new URI(url.trim()).getHost(); + return null == host ? null : host.toLowerCase(Locale.ROOT); + } catch (URISyntaxException e) { + return null; + } + } +} diff --git a/src/main/java/com/dotcms/saml/utils/SamlUtils.java b/src/main/java/com/dotcms/saml/utils/SamlUtils.java index 01e8415..e12051c 100644 --- a/src/main/java/com/dotcms/saml/utils/SamlUtils.java +++ b/src/main/java/com/dotcms/saml/utils/SamlUtils.java @@ -18,7 +18,6 @@ import org.opensaml.messaging.context.MessageContext; import org.opensaml.messaging.handler.MessageHandlerException; import org.opensaml.messaging.handler.impl.BasicMessageHandlerChain; -import org.opensaml.saml.common.SignableSAMLObject; import org.opensaml.saml.saml2.core.NameID; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -65,9 +64,10 @@ public static String generateSecureRandomId() { */ public static String toXMLObjectString(final XMLObject object) { - final Element element = object instanceof SignableSAMLObject - && SignableSAMLObject.class.cast(object).isSigned() && object.getDOM() != null? - object.getDOM() : toElement(object); + // Serialize the existing DOM when there is one. Marshalling again would move the element into a new + // document and release its parent's DOM, and OpenSAML reads the signature of the parent (for example + // the Response around an unsigned assertion) from that DOM. + final Element element = null != object.getDOM()? object.getDOM() : toElement(object); return toElementString(element); } diff --git a/src/test/java/com/dotcms/saml/service/handler/TestHttpPostAssertionResolverHandler.java b/src/test/java/com/dotcms/saml/service/handler/TestHttpPostAssertionResolverHandler.java new file mode 100644 index 0000000..33cfc42 --- /dev/null +++ b/src/test/java/com/dotcms/saml/service/handler/TestHttpPostAssertionResolverHandler.java @@ -0,0 +1,475 @@ +package com.dotcms.saml.service.handler; + +import com.dotcms.saml.IdentityProviderConfiguration; +import com.dotcms.saml.SamlName; +import com.dotcms.saml.service.external.MetaData; +import com.dotcms.saml.service.external.SamlConstants; +import com.dotcms.saml.service.external.SamlException; +import com.dotcms.saml.service.impl.CredentialServiceImpl; +import com.dotcms.saml.service.impl.InMemoryAssertionReplayStore; +import com.dotcms.saml.service.impl.MockMessageObserver; +import com.dotcms.saml.service.impl.MockSamlConfigurationService; +import com.dotcms.saml.service.impl.SamlCoreServiceImpl; +import com.dotcms.saml.service.impl.SamlResponseValidator; +import com.dotcms.saml.service.init.SamlInitializer; +import com.dotcms.saml.service.internal.CredentialProvider; +import com.dotcms.saml.service.internal.EndpointService; +import com.dotcms.saml.service.internal.MetaDataService; +import com.dotcms.saml.service.internal.MetaDescriptorService; +import com.dotcms.saml.utils.AuthnRequestStateCookie; +import net.shibboleth.utilities.java.support.xml.SerializeSupport; +import org.joda.time.DateTime; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.opensaml.core.xml.XMLObjectBuilderFactory; +import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; +import org.opensaml.saml.saml2.core.Assertion; +import org.opensaml.saml.saml2.core.Audience; +import org.opensaml.saml.saml2.core.AudienceRestriction; +import org.opensaml.saml.saml2.core.AuthnStatement; +import org.opensaml.saml.saml2.core.Conditions; +import org.opensaml.saml.saml2.core.EncryptedAssertion; +import org.opensaml.saml.saml2.core.Issuer; +import org.opensaml.saml.saml2.core.LogoutResponse; +import org.opensaml.saml.saml2.core.NameID; +import org.opensaml.saml.saml2.core.Response; +import org.opensaml.saml.saml2.core.Status; +import org.opensaml.saml.saml2.core.StatusCode; +import org.opensaml.saml.saml2.core.Subject; +import org.opensaml.saml.saml2.core.SubjectConfirmation; +import org.opensaml.saml.saml2.core.SubjectConfirmationData; +import org.opensaml.saml.saml2.encryption.Encrypter; +import org.opensaml.security.credential.Credential; +import org.opensaml.security.credential.CredentialSupport; +import org.opensaml.security.crypto.KeySupport; +import org.opensaml.xmlsec.encryption.support.DataEncryptionParameters; +import org.opensaml.xmlsec.encryption.support.EncryptionConstants; +import org.opensaml.xmlsec.encryption.support.KeyEncryptionParameters; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.support.SignatureConstants; +import org.opensaml.xmlsec.signature.support.Signer; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.xml.namespace.QName; +import java.lang.reflect.Proxy; +import java.nio.charset.StandardCharsets; +import java.security.KeyPair; +import java.time.Clock; +import java.util.Base64; +import java.util.Collection; +import java.util.Collections; +import java.util.UUID; + +/** + * End to end through the HTTP-POST assertion consumer handler: decoding, signature rules and the profile + * checks. + */ +public class TestHttpPostAssertionResolverHandler { + + private static final String IDP_ENTITY_ID = "https://idp.example.com/metadata"; + private static final String SP_ENTITY_ID = "https://dotcms.example.com/dotAdmin"; + private static final String CONFIG_ID = "48190c8c-42c4-46af-8d1a-0cd5db894797"; + private static final String ACS_URL = "https://dotcms.example.com/dotsaml/login/" + CONFIG_ID; + + private static XMLObjectBuilderFactory builderFactory; + private static Credential idpCredential; + private static Credential spCredential; + + @BeforeClass + public static void initOpenSaml() throws Exception { + + new SamlInitializer().init(Collections.emptyMap()); + builderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory(); + final KeyPair keyPair = KeySupport.generateKeyPair("RSA", 2048, null); + idpCredential = CredentialSupport.getSimpleCredential(keyPair.getPublic(), keyPair.getPrivate()); + final KeyPair spKeyPair = KeySupport.generateKeyPair("RSA", 2048, null); + spCredential = CredentialSupport.getSimpleCredential(spKeyPair.getPublic(), spKeyPair.getPrivate()); + } + + @Test + public void unsignedResponseIsRejectedWhenValidationTypeIsNone() throws Exception { + + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.NONE, false)); + + assertRejected("must be signed", () -> handler().resolveAssertion(post(samlResponse, requestId), + response(), idp("none"))); + } + + @Test + public void validSignedAssertionIsResolved() throws Exception { + + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.ASSERTION, false)); + + final Assertion assertion = handler().resolveAssertion(post(samlResponse, requestId), response(), + idp(SamlConstants.ASSERTION)); + Assert.assertEquals("admin@example.com", assertion.getSubject().getNameID().getValue()); + } + + @Test + public void responseSignedIdpWithPlainAssertionIsResolved() throws Exception { + + // e.g. Entra ID set to sign the SAML response only + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.RESPONSE, false)); + + final Assertion assertion = handler().resolveAssertion(post(samlResponse, requestId), response(), + idp(SamlConstants.RESPONSE)); + Assert.assertEquals("admin@example.com", assertion.getSubject().getNameID().getValue()); + } + + @Test + public void responseSignedIdpWithEncryptedAssertionIsResolved() throws Exception { + + // Shibboleth's default: signed response, encrypted (unsigned) assertion + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.RESPONSE, true)); + + final Assertion assertion = handler(encryptedAssertionConfiguration()).resolveAssertion( + post(samlResponse, requestId), response(), idp(SamlConstants.RESPONSE)); + Assert.assertEquals("admin@example.com", assertion.getSubject().getNameID().getValue()); + } + + @Test + public void responseAndAssertionSignedIdpIsResolvedWhicheverPartIsRequired() throws Exception { + + for (final String validationType : new String[]{SamlConstants.RESPONSE_AND_ASSERTION, + SamlConstants.RESPONSE, SamlConstants.ASSERTION}) { + + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.BOTH, false)); + + final Assertion assertion = handler().resolveAssertion(post(samlResponse, requestId), response(), + idp(validationType)); + Assert.assertEquals(validationType, "admin@example.com", assertion.getSubject().getNameID().getValue()); + } + } + + @Test + public void responseOnlySignatureIsRejectedWhenTheAssertionMustBeSigned() throws Exception { + + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.RESPONSE, false)); + + assertRejected("Assertion for IdP 'Test IdP' must be signed", () -> handler().resolveAssertion( + post(samlResponse, requestId), response(), idp(SamlConstants.RESPONSE_AND_ASSERTION))); + } + + @Test + public void alteredSignedResponseIsRejected() throws Exception { + + final String requestId = newId(); + final String xml = SerializeSupport.nodeToString( + response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.RESPONSE, false).getDOM()); + final String altered = Base64.getEncoder().encodeToString( + xml.replace("admin@example.com", "root@example.com").getBytes(StandardCharsets.UTF_8)); + + assertRejected("Response Signature cannot be validated", () -> handler().resolveAssertion( + post(altered, requestId), response(), idp(SamlConstants.RESPONSE))); + } + + @Test + public void validlySignedAssertionForAnotherServiceProviderIsRejected() throws Exception { + + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, "https://partner-app.example.com"), + Signing.ASSERTION, false)); + + assertRejected("AudienceRestriction does not include this service provider", + () -> handler().resolveAssertion(post(samlResponse, requestId), response(), idp(SamlConstants.ASSERTION))); + } + + @Test + public void samlMessageThatIsNotAResponseIsRejected() throws Exception { + + final LogoutResponse logoutResponse = build(LogoutResponse.DEFAULT_ELEMENT_NAME); + logoutResponse.setID(newId()); + logoutResponse.setIssueInstant(new DateTime()); + logoutResponse.setIssuer(issuer()); + XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(logoutResponse).marshall(logoutResponse); + final String posted = Base64.getEncoder().encodeToString( + SerializeSupport.nodeToString(logoutResponse.getDOM()).getBytes(StandardCharsets.UTF_8)); + + assertRejected("is not a Response", () -> handler().resolveAssertion(post(posted, newId()), response(), + idp(SamlConstants.ASSERTION))); + } + + @Test + public void handlerWithoutValidatorFailsClosed() throws Exception { + + final String requestId = newId(); + final String samlResponse = encode(response(requestId, assertion(requestId, SP_ENTITY_ID), Signing.ASSERTION, false)); + final MockSamlConfigurationService configurationService = new MockSamlConfigurationService(); + final HttpPostAssertionResolverHandlerImpl handler = new HttpPostAssertionResolverHandlerImpl( + new MockMessageObserver(), samlCoreService(configurationService), configurationService, null); + + assertRejected("validator is not available", + () -> handler.resolveAssertion(post(samlResponse, requestId), response(), idp(SamlConstants.ASSERTION))); + } + + // --------------------------------------------------------------------------------------------------------- + + private static void assertRejected(final String expectedReason, final Runnable resolution) { + + try { + resolution.run(); + Assert.fail("Expected the SAML Response to be rejected: " + expectedReason); + } catch (SamlException e) { + Assert.assertTrue("Unexpected reason: " + e.getMessage(), e.getMessage().contains(expectedReason)); + } + } + + private enum Signing { NONE, ASSERTION, RESPONSE, BOTH } + + private static HttpPostAssertionResolverHandlerImpl handler() { + + return handler(new MockSamlConfigurationService()); + } + + private static HttpPostAssertionResolverHandlerImpl handler(final MockSamlConfigurationService configurationService) { + + return new HttpPostAssertionResolverHandlerImpl(new MockMessageObserver(), samlCoreService(configurationService), + configurationService, new SamlResponseValidator(endpointService(), metaDataService(), configurationService, + new MockMessageObserver(), new InMemoryAssertionReplayStore(), Clock.systemUTC())); + } + + private static SamlCoreServiceImpl samlCoreService(final MockSamlConfigurationService configurationService) { + + return new SamlCoreServiceImpl(new CredentialServiceImpl(configurationService), endpointService(), + metaDataService(), new MockMessageObserver(), configurationService, null); + } + + /** Assertions arrive encrypted for the SP key held by {@link TestSpCredentialProvider}. */ + private static MockSamlConfigurationService encryptedAssertionConfiguration() { + + return new MockSamlConfigurationService() { + + @Override + public Boolean getConfigAsBoolean(final IdentityProviderConfiguration idp, final SamlName samlName) { + return SamlName.DOTCMS_SAML_IS_ASSERTION_ENCRYPTED == samlName || super.getConfigAsBoolean(idp, samlName); + } + + @Override + public String getConfigAsString(final IdentityProviderConfiguration idp, final SamlName samlName) { + return SamlName.DOT_SAML_SERVICE_PROVIDER_CUSTOM_CREDENTIAL_PROVIDER_CLASSNAME == samlName + ? TestSpCredentialProvider.class.getName() : super.getConfigAsString(idp, samlName); + } + }; + } + + /** The SP decryption key, loaded by name the way a custom credential provider is configured. */ + public static class TestSpCredentialProvider implements CredentialProvider { + + @Override + public Credential createCredential() { + return spCredential; + } + } + + private static String encode(final Response response) { + + // no pretty printing: whitespace inside the signed assertion is covered by its digest + return Base64.getEncoder().encodeToString( + SerializeSupport.nodeToString(response.getDOM()).getBytes(StandardCharsets.UTF_8)); + } + + private static Response response(final String requestId, final Assertion assertion, final Signing signing, + final boolean encryptAssertion) throws Exception { + + final Response response = build(Response.DEFAULT_ELEMENT_NAME); + response.setID(newId()); + response.setIssueInstant(new DateTime()); + response.setDestination(ACS_URL); + response.setInResponseTo(requestId); + response.setIssuer(issuer()); + + final StatusCode statusCode = build(StatusCode.DEFAULT_ELEMENT_NAME); + statusCode.setValue(StatusCode.SUCCESS); + final Status status = build(Status.DEFAULT_ELEMENT_NAME); + status.setStatusCode(statusCode); + response.setStatus(status); + + final boolean signAssertion = Signing.ASSERTION == signing || Signing.BOTH == signing; + final boolean signResponse = Signing.RESPONSE == signing || Signing.BOTH == signing; + final Signature assertionSignature = signAssertion ? signature() : null; + final Signature responseSignature = signResponse ? signature() : null; + assertion.setSignature(assertionSignature); + response.setSignature(responseSignature); + + if (encryptAssertion) { + + // the assertion is signed (if at all) before it is encrypted + XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(assertion).marshall(assertion); + if (null != assertionSignature) { + Signer.signObject(assertionSignature); + } + response.getEncryptedAssertions().add(encrypt(assertion)); + XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(response).marshall(response); + } else { + + response.getAssertions().add(assertion); + XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(response).marshall(response); + if (null != assertionSignature) { + Signer.signObject(assertionSignature); + } + } + + // the response signature covers the assertion, so it is computed last + if (null != responseSignature) { + Signer.signObject(responseSignature); + } + + return response; + } + + private static Signature signature() { + + final Signature signature = build(Signature.DEFAULT_ELEMENT_NAME); + signature.setSigningCredential(idpCredential); + signature.setSignatureAlgorithm(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256); + signature.setCanonicalizationAlgorithm(SignatureConstants.ALGO_ID_C14N_EXCL_OMIT_COMMENTS); + return signature; + } + + private static EncryptedAssertion encrypt(final Assertion assertion) throws Exception { + + final DataEncryptionParameters dataParameters = new DataEncryptionParameters(); + dataParameters.setAlgorithm(EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES128); + final KeyEncryptionParameters keyParameters = new KeyEncryptionParameters(); + keyParameters.setEncryptionCredential(spCredential); + keyParameters.setAlgorithm(EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP); + final Encrypter encrypter = new Encrypter(dataParameters, keyParameters); + encrypter.setKeyPlacement(Encrypter.KeyPlacement.INLINE); + return encrypter.encrypt(assertion); + } + + private static Assertion assertion(final String requestId, final String audienceUri) { + + final long now = System.currentTimeMillis(); + final Assertion assertion = build(Assertion.DEFAULT_ELEMENT_NAME); + assertion.setID(newId()); + assertion.setIssueInstant(new DateTime(now)); + assertion.setIssuer(issuer()); + + final NameID nameID = build(NameID.DEFAULT_ELEMENT_NAME); + nameID.setValue("admin@example.com"); + + final SubjectConfirmationData data = build(SubjectConfirmationData.DEFAULT_ELEMENT_NAME); + data.setRecipient(ACS_URL); + data.setNotOnOrAfter(new DateTime(now + 300_000)); + data.setInResponseTo(requestId); + + final SubjectConfirmation subjectConfirmation = build(SubjectConfirmation.DEFAULT_ELEMENT_NAME); + subjectConfirmation.setMethod(SubjectConfirmation.METHOD_BEARER); + subjectConfirmation.setSubjectConfirmationData(data); + + final Subject subject = build(Subject.DEFAULT_ELEMENT_NAME); + subject.setNameID(nameID); + subject.getSubjectConfirmations().add(subjectConfirmation); + assertion.setSubject(subject); + + final Audience audience = build(Audience.DEFAULT_ELEMENT_NAME); + audience.setAudienceURI(audienceUri); + final AudienceRestriction audienceRestriction = build(AudienceRestriction.DEFAULT_ELEMENT_NAME); + audienceRestriction.getAudiences().add(audience); + final Conditions conditions = build(Conditions.DEFAULT_ELEMENT_NAME); + conditions.setNotBefore(new DateTime(now - 5_000)); + conditions.setNotOnOrAfter(new DateTime(now + 300_000)); + conditions.getAudienceRestrictions().add(audienceRestriction); + assertion.setConditions(conditions); + + final AuthnStatement authnStatement = build(AuthnStatement.DEFAULT_ELEMENT_NAME); + authnStatement.setAuthnInstant(new DateTime(now)); + assertion.getAuthnStatements().add(authnStatement); + + return assertion; + } + + private static Issuer issuer() { + + final Issuer issuer = build(Issuer.DEFAULT_ELEMENT_NAME); + issuer.setValue(IDP_ENTITY_ID); + return issuer; + } + + private static String newId() { + + return "_" + UUID.randomUUID().toString().replace("-", ""); + } + + @SuppressWarnings("unchecked") + private static T build(final QName elementName) { + + return (T) builderFactory.getBuilder(elementName).buildObject(elementName); + } + + private static HttpServletRequest post(final String samlResponse, final String requestId) { + + final Cookie[] cookies = {new Cookie(AuthnRequestStateCookie.COOKIE_PREFIX + CONFIG_ID, + requestId + ":" + (System.currentTimeMillis() / 1000L))}; + return (HttpServletRequest) Proxy.newProxyInstance(TestHttpPostAssertionResolverHandler.class.getClassLoader(), + new Class[]{HttpServletRequest.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getMethod": return "POST"; + case "getCookies": return cookies; + case "getParameter": return "SAMLResponse".equals(args[0]) ? samlResponse : null; + default: return null; + } + }); + } + + private static HttpServletResponse response() { + + return (HttpServletResponse) Proxy.newProxyInstance(TestHttpPostAssertionResolverHandler.class.getClassLoader(), + new Class[]{HttpServletResponse.class}, (proxy, method, args) -> null); + } + + private static IdentityProviderConfiguration idp(final String signatureValidationType) { + + return (IdentityProviderConfiguration) Proxy.newProxyInstance(TestHttpPostAssertionResolverHandler.class.getClassLoader(), + new Class[]{IdentityProviderConfiguration.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getId": return CONFIG_ID; + case "getIdpName": return "Test IdP"; + case "getSpIssuerURL": return SP_ENTITY_ID; + case "getSpEndpointHostname": return "dotcms.example.com"; + case "getSignatureValidationType": return signatureValidationType; + case "isEnabled": return true; + case "containsOptionalProperty": return false; + default: return null; + } + }); + } + + private static EndpointService endpointService() { + + return new EndpointService() { + @Override public String getAssertionConsumerEndpoint(final IdentityProviderConfiguration idp) { return ACS_URL; } + @Override public String getSingleLogoutEndpoint(final IdentityProviderConfiguration idp) { return null; } + @Override public String[] getAccessFilterArray(final IdentityProviderConfiguration idp) { return null; } + @Override public String[] getLogoutPathArray(final IdentityProviderConfiguration idp) { return null; } + @Override public String[] getIncludePathArray(final IdentityProviderConfiguration idp) { return null; } + }; + } + + private static MetaDataService metaDataService() { + + final MetaData metaData = new MetaData(IDP_ENTITY_ID, null, Collections.emptyMap(), Collections.emptyMap(), + Collections.singletonList(idpCredential)); + return new MetaDataService() { + @Override public MetaData getMetaData(final IdentityProviderConfiguration idp) { return metaData; } + @Override public MetaDescriptorService getMetaDescriptorService(final IdentityProviderConfiguration idp) { return null; } + @Override public Collection getSigningCredentials(final IdentityProviderConfiguration idp) { + return Collections.singletonList(idpCredential); + } + @Override public String getIdentityProviderDestinationSSOURL(final IdentityProviderConfiguration idp) { return null; } + @Override public String getIdentityProviderDestinationSLOURL(final IdentityProviderConfiguration idp) { return null; } + }; + } +} diff --git a/src/test/java/com/dotcms/saml/service/impl/InMemoryAssertionReplayStore.java b/src/test/java/com/dotcms/saml/service/impl/InMemoryAssertionReplayStore.java new file mode 100644 index 0000000..1758890 --- /dev/null +++ b/src/test/java/com/dotcms/saml/service/impl/InMemoryAssertionReplayStore.java @@ -0,0 +1,22 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.service.internal.AssertionReplayStore; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Test double for {@link AssertionReplayStore}: same contract as the database store, in memory. + */ +public class InMemoryAssertionReplayStore implements AssertionReplayStore { + + private final Map used = new ConcurrentHashMap<>(); + + @Override + public boolean markUsed(final String key, final long expiresAt) { + + final long now = System.currentTimeMillis(); + this.used.values().removeIf(expiry -> expiry < now); + return null == this.used.putIfAbsent(key, expiresAt); + } +} diff --git a/src/test/java/com/dotcms/saml/service/impl/TestDatabaseAssertionReplayStore.java b/src/test/java/com/dotcms/saml/service/impl/TestDatabaseAssertionReplayStore.java new file mode 100644 index 0000000..866091b --- /dev/null +++ b/src/test/java/com/dotcms/saml/service/impl/TestDatabaseAssertionReplayStore.java @@ -0,0 +1,169 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.service.external.SamlException; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; + +/** + * {@link DatabaseAssertionReplayStore} against a fake JDBC layer that behaves like Postgres for the four + * statements the store uses (the SQL itself is checked against a real Postgres separately). + */ +public class TestDatabaseAssertionReplayStore { + + private final Map table = new HashMap<>(); + private final List executed = new ArrayList<>(); + private final AtomicLong now = new AtomicLong(1_000_000L); + private boolean failCreate; + private boolean failInsert; + private DatabaseAssertionReplayStore store; + + @Before + public void setUp() { + + this.store = new DatabaseAssertionReplayStore(this::connection, this.now::get, new AtomicBoolean(false)); + } + + @Test + public void firstUseIsAcceptedAndAReplayIsRejected() { + + Assert.assertTrue(this.store.markUsed("idp|_assertion1", 2_000_000L)); + Assert.assertFalse(this.store.markUsed("idp|_assertion1", 2_000_000L)); + Assert.assertTrue(this.store.markUsed("idp|_assertion2", 2_000_000L)); + } + + @Test + public void storesAFixedLengthHashOfTheKey() { + + this.store.markUsed("idp|" + new String(new char[2000]).replace('\0', 'x'), 2_000_000L); + + final String storedKey = this.table.keySet().iterator().next(); + Assert.assertEquals(64, storedKey.length()); + Assert.assertTrue(storedKey.matches("[0-9a-f]{64}")); + } + + @Test + public void expiredRowsAreDeletedBeforeTheCheck() { + + this.store.markUsed("idp|_old", 1_500_000L); + this.now.set(1_600_000L); + this.store.markUsed("idp|_new", 3_000_000L); + + Assert.assertEquals(1, this.table.size()); + } + + @Test + public void tableIsCreatedOnce() { + + this.store.markUsed("idp|_a", 2_000_000L); + this.store.markUsed("idp|_b", 2_000_000L); + + Assert.assertEquals(1, this.executed.stream().filter(sql -> sql.startsWith("CREATE TABLE")).count()); + Assert.assertEquals(1, this.executed.stream().filter(sql -> sql.startsWith("CREATE INDEX")).count()); + } + + @Test + public void failedTableCreationIsRetriedAndDoesNotSkipTheCheck() { + + this.failCreate = true; + Assert.assertTrue(this.store.markUsed("idp|_a", 2_000_000L)); + this.failCreate = false; + Assert.assertFalse(this.store.markUsed("idp|_a", 2_000_000L)); + + Assert.assertEquals(2, this.executed.stream().filter(sql -> sql.startsWith("CREATE TABLE")).count()); + } + + @Test + public void databaseErrorIsReportedNotTreatedAsFirstUse() { + + this.failInsert = true; + try { + this.store.markUsed("idp|_a", 2_000_000L); + Assert.fail("expected a SamlException"); + } catch (SamlException e) { + Assert.assertTrue(e.getMessage().contains("already been used")); + } + } + + // --------------------------------------------------------------------------------------------------------- + + private Connection connection() { + + return (Connection) Proxy.newProxyInstance(getClass().getClassLoader(), new Class[]{Connection.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "createStatement": return statement(); + case "prepareStatement": return preparedStatement((String) args[0]); + default: return null; + } + }); + } + + private Statement statement() { + + return (Statement) Proxy.newProxyInstance(getClass().getClassLoader(), new Class[]{Statement.class}, + (proxy, method, args) -> { + if ("execute".equals(method.getName())) { + this.executed.add((String) args[0]); + if (this.failCreate) { + throw new SQLException("relation already exists"); + } + return false; + } + return null; + }); + } + + private PreparedStatement preparedStatement(final String sql) { + + final Map parameters = new HashMap<>(); + return (PreparedStatement) Proxy.newProxyInstance(getClass().getClassLoader(), + new Class[]{PreparedStatement.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "setString": + case "setLong": + parameters.put((Integer) args[0], args[1]); + return null; + case "executeUpdate": + return this.execute(sql, parameters); + default: + return null; + } + }); + } + + private int execute(final String sql, final Map parameters) throws SQLException { + + this.executed.add(sql); + if (DatabaseAssertionReplayStore.DELETE_EXPIRED.equals(sql)) { + + final long cutoff = (Long) parameters.get(1); + final int before = this.table.size(); + this.table.values().removeIf(expiresAt -> expiresAt < cutoff); + return before - this.table.size(); + } + + if (DatabaseAssertionReplayStore.INSERT.equals(sql)) { + + if (this.failInsert) { + throw new SQLException("connection refused"); + } + return null == this.table.putIfAbsent((String) parameters.get(1), (Long) parameters.get(2)) ? 1 : 0; + } + + throw new SQLException("unexpected statement: " + sql); + } +} diff --git a/src/test/java/com/dotcms/saml/service/impl/TestLoginStartHost.java b/src/test/java/com/dotcms/saml/service/impl/TestLoginStartHost.java new file mode 100644 index 0000000..31abe09 --- /dev/null +++ b/src/test/java/com/dotcms/saml/service/impl/TestLoginStartHost.java @@ -0,0 +1,82 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.IdentityProviderConfiguration; +import com.dotcms.saml.SamlAuthenticationService; +import com.dotcms.saml.utils.EndpointHostRedirect; +import org.junit.Assert; +import org.junit.Test; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * A login started on a site alias is moved to the Service Provider Endpoint Hostname before any AuthnRequest + * (and request-binding cookie) is issued, so the cookie is set on the host the IdP posts back to. + */ +public class TestLoginStartHost { + + private static final String CONFIG_ID = "48190c8c-42c4-46af-8d1a-0cd5db894797"; + + @Test + public void loginStartedOnAnAliasIsRedirectedToTheEndpointHostBeforeTheAuthnRequest() { + + final SamlAuthenticationService authenticationService = new SamlServiceBuilderImpl().buildAuthenticationService( + new MockIdentityProviderConfigurationFactory(), null, new MockMessageObserver(), + new MockSamlConfigurationService()); + final List redirects = new ArrayList<>(); + final List setCookies = new ArrayList<>(); + + authenticationService.authentication(request("alias.example.org"), response(redirects, setCookies), idp(), null); + + Assert.assertEquals(Collections.singletonList("https://dotcms.example.com/dotsaml/login/" + CONFIG_ID + + "?" + EndpointHostRedirect.MARKER_PARAMETER + "=1"), redirects); + Assert.assertTrue("no AuthnRequest or request cookie on the alias host", setCookies.isEmpty()); + } + + private static HttpServletRequest request(final String serverName) { + + return (HttpServletRequest) Proxy.newProxyInstance(TestLoginStartHost.class.getClassLoader(), + new Class[]{HttpServletRequest.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getServerName": return serverName; + case "getRequestURI": return "/dotAdmin/"; + default: return null; + } + }); + } + + private static HttpServletResponse response(final List redirects, final List setCookies) { + + return (HttpServletResponse) Proxy.newProxyInstance(TestLoginStartHost.class.getClassLoader(), + new Class[]{HttpServletResponse.class}, + (proxy, method, args) -> { + if ("sendRedirect".equals(method.getName())) { + redirects.add((String) args[0]); + } else if ("addHeader".equals(method.getName()) && "Set-Cookie".equals(args[0])) { + setCookies.add((String) args[1]); + } + return null; + }); + } + + private static IdentityProviderConfiguration idp() { + + return (IdentityProviderConfiguration) Proxy.newProxyInstance(TestLoginStartHost.class.getClassLoader(), + new Class[]{IdentityProviderConfiguration.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getId": return CONFIG_ID; + case "getIdpName": return "Test IdP"; + case "getSpEndpointHostname": return "dotcms.example.com"; + case "isEnabled": return true; + case "containsOptionalProperty": return false; + default: return null; + } + }); + } +} diff --git a/src/test/java/com/dotcms/saml/service/impl/TestSamlResponseValidator.java b/src/test/java/com/dotcms/saml/service/impl/TestSamlResponseValidator.java new file mode 100644 index 0000000..73c1713 --- /dev/null +++ b/src/test/java/com/dotcms/saml/service/impl/TestSamlResponseValidator.java @@ -0,0 +1,542 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.IdentityProviderConfiguration; +import com.dotcms.saml.service.external.MetaData; +import com.dotcms.saml.service.external.SamlConstants; +import com.dotcms.saml.service.external.SamlException; +import com.dotcms.saml.service.init.SamlInitializer; +import com.dotcms.saml.service.internal.EndpointService; +import com.dotcms.saml.service.internal.MetaDataService; +import com.dotcms.saml.service.internal.MetaDescriptorService; +import com.dotcms.saml.utils.AuthnRequestStateCookie; +import org.joda.time.DateTime; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.opensaml.core.xml.XMLObjectBuilderFactory; +import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; +import org.opensaml.saml.saml2.core.Assertion; +import org.opensaml.saml.saml2.core.Audience; +import org.opensaml.saml.saml2.core.AudienceRestriction; +import org.opensaml.saml.saml2.core.AuthnStatement; +import org.opensaml.saml.saml2.core.Conditions; +import org.opensaml.saml.saml2.core.Issuer; +import org.opensaml.saml.saml2.core.NameID; +import org.opensaml.saml.saml2.core.Response; +import org.opensaml.saml.saml2.core.Subject; +import org.opensaml.saml.saml2.core.SubjectConfirmation; +import org.opensaml.saml.saml2.core.SubjectConfirmationData; +import org.opensaml.security.credential.Credential; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.xml.namespace.QName; +import java.lang.reflect.Proxy; +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +/** + * Covers the SAML 2.0 Web Browser SSO profile checks done by {@link SamlResponseValidator}. + */ +public class TestSamlResponseValidator { + + private static final String IDP_ENTITY_ID = "https://idp.example.com/metadata"; + private static final String SP_ENTITY_ID = "https://dotcms.example.com/dotAdmin"; + private static final String CONFIG_ID = "48190c8c-42c4-46af-8d1a-0cd5db894797"; + private static final String ACS_URL = "https://dotcms.example.com/dotsaml/login/" + CONFIG_ID; + // real time: the replay cache storage expires entries against the system clock + private static final long NOW = System.currentTimeMillis(); + + private static XMLObjectBuilderFactory builderFactory; + + private Map optionalProperties; + private SamlResponseValidator validator; + + @BeforeClass + public static void initOpenSaml() { + + new SamlInitializer().init(Collections.emptyMap()); + builderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory(); + } + + @Before + public void setUp() { + + this.optionalProperties = new HashMap<>(); + this.validator = newValidator(NOW); + } + + @Test + public void validSolicitedResponseIsAccepted() { + + final String requestId = newId(); + final List setCookies = new ArrayList<>(); + + this.validator.validate(response(requestId), assertion(requestId), requestWithState(requestId), + responseCapturing(setCookies), idp()); + + Assert.assertTrue("the outstanding request must be cleared", + setCookies.stream().anyMatch(cookie -> cookie.startsWith(AuthnRequestStateCookie.COOKIE_PREFIX + CONFIG_ID + "=;") + && cookie.contains("Max-Age=0"))); + } + + @Test + public void apiV1AssertionConsumerUrlIsAccepted() { + + final String requestId = newId(); + final String apiAcsUrl = "https://dotcms.example.com/api/v1/dotsaml/login/" + CONFIG_ID; + final Response response = response(requestId); + response.setDestination(apiAcsUrl); + final Assertion assertion = assertion(requestId); + assertion.getSubject().getSubjectConfirmations().get(0).getSubjectConfirmationData().setRecipient(apiAcsUrl); + + this.validator.validate(response, assertion, requestWithState(requestId), responseCapturing(new ArrayList<>()), idp()); + } + + @Test + public void replayedAssertionIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + this.validator.validate(response(requestId), assertion, requestWithState(requestId), responseCapturing(new ArrayList<>()), idp()); + + assertRejected("has already been used", () -> this.validator.validate(response(requestId), assertion, + requestWithState(requestId), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void unavailableReplayStoreRejectsTheResponse() { + + final SamlResponseValidator failingStore = new SamlResponseValidator(endpointService(), metaDataService(), + new MockSamlConfigurationService(), new MockMessageObserver(), + (key, expiresAt) -> { throw new SamlException("database down"); }, + Clock.fixed(Instant.ofEpochMilli(NOW), ZoneOffset.UTC)); + final String requestId = newId(); + + assertRejected("Could not check whether the SAML Assertion", () -> failingStore.validate(response(requestId), + assertion(requestId), requestWithState(requestId), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void assertionForAnotherServiceProviderIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getConditions().getAudienceRestrictions().get(0).getAudiences().get(0) + .setAudienceURI("https://partner-app.example.com"); + + assertRejected("AudienceRestriction does not include this service provider", () -> validate(requestId, assertion)); + } + + @Test + public void everyAudienceRestrictionMustIncludeTheServiceProvider() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getConditions().getAudienceRestrictions().add(audienceRestriction("https://partner-app.example.com")); + + assertRejected("AudienceRestriction does not include this service provider", () -> validate(requestId, assertion)); + } + + @Test + public void assertionWithoutAudienceRestrictionIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getConditions().getAudienceRestrictions().clear(); + + assertRejected("no AudienceRestriction", () -> validate(requestId, assertion)); + } + + @Test + public void assertionWithoutConditionsIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.setConditions(null); + + assertRejected("has no Conditions", () -> validate(requestId, assertion)); + } + + @Test + public void expiredConditionsAreRejectedEvenWhenTheResponseIssueInstantIsFresh() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + // the assertion's own validity window applies whatever the Response IssueInstant says + assertion.getConditions().setNotOnOrAfter(new DateTime(NOW - 60_000)); + + assertRejected("expired at", () -> validate(requestId, assertion)); + } + + @Test + public void notYetValidConditionsAreRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getConditions().setNotBefore(new DateTime(NOW + 60_000)); + + assertRejected("not valid before", () -> validate(requestId, assertion)); + } + + @Test + public void expiredSubjectConfirmationIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + confirmationData(assertion).setNotOnOrAfter(new DateTime(NOW - 60_000)); + + assertRejected("no valid bearer SubjectConfirmation", () -> validate(requestId, assertion)); + } + + @Test + public void subjectConfirmationWithoutNotOnOrAfterIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + confirmationData(assertion).setNotOnOrAfter(null); + + assertRejected("no NotOnOrAfter", () -> validate(requestId, assertion)); + } + + @Test + public void subjectConfirmationForAnotherRecipientIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + confirmationData(assertion).setRecipient("https://partner-app.example.com/acs"); + + assertRejected("is not this service provider's assertion consumer URL", () -> validate(requestId, assertion)); + } + + @Test + public void nonBearerSubjectConfirmationIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getSubject().getSubjectConfirmations().get(0).setMethod(SubjectConfirmation.METHOD_SENDER_VOUCHES); + + assertRejected("is not bearer", () -> validate(requestId, assertion)); + } + + @Test + public void wrongDestinationIsRejected() { + + final String requestId = newId(); + final Response response = response(requestId); + response.setDestination("https://partner-app.example.com/acs"); + + assertRejected("Destination", () -> this.validator.validate(response, assertion(requestId), + requestWithState(requestId), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void assertionFromAnotherIssuerIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getIssuer().setValue("https://other-idp.example.com"); + + assertRejected("is not the configured IdP", () -> validate(requestId, assertion)); + } + + @Test + public void assertionWithoutIssuerIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.setIssuer(null); + + assertRejected("Assertion has no Issuer", () -> validate(requestId, assertion)); + } + + @Test + public void responseFromAnotherIssuerIsRejected() { + + final String requestId = newId(); + final Response response = response(requestId); + response.getIssuer().setValue("https://other-idp.example.com"); + + assertRejected("Response Issuer", () -> this.validator.validate(response, assertion(requestId), + requestWithState(requestId), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void responseNotStartedFromThisBrowserIsRejected() { + + final String requestId = newId(); + + assertRejected("does not match an authentication request started from this browser", + () -> this.validator.validate(response(requestId), assertion(requestId), + requestWithState(newId()), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void requestStateForAnotherIdpConfigurationIsRejected() { + + final String requestId = newId(); + // the request was recorded for a different IdP configuration + final HttpServletRequest request = request(new Cookie( + AuthnRequestStateCookie.COOKIE_PREFIX + "8a7d5e23-da1e-420a-b4f0-471e7da8ea2d", stateEntry(requestId))); + + assertRejected("does not match an authentication request started from this browser", + () -> this.validator.validate(response(requestId), assertion(requestId), request, + responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void unsolicitedResponseIsRejectedByDefault() { + + assertRejected("Unsolicited SAML Responses", () -> this.validator.validate(response(null), assertion(null), + request(), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void unsolicitedResponseIsAcceptedWhenAllowed() { + + this.optionalProperties.put(SamlConstants.ALLOW_UNSOLICITED_RESPONSES, "true"); + + this.validator.validate(response(null), assertion(null), request(), responseCapturing(new ArrayList<>()), idp()); + } + + @Test + public void confirmationInResponseToMustMatchTheResponse() { + + this.optionalProperties.put(SamlConstants.ALLOW_UNSOLICITED_RESPONSES, "true"); + final String requestId = newId(); + + // Response without InResponseTo, confirmation answering a request + assertRejected("InResponseTo does not match the SAML Response", () -> this.validator.validate(response(null), + assertion(requestId), request(), responseCapturing(new ArrayList<>()), idp())); + } + + @Test + public void assertionWithoutAuthnStatementIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getAuthnStatements().clear(); + + assertRejected("no AuthnStatement", () -> validate(requestId, assertion)); + } + + @Test + public void endedIdpSessionIsRejected() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + assertion.getAuthnStatements().get(0).setSessionNotOnOrAfter(new DateTime(NOW - 60_000)); + + assertRejected("IdP session", () -> validate(requestId, assertion)); + } + + @Test + public void clockSkewIsTolerated() { + + final String requestId = newId(); + final Assertion assertion = assertion(requestId); + confirmationData(assertion).setNotOnOrAfter(new DateTime(NOW - 500)); // default skew is 1000 ms + + validate(requestId, assertion); + } + + @Test + public void normalizeUrlIgnoresCaseOfHostDefaultPortAndTrailingSlash() { + + Assert.assertEquals(SamlResponseValidator.normalizeUrl("https://dotcms.example.com/dotsaml/login/1"), + SamlResponseValidator.normalizeUrl("HTTPS://DotCMS.Example.com:443/dotsaml/login/1/")); + Assert.assertNotEquals(SamlResponseValidator.normalizeUrl("https://dotcms.example.com/dotsaml/login/1"), + SamlResponseValidator.normalizeUrl("https://dotcms.example.com:8443/dotsaml/login/1")); + Assert.assertNull(SamlResponseValidator.normalizeUrl("javascript:alert(1)")); + Assert.assertNull(SamlResponseValidator.normalizeUrl("/dotsaml/login/1")); + } + + // --------------------------------------------------------------------------------------------------------- + + private void validate(final String requestId, final Assertion assertion) { + + this.validator.validate(response(requestId), assertion, requestWithState(requestId), + responseCapturing(new ArrayList<>()), idp()); + } + + private static void assertRejected(final String expectedReason, final Runnable validation) { + + try { + validation.run(); + Assert.fail("Expected the SAML Response to be rejected: " + expectedReason); + } catch (SamlException e) { + Assert.assertTrue("Unexpected reason: " + e.getMessage(), e.getMessage().contains(expectedReason)); + } + } + + private SamlResponseValidator newValidator(final long now) { + + return new SamlResponseValidator(endpointService(), metaDataService(), new MockSamlConfigurationService(), + new MockMessageObserver(), new InMemoryAssertionReplayStore(), + Clock.fixed(Instant.ofEpochMilli(now), ZoneOffset.UTC)); + } + + private static String newId() { + + return "_" + UUID.randomUUID().toString().replace("-", ""); + } + + private static Response response(final String inResponseTo) { + + final Response response = build(Response.DEFAULT_ELEMENT_NAME); + response.setID(newId()); + response.setIssueInstant(new DateTime(NOW)); + response.setDestination(ACS_URL); + response.setInResponseTo(inResponseTo); + response.setIssuer(issuer(IDP_ENTITY_ID)); + return response; + } + + private static Assertion assertion(final String inResponseTo) { + + final Assertion assertion = build(Assertion.DEFAULT_ELEMENT_NAME); + assertion.setID(newId()); + assertion.setIssueInstant(new DateTime(NOW)); + assertion.setIssuer(issuer(IDP_ENTITY_ID)); + + final NameID nameID = build(NameID.DEFAULT_ELEMENT_NAME); + nameID.setValue("user@example.com"); + + final SubjectConfirmationData data = build(SubjectConfirmationData.DEFAULT_ELEMENT_NAME); + data.setRecipient(ACS_URL); + data.setNotOnOrAfter(new DateTime(NOW + 300_000)); + data.setInResponseTo(inResponseTo); + + final SubjectConfirmation subjectConfirmation = build(SubjectConfirmation.DEFAULT_ELEMENT_NAME); + subjectConfirmation.setMethod(SubjectConfirmation.METHOD_BEARER); + subjectConfirmation.setSubjectConfirmationData(data); + + final Subject subject = build(Subject.DEFAULT_ELEMENT_NAME); + subject.setNameID(nameID); + subject.getSubjectConfirmations().add(subjectConfirmation); + assertion.setSubject(subject); + + final Conditions conditions = build(Conditions.DEFAULT_ELEMENT_NAME); + conditions.setNotBefore(new DateTime(NOW - 5_000)); + conditions.setNotOnOrAfter(new DateTime(NOW + 300_000)); + conditions.getAudienceRestrictions().add(audienceRestriction(SP_ENTITY_ID)); + assertion.setConditions(conditions); + + final AuthnStatement authnStatement = build(AuthnStatement.DEFAULT_ELEMENT_NAME); + authnStatement.setAuthnInstant(new DateTime(NOW)); + authnStatement.setSessionNotOnOrAfter(new DateTime(NOW + 3_600_000)); + assertion.getAuthnStatements().add(authnStatement); + + return assertion; + } + + private static AudienceRestriction audienceRestriction(final String audienceUri) { + + final Audience audience = build(Audience.DEFAULT_ELEMENT_NAME); + audience.setAudienceURI(audienceUri); + final AudienceRestriction audienceRestriction = build(AudienceRestriction.DEFAULT_ELEMENT_NAME); + audienceRestriction.getAudiences().add(audience); + return audienceRestriction; + } + + private static SubjectConfirmationData confirmationData(final Assertion assertion) { + + return assertion.getSubject().getSubjectConfirmations().get(0).getSubjectConfirmationData(); + } + + private static Issuer issuer(final String value) { + + final Issuer issuer = build(Issuer.DEFAULT_ELEMENT_NAME); + issuer.setValue(value); + return issuer; + } + + @SuppressWarnings("unchecked") + private static T build(final QName elementName) { + + return (T) builderFactory.getBuilder(elementName).buildObject(elementName); + } + + private static HttpServletRequest requestWithState(final String requestId) { + + return request(new Cookie(AuthnRequestStateCookie.COOKIE_PREFIX + CONFIG_ID, stateEntry(requestId))); + } + + private static String stateEntry(final String requestId) { + + return requestId + ":" + (System.currentTimeMillis() / 1000L); + } + + private static HttpServletRequest request(final Cookie... cookies) { + + return (HttpServletRequest) Proxy.newProxyInstance(TestSamlResponseValidator.class.getClassLoader(), + new Class[]{HttpServletRequest.class}, + (proxy, method, args) -> "getCookies".equals(method.getName()) ? cookies : null); + } + + private static HttpServletResponse responseCapturing(final List setCookies) { + + return (HttpServletResponse) Proxy.newProxyInstance(TestSamlResponseValidator.class.getClassLoader(), + new Class[]{HttpServletResponse.class}, + (proxy, method, args) -> { + if ("addHeader".equals(method.getName()) && "Set-Cookie".equals(args[0])) { + setCookies.add((String) args[1]); + } + return null; + }); + } + + private IdentityProviderConfiguration idp() { + + final Map properties = this.optionalProperties; + return (IdentityProviderConfiguration) Proxy.newProxyInstance(TestSamlResponseValidator.class.getClassLoader(), + new Class[]{IdentityProviderConfiguration.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getId": return CONFIG_ID; + case "getIdpName": return "Test IdP"; + case "getSpIssuerURL": return SP_ENTITY_ID; + case "getSpEndpointHostname": return "dotcms.example.com"; + case "getSignatureValidationType": return SamlConstants.ASSERTION; + case "isEnabled": return true; + case "containsOptionalProperty": return properties.containsKey((String) args[0]); + case "getOptionalProperty": return properties.get((String) args[0]); + default: return null; + } + }); + } + + private static EndpointService endpointService() { + + return new EndpointService() { + @Override public String getAssertionConsumerEndpoint(final IdentityProviderConfiguration idp) { return ACS_URL; } + @Override public String getSingleLogoutEndpoint(final IdentityProviderConfiguration idp) { return null; } + @Override public String[] getAccessFilterArray(final IdentityProviderConfiguration idp) { return null; } + @Override public String[] getLogoutPathArray(final IdentityProviderConfiguration idp) { return null; } + @Override public String[] getIncludePathArray(final IdentityProviderConfiguration idp) { return null; } + }; + } + + private static MetaDataService metaDataService() { + + final MetaData metaData = new MetaData(IDP_ENTITY_ID, null, Collections.emptyMap(), Collections.emptyMap(), + Collections.emptyList()); + return new MetaDataService() { + @Override public MetaData getMetaData(final IdentityProviderConfiguration idp) { return metaData; } + @Override public MetaDescriptorService getMetaDescriptorService(final IdentityProviderConfiguration idp) { return null; } + @Override public Collection getSigningCredentials(final IdentityProviderConfiguration idp) { return null; } + @Override public String getIdentityProviderDestinationSSOURL(final IdentityProviderConfiguration idp) { return null; } + @Override public String getIdentityProviderDestinationSLOURL(final IdentityProviderConfiguration idp) { return null; } + }; + } +} diff --git a/src/test/java/com/dotcms/saml/service/impl/TestSamlSignatureVerification.java b/src/test/java/com/dotcms/saml/service/impl/TestSamlSignatureVerification.java new file mode 100644 index 0000000..1fb80d3 --- /dev/null +++ b/src/test/java/com/dotcms/saml/service/impl/TestSamlSignatureVerification.java @@ -0,0 +1,311 @@ +package com.dotcms.saml.service.impl; + +import com.dotcms.saml.IdentityProviderConfiguration; +import com.dotcms.saml.service.external.MetaData; +import com.dotcms.saml.service.external.SamlConstants; +import com.dotcms.saml.service.external.SamlException; +import com.dotcms.saml.service.init.SamlInitializer; +import com.dotcms.saml.service.internal.MetaDataService; +import com.dotcms.saml.service.internal.MetaDescriptorService; +import org.joda.time.DateTime; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.opensaml.core.xml.XMLObjectBuilderFactory; +import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; +import org.opensaml.saml.common.SAMLObjectContentReference; +import org.opensaml.saml.saml2.core.Assertion; +import org.opensaml.saml.saml2.core.Issuer; +import org.opensaml.saml.saml2.core.Response; +import org.opensaml.security.credential.Credential; +import org.opensaml.security.credential.CredentialSupport; +import org.opensaml.security.crypto.KeySupport; +import org.opensaml.xmlsec.signature.Signature; +import org.opensaml.xmlsec.signature.support.SignatureConstants; +import org.opensaml.xmlsec.signature.support.Signer; + +import javax.xml.namespace.QName; +import java.lang.reflect.Proxy; +import java.security.KeyPair; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; +import java.util.UUID; + +/** + * Signature rules: an unrecognized validation type fails closed, a required signature must be present, and + * a signature that is present is always verified (it is never rejected just for being there). + */ +public class TestSamlSignatureVerification { + + private static XMLObjectBuilderFactory builderFactory; + private static Credential idpCredential; + private static Credential otherCredential; + + @BeforeClass + public static void initOpenSaml() throws Exception { + + new SamlInitializer().init(Collections.emptyMap()); + builderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory(); + idpCredential = newCredential(); + otherCredential = newCredential(); + } + + @Test + public void unrecognizedValidationTypesFailClosed() { + + for (final String configured : new String[]{"none", "None", "", null, "signature"}) { + + Assert.assertEquals("type '" + configured + "'", SamlConstants.RESPONSE_AND_ASSERTION, + CredentialServiceImpl.resolveSignatureValidationType(idp(configured))); + } + + Assert.assertEquals(SamlConstants.RESPONSE_AND_ASSERTION, + CredentialServiceImpl.resolveSignatureValidationType(idp("responseAndAssertion "))); + Assert.assertEquals(SamlConstants.ASSERTION, CredentialServiceImpl.resolveSignatureValidationType(idp(" Assertion"))); + Assert.assertEquals(SamlConstants.RESPONSE, CredentialServiceImpl.resolveSignatureValidationType(idp("response"))); + } + + @Test + public void signatureChecksCanNotBeTurnedOff() { + + // MockSamlConfigurationService answers false for every boolean, i.e. verify.signature.* = false + final CredentialServiceImpl credentialService = new CredentialServiceImpl(new MockSamlConfigurationService()); + + Assert.assertTrue(credentialService.isVerifySignatureCredentialsNeeded(idp(SamlConstants.ASSERTION))); + Assert.assertTrue(credentialService.isVerifySignatureProfileNeeded(idp(SamlConstants.ASSERTION))); + } + + @Test + public void unsignedResponseAndAssertionAreRejectedWhenValidationTypeIsNone() { + + final SamlCoreServiceImpl samlCoreService = samlCoreService(); + final IdentityProviderConfiguration idp = idp("none"); + + assertRejected("must be signed", () -> samlCoreService.verifyResponseSignature(response(), idp)); + assertRejected("must be signed", () -> samlCoreService.verifyAssertionSignature(assertion(), idp)); + } + + @Test + public void unsignedAssertionIsRejectedWhenItsSignatureIsRequired() { + + assertRejected("must be signed", + () -> samlCoreService().verifyAssertionSignature(assertion(), idp(SamlConstants.ASSERTION))); + } + + @Test + public void unsignedAssertionIsAcceptedWhenTheResponseSignatureCoversIt() { + + samlCoreService().verifyAssertionSignature(assertion(), idp(SamlConstants.RESPONSE)); + } + + @Test + public void validAssertionSignatureIsAccepted() throws Exception { + + samlCoreService().verifyAssertionSignature(signedAssertion(idpCredential), idp(SamlConstants.ASSERTION)); + } + + @Test + public void presentAssertionSignatureIsVerifiedEvenWhenNotRequired() throws Exception { + + // previously a signed assertion was rejected outright in "response" mode; now it is verified + samlCoreService().verifyAssertionSignature(signedAssertion(idpCredential), idp(SamlConstants.RESPONSE)); + + final Assertion signedWithAnotherKey = signedAssertion(otherCredential); + assertRejected("Signature cannot be validated", + () -> samlCoreService().verifyAssertionSignature(signedWithAnotherKey, idp(SamlConstants.RESPONSE))); + } + + @Test + public void assertionSignedWithAnotherKeyIsRejected() throws Exception { + + final Assertion assertion = signedAssertion(otherCredential); + + assertRejected("Signature cannot be validated", + () -> samlCoreService().verifyAssertionSignature(assertion, idp(SamlConstants.ASSERTION))); + } + + @Test + public void sha1SignatureIsRejectedByDefault() throws Exception { + + final Assertion assertion = signedAssertion(idpCredential, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1, SignatureConstants.ALGO_ID_DIGEST_SHA1); + + assertRejected("signature algorithm '" + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1 + "' is not allowed", + () -> samlCoreService().verifyAssertionSignature(assertion, idp(SamlConstants.ASSERTION))); + } + + @Test + public void sha1DigestIsRejectedByDefault() throws Exception { + + final Assertion assertion = signedAssertion(idpCredential, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256, SignatureConstants.ALGO_ID_DIGEST_SHA1); + + assertRejected("digest algorithm '" + SignatureConstants.ALGO_ID_DIGEST_SHA1 + "' is not allowed", + () -> samlCoreService().verifyAssertionSignature(assertion, idp(SamlConstants.ASSERTION))); + } + + @Test + public void sha1IsAcceptedWhenAllowed() throws Exception { + + final Assertion assertion = signedAssertion(idpCredential, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1, SignatureConstants.ALGO_ID_DIGEST_SHA1); + + samlCoreService().verifyAssertionSignature(assertion, + idp(SamlConstants.ASSERTION, Collections.singletonMap(SamlConstants.ALLOW_SHA1_SIGNATURES, "true"))); + } + + @Test + public void strongerRsaAlgorithmsAreAccepted() throws Exception { + + samlCoreService().verifyAssertionSignature(signedAssertion(idpCredential, + SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA512, SignatureConstants.ALGO_ID_DIGEST_SHA384), + idp(SamlConstants.ASSERTION)); + } + + @Test + public void hmacSignatureIsRejectedEvenWhenSha1IsAllowed() throws Exception { + + final Credential hmacCredential = CredentialSupport.getSimpleCredential( + KeySupport.generateKey("HmacSHA256", 256, null)); + final Assertion assertion = signedAssertion(hmacCredential, + SignatureConstants.ALGO_ID_MAC_HMAC_SHA256, SignatureConstants.ALGO_ID_DIGEST_SHA256); + + assertRejected("signature algorithm '" + SignatureConstants.ALGO_ID_MAC_HMAC_SHA256 + "' is not allowed", + () -> samlCoreService().verifyAssertionSignature(assertion, + idp(SamlConstants.ASSERTION, Collections.singletonMap(SamlConstants.ALLOW_SHA1_SIGNATURES, "true")))); + } + + @Test + public void responseWithMoreThanOneAssertionIsRejected() { + + final Response response = response(); + response.getAssertions().add(assertion()); + response.getAssertions().add(assertion()); + + assertRejected("exactly one unencrypted assertion", + () -> samlCoreService().getAssertion(response, idp(SamlConstants.ASSERTION))); + } + + @Test + public void responseWithoutAssertionIsRejected() { + + assertRejected("exactly one unencrypted assertion", + () -> samlCoreService().getAssertion(response(), idp(SamlConstants.ASSERTION))); + } + + // --------------------------------------------------------------------------------------------------------- + + private static void assertRejected(final String expectedReason, final Runnable verification) { + + try { + verification.run(); + Assert.fail("Expected a rejection: " + expectedReason); + } catch (SamlException e) { + Assert.assertTrue("Unexpected reason: " + e.getMessage(), e.getMessage().contains(expectedReason)); + } + } + + private static SamlCoreServiceImpl samlCoreService() { + + final MockSamlConfigurationService configurationService = new MockSamlConfigurationService(); + return new SamlCoreServiceImpl(new CredentialServiceImpl(configurationService), null, + metaDataService(), new MockMessageObserver(), configurationService, null); + } + + private static Credential newCredential() throws Exception { + + final KeyPair keyPair = KeySupport.generateKeyPair("RSA", 2048, null); + return CredentialSupport.getSimpleCredential(keyPair.getPublic(), keyPair.getPrivate()); + } + + private static Response response() { + + final Response response = build(Response.DEFAULT_ELEMENT_NAME); + response.setID(newId()); + response.setIssueInstant(new DateTime()); + return response; + } + + private static Assertion assertion() { + + final Assertion assertion = build(Assertion.DEFAULT_ELEMENT_NAME); + assertion.setID(newId()); + assertion.setIssueInstant(new DateTime()); + final Issuer issuer = build(Issuer.DEFAULT_ELEMENT_NAME); + issuer.setValue("https://idp.example.com/metadata"); + assertion.setIssuer(issuer); + return assertion; + } + + private static Assertion signedAssertion(final Credential signingCredential) throws Exception { + + return signedAssertion(signingCredential, SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256, + SignatureConstants.ALGO_ID_DIGEST_SHA256); + } + + private static Assertion signedAssertion(final Credential signingCredential, final String signatureAlgorithm, + final String digestAlgorithm) throws Exception { + + final Assertion assertion = assertion(); + final Signature signature = build(Signature.DEFAULT_ELEMENT_NAME); + signature.setSigningCredential(signingCredential); + signature.setSignatureAlgorithm(signatureAlgorithm); + signature.setCanonicalizationAlgorithm(SignatureConstants.ALGO_ID_C14N_EXCL_OMIT_COMMENTS); + assertion.setSignature(signature); + ((SAMLObjectContentReference) signature.getContentReferences().get(0)).setDigestAlgorithm(digestAlgorithm); + + XMLObjectProviderRegistrySupport.getMarshallerFactory().getMarshaller(assertion).marshall(assertion); + Signer.signObject(signature); + return assertion; + } + + private static String newId() { + + return "_" + UUID.randomUUID().toString().replace("-", ""); + } + + @SuppressWarnings("unchecked") + private static T build(final QName elementName) { + + return (T) builderFactory.getBuilder(elementName).buildObject(elementName); + } + + private static IdentityProviderConfiguration idp(final String signatureValidationType) { + + return idp(signatureValidationType, Collections.emptyMap()); + } + + private static IdentityProviderConfiguration idp(final String signatureValidationType, + final Map optionalProperties) { + + return (IdentityProviderConfiguration) Proxy.newProxyInstance(TestSamlSignatureVerification.class.getClassLoader(), + new Class[]{IdentityProviderConfiguration.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "getId": return "48190c8c-42c4-46af-8d1a-0cd5db894797"; + case "getIdpName": return "Test IdP"; + case "getSpEndpointHostname": return "dotcms.example.com"; + case "getSignatureValidationType": return signatureValidationType; + case "containsOptionalProperty": return optionalProperties.containsKey((String) args[0]); + case "getOptionalProperty": return optionalProperties.get((String) args[0]); + default: return null; + } + }); + } + + private static MetaDataService metaDataService() { + + final MetaData metaData = new MetaData("https://idp.example.com/metadata", null, Collections.emptyMap(), + Collections.emptyMap(), Collections.singletonList(idpCredential)); + return new MetaDataService() { + @Override public MetaData getMetaData(final IdentityProviderConfiguration idp) { return metaData; } + @Override public MetaDescriptorService getMetaDescriptorService(final IdentityProviderConfiguration idp) { return null; } + @Override public Collection getSigningCredentials(final IdentityProviderConfiguration idp) { + return Collections.singletonList(idpCredential); + } + @Override public String getIdentityProviderDestinationSSOURL(final IdentityProviderConfiguration idp) { return null; } + @Override public String getIdentityProviderDestinationSLOURL(final IdentityProviderConfiguration idp) { return null; } + }; + } +} diff --git a/src/test/java/com/dotcms/saml/utils/TestAuthnRequestStateCookie.java b/src/test/java/com/dotcms/saml/utils/TestAuthnRequestStateCookie.java new file mode 100644 index 0000000..9090238 --- /dev/null +++ b/src/test/java/com/dotcms/saml/utils/TestAuthnRequestStateCookie.java @@ -0,0 +1,156 @@ +package com.dotcms.saml.utils; + +import com.dotcms.saml.IdentityProviderConfiguration; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.lang.reflect.Proxy; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicLong; + +/** + * The request-binding cookie: one per IdP configuration, bounded, expiring, single use. + */ +public class TestAuthnRequestStateCookie { + + private static final String CONFIG_ID = "48190c8c-42c4-46af-8d1a-0cd5db894797"; + + private final AtomicLong now = new AtomicLong(1_790_950_000_000L); + private Cookie browserCookie; + + @Before + public void setUp() { + + this.browserCookie = null; + } + + @Test + public void cookieIsHostPrefixedSecureAndScopedToTheIdpConfiguration() { + + final List headers = new ArrayList<>(); + AuthnRequestStateCookie.remember(request(), response(headers), idp(CONFIG_ID), "_request1", this.now::get); + + final String header = headers.get(0); + Assert.assertTrue(header, header.startsWith("__Host-dotsaml_req_" + CONFIG_ID + "=")); + for (final String attribute : new String[]{"; Path=/", "; Secure", "; HttpOnly", "; SameSite=None"}) { + Assert.assertTrue(attribute, header.contains(attribute)); + } + Assert.assertFalse("__Host- cookies must not have a Domain", header.contains("Domain")); + } + + @Test + public void requestStartedInThisBrowserIsAcceptedOnce() { + + remember("_request1"); + + Assert.assertTrue(consume("_request1")); + Assert.assertFalse("a request can only be answered once", consume("_request1")); + } + + @Test + public void severalTabsCanHaveOutstandingRequests() { + + remember("_tab1"); + remember("_tab2"); + + Assert.assertTrue(consume("_tab1")); + Assert.assertTrue(consume("_tab2")); + } + + @Test + public void onlyTheMostRecentRequestsAreKept() { + + for (int i = 1; i <= AuthnRequestStateCookie.MAX_OUTSTANDING_REQUESTS + 2; i++) { + remember("_request" + i); + } + + Assert.assertFalse("the oldest requests are dropped", consume("_request1")); + Assert.assertFalse(consume("_request2")); + Assert.assertTrue(consume("_request" + (AuthnRequestStateCookie.MAX_OUTSTANDING_REQUESTS + 2))); + Assert.assertTrue(this.browserCookie.getValue().split("\\.").length < AuthnRequestStateCookie.MAX_OUTSTANDING_REQUESTS); + } + + @Test + public void expiredRequestIsRejected() { + + remember("_request1"); + this.now.addAndGet((AuthnRequestStateCookie.getMaxAgeSeconds(idp(CONFIG_ID)) + 1) * 1000L); + + Assert.assertFalse(consume("_request1")); + } + + @Test + public void requestForAnotherIdpConfigurationIsRejected() { + + remember("_request1"); + + Assert.assertFalse(AuthnRequestStateCookie.consume(request(), response(new ArrayList<>()), + idp("8a7d5e23-da1e-420a-b4f0-471e7da8ea2d"), "_request1", this.now::get)); + } + + @Test + public void malformedCookieValuesAreIgnored() { + + this.browserCookie = new Cookie(AuthnRequestStateCookie.COOKIE_PREFIX + CONFIG_ID, + "_bad id:1.