-
Notifications
You must be signed in to change notification settings - Fork 26
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
Unable to do strict issuer verification #44
Comments
Looked a bit further into it and found the one (and only) call site of As far as I understand the code, the |
it was not implemented yet, since the most common configs don't provide this value, but issuer validation when the verification was configured through metadata was just added; thanks for reporting and digging in! |
Awesome! I was sifting through the code to find a way to fix it, but you fixed it before I could even find a suitable starting point. Thanks! |
edit: it needed 44a3892 as well... |
Is it possible for you to create a new release with this patch? Or provide details on how to do it myself? I'm trying to create a deb myself, but its a bit finicky. Also, it would help us track the exact version used. |
This is great! Thanks again! 🙏 |
Not entirely sure how this works. I see the release jobs are finished, but there is no new item on the release page. Should I just wait, or is there a step missing? |
hold on, still building... |
Ah, thought the release job in github actions was enough. Thanks! |
Finally got a round testing this, but it still seems broken! I get the exact same message as before ( |
are you sure there is an "Issuer" value in the configured metadata document? if so, can you debug trace what happens? |
Verified it and keycloak does provide a
|
that really looks like you're still on 1.5.0, can you check the info log message at startup? |
Very strange. Seems like you're right, it even says |
Something is still off. Since there is no easy way to verify whether I have the expected version of the libraries, I did the following:
This all checks out. So, I believe I have installed the correct version, AFAIK. |
the log info output is leading, you still have another version on your system |
Thats strange, because I bootstrapped a clean server and got this problem! 🤔 P.S. |
|
Can reproduce my situation with docker: FROM httpd:2.4
ENV DEBIAN_FRONTEND=noninteractive
RUN apt update && apt install -y wget
WORKDIR /tmp
RUN wget https://github.com/OpenIDC/mod_oauth2/releases/download/v3.3.1/libapache2-mod-oauth2_3.3.1-1.bullseye_amd64.deb \
https://github.com/OpenIDC/liboauth2/releases/download/v1.5.1/liboauth2-apache_1.5.1-1.bullseye_amd64.deb \
https://github.com/OpenIDC/liboauth2/releases/download/v1.5.1/liboauth2_1.5.1-1.bullseye_amd64.deb \
&& apt-get install -y \
./liboauth2_1.5.1-1.bullseye_amd64.deb \
./liboauth2-apache_1.5.1-1.bullseye_amd64.deb \
./libapache2-mod-oauth2_3.3.1-1.bullseye_amd64.deb Build this with
Change the metadata url to something you can create tokens for (I used a cloud-iam account). Now run this setup with For this PoC, I used client credentials and got a token with this oneliner: Finally, I used Now change the P.S. I found that the parameter values ( |
ok, thanks for putting in the effort in reproducing this! I will try this and get back to you asap |
well, firstly it seems the compiler optimized away the runtime version reporting so it reports the version of liboauth2 compiled against rather than running against...; the liboauth2 package seems to be OK, but the issue seems to be with mod_oauth2 (3.3.1). it doesn't explain why the issuer verification doesn't work but recompiling against that version does change things... whilst I'm trying to wrap my head around the latter, you can use a version of mod_auth2 that was recompiled against 1.5.1 here: https://www.mod-auth-openidc.org/downloads/libapache2-mod-oauth2_3.3.1-1.bullseye_amd64.deb |
Cool, will try that as soon as I can and report back here (might take a while). |
Forgot that I can quickly verify with my PoC version. Did that and it works! Its really confusing though, as the file names suggest its the same file, but I can't reproduce the bug with this "other" version. :) |
thanks for helping and and sorry that it took this effort; I can't explain yet why "the other" version works since it is the same code compiled against a different version of the library and the whole idea of the library is that it can be developed and deployed independently of the module using it (the API did not change); I'll look into it in the coming days and let you know what I find |
it turns out to be a liboauth2 packaging error for the Debian based platforms: the liboauth2.so.0 files were packaged, but not the .so files so the compilation of dependent modules such as mod_oauth2 would fallback to static linking; I will have to issue new releases for all to make sure this gets out of the way once and for all; thank you for helping to sort this out |
see #46 as a final note |
depend on liboauth2 >= 1.5.1; see OpenIDC/liboauth2#44 Signed-off-by: Hans Zandbelt <[email protected]>
According to the example apache config, I can force strict issuer verification with the
verify.iss=required
option. However, when I set this option torequired
it fails, while it succeeds withoptional
.My config looks like this:
One interesting excerpt from the logs:
This suggests to me that the value of
iss
is not passed along.I'm using KeyCloak as my IdP and verified that the
iss
field in token corresponds with theissuer
field in the metadata.The text was updated successfully, but these errors were encountered: