Require SSL mode "External requests" does not work with IPv6 local addresses

[tsaarni]

Before reporting an issue

• [X] I have read and understood the above terms for submitting issues, and I understand that my issue may be closed without action if I do not follow them.

Area

core

Describe the bug

When using IPv6 and client is sending request from link-local or unique local address (ULA), client is blocked from sending clear-text HTTP requests when realm configuration is set to require SSL for "External request" (link).

Version

main

Regression

• [ ] The issue is a regression

Expected behavior

Requests sent from IPv6 link-local or unique local address should succeed.

Actual behavior

Requests are rejected with response 403 Forbidden with body

{
    "error": "invalid_request",
    "error_description": "HTTPS required"
}

How to Reproduce?

One way to reproduce is to use Kubernetes cluster with IPv6 enabled

1. Run Keycloak in a Kubernetes cluster with IPv6 enabled.
2. Run a client in another pod and check that the pod's network interface has IPv6 address either from link-local address range or unique local address range. Send request using IPv6.
3. Observe response 403 Forbidden

For example for local development, Kind would use link-local addresses for pods and services within the cluster and a "real" CNI like Calico would typically use unique local addresses (ULA).

Anything else?

The code that checks for the request is here https://github.com/keycloak/keycloak/blob/ece72cd491b68422940136bc6171392e07db6679/common/src/main/java/org/keycloak/common/enums/SslRequired.java#L53-L54

Looking at the git history, it seems that initially the code aimed to cover only clients within localhost, and later it took the current form where it was extended to allow IPv4 private addresses (link).

In case of IPv6 the private address ranges are following:

address type	binary prefix	IPv6 notation
Link-local unicast	1111 1110 10	FE80::/10
Unique local (ULA)	1111 110	FC00::/7
Site-local unicast (deprecated)	1111 1110 11	FEF0::/10

While the JDK method Inet4Address.isSiteLocalAddress() covers private address ranges for IPv4 (link), the IPv6 version Inet6Address.isSiteLocalAddress() only covers site-local unicast address range (link). According to RFC4291 section2.5.7 this address range is now deprecated and should be considered as global address by new implementation

The special behavior of this prefix defined in [RFC3513] must no longer be supported in new implementations (i.e., new implementations must treat this prefix as Global Unicast).

Existing implementations and deployments may continue to use this prefix.

While the documentation for interface InetAddress.isSiteLocalAddress() says (link)

Utility routine to check if the InetAddress is a site local address.

it seems likely to me that applications may rather use it for the effect of "all private address ranges", than for the specific (now deprecated) IPv6 site-local definition, especially considering the method is used through the upper class that covers both IPv4 and IPv6. Still, the JDK IPv6 implementation seems to take strict interpretation of covering site-local only and does not offer any method for checking for IPv6 private address ranges.

[ahus1]

Hi @tsaarni - thank you for raising this issue.

IMHO the concept of some request requiring TLS, while others don't, might something that worked in the past where we divided the world into external, untrusted networks and internal, trusted networks. This is IMHO no longer the case in a world where zero trust is promoted.

So I'd say this option didn't age well, and it should rather be removed from Keycloak, than being extended. Using plain HTTP in a development environment might work, still in a production environment all requests should use HTTPS.

I'd be happy to hear your thoughts on this, especially when this concept should still be used.

cc: @stianst

[tsaarni]

We sometimes use it inside Kubernetes cluster. TLS is terminated at the ingress controller, at the edge of the cluster internal network. The internal "connection leg" can be optionally configured for clear-text HTTP like this:

[ahus1]

Thank you for providing the diagram. I assume the ingress controller adds HTTP headers which Keycloak then uses to identify the original IP address used by the external client to connect to the ingress controller, and it will also use those extra HTTP headers to identify that the request originated via HTTPS?

If we would drop the check altogether, is there any use case that would not work any more for you?

[shawkins]

Adding back the triage label and a comment to the PR.

[shawkins]

Let's see if we can get this out of triage. To elaborate the options seem to be:

1. Proceed with #30751 and log a separate issue about the deprecation of EXTERNAL / SslRequired.
2. Proceed directly to deprecating EXTERNAL and document instead that we are defaulting to NONE instead. Existing realms will eventually need to be updated to remove the usage of EXTERNAL. Users who want to enforce SSL at the Keycloak level will need explicitly set realms they create to ALL.
3. Proceed directly to deprecating SslRequired (effectively deprecating EXTERNAL and ALL) - this either means enforcing SSL at the Keycloak level is not needed or the security profiles feature needs to be implemented. Since security profiles haven't been implemented yet, it's not clear to me if we're ready for this. Other servers, like postgresql, do have server options to declare when SSL is required / optional.

[tsaarni]

@ahus1 wrote:

I assume the ingress controller adds HTTP headers which Keycloak then uses to identify the original IP address used by the external client to connect to the ingress controller, and it will also use those extra HTTP headers to identify that the request originated via HTTPS?

If we would drop the check altogether, is there any use case that would not work any more for you?

I'm not aware of any existing checks for the HTTPS scheme in the X-Forwarded-Proto header from the proxy. As @shawkins mentioned, if we decide to remove the check entirely (option 3) there might not be any check for TLS unless a new one is implemented.

Defaulting to NONE (option 2) seems bit counter-intuitive, especially if we are deprecating EXTERNAL to promote a zero trust architecture. An alternative option could be to deprecate EXTERNAL and set the new default to ALL.

Options other than (1) could potentially break backwards compatibility and might require extra work for users.

[ahus1]

To explain more about my previous comment about the deprecation:

• If someone has the requirement that all external requests should use TLS, the way to configure this is to allow only TLS connections via the Ingress. There is nothing else that Keycloak should need to do. This is my primary reason why we should deprecate this check in Keycloak altogether.

To explain more about the current logic in Keycloak how the TLS detection works when Keycloak is behind a proxy that terminates (or re-encrypts) the TLS connection:

• For Keycloak to know that the connection coming from the Ingress was a HTTPS connection on the outside, the Ingress adds a forwarding HTTP header indicating the original protocol. Keycloak then trusts this information and even though the incoming connection is cleartext, it assumes that the connection is secured. While this works, there is no need to check this anyway if the Ingress is configured to accept only TLS connections. People usually configure an Ingress with a HTTP to HTTPS redirection, plus HSTS, again all items that work very well independent of Keycloak.

While I'm ok to proceed with this PR, we should have a separate issue to deprecate this feature including a discussion around this. Once it will then be removed, it would default to a no-op, which would be "NONE".

[keycloak-github-bot[bot]]

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a :thumbsup: to the description. We would also welcome a contribution to fix the issue.

[shawkins]

@ahus1 converted your last comment to a new issue https://github.com/keycloak/keycloak/issues/31914