libre.sh/keycloak-scim
4
0
Fork 0
Code Issues 15 Pull requests Releases 1 Activity Actions

Fix #10982 SAML Client - Introduce SAML Issuer validation

Browse source
This commit is contained in:
Luca Leonardo Scorcia 2022-03-29 08:39:42 -04:00 committed by Hynek Mlnařík
parent c462468577
commit 27650ab816
7 changed files with 114 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
View file

@ -444,6 +444,17 @@ public class SAMLEndpoint {
assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
}
// Validate the response Issuer
final String responseIssuer = responseType.getIssuer() != null ? responseType.getIssuer().getValue(): null;
final boolean responseIssuerValidationSuccess = config.getIdpEntityId() == null ||
(responseIssuer != null && responseIssuer.equals(config.getIdpEntityId()));
if (!responseIssuerValidationSuccess) {
logger.errorf("Response Issuer validation failed: expected %s, actual %s", config.getIdpEntityId(), responseIssuer);
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.error(Errors.INVALID_SAML_RESPONSE);
return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.INVALID_REQUESTER);
}
// Validate InResponseTo attribute: must match the generated request ID
String expectedRequestId = authSession.getClientNote(SamlProtocol.SAML_REQUEST_ID_BROKER);
final boolean inResponseToValidationSuccess = validateInResponseToAttribute(responseType, expectedRequestId);
@ -470,7 +481,20 @@ public class SAMLEndpoint {
// This methods writes the parsed and decrypted id back on the responseType parameter:
AssertionUtil.decryptId(responseType, keys.getPrivateKey());
}
AssertionType assertion = responseType.getAssertions().get(0).getAssertion();
// Validate the assertion Issuer
final String assertionIssuer = assertion.getIssuer() != null ? assertion.getIssuer().getValue(): null;
final boolean assertionIssuerValidationSuccess = config.getIdpEntityId() == null ||
(assertionIssuer != null && assertionIssuer.equals(config.getIdpEntityId()));
if (!assertionIssuerValidationSuccess) {
logger.errorf("Assertion Issuer validation failed: expected %s, actual %s", config.getIdpEntityId(), assertionIssuer);
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.error(Errors.INVALID_SAML_RESPONSE);
return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.INVALID_REQUESTER);
}
NameIDType subjectNameID = getSubjectNameID(assertion);
String principal = getPrincipal(assertion);

9
services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
View file

@ -35,6 +35,7 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
public static final XmlKeyInfoKeyNameTransformer DEFAULT_XML_KEY_INFO_KEY_NAME_TRANSFORMER = XmlKeyInfoKeyNameTransformer.NONE;
public static final String ENTITY_ID = "entityId";
public static final String IDP_ENTITY_ID = "idpEntityId";
public static final String ADD_EXTENSIONS_ELEMENT_WITH_KEY_INFO = "addExtensionsElementWithKeyInfo";
public static final String BACKCHANNEL_SUPPORTED = "backchannelSupported";
public static final String ENCRYPTION_PUBLIC_KEY = "encryptionPublicKey";
@ -78,6 +79,14 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
getConfig().put(ENTITY_ID, entityId);
}
public String getIdpEntityId() {
return getConfig().get(IDP_ENTITY_ID);
}
public void setIdpEntityId(String idpEntityId) {
getConfig().put(IDP_ENTITY_ID, idpEntityId);
}
public String getSingleSignOnServiceUrl() {
return getConfig().get(SINGLE_SIGN_ON_SERVICE_URL);
}

1
services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderFactory.java
View file

@ -123,6 +123,7 @@ public class SAMLIdentityProviderFactory extends AbstractIdentityProviderFactory
}
}
samlIdentityProviderConfig.setIdpEntityId(entityType.getEntityID());
samlIdentityProviderConfig.setSingleLogoutServiceUrl(singleLogoutServiceUrl);
samlIdentityProviderConfig.setSingleSignOnServiceUrl(singleSignOnServiceUrl);
samlIdentityProviderConfig.setWantAuthnRequestsSigned(idpDescriptor.isWantAuthnRequestsSigned());

4
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
View file

@ -924,7 +924,8 @@ public class IdentityProviderTest extends AbstractAdminTest {
"signingCertificate",
"addExtensionsElementWithKeyInfo",
"loginHint",
"hideOnLoginPage"
"hideOnLoginPage",
"idpEntityId"
));
assertThat(config, hasEntry("validateSignature", "true"));
assertThat(config, hasEntry("singleLogoutServiceUrl", "http://localhost:8080/auth/realms/master/protocol/saml"));
@ -935,6 +936,7 @@ public class IdentityProviderTest extends AbstractAdminTest {
assertThat(config, hasEntry("addExtensionsElementWithKeyInfo", "false"));
assertThat(config, hasEntry("nameIDPolicyFormat", "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"));
assertThat(config, hasEntry("hideOnLoginPage", "true"));
assertThat(config, hasEntry("idpEntityId", "http://localhost:8080/auth/realms/master"));
assertThat(config, hasEntry(is("signingCertificate"), notNullValue()));
}

68
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlBrokerTest.java
View file

@ -4,6 +4,7 @@ import org.keycloak.admin.client.resource.IdentityProviderResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.UserResource;
import com.google.common.collect.ImmutableMap;
import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
import org.keycloak.broker.saml.mappers.AttributeToRoleMapper;
import org.keycloak.broker.saml.mappers.UserAttributeMapper;
import org.keycloak.dom.saml.v2.assertion.AssertionType;
@ -25,9 +26,12 @@ import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
import org.keycloak.saml.processing.core.parsers.saml.protocol.SAMLProtocolQNames;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.testsuite.saml.AbstractSamlTest;
import org.keycloak.testsuite.updaters.IdentityProviderAttributeUpdater;
import org.keycloak.testsuite.util.SamlClient;
import org.keycloak.testsuite.util.SamlClient.Binding;
import org.keycloak.testsuite.util.SamlClientBuilder;
import java.io.Closeable;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@ -342,6 +346,70 @@ public final class KcSamlBrokerTest extends AbstractAdvancedBrokerTest {
}
// Issue #10982
@Test
public void loginWithIdpEntityIdCorrect() throws Exception {
// Set the expected IDP Entity ID to the correct value
try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
.setAttribute(SAMLIdentityProviderConfig.IDP_ENTITY_ID, "https://localhost:8543/auth/realms/provider")
.update())
{
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted", getConsumerRoot() + "/sales-post/saml", null);
Document doc = SAML2Request.convert(loginRep);
SAMLDocumentHolder samlResponse = new SamlClientBuilder()
.authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST).build() // Request to consumer IdP
.login().idp(bc.getIDPAlias()).build()
.processSamlResponse(Binding.POST) // AuthnRequest to producer IdP
.targetAttributeSamlRequest()
.build()
.login().user(bc.getUserLogin(), bc.getUserPassword()).build()
.processSamlResponse(Binding.POST) // Response from producer IdP
.build()
// first-broker flow
.updateProfile().firstName("a").lastName("b").email(bc.getUserEmail()).username(bc.getUserLogin()).build()
.followOneRedirect()
.getSamlResponse(Binding.POST); // Response from consumer IdP
Assert.assertThat(samlResponse, Matchers.notNullValue());
Assert.assertThat(samlResponse.getSamlObject(), isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
}
}
// Issue #10982
@Test
public void loginWithIdpEntityIdMismatchResponse() throws Exception {
// Set the expected IDP Entity ID to a wrong value
try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
.setAttribute(SAMLIdentityProviderConfig.IDP_ENTITY_ID, "http://my.custom.idp.entity.id")
.update())
{
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST + ".dot/ted", getConsumerRoot() + "/sales-post/saml", null);
Document doc = SAML2Request.convert(loginRep);
new SamlClientBuilder()
.authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST).build() // Request to consumer IdP
.login().idp(bc.getIDPAlias()).build()
.processSamlResponse(Binding.POST) // AuthnRequest to producer IdP
.targetAttributeSamlRequest()
.build()
.login().user(bc.getUserLogin(), bc.getUserPassword()).build()
.processSamlResponse(Binding.POST) // Response from producer IdP
.build()
.execute(hr -> assertThat(hr, statusCodeIsHC(Response.Status.BAD_REQUEST))); // Response from consumer IdP
}
}
// KEYCLOAK-17935
@Test
public void loginInResponseToMismatch() throws Exception {

2
themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
View file

@ -772,6 +772,8 @@ import-from-file=Import from file
identity-provider.import-from-file.tooltip=Import metadata from a downloaded IDP discovery descriptor.
identity-provider.saml.entity-id=Service Provider Entity ID
identity-provider.saml.entity-id.tooltip=The Entity ID that will be used to uniquely identify this SAML Service Provider
identity-provider.saml.idp-entity-id=Identity Provider Entity ID
identity-provider.saml.idp-entity-id.tooltip=The Entity ID used to validate the Issuer for received SAML assertions. If empty, no Issuer validation is performed.
identity-provider.saml.protocol-endpoints.saml=SAML 2.0 Service Provider Metadata
identity-provider.saml.protocol-endpoints.saml.tooltip=Shows the configuration of the Service Provider endpoint
identity-provider.saml.attribute-consuming-service-index=Attribute Consuming Service Index

7
themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
View file

@ -140,6 +140,13 @@
</div>
<kc-tooltip>{{:: 'identity-provider.saml.entity-id.tooltip' | translate}}</kc-tooltip>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="idpEntityId">{{:: 'identity-provider.saml.idp-entity-id' | translate}}</label>
<div class="col-md-6">
<input kc-no-reserved-chars class="form-control" id="idpEntityId" type="text" ng-model="identityProvider.config.idpEntityId">
</div>
<kc-tooltip>{{:: 'identity-provider.saml.idp-entity-id.tooltip' | translate}}</kc-tooltip>
</div>
<div class="form-group clearfix">
<label class="col-md-2 control-label" for="singleSignOnServiceUrl"><span class="required">*</span> {{:: 'single-signon-service-url' | translate}}</label>
<div class="col-md-6">