Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quarkus 1.13.0: 401 Unauthorized using different host names for Keycloak #16294

Closed
mrizzi opened this issue Apr 6, 2021 · 33 comments · Fixed by #16324
Closed

Quarkus 1.13.0: 401 Unauthorized using different host names for Keycloak #16294

mrizzi opened this issue Apr 6, 2021 · 33 comments · Fixed by #16324
Assignees
Labels
area/oidc kind/bug Something isn't working
Milestone

Comments

@mrizzi
Copy link
Contributor

mrizzi commented Apr 6, 2021

Describe the bug

The authorization configuration for endpoints based on quarkus.http.auth properties that was working on Quarkus 1.12.2 provides a 401 Unauthorized once migrated to Quarkus 1.13.0.
At my understanding, the issue is triggered by requesting the access token to Keycloak using a URL that is different from the one in the application's configurations for the property quarkus.oidc.auth-server-url.
But this seems to a fair common situation for a K8s/OCP deployment:

  1. a Web UI running in a client's browser requests the access token using the keycloak's external URL
  2. the backend endpoint receives the access token and validates it using the k8s' service name associated with Keycloak

Expected behavior

Having the requests to be authorized.

Actual behavior

Requests are unauthorized.

To Reproduce

Reproducer based on security-keycloak-authorization-quickstart available at https://github.com/mrizzi/quarkus-quickstarts/tree/401-unauthorized/security-keycloak-authorization-quickstart

Steps to reproduce the behavior (based on the commands from https://quarkus.io/guides/security-keycloak-authorization):

  1. start keycloak within a container in docker/podman
  2. ./mvnw clean compile quarkus:dev
  3. request the access token using the host's IP (e.g. 192.168.49.1):
    export access_token=$(\                                                                          
     curl --insecure -X POST https://192.168.49.1:8543/auth/realms/quarkus/protocol/openid-connect/token \
     --user backend-service:secret \
     -H 'content-type: application/x-www-form-urlencoded' \
     -d 'username=alice&password=alice&grant_type=password' | jq --raw-output '.access_token' \
    )
  4. curl -v -X GET http://localhost:8080/api/users/me -H "Authorization: Bearer "$access_token gives 401 Unauthorized
  5. restart the application with ./mvnw clean compile quarkus:dev -Pquarkus-12 (a profile I've added to use Quarkus 1.12.2)
  6. request the access token using the host's IP as above
  7. curl -v -X GET http://localhost:8080/api/users/me -H "Authorization: Bearer "$access_token works (i.e. provided {"userName":"alice"})

I've also added the quarkus-999 profile to check if the issue was already fixed upstream but it's not solved.

Configuration

I've disabled the policy enforcer to use the Quarkus integrated web security layer because it's the one we're using in our application. The OIDC part is unchanged.

# Configuration file
quarkus.oidc.auth-server-url=https://localhost:8543/auth/realms/quarkus
quarkus.oidc.client-id=backend-service
quarkus.oidc.credentials.secret=secret
quarkus.oidc.tls.verification=none
quarkus.http.cors=true

# Enable Policy Enforcement
#quarkus.keycloak.policy-enforcer.enable=true

# Disables policy enforcement for a path
#quarkus.keycloak.policy-enforcer.paths.1.path=/api/public
#quarkus.keycloak.policy-enforcer.paths.1.enforcement-mode=DISABLED

quarkus.http.auth.permission.authenticated.paths=/api/users/*
quarkus.http.auth.permission.authenticated.policy=authenticated
quarkus.http.auth.policy.role-policy.roles-allowed=admin
quarkus.http.auth.permission.role.paths=/api/admin/*
quarkus.http.auth.permission.role.policy=role-policy
quarkus.http.auth.permission.permit.paths=/api/public/*
quarkus.http.auth.permission.permit.policy=permit

Environment (please complete the following information):

Output of uname -a or ver

Linux fedora-p1 5.11.10-200.fc33.x86_64 #1 SMP Thu Mar 25 16:51:31 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment 18.9 (build 11.0.10+9)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.10+9, mixed mode, sharing)

GraalVM version (if different from Java)

Quarkus version or git rev

1.13.0

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)

@mrizzi mrizzi added the kind/bug Something isn't working label Apr 6, 2021
@gsmet
Copy link
Member

gsmet commented Apr 6, 2021

/cc @sberyozkin @pedroigor

@sberyozkin sberyozkin self-assigned this Apr 6, 2021
@sberyozkin
Copy link
Member

@mrizzi Can you please add

quarkus.log.category."io.quarkus.oidc.runtime.OidcProvider".min-level=TRACE
quarkus.log.category."io.quarkus.oidc.runtime.OidcProvider".level=TRACE

and check what is being logged ?

thanks

@sberyozkin
Copy link
Member

@mrizzi By the way, once this issue is fixed - please note you don't need to configure keycloak-authorization to access the public resources in 1.13.x - as it is already done in the http authentication configuration

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@mrizzi Can you please add

quarkus.log.category."io.quarkus.oidc.runtime.OidcProvider".min-level=TRACE
quarkus.log.category."io.quarkus.oidc.runtime.OidcProvider".level=TRACE

and check what is being logged ?

thanks

Hi @sberyozkin,
this is the line of log I got:

DEBUG [io.qua.oid.run.OidcProvider] (vert.x-eventloop-thread-21) Token verification has failed: Issuer (iss) claim value (https://192.168.49.1:8543/auth/realms/quarkus) doesn't match expected value of https://localhost:8543/auth/realms/quarkus

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@sberyozkin maybe I'm wrong but was this a bug of previous Quarkus versions?
I mean, is Quarkus 1.13.0 working as expected about issuer validation?
It seems like previously the issuer validation was done only if quarkus.oidc.token.issuer property was provided (ref. OidcUtils#validateClaims in Quarkus 1.12.2)
If the failure comes from the Iss validator, then if I add the quarkus.oidc.token.issuer=https://192.168.49.1:8543/auth/realms/quarkus property it works.
If all confirmed, the only improvement I can see would be to have quarkus.oidc.token.issuer to become a list of values rather than only one to be able to have both internal and external URLs as valid issuers without having to choose only one.
Does it make sense?

@sberyozkin
Copy link
Member

Hi @mrizzi so, interesting, the issuer is set dynamically using the current request's host, but not to the one advertised in the discovery doc. CC @pedroigor, FYI.
So, I think the reason it worked in 1.12.x was that the issuer, by default, was not checked for the bearer tokens - I recall it is checked as part of the code flow token acquisition only where it is matched against auth-server-url site value (sorry if I'm missing something - I've just looked at Vert.x code and do not the see the issuer verification when the token is just decoded)
Yes, if the issuer was configured it was always checked.

Hmm... I'm not sure what to do at the moment. Keycloak returns a localhost: in the discovery doc but sets a diff issuer value when generating the token. @mrizzi, what Keycloak version is it ?

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@sberyozkin locally I'm using Keycloak 12.0.4

@sberyozkin
Copy link
Member

Well the good news one can indeed set quarkus.oidc.token.issuer=https://192.168.49.1:8543/auth/realms/quarkus and that just works - as it overrides the discovered value.

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

Well the good news one can indeed set quarkus.oidc.token.issuer=https://192.168.49.1:8543/auth/realms/quarkus and that just works - as it overrides the discovered value.

It's good because it works but it's not so good when talking about K8s/OCP because it means that when doing a basic deployment using templates, that value can not be provided because it will be generated dynamically at deployment time requiring manual changes to the user.
The previous behavior was better because the configuration required no manual intervention so if there's another option to avoid the quarkus.oidc.token.issue configuration, it would be great.

@sberyozkin
Copy link
Member

sberyozkin commented Apr 7, 2021

@mrizzi

The previous behavior was better because the configuration required no manual intervention

It was not better as it was less safe - the issuer was not verified at all by default for the bearer tokens.
Perhaps I can update the code to check if quarkus.oidc.token.issuer is empty or equal to some value like any - in which case the issuer verification will be skipped - I don't like this idea but for the moment see no other viable options.

@pedroigor is it possible to configure Keycloak to use the fixed issuer value ? I'm not sure about the issuer not matching the discovered doc property.

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@sberyozkin absolutely right: security always wins 👍
No need to add the any option for me: let's keep it as it is from the Quarkus side.

@sberyozkin
Copy link
Member

@mrizzi Hi, it is an interesting issue, sorry it is happening for your deployments with 1.13.x - but indeed the fact it worked because the issuer was not verified was certainly easier to deal with :-) but the key property check was skipped was also problematic.
That said, you are likely not the last user who will report this issue :-). So if we have a case where the issuer is dynamically generated or for example, we have a multi-IP or dynamic host system, we can't really capture statically the list of issuers in the configuration property - so any would be the only option - this just says skip the issuer verification only when it is needed.
I wonder though if Keycloak can be configured to issue the same issuer value

@sberyozkin
Copy link
Member

@mrizzi Have a look at https://stackoverflow.com/questions/61966384/keycloak-invalid-token-issuer, can any of the recommendations there work for you ?

@sberyozkin
Copy link
Member

@mrizzi @pedroigor FYI, #16324

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@mrizzi Have a look at https://stackoverflow.com/questions/61966384/keycloak-invalid-token-issuer, can any of the recommendations there work for you ?

So, interesting tests starting keycloak's container adding -e KEYCLOAK_FRONTEND_URL=https://192.168.49.1:8543/auth:

  • Quarkus 1.13.0 works without the quarkus.oidc.token.issuer property (even if, from my perspective, it's the same solution since it's always a matter of adding a property to the K8s deployment with the Keycloak's external URL)
  • Quarkus 1.12.2 fails to start the application with the message:
    ERROR [io.qua.run.Application] (Quarkus Main Thread) Failed to start application (with profile dev): io.vertx.core.impl.NoStackTraceThrowable: issuer validation failed: received [https://192.168.49.1:8543/auth/realms/quarkus]

Furthemore, with Quarkus 1.13.0, it works when the token is requested using the URL
https://192.168.49.1:8543/auth/realms/quarkus/protocol/openid-connect/token
but also when using
https://fedora-p1:8543/auth/realms/quarkus/protocol/openid-connect/token
so with the host name fedora-p1 instead of the IP which is not something that worked with the solution of adding the quarkus.oidc.token.issuer property so the behavior is different.

The other solution proposed in SO about using the external URL every time might work but it seems to me a useless trip for requests between pods in the same k8s namespace to talk to each other using external URLs.

@sberyozkin
Copy link
Member

@mrizzi

Ok, thanks for the tests - so it looks not bad in 1.13.x with KEYCLOAK_FRONTEND_URL - looks like it affects how Keycloak sets the issuer in the discovery doc and in the iss claim - same value - so no need to customize with the property ; combined with the possibility of skipping the authentication completely, it should cover this issuer space qutie well

@sberyozkin
Copy link
Member

sberyozkin commented Apr 7, 2021

fedora-ip vs the ipaddress is the same problem as with localhost vs ip, when using the issuer property - so indeed, setting the frontend url property at the startup seems the best option - or skipping it if really needed

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@sberyozkin my opinion right now is that we will wait for quarkus.oidc.token.issuer=any to be available from #16324 so that we can keep our k8s/OCP templates basic as they're without any need for manual intervention only for development purposes

Then when my team will work on creating an operator, we will enforce there all the "production level" security requirements

@pedroigor
Copy link
Contributor

@mrizzi @sberyozkin I was about to suggest a similar setting in Keycloak. As a quick background, that flag forces Keycloak to publish all the frontend URLS (issuer, jwks, authorization endpoint, etc) with a specific URL. There is also a setting for forcing backchannel URLs to use the URL set to the frontend URLs. This configuration is usually done when deploying to production. See https://www.keycloak.org/docs/latest/server_installation/#_hostname.

However, it is not yet clear to me why that is now impacting Quarkus apps?

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 7, 2021

@pedroigor my understanding/guessing is because the validation implementation changed with Quarkus 1.13.0 from the previous OidcUtils#validateClaims in Quarkus 1.12.2.

What surprised me is that our tests were not able to catch this but using keycloak with testcontainer let us use localhost everywhere so the issue was not caught by any test.

@sberyozkin
Copy link
Member

Hi @pedroigor @mrizzi Not quite - what has changed is now, by default, the issuer is verified - while up to 1.13.x - it was not verified at all for the bearer tokens unless the user manually set quarkus.oidc.token.issuer

@pedroigor
Copy link
Contributor

I see. The default behavior looks more correct to me.

But I think this is not an issue in Quarkus. But missing config in Keycloak. As previously mentioned.

Suppose your app is set to quarkus.oidc.auth-server-url=http://localhost:8180/auth/realms/quarkus.

But you set the frontendUrl in Keycloak to http://mykeycloak/auth.

In this case, the issuer will be http://mykeycloak/auth/realms/quarkus. From the discovery document.

Even though you have set localhost to the quarkus.oidc.auth-server-url property.

@sberyozkin
Copy link
Member

@pedroigor thanks for the explanation

@mrizzi can you clarify please why setting KEYCLOAK_FRONTEND_URL can be problematic in your case ?

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 8, 2021

I think the solutions proposed, from my perspective, are the same: in any case the user has to set a property at runtime (i.e. the keycloak's URL) or in the quarkus.oidc.token.issuer in the backend application or as KEYCLOAK_FRONTEND_URL in the keycloak instance.
Both give me no way to provide a k8s/OCP template that can work for setting up a development application instance without requiring the user manual intervention.

Considering the proposed solutions from a general perspective, I think each developers for sure owns the application s/he's developing so adding quarkus.oidc.token.issuer property will be easier to achieve considering keycloak might be provided to developers to be integrated without any chance for them to ask for changes in keycloak's configuration.
From this perspective, I feel like having the chance to define quarkus.oidc.token.issuer=any will "help" the integration of applications with Keycloak's instances no matter all the configurations of URLs we can have in real world instances.

I hope it makes sense and helps.

@pedroigor
Copy link
Contributor

pedroigor commented Apr 8, 2021

@mrizzi So your proposal is more a dev option than actually something for production?

Ignoring the issuer is really a problem for production-grade applications.

Perhaps you can use some placeholder to resolve the issuer based on an environment variable that can be set in your template.

However, even though devs don't have access to Keycloak configuration, I would expect this server properly configured in k8s/OCP with a frontend URL. In this case, even though you obtain tokens using the IP address, you will get the same issuer as your application.

@sberyozkin
Copy link
Member

Hi @pedroigor - we have another issue, #16384

@sberyozkin
Copy link
Member

sberyozkin commented Apr 9, 2021

Hi @mrizzi and @pedroigor - I've been thinking about it as well, so there is one option you can use right now, even without this PR to disable the issuer verification - disable the auto-discovery - set auth-server-url, discovery-enabled=false, and jwks-path=/certs (point to the correct KC path) - and no issuer will be discovered - and the check will be skipped.

Pedro, given the above - where one can indirectly disable it - giving a configuration option to do the same does not change the picture but simply makes it consistent. We can certainly make the message stronger about recommending KEYCLOAK_FRONTEND_URL ?

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 9, 2021

@pedroigor yep, my proposal is to let devs have full control without having to request any change to the authentication application (keycloak in my case) and the change for allowing the usage of any seems reasonable to me.

@sberyozkin sorry, I know it's not my business, but for #16384, I think that my previous suggestiong about quarkus.oidc.token.issuer property to become a list of values rather than only one still can be considered a valuable improvement to let add all the tenants values as a list to quarkus.oidc.token.issuer

@sberyozkin
Copy link
Member

sberyozkin commented Apr 9, 2021

@mrizzi You are welcome to comment on other issues for sure :-). It won't work unfortunately - it is a dynamic case so they can't be pre-configured statically. In general I don't like the idea of overloading the properties - multiple issuers, then what happens if some other property like the path to the roles depends on the issuer, etc...

@sberyozkin
Copy link
Member

sberyozkin commented Apr 9, 2021

@mrizzi

my proposal is to let devs have full control without having to request any change to the authentication application

By the way, I had another question - who is controlling the code (SPA script) which uses IP host address ? Are these the same developers who may have to set quarkus.oidc.token.issuer=my-ip-address/issuer ?
If it is the case then it is not a restriction IMHO - if the devs have to write their script with the specific IP address in mind - then nothing really changes for them when it comes to having to set quarkus.oidc.token.issuer=my-ip-address/issuer.
And if the script is controlled by the same devs who deploy Keycloak - then again they can just set KEYCLOAK_FRONTEND_URL.

Please note - I'm just trying to understand the whole picture - I feel the PR may be viable for the consistency's purposes - if anything else
thanks

@sberyozkin
Copy link
Member

So, #16384 is related after all - but in a different way - both issues are to do with the fact the issuer is verified by default if it has been discovered - at least here we can control Keycloak....

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 9, 2021

Hi @mrizzi and @pedroigor - I've been thinking about it as well, so there is one option you can use right now, even without this PR to disable the issuer verification - disable the auto-discovery - set auth-server-url, discovery-enabled=false, and jwks-path=/certs (point to the correct KC path) - and no issuer will be discovered - and the check will be skipped.

@sberyozkin Thanks for the further solution: I've just found the time to test it.
So I can confirm that adding the properties:

quarkus.oidc.discovery-enabled=false
quarkus.oidc.jwks-path=protocol/openid-connect/certs

works without requiring further application or keycloak configuration.
I'm fully aware that disabling checks decrease the security and I want to emphasize that is not meant for production for whoever will read this issue in the future.

@mrizzi
Copy link
Contributor Author

mrizzi commented Apr 9, 2021

@sberyozkin during the test I had to debug the Quarkus code to understand that in the hurry of testing I put the absolute URL for quarkus.oidc.jwks-path property instead of the relative one, because the OIDCException exception was "mute".

So, if it's fine for you, I would open a PR to instantiate the OIDCException with a message like
String.format("Received %s %s due to %s", resp.statusCode(), resp.statusMessage(), resp.bodyAsString()));
Worth doing it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oidc kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants