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

[Bug]: ACTUAL_TRUSTED_PROXIES has not desired effect #371

Open
1 task done
tuetenk0pp opened this issue Jun 9, 2024 · 14 comments · May be fixed by #499
Open
1 task done

[Bug]: ACTUAL_TRUSTED_PROXIES has not desired effect #371

tuetenk0pp opened this issue Jun 9, 2024 · 14 comments · May be fixed by #499
Labels
bug Something isn't working

Comments

@tuetenk0pp
Copy link

tuetenk0pp commented Jun 9, 2024

Verified issue does not already exist?

  • I have searched and found no existing issue

What happened?

I set up actual budget to allow for header authentication via authentik. So I added the group attributes in authentik:

additionalHeaders:
  X-Actual-Password: ***

I also added the following environment variables to my docker setup:

ACTUAL_LOGIN_METHOD=header
ACTUAL_TRUSTED_PROXIES=<proxy_ip>

This is how I configured Caddy:

(header) {
        header {
                Strict-Transport-Security "max-age=31536000; includeSubdomains"
                X-XSS-Protection "1; mode=block"
                X-Content-Type-Options "nosniff"
                Referrer-Policy "same-origin"
                X-Frame-Options "ALLOW-FROM *.<domain.tld>"
                -Server
                Content-Security-Policy "frame-ancestors <domain.tld> *.<domain.tld>"
                Permissions-Policy "geolocation=(self <domain.tld> *.<domain.tld>), microphone=()"
        }
}

(errors) {
        handle_errors {
                rewrite * /500.html
                file_server
        }
}
https://budget.domain.tld {
        # use header snippet
        import header

        # always forward outpost path to actual outpost
        reverse_proxy /outpost.goauthentik.io/* http://<authentik_ip>:9000

        # forward authentication to outpost
        forward_auth http://<authentik_ip>:9000 {
                uri /outpost.goauthentik.io/auth/caddy

                # capitalization of the headers is important, otherwise they will be empty
                copy_headers X-Authentik-Username X-Authentik-Groups X-Authentik-Email X-Authentik-Name X-Authentik-Uid X-Authentik-Jwt X-Authentik-Meta-Jwks X-Authentik-Meta-Outpost X-Authentik-Meta-Provider X-Authentik-Meta-App X-Authentik-Meta-Version X-Actual-Password

                # optional, in this config trust all private ranges, should probably be set to the outposts IP
                trusted_proxies private_ranges
        }

        
        encode gzip zstd
        reverse_proxy http://<actual_budget_ip>:5006

        # use errors snippet
        import errors
}

What error did you receive?

When I try to login into my actual budget instance, I get this error:

grafik

The server log prints:

Checking if there are any migrations to run for direction "up"...
Migrations: DONE
Listening on :::5006...
Logging in via header
HEADER VALUE: ***
Header Auth Login attempted from <my_external_ip>

So the header authentication works in principle, but appearently, actual budget confuses my external IP and the reverse proxy IP. I suspect the issue might be related to the validateAuthHeader function.

export function validateAuthHeader(req) {

I also checked my caddy logs, so here's an example:

{
  "level": "debug",
  "ts": 1717958752.5008152,
  "logger": "http.handlers.reverse_proxy",
  "msg": "upstream roundtrip",
  "upstream": "<actual_budget_ip>:5006",
  "duration": 0.023609725,
  "request": {
    "remote_ip": "<my_external_ip>",
    "remote_port": "50292",
    "client_ip": "<my_external_ip>",
    "proto": "HTTP/2.0",
    "method": "GET",
    "host": "budget.<domain.tld>",
    "uri": "/sw.js",
    "headers": {
      "Cache-Control": [
        "max-age=0"
      ],
      "X-Authentik-Uid": [
        "***"
      ],
      "X-Actual-Password": [
        "***"
      ],
      "X-Forwarded-Host": [
        "budget.<domain.tld>"
      ],
      "X-Authentik-Meta-Provider": [
        "Actual Budget"
      ],
      "Service-Worker": [
        "script"
      ],
      "Sec-Fetch-Dest": [
        "serviceworker"
      ],
      "Sec-Fetch-Site": [
        "same-origin"
      ],
      "X-Forwarded-For": [
        "<my_external_ip>"
      ],
      "X-Authentik-Name": [
        "***"
      ],
      "Dnt": [
        "1"
      ],
      "Te": [
        "trailers"
      ],
      "User-Agent": [
        "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:126.0) Gecko/20100101 Firefox/126.0"
      ],
      "X-Authentik-Jwt": [
        "***"
      ],
      "Accept": [
        "*/*"
      ],
      "Accept-Language": [
        "de,en-US;q=0.7,en;q=0.3"
      ],
      "Sec-Fetch-Mode": [
        "same-origin"
      ],
      "Accept-Encoding": [
        "gzip, deflate, br, zstd"
      ],
      "X-Authentik-Meta-App": [
        "actual-budget"
      ],
      "X-Authentik-Meta-Jwks": [
        "https://auth.<domain.tld>/application/o/actual-budget/jwks/"
      ],
      "X-Authentik-Meta-Version": [
        "goauthentik.io/outpost/2024.4.2"
      ],
      "X-Forwarded-Proto": [
        "https"
      ],
      "Priority": [
        "u=4"
      ],
      "Cookie": [
        "REDACTED"
      ],
      "X-Authentik-Meta-Outpost": [
        "authentik Embedded Outpost"
      ],
      "X-Authentik-Email": [
        "***"
      ],
      "X-Authentik-Username": [
        "***"
      ],
      "X-Authentik-Groups": [
        "Actual Budget"
      ]
    },
    "tls": {
      "resumed": false,
      "version": 772,
      "cipher_suite": 4865,
      "proto": "h2",
      "server_name": "budget.<domain.tld>"
    }
  },
  "headers": {
    "Date": [
      "Sun, 09 Jun 2024 18:45:52 GMT"
    ],
    "Connection": [
      "keep-alive"
    ],
    "Ratelimit-Remaining": [
      "496"
    ],
    "Ratelimit-Limit": [
      "500"
    ],
    "Ratelimit-Reset": [
      "44"
    ],
    "Cross-Origin-Opener-Policy": [
      "same-origin"
    ],
    "Cross-Origin-Embedder-Policy": [
      "require-corp"
    ],
    "Access-Control-Allow-Origin": [
      "*"
    ],
    "Content-Type": [
      "application/javascript; charset=UTF-8"
    ],
    "Keep-Alive": [
      "timeout=5"
    ],
    "Last-Modified": [
      "Mon, 03 Jun 2024 10:11:38 GMT"
    ],
    "Cache-Control": [
      "public, max-age=0"
    ],
    "Etag": [
      "W/\"522a-18fdd954390\""
    ],
    "Content-Length": [
      "21034"
    ],
    "Accept-Ranges": [
      "bytes"
    ]
  },
  "status": 200
}

As you can see, it correctly passes the X-Forwarded-For and X-Forwarded-Host headers to actual budget.

I was able to login after I added <my_external_ip> to the ACTUAL_TRUSTED_PROXIES. Obviously, this is not how it should work.

Let me know if I can assist in any way.

Where are you hosting Actual?

Docker

What browsers are you seeing the problem on?

No response

Operating System

None

@tuetenk0pp tuetenk0pp added the bug Something isn't working label Jun 9, 2024
@tuetenk0pp
Copy link
Author

I just had a quick look at the description of proxyaddr:

The closest untrusted address is returned.

So this means that proxyaddr() returns the client IP, which in my case is <my_external_ip>.

@tuetenk0pp
Copy link
Author

I am getting the idea that it would be better to use proxyaddr.all(req, 'uniquelocal') (docs), lose the last object of the returned list and check the remaining list against the allowed_ips.

@twk3
Copy link
Contributor

twk3 commented Jul 2, 2024

@tuetenk0pp at the moment if your proxy is a private IP it would have been already trusted, and so you don't need to specify any proxy in the args.

Because we are using 'uniquelocal' we only need folks to pass a trusted proxy that matches the closest public IP range proxy before actualbudget server. In this case we only care about a single trusted endpoint, our closest one. We want to make sure the auth header is coming through a proxy we trust. (just as a form of allowing users to turn on header auth for specific routes, rather than the public way to your budget if you have one)

A better experience would likely be to not use uniquelocal and instead include the private ranges in our allowed_ips. That way even if folks provide a private IP range (even though they don't have to), it would still work as they expected.

@tuetenk0pp
Copy link
Author

@twk3 I understand this is the way it should work. Unfortunately for me, it produces the described behavior. My reverse proxy is in fact in a private range.

The way I understand the validateAuthHeader() funtion is that if no trusted proxy setting is provided, it returns true immediately. So this actually does not correspond to the desired behavior that it would check for matches in private ranges by default.

But the main issue I see is something else. From what I understand, the proxyaddr() function is not used properly. As it says in the docs:

The closest untrusted address is returned.

So this means, if my reverse proxy were at 192.168.1.100, actual budget were at 192.168.1.101, my external IP were 100.100.100.100 and I set the trusted proxy to 192.168.1.100, proxyaddr(req, 'uniquelocal') would return my external IP: 100.100.100.100. But then, it uses the external IP to compare it against the trusted proxy, which in this example is 192.168.1.100 using the subnetMatch() funktion. So of course, the comparison fails and validateAuthHeader() returns false.

That's why I created #379.

@twk3
Copy link
Contributor

twk3 commented Jul 8, 2024

The way I understand the validateAuthHeader() funtion is that if no trusted proxy setting is provided, it returns true immediately. So this actually does not correspond to the desired behavior that it would check for matches in private ranges by default.

Ahh yes, that is correct. We need a change that fixes that.

What we want to be checking is the closest untrusted proxy though, and not all IPs in the request. The ideal that I would want would be if you could define the proxies in your network between the server and the proxy that's we're trying to ignore. (These maybe load balancers and gateways in your network, today we've hardcoded these to uniquelocal), and the auth proxy. (this is what we've defined as the trusted proxy in our config). Then all other IPs beyond that need to be ignored (cloudflare/cloudfront etc and the like, where you have very little control of the IP even if they are the front of your network, and of course the client ip)

Maybe we named the config poorly, maybe it should have been authProxy instead of trustedProxies. (as trusted proxies would usually be the list that goes into the ipaddr function instead of uniquelocal)

@ankel
Copy link

ankel commented Jul 9, 2024

Not related to this bug but I saw your caddy log as X-Forwarded-For header; did you do anything else with your setup?
With a simple traefik -> actual-server, I got #392

@tuetenk0pp
Copy link
Author

Not related to this bug but I saw your caddy log as X-Forwarded-For header; did you do anything else with your setup? With a simple traefik -> actual-server, I got #392

Now I also get this error.

@tuetenk0pp
Copy link
Author

What we want to be checking is the closest untrusted proxy though, and not all IPs in the request. The ideal that I would want would be if you could define the proxies in your network between the server and the proxy that's we're trying to ignore. (These maybe load balancers and gateways in your network, today we've hardcoded these to uniquelocal), and the auth proxy. (this is what we've defined as the trusted proxy in our config). Then all other IPs beyond that need to be ignored (cloudflare/cloudfront etc and the like, where you have very little control of the IP even if they are the front of your network, and of course the client ip)

Maybe we named the config poorly, maybe it should have been authProxy instead of trustedProxies. (as trusted proxies would usually be the list that goes into the ipaddr function instead of uniquelocal)

Caution

Still, the ACTUAL_TRUSTED_PROXIES setting does not work and validateAuthHeader() will always check an untrusted IP against the trusted IPs and therefore always return False.

I see #399 will hopefully resolve this so there is probably no need for #379. Feel free to close that / I will close it once #399 hits main.

@SDR3078
Copy link

SDR3078 commented Oct 6, 2024

I have the same issue with using Authentik and NGINX. Also bypassed this by adding all IPv6 addresses to trusted proxies which is indeed not the best way. As I understand it is that the X-Forwarded-For header is used to determine the proxy? Would it be a temp fix to change this header to $server_addr in the NGINX config?

@ColinHebert
Copy link

@tuetenk0pp would it be possible to reopen your PR, given that #399 seems to be abandonned?

@tuetenk0pp
Copy link
Author

@tuetenk0pp would it be possible to reopen your PR, given that #399 seems to be abandonned?

Probably a good idea. I will check on it later.

@twk3
Copy link
Contributor

twk3 commented Oct 23, 2024

@tuetenk0pp #379 breaks other use cases by requiring all Ips to be trusted other than the last one. This won't work if you are on most university/hotel internet where you are going through additional proxies.

I think the fix for now is:

  • don't require people to pass the trusted ip when it's just the default private ranges
  • If a user provides a private range in the trust, don't use uniquelocal.

This should allow uses with proxies i the private ranges to set this up without needing to specif the trust, AND if they do setup the trust, it should still work.

I can take a look at making these changes over the coming week, @tuetenk0pp I'll ping you when they're ready so you can take a look and see if it fixes the problem for you.

@twk3
Copy link
Contributor

twk3 commented Nov 27, 2024

I have the fix in #499 but it would be great if someone who is already using header auth can give it a try before it goes over to maintainer review.

@tuetenk0pp
Copy link
Author

I have the fix in #499 but it would be great if someone who is already using header auth can give it a try before it goes over to maintainer review.

If this can wait another 10 days, I might be able to do it.

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