dataware/datawareauthic
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: returning addresses instead of hosts on the ClientConnection (#35247)

Browse source 
also consolidates checks of whether a host or address is local

closes: #35216
closes: #34671

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2024-11-22 14:57:20 -05:00 • committed by GitHub
parent 198214310e
commit 0eb0281bf2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 98 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
common/src/main/java/org/keycloak/common/ClientConnection.java
View file

@ -25,6 +25,9 @@ package org.keycloak.common;
*/
public interface ClientConnection {
/**
* @return the address as a string if it is available, otherwise null
*/
String getRemoteAddr();
String getRemoteHost();
int getRemotePort();

16
common/src/main/java/org/keycloak/common/enums/SslRequired.java
View file

@ -35,23 +35,29 @@ public enum SslRequired {
return isRequired(connection.getRemoteAddr());
}
public boolean isRequired(String address) {
public boolean isRequired(String host) {
switch (this) {
case ALL:
return true;
case NONE:
return false;
case EXTERNAL:
return !isLocal(address);
// NOTE: this is sometimes using hostnames here, which require DNS resolution
// It assumes that the resolution will be the same on the client side
// - this will go away once EXTERNAL is no longer supported
return !isLocal(host);
default:
return true;
}
}
private boolean isLocal(String remoteAddress) {
private boolean isLocal(String host) {
if (host == null || host.isEmpty()) {
return false; // InetAddress.getByName returns localhost for these
}
try {
InetAddress inetAddress = InetAddress.getByName(remoteAddress);
return inetAddress.isAnyLocalAddress() || inetAddress.isLoopbackAddress() || inetAddress.isSiteLocalAddress() || inetAddress.isLinkLocalAddress() || isUniqueLocal(inetAddress);
InetAddress inetAddress = InetAddress.getByName(host);
return inetAddress.isLoopbackAddress() || inetAddress.isSiteLocalAddress() || inetAddress.isLinkLocalAddress() || isUniqueLocal(inetAddress);
} catch (UnknownHostException e) {
return false;
}

20
common/src/test/java/org/keycloak/common/enums/SslRequiredTest.java Normal file
View file

@ -0,0 +1,20 @@
package org.keycloak.common.enums;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.io.IOException;
import org.junit.Test;
public class SslRequiredTest {
@Test
public void sslRequiredExternalTest() throws IOException {
assertFalse(SslRequired.EXTERNAL.isRequired("127.0.0.1"));
assertTrue(SslRequired.EXTERNAL.isRequired((String)null));
assertTrue(SslRequired.EXTERNAL.isRequired(""));
assertTrue(SslRequired.EXTERNAL.isRequired("0.0.0.0"));
}
}

4
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/resteasy/QuarkusClientConnection.java
View file

@ -31,7 +31,7 @@ public final class QuarkusClientConnection implements ClientConnection {
@Override
public String getRemoteAddr() {
return request.remoteAddress().host();
return request.remoteAddress().hostAddress();
}
@Override
@ -46,7 +46,7 @@ public final class QuarkusClientConnection implements ClientConnection {
@Override
public String getLocalAddr() {
return request.localAddress().host();
return request.localAddress().hostAddress();
}
@Override

22
quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ProxyHostnameV2DistTest.java vendored
View file

@ -18,6 +18,7 @@
package org.keycloak.it.cli.dist;
import io.quarkus.test.junit.main.Launch;
import io.quarkus.test.junit.main.LaunchResult;
import io.restassured.RestAssured;
import io.restassured.config.RedirectConfig;
import io.restassured.config.RestAssuredConfig;
@ -41,6 +42,9 @@ import static org.hamcrest.Matchers.containsString;
@RawDistOnly(reason = "Containers are immutable")
public class ProxyHostnameV2DistTest {
private static final String ADDRESS = "12.23.45.67";
private static final String NOT_ADDRESS = "notaddress";
@BeforeAll
public static void onBeforeAll() {
RestAssured.useRelaxedHTTPSValidation();
@ -78,10 +82,14 @@ public class ProxyHostnameV2DistTest {
}
@Test
@Launch({ "start-dev", "--hostname-strict=false", "--proxy-headers=forwarded" })
public void testForwardedProxyHeaders() {
@Launch({ "start-dev", "--hostname-strict=false", "--proxy-headers=forwarded", "--spi-event-listener-provider=jboss-logging" })
public void testForwardedProxyHeaders(LaunchResult result) {
assertForwardedHeader();
assertXForwardedHeadersAreIgnored();
CLIResult cliResult = (CLIResult)result;
cliResult.assertNoMessage(NOT_ADDRESS);
cliResult.assertMessage(ADDRESS);
}
@Test
@ -92,13 +100,15 @@ public class ProxyHostnameV2DistTest {
}
private void assertForwardedHeader() {
assertForwardedHeader("https://test:1234/admin");
// trigger a login error
assertForwardedHeader("http://mykeycloak.org:8080/realms/master/protocol/openid-connect/auth?client_id=security-admin-console", "https://test:1234/admin", ADDRESS);
assertForwardedHeader("http://mykeycloak.org:8080/realms/master/protocol/openid-connect/auth?client_id=security-admin-console", "https://test:1234/admin", NOT_ADDRESS);
}
private void assertForwardedHeader(String expectedUrl) {
private void assertForwardedHeader(String url, String expectedUrl, String forAddress) {
given()
.header("Forwarded", "for=12.34.56.78;host=test:1234;proto=https, for=23.45.67.89")
.when().get("http://mykeycloak.org:8080")
.header("Forwarded", "for="+forAddress+";host=test:1234;proto=https, for=23.45.67.89")
.when().get(url)
.then().header(HttpHeaders.LOCATION, containsString(expectedUrl));
}

31
services/src/main/java/org/keycloak/services/resources/WelcomeResource.java
View file

@ -51,13 +51,12 @@ import org.keycloak.theme.Theme;
import org.keycloak.theme.freemarker.FreeMarkerProvider;
import org.keycloak.urls.UrlType;
import org.keycloak.utils.MediaType;
import org.keycloak.utils.SecureContextResolver;
import java.io.IOException;
import java.io.InputStream;
import java.net.InetAddress;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
@ -271,25 +270,17 @@ public class WelcomeResource {
}
private boolean isLocal() {
try {
ClientConnection clientConnection = session.getContext().getConnection();
InetAddress remoteInetAddress = InetAddress.getByName(clientConnection.getRemoteAddr());
InetAddress localInetAddress = InetAddress.getByName(clientConnection.getLocalAddr());
HttpRequest request = session.getContext().getHttpRequest();
HttpHeaders headers = request.getHttpHeaders();
String xForwardedFor = headers.getHeaderString("X-Forwarded-For");
logger.debugf("Checking WelcomePage. Remote address: %s, Local address: %s, X-Forwarded-For header: %s", remoteInetAddress.toString(), localInetAddress.toString(), xForwardedFor);
ClientConnection clientConnection = session.getContext().getConnection();
String remoteAddress = clientConnection.getRemoteAddr();
String localAddress = clientConnection.getLocalAddr();
HttpRequest request = session.getContext().getHttpRequest();
HttpHeaders headers = request.getHttpHeaders();
String xForwardedFor = headers.getHeaderString("X-Forwarded-For");
String forwarded = headers.getHeaderString("Forwarded");
logger.debugf("Checking WelcomePage. Remote address: %s, Local address: %s, X-Forwarded-For header: %s, Forwarded header: %s", remoteAddress.toString(), localAddress.toString(), xForwardedFor, forwarded);
// Access through AJP protocol (loadbalancer) may cause that remoteAddress is "127.0.0.1".
// So consider that welcome page accessed locally just if it was accessed really through "localhost" URL and without loadbalancer (x-forwarded-for header is empty).
return isLocalAddress(remoteInetAddress) && isLocalAddress(localInetAddress) && xForwardedFor == null;
} catch (UnknownHostException e) {
throw new WebApplicationException(e, Response.Status.INTERNAL_SERVER_ERROR);
}
}
private boolean isLocalAddress(InetAddress inetAddress) {
return inetAddress.isAnyLocalAddress() || inetAddress.isLoopbackAddress();
// Consider that welcome page accessed locally just if it was accessed really through "localhost" URL and without loadbalancer (x-forwarded-for and forwarded header is empty).
return xForwardedFor == null && forwarded == null && SecureContextResolver.isLocalAddress(remoteAddress) && SecureContextResolver.isLocalAddress(localAddress);
}
private String setCsrfCookie() {

30
services/src/main/java/org/keycloak/utils/SecureContextResolver.java
View file

@ -57,13 +57,7 @@ public class SecureContextResolver {
return false;
}
// The host matches a CIDR notation of ::1/128
if (host.equals("[::1]") || host.equals("[0000:0000:0000:0000:0000:0000:0000:0001]")) {
return true;
}
// The host matches a CIDR notation of 127.0.0.0/8
if (LOCALHOST_IPV4.matcher(host).matches()) {
if (isLocalAddress(host)) {
return true;
}
@ -73,4 +67,26 @@ public class SecureContextResolver {
return host.endsWith(".localhost") || host.endsWith(".localhost.");
}
/**
* Test whether the given address is the localhost
* @param address
* @return false if the address is not localhost or not an address value
*/
public static boolean isLocalAddress(String address) {
if (address == null) {
return false;
}
// The host matches a CIDR notation of ::1/128
if (address.equals("[::1]") || address.equals("[0000:0000:0000:0000:0000:0000:0000:0001]")) {
return true;
}
// The host matches a CIDR notation of 127.0.0.0/8
if (LOCALHOST_IPV4.matcher(address).matches()) {
return true;
}
return false;
}
}

12
services/src/test/java/org/keycloak/utils/SecureContextResolverTest.java
View file

@ -4,6 +4,9 @@ import org.junit.Assert;
import org.junit.Test;
import org.keycloak.representations.account.DeviceRepresentation;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.function.Supplier;
@ -47,6 +50,7 @@ public class SecureContextResolverTest {
assertSecureContext("http://[::2]", false);
assertSecureContext("http://[2001:0000:130F:0000:0000:09C0:876A:130B]", false);
assertSecureContext("http://::1", false);
assertSecureContext("http://[FE80:0000:130F:0000:0000:09C0:876A:130B]", false);
}
@Test
@ -59,6 +63,14 @@ public class SecureContextResolverTest {
assertSecureContext("http://test.localhostn", false);
assertSecureContext("http://test.localhost.not", false);
}
@Test
public void testIsLocalhost() {
assertTrue(SecureContextResolver.isLocalAddress("127.0.0.1"));
assertFalse(SecureContextResolver.isLocalAddress("not.an.ip"));
assertFalse(SecureContextResolver.isLocalAddress(null));
assertFalse(SecureContextResolver.isLocalAddress(""));
}
@Test
public void testQuirksSafari() {