From 10077b1efe1490bc91db2816be304b0d53f09301 Mon Sep 17 00:00:00 2001
From: Luca Leonardo Scorcia <l.scorcia@gmail.com>
Date: Tue, 8 Sep 2020 17:33:44 -0400
Subject: [PATCH] KEYCLOAK-15485 Add option to enable SAML SP metadata
 signature

---
 .../keycloak/saml/SPMetadataDescriptor.java   |  2 +
 .../broker/saml/SAMLIdentityProvider.java     | 26 +++++++
 .../saml/SAMLIdentityProviderConfig.java      |  9 +++
 .../testsuite/admin/IdentityProviderTest.java | 71 +++++++++++++++++++
 .../messages/admin-messages_en.properties     |  2 +
 .../realm-identity-provider-saml.html         |  7 ++
 6 files changed, 117 insertions(+)

diff --git a/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java b/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java
index 724a64962f..f346815248 100755
--- a/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java
+++ b/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java
@@ -36,6 +36,7 @@ import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamWriter;
 import org.keycloak.saml.common.util.StaxUtil;
 import org.keycloak.saml.common.exceptions.ProcessingException;
+import org.keycloak.saml.processing.core.saml.v2.common.IDGenerator;
 import org.keycloak.saml.processing.core.saml.v2.writers.SAMLMetadataWriter;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -60,6 +61,7 @@ public class SPMetadataDescriptor {
         SAMLMetadataWriter metadataWriter = new SAMLMetadataWriter(writer);
 
         EntityDescriptorType entityDescriptor = new EntityDescriptorType(entityId);
+        entityDescriptor.setID(IDGenerator.create("ID_"));
 
         SPSSODescriptorType spSSODescriptor = new SPSSODescriptorType(Arrays.asList(PROTOCOL_NSURI.get()));
         spSSODescriptor.setAuthnRequestsSigned(wantAuthnRequestsSigned);
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
index 61dd5afee1..c590e31fc6 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
@@ -41,19 +41,25 @@ import org.keycloak.saml.SamlProtocolExtensionsAwareBuilder.NodeGenerator;
 import org.keycloak.saml.common.constants.GeneralConstants;
 import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
 import org.keycloak.saml.common.exceptions.ConfigurationException;
+import org.keycloak.saml.common.util.DocumentUtil;
 import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
+import org.keycloak.saml.processing.api.saml.v2.sig.SAML2Signature;
 import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
 import org.keycloak.saml.validators.DestinationValidator;
 import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.util.JsonSerialization;
 
+import org.w3c.dom.Document;
 import org.w3c.dom.Element;
+import org.w3c.dom.Node;
 
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
+import javax.xml.crypto.dsig.CanonicalizationMethod;
 import java.net.URI;
+import java.security.KeyPair;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.LinkedList;
@@ -332,6 +338,26 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
               wantAuthnRequestsSigned, wantAssertionsSigned, wantAssertionsEncrypted,
               entityId, nameIDPolicyFormat, signingKeys, encryptionKeys);
 
+            // Metadata signing
+            if (getConfig().isSignSpMetadata())
+            {
+                KeyManager.ActiveRsaKey activeKey = session.keys().getActiveRsaKey(realm);
+                String keyName = getConfig().getXmlSigKeyInfoKeyNameTransformer().getKeyName(activeKey.getKid(), activeKey.getCertificate());
+                KeyPair keyPair = new KeyPair(activeKey.getPublicKey(), activeKey.getPrivateKey());
+
+                Document metadataDocument = DocumentUtil.getDocument(descriptor);
+                SAML2Signature signatureHelper = new SAML2Signature();
+                signatureHelper.setSignatureMethod(getSignatureAlgorithm().getXmlSignatureMethod());
+                signatureHelper.setDigestMethod(getSignatureAlgorithm().getXmlSignatureDigestMethod());
+
+                Node nextSibling = metadataDocument.getDocumentElement().getFirstChild();
+                signatureHelper.setNextSibling(nextSibling);
+
+                signatureHelper.signSAMLDocument(metadataDocument, keyName, keyPair, CanonicalizationMethod.EXCLUSIVE);
+
+                descriptor = DocumentUtil.getDocumentAsString(metadataDocument);
+            }
+
             return Response.ok(descriptor, MediaType.APPLICATION_XML_TYPE).build();
         } catch (Exception e) {
             logger.warn("Failed to export SAML SP Metadata!", e);
diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
index db954fb183..7ec28997f9 100755
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProviderConfig.java
@@ -56,6 +56,7 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
     public static final String AUTHN_CONTEXT_COMPARISON_TYPE = "authnContextComparisonType";
     public static final String AUTHN_CONTEXT_CLASS_REFS = "authnContextClassRefs";
     public static final String AUTHN_CONTEXT_DECL_REFS = "authnContextDeclRefs";
+    public static final String SIGN_SP_METADATA = "signSpMetadata";
 
     public SAMLIdentityProviderConfig() {
     }
@@ -317,6 +318,14 @@ public class SAMLIdentityProviderConfig extends IdentityProviderModel {
         getConfig().put(AUTHN_CONTEXT_DECL_REFS, authnContextDeclRefs);
     }
 
+    public boolean isSignSpMetadata() {
+        return Boolean.valueOf(getConfig().get(SIGN_SP_METADATA));
+    }
+
+    public void setSignSpMetadata(boolean signSpMetadata) {
+        getConfig().put(SIGN_SP_METADATA, String.valueOf(signSpMetadata));
+    }
+
     @Override
     public void validate(RealmModel realm) {
         SslRequired sslRequired = realm.getSslRequired();
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
index 9c7dd40743..4235c8f731 100755
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/IdentityProviderTest.java
@@ -42,12 +42,17 @@ import org.keycloak.representations.idm.ErrorRepresentation;
 import org.keycloak.representations.idm.IdentityProviderMapperRepresentation;
 import org.keycloak.representations.idm.IdentityProviderMapperTypeRepresentation;
 import org.keycloak.representations.idm.IdentityProviderRepresentation;
+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.core.parsers.saml.SAMLParser;
 import org.keycloak.testsuite.Assert;
 import org.keycloak.testsuite.broker.OIDCIdentityProviderConfigRep;
 import org.keycloak.testsuite.updaters.RealmAttributeUpdater;
 import org.keycloak.testsuite.util.AdminEventPaths;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
 
 import javax.ws.rs.ClientErrorException;
@@ -87,6 +92,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.keycloak.saml.common.constants.JBossSAMLURIConstants.XMLDSIG_NSURI;
 import static org.keycloak.testsuite.util.ServerURLs.AUTH_SERVER_SSL_REQUIRED;
 import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
 import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE;
@@ -1007,4 +1013,69 @@ public class IdentityProviderTest extends AbstractAdminTest {
         Assert.assertEquals("id", id, info.get("id"));
         Assert.assertEquals("name", name, info.get("name"));
     }
+
+    @Test
+    public void testSamlExportSignatureOff() throws URISyntaxException, IOException, ConfigurationException, ParsingException, ProcessingException {
+        // Use import-config to convert IDPSSODescriptor file into key value pairs
+        // to use when creating a SAML Identity Provider
+        MultipartFormDataOutput form = new MultipartFormDataOutput();
+        form.addFormData("providerId", "saml", MediaType.TEXT_PLAIN_TYPE);
+
+        URL idpMeta = getClass().getClassLoader().getResource("admin-test/saml-idp-metadata.xml");
+        byte [] content = Files.readAllBytes(Paths.get(idpMeta.toURI()));
+        String body = new String(content, Charset.forName("utf-8"));
+        form.addFormData("file", body, MediaType.APPLICATION_XML_TYPE, "saml-idp-metadata.xml");
+
+        Map<String, String> result = realm.identityProviders().importFrom(form);
+
+        // Explicitly disable SP Metadata Signature
+        result.put(SAMLIdentityProviderConfig.SIGN_SP_METADATA, "false");
+
+        // Create new SAML identity provider using configuration retrieved from import-config
+        IdentityProviderRepresentation idpRep = createRep("saml", "saml", true, result);
+        create(idpRep);
+
+        // Perform export, and make sure some of the values are like they're supposed to be
+        Response response = realm.identityProviders().get("saml").export("xml");
+        Assert.assertEquals(200, response.getStatus());
+        body = response.readEntity(String.class);
+        response.close();
+
+        Document document = DocumentUtil.getDocument(body);
+        Element signatureElement = DocumentUtil.getDirectChildElement(document.getDocumentElement(), XMLDSIG_NSURI.get(), "Signature");
+        Assert.assertNull(signatureElement);
+    }
+
+    @Test
+    public void testSamlExportSignatureOn() throws URISyntaxException, IOException, ConfigurationException, ParsingException, ProcessingException {
+        // Use import-config to convert IDPSSODescriptor file into key value pairs
+        // to use when creating a SAML Identity Provider
+        MultipartFormDataOutput form = new MultipartFormDataOutput();
+        form.addFormData("providerId", "saml", MediaType.TEXT_PLAIN_TYPE);
+
+        URL idpMeta = getClass().getClassLoader().getResource("admin-test/saml-idp-metadata.xml");
+        byte [] content = Files.readAllBytes(Paths.get(idpMeta.toURI()));
+        String body = new String(content, Charset.forName("utf-8"));
+        form.addFormData("file", body, MediaType.APPLICATION_XML_TYPE, "saml-idp-metadata.xml");
+
+        Map<String, String> result = realm.identityProviders().importFrom(form);
+
+        // Explicitly enable SP Metadata Signature
+        result.put(SAMLIdentityProviderConfig.SIGN_SP_METADATA, "true");
+
+        // Create new SAML identity provider using configuration retrieved from import-config
+        IdentityProviderRepresentation idpRep = createRep("saml", "saml", true, result);
+        create(idpRep);
+
+        // Perform export, and make sure some of the values are like they're supposed to be
+        Response response = realm.identityProviders().get("saml").export("xml");
+        Assert.assertEquals(200, response.getStatus());
+        body = response.readEntity(String.class);
+        response.close();
+
+        Document document = DocumentUtil.getDocument(body);
+
+        Element signatureElement = DocumentUtil.getDirectChildElement(document.getDocumentElement(), XMLDSIG_NSURI.get(), "Signature");
+        Assert.assertNotNull(signatureElement);
+    }
 }
diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
index 0e5d29f390..8d0d42c969 100644
--- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
+++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
@@ -699,6 +699,8 @@ validating-x509-certificate.tooltip=The certificate in PEM format that must be u
 saml.loginHint=Pass subject
 saml.loginHint.tooltip=During login phase, forward an optional login_hint query parameter to SAML AuthnRequest's Subject.
 saml.import-from-url.tooltip=Import metadata from a remote IDP SAML entity descriptor.
+identity-provider.saml.sign-sp-metadata=Sign Service Provider Metadata
+identity-provider.saml.sign-sp-metadata.tooltip=Enable/disable signature of the provider SAML metadata
 identity-provider.saml.requested-authncontext=Requested AuthnContext Constraints
 identity-provider.saml.requested-authncontext.tooltip=Allows the SP to specify the authentication context requirements of authentication statements returned.
 identity-provider.saml.authncontext-comparison-type=Comparison
diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html b/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
index 36d81c7f07..c88e09d7a2 100755
--- a/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
+++ b/themes/src/main/resources/theme/base/admin/resources/partials/realm-identity-provider-saml.html
@@ -267,6 +267,13 @@
                 </div>
                 <kc-tooltip>{{:: 'validating-x509-certificate.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group">
+                <label class="col-md-2 control-label" for="signSpMetadata">{{:: 'identity-provider.saml.sign-sp-metadata' | translate}}</label>
+                <div class="col-md-6">
+                    <input ng-model="identityProvider.config.signSpMetadata" id="signSpMetadata" onoffswitchvalue on-text="{{:: 'onText' | translate}}" off-text="{{:: 'offText' | translate}}" />
+                </div>
+                <kc-tooltip>{{:: 'identity-provider.saml.sign-sp-metadata.tooltip' | translate}}</kc-tooltip>
+            </div>
             <div class="form-group">
                 <label class="col-sm-2 control-label" for="loginHint">{{:: 'saml.loginHint' | translate}}</label>
                 <div class="col-sm-4">