KEYCLOAK-1881 Include key ID for REDIRECT and use it for validation

Contrary to POST binding, signature of SAML protocol message sent using
REDIRECT binding is contained in query parameters and not in the
message. This renders <dsig:KeyName> key ID hint unusable. This commit
adds <Extensions> element in SAML protocol message containing key ID so
that key ID is present in the SAML protocol message.

adapters/saml/core/pom.xml

@@ -34,6 +34,7 @@
         <timestamp>${maven.build.timestamp}</timestamp>
         <maven.build.timestamp.format>yyyy-MM-dd HH:mm</maven.build.timestamp.format>
     </properties>
+
     <dependencies>
         <dependency>
             <groupId>org.keycloak</groupId>
@@ -70,6 +71,11 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.httpcomponents</groupId>
+            <artifactId>httpclient</artifactId>
+            <scope>provided</scope>
+        </dependency>
     </dependencies>
     <build>
         <plugins>

adapters/saml/core/src/main/java/org/keycloak/adapters/saml/AbstractInitiateLogin.java

@@ -80,6 +80,8 @@ public abstract class AbstractInitiateLogin implements AuthChallenge {
             }
 
             binding.signWith(null, keypair);
+            // TODO: As part of KEYCLOAK-3810, add KeyID to the SAML document
+            //   <related DocumentBuilder>.addExtension(new KeycloakKeySamlExtensionGenerator(<key ID>));
             binding.signDocument();
         }
         return binding;

adapters/saml/core/src/main/java/org/keycloak/adapters/saml/DefaultSamlDeployment.java

@@ -24,6 +24,12 @@ import java.security.KeyPair;
 import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.util.Set;
+import org.apache.http.client.HttpClient;
+import org.keycloak.adapters.HttpClientBuilder;
+import org.keycloak.adapters.saml.rotation.SamlDescriptorPublicKeyLocator;
+import org.keycloak.rotation.CompositeKeyLocator;
+import org.keycloak.rotation.HardcodedKeyLocator;
+import org.keycloak.rotation.KeyLocator;
 
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@@ -179,10 +185,14 @@ public class DefaultSamlDeployment implements SamlDeployment {
 
     public static class DefaultIDP implements IDP {
 
+        private static final int DEFAULT_CACHE_TTL = 24 * 60 * 60;
+
         private String entityID;
-        private PublicKey signatureValidationKey;
+        private final CompositeKeyLocator signatureValidationKeyLocator = new CompositeKeyLocator();
         private SingleSignOnService singleSignOnService;
         private SingleLogoutService singleLogoutService;
+        private HardcodedKeyLocator hardcodedKeyLocator;
+        private int minTimeBetweenDescriptorRequests;
 
         @Override
         public String getEntityID() {
@@ -200,8 +210,17 @@ public class DefaultSamlDeployment implements SamlDeployment {
         }
 
         @Override
-        public PublicKey getSignatureValidationKey() {
-            return signatureValidationKey;
+        public KeyLocator getSignatureValidationKeyLocator() {
+            return this.signatureValidationKeyLocator;
+        }
+
+        @Override
+        public int getMinTimeBetweenDescriptorRequests() {
+            return minTimeBetweenDescriptorRequests;
+        }
+
+        public void setMinTimeBetweenDescriptorRequests(int minTimeBetweenDescriptorRequests) {
+            this.minTimeBetweenDescriptorRequests = minTimeBetweenDescriptorRequests;
         }
 
         public void setEntityID(String entityID) {
@@ -209,16 +228,35 @@ public class DefaultSamlDeployment implements SamlDeployment {
         }
 
         public void setSignatureValidationKey(PublicKey signatureValidationKey) {
-            this.signatureValidationKey = signatureValidationKey;
+            this.hardcodedKeyLocator = signatureValidationKey == null ? null : new HardcodedKeyLocator(signatureValidationKey);
+            refreshKeyLocatorConfiguration();
         }
 
         public void setSingleSignOnService(SingleSignOnService singleSignOnService) {
             this.singleSignOnService = singleSignOnService;
+            refreshKeyLocatorConfiguration();
         }
 
         public void setSingleLogoutService(SingleLogoutService singleLogoutService) {
             this.singleLogoutService = singleLogoutService;
         }
+
+        public void refreshKeyLocatorConfiguration() {
+            this.signatureValidationKeyLocator.clear();
+
+            // When key is set, use that (and only that), otherwise configure dynamic key locator
+            if (this.hardcodedKeyLocator != null) {
+                this.signatureValidationKeyLocator.add(this.hardcodedKeyLocator);
+            } else if (this.singleSignOnService != null) {
+                String samlDescriptorUrl = singleSignOnService.getRequestBindingUrl() + "/descriptor";
+                // TODO
+                HttpClient httpClient = new HttpClientBuilder().build();
+                SamlDescriptorPublicKeyLocator samlDescriptorPublicKeyLocator =
+                  new SamlDescriptorPublicKeyLocator(
+                    samlDescriptorUrl, this.minTimeBetweenDescriptorRequests, DEFAULT_CACHE_TTL, httpClient);
+                this.signatureValidationKeyLocator.add(samlDescriptorPublicKeyLocator);
+            }
+        }
     }
 
     private IDP idp;

adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlDeployment.java

@@ -22,14 +22,17 @@ import org.keycloak.saml.SignatureAlgorithm;
 
 import java.security.KeyPair;
 import java.security.PrivateKey;
-import java.security.PublicKey;
 import java.util.Set;
+import org.keycloak.rotation.KeyLocator;
 
 /**
+ * Represents SAML deployment configuration.
+ * 
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
  */
 public interface SamlDeployment {
+
     enum Binding {
         POST,
         REDIRECT;
@@ -41,20 +44,62 @@ public interface SamlDeployment {
     }
 
     public interface IDP {
+        /**
+         * Returns entity identifier of this IdP.
+         * @return see description.
+         */
         String getEntityID();
 
+        /**
+         * Returns Single sign on service configuration for this IdP.
+         * @return see description.
+         */
         SingleSignOnService getSingleSignOnService();
+
+        /**
+         * Returns Single logout service configuration for this IdP.
+         * @return see description.
+         */
         SingleLogoutService getSingleLogoutService();
-        PublicKey getSignatureValidationKey();
+
+        /**
+         * Returns {@link KeyLocator} looking up public keys used for validation of IdP signatures.
+         * @return see description.
+         */
+        KeyLocator getSignatureValidationKeyLocator();
+
+        /**
+         * Returns minimum time (in seconds) between issuing requests to IdP SAML descriptor.
+         * Used e.g. by {@link KeyLocator} looking up public keys for validation of IdP signatures
+         * to prevent too frequent requests.
+         *
+         * @return see description.
+         */
+        int getMinTimeBetweenDescriptorRequests();
 
         public interface SingleSignOnService {
+            /**
+             * Returns {@code true} if the requests to IdP need to be signed by SP key.
+             * @return see dscription
+             */
             boolean signRequest();
+            /**
+             * Returns {@code true} if the complete response message from IdP should
+             * be checked for valid signature.
+             * @return see dscription
+             */
             boolean validateResponseSignature();
+            /**
+             * Returns {@code true} if individual assertions in response from IdP should
+             * be checked for valid signature.
+             * @return see dscription
+             */
             boolean validateAssertionSignature();
             Binding getRequestBinding();
             Binding getResponseBinding();
             String getRequestBindingUrl();
         }
+
         public interface SingleLogoutService {
             boolean validateRequestSignature();
             boolean validateResponseSignature();
@@ -67,10 +112,19 @@ public interface SamlDeployment {
         }
     }
 
+    /**
+     * Returns Identity Provider configuration for this SAML deployment.
+     * @return see description.
+     */
     public IDP getIDP();
 
     public boolean isConfigured();
     SslRequired getSslRequired();
+
+    /**
+     * Returns entity identifier of this SP.
+     * @return see description.
+     */
     String getEntityID();
     String getNameIDPolicyFormat();
     boolean isForceAuthentication();

adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/DeploymentBuilder.java

@@ -202,6 +202,7 @@ public class DeploymentBuilder {
             }
         }
 
+        idp.refreshKeyLocatorConfiguration();
 
         return deployment;
     }

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

@@ -64,11 +64,20 @@ import org.w3c.dom.Node;
 
 import java.io.IOException;
 import java.net.URI;
+import java.security.InvalidKeyException;
+import java.security.Key;
+import java.security.KeyManagementException;
 import java.security.PublicKey;
 import java.security.Signature;
+import java.security.SignatureException;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import org.keycloak.dom.saml.v2.SAML2Object;
+import org.keycloak.dom.saml.v2.protocol.ExtensionsType;
+import org.keycloak.rotation.KeyLocator;
+import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
+import org.w3c.dom.Element;
 
 /**
  *
@@ -257,13 +266,44 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
     }
 
     private void validateSamlSignature(SAMLDocumentHolder holder, boolean postBinding, String paramKey) throws VerificationException {
+        KeyLocator signatureValidationKey = deployment.getIDP().getSignatureValidationKeyLocator();
         if (postBinding) {
-            verifyPostBindingSignature(holder.getSamlDocument(), deployment.getIDP().getSignatureValidationKey());
+            verifyPostBindingSignature(holder.getSamlDocument(), signatureValidationKey);
         } else {
-            verifyRedirectBindingSignature(deployment.getIDP().getSignatureValidationKey(), paramKey);
+            String keyId = getMessageSigningKeyId(holder.getSamlObject());
+            verifyRedirectBindingSignature(paramKey, signatureValidationKey, keyId);
         }
     }
 
+    private String getMessageSigningKeyId(SAML2Object doc) {
+        final ExtensionsType extensions;
+        if (doc instanceof RequestAbstractType) {
+            extensions = ((RequestAbstractType) doc).getExtensions();
+        } else if (doc instanceof StatusResponseType) {
+            extensions = ((StatusResponseType) doc).getExtensions();
+        } else {
+            return null;
+        }
+
+        if (extensions == null) {
+            return null;
+        }
+
+        for (Object ext : extensions.getAny()) {
+            if (! (ext instanceof Element)) {
+                continue;
+            }
+
+            String res = KeycloakKeySamlExtensionGenerator.getMessageSigningKeyIdFromElement((Element) ext);
+
+            if (res != null) {
+                return res;
+            }
+        }
+
+        return null;
+    }
+
     private boolean checkStatusCodeValue(StatusCodeType statusCode, String expectedValue){
         if(statusCode != null && statusCode.getValue()!=null){
             String v = statusCode.getValue().toString();
@@ -473,10 +513,10 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
         return false;
     }
 
-    public void verifyPostBindingSignature(Document document, PublicKey publicKey) throws VerificationException {
+    public void verifyPostBindingSignature(Document document, KeyLocator keyLocator) throws VerificationException {
         SAML2Signature saml2Signature = new SAML2Signature();
         try {
-            if (!saml2Signature.validate(document, publicKey)) {
+            if (!saml2Signature.validate(document, keyLocator)) {
                 throw new VerificationException("Invalid signature on document");
             }
         } catch (ProcessingException e) {
@@ -484,7 +524,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
         }
     }
 
-    public void verifyRedirectBindingSignature(PublicKey publicKey, String paramKey) throws VerificationException {
+    private void verifyRedirectBindingSignature(String paramKey, KeyLocator keyLocator, String keyId) throws VerificationException {
         String request = facade.getRequest().getQueryParamValue(paramKey);
         String algorithm = facade.getRequest().getQueryParamValue(GeneralConstants.SAML_SIG_ALG_REQUEST_KEY);
         String signature = facade.getRequest().getQueryParamValue(GeneralConstants.SAML_SIGNATURE_REQUEST_KEY);
@@ -511,16 +551,80 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
         try {
             //byte[] decodedSignature = RedirectBindingUtil.urlBase64Decode(signature);
             byte[] decodedSignature = Base64.decode(signature);
+            byte[] rawQueryBytes = rawQuery.getBytes("UTF-8");
 
             SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm.getFromXmlMethod(decodedAlgorithm);
-            Signature validator = signatureAlgorithm.createSignature(); // todo plugin signature alg
-            validator.initVerify(publicKey);
-            validator.update(rawQuery.getBytes("UTF-8"));
-            if (!validator.verify(decodedSignature)) {
+
+            if (! validateRedirectBindingSignature(signatureAlgorithm, rawQueryBytes, decodedSignature, keyLocator, keyId)) {
                 throw new VerificationException("Invalid query param signature");
             }
         } catch (Exception e) {
             throw new VerificationException(e);
         }
     }
+
+    private boolean validateRedirectBindingSignature(SignatureAlgorithm sigAlg, byte[] rawQueryBytes, byte[] decodedSignature, KeyLocator locator, String keyId)
+      throws KeyManagementException, VerificationException {
+        try {
+            Key key;
+            try {
+                key = locator.getKey(keyId);
+                boolean keyLocated = key != null;
+
+                if (validateRedirectBindingSignatureForKey(sigAlg, rawQueryBytes, decodedSignature, key)) {
+                    return true;
+                }
+
+                if (keyLocated) {
+                    return false;
+                }
+            } catch (KeyManagementException ex) {
+            }
+        } catch (SignatureException ex) {
+            log.debug("Verification failed for key %s: %s", keyId, ex);
+            log.trace(ex);
+        }
+
+        if (locator instanceof Iterable) {
+            Iterable<Key> availableKeys = (Iterable<Key>) locator;
+
+            log.trace("Trying hard to validate XML signature using all available keys.");
+
+            for (Key key : availableKeys) {
+                try {
+                    if (validateRedirectBindingSignatureForKey(sigAlg, rawQueryBytes, decodedSignature, key)) {
+                        return true;
+                    }
+                } catch (SignatureException ex) {
+                    log.debug("Verification failed: %s", ex);
+                }
+            }
+        }
+
+        return false;
+    }
+
+    private boolean validateRedirectBindingSignatureForKey(SignatureAlgorithm sigAlg, byte[] rawQueryBytes, byte[] decodedSignature, Key key)
+      throws SignatureException {
+        if (key == null) {
+            return false;
+        }
+
+        if (! (key instanceof PublicKey)) {
+            log.warnf("Unusable key for signature validation: %s", key);
+            return false;
+        }
+
+        Signature signature = sigAlg.createSignature(); // todo plugin signature alg
+        try {
+            signature.initVerify((PublicKey) key);
+        } catch (InvalidKeyException ex) {
+            log.warnf(ex, "Unusable key for signature validation: %s", key);
+            return false;
+        }
+
+        signature.update(rawQueryBytes);
+
+        return signature.verify(decodedSignature);
+    }
 }

adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/webbrowsersso/WebBrowserSsoAuthenticationHandler.java

@@ -84,6 +84,8 @@ public class WebBrowserSsoAuthenticationHandler extends AbstractSamlAuthenticati
             binding.signatureAlgorithm(deployment.getSignatureAlgorithm())
                     .signWith(null, deployment.getSigningKeyPair())
                     .signDocument();
+            // TODO: As part of KEYCLOAK-3810, add KeyID to the SAML document
+            //   <related DocumentBuilder>.addExtension(new KeycloakKeySamlExtensionGenerator(<key ID>));
         }
 
 
@@ -115,6 +117,8 @@ public class WebBrowserSsoAuthenticationHandler extends AbstractSamlAuthenticati
             binding.signatureAlgorithm(deployment.getSignatureAlgorithm());
             binding.signWith(null, deployment.getSigningKeyPair())
                     .signDocument();
+            // TODO: As part of KEYCLOAK-3810, add KeyID to the SAML document
+            //   <related DocumentBuilder>.addExtension(new KeycloakKeySamlExtensionGenerator(<key ID>));
         }
 
         binding.relayState("logout");