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

Reverse Proxy authentication trust doesn't work as expected #344

Open
cfstras opened this issue Mar 20, 2023 · 8 comments
Open

Reverse Proxy authentication trust doesn't work as expected #344

cfstras opened this issue Mar 20, 2023 · 8 comments
Labels

Comments

@cfstras
Copy link

cfstras commented Mar 20, 2023

I've come across two issues when looking at reverse proxy authentication:

  1. Using ProxyHeadersMiddleware with client on trusted proxy results in invalid client information encode/uvicorn#1068
    This is mostly only an issue when f.ex. testing with localhost. As a workaround, one can use the LAN IP to access the proxy instead.

More importantly:

  1. The application server used, uvicorn, has its own logic for parsing the X-Forwarded-For header. Combined with the functionality in timetagger, this will mangle/break the list of forwarded IPs for incoming requests, potentially even trusting fake headers sent by a client!
    To fix this, one has to export FORWARDED_ALLOW_IPS="" to disable the uvicorn parsing.
    See uvicorn docs.

For reference, I'm passing

--proxy_auth_header="X-Forwarded-User"
--proxy_auth_trusted="127.0.0.1"
--proxy_auth_enabled=True
@cfstras cfstras changed the title Reverse Proxy authentication trust doesn't work Reverse Proxy authentication trust doesn't work as expected Mar 20, 2023
@almarklein
Copy link
Owner

Sorry for responding so late, this slipped my attention. I haven't looked looked into proxy authentication that much, to be honest. Are there things in TimeTagger that we can do to resolve these issues? PR's welcome :)

Also cc @Rynoxx @mtn-mathi @foorschtbar

@cfstras
Copy link
Author

cfstras commented Apr 22, 2023

My first instinct would be to remove the X-Forwarded-For code from timetagger, trust the functionality in uvicorn and configure it automatically and/or document how to. Though I‘m not sure why it was added in the first place if uvicorn was always used, or, in which usecases it was tested, so we can make sure not to break these.

@foorschtbar
Copy link

Sorry for responding so late, this slipped my attention. I haven't looked looked into proxy authentication that much, to be honest. Are there things in TimeTagger that we can do to resolve these issues? PR's welcome :)

Also cc @Rynoxx @mtn-mathi @foorschtbar

For me, the current solution works at the moment.

@cfstras
Copy link
Author

cfstras commented Apr 23, 2023

@foorschtbar Could you please describe the setup you’re using, the flags and env vars, and which headers are passed by your reverse proxy?
In my tests, using oauth2-proxy, timetagger (without the described workaround) would always ignore the X-Forwarded-User header.
Upon debugging, I found that uvicorn already parses the X-Forwarded-For, and clears it, and then timetagger tries to check it again and decides to skip proxy auth handling because it‘s missing.

@Rynoxx
Copy link
Contributor

Rynoxx commented Apr 23, 2023

@cfstras This config works for me in kuberenetes behind Authentik as the proxy-auth provider.
I'm not seeing any differences in behavior between having FORWARDED_ALLOW_IPS="" or not, also tried *.

But this might also be because authentik is using a custom header (X-authentik-username) for it and not a X-Forwarded-* header?

          env:
            - name: TIMETAGGER_BIND
              value: "0.0.0.0:80"
            - name: TIMETAGGER_CREDENTIALS
              value: "<redacted>"
            - name: TIMETAGGER_LOG_LEVEL
              value: "info"
            - name: TIMETAGGER_DATADIR
              value: "/root/_timetagger"
            - name: TIMETAGGER_PROXY_AUTH_ENABLED
              value: "True"
            - name: TIMETAGGER_PROXY_AUTH_HEADER
              value: "X-authentik-username"
            - name: TIMETAGGER_PROXY_AUTH_TRUSTED
              value: "10.0.0.1/8"
            - name: TZ
              value: "Europe/Stockholm"

@cfstras
Copy link
Author

cfstras commented Apr 23, 2023

Ah, that explains it. Just checked the uvicorn code: https://github.com/encode/uvicorn/blob/87387273945624452c1f0e797bcf2a539b0c9211/uvicorn/middleware/proxy_headers.py#L28

The behavior there seems to only happen when the reverse proxy uses 127.0.0.1.

It‘s not clear to me why setting it to * doesn’t trigger the bug for you though… Maybe the config is not getting through for some reason?

IMO we either should disable that default in uvicorn, or just pass the TIMETAGGER_PROXY_AUTH_TRUSTED to the uvicorn middleware config instead of manual handling?
The latter could make the code a lot simpler.

@foorschtbar
Copy link

@foorschtbar Could you please describe the setup you’re using, the flags and env vars, and which headers are passed by your reverse proxy? In my tests, using oauth2-proxy, timetagger (without the described workaround) would always ignore the X-Forwarded-User header. Upon debugging, I found that uvicorn already parses the X-Forwarded-For, and clears it, and then timetagger tries to check it again and decides to skip proxy auth handling because it‘s missing.

I use Authelia (no setup needed for Timetagger) and Traefik:

Treafik:

- "traefik.http.middlewares.timetagger-auth.forwardauth.address=http://authelia:9091/api/verify?rd=https%3A%2F%2Fauth.xxx.tld"
- "traefik.http.middlewares.timetagger-auth.forwardAuth.trustForwardHeader=true"
- "traefik.http.middlewares.timetagger-auth.forwardAuth.authResponseHeaders=Remote-Email"

Timetagger:

- TIMETAGGER_PROXY_AUTH_ENABLED=True
- TIMETAGGER_PROXY_AUTH_TRUSTED=127.0.0.1,172.xxx.xxx.xxx/24
- TIMETAGGER_PROXY_AUTH_HEADER=Remote-Email

@cfstras
Copy link
Author

cfstras commented Apr 24, 2023

Hm, @foorschtbar I‘d guess that in your case the traefik requests are coming to timetagger from 172.x.x.x, so you‘re not triggering this bug.

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

No branches or pull requests

4 participants