KEYCLOAK-10729 Do not serialize SAML signature

saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLAssertionWriter.java

@@ -39,7 +39,6 @@ import org.keycloak.saml.common.constants.JBossSAMLConstants;
 import org.keycloak.saml.common.exceptions.ProcessingException;
 import org.keycloak.saml.common.util.StaxUtil;
 import org.keycloak.saml.processing.core.parsers.saml.assertion.SAMLAssertionQNames;
-import org.w3c.dom.Element;
 
 import javax.xml.datatype.XMLGregorianCalendar;
 import javax.xml.namespace.QName;
@@ -48,7 +47,6 @@ import java.net.URI;
 import java.util.List;
 import java.util.Set;
 
-import javax.xml.crypto.dsig.XMLSignature;
 import static org.keycloak.saml.common.constants.JBossSAMLURIConstants.ASSERTION_NSURI;
 
 /**
@@ -71,17 +69,8 @@ public class SAMLAssertionWriter extends BaseWriter {
      * @throws org.keycloak.saml.common.exceptions.ProcessingException
      */
     public void write(AssertionType assertion) throws ProcessingException {
-        write(assertion, false);
-    }
-
-    public void write(AssertionType assertion, boolean forceWriteDsigNamespace) throws ProcessingException {
-        Element sig = assertion.getSignature();
-
         StaxUtil.writeStartElement(writer, ASSERTION_PREFIX, JBossSAMLConstants.ASSERTION.get(), ASSERTION_NSURI.get());
         StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, ASSERTION_NSURI.get());
-        if (forceWriteDsigNamespace && sig != null && sig.getPrefix() != null && ! sig.hasAttribute("xmlns:" + sig.getPrefix())) {
-            StaxUtil.writeNameSpace(writer, sig.getPrefix(), XMLSignature.XMLNS);
-        }
         StaxUtil.writeDefaultNameSpace(writer, ASSERTION_NSURI.get());
 
         // Attributes
@@ -93,9 +82,6 @@ public class SAMLAssertionWriter extends BaseWriter {
         if (issuer != null)
             write(issuer, new QName(ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX));
 
-        if (sig != null)
-            StaxUtil.writeDOMElement(writer, sig);
-
         SubjectType subject = assertion.getSubject();
         if (subject != null) {
             write(subject);

saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java

@@ -64,17 +64,8 @@ public class SAMLResponseWriter extends BaseWriter {
      * @throws org.keycloak.saml.common.exceptions.ProcessingException
      */
     public void write(ResponseType response) throws ProcessingException {
-        write(response, false);
-    }
-
-    public void write(ResponseType response, boolean forceWriteDsigNamespace) throws ProcessingException {
-        Element sig = response.getSignature();
-
         StaxUtil.writeStartElement(writer, PROTOCOL_PREFIX, JBossSAMLConstants.RESPONSE__PROTOCOL.get(), JBossSAMLURIConstants.PROTOCOL_NSURI.get());
 
-        if (forceWriteDsigNamespace && sig != null && sig.getPrefix() != null && ! sig.hasAttribute("xmlns:" + sig.getPrefix())) {
-            StaxUtil.writeNameSpace(writer, sig.getPrefix(), XMLSignature.XMLNS);
-        }
         StaxUtil.writeNameSpace(writer, PROTOCOL_PREFIX, JBossSAMLURIConstants.PROTOCOL_NSURI.get());
         StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, JBossSAMLURIConstants.ASSERTION_NSURI.get());
 
@@ -85,9 +76,6 @@ public class SAMLResponseWriter extends BaseWriter {
             write(issuer, new QName(JBossSAMLURIConstants.ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX));
         }
 
-        if (sig != null) {
-            StaxUtil.writeDOMElement(writer, sig);
-        }
         ExtensionsType extensions = response.getExtensions();
         if (extensions != null && extensions.getAny() != null && ! extensions.getAny().isEmpty()) {
             write(extensions);
@@ -101,7 +89,7 @@ public class SAMLResponseWriter extends BaseWriter {
             for (ResponseType.RTChoiceType choiceType : choiceTypes) {
                 AssertionType assertion = choiceType.getAssertion();
                 if (assertion != null) {
-                    assertionWriter.write(assertion, forceWriteDsigNamespace);
+                    assertionWriter.write(assertion);
                 }
 
                 EncryptedAssertionType encryptedAssertion = choiceType.getEncryptedAssertion();

services/src/main/java/org/keycloak/broker/saml/SAMLDataMarshaller.java

@@ -49,11 +49,11 @@ public class SAMLDataMarshaller extends DefaultDataMarshaller {
                 if (obj instanceof ResponseType) {
                     ResponseType responseType = (ResponseType) obj;
                     SAMLResponseWriter samlWriter = new SAMLResponseWriter(StaxUtil.getXMLStreamWriter(bos));
-                    samlWriter.write(responseType, true);
+                    samlWriter.write(responseType);
                 } else if (obj instanceof AssertionType) {
                     AssertionType assertion = (AssertionType) obj;
                     SAMLAssertionWriter samlWriter = new SAMLAssertionWriter(StaxUtil.getXMLStreamWriter(bos));
-                    samlWriter.write(assertion, true);
+                    samlWriter.write(assertion);
                 } else if (obj instanceof AuthnStatementType) {
                     AuthnStatementType authnStatement = (AuthnStatementType) obj;
                     SAMLAssertionWriter samlWriter = new SAMLAssertionWriter(StaxUtil.getXMLStreamWriter(bos));

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/ModifySamlResponseStepBuilder.java

@@ -149,7 +149,11 @@ public class ModifySamlResponseStepBuilder extends SamlDocumentStepBuilder<SAML2
 
         final String attrName = this.targetAttribute != null ? this.targetAttribute : samlParam.getName();
 
-        return createRequest(locationUri, attrName, transformed, params);
+        final URI uri = this.targetUri != null
+          ? this.targetUri
+          : locationUri;
+
+        return createRequest(uri, attrName, transformed, params);
     }
 
     private HttpUriRequest handlePostBinding(CloseableHttpResponse currentResponse) throws Exception {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/AbstractSamlTest.java

@@ -10,6 +10,12 @@ import org.keycloak.testsuite.util.SamlClient;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriBuilderException;
 import java.net.URI;
+import java.security.KeyFactory;
+import java.security.PrivateKey;
+import java.security.PublicKey;
+import java.security.spec.PKCS8EncodedKeySpec;
+import java.security.spec.X509EncodedKeySpec;
+import java.util.Base64;
 import java.util.List;
 
 import static org.keycloak.testsuite.arquillian.AuthServerTestEnricher.AUTH_SERVER_PORT;
@@ -38,6 +44,19 @@ public abstract class AbstractSamlTest extends AbstractAuthTest {
     public static final String SAML_URL_SALES_POST_SIG = "http://localhost:8080/sales-post-sig/";
     public static final String SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY = "MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBANUbxrvEY3pkiQNt55zJLKBwN+zKmNQw08ThAmOKzwHfXoK+xlDSFxNMtTKJGkeUdnKzaTfESEcEfKYULUA41y/NnOlvjS0CEsc7Wq0Ce63TSSGMB2NHea4tV0aQz/MwLsbmz2IjAFWHA5CHL5WwacIf3UTOSNnhJUSvnkomjJAlAgMBAAECgYANpO2gb/5+g5lSIuNFYov86bJq8r2+ODIW1OE2Rljioc6HSHeiDRF1JuAjECwikRrUVTBTZbnK8jqY14neJsWAKBzGo+ToaQALsNZ9B91DxxL50K5oVOzw5shAS9TnRjN40+KIXFED4ydq4JRdoqb8+cN+N3i0+Cu7tdm+UaHTAQJBAOwFs3ZwqQEqmv9vmgmIFwFpJm1aIw25gEOf3Hy45GP4bL/j0FQgwcXYRbLE5bPqhw/liLKc1GQ97bVm6zs8SvUCQQDnJZA6TFRMiDjezinE1J4e0v4RupyDniVjbE5ArTK5/FRVkjw4Ny0AqZUEyIIqlTeZlCq45pCJy4a2hymDGVJxAj9gzfXNnmezEsZ//kYvoqHM8lPQhifaeTsigW7tuOf0GPCBw+6uksDnZM0xhZCxOoArBPoMSEbU1pGo1Y2lvhUCQF6E5sBgHAybm53Ich4Rz4LNRqWbSIstrR5F2I3sBRU2kInZXZSjQ1zE+7HUCB4/nFfJ1dp8NdiTCEg1Zw072pECQQDnxyQALmWhQbBTl0tq6CwYf9rZDwBzxuY+CXB8Ky1gOmXwan96KZvV4rK8MQQs6HIiYC/j+5lX3A3zlXTFldaz";
     public static final String SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY = "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDVG8a7xGN6ZIkDbeecySygcDfsypjUMNPE4QJjis8B316CvsZQ0hcTTLUyiRpHlHZys2k3xEhHBHymFC1AONcvzZzpb40tAhLHO1qtAnut00khjAdjR3muLVdGkM/zMC7G5s9iIwBVhwOQhy+VsGnCH91EzkjZ4SVEr55KJoyQJQIDAQAB";
+    public static final PrivateKey SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY_PK;
+    public static final PublicKey SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY_PK;
+
+    static {
+        try {
+            KeyFactory kfRsa = KeyFactory.getInstance("RSA");
+            SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY_PK = kfRsa.generatePrivate(new PKCS8EncodedKeySpec(Base64.getDecoder().decode(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY)));
+            SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY_PK = kfRsa.generatePublic(new X509EncodedKeySpec(Base64.getDecoder().decode(SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)));
+        } catch (Exception ex) {
+            throw new RuntimeException(ex);
+        }
+    }
+
 
     // Set date to past; then: openssl req -x509 -newkey rsa:1024 -keyout key.pem -out cert.pem -days 1 -nodes -subj '/CN=http:\/\/localhost:8080\/sales-post-sig\/'
     public static final String SAML_CLIENT_SALES_POST_SIG_EXPIRED_PRIVATE_KEY = "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAMrGzRp3HVf6Ti75rl5mPAPXua8APCCLANikzOd82VI0R8Ml0UAchkfRUBvBedobJIn9r8wwxMeXLmKsMynW52SYeC/Zx5b5K6ayMS3GWJIgqLpp/n1piUeI4sbJXlUj9UtW+QTpGhrHt9n7s7znwoNqGDUkjmyZiekEspjdfzzlAgMBAAECgYBJvPFo5lftXkCAJJucCGFapGAJm3RCAUpVfdhldakxk4FlHaNyRO0vwJX5AeplvekTpQUAo9trGTbs+uHAHT4XWOnwhHHyBRkWdiwXX9bzNdHnIwf/0SLIBBYUk0hoWEDvpklBPqllM215a0sEnB2ykYSsMDBSkFB7Ah+RK7zTAQJBAOw9v7SsfIhOXci9vnkQPuQpL8T4kwj7nWi+YtRGrXbF/bJGwjsgXN5i7otwBV/W+TNzI5H7s2opPUXdIxfP9C0CQQDbvIcxXjwjO1hjXXY4axiT1sxU8Oq1bds033atMoN9pib7IxkWh6ouOQZT8bxwQ2ElH0rswZ0/2CusrIUIekaZAkEAk9UUSQiDKXz4vSzXq8SZxodriDQRNtbVqv0wtSvBUwkU9+HFm+BlnRiFtCYWhuHsseCESs8ad/10hWqbkkQkxQJAZOvN2+rADB5xlhGS/o6RlzUMW+bapcFy8HHB/AI7SjZJqQaRuztL+jbOpTddqOIJeBdLPjoekvgh9wi1gRNH4QJBAMjfB1xYxmztfbUcUuOsATz3s7StprOAukd+hhBiMukxcKhi1IQp7tFhfFe/+xUY3fSh1a3KlyItFKxp68EdDRk=";

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java

@@ -35,10 +35,13 @@ import org.keycloak.dom.saml.v2.protocol.ResponseType;
 import org.keycloak.models.AuthenticationExecutionModel.Requirement;
 import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
+import org.keycloak.saml.BaseSAML2BindingBuilder;
 import org.keycloak.saml.SAML2LoginResponseBuilder;
 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.processing.core.parsers.saml.xmldsig.XmlDSigQNames;
+import org.keycloak.saml.processing.core.parsers.util.HasQName;
 import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
 import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil;
 import org.keycloak.testsuite.updaters.IdentityProviderCreator;
@@ -46,17 +49,24 @@ import org.keycloak.testsuite.util.IdentityProviderBuilder;
 import org.keycloak.testsuite.util.SamlClientBuilder;
 import java.io.IOException;
 import java.net.URI;
+import java.security.KeyPair;
 import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
 import javax.ws.rs.core.Response.Status;
 import javax.xml.datatype.XMLGregorianCalendar;
+import javax.xml.namespace.QName;
 import org.apache.http.Header;
 import org.apache.http.HttpHeaders;
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Test;
+import org.w3c.dom.DOMException;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
 import static org.junit.Assert.assertThat;
+import static org.keycloak.saml.SignatureAlgorithm.RSA_SHA1;
 import static org.keycloak.testsuite.saml.AbstractSamlTest.REALM_NAME;
 import static org.keycloak.testsuite.saml.AbstractSamlTest.SAML_ASSERTION_CONSUMER_URL_SALES_POST;
 import static org.keycloak.testsuite.saml.AbstractSamlTest.SAML_CLIENT_ID_SALES_POST;
@@ -192,6 +202,62 @@ public class BrokerTest extends AbstractSamlTest {
         }
     }
 
+    private static final String XMLNS_VETINARI = "vetinari";
+    private static final String NS_VETINARI = "urn:dw:am:havelock";
+
+    private static Element appendNewElement(Element parent, QName qName, String prefix) throws DOMException {
+        Document doc = parent.getOwnerDocument();
+        final Element res = doc.createElementNS(qName.getNamespaceURI(), prefix + ":" + qName.getLocalPart());
+        parent.appendChild(res);
+        return res;
+    }
+
+    private static void signAndAddCustomNamespaceElementToSignature(Document doc) {
+        doc.getDocumentElement().setAttribute("xmlns:" + XMLNS_VETINARI, NS_VETINARI);
+
+        BaseSAML2BindingBuilder<BaseSAML2BindingBuilder> sb = new BaseSAML2BindingBuilder();
+        try {
+            KeyPair keyPair = new KeyPair(SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY_PK, SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY_PK);
+            sb.signWith("kn", keyPair)
+              .signatureAlgorithm(RSA_SHA1)
+              .signAssertions()
+              .signAssertion(doc);
+        } catch (ProcessingException ex) {
+            throw new RuntimeException(ex);
+        }
+
+        // KeyInfo has lax and can contain custom elements, see https://www.w3.org/TR/xmldsig-core1/#sec-KeyInfo
+        Element el = findFirstElement(doc, XmlDSigQNames.KEY_INFO);
+        appendNewElement(el, new QName(NS_VETINARI, "Patrician"), XMLNS_VETINARI);
+    }
+
+    private static Element findFirstElement(Document doc, HasQName qName) {
+        NodeList nl = doc.getElementsByTagNameNS(qName.getQName().getNamespaceURI(), qName.getQName().getLocalPart());
+        return (nl == null || nl.getLength() == 0) ? null : (Element) nl.item(0);
+    }
+
+    @Test
+    public void testAnyNamespacePreservedInContext() throws IOException {
+        final RealmResource realm = adminClient.realm(REALM_NAME);
+
+        try (IdentityProviderCreator idp = new IdentityProviderCreator(realm, addIdentityProvider("https://saml.idp/"))) {
+            new SamlClientBuilder()
+              .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST)
+                .build()
+              .login().idp(SAML_BROKER_ALIAS).build()
+              // Virtually perform login at IdP (return artificial SAML response)
+              .processSamlResponse(REDIRECT)
+                .transformObject(this::createAuthnResponse)
+                .transformDocument(BrokerTest::signAndAddCustomNamespaceElementToSignature)
+                .targetAttributeSamlResponse()
+                .targetUri(getSamlBrokerUrl(REALM_NAME))
+                .targetBinding(POST)
+                .build()
+              .assertResponse(org.keycloak.testsuite.util.Matchers.statusCodeIsHC(Status.OK))
+              .execute();
+        }
+    }
+
     @Test
     public void testExpiredAssertion() throws Exception {
         XMLGregorianCalendar now = XMLTimeUtil.getIssueInstant();