External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-09 23:12:06 -03:30
Code Issues Packages Projects Releases Wiki Activity

fix: the assertion is stripped of its signature when it is manipulated during artifact binding resolution

Browse Source 
Signed-off-by: tmorin <git@morin.io>
This commit is contained in:
Thibault Morin 2025-02-06 11:19:59 +01:00 committed by Pedro Igor
parent 4da2d67145
commit 82f9421e0a
5 changed files with 200 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/ArtifactResponseUtil.java Normal file
View File

@ -0,0 +1,90 @@
package org.keycloak.saml.processing.core.saml.v2.util;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import javax.xml.XMLConstants;
import javax.xml.transform.OutputKeys;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import java.io.StringWriter;
import java.util.Optional;
/**
* Utility class to manipulate SAML ArtifactResponse and embedded Response.
*/
public final class ArtifactResponseUtil {
private ArtifactResponseUtil() {
}
/**
* Convert the Document to a string.
* <p>
* The Response shall match the namespace "urn:oasis:names:tc:SAML:2.0:protocol" and the element "Response".
*
* @param document the Document to convert
* @return the Document as a string
*/
public static Optional<String> convertResponseToString(Document document) {
return extractResponseElement(document).map(ArtifactResponseUtil::nodeToString);
}
/**
* Convert a Node to a string.
*
* @param node the Node to convert
* @return the Node as a string
*/
static String nodeToString(Node node) {
try {
// Transform stuff are not thread sage and shall be instantiated each time
final TransformerFactory tf = TransformerFactory.newInstance();
// Secure processing is enabled to avoid XXE attacks
tf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
final Transformer transformer = tf.newTransformer();
transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
final StringWriter writer = new StringWriter();
transformer.transform(new DOMSource(node), new StreamResult(writer));
return writer.getBuffer().toString();
} catch (TransformerException e) {
throw new IllegalStateException("Error converting node to string", e);
}
}
/**
* Extract the Response element from the Document.
*
* @param document the Document to extract the Response element from
* @return the Response element
*/
static Optional<Element> extractResponseElement(Document document) {
// extract from the ArtifactResponse the embedded Response
final NodeList responseNodeList = document.getElementsByTagNameNS(
"urn:oasis:names:tc:SAML:2.0:protocol",
"Response"
);
// leave early if there is no embedded Response
if (responseNodeList.getLength() != 1) {
return Optional.empty();
}
// convert the embedded Response to a string and then to a base64 serialized string
final Node responseNode = responseNodeList.item(0);
// leave early if the response node is not an Element
if (responseNode.getNodeType() != Node.ELEMENT_NODE) {
return Optional.empty();
}
// return the response node as an Element
return Optional.of((Element) responseNode);
}
}

89
saml-core/src/test/java/org/keycloak/saml/processing/core/saml/v2/util/ArtifactResponseUtilTest.java Normal file
View File

@ -0,0 +1,89 @@
package org.keycloak.saml.processing.core.saml.v2.util;
import junit.framework.TestCase;
import org.junit.Assert;
import org.keycloak.saml.common.util.DocumentUtil;
import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import java.io.InputStream;
import java.util.Optional;
public class ArtifactResponseUtilTest extends TestCase {
public void testConvertResponseToString() throws Exception {
InputStream artifactResponseAsInputStream = ArtifactResponseUtilTest.class.getResourceAsStream(
"saml20-artifact-response-assertion-signed.xml"
);
Assert.assertNotNull(artifactResponseAsInputStream);
InputStream expectedResponseAsInputStream = ArtifactResponseUtilTest.class.getResourceAsStream(
"saml20-response-assertion-signed.xml"
);
Assert.assertNotNull(expectedResponseAsInputStream);
String expectedResponseAsString = new String(expectedResponseAsInputStream.readAllBytes());
// transform the InputStream to a SAMLDocumentHolder to get the Document as implemented in org.keycloak.broker.saml.SAMLEndpoint
SAMLDocumentHolder saml2ObjectFromDocument = SAML2Request.getSAML2ObjectFromStream(artifactResponseAsInputStream);
Assert.assertNotNull(saml2ObjectFromDocument);
// the value shall be present
Optional<String> optionalString = ArtifactResponseUtil.convertResponseToString(saml2ObjectFromDocument.getSamlDocument());
Assert.assertTrue(optionalString.isPresent());
// the value shall be equal to the expected value
Assert.assertEquals(expectedResponseAsString, optionalString.get());
}
public void testNodeToString() throws Exception {
String documentAstring = "<foo><bar>VALUE</bar></foo>";
// create a document
Document document = DocumentUtil.getDocument(documentAstring);
Assert.assertNotNull(document);
// transform the document to a string
String transformedDocument = ArtifactResponseUtil.nodeToString(document);
// assert the transformed document is equal to the original document
Assert.assertEquals(documentAstring, transformedDocument);
}
public void testExtractResponseElement() throws Exception {
InputStream artifactResponseAsInputStream = ArtifactResponseUtilTest.class.getResourceAsStream(
"saml20-artifact-response-assertion-signed.xml"
);
Assert.assertNotNull(artifactResponseAsInputStream);
// transform the InputStream to a SAMLDocumentHolder to get the Document as implemented in org.keycloak.broker.saml.SAMLEndpoint
SAMLDocumentHolder saml2ObjectFromDocument = SAML2Request.getSAML2ObjectFromStream(artifactResponseAsInputStream);
// the element shall be present
Optional<Element> optionalElement = ArtifactResponseUtil.extractResponseElement(saml2ObjectFromDocument.getSamlDocument());
Assert.assertTrue(optionalElement.isPresent());
}
public void testExtractResponseElementWhenResponseNotFound() throws Exception {
InputStream artifactResponseAsInputStream = ArtifactResponseUtilTest.class.getResourceAsStream(
"saml20-artifact-response-assertion-signed.xml"
);
Assert.assertNotNull(artifactResponseAsInputStream);
// transform the InputStream to a SAMLDocumentHolder to get the Document as implemented in org.keycloak.broker.saml.SAMLEndpoint
SAMLDocumentHolder saml2ObjectFromDocument = SAML2Request.getSAML2ObjectFromStream(artifactResponseAsInputStream);
// get the Response element and remove it
Node responseNode = saml2ObjectFromDocument.getSamlDocument().getElementsByTagNameNS(
"urn:oasis:names:tc:SAML:2.0:protocol",
"Response"
).item(0);
responseNode.getParentNode().removeChild(responseNode);
// the element shall be absent
Optional<Element> optionalElement = ArtifactResponseUtil.extractResponseElement(saml2ObjectFromDocument.getSamlDocument());
Assert.assertTrue(optionalElement.isEmpty());
}
}

1
saml-core/src/test/resources/org/keycloak/saml/processing/core/saml/v2/util/saml20-artifact-response-assertion-signed.xml Normal file
View File

File diff suppressed because one or more lines are too long

1
saml-core/src/test/resources/org/keycloak/saml/processing/core/saml/v2/util/saml20-response-assertion-signed.xml Normal file
View File

File diff suppressed because one or more lines are too long

27
services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
View File

@ -63,12 +63,11 @@ import org.keycloak.saml.common.constants.GeneralConstants;
import org.keycloak.saml.common.constants.JBossSAMLConstants;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
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.common.util.DocumentUtil;
import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants;
import org.keycloak.saml.processing.core.saml.v2.util.ArtifactResponseUtil;
import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
import org.keycloak.saml.processing.core.util.XMLEncryptionUtil;
import org.keycloak.saml.processing.core.util.XMLSignatureUtil;
@ -121,7 +120,6 @@ import org.keycloak.saml.validators.DestinationValidator;
import org.keycloak.services.util.CacheControlUtil;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.utils.StringUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
@ -471,15 +469,28 @@ public class SAMLEndpoint {
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST);
}
// extract the SAML Response from the original SAML ArtifactResponse
Optional<String> optionalEmbeddedResponseAsString = ArtifactResponseUtil.convertResponseToString(
samlDocumentHolder.getSamlDocument()
);
// leave early if the embedded Response cannot be converted to string
if(optionalEmbeddedResponseAsString.isEmpty()) {
logger.error("artifact binding failed: the embedded Response cannot be converted to string");
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.detail(Details.REASON, Errors.INVALID_SAML_ARTIFACT_RESPONSE);
event.error(Errors.INVALID_REQUEST);
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REQUEST);
}
// convert the embedded SAML response to a base64 serialized string
Document embeddedResponseAsDoc = SAML2Request.convert(embeddedResponse);
String embeddedResponseAsString = DocumentUtil.getDocumentAsString(embeddedResponseAsDoc);
String embeddedResponseAsString = optionalEmbeddedResponseAsString.get();
logger.debugf("embeddedResponseAsString %s", embeddedResponseAsString);
String embeddedResponseAsBase64 = PostBindingUtil.base64Encode(embeddedResponseAsString);
// continue the flow with POST binding
return execute(null, embeddedResponseAsBase64, null, relayState, clientId);
} catch (IOException | ConfigurationException | ProcessingException | ParsingException e) {
} catch (IOException e) {
logger.error("artifact binding failed", e);
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.detail(Details.REASON, Errors.INVALID_SAML_ARTIFACT_RESPONSE);
@ -594,11 +605,11 @@ public class SAMLEndpoint {
final boolean signatureNotValid = signed && config.isValidateSignature() && !AssertionUtil.isSignatureValid(assertionElement, getIDPKeyLocator());
final boolean hasNoSignatureWhenRequired = ! signed && config.isValidateSignature() && ! containsUnencryptedSignature(holder);
if (!isArtifactBinding && (assertionSignatureNotExistsWhenRequired || signatureNotValid || hasNoSignatureWhenRequired)) {
if (assertionSignatureNotExistsWhenRequired || signatureNotValid || hasNoSignatureWhenRequired) {
logger.error("validation failed");
event.event(EventType.IDENTITY_PROVIDER_RESPONSE);
event.error(Errors.INVALID_SIGNATURE);
return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.INVALID_REQUESTER);
return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, Messages.IDENTITY_PROVIDER_INVALID_SIGNATURE);
}
if (AssertionUtil.isIdEncrypted(responseType)) {