From 58d742bb5cab03b220f4b05344557c683bbab237 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Fri, 6 Sep 2024 11:49:25 -0400 Subject: [PATCH] fix: refining v2 hostname validation (#32659) closes: #32643 Signed-off-by: Steve Hawkins --- .../url/HostnameV2ProviderFactory.java | 50 +++++++++------ .../url/HostnameV2ProviderFactoryTest.java | 62 +++++++++++++++++++ .../testsuite/url/HostnameV2Test.java | 2 +- 3 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 services/src/test/java/org/keycloak/url/HostnameV2ProviderFactoryTest.java diff --git a/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java b/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java index abbf6bf29f..30cd290dc9 100644 --- a/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java +++ b/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java @@ -17,6 +17,10 @@ package org.keycloak.url; +import java.net.URI; +import java.util.Arrays; +import java.util.Optional; + import org.keycloak.Config; import org.keycloak.common.Profile; import org.keycloak.models.KeycloakSession; @@ -24,24 +28,16 @@ import org.keycloak.provider.EnvironmentDependentProviderFactory; import org.keycloak.urls.HostnameProvider; import org.keycloak.urls.HostnameProviderFactory; -import java.net.URI; -import java.util.Optional; -import java.util.regex.Pattern; - /** * @author Vaclav Muzikar */ public class HostnameV2ProviderFactory implements HostnameProviderFactory, EnvironmentDependentProviderFactory { + private static final String INVALID_HOSTNAME = "Provided hostname is neither a plain hostname nor a valid URL"; private String hostname; private URI hostnameUrl; private URI adminUrl; private Boolean backchannelDynamic; - // Simplified regexes for hostname validations; further validations are performed when instantiating URI object - private static final String hostnameStringPattern = "[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*"; - private static final Pattern hostnamePattern = Pattern.compile("^" + hostnameStringPattern + "$"); - private static final Pattern hostnameUrlPattern = Pattern.compile("^(http|https)://" + hostnameStringPattern + "(:\\d+)?(/[\\w-]+)*/?$"); - @Override public void init(Config.Scope config) { // Strict mode is used just for enforcing that hostname is set @@ -57,11 +53,10 @@ public class HostnameV2ProviderFactory implements HostnameProviderFactory, Envir // Set hostname, can be either a full URL, or just hostname if (hostnameRaw != null) { - if (hostnamePattern.matcher(hostnameRaw).matches()) { - hostname = hostnameRaw; - } - else { - hostnameUrl = validateAndCreateUri(hostnameRaw, "Provided hostname is neither a plain hostname or a valid URL"); + if (!(hostnameRaw.startsWith("http://") || hostnameRaw.startsWith("https://"))) { + validateAndSetHostname(hostnameRaw); + } else { + hostnameUrl = validateAndCreateUri(hostnameRaw, INVALID_HOSTNAME); } } @@ -78,17 +73,36 @@ public class HostnameV2ProviderFactory implements HostnameProviderFactory, Envir throw new IllegalArgumentException("hostname-backchannel-dynamic must be set to false if hostname is not provided as full URL"); } } + + private void validateAndSetHostname(String hostname) { + URI result; + try { + result = URI.create("http://"+hostname); + } + catch (IllegalArgumentException e) { + throw new IllegalArgumentException(INVALID_HOSTNAME, e); + } + if (result.getHost() == null || !result.getHost().equals(hostname)) { + throw new IllegalArgumentException(INVALID_HOSTNAME); + } + this.hostname = hostname; + } private URI validateAndCreateUri(String uri, String validationFailedMessage) { - if (!hostnameUrlPattern.matcher(uri).matches()) { - throw new IllegalArgumentException(validationFailedMessage); - } + URI result; try { - return URI.create(uri); + result = URI.create(uri); } catch (IllegalArgumentException e) { throw new IllegalArgumentException(validationFailedMessage, e); } + if (!Arrays.asList("http", "https").contains(result.getScheme())) { + throw new IllegalArgumentException(validationFailedMessage); + } + if (result.getRawUserInfo() != null || result.getRawQuery() != null || result.getRawFragment() != null) { + throw new IllegalArgumentException(validationFailedMessage); + } + return result; } @Override diff --git a/services/src/test/java/org/keycloak/url/HostnameV2ProviderFactoryTest.java b/services/src/test/java/org/keycloak/url/HostnameV2ProviderFactoryTest.java new file mode 100644 index 0000000000..7a3a6ac1b0 --- /dev/null +++ b/services/src/test/java/org/keycloak/url/HostnameV2ProviderFactoryTest.java @@ -0,0 +1,62 @@ +/* + * 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.url; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import org.junit.Test; +import org.keycloak.utils.ScopeUtil; + +public class HostnameV2ProviderFactoryTest { + + @Test + public void hostnameUrlValidationTest() throws IOException{ + assertHostname("https://my-example.com/auth.this", true); + assertHostname("https://my-example.com:8080", true); + assertHostname("https://my-example.com/auth%20this", true); + assertHostname("my-example.com", true); + assertHostname("192.196.0.0", true); + assertHostname("[2001:0000:130F:0000:0000:09C0:876A:130B]", true); + + assertHostname("https://my-example.com?auth.this", false); + assertHostname("my-example.com/auth.this", false); + assertHostname("https://my-example.com:8080#fragment", false); + assertHostname("my-example.com:8080", false); + assertHostname("ldap://my-example.com/auth%20this", false); + assertHostname("?my-example.com", false); + assertHostname("192.196.0.5555", false); + } + + private void assertHostname(String hostname, boolean valid) { + Map values = new HashMap<>(); + values.put("hostname", hostname); + HostnameV2ProviderFactory factory = new HostnameV2ProviderFactory(); + try { + factory.init(ScopeUtil.createScope(values)); + assertTrue(valid); + } catch (IllegalArgumentException e) { + assertFalse(valid); + } + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/url/HostnameV2Test.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/url/HostnameV2Test.java index 6bfc7278ad..aa0c8f9f2c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/url/HostnameV2Test.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/url/HostnameV2Test.java @@ -180,7 +180,7 @@ public class HostnameV2Test extends AbstractKeycloakTest { @Test public void testInvalidHostnameUrl() { - testStartupFailure("Provided hostname is neither a plain hostname or a valid URL", + testStartupFailure("Provided hostname is neither a plain hostname nor a valid URL", "htt://127.0.0.1.nip.io", null, null, true); }