Ensure that an encrypted assertion is signed if response is not signed (#355) (#46929)

Closes CVE-2026-2092

Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
Ricardo Martin
2026-03-09 10:25:27 +01:00
committed by GitHub
parent 08ae2bebaa
commit b40a25908d
4 changed files with 150 additions and 54 deletions
@@ -29,7 +29,6 @@ import java.util.Objects;
import java.util.Set;
import javax.xml.crypto.dsig.XMLSignature;
import javax.xml.datatype.XMLGregorianCalendar;
import javax.xml.namespace.QName;
import org.keycloak.adapters.saml.AbstractInitiateLogin;
import org.keycloak.adapters.saml.AdapterConstants;
@@ -70,7 +69,6 @@ import org.keycloak.saml.SAML2AuthnRequestBuilder;
import org.keycloak.saml.SAMLRequestParser;
import org.keycloak.saml.SignatureAlgorithm;
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.ProcessingException;
@@ -81,7 +79,6 @@ import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
import org.keycloak.saml.processing.core.util.RedirectBindingSignatureUtil;
import org.keycloak.saml.processing.core.util.XMLEncryptionUtil;
import org.keycloak.saml.processing.web.util.PostBindingUtil;
import org.keycloak.saml.validators.ConditionsValidator;
import org.keycloak.saml.validators.DestinationValidator;
@@ -377,8 +374,15 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
} else if (!isSuccessfulSamlResponse(responseType) || responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) {
return failed(createAuthChallenge403(responseType));
}
Element assertionElement = null;
boolean isAssertionEncrypted = false;
try {
assertion = AssertionUtil.getAssertion(responseHolder, responseType, deployment.getDecryptionKey());
isAssertionEncrypted = AssertionUtil.isAssertionEncrypted(responseType);
assertionElement = isAssertionEncrypted
? AssertionUtil.decryptAssertion(responseType, deployment.getDecryptionKey())
: AssertionUtil.getAssertionElement(responseHolder);
assertion = responseType.getAssertions().get(0).getAssertion();
ConditionsValidator.Builder cvb = new ConditionsValidator.Builder(assertion.getID(), assertion.getConditions(), destinationValidator);
SubjectConfirmationDataValidator.Builder scdvb = new SubjectConfirmationDataValidator.Builder(assertion.getID(), getSubjectConfirmationData(assertion), destinationValidator)
.clockSkewInMillis(deployment.getIDP().getAllowedClockSkew());
@@ -403,10 +407,11 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
return failed(CHALLENGE_EXTRACTION_FAILURE);
}
Element assertionElement = null;
if (deployment.getIDP().getSingleSignOnService().validateAssertionSignature()) {
if (deployment.getIDP().getSingleSignOnService().validateAssertionSignature()
|| (deployment.getIDP().getSingleSignOnService().validateResponseSignature()
&& postBinding && isAssertionEncrypted
&& !AssertionUtil.isSignedElement(responseHolder.getSamlDocument().getDocumentElement()))) {
try {
assertionElement = getAssertionFromResponse(responseHolder);
if (!AssertionUtil.isSignatureValid(assertionElement, deployment.getIDP().getSignatureValidationKeyLocator())) {
log.error("Failed to verify saml assertion signature");
return failed(CHALLENGE_INVALID_SIGNATURE);
@@ -492,10 +497,6 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
URI nameFormat = subjectNameID == null ? null : subjectNameID.getFormat();
String nameFormatString = nameFormat == null ? JBossSAMLURIConstants.NAMEID_FORMAT_UNSPECIFIED.get() : nameFormat.toString();
if (deployment.isKeepDOMAssertion() && assertionElement == null) {
// obtain the assertion from the response to add the DOM document to the principal
assertionElement = getAssertionFromResponseNoException(responseHolder);
}
final SamlPrincipal principal = new SamlPrincipal(assertion,
deployment.isKeepDOMAssertion()? getAssertionDocumentFromElement(assertionElement) : null,
principalName, principalName, nameFormatString, attributes, friendlyAttributes);
@@ -569,28 +570,6 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
&& Objects.equals(status.getStatusCode().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_AUTHNFAILED.get());
}
private Element getAssertionFromResponse(final SAMLDocumentHolder responseHolder) throws ConfigurationException, ProcessingException {
Element encryptedAssertion = DocumentUtil.getElement(responseHolder.getSamlDocument(), new QName(JBossSAMLConstants.ENCRYPTED_ASSERTION.get()));
if (encryptedAssertion != null) {
// encrypted assertion.
// We'll need to decrypt it first.
Document encryptedAssertionDocument = DocumentUtil.createDocument();
encryptedAssertionDocument.appendChild(encryptedAssertionDocument.importNode(encryptedAssertion, true));
return XMLEncryptionUtil.decryptElementInDocument(encryptedAssertionDocument, data -> Collections.singletonList(deployment.getDecryptionKey()));
}
return DocumentUtil.getElement(responseHolder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
}
private Element getAssertionFromResponseNoException(final SAMLDocumentHolder responseHolder) {
try {
return getAssertionFromResponse(responseHolder);
} catch (ConfigurationException|ProcessingException e) {
log.warn("Cannot obtain DOM assertion element", e);
return null;
}
}
private Document getAssertionDocumentFromElement(final Element assertionElement) {
if (assertionElement == null) {
return null;
@@ -23,7 +23,6 @@ import java.security.PublicKey;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import javax.xml.datatype.XMLGregorianCalendar;
import javax.xml.stream.XMLEventReader;
@@ -71,6 +70,7 @@ import org.keycloak.saml.processing.core.util.XMLSignatureUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
/**
* Utility to deal with assertions
@@ -572,6 +572,30 @@ public class AssertionUtil {
return responseType.getAssertions().get(0).getAssertion();
}
public static Element getAssertionElement(SAMLDocumentHolder holder) throws ProcessingException {
Document doc = holder.getSamlDocument();
Element response = doc.getDocumentElement();
if (!JBossSAMLConstants.RESPONSE__PROTOCOL.getNsUri().get().equals(response.getNamespaceURI())
|| !JBossSAMLConstants.RESPONSE__PROTOCOL.get().equals(response.getLocalName())) {
throw new ProcessingException("No response type.");
}
// get the first assertion in the response
NodeList children = response.getChildNodes();
for(int i = 0; i < children.getLength(); i++) {
Node childNode = children.item(i);
if (childNode instanceof Element) {
Element childElement = (Element) childNode;
if (JBossSAMLConstants.ASSERTION.getNsUri().get().equals(childElement.getNamespaceURI())
&& JBossSAMLConstants.ASSERTION.get().equals(childElement.getLocalName())) {
return childElement;
}
}
}
throw new ProcessingException("No assertion from response.");
}
public static boolean isAssertionEncrypted(ResponseType responseType) throws ProcessingException {
List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
@@ -596,13 +620,17 @@ public class AssertionUtil {
* @return the assertion element as it was decrypted. This can be used in signature verification.
*/
public static Element decryptAssertion(ResponseType responseType, XMLEncryptionUtil.DecryptionKeyLocator decryptionKeyLocator) throws ParsingException, ProcessingException, ConfigurationException {
Element enc = responseType.getAssertions().stream()
.map(ResponseType.RTChoiceType::getEncryptedAssertion)
.filter(Objects::nonNull)
.findFirst()
.map(EncryptedElementType::getEncryptedElement)
.orElseThrow(() -> new ProcessingException("No encrypted assertion found."));
List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
if (assertions.isEmpty()) {
throw new ProcessingException("No encrypted assertion found.");
}
ResponseType.RTChoiceType rtChoiceType = assertions.get(0);
if (rtChoiceType.getEncryptedAssertion() == null || rtChoiceType.getEncryptedAssertion().getEncryptedElement() == null) {
throw new ProcessingException("No encrypted assertion found.");
}
Element enc = rtChoiceType.getEncryptedAssertion().getEncryptedElement();
String oldID = enc.getAttribute(JBossSAMLConstants.ID.get());
Document newDoc = DocumentUtil.createDocument();
Node importedNode = newDoc.importNode(enc, true);
@@ -34,7 +34,6 @@ import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import javax.xml.crypto.dsig.XMLSignature;
import javax.xml.namespace.QName;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.FormParam;
@@ -101,11 +100,9 @@ import org.keycloak.rotation.KeyLocator;
import org.keycloak.saml.SAML2LogoutResponseBuilder;
import org.keycloak.saml.SAMLRequestParser;
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.ProcessingException;
import org.keycloak.saml.common.util.DocumentUtil;
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;
@@ -258,6 +255,7 @@ public class SAMLEndpoint {
protected abstract String getBindingType();
protected abstract boolean containsUnencryptedSignature(SAMLDocumentHolder documentHolder);
protected abstract boolean isMessageFullySigned(SAMLDocumentHolder documentHolder);
protected abstract void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException;
protected abstract SAMLDocumentHolder extractRequestDocument(String samlRequest);
protected abstract SAMLDocumentHolder extractResponseDocument(String response);
@@ -578,7 +576,7 @@ public class SAMLEndpoint {
} else {
/* We verify the assertion using original document to handle cases where the IdP
includes whitespace and/or newlines inside tags. */
assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
assertionElement = AssertionUtil.getAssertionElement(holder);
}
// Validate the response Issuer
@@ -604,7 +602,7 @@ public class SAMLEndpoint {
boolean signed = AssertionUtil.isSignedElement(assertionElement);
final boolean assertionSignatureNotExistsWhenRequired = config.isWantAssertionsSigned() && !signed;
final boolean signatureNotValid = signed && config.isValidateSignature() && !AssertionUtil.isSignatureValid(assertionElement, getIDPKeyLocator());
final boolean hasNoSignatureWhenRequired = ! signed && config.isValidateSignature() && ! containsUnencryptedSignature(holder);
final boolean hasNoSignatureWhenRequired = !signed && config.isValidateSignature() && !isMessageFullySigned(holder);
if (assertionSignatureNotExistsWhenRequired || signatureNotValid || hasNoSignatureWhenRequired) {
logger.error("validation failed");
@@ -840,6 +838,11 @@ public class SAMLEndpoint {
return (nl != null && nl.getLength() > 0);
}
@Override
protected boolean isMessageFullySigned(SAMLDocumentHolder documentHolder) {
return AssertionUtil.isSignedElement(documentHolder.getSamlDocument().getDocumentElement());
}
@Override
protected void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException {
if ((! containsUnencryptedSignature(documentHolder)) && (documentHolder.getSamlObject() instanceof ResponseType)) {
@@ -879,14 +882,17 @@ public class SAMLEndpoint {
return algorithm != null && signature != null;
}
@Override
protected boolean isMessageFullySigned(SAMLDocumentHolder documentHolder) {
return containsUnencryptedSignature(documentHolder);
}
@Override
protected void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException {
KeyLocator locator = getIDPKeyLocator();
SamlProtocolUtils.verifyRedirectSignature(documentHolder, locator, session.getContext().getUri(), key);
}
@Override
protected SAMLDocumentHolder extractRequestDocument(String samlRequest) {
return SAMLRequestParser.parseRequestRedirectBinding(samlRequest, maxInflatingSize);
@@ -906,20 +912,31 @@ public class SAMLEndpoint {
protected class ArtifactBinding extends Binding {
private boolean unencryptedSignaturesVerified = false;
// artifact binding is processed twice, first with the art and then with the response, vars to store the first pass
private Boolean containsUnencryptedSignature = null;
private Boolean messageFullySigned = null;
private Boolean verified = null;
@Override
protected boolean containsUnencryptedSignature(SAMLDocumentHolder documentHolder) {
if (unencryptedSignaturesVerified) {
return true;
if (containsUnencryptedSignature != null) {
return containsUnencryptedSignature;
}
NodeList nl = documentHolder.getSamlDocument().getElementsByTagNameNS(XMLSignature.XMLNS, "Signature");
return (nl != null && nl.getLength() > 0);
containsUnencryptedSignature = (nl != null && nl.getLength() > 0);
messageFullySigned = AssertionUtil.isSignedElement(documentHolder.getSamlDocument().getDocumentElement());
return containsUnencryptedSignature;
}
@Override
protected boolean isMessageFullySigned(SAMLDocumentHolder documentHolder) {
containsUnencryptedSignature(documentHolder);
return messageFullySigned;
}
@Override
protected void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException {
if (unencryptedSignaturesVerified) {
if (verified != null) {
// this is the second pass and signatures were already verified in the artifact response time
return;
}
@@ -938,13 +955,14 @@ public class SAMLEndpoint {
}
}
SamlProtocolUtils.verifyDocumentSignature(documentHolder.getSamlDocument(), getIDPKeyLocator());
unencryptedSignaturesVerified = true; // mark signatures as verified
verified = true;
}
@Override
protected SAMLDocumentHolder extractRequestDocument(String samlRequest) {
throw new UnsupportedOperationException("SAML request is not compliant with Artifact binding");
}
@Override
protected SAMLDocumentHolder extractResponseDocument(String response) {
byte[] samlBytes = PostBindingUtil.base64Decode(response);
@@ -17,25 +17,39 @@
package org.keycloak.testsuite.adapter.servlet;
import java.net.URI;
import java.security.PublicKey;
import java.util.List;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import javax.crypto.spec.SecretKeySpec;
import javax.xml.XMLConstants;
import javax.xml.crypto.dsig.XMLSignature;
import javax.xml.namespace.QName;
import jakarta.ws.rs.core.Response.Status;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
import org.keycloak.broker.saml.SAMLIdentityProviderFactory;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.saml.SamlConfigAttributes;
import org.keycloak.protocol.saml.SamlProtocol;
import org.keycloak.representations.idm.KeysMetadataRepresentation;
import org.keycloak.representations.idm.KeysMetadataRepresentation.KeyMetadataRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.saml.RandomSecret;
import org.keycloak.saml.common.constants.JBossSAMLConstants;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
import org.keycloak.saml.processing.core.util.XMLEncryptionUtil;
import org.keycloak.testsuite.adapter.AbstractAdapterTest;
import org.keycloak.testsuite.adapter.page.SAMLServlet;
import org.keycloak.testsuite.adapter.page.SalesPostAssertionAndResponseSig;
import org.keycloak.testsuite.adapter.page.SalesPostEncServlet;
import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
import org.keycloak.testsuite.saml.AbstractSamlTest;
import org.keycloak.testsuite.updaters.Creator;
import org.keycloak.testsuite.util.ClientBuilder;
import org.keycloak.testsuite.util.IdentityProviderBuilder;
@@ -49,6 +63,7 @@ import org.keycloak.testsuite.util.UserBuilder;
import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.xml.security.encryption.XMLCipher;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.graphene.page.Page;
import org.jboss.shrinkwrap.api.spec.WebArchive;
@@ -201,10 +216,37 @@ public class SamlSignatureTest extends AbstractAdapterTest {
evilAssertion.setAttribute("ID", "_evil_assertion_ID");
document.getDocumentElement().insertBefore(evilAssertion, assertion);
}
public static void encryptedAssertionWithoutSignature(Document document, PublicKey publicKey) {
// remove the signature for the whole response
removeDocumentSignature(document);
// get the assertion signed
Element assertion = (Element) document.getElementsByTagNameNS(ASSERTION_NSURI.get(), "Assertion").item(0);
// create a second assertion and encrypt it, put it first
Element evilAssertion = (Element) assertion.cloneNode(true);
evilAssertion.setAttributeNS(XMLConstants.XMLNS_ATTRIBUTE_NS_URI, "xmlns:" + assertion.getPrefix(), JBossSAMLURIConstants.ASSERTION_NSURI.get());
Element signature = (Element) evilAssertion.getElementsByTagNameNS(XMLSignature.XMLNS, "Signature").item(0);
evilAssertion.removeChild(signature);
document.getDocumentElement().insertBefore(evilAssertion, assertion);
final int keySize = XMLEncryptionUtil.getKeyLengthFromURI(XMLCipher.AES_256_GCM);
try {
XMLEncryptionUtil.encryptElement(
new QName(JBossSAMLURIConstants.ASSERTION_NSURI.get(), JBossSAMLConstants.ASSERTION.get(), assertion.getPrefix()),
document, publicKey,
new SecretKeySpec(RandomSecret.createRandomSecret(keySize/8), XMLEncryptionUtil.getJCEKeyAlgorithmFromURI(XMLCipher.AES_256_GCM)),
keySize,
new QName(JBossSAMLURIConstants.ASSERTION_NSURI.get(), JBossSAMLConstants.ENCRYPTED_ASSERTION.get(), assertion.getPrefix()),
true);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}
@Page
private SalesPostAssertionAndResponseSig salesPostAssertionAndResponseSigPage;
@Page
private SalesPostEncServlet salesPostEncPage;
private UserRepresentation user;
@@ -213,6 +255,11 @@ public class SamlSignatureTest extends AbstractAdapterTest {
return samlServletDeployment(SalesPostAssertionAndResponseSig.DEPLOYMENT_NAME, SendUsernameServlet.class);
}
@Deployment(name = SalesPostEncServlet.DEPLOYMENT_NAME)
protected static WebArchive salesPostEnc() {
return samlServletDeployment(SalesPostEncServlet.DEPLOYMENT_NAME, SendUsernameServlet.class);
}
@Override
protected boolean isImportAfterEachMethod() {
return false;
@@ -235,6 +282,9 @@ public class SamlSignatureTest extends AbstractAdapterTest {
final ClientBuilder salesPostClient = signingSamlClient(APP_CLIENT_ID)
.baseUrl("http://localhost:8080/sales-post-assertion-and-response-sig")
.redirectUris("http://localhost:8080/sales-post-assertion-and-response-sig/*");
final ClientBuilder salesPostEncClient = signingSamlClient(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST_ENC)
.baseUrl("http://localhost:8080/sales-post-enc")
.redirectUris("http://localhost:8080/sales-post-enc/*");
final String brokerBaseUrl = getAuthServerRoot() + "realms/" + BROKER;
final ClientBuilder brokerRealmIdPClient = signingSamlClient(brokerBaseUrl)
.baseUrl(brokerBaseUrl + "/broker/" + REALM_NAME + "/endpoint")
@@ -245,6 +295,7 @@ public class SamlSignatureTest extends AbstractAdapterTest {
.publicKey(REALM_PUBLIC_KEY)
.privateKey(REALM_PRIVATE_KEY)
.client(salesPostClient)
.client(salesPostEncClient)
.client(brokerRealmIdPClient)
.roles(RolesBuilder.create().realmRole(REQUIRED_ROLE))
.build()
@@ -300,8 +351,12 @@ public class SamlSignatureTest extends AbstractAdapterTest {
}
private void testSamlResponseModificationsClient(Consumer<Document> samlResponseModifier, Consumer<CloseableHttpResponse> assertions) {
testSamlResponseModificationsClient(salesPostAssertionAndResponseSigPage, samlResponseModifier, assertions);
}
private void testSamlResponseModificationsClient(SAMLServlet page, Consumer<Document> samlResponseModifier, Consumer<CloseableHttpResponse> assertions) {
new SamlClientBuilder()
.navigateTo(salesPostAssertionAndResponseSigPage)
.navigateTo(page)
.processSamlResponse(Binding.POST).build()
.login().user(user).build()
.processSamlResponse(Binding.POST).transformDocument(d -> { samlResponseModifier.accept(d); return d; }).build()
@@ -405,4 +460,20 @@ public class SamlSignatureTest extends AbstractAdapterTest {
public void testNoDocumentSignatureOnlyOneAssertionSignedBelowResponse() throws Exception {
testSamlResponseModifications(XSWHelpers::noDocumentSignatureOnlyOneAssertionSignedBelowResponse, false);
}
@Test
public void testEncryptedAssertionWithoutSignature() throws Exception {
KeysMetadataRepresentation keysMetadata = adminClient.realm(REALM_NAME).keys().getKeyMetadata();
String kid = keysMetadata.getActive().get("RSA-OAEP");
KeyMetadataRepresentation keyMetadata = keysMetadata.getKeys().stream()
.filter(k -> kid.equals(k.getKid())).findAny().orElse(null);
PublicKey realmPubKey = KeycloakModelUtils.getPublicKey(keyMetadata.getPublicKey());
PublicKey clientPubKey = KeycloakModelUtils.getPublicKey(AbstractSamlTest.SAML_CLIENT_SALES_POST_ENC_PUBLIC_KEY);
testSamlResponseModificationsClient(salesPostEncPage,
d -> XSWHelpers.encryptedAssertionWithoutSignature(d, clientPubKey),
SamlSignatureTest::assertUserAccessDenied);
testSamlResponseModificationsBroker(d -> XSWHelpers.encryptedAssertionWithoutSignature(d, realmPubKey),
SamlSignatureTest::assertNotUpdateProfilePage);
}
}