Skip to content

Commit b40a259

Browse files
rmartincrmartinc
authored
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>
1 parent 08ae2be commit b40a259

4 files changed

Lines changed: 150 additions & 54 deletions

File tree

‎adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java‎

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.Set;
3030
import javax.xml.crypto.dsig.XMLSignature;
3131
import javax.xml.datatype.XMLGregorianCalendar;
32-
import javax.xml.namespace.QName;
3332

3433
import org.keycloak.adapters.saml.AbstractInitiateLogin;
3534
import org.keycloak.adapters.saml.AdapterConstants;
@@ -70,7 +69,6 @@
7069
import org.keycloak.saml.SAMLRequestParser;
7170
import org.keycloak.saml.SignatureAlgorithm;
7271
import org.keycloak.saml.common.constants.GeneralConstants;
73-
import org.keycloak.saml.common.constants.JBossSAMLConstants;
7472
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
7573
import org.keycloak.saml.common.exceptions.ConfigurationException;
7674
import org.keycloak.saml.common.exceptions.ProcessingException;
@@ -81,7 +79,6 @@
8179
import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil;
8280
import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
8381
import org.keycloak.saml.processing.core.util.RedirectBindingSignatureUtil;
84-
import org.keycloak.saml.processing.core.util.XMLEncryptionUtil;
8582
import org.keycloak.saml.processing.web.util.PostBindingUtil;
8683
import org.keycloak.saml.validators.ConditionsValidator;
8784
import org.keycloak.saml.validators.DestinationValidator;
@@ -377,8 +374,15 @@ protected AuthOutcome handleLoginResponse(SAMLDocumentHolder responseHolder, boo
377374
} else if (!isSuccessfulSamlResponse(responseType) || responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) {
378375
return failed(createAuthChallenge403(responseType));
379376
}
377+
378+
Element assertionElement = null;
379+
boolean isAssertionEncrypted = false;
380380
try {
381-
assertion = AssertionUtil.getAssertion(responseHolder, responseType, deployment.getDecryptionKey());
381+
isAssertionEncrypted = AssertionUtil.isAssertionEncrypted(responseType);
382+
assertionElement = isAssertionEncrypted
383+
? AssertionUtil.decryptAssertion(responseType, deployment.getDecryptionKey())
384+
: AssertionUtil.getAssertionElement(responseHolder);
385+
assertion = responseType.getAssertions().get(0).getAssertion();
382386
ConditionsValidator.Builder cvb = new ConditionsValidator.Builder(assertion.getID(), assertion.getConditions(), destinationValidator);
383387
SubjectConfirmationDataValidator.Builder scdvb = new SubjectConfirmationDataValidator.Builder(assertion.getID(), getSubjectConfirmationData(assertion), destinationValidator)
384388
.clockSkewInMillis(deployment.getIDP().getAllowedClockSkew());
@@ -403,10 +407,11 @@ protected AuthOutcome handleLoginResponse(SAMLDocumentHolder responseHolder, boo
403407
return failed(CHALLENGE_EXTRACTION_FAILURE);
404408
}
405409

406-
Element assertionElement = null;
407-
if (deployment.getIDP().getSingleSignOnService().validateAssertionSignature()) {
410+
if (deployment.getIDP().getSingleSignOnService().validateAssertionSignature()
411+
|| (deployment.getIDP().getSingleSignOnService().validateResponseSignature()
412+
&& postBinding && isAssertionEncrypted
413+
&& !AssertionUtil.isSignedElement(responseHolder.getSamlDocument().getDocumentElement()))) {
408414
try {
409-
assertionElement = getAssertionFromResponse(responseHolder);
410415
if (!AssertionUtil.isSignatureValid(assertionElement, deployment.getIDP().getSignatureValidationKeyLocator())) {
411416
log.error("Failed to verify saml assertion signature");
412417
return failed(CHALLENGE_INVALID_SIGNATURE);
@@ -492,10 +497,6 @@ protected AuthOutcome handleLoginResponse(SAMLDocumentHolder responseHolder, boo
492497

493498
URI nameFormat = subjectNameID == null ? null : subjectNameID.getFormat();
494499
String nameFormatString = nameFormat == null ? JBossSAMLURIConstants.NAMEID_FORMAT_UNSPECIFIED.get() : nameFormat.toString();
495-
if (deployment.isKeepDOMAssertion() && assertionElement == null) {
496-
// obtain the assertion from the response to add the DOM document to the principal
497-
assertionElement = getAssertionFromResponseNoException(responseHolder);
498-
}
499500
final SamlPrincipal principal = new SamlPrincipal(assertion,
500501
deployment.isKeepDOMAssertion()? getAssertionDocumentFromElement(assertionElement) : null,
501502
principalName, principalName, nameFormatString, attributes, friendlyAttributes);
@@ -569,28 +570,6 @@ private boolean isRetrayableSamlResponse(ResponseType responseType) {
569570
&& Objects.equals(status.getStatusCode().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_AUTHNFAILED.get());
570571
}
571572

572-
private Element getAssertionFromResponse(final SAMLDocumentHolder responseHolder) throws ConfigurationException, ProcessingException {
573-
Element encryptedAssertion = DocumentUtil.getElement(responseHolder.getSamlDocument(), new QName(JBossSAMLConstants.ENCRYPTED_ASSERTION.get()));
574-
if (encryptedAssertion != null) {
575-
// encrypted assertion.
576-
// We'll need to decrypt it first.
577-
Document encryptedAssertionDocument = DocumentUtil.createDocument();
578-
encryptedAssertionDocument.appendChild(encryptedAssertionDocument.importNode(encryptedAssertion, true));
579-
580-
return XMLEncryptionUtil.decryptElementInDocument(encryptedAssertionDocument, data -> Collections.singletonList(deployment.getDecryptionKey()));
581-
}
582-
return DocumentUtil.getElement(responseHolder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
583-
}
584-
585-
private Element getAssertionFromResponseNoException(final SAMLDocumentHolder responseHolder) {
586-
try {
587-
return getAssertionFromResponse(responseHolder);
588-
} catch (ConfigurationException|ProcessingException e) {
589-
log.warn("Cannot obtain DOM assertion element", e);
590-
return null;
591-
}
592-
}
593-
594573
private Document getAssertionDocumentFromElement(final Element assertionElement) {
595574
if (assertionElement == null) {
596575
return null;

‎saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.util.ArrayList;
2424
import java.util.Collections;
2525
import java.util.List;
26-
import java.util.Objects;
2726
import java.util.Set;
2827
import javax.xml.datatype.XMLGregorianCalendar;
2928
import javax.xml.stream.XMLEventReader;
@@ -71,6 +70,7 @@
7170
import org.w3c.dom.Document;
7271
import org.w3c.dom.Element;
7372
import org.w3c.dom.Node;
73+
import org.w3c.dom.NodeList;
7474

7575
/**
7676
* Utility to deal with assertions
@@ -572,6 +572,30 @@ public static AssertionType getAssertion(SAMLDocumentHolder holder, ResponseType
572572
return responseType.getAssertions().get(0).getAssertion();
573573
}
574574

575+
public static Element getAssertionElement(SAMLDocumentHolder holder) throws ProcessingException {
576+
Document doc = holder.getSamlDocument();
577+
Element response = doc.getDocumentElement();
578+
if (!JBossSAMLConstants.RESPONSE__PROTOCOL.getNsUri().get().equals(response.getNamespaceURI())
579+
|| !JBossSAMLConstants.RESPONSE__PROTOCOL.get().equals(response.getLocalName())) {
580+
throw new ProcessingException("No response type.");
581+
}
582+
583+
// get the first assertion in the response
584+
NodeList children = response.getChildNodes();
585+
for(int i = 0; i < children.getLength(); i++) {
586+
Node childNode = children.item(i);
587+
if (childNode instanceof Element) {
588+
Element childElement = (Element) childNode;
589+
if (JBossSAMLConstants.ASSERTION.getNsUri().get().equals(childElement.getNamespaceURI())
590+
&& JBossSAMLConstants.ASSERTION.get().equals(childElement.getLocalName())) {
591+
return childElement;
592+
}
593+
}
594+
}
595+
596+
throw new ProcessingException("No assertion from response.");
597+
}
598+
575599
public static boolean isAssertionEncrypted(ResponseType responseType) throws ProcessingException {
576600
List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
577601

@@ -596,13 +620,17 @@ public static Element decryptAssertion(ResponseType responseType, PrivateKey pri
596620
* @return the assertion element as it was decrypted. This can be used in signature verification.
597621
*/
598622
public static Element decryptAssertion(ResponseType responseType, XMLEncryptionUtil.DecryptionKeyLocator decryptionKeyLocator) throws ParsingException, ProcessingException, ConfigurationException {
599-
Element enc = responseType.getAssertions().stream()
600-
.map(ResponseType.RTChoiceType::getEncryptedAssertion)
601-
.filter(Objects::nonNull)
602-
.findFirst()
603-
.map(EncryptedElementType::getEncryptedElement)
604-
.orElseThrow(() -> new ProcessingException("No encrypted assertion found."));
623+
List<ResponseType.RTChoiceType> assertions = responseType.getAssertions();
624+
if (assertions.isEmpty()) {
625+
throw new ProcessingException("No encrypted assertion found.");
626+
}
627+
628+
ResponseType.RTChoiceType rtChoiceType = assertions.get(0);
629+
if (rtChoiceType.getEncryptedAssertion() == null || rtChoiceType.getEncryptedAssertion().getEncryptedElement() == null) {
630+
throw new ProcessingException("No encrypted assertion found.");
631+
}
605632

633+
Element enc = rtChoiceType.getEncryptedAssertion().getEncryptedElement();
606634
String oldID = enc.getAttribute(JBossSAMLConstants.ID.get());
607635
Document newDoc = DocumentUtil.createDocument();
608636
Node importedNode = newDoc.importNode(enc, true);

‎services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import java.util.function.Predicate;
3535
import java.util.stream.Collectors;
3636
import javax.xml.crypto.dsig.XMLSignature;
37-
import javax.xml.namespace.QName;
3837

3938
import jakarta.ws.rs.Consumes;
4039
import jakarta.ws.rs.FormParam;
@@ -101,11 +100,9 @@
101100
import org.keycloak.saml.SAML2LogoutResponseBuilder;
102101
import org.keycloak.saml.SAMLRequestParser;
103102
import org.keycloak.saml.common.constants.GeneralConstants;
104-
import org.keycloak.saml.common.constants.JBossSAMLConstants;
105103
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
106104
import org.keycloak.saml.common.exceptions.ConfigurationException;
107105
import org.keycloak.saml.common.exceptions.ProcessingException;
108-
import org.keycloak.saml.common.util.DocumentUtil;
109106
import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
110107
import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants;
111108
import org.keycloak.saml.processing.core.saml.v2.util.ArtifactResponseUtil;
@@ -258,6 +255,7 @@ protected Response basicChecks(String samlRequest, String samlResponse, String s
258255

259256
protected abstract String getBindingType();
260257
protected abstract boolean containsUnencryptedSignature(SAMLDocumentHolder documentHolder);
258+
protected abstract boolean isMessageFullySigned(SAMLDocumentHolder documentHolder);
261259
protected abstract void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException;
262260
protected abstract SAMLDocumentHolder extractRequestDocument(String samlRequest);
263261
protected abstract SAMLDocumentHolder extractResponseDocument(String response);
@@ -578,7 +576,7 @@ protected Response handleLoginResponse(String samlResponse, SAMLDocumentHolder h
578576
} else {
579577
/* We verify the assertion using original document to handle cases where the IdP
580578
includes whitespace and/or newlines inside tags. */
581-
assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get()));
579+
assertionElement = AssertionUtil.getAssertionElement(holder);
582580
}
583581

584582
// Validate the response Issuer
@@ -604,7 +602,7 @@ protected Response handleLoginResponse(String samlResponse, SAMLDocumentHolder h
604602
boolean signed = AssertionUtil.isSignedElement(assertionElement);
605603
final boolean assertionSignatureNotExistsWhenRequired = config.isWantAssertionsSigned() && !signed;
606604
final boolean signatureNotValid = signed && config.isValidateSignature() && !AssertionUtil.isSignatureValid(assertionElement, getIDPKeyLocator());
607-
final boolean hasNoSignatureWhenRequired = ! signed && config.isValidateSignature() && ! containsUnencryptedSignature(holder);
605+
final boolean hasNoSignatureWhenRequired = !signed && config.isValidateSignature() && !isMessageFullySigned(holder);
608606

609607
if (assertionSignatureNotExistsWhenRequired || signatureNotValid || hasNoSignatureWhenRequired) {
610608
logger.error("validation failed");
@@ -840,6 +838,11 @@ protected boolean containsUnencryptedSignature(SAMLDocumentHolder documentHolder
840838
return (nl != null && nl.getLength() > 0);
841839
}
842840

841+
@Override
842+
protected boolean isMessageFullySigned(SAMLDocumentHolder documentHolder) {
843+
return AssertionUtil.isSignedElement(documentHolder.getSamlDocument().getDocumentElement());
844+
}
845+
843846
@Override
844847
protected void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException {
845848
if ((! containsUnencryptedSignature(documentHolder)) && (documentHolder.getSamlObject() instanceof ResponseType)) {
@@ -879,14 +882,17 @@ protected boolean containsUnencryptedSignature(SAMLDocumentHolder documentHolder
879882
return algorithm != null && signature != null;
880883
}
881884

885+
@Override
886+
protected boolean isMessageFullySigned(SAMLDocumentHolder documentHolder) {
887+
return containsUnencryptedSignature(documentHolder);
888+
}
889+
882890
@Override
883891
protected void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException {
884892
KeyLocator locator = getIDPKeyLocator();
885893
SamlProtocolUtils.verifyRedirectSignature(documentHolder, locator, session.getContext().getUri(), key);
886894
}
887895

888-
889-
890896
@Override
891897
protected SAMLDocumentHolder extractRequestDocument(String samlRequest) {
892898
return SAMLRequestParser.parseRequestRedirectBinding(samlRequest, maxInflatingSize);
@@ -906,20 +912,31 @@ protected String getBindingType() {
906912

907913
protected class ArtifactBinding extends Binding {
908914

909-
private boolean unencryptedSignaturesVerified = false;
915+
// artifact binding is processed twice, first with the art and then with the response, vars to store the first pass
916+
private Boolean containsUnencryptedSignature = null;
917+
private Boolean messageFullySigned = null;
918+
private Boolean verified = null;
910919

911920
@Override
912921
protected boolean containsUnencryptedSignature(SAMLDocumentHolder documentHolder) {
913-
if (unencryptedSignaturesVerified) {
914-
return true;
922+
if (containsUnencryptedSignature != null) {
923+
return containsUnencryptedSignature;
915924
}
916925
NodeList nl = documentHolder.getSamlDocument().getElementsByTagNameNS(XMLSignature.XMLNS, "Signature");
917-
return (nl != null && nl.getLength() > 0);
926+
containsUnencryptedSignature = (nl != null && nl.getLength() > 0);
927+
messageFullySigned = AssertionUtil.isSignedElement(documentHolder.getSamlDocument().getDocumentElement());
928+
return containsUnencryptedSignature;
929+
}
930+
931+
@Override
932+
protected boolean isMessageFullySigned(SAMLDocumentHolder documentHolder) {
933+
containsUnencryptedSignature(documentHolder);
934+
return messageFullySigned;
918935
}
919936

920937
@Override
921938
protected void verifySignature(String key, SAMLDocumentHolder documentHolder) throws VerificationException {
922-
if (unencryptedSignaturesVerified) {
939+
if (verified != null) {
923940
// this is the second pass and signatures were already verified in the artifact response time
924941
return;
925942
}
@@ -938,13 +955,14 @@ protected void verifySignature(String key, SAMLDocumentHolder documentHolder) th
938955
}
939956
}
940957
SamlProtocolUtils.verifyDocumentSignature(documentHolder.getSamlDocument(), getIDPKeyLocator());
941-
unencryptedSignaturesVerified = true; // mark signatures as verified
958+
verified = true;
942959
}
943960

944961
@Override
945962
protected SAMLDocumentHolder extractRequestDocument(String samlRequest) {
946963
throw new UnsupportedOperationException("SAML request is not compliant with Artifact binding");
947964
}
965+
948966
@Override
949967
protected SAMLDocumentHolder extractResponseDocument(String response) {
950968
byte[] samlBytes = PostBindingUtil.base64Decode(response);

0 commit comments

Comments
 (0)