From f1a7a4804e6b45318d63fc24a18ce9c23e245897 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Sat, 28 Sep 2024 04:48:09 -0400 Subject: [PATCH] fix: adds additional info / warnings to hostname v2 (#33261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: adds additional info / warnings to hostname v2 closes: #24815 Signed-off-by: Steve Hawkins * refining the proxy-headers language from #33209 Signed-off-by: Steve Hawkins * adding hostname-strict-https Signed-off-by: Steve Hawkins * moving removed property check to the quarkus side Signed-off-by: Steve Hawkins * Update quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/HostnameV2PropertyMappers.java Co-authored-by: Martin Bartoš Signed-off-by: Steven Hawkins * Update docs/guides/server/hostname.adoc Signed-off-by: Steven Hawkins --------- Signed-off-by: Steve Hawkins Signed-off-by: Steven Hawkins Co-authored-by: Martin Bartoš --- docs/guides/server/hostname.adoc | 2 +- docs/guides/server/reverseproxy.adoc | 2 +- .../cli/command/AbstractStartCommand.java | 3 +++ .../mappers/HostnameV2PropertyMappers.java | 27 ++++++++++++++----- .../endpoints/LoginStatusIframeEndpoint.java | 2 +- .../url/HostnameV2ProviderFactory.java | 7 ++++- 6 files changed, 33 insertions(+), 10 deletions(-) diff --git a/docs/guides/server/hostname.adoc b/docs/guides/server/hostname.adoc index 807f59d12e..30ef8e1123 100644 --- a/docs/guides/server/hostname.adoc +++ b/docs/guides/server/hostname.adoc @@ -48,7 +48,7 @@ The result of this configuration is that you can continue to access {project_nam == Using a reverse proxy -When a proxy is in use, the `proxy-headers` should be set. Depending on the hostname settings, some or all of the URL, may be dynamically determined. +When a proxy is forwarding http or reencrypted TLS requests, the `proxy-headers` option should be set. Depending on the hostname settings, some or all of the URL, may be dynamically determined. WARNING: If either `forwarded` or `xforwarded` is selected, make sure your reverse proxy properly sets and overwrites the `Forwarded` or `X-Forwarded-*` headers respectively. To set these headers, consult the documentation for your reverse proxy. Misconfiguration will leave {project_name} exposed to security vulnerabilities. diff --git a/docs/guides/server/reverseproxy.adoc b/docs/guides/server/reverseproxy.adoc index 4a8377e041..931c6ec21d 100644 --- a/docs/guides/server/reverseproxy.adoc +++ b/docs/guides/server/reverseproxy.adoc @@ -18,7 +18,7 @@ Distributed environments frequently require the use of a reverse proxy. {project * `forwarded` enables parsing of the `Forwarded` header as per https://www.rfc-editor.org/rfc/rfc7239.html[RFC7239]. * `xforwarded` enables parsing of non-standard `X-Forwarded-*` headers, such as `X-Forwarded-For`, `X-Forwarded-Proto`, `X-Forwarded-Host`, and `X-Forwarded-Port`. -NOTE: If you are using a reverse proxy and do not set the `proxy-headers` option, then by default you will see 403 Forbidden responses to requests via the proxy that perform origin checking. +NOTE: If you are using a reverse proxy for anything other than https passthrough and do not set the `proxy-headers` option, then by default you will see 403 Forbidden responses to requests via the proxy that perform origin checking. For example: diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java index 43b942cab5..f50e3aa791 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/command/AbstractStartCommand.java @@ -23,7 +23,9 @@ import org.keycloak.quarkus.runtime.KeycloakMain; import org.keycloak.quarkus.runtime.Messages; import org.keycloak.quarkus.runtime.cli.ExecutionExceptionHandler; import org.keycloak.quarkus.runtime.configuration.ConfigArgsConfigSource; +import org.keycloak.quarkus.runtime.configuration.mappers.HostnameV2PropertyMappers; import org.keycloak.quarkus.runtime.configuration.mappers.HttpPropertyMappers; +import org.keycloak.url.HostnameV2ProviderFactory; import java.util.EnumSet; import java.util.List; @@ -44,6 +46,7 @@ public abstract class AbstractStartCommand extends AbstractCommand implements Ru doBeforeRun(); CommandLine cmd = spec.commandLine(); HttpPropertyMappers.validateConfig(); + HostnameV2PropertyMappers.validateConfig(); validateConfig(); if (ConfigArgsConfigSource.getAllCliArgs().contains(OPTIMIZED_BUILD_OPTION_LONG) && !wasBuildEverRun()) { diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/HostnameV2PropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/HostnameV2PropertyMappers.java index 5fd30fe062..004a8847c7 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/HostnameV2PropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/HostnameV2PropertyMappers.java @@ -1,13 +1,20 @@ package org.keycloak.quarkus.runtime.configuration.mappers; -import org.keycloak.common.Profile; -import org.keycloak.config.HostnameV2Options; - -import java.util.stream.Stream; - import static org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper.fromOption; -final class HostnameV2PropertyMappers { +import java.util.Arrays; +import java.util.List; +import java.util.stream.Stream; + +import org.jboss.logging.Logger; +import org.keycloak.common.Profile; +import org.keycloak.config.HostnameV2Options; +import org.keycloak.quarkus.runtime.configuration.Configuration; + +public final class HostnameV2PropertyMappers { + + private static final Logger LOGGER = Logger.getLogger(PropertyMappers.class); + private static final List REMOVED_OPTIONS = Arrays.asList("hostname-admin-url", "hostname-path", "hostname-port", "hostname-strict-backchannel", "hostname-url", "proxy", "hostname-strict-https"); private HostnameV2PropertyMappers(){} @@ -28,5 +35,13 @@ final class HostnameV2PropertyMappers { .map(b -> b.isEnabled(() -> Profile.isFeatureEnabled(Profile.Feature.HOSTNAME_V2), "hostname:v2 feature is enabled").build()) .toArray(s -> new PropertyMapper[s]); } + + public static void validateConfig() { + List inUse = REMOVED_OPTIONS.stream().filter(s -> Configuration.getOptionalKcValue(s).isPresent()).toList(); + + if (!inUse.isEmpty()) { + LOGGER.errorf("Hostname v1 options %s are still in use, please review your configuration", inUse); + } + } } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java index 705c958159..8fcd6d106c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/LoginStatusIframeEndpoint.java @@ -68,7 +68,7 @@ public class LoginStatusIframeEndpoint { if (validWebOrigins.contains("*") || validWebOrigins.contains(origin)) { return Response.noContent().build(); } - logger.debugf("client %s does not allow origin=%s for requestOrigin=%s (as determined by hostname settings), init will return a 403", clientId, origin, requestOrigin); + logger.debugf("client %s does not allow origin=%s for requestOrigin=%s (as determined by the proxy-header setting), init will return a 403", clientId, origin, requestOrigin); } else { logger.debugf("client %s does not exist or not enabled, init will return a 403", clientId); } diff --git a/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java b/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java index 3f79f7c1be..8b38ac17fc 100644 --- a/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java +++ b/services/src/main/java/org/keycloak/url/HostnameV2ProviderFactory.java @@ -21,6 +21,7 @@ import java.net.URI; import java.util.Arrays; import java.util.Optional; +import org.jboss.logging.Logger; import org.keycloak.Config; import org.keycloak.common.Profile; import org.keycloak.models.KeycloakSession; @@ -32,6 +33,9 @@ import org.keycloak.urls.HostnameProviderFactory; * @author Vaclav Muzikar */ public class HostnameV2ProviderFactory implements HostnameProviderFactory, EnvironmentDependentProviderFactory { + + private static final Logger LOGGER = Logger.getLogger(HostnameV2ProviderFactory.class); + private static final String INVALID_HOSTNAME = "Provided hostname is neither a plain hostname nor a valid URL"; private String hostname; private URI hostnameUrl; @@ -41,7 +45,7 @@ public class HostnameV2ProviderFactory implements HostnameProviderFactory, Envir @Override public void init(Config.Scope config) { // Strict mode is used just for enforcing that hostname is set - Boolean strictMode = config.getBoolean("hostname-strict", false); + boolean strictMode = config.getBoolean("hostname-strict", false); String hostnameRaw = config.get("hostname"); if (strictMode && hostnameRaw == null) { @@ -49,6 +53,7 @@ public class HostnameV2ProviderFactory implements HostnameProviderFactory, Envir } else if (hostnameRaw != null && !strictMode) { // We might not need this validation as it doesn't matter in this case if strict is true or false. It's just for consistency – hostname XOR !strict. // throw new IllegalArgumentException("hostname is configured, hostname-strict must be set to true"); + LOGGER.info("If hostanme is specified, hostname-strict is effectively ignored"); } // Set hostname, can be either a full URL, or just hostname