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

KEYCLOAK-17935 SAML Client - Validate InResponseTo attribute

Browse source
This commit is contained in:
Luca Leonardo Scorcia 2021-05-30 05:04:43 -04:00 committed by Hynek Mlnařík
parent 13f7831a77
commit f5123cb51b
3 changed files with 168 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -31,6 +31,8 @@ import org.keycloak.dom.saml.v2.assertion.AttributeStatementType;
import org.keycloak.dom.saml.v2.assertion.AttributeType;
import org.keycloak.dom.saml.v2.assertion.AuthnStatementType;
import org.keycloak.dom.saml.v2.assertion.NameIDType;
import org.keycloak.dom.saml.v2.assertion.SubjectConfirmationDataType;
import org.keycloak.dom.saml.v2.assertion.SubjectConfirmationType;
import org.keycloak.dom.saml.v2.assertion.SubjectType;
import org.keycloak.dom.saml.v2.protocol.LogoutRequestType;
import org.keycloak.dom.saml.v2.protocol.RequestAbstractType;
@ -442,6 +444,16 @@ public class SAMLEndpoint {
assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
}
// Validate InResponseTo attribute: must match the generated request ID
String expectedRequestId = authSession.getClientNote(SamlProtocol.SAML_REQUEST_ID);
final boolean inResponseToValidationSuccess = validateInResponseToAttribute(responseType, expectedRequestId);
if (!inResponseToValidationSuccess)
{
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.error(Errors.INVALID_SAML_RESPONSE);
return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.INVALID_REQUESTER);
}
boolean signed = AssertionUtil.isSignedElement(assertionElement);
final boolean assertionSignatureNotExistsWhenRequired = config.isWantAssertionsSigned() && !signed;
final boolean signatureNotValid = signed && config.isValidateSignature() && !AssertionUtil.isSignatureValid(assertionElement, getIDPKeyLocator());
@ -782,4 +794,65 @@ public class SAMLEndpoint {
SubjectType.STSubType subType = subject.getSubType();
return subType != null ? (NameIDType) subType.getBaseID() : null;
}
private boolean validateInResponseToAttribute(ResponseType responseType, String expectedRequestId) {
// If we are not expecting a request ID, don't bother
if (expectedRequestId == null || expectedRequestId.isEmpty())
return true;
// We are expecting a request ID so we are in SP-initiated login, attribute InResponseTo must be present
if (responseType.getInResponseTo() == null) {
logger.error("Response Validation Error: InResponseTo attribute was expected but not present in received response");
return false;
}
// Attribute is present, proceed with validation
// 1) Attribute Response > InResponseTo must not be empty
String responseInResponseToValue = responseType.getInResponseTo();
if (responseInResponseToValue.isEmpty()) {
logger.error("Response Validation Error: InResponseTo attribute was expected but it is empty in received response");
return false;
}
// 2) Attribute Response > InResponseTo must match request ID
if (!responseInResponseToValue.equals(expectedRequestId)) {
logger.error("Response Validation Error: received InResponseTo attribute does not match the expected request ID");
return false;
}
// If present, Assertion > Subject > Confirmation > SubjectConfirmationData > InResponseTo must also be validated
if (responseType.getAssertions().isEmpty())
return true;
SubjectType subjectElement = responseType.getAssertions().get(0).getAssertion().getSubject();
if (subjectElement != null) {
if (subjectElement.getConfirmation() != null && !subjectElement.getConfirmation().isEmpty())
{
SubjectConfirmationType subjectConfirmationElement = subjectElement.getConfirmation().get(0);
if (subjectConfirmationElement != null) {
SubjectConfirmationDataType subjectConfirmationDataElement = subjectConfirmationElement.getSubjectConfirmationData();
if (subjectConfirmationDataElement != null) {
if (subjectConfirmationDataElement.getInResponseTo() != null) {
// 3) Assertion > Subject > Confirmation > SubjectConfirmationData > InResponseTo is empty
String subjectConfirmationDataInResponseToValue = subjectConfirmationDataElement.getInResponseTo();
if (subjectConfirmationDataInResponseToValue.isEmpty()) {
logger.error("Response Validation Error: SubjectConfirmationData InResponseTo attribute was expected but it is empty in received response");
return false;
}
// 4) Assertion > Subject > Confirmation > SubjectConfirmationData > InResponseTo does not match request ID
if (!subjectConfirmationDataInResponseToValue.equals(expectedRequestId)) {
logger.error("Response Validation Error: received SubjectConfirmationData InResponseTo attribute does not match the expected request ID");
return false;
}
}
}
}
}
}
return true;
}
}

4
services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
View file

@ -42,6 +42,7 @@ import org.keycloak.models.RealmModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.saml.JaxrsSAML2BindingBuilder;
import org.keycloak.protocol.saml.SamlProtocol;
import org.keycloak.protocol.saml.SamlService;
import org.keycloak.protocol.saml.SamlSessionUtils;
import org.keycloak.protocol.saml.preprocessor.SamlAuthenticationPreprocessor;
@ -170,6 +171,9 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
destinationUrl = authnRequest.getDestination().toString();
}
// Save the current RequestID in the Auth Session as we need to verify it against the ID returned from the IdP
request.getAuthenticationSession().setClientNote(SamlProtocol.SAML_REQUEST_ID, authnRequest.getID());
if (postBinding) {
return binding.postBinding(authnRequestBuilder.toDocument()).request(destinationUrl);
} else {

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

@ -22,6 +22,7 @@ import org.keycloak.saml.common.exceptions.ConfigurationException;
import org.keycloak.saml.common.exceptions.ParsingException;
import org.keycloak.saml.common.exceptions.ProcessingException;
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.util.SamlClient;
@ -33,16 +34,19 @@ import java.util.HashMap;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.ws.rs.core.Response;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertThat;
import static org.keycloak.testsuite.saml.RoleMapperTest.ROLE_ATTRIBUTE_NAME;
import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
import static org.keycloak.testsuite.util.Matchers.statusCodeIsHC;
import static org.keycloak.testsuite.util.SamlStreams.assertionsUnencrypted;
import static org.keycloak.testsuite.util.SamlStreams.attributeStatements;
import static org.keycloak.testsuite.util.SamlStreams.attributesUnecrypted;
@ -337,4 +341,91 @@ public final class KcSamlBrokerTest extends AbstractAdvancedBrokerTest {
assertThat(attributeValues, hasItems(EMPTY_ATTRIBUTE_ROLE));
}
// KEYCLOAK-17935
@Test
public void loginInResponseToMismatch() throws Exception {
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
.transformDocument(this::tamperInResponseTo)
.build()
.execute(hr -> assertThat(hr, statusCodeIsHC(Response.Status.BAD_REQUEST))); // Response from consumer IdP
}
// KEYCLOAK-17935
@Test
public void loginInResponseToMissing() throws Exception {
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
.transformDocument(this::removeInResponseTo)
.build()
.execute(hr -> assertThat(hr, statusCodeIsHC(Response.Status.BAD_REQUEST))); // Response from consumer IdP
}
// KEYCLOAK-17935
@Test
public void loginInResponseToEmpty() throws Exception {
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
.transformDocument(this::clearInResponseTo)
.build()
.execute(hr -> assertThat(hr, statusCodeIsHC(Response.Status.BAD_REQUEST))); // Response from consumer IdP
}
private Document tamperInResponseTo(Document orig) {
Element rootElement = orig.getDocumentElement();
rootElement.setAttribute(SAMLProtocolQNames.ATTR_IN_RESPONSE_TO.getQName().getLocalPart(), "TAMPERED_" + rootElement.getAttribute("InResponseTo"));
return orig;
}
private Document removeInResponseTo(Document orig) {
Element rootElement = orig.getDocumentElement();
rootElement.removeAttribute(SAMLProtocolQNames.ATTR_IN_RESPONSE_TO.getQName().getLocalPart());
return orig;
}
private Document clearInResponseTo(Document orig) {
Element rootElement = orig.getDocumentElement();
rootElement.setAttribute(SAMLProtocolQNames.ATTR_IN_RESPONSE_TO.getQName().getLocalPart(), "");
return orig;
}
}