This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch 4.18
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.18 by this push:
new e41add31e72 saml: signature check improvements
e41add31e72 is described below
commit e41add31e72928ebbca13fb043189c70fcbcbf8c
Author: Abhishek Kumar <[email protected]>
AuthorDate: Mon Jul 15 17:33:05 2024 +0530
saml: signature check improvements
Adminstrators should ensure that IDP configuration has a signing
certificate for the actual signature check to be performed. In addition to
this, this change introduces a new global setting saml2.check.signature, with
the default value of true, which can deliberately fail a SAML login attempt
when the SAML response has a missing signature.
Purges the SAML token upon handling the first SAML response.
Authored-by: Rohit Yadav <[email protected]>
Signed-off-by: Abhishek Kumar <[email protected]>
---
.../api/command/SAML2LoginAPIAuthenticatorCmd.java | 16 +++++--
.../apache/cloudstack/saml/SAML2AuthManager.java | 50 ++++++++++++----------
.../cloudstack/saml/SAML2AuthManagerImpl.java | 9 +++-
.../command/SAML2LoginAPIAuthenticatorCmdTest.java | 24 +++++++++++
4 files changed, 72 insertions(+), 27 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..332e0602784 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<String,
Object[]> params, final HttpSession session, final InetAddress remoteAddress,
final String responseType, final StringBuilder auditTrailSb, final
HttpServletRequest req, final HttpServletResponse resp) throws
ServerApiException {
try {
@@ -220,11 +228,13 @@ public class SAML2LoginAPIAuthenticatorCmd extends
BaseCmd implements APIAuthent
"Received SAML response for a SSO request that we
may not have made or has expired, please try logging in again",
params, responseType));
}
+ samlAuthManager.purgeToken(token);
// Set IdpId for this session
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 +248,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 +281,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..3a4030f9c0d 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,59 +25,63 @@ import java.util.Collection;
public interface SAML2AuthManager extends PluggableAPIAuthenticator,
PluggableService {
- public static final ConfigKey<Boolean> SAMLIsPluginEnabled = new
ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.enabled", "false",
+ ConfigKey<Boolean> SAMLIsPluginEnabled = new
ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.enabled", "false",
"Indicates whether SAML SSO plugin is enabled or not", true);
- public static final ConfigKey<String> SAMLServiceProviderID = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.id",
"org.apache.cloudstack",
+ ConfigKey<String> SAMLServiceProviderID = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.id",
"org.apache.cloudstack",
"SAML2 Service Provider Identifier String", true);
- public static final ConfigKey<String> SAMLServiceProviderContactPersonName
= new ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.person",
"CloudStack Developers",
+ ConfigKey<String> SAMLServiceProviderContactPersonName = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.person",
"CloudStack Developers",
"SAML2 Service Provider Contact Person Name", true);
- public static final ConfigKey<String> SAMLServiceProviderContactEmail =
new ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.email",
"[email protected]",
+ ConfigKey<String> SAMLServiceProviderContactEmail = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.email",
"[email protected]",
"SAML2 Service Provider Contact Email Address", true);
- public static final ConfigKey<String> SAMLServiceProviderOrgName = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.org.name", "Apache
CloudStack",
+ ConfigKey<String> SAMLServiceProviderOrgName = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.org.name", "Apache
CloudStack",
"SAML2 Service Provider Organization Name", true);
- public static final ConfigKey<String> SAMLServiceProviderOrgUrl = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.org.url",
"http://cloudstack.apache.org";,
+ ConfigKey<String> SAMLServiceProviderOrgUrl = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.org.url",
"http://cloudstack.apache.org";,
"SAML2 Service Provider Organization URL", true);
- public static final ConfigKey<String> SAMLServiceProviderSingleSignOnURL =
new ConfigKey<String>("Advanced", String.class, "saml2.sp.sso.url",
"http://localhost:8080/client/api?command=samlSso";,
+ ConfigKey<String> SAMLServiceProviderSingleSignOnURL = new
ConfigKey<String>("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<String> SAMLServiceProviderSingleLogOutURL =
new ConfigKey<String>("Advanced", String.class, "saml2.sp.slo.url",
"http://localhost:8080/client/";,
+ ConfigKey<String> SAMLServiceProviderSingleLogOutURL = new
ConfigKey<String>("Advanced", String.class, "saml2.sp.slo.url",
"http://localhost:8080/client/";,
"SAML2 CloudStack Service Provider Single Log Out URL", true);
- public static final ConfigKey<String> SAMLCloudStackRedirectionUrl = new
ConfigKey<String>("Advanced", String.class, "saml2.redirect.url",
"http://localhost:8080/client";,
+ ConfigKey<String> SAMLCloudStackRedirectionUrl = new
ConfigKey<String>("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<String> SAMLUserAttributeName = new
ConfigKey<String>("Advanced", String.class, "saml2.user.attribute", "uid",
+ ConfigKey<String> SAMLUserAttributeName = new
ConfigKey<String>("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<String> SAMLIdentityProviderMetadataURL =
new ConfigKey<String>("Advanced", String.class, "saml2.idp.metadata.url",
"https://openidp.feide.no/simplesaml/saml2/idp/metadata.php";,
+ ConfigKey<String> SAMLIdentityProviderMetadataURL = new
ConfigKey<String>("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<String> SAMLDefaultIdentityProviderId = new
ConfigKey<String>("Advanced", String.class, "saml2.default.idpid",
"https://openidp.feide.no";,
+ ConfigKey<String> SAMLDefaultIdentityProviderId = new
ConfigKey<String>("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<String> SAMLSignatureAlgorithm = new
ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1",
+ ConfigKey<String> 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<Boolean> SAMLAppendDomainSuffix = new
ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.append.idpdomain", "false",
+ ConfigKey<Boolean> SAMLAppendDomainSuffix = new
ConfigKey<Boolean>("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<Integer> SAMLTimeout = new
ConfigKey<Integer>("Advanced", Integer.class, "saml2.timeout", "1800",
+ ConfigKey<Integer> SAMLTimeout = new ConfigKey<Integer>("Advanced",
Integer.class, "saml2.timeout", "1800",
"SAML2 IDP Metadata refresh interval in seconds, minimum value is
set to 300", true);
- public SAMLProviderMetadata getSPMetadata();
- public SAMLProviderMetadata getIdPMetadata(String entityId);
- public Collection<SAMLProviderMetadata> getAllIdPMetadata();
+ ConfigKey<Boolean> SAMLCheckSignature = new ConfigKey<Boolean>("Advanced",
Boolean.class, "saml2.check.signature", "true",
+ "When enabled (default and recommended), SAML2 signature checks
are enforced and lack of signature in the SAML SSO response will cause login
exception. Disabling this is not advisable but provided for backward
compatibility for users who are able to accept the risks.", false);
- public boolean isUserAuthorized(Long userId, String entityId);
- public boolean authorizeUser(Long userId, String entityId, boolean enable);
+ SAMLProviderMetadata getSPMetadata();
+ SAMLProviderMetadata getIdPMetadata(String entityId);
+ Collection<SAMLProviderMetadata> getAllIdPMetadata();
- public void saveToken(String authnId, String domain, String entity);
- public SAMLTokenVO getToken(String authnId);
- public void expireTokens();
+ boolean isUserAuthorized(Long userId, String entityId);
+ boolean authorizeUser(Long userId, String entityId, boolean enable);
+
+ void saveToken(String authnId, String domain, String entity);
+ SAMLTokenVO getToken(String authnId);
+ void purgeToken(SAMLTokenVO token);
+ void expireTokens();
}
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..dfa76414fb7 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
@@ -487,6 +487,13 @@ public class SAML2AuthManagerImpl extends AdapterBase
implements SAML2AuthManage
return _samlTokenDao.findByUuid(authnId);
}
+ @Override
+ public void purgeToken(SAMLTokenVO token) {
+ if (token != null) {
+ _samlTokenDao.remove(token.getId());
+ }
+ }
+
@Override
public void expireTokens() {
_samlTokenDao.expireTokens();
@@ -535,6 +542,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..48a3139052d 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,
"_value", 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,
"_value", 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);
