From 78ace3a750c0285884b52646d4926052023ebd00 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 13 Jun 2024 11:30:33 +0530 Subject: [PATCH] saml: introduce saml2.check.signature (#9219) Adminstrators should ensure that IDP configuration has signing certificate for the actual signature check to be performed. In addition to this, this change introduces a new global setting `saml2.check.signature` which can deliberately fail a SAML login attempt when the SAML response has missing signature. Signed-off-by: Rohit Yadav --- .../SAML2LoginAPIAuthenticatorCmd.java | 15 +++++++-- .../cloudstack/saml/SAML2AuthManager.java | 33 ++++++++++--------- .../cloudstack/saml/SAML2AuthManagerImpl.java | 2 +- .../SAML2LoginAPIAuthenticatorCmdTest.java | 24 ++++++++++++++ 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index 6bb3e788a95..fd4da7a59b2 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -144,6 +144,14 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent return responseObject; } + protected void checkAndFailOnMissingSAMLSignature(Signature signature) { + if (signature == null && SAML2AuthManager.SAMLCheckSignature.value()) { + s_logger.error("Failing SAML login due to missing signature in the SAML response and signature check is enforced. " + + "Please check and ensure the IDP configuration has signing certificate or relax the saml2.check.signature setting."); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Signature is missing from the SAML Response. Please contact the Administrator"); + } + } + @Override public String authenticate(final String command, final Map params, final HttpSession session, final InetAddress remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException { try { @@ -225,6 +233,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent session.setAttribute(SAMLPluginConstants.SAML_IDPID, issuer.getValue()); Signature sig = processedSAMLResponse.getSignature(); + checkAndFailOnMissingSAMLSignature(sig); if (idpMetadata.getSigningCertificate() != null && sig != null) { BasicX509Credential credential = new BasicX509Credential(); credential.setEntityCertificate(idpMetadata.getSigningCertificate()); @@ -238,9 +247,8 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent params, responseType)); } } - if (username == null) { - username = SAMLUtils.getValueFromAssertions(processedSAMLResponse.getAssertions(), SAML2AuthManager.SAMLUserAttributeName.value()); - } + + username = SAMLUtils.getValueFromAssertions(processedSAMLResponse.getAssertions(), SAML2AuthManager.SAMLUserAttributeName.value()); for (Assertion assertion: processedSAMLResponse.getAssertions()) { if (assertion!= null && assertion.getSubject() != null && assertion.getSubject().getNameID() != null) { @@ -272,6 +280,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent continue; } Signature encSig = assertion.getSignature(); + checkAndFailOnMissingSAMLSignature(encSig); if (idpMetadata.getSigningCertificate() != null && encSig != null) { BasicX509Credential sigCredential = new BasicX509Credential(); sigCredential.setEntityCertificate(idpMetadata.getSigningCertificate()); diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java index e52a7e32695..a5dae36581c 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java @@ -25,51 +25,54 @@ import java.util.Collection; public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableService { - public static final ConfigKey SAMLIsPluginEnabled = new ConfigKey("Advanced", Boolean.class, "saml2.enabled", "false", + ConfigKey SAMLIsPluginEnabled = new ConfigKey("Advanced", Boolean.class, "saml2.enabled", "false", "Indicates whether SAML SSO plugin is enabled or not", true); - public static final ConfigKey SAMLServiceProviderID = new ConfigKey("Advanced", String.class, "saml2.sp.id", "org.apache.cloudstack", + ConfigKey SAMLServiceProviderID = new ConfigKey("Advanced", String.class, "saml2.sp.id", "org.apache.cloudstack", "SAML2 Service Provider Identifier String", true); - public static final ConfigKey SAMLServiceProviderContactPersonName = new ConfigKey("Advanced", String.class, "saml2.sp.contact.person", "CloudStack Developers", + ConfigKey SAMLServiceProviderContactPersonName = new ConfigKey("Advanced", String.class, "saml2.sp.contact.person", "CloudStack Developers", "SAML2 Service Provider Contact Person Name", true); - public static final ConfigKey SAMLServiceProviderContactEmail = new ConfigKey("Advanced", String.class, "saml2.sp.contact.email", "dev@cloudstack.apache.org", + ConfigKey SAMLServiceProviderContactEmail = new ConfigKey("Advanced", String.class, "saml2.sp.contact.email", "dev@cloudstack.apache.org", "SAML2 Service Provider Contact Email Address", true); - public static final ConfigKey SAMLServiceProviderOrgName = new ConfigKey("Advanced", String.class, "saml2.sp.org.name", "Apache CloudStack", + ConfigKey SAMLServiceProviderOrgName = new ConfigKey("Advanced", String.class, "saml2.sp.org.name", "Apache CloudStack", "SAML2 Service Provider Organization Name", true); - public static final ConfigKey SAMLServiceProviderOrgUrl = new ConfigKey("Advanced", String.class, "saml2.sp.org.url", "http://cloudstack.apache.org", + ConfigKey SAMLServiceProviderOrgUrl = new ConfigKey("Advanced", String.class, "saml2.sp.org.url", "http://cloudstack.apache.org", "SAML2 Service Provider Organization URL", true); - public static final ConfigKey SAMLServiceProviderSingleSignOnURL = new ConfigKey("Advanced", String.class, "saml2.sp.sso.url", "http://localhost:8080/client/api?command=samlSso", + ConfigKey SAMLServiceProviderSingleSignOnURL = new ConfigKey("Advanced", String.class, "saml2.sp.sso.url", "http://localhost:8080/client/api?command=samlSso", "SAML2 CloudStack Service Provider Single Sign On URL", true); - public static final ConfigKey SAMLServiceProviderSingleLogOutURL = new ConfigKey("Advanced", String.class, "saml2.sp.slo.url", "http://localhost:8080/client/", + ConfigKey SAMLServiceProviderSingleLogOutURL = new ConfigKey("Advanced", String.class, "saml2.sp.slo.url", "http://localhost:8080/client/", "SAML2 CloudStack Service Provider Single Log Out URL", true); - public static final ConfigKey SAMLCloudStackRedirectionUrl = new ConfigKey("Advanced", String.class, "saml2.redirect.url", "http://localhost:8080/client", + ConfigKey SAMLCloudStackRedirectionUrl = new ConfigKey("Advanced", String.class, "saml2.redirect.url", "http://localhost:8080/client", "The CloudStack UI url the SSO should redirected to when successful", true); - public static final ConfigKey SAMLUserAttributeName = new ConfigKey("Advanced", String.class, "saml2.user.attribute", "uid", + ConfigKey SAMLUserAttributeName = new ConfigKey("Advanced", String.class, "saml2.user.attribute", "uid", "Attribute name to be looked for in SAML response that will contain the username", true); - public static final ConfigKey SAMLIdentityProviderMetadataURL = new ConfigKey("Advanced", String.class, "saml2.idp.metadata.url", "https://openidp.feide.no/simplesaml/saml2/idp/metadata.php", + ConfigKey SAMLIdentityProviderMetadataURL = new ConfigKey("Advanced", String.class, "saml2.idp.metadata.url", "https://openidp.feide.no/simplesaml/saml2/idp/metadata.php", "SAML2 Identity Provider Metadata XML Url", true); - public static final ConfigKey SAMLDefaultIdentityProviderId = new ConfigKey("Advanced", String.class, "saml2.default.idpid", "https://openidp.feide.no", + ConfigKey SAMLDefaultIdentityProviderId = new ConfigKey("Advanced", String.class, "saml2.default.idpid", "https://openidp.feide.no", "The default IdP entity ID to use only in case of multiple IdPs", true); - public static final ConfigKey SAMLSignatureAlgorithm = new ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1", + ConfigKey SAMLSignatureAlgorithm = new ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1", "The algorithm to use to when signing a SAML request. Default is SHA1, allowed algorithms: SHA1, SHA256, SHA384, SHA512", true, ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select, "SHA1,SHA256,SHA384,SHA512"); - public static final ConfigKey SAMLAppendDomainSuffix = new ConfigKey("Advanced", Boolean.class, "saml2.append.idpdomain", "false", + ConfigKey SAMLAppendDomainSuffix = new ConfigKey("Advanced", Boolean.class, "saml2.append.idpdomain", "false", "If enabled, create account/user dialog with SAML SSO enabled will append the IdP domain to the user or account name in the UI dialog", true); - public static final ConfigKey SAMLTimeout = new ConfigKey("Advanced", Integer.class, "saml2.timeout", "1800", + ConfigKey SAMLTimeout = new ConfigKey("Advanced", Integer.class, "saml2.timeout", "1800", "SAML2 IDP Metadata refresh interval in seconds, minimum value is set to 300", true); + ConfigKey SAMLCheckSignature = new ConfigKey("Advanced", Boolean.class, "saml2.check.signature", "false", + "Whether SAML2 signature must be checked, when enforced and when the SAML response does not have a signature would lead to login exception", true); + public SAMLProviderMetadata getSPMetadata(); public SAMLProviderMetadata getIdPMetadata(String entityId); public Collection getAllIdPMetadata(); diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java index ba85b151eea..3ecebf4d185 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java @@ -535,6 +535,6 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage SAMLServiceProviderSingleSignOnURL, SAMLServiceProviderSingleLogOutURL, SAMLCloudStackRedirectionUrl, SAMLUserAttributeName, SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId, - SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout}; + SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature}; } } diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java index 39c8c231bf0..ebdc5be8fe9 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java @@ -271,6 +271,30 @@ public class SAML2LoginAPIAuthenticatorCmdTest { verifyTestWhenFailToAuthenticateThrowExceptionOrRedirectToUrl(false, hasThrownServerApiException, 0, 0); } + private void overrideDefaultConfigValue(final ConfigKey configKey, final String name, final Object o) throws IllegalAccessException, NoSuchFieldException { + Field f = ConfigKey.class.getDeclaredField(name); + f.setAccessible(true); + f.set(configKey, o); + } + + @Test + public void testFailOnSAMLSignatureCheckWhenFalse() throws NoSuchFieldException, IllegalAccessException { + overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_defaultValue", "false"); + SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd(); + try { + cmd.checkAndFailOnMissingSAMLSignature(null); + } catch(Exception e) { + Assert.fail("This shouldn't throw any exception"); + } + } + + @Test(expected = ServerApiException.class) + public void testFailOnSAMLSignatureCheckWhenTrue() throws NoSuchFieldException, IllegalAccessException { + overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_defaultValue", "true"); + SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd(); + cmd.checkAndFailOnMissingSAMLSignature(null); + } + private UserAccountVO configureTestWhenFailToAuthenticateThrowExceptionOrRedirectToUrl(String entity, String configurationValue, Boolean isUserAuthorized) throws IOException { Mockito.when(samlAuthManager.isUserAuthorized(nullable(Long.class), nullable(String.class))).thenReturn(isUserAuthorized);