actualbudget / actual-server

Actual's server
https://actualbudget.org
MIT License
2.97k stars 552 forks source link

[Bug]: ACTUAL_TRUSTED_PROXIES has not desired effect #371

Open tuetenk0pp opened 1 month ago

tuetenk0pp commented 1 month ago

Verified issue does not already exist?

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.

https://github.com/actualbudget/actual-server/blob/1af5ab09e92954235cc3efff74c274c10fb4a2e9/src/util/validate-user.js#L32

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 commented 1 month ago

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 commented 1 month ago

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 commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

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 commented 1 week ago

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 commented 6 days ago

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 commented 6 days ago

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.