From 4c2542b91f85a98dcbaeb392934e15620b19b3f0 Mon Sep 17 00:00:00 2001 From: Ricardo Martin Date: Thu, 18 Apr 2024 16:06:50 +0200 Subject: [PATCH] Better management of domains in TrustedHostClientRegistrationPolicy (#139) (#28876) Closes keycloak/keycloak-private#63 Signed-off-by: rmartinc --- .../TrustedHostClientRegistrationPolicy.java | 61 +++++---- .../{policies => policy}/impl/HostsTest.java | 2 +- ...ustedHostClientRegistrationPolicyTest.java | 125 ++++++++++++++++++ 3 files changed, 161 insertions(+), 27 deletions(-) rename services/src/test/java/org/keycloak/services/clientregistration/{policies => policy}/impl/HostsTest.java (97%) create mode 100644 services/src/test/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicyTest.java diff --git a/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicy.java b/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicy.java index 6d79c701b4..65d26fbc66 100644 --- a/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicy.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicy.java @@ -23,7 +23,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.UnknownHostException; -import java.util.LinkedList; +import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -97,6 +97,10 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo String hostAddress = session.getContext().getConnection().getRemoteAddr(); + verifyHost(hostAddress); + } + + protected void verifyHost(String hostAddress) throws ClientRegistrationPolicyException { logger.debugf("Verifying remote host : %s", hostAddress); List trustedHosts = getTrustedHosts(); @@ -130,20 +134,9 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo protected List getTrustedDomains() { - List trustedHostsConfig = componentModel.getConfig().getList(TrustedHostClientRegistrationPolicyFactory.TRUSTED_HOSTS); - List domains = new LinkedList<>(); - - for (String hostname : trustedHostsConfig) { - if (hostname.startsWith("*.")) { - hostname = hostname.substring(2); - domains.add(hostname); - } - } - - return domains; + return componentModel.getConfig().getList(TrustedHostClientRegistrationPolicyFactory.TRUSTED_HOSTS); } - protected String verifyHostInTrustedHosts(String hostAddress, List trustedHosts) { for (String confHostName : trustedHosts) { try { @@ -162,23 +155,39 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo return null; } + private boolean checkTrustedDomain(String hostname, String trustedDomain) { + if (trustedDomain.startsWith("*.")) { + String domain = trustedDomain.substring(2); + return hostname.equals(domain) || hostname.endsWith("." + domain); + } + return hostname.equals(trustedDomain); + } protected String verifyHostInTrustedDomains(String hostAddress, List trustedDomains) { - if (!trustedDomains.isEmpty()) { - try { - String hostname = InetAddress.getByName(hostAddress).getHostName(); + try { + InetAddress address = InetAddress.getByName(hostAddress); + String hostname = address.getHostName(); - logger.debugf("Trying verify request from address '%s' of host '%s' by domains", hostAddress, hostname); + logger.debugf("Trying verify request from address '%s' of host '%s' by domains", hostAddress, hostname); - for (String confDomain : trustedDomains) { - if (hostname.endsWith(confDomain)) { - logger.debugf("Successfully verified host '%s' by trusted domain '%s'", hostname, confDomain); - return hostname; - } - } - } catch (UnknownHostException uhe) { - logger.debugf(uhe, "Request of address '%s' came from unknown host. Skip verification by domains", hostAddress); + if (hostname.equals(address.getHostAddress())) { + logger.debugf("The hostAddress '%s' was not resolved to a hostname", hostAddress); + return null; } + + if (Arrays.stream(InetAddress.getAllByName(hostname)).filter(a -> address.equals(a)).findAny().isEmpty()) { + logger.debugf("The hostAddress '%s' is not among the direct lookups returned resolving '%s'", hostAddress, hostname); + return null; + } + + for (String confDomain : trustedDomains) { + if (checkTrustedDomain(hostname, confDomain)) { + logger.debugf("Successfully verified host '%s' by trusted domain '%s'", hostname, confDomain); + return hostname; + } + } + } catch (UnknownHostException uhe) { + logger.debugf(uhe, "Request of address '%s' came from unknown host. Skip verification by domains", hostAddress); } return null; @@ -260,7 +269,7 @@ public class TrustedHostClientRegistrationPolicy implements ClientRegistrationPo } for (String trustedDomain : trustedDomains) { - if (host.endsWith(trustedDomain)) { + if (checkTrustedDomain(host, trustedDomain)) { return true; } } diff --git a/services/src/test/java/org/keycloak/services/clientregistration/policies/impl/HostsTest.java b/services/src/test/java/org/keycloak/services/clientregistration/policy/impl/HostsTest.java similarity index 97% rename from services/src/test/java/org/keycloak/services/clientregistration/policies/impl/HostsTest.java rename to services/src/test/java/org/keycloak/services/clientregistration/policy/impl/HostsTest.java index 5ca4407f6b..98699f7a3b 100644 --- a/services/src/test/java/org/keycloak/services/clientregistration/policies/impl/HostsTest.java +++ b/services/src/test/java/org/keycloak/services/clientregistration/policy/impl/HostsTest.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.keycloak.services.clientregistration.policies.impl; +package org.keycloak.services.clientregistration.policy.impl; import java.net.InetAddress; diff --git a/services/src/test/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicyTest.java b/services/src/test/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicyTest.java new file mode 100644 index 0000000000..0ffbc23324 --- /dev/null +++ b/services/src/test/java/org/keycloak/services/clientregistration/policy/impl/TrustedHostClientRegistrationPolicyTest.java @@ -0,0 +1,125 @@ +/* + * Copyright 2024 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.services.clientregistration.policy.impl; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.keycloak.common.Profile; +import org.keycloak.common.crypto.CryptoIntegration; +import org.keycloak.common.crypto.CryptoProvider; +import org.keycloak.component.ComponentModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.services.clientregistration.policy.ClientRegistrationPolicyException; +import org.keycloak.services.resteasy.ResteasyKeycloakSession; +import org.keycloak.services.resteasy.ResteasyKeycloakSessionFactory; + +/** + * + * @author rmartinc + */ +public class TrustedHostClientRegistrationPolicyTest { + + private static KeycloakSession session; + + @BeforeClass + public static void beforeClass() { + Profile.defaults(); + CryptoIntegration.init(CryptoProvider.class.getClassLoader()); + ResteasyKeycloakSessionFactory sessionFactory = new ResteasyKeycloakSessionFactory(); + sessionFactory.init(); + session = new ResteasyKeycloakSession(sessionFactory); + } + + @Test + public void testLocalhostName() { + TrustedHostClientRegistrationPolicyFactory factory = new TrustedHostClientRegistrationPolicyFactory(); + ComponentModel model = createComponentModel("localhost"); + TrustedHostClientRegistrationPolicy policy = (TrustedHostClientRegistrationPolicy) factory.create(session, model); + + policy.verifyHost("127.0.0.1"); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.verifyHost("10.0.0.1")); + policy.checkURLTrusted("https://localhost", policy.getTrustedHosts(), policy.getTrustedDomains()); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.checkURLTrusted("https://otherhost", + policy.getTrustedHosts(), policy.getTrustedDomains())); + } + + @Test + public void testLocalhostDomain() { + TrustedHostClientRegistrationPolicyFactory factory = new TrustedHostClientRegistrationPolicyFactory(); + ComponentModel model = createComponentModel("*.localhost"); + TrustedHostClientRegistrationPolicy policy = (TrustedHostClientRegistrationPolicy) factory.create(session, model); + + policy.verifyHost("127.0.0.1"); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.verifyHost("10.0.0.1")); + policy.checkURLTrusted("https://localhost", policy.getTrustedHosts(), policy.getTrustedDomains()); + policy.checkURLTrusted("https://other.localhost", policy.getTrustedHosts(), policy.getTrustedDomains()); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.checkURLTrusted("https://otherlocalhost", + policy.getTrustedHosts(), policy.getTrustedDomains())); + } + + @Test + public void testLocalhostIP() { + TrustedHostClientRegistrationPolicyFactory factory = new TrustedHostClientRegistrationPolicyFactory(); + ComponentModel model = createComponentModel("127.0.0.1"); + TrustedHostClientRegistrationPolicy policy = (TrustedHostClientRegistrationPolicy) factory.create(session, model); + + policy.verifyHost("127.0.0.1"); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.verifyHost("10.0.0.1")); + policy.checkURLTrusted("https://127.0.0.1", policy.getTrustedHosts(), policy.getTrustedDomains()); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.checkURLTrusted("https://localhost", + policy.getTrustedHosts(), policy.getTrustedDomains())); + } + + @Test + public void testGoogleCrawlBot() { + // https://developers.google.com/search/blog/2006/09/how-to-verify-googlebot + TrustedHostClientRegistrationPolicyFactory factory = new TrustedHostClientRegistrationPolicyFactory(); + ComponentModel model = createComponentModel("*.googlebot.com"); + TrustedHostClientRegistrationPolicy policy = (TrustedHostClientRegistrationPolicy) factory.create(session, model); + + policy.verifyHost("66.249.66.1"); + policy.checkURLTrusted("https://www.googlebot.com", policy.getTrustedHosts(), policy.getTrustedDomains()); + policy.checkURLTrusted("https://googlebot.com", policy.getTrustedHosts(), policy.getTrustedDomains()); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.checkURLTrusted("https://www.othergooglebot.com", + policy.getTrustedHosts(), policy.getTrustedDomains())); + } + + @Test + public void testGithubDomain() throws UnknownHostException { + TrustedHostClientRegistrationPolicyFactory factory = new TrustedHostClientRegistrationPolicyFactory(); + ComponentModel model = createComponentModel("*.github.com"); + TrustedHostClientRegistrationPolicy policy = (TrustedHostClientRegistrationPolicy) factory.create(session, model); + + policy.verifyHost(InetAddress.getByName("www.github.com").getHostAddress()); + policy.verifyHost(InetAddress.getByName("github.com").getHostAddress()); + policy.checkURLTrusted("https://www.github.com", policy.getTrustedHosts(), policy.getTrustedDomains()); + policy.checkURLTrusted("https://github.com", policy.getTrustedHosts(), policy.getTrustedDomains()); + Assert.assertThrows(ClientRegistrationPolicyException.class, () -> policy.checkURLTrusted("https://othergithub.com", + policy.getTrustedHosts(), policy.getTrustedDomains())); + } + + private ComponentModel createComponentModel(String... hosts) { + ComponentModel model = new ComponentModel(); + model.put(TrustedHostClientRegistrationPolicyFactory.HOST_SENDING_REGISTRATION_REQUEST_MUST_MATCH, "true"); + model.put(TrustedHostClientRegistrationPolicyFactory.CLIENT_URIS_MUST_MATCH, "true"); + model.getConfig().addAll(TrustedHostClientRegistrationPolicyFactory.TRUSTED_HOSTS, hosts); + return model; + } +}