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

mod_auth_openidc: newer versions cause lots of warnings about OIDCXForwardedHeaders #3880

Open
CSC-swesters opened this issue Oct 17, 2024 · 2 comments

Comments

@CSC-swesters
Copy link
Contributor

We upgraded mod_auth_openidc to a newer version (we have tested 2.4.15.7 and 2.4.16.4 specifically), in order to get certain site-specific functionality working. We noticed that there is a noticeable amount of warning logs from the module, which complain about mismatches in expectations of X-Forwarded-* headers. See the configuration documentation here for details.

The default value of OIDCXForwardedHeaders none causes warnings each time one of the headers appears, and if one were to set it to X-Forwarded-Proto for example, it would warn whenever the header is missing.

The X-Forwarded-Proto header is the only one we've observed in our OOD deployments, and the total amount of logging is roughly 35% of the OOD instance's log line count, when using the default OIDCXForwardedHeaders none configuration. So it's a significant issue.

The mod_auth_openidc project seems to not offer any way of avoiding this check, or warning log, which indicates to me that sites should have this working correctly.

My questions for OOD are:

  1. Why are we seeing X-Forwarded-Proto warnings from making a request to the dashboard?

I noticed that the header is set in the Lua proxy code, and this is what triggers the warnings from mod_auth_openidc. Commenting out this line in proxy.lua removes the warning logs altogether.

  1. What is this header used for downstream?
  2. Could it simply be removed?
  3. If not, could it be injected in some other way, which would not irritate mod_auth_openidc?

Some additional info:

  • While debugging httpd behavior, it seems like mod_auth_openidc checks the headers multiple times per end-user HTTP request (e.g. GET /pun/sys/dashboard). I saw three checks per request in our deployment. It seems that mod_auth_openidc is hooked into multiple stages of request processing.
  • We experimented with always removing the X-Forwarded-Proto header using RequestHeader unset X-Forwarded-Proto early. This didn't work completely, since the proxy.lua addition still happened by the end of request handling, and mod_auth_openidc ran its check once after that as well...

Thanks in advance!

@osc-bot osc-bot added this to the Backlog milestone Oct 17, 2024
@johrstrom
Copy link
Contributor

  1. Why are we seeing X-Forwarded-Proto warnings from making a request to the dashboard?
  1. What is this header used for downstream?

Unfortunately the commit that adds it doesn't give us much information. Ostensibly, it's so the origin server can make some sort of toggle. While the dashboard may not care, it's sort of a larger issue when we talk about what other Passenger apps could be out there in the wild, and how they may be reacting to this.

c7ab1c7

  1. Could it simply be removed?

Maybe. But again, it brings into question comparability with Passenger apps in the wild. Someone may be using it, though I can't say for sure.

Why not just configure OIDCXForwardedHeaders to allow the X-Forwarded-Proto header?

@CSC-swesters
Copy link
Contributor Author

While the dashboard may not care, it's sort of a larger issue when we talk about what other Passenger apps could be out there in the wild, and how they may be reacting to this.

Based on my understanding this is the risk with disabling it, yes. Downstream apps might behave differently if they do not know whether the connection is secure or not. I suppose we would need to test this and make a decision based on the outcomes. But good to know that OOD's default apps don't rely on it.

Why not just configure OIDCXForwardedHeaders to allow the X-Forwarded-Proto header?

Good question, sorry for not bringing that up earlier. I already tested this as well, and it also causes a non-zero amount of warnings due to the logic I described above:

if one were to set it to X-Forwarded-Proto for example, it would warn whenever the header is missing.

A quick 1-minute test with this option on yields 89 warning lines in a total of 256 httpd log lines. So this is not really a viable option either.

$ ts="2024-10-21T14:43:00" ; \
>    gawk "{if (\$1 > $(date +%s --date=${ts:?})) print }" < log | grep -cE ' ondemand_httpd ' ;  \
>    gawk "{if (\$1 > $(date +%s --date=${ts:?})) print }" < log | grep -cE 'oidc_check_x_forwarded_hdr' ;
256 # Total httpd log lines
89  # OIDC warning lines

I believe that the mod_auth_openidc that OOD bundles doesn't yet have these warnings, but if you plan on upgrading that to something newer, other sites will run into this as well. The parsing of these headers seems to have been introduced in version 2.4.11rc0 (commit).

It would be nice to understand what the correct way of handling this might be, and how to avoid the warning logs, so please let us know if you have ideas here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants