From f7ba306016d60e2eae83b74d104eef9c9b45bb8a Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Wed, 17 Feb 2016 11:51:58 -0200 Subject: [PATCH] [KEYCLOAK-2497] - Prevent inserting malicious SAML assertion --- .../core/util/XMLSignatureUtil.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java index febb720eb3..94a6c2bcfb 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java @@ -18,6 +18,7 @@ package org.keycloak.saml.processing.core.util; import org.keycloak.saml.common.PicketLinkLogger; import org.keycloak.saml.common.PicketLinkLoggerFactory; +import org.keycloak.saml.common.constants.JBossSAMLConstants; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.saml.common.constants.WSTrustConstants; import org.keycloak.saml.common.exceptions.ParsingException; @@ -376,7 +377,19 @@ public class XMLSignatureUtil { if (publicKey == null) throw logger.nullValueError("Public Key"); + int signedAssertions = 0; + String assertionNameSpaceUri = null; + for (int i = 0; i < nl.getLength(); i++) { + Node signatureNode = nl.item(i); + Node parent = signatureNode.getParentNode(); + if (parent != null && JBossSAMLConstants.ASSERTION.get().equals(parent.getLocalName())) { + ++signedAssertions; + if (assertionNameSpaceUri == null) { + assertionNameSpaceUri = parent.getNamespaceURI(); + } + } + DOMValidateContext valContext = new DOMValidateContext(publicKey, nl.item(i)); XMLSignature signature = fac.unmarshalXMLSignature(valContext); @@ -397,6 +410,16 @@ public class XMLSignatureUtil { } } + NodeList assertions = signedDoc.getElementsByTagNameNS(assertionNameSpaceUri, JBossSAMLConstants.ASSERTION.get()); + + if (signedAssertions > 0 && assertions != null && assertions.getLength() != signedAssertions) { + if (logger.isDebugEnabled()) { + logger.debug("SAML Response document may contain malicious assertions. Signature validation will fail."); + } + // there are unsigned assertions mixed with signed ones + return false; + } + return true; }