fix: enables sni for tls passthrough to determine misdirected requests#50607
Merged
Conversation
Contributor
|
Also, I realized some test for this would be good. |
Contributor
Author
Yes, I just wanted to make sure the approach was acceptable before investing too much more time. |
michalvavrik
approved these changes
Jul 15, 2026
michalvavrik
left a comment
Member
There was a problem hiding this comment.
I understand there are some details to be finalized, but LGTM.
TBH I have not figured why are we implementing it instead of providing such a feature upstream (Quarkus or Vert.x with config option in Quarkus). There can be something specific to Keycloak, I am just not sure.
Contributor
Author
That was a thought I had on the private issue as well. I think based upon what is being done here that it could be. Basically it could be an option like I'll push another commit here based upon removing any explicit configuration, and we can decide whether to wait for something from upstream. |
Member
|
sounds good, thanks |
Contributor
Author
|
Removed the explicit configuration and added an AI generated test - I could not for the life of me find an httpclient that allows you to set the authority differently from the indicated name. The only other piece here would be some documentation about not using keycloak as the default backend when you are using a wildcard cert. #50896 was opened as well to remove the operators usage of defaultBackend at the ingress level. |
michalvavrik
self-requested a review
July 16, 2026 13:52
| } | ||
|
|
||
| var misdirectedRequestFilter = recorder.getMisdirectedRequestFilter(); | ||
| if (misdirectedRequestFilter != null) { |
Member
There was a problem hiding this comment.
@shawkins just because getMisdirectedRequestFilter is recorder in the order of invocation of the build steps (for now, interesting design plans upstream), that doesn't mean that it is recorded now, so it doesn't mean that you actually have response here.
I think this condition is always true, I tried to verify it and I did this:
diff --git a/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java b/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java
index 0689ab2b33..b62924a7c2 100644
--- a/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java
+++ b/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java
@@ -301,9 +301,14 @@ class KeycloakProcessor {
}
var misdirectedRequestFilter = recorder.getMisdirectedRequestFilter();
+ String msgis = "///".repeat(1000);
if (misdirectedRequestFilter != null) {
filters.produce(new FilterBuildItem(misdirectedRequestFilter, SecurityHandlerPriorities.CORS + 2));
+ msgis += " not null";
+ } else {
+ msgis += " null";
}
+ logger.error(msgis);
}
@Record(ExecutionTime.STATIC_INIT)
diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
index 281dd05134..ae50b0239d 100644
--- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
+++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
@@ -140,7 +140,7 @@ public class KeycloakRecorder {
public Handler<RoutingContext> getMisdirectedRequestFilter() {
// not checking for http/2 enablement - it is enabled by default and not exposed as a supported configuration option
- if (!Configuration.isTrue(HttpPropertyMappers.QUARKUS_HTTPS_SNI) || !HttpPropertyMappers.isHttpsEnabled() || Configuration.getConfigValue(ProxyOptions.PROXY_HEADERS).getValue() != null) {
+ if (true || !Configuration.isTrue(HttpPropertyMappers.QUARKUS_HTTPS_SNI) || !HttpPropertyMappers.isHttpsEnabled() || Configuration.getConfigValue(ProxyOptions.PROXY_HEADERS).getValue() != null) {
return null;
}which returned:
2026-07-16 16:11:12,456 ERROR [org.keycloak.quarkus.deployment.KeycloakProcessor] (build-5) //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// not nulland
[ERROR] org.keycloak.it.cli.dist.HttpDistTest.misdirectedRequestDetection -- Time elapsed: 17.87 s <<< FAILURE!
java.lang.AssertionError:
Expected HTTP 421 Misdirected Request for SNI/authority mismatch
Expected: is "421"
but: was "200"
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.keycloak.it.cli.dist.HttpDistTest.misdirectedRequestDetection(HttpDistTest.java:128)
[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] HttpDistTest.misdirectedRequestDetection:128 Expected HTTP 421 Misdirected Request for SNI/authority mismatch
Expected: is "421"
but: was "200"
(and if you are going to fix it, we shouldn't return null handler in the build item, just in case you considered it)
I think you have 2 options:
- either provide a handler that is always present and just does
next()when we don't need it - or you can add a route handler based on a runtime config as I do in Quarkus for a form auth handler:
- https://github.com/quarkusio/quarkus/blob/a84fae1ba8ef1c2ad83fe41f0c7b5c2cd1738950/extensions/vertx-http/deployment/src/main/java/io/quarkus/vertx/http/deployment/HttpSecurityProcessor.java#L127
- https://github.com/quarkusio/quarkus/blob/a84fae1ba8ef1c2ad83fe41f0c7b5c2cd1738950/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java#L110
Contributor
Author
There was a problem hiding this comment.
I was following the pattern of the getRejectNonNormalizedPathFilter - does this mean we have a bug there?
Member
There was a problem hiding this comment.
getRejectNonNormalizedPathFilter
I haven't mentioned. It just means that you are always producing FilterBuildItem because that is not how the byte recording works. From the fact that Keycloak works, I'd say that Quarkus handles null for the hanlder. So maybe drop the if as it makes a wrong impression?
Contributor
Author
There was a problem hiding this comment.
I see, we should at least be using Optional or Supplier as the return value, rather than just the raw value and using null. I'll correct both spots here, or another issue can be captured to correct getRejectNonNormalizedPathFilter
Contributor
Author
There was a problem hiding this comment.
Updated the recorder to consume the router runtimevalue instead.
closes: keycloak#50602 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Signed-off-by: Steve Hawkins <shawkins@redhat.com>
| status, Matchers.is("421")); | ||
| } | ||
|
|
||
| private static String http2Request(javax.net.ssl.SSLSocketFactory factory, String indicatedHostname, String authority) throws IOException, SocketException { |
Member
There was a problem hiding this comment.
This is very complex and hard to understand. You said you didn't find suitable HTTP client. I tried using Vert.x HTTP client, and my agent produced a POC (see at the end of the comment) which looks legit to me. I think that the difference in ports (like sending example:433) is just cosmetic as we are only comparing the authority host.
So could your test look like this instead?
@Test
@Launch({"start-dev", "--hostname=https://example.com"})
public void misdirectedRequestDetection() throws Exception {
Vertx vertx = Vertx.vertx();
try {
HttpClient client = vertx.createHttpClient(new HttpClientOptions()
.setSsl(true)
.setTrustAll(true)
.setVerifyHost(false)
.setProtocolVersion(HttpVersion.HTTP_2)
.setUseAlpn(true));
try {
assertThat("Matching indicated to authority is allowed",
misdirectedRequest(client, "servicehost.com", "servicehost.com"), Matchers.is(200));
// null sniHostname → defaults to "localhost" (non-FQDN → Java skips SNI → indicatedServerName is null)
assertThat("No indicated name is allowed",
misdirectedRequest(client, null, "example.com"), Matchers.is(200));
// connection originated from another backend, but we're reusing it for a request to the keycloak server
assertThat("Matching a known host is allowed",
misdirectedRequest(client, "other-example.com", "example.com"), Matchers.is(200));
// connection originated from keycloak, but the proxy is mistakenly reusing for another service
assertThat("Expected HTTP 421 Misdirected Request for SNI/authority mismatch",
misdirectedRequest(client, "example.com", "misdirected.com"), Matchers.is(421));
} finally {
client.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
}
} finally {
vertx.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
}
}
private static int misdirectedRequest(HttpClient client, String sniHostname, String authorityHost) throws Exception {
RequestOptions options = new RequestOptions()
.setServer(SocketAddress.inetSocketAddress(8443, "localhost"))
.setPort(8443)
.setSsl(true)
.setURI("/realms/master")
.setMethod(HttpMethod.GET);
if (sniHostname != null) {
options.setHost(sniHostname);
}
return client.request(options)
.compose(req -> {
req.authority(HostAndPort.create(authorityHost, 8443));
return req.send();
})
.map(HttpClientResponse::statusCode)
.toCompletionStage()
.toCompletableFuture()
.get(10, TimeUnit.SECONDS);
}I tried it and the test is passing.
POC:
diff --git a/di b/di
new file mode 100644
index 0000000000..1dfe2f8895
--- /dev/null
+++ b/di
@@ -0,0 +1,32 @@
+diff --git a/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java b/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java
+index 0689ab2b33..b62924a7c2 100644
+--- a/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java
++++ b/quarkus/deployment/src/main/java/org/keycloak/quarkus/deployment/KeycloakProcessor.java
+@@ -301,9 +301,14 @@ class KeycloakProcessor {
+ }
+
+ var misdirectedRequestFilter = recorder.getMisdirectedRequestFilter();
++ String msgis = "///".repeat(1000);
+ if (misdirectedRequestFilter != null) {
+ filters.produce(new FilterBuildItem(misdirectedRequestFilter, SecurityHandlerPriorities.CORS + 2));
++ msgis += " not null";
++ } else {
++ msgis += " null";
+ }
++ logger.error(msgis);
+ }
+
+ @Record(ExecutionTime.STATIC_INIT)
+diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
+index 281dd05134..ae50b0239d 100644
+--- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
++++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/KeycloakRecorder.java
+@@ -140,7 +140,7 @@ public class KeycloakRecorder {
+
+ public Handler<RoutingContext> getMisdirectedRequestFilter() {
+ // not checking for http/2 enablement - it is enabled by default and not exposed as a supported configuration option
+- if (!Configuration.isTrue(HttpPropertyMappers.QUARKUS_HTTPS_SNI) || !HttpPropertyMappers.isHttpsEnabled() || Configuration.getConfigValue(ProxyOptions.PROXY_HEADERS).getValue() != null) {
++ if (true || !Configuration.isTrue(HttpPropertyMappers.QUARKUS_HTTPS_SNI) || !HttpPropertyMappers.isHttpsEnabled() || Configuration.getConfigValue(ProxyOptions.PROXY_HEADERS).getValue() != null) {
+ return null;
+ }
+
diff --git a/quarkus/deployment/src/test/java/test/org/keycloak/quarkus/services/http/HttpsSniProfile.java b/quarkus/deployment/src/test/java/test/org/keycloak/quarkus/services/http/HttpsSniProfile.java
new file mode 100644
index 0000000000..7d4cf2f8d4
--- /dev/null
+++ b/quarkus/deployment/src/test/java/test/org/keycloak/quarkus/services/http/HttpsSniProfile.java
@@ -0,0 +1,37 @@
+package test.org.keycloak.quarkus.services.http;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import io.quarkus.test.junit.QuarkusTestProfile;
+
+public class HttpsSniProfile implements QuarkusTestProfile {
+
+ @Override
+ public Map<String, String> getConfigOverrides() {
+ String certPath = resolveResourcePath("test-cert.pem");
+ String keyPath = resolveResourcePath("test-key.pem");
+
+ Map<String, String> config = new HashMap<>();
+ config.put("kc.http-enabled", "true");
+ config.put("kc.https-certificate-file", certPath);
+ config.put("kc.https-certificate-key-file", keyPath);
+ config.put("kc.hostname", "https://example.com");
+ config.put("kc.hostname-strict", "true");
+ config.put("kc.cluster", "local");
+ config.put("kc.db", "dev-mem");
+ config.put("kc.cache", "local");
+ config.put("kc.server-async-bootstrap", "false");
+ config.put("quarkus.class-loading.removed-artifacts",
+ "io.quarkus:quarkus-jdbc-oracle,io.quarkus:quarkus-jdbc-oracle-deployment");
+ return config;
+ }
+
+ private static String resolveResourcePath(String resource) {
+ var url = Thread.currentThread().getContextClassLoader().getResource(resource);
+ if (url == null) {
+ throw new IllegalStateException("Test resource not found: " + resource);
+ }
+ return url.getPath();
+ }
+}
diff --git a/quarkus/deployment/src/test/java/test/org/keycloak/quarkus/services/http/MisdirectedRequestTest.java b/quarkus/deployment/src/test/java/test/org/keycloak/quarkus/services/http/MisdirectedRequestTest.java
new file mode 100644
index 0000000000..30864790f6
--- /dev/null
+++ b/quarkus/deployment/src/test/java/test/org/keycloak/quarkus/services/http/MisdirectedRequestTest.java
@@ -0,0 +1,92 @@
+package test.org.keycloak.quarkus.services.http;
+
+import java.util.concurrent.TimeUnit;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.TestProfile;
+import io.vertx.core.Vertx;
+import io.vertx.core.http.HttpClient;
+import io.vertx.core.http.HttpClientOptions;
+import io.vertx.core.http.HttpClientResponse;
+import io.vertx.core.http.HttpMethod;
+import io.vertx.core.http.HttpVersion;
+import io.vertx.core.http.RequestOptions;
+import io.vertx.core.net.HostAndPort;
+import io.vertx.core.net.SocketAddress;
+
+import jakarta.inject.Inject;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@QuarkusTest
+@TestProfile(HttpsSniProfile.class)
+public class MisdirectedRequestTest {
+
+ private static final int HTTPS_PORT = 8444;
+
+ private static Vertx vertx;
+ private static HttpClient httpClient;
+
+ @BeforeAll
+ static void setUp() {
+ vertx = Vertx.vertx();
+ httpClient = vertx.createHttpClient(new HttpClientOptions()
+ .setSsl(true)
+ .setTrustAll(true)
+ .setVerifyHost(false)
+ .setProtocolVersion(HttpVersion.HTTP_2)
+ .setUseAlpn(true));
+ }
+
+ @AfterAll
+ static void tearDown() throws Exception {
+ if (httpClient != null) {
+ httpClient.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
+ }
+ if (vertx != null) {
+ vertx.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
+ }
+ }
+
+ @Test
+ void sniMatchesAuthority() throws Exception {
+ int status = sendRequest("example.com", "example.com");
+ assertEquals(200, status, "Matching SNI and authority should be allowed");
+ }
+
+ @Test
+ void sniDiffersFromAuthorityButAuthorityIsKnownHost() throws Exception {
+ int status = sendRequest("other.com", "example.com");
+ assertEquals(200, status, "Authority matching a known hostname should be allowed despite SNI mismatch");
+ }
+
+ @Test
+ void sniDiffersFromAuthorityAndAuthorityUnknown() throws Exception {
+ int status = sendRequest("example.com", "misdirected.com");
+ assertEquals(421, status, "SNI/authority mismatch with unknown authority should return 421");
+ }
+
+ private int sendRequest(String sniHostname, String authorityHost) throws Exception {
+ RequestOptions requestOptions = new RequestOptions()
+ .setServer(SocketAddress.inetSocketAddress(HTTPS_PORT, "localhost"))
+ .setHost(sniHostname)
+ .setPort(HTTPS_PORT)
+ .setSsl(true)
+ .setURI("/realms/master")
+ .setMethod(HttpMethod.GET);
+
+ return httpClient.request(requestOptions)
+ .compose(req -> {
+ req.authority(HostAndPort.create(authorityHost, HTTPS_PORT));
+ return req.send();
+ })
+ .map(HttpClientResponse::statusCode)
+ .toCompletionStage()
+ .toCompletableFuture()
+ .get(10, TimeUnit.SECONDS);
+ }
+}
diff --git a/quarkus/deployment/src/test/resources/test-cert.pem b/quarkus/deployment/src/test/resources/test-cert.pem
new file mode 100644
index 0000000000..955e4f7f7b
--- /dev/null
+++ b/quarkus/deployment/src/test/resources/test-cert.pem
@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDCTCCAfGgAwIBAgIUR6tnTKfOEWLYllEmtXK7e9v0eVgwDQYJKoZIhvcNAQEL
+BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI2MDcxNjE0NDg1NVoXDTM2MDcx
+MzE0NDg1NVowFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEF
+AAOCAQ8AMIIBCgKCAQEAnR/gZ206p3i7V/O1UaWBPAIS8xQM+a1duI/DQt8jaK3b
+QEHddbUqAN8A6jB9dLMbJfKqGof/+I0fVNSLwxVvw16vE06Z1O/AiyIYJ6Jh+OKy
+H+TQu7m08Q71iOr6J85Dc1saHpaM8wamRz1oqd2rVjrr/h/YrSCOZmcHXgdFElcY
+7kSEti8uHHnJ9hgNczcu5KM6NNjQqjBXprx7o6iE04KP4zFLk4PlTJ9Hzx/I9mMx
+DfZtH7fkajJmmEsdUyeyPm7atgUsmcoVDmDV5ATyR+3jNipdZtl3Bri85l5x+Y22
+3rgieQm2qJLCE7xMfSz3p8qlKuH8lAdCFEaoy0difQIDAQABo1MwUTAdBgNVHQ4E
+FgQUxgDc4JsMgitmz6qzVyJXZsZoYAgwHwYDVR0jBBgwFoAUxgDc4JsMgitmz6qz
+VyJXZsZoYAgwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAYjrG
+EnhHSvr6xES9ZDEQiCMWoxvr4hRzkpYGn2SVvyU34ZanJBtVH0aikFBod3RckU9j
+2opX9OSX+ixOeh+SB1M445cOU6oBYm74Y6fipIBIJGkRUHru0ZaDvYzDW3IudXIH
+Jf7tjczukB87SxzvEa2g1Nagi2ZLRZcJ9fhFREWtNGFjy+d+veOzMy1l0chs7RLV
+DBz+rIvURtm8jLBxgoZi6CkejyMy4o3dgPF7Wsd0DwZNad7EQM+pFBvNFlxYg/qq
+7MzE1LaMtqG2BNlssBgpH/u3ByLPfWzBZkiFUYSTKTcmLVCSAqc3YWRd5zf9GHsR
+xVHlaNebaYIvc9PToQ==
+-----END CERTIFICATE-----
diff --git a/quarkus/deployment/src/test/resources/test-key.pem b/quarkus/deployment/src/test/resources/test-key.pem
new file mode 100644
index 0000000000..6aae433b29
--- /dev/null
+++ b/quarkus/deployment/src/test/resources/test-key.pem
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCdH+BnbTqneLtX
+87VRpYE8AhLzFAz5rV24j8NC3yNordtAQd11tSoA3wDqMH10sxsl8qoah//4jR9U
+1IvDFW/DXq8TTpnU78CLIhgnomH44rIf5NC7ubTxDvWI6vonzkNzWxoelozzBqZH
+PWip3atWOuv+H9itII5mZwdeB0USVxjuRIS2Ly4cecn2GA1zNy7kozo02NCqMFem
+vHujqITTgo/jMUuTg+VMn0fPH8j2YzEN9m0ft+RqMmaYSx1TJ7I+btq2BSyZyhUO
+YNXkBPJH7eM2Kl1m2XcGuLzmXnH5jbbeuCJ5CbaoksITvEx9LPenyqUq4fyUB0IU
+RqjLR2J9AgMBAAECggEAEs7HwQGbaYQDYtwGens17L2v0H54LiPRKeA1honotmVH
+LxojPO1+VWPcO2wp/D/bMk/piir42iLkWvZlM2kbu3ZYPRalxc4cDVt7qjTfBth1
+62jrjmkEjm1UuTqtTnn7a3G+fuZVAVssqK7iWiDxWy1K5dyghlq1CO9wu5Hhs/fe
+SS4P+v3iAYj41K+VQpk3/KSkR1T3mt2h+BSVH+nVtp8jQD8PPH3azj+/cPRGnTUE
+ry765t2k7AfveFxl48sb+/INH7Kpcrtk1Jv8wunoVIa+6UFHrAru8jQxjnpZqlpM
+vTTV6gEJwCydKBUBaf+WcJVgyusZH7uC858XZ7OX2QKBgQDb+G7xwecDdESRUHvk
+OW0d6gU7ZzRj/Mg8A7k/Z2OojbFikslYd9Y+90VTBmwdRFkyn2YrS7LZKXCoceM7
+ohN6JNeWpUkDJdaujvHWjcmUu1oYw70+m73zS89RhDESc42ZcO5tViLSKTLgzl83
+tJnX+s8PCePsehz0SmSpPD8ZaQKBgQC23EE1r7+QUhKzGzkjF8k/aZlKX+vP2OQU
+PQYVggxmlQoNhl8S7pvYw8fb4NGkYtuy7Z2okiwhFkoLOF3WtFFffqWZCMmifDAM
+HJO0t0UGa5+U5U419e1UpvGeRP96V2ShclZ70CMmWDgQkQbZDFJEX9oNNuKAsQ1J
+9UBETrhp9QKBgQDMwJwspuvs+C3XAY7A6n9aR/okyDUpGSQdUO3/SbKnM3U5Jx8K
+HUotBCfgV2sNdxB8IYmNpYHNyFgYQyJb4Mq2eFLA5AtpRbBmfaI4r3RC4H3F7XVb
+MMcID7njy+ONpEfa2xbkaTgyXa0BOrfA93f4ZXqVKgShmy0Wa6T1LqIheQKBgCfS
+T2EhpNYJCYl3pDnANFNa68cGnJPm58SAhZKpf4nXaxkG6i11SPsrp5p6myzxWmbc
+2DidnH6YHAfVfpoDKrVhHy70evSnls8Mah4wyxda2KXUSfP2WnjU4klVPU2nOoLu
+I2dTLFRtYcg5zQP3avL47MNz8F+WLw6sGhLGEeMBAoGAbHm9TkPqnw6huvpKLOU4
+ymkEZ5Ht6uf7eRHsYFF+5xmlQrWDOp/vVQD3VpCfPqPM+XA/lrCYovfHEusJWuHa
+doM9SUcyIhk2YWpB2WGTzXt115AYBLmtAYb9H8XNrbqMF+bRc4cT5xD/heB2TrNF
+Pq9G5OTLzw8trtL8pl1V5tU=
+-----END PRIVATE KEY-----
Contributor
Author
There was a problem hiding this comment.
You said you didn't find suitable HTTP client. I tried using Vert.x HTTP client
My mistake was trying the vertx webclient :(
Using the vertx http client looks great, thank you. I just made some minor changes.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds SNI-based detection of HTTP/2 requests misdirected through TLS-passthrough proxies.
Changes:
- Enables SNI automatically for applicable HTTPS configurations.
- Returns HTTP 421 for unsafe SNI/authority mismatches.
- Adds integration tests, configuration tests, and proxy guidance.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
quarkus/tests/integration/.../HttpDistTest.java |
Tests SNI/authority handling. |
quarkus/runtime/.../ConfigurationTest.java |
Tests automatic SNI configuration. |
quarkus/runtime/.../MisdirectedFilter.java |
Implements HTTP 421 detection. |
quarkus/runtime/.../KeycloakRecorder.java |
Registers runtime request filters. |
quarkus/runtime/.../HttpPropertyMappers.java |
Maps automatic SNI enablement. |
quarkus/deployment/.../KeycloakProcessor.java |
Wires filters into Vert.x routers. |
quarkus/config-api/.../HttpOptions.java |
Defines the synthetic SNI option. |
docs/guides/server/reverseproxy.adoc |
Documents default-backend risks. |
incorporating the vertx httpclient Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Contributor
Author
|
Marked as ready for review. To recap, the only case where someone will see new / unnecessary 421's is if Keycloak is configured with realm frontend urls like: As realm frontend values are configurable at runtime, it will introduce some complexity / overhead in maintaining the known set - perhaps this could be generalized via a pattern if need be. Let's see if we can get consensus on whether in general functionality like this belongs upstream? Our general conditions are:
|
Contributor
Author
|
Based upon the quarkus allowed hosts feature, to future proof this PR the order value will need to change - so that the misdirected check happens before the allowed hosts check. Otherwise, if allowed hosts are specified a 400 would be seen rather than a 421. |
vmuzikar
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes: #50602
@ahus1 @vmuzikar thinking about my initial comment from the private issue more, I believe that satisfying the security hardening via sni is a valid approach.
To minimize potential issues, the sni enablement and the check shown here is initially limited to just the situation we are trying to harden - https requests on the main interface when https is enabled and forwarded headers are not set. I don't think it's currently a required hardening to otherwise do the misdirected check - and admins will typically notice those requests failing cors checks, as we mention in the docs
keycloak/docs/guides/server/reverseproxy.adoc
Line 179 in a4e6b73
The quarkus behavior with sni enabled, but when no sni information is sent, is to set the indicated hostname to null and supply the client with the default certificate. In our case the only supported configuration is a single cert, so that will automatically be the default.
When keycloak is in passthrough mode behind a proxy, it will automatically receive the sni information that the proxy used to make the routing decision.
To allow for requests not via the proxy it's sufficient to allow for a null indicated name or to check that the indicated name matches the authority host - as that means the request is going to the intended place. If you access for example via the kubernetes service host (as the operator is doing for the admin connection), via localhost, etc. - those requests will pass have an indicated name that matches the authority and are all currently valid regardless of whether dynamic backchannel is enabled.
If there is a mismatch between the indicated name and the authority host, there may be a misdirected request. To minimize the amount of 421 responses there is an additional check to see if the authority matches the hostname or the admin hostname - it is not simple to check against the realm frontend urls as this handler is running too early.
The potential problems or things to note with what is shown here:
keycloak/operator/src/main/java/org/keycloak/operator/controllers/KeycloakIngressDependentResource.java
Line 123 in a4e6b73