foundryvtt / foundryvtt

Public issue tracking and documentation for Foundry Virtual Tabletop - software connecting RPG gamers in a shared multiplayer environment with an intuitive interface and powerful API.
https://foundryvtt.com/
198 stars 10 forks source link

Remove the requirment that setup request have the same host origin, returning to prior 0.7.x behavior. #5263

Closed aaclayton closed 2 years ago

aaclayton commented 3 years ago

Originally in GitLab by @foresto

Environment Details

Issue Description

After letting Foundry 0.7.10 upgrade itself to 0.8.6, agreeing to the new license, and attempting to log in to the setup page, Foundry now responds with HTTP/1.1 403 Forbidden and the following json document:

{"error":"Cross-origin requests are not allowed"}

Nothing is written to the Foundry log when this happens.

The Firefox console includes this message after noting Foundry's 403 error:

Content Security Policy: The page’s settings blocked the loading of a resource at https://myhost.mydomain:myport/favicon.ico (“default-src”). [2 repeats] FaviconLoader.jsm:191:19

Nothing in the client or server stack was changed during the upgrade, except the files from the Foundry 0.8.6 zip archive. Node.js was already at version 14. Reverting to Foundry 0.7.10 restores functionality.

I have been told that some people on Foundry's Discord reported this same problem with both nginx and Microsoft reverse proxies. Someone also reported it here: https://github.com/felddy/foundryvtt-docker/discussions/210

Here is the TCP payload of the request packet sent from nginx to Foundry, captured by tcpdump (with sensitive data replaced):

POST /setup HTTP/1.1
Connection: upgrade
Host: 127.0.0.1:30000
Content-Length: [REDACTED]
User-Agent: Mozilla/5.0 ([REDACTED]) Gecko/20100101 Firefox/88.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
Content-Type: application/x-www-form-urlencoded
Origin: https://my-external-domain-name:my-external-port
DNT: 1
Referer: https://my-external-domain-name:my-external-port/setup
Cookie: session=[REDACTED]
Upgrade-Insecure-Requests: 1

adminKey=[REDACTED]&action=adminAuth

Here is the TCP payload of Foundry's response packet:

HTTP/1.1 403 Forbidden
X-Powered-By: Express
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept
X-Frame-Options: DENY
Content-Type: application/json; charset=utf-8
Content-Length: 49
ETag: [REDACTED]
Date: [REDACTED]
Connection: keep-alive
Keep-Alive: timeout=5

{"error":"Cross-origin requests are not allowed"}

My Config/options.json file contains hostname and proxyPort values matching my-external-domain-name and my-external-port in the request above, as well as proxySSL: true since nginx is indeed serving Foundry through https. Maybe Foundry 0.8.x doesn't check these values correctly (or doesn't pass them to Expressjs) when detecting cross-origin requests?

I don't know how to investigate further, since Foundry isn't telling us why it thinks there's a cross-origin request. Maybe Expressjs is screwing up, or maybe Foundry 0.8 has misconfigured it?

aaclayton commented 2 years ago

We have discussed internally and agreed that the reliance on the Host header as a mechanism for identifying cross-origin post requests is not ideal. We will remove this requirement in 0.8.7 (returning to 0.7.x behavior) and re-evaluate in 0.9.x adding a conventional CSRF token to the views which we want to protect in this way.

5380 (private)

aaclayton commented 2 years ago

Originally in GitLab by @mattkemp1

Having similar problems with Foundry 0.8.6 being fronted by Nginx (see Discord). But only with loading /game. Accessing /setup and /join work no problem. Once authenticated /game takes over a minute to for a request to /my-path/socket.io/?session=[REDACTED]&EIO=4&transport=websocket to finally return.

Nothing useful in the foundry logs that I can see. Reviewed the options.json and Nginx config with a Anathema before posting here. I did find this in the Ngnix logs:

2021/06/10 19:41:46 [error] 3066#0: *2285 connect() failed (111: Connection refused) while connecting to upstream, client: [REDACTED], server: foundry.[REDACTED], request: "GET /my-path/socket.io/?session=[REDACTED]&EIO=4&transport=websocket HTTP/1.1", upstream: "http://127.0.0.1:30003/mypath/socket.io/?session=[REDACTED]&EIO=4&transport=websocket", host: "foundry.[REDACTED]"

FWIW, I have tried the express.mjs patch and using other browsers to no avail.

aaclayton commented 2 years ago

Originally in GitLab by @Brendan-McCoy

Still no good for me. My nginx:

server {

    listen <nginxhost>:443 ssl http2;

    ssl_certificate /etc/ssl/private/fullchain.cer;
    ssl_certificate_key /etc/ssl/private/cert.key;

    # Enter your fully qualified domain name or leave blank
    server_name             <my.domain>

    # Sets the Max Upload size to 300 MB
    client_max_body_size 300M;

    # Proxy Requests to Foundry VTT
    location /vtt {

        # Set proxy headers
        proxy_set_header Host $host;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;

        # These are important to support WebSockets
        proxy_set_header Upgrade $http_upgrade;
        proxy_set_header Connection "Upgrade";

        # Make sure to set your Foundry VTT port number
        proxy_pass http://<foundrycontainer>:30000;
    }
}

proxy_set_header Host $http_host; also doesn't work

options.json:

{
  "port": 30000,
  "upnp": false,
  "fullscreen": false,
  "hostname": "<my.domain>",
  "routePrefix": "vtt",
  "sslCert": null,
  "sslKey": null,
  "awsConfig": null,
  "dataPath": "/data",
  "proxySSL": true,
  "proxyPort": 443,
  "minifyStaticFiles": true,
  "updateChannel": "release",
  "language": "en.core",
  "world": null
}
aaclayton commented 2 years ago

Originally in GitLab by @mwliad

Hi,

Got the same issue on Linux (Ubuntu 18.04) with Caddy Server 1.0.4, using server-hosted VTT Foundry 0.8.6

In order to make it work I had to add the header_upstream line to my config:

my.domain.name {
        gzip
        proxy / http://localhost:30000 {
                header_upstream Host "my.domain.name"
                websocket
        }
}

You may have to update your wiki.

Cheers

aaclayton commented 2 years ago

Originally in GitLab by @kakaroto

Humm... we had a user today complaining about the exact same issue on the Forge, they weren't able to enter the admin access key, and we couldn't figure out why, as nobody was able to reproduce it. Obviously, there's no difference between my setup and the user's with regards to the reverse proxy, or which headers are included, so I don't know what might have caused it for that game specifically (sorry if this comment isn't particularly helpful).

aaclayton commented 2 years ago

Originally in GitLab by @sylvercode

Thanks for the help, it's appreciated. But for this bug, I am the one to blame. I confirm that proxy_set_header Host $http_host works!

For anyone using docker-swag, this is how you update the nginx config: https://github.com/linuxserver/docker-swag#updating-configs

aaclayton commented 2 years ago

Originally in GitLab by @foresto

Capturing the packets between nginx and Foundry could help in troubleshooting, as it would allow you to see what the request header looks like to Foundry. (Unfortunately, Foundry doesn't explain itself when producing this error, but we might figure it out if we see what it sees.) Wireshark is a popular tool for this sort of thing.

aaclayton commented 2 years ago

Originally in GitLab by @sylvercode

I have the same error and the mention patch (proxy_set_header Host $http_host) does not work for me. My setup is a bit more complex I think.

--> MyRouter (port 6666) --> Nginx port: 443 --> Fondry port: 30000

My ISP block port 443 so I need to use another port. Nginx is in a docker container and its purpose is to add SSL. Fondry is in another container and is in .

If I understand well here, the port is the problem here. I tried many combination of proxy_set_header Host ... but nothing seems to work. My next test is to match the external router port and the Nginx port. Any advice?

aaclayton commented 2 years ago

Originally in GitLab by @foresto

Yes, this is normally addressed through a combination of CSRF tokens and CORS.

Checking the host header seems pointless since a "malicious" site can simply forge that.

It can if the user agent allows it. (That's why CORS works: A trustworthy browser is in the best position to handle this part of the problem, while an untrustworthy browser cannot be stopped by a web app anyway.)

aaclayton commented 2 years ago

Yes, a CSRF token would be a better solution, but not a quick fix depending on whether we want to try and make a change in 0.8.7.

aaclayton commented 2 years ago

Originally in GitLab by @Qowy

Isn't this a classic problem of preventing "cross-site request forgery" which was traditionally solved by a CSRF token?

Checking the host header seems pointless since a "malicious" site can simply forge that.

aaclayton commented 2 years ago

These are all related issues.

  1. The Foundry VTT software currently uses the Host header to detect whether requests are cross-origin. There is a separate discussion about whether we should be doing that or whether we should be using some other approach.

  2. Because the Foundry software uses this header, it is expected to be present. That is why our reverse proxy configuration instructions designate that, for example in the case of nginx, proxy_set_header Host $host should be used. There is a further source of conversation around whether $host or $http_host is preferred which is also a separate point.

  3. In the absence of this header being set correctly, the port number of the proxied app will appear in the host header causing the logic in the Foundry software to fail - this could also be addressed but it's a downstream issue of the above 2 points and isn't really the root of the issue.

I haven't devoted enough mental energy to this yet to form an idea about what the right solution is. We do require the ability to determine whether a request comes from the Foundry VTT client (where the host header matches the request origin) or whether it comes from an external HTTP request that is externally posting to the Foundry web host (where it does not). There may be a better way to perform this test.

aaclayton commented 2 years ago

Originally in GitLab by @tyrodreamscape

This message in the discord provided by the user sPOIDar seems to indicate a possible source of the issue.

To bring the quote in here:

I see what the problem is - the isSameOrigin() function in views.mjs uses a regex match against the hostname only, and doesn't account for ports, so it happens to work for the standard 80/443 ports on the frontend, but not when using a non-standard port.

aaclayton commented 3 years ago

Originally in GitLab by @foresto

Caveat: this has to be set on a server level so it might affect other applications on the same Server

Thanks for pointing that out. It reinforces my view that Foundry probably shouldn't be depending on the Host: header field at all.

aaclayton commented 3 years ago

Originally in GitLab by @foresto

I want to address a few issues that I see here and on felddy's issue tracker:

  1. proxy_set_header Host $host is incorrect, because it fails to convey the port number as requested by the client, and therefore will work only for installations that use the same internal and external port number. On other installations, it will appear to work at login time, but quietly break other things like xmlhttp requests made by Foundry's UI. (Attempting to check modules for updates will reveal this.) Instead, it should be proxy_set_header Host $http_host; or proxy_set_header Host $host:$server_port; The nginx docs explain the differences. Foundry's nginx article needs fixing. (Edit: Ignoring the port number as Atropos suggested might solve that problem, but would also fail to flag same-server-different-port cross-origin requests. See also: my comments on point 3, below.)

  2. Using the term "CORS" is a little misleading here, since that term traditionally describes checks made by the web browser to prevent unauthorized cross-origin requests from being sent. The behavior we see here does not come from the browser at all; it comes from Foundry (maybe a library that Foundry integrates, but still part of Foundry) after a request has already been sent.

  3. I don't understand why this new Foundry version is looking at the Host header field at all. As I recall, that field was designed for a web server to distinguish virtual hosts, and a quick review of rfc 2616 confirms it. Since Foundry doesn't implement multitenancy, I would expect it to ignore that field. Using it for some kind of cross-origin check smells like a loss of separation of concerns, and a possible sign that a developer somewhere has misunderstood its purpose. (I admit that might be me; it's possible that some new standard has emerged since I stopped closely following HTTP protocol specs a few years ago.) If the header field is indeed being misused here, it might be wise to stop, lest it cause more trouble in the future.

aaclayton commented 3 years ago

Originally in GitLab by @Qowy

Thanks this helped me to fix the same problem with Microsoft IIS as a reverse Proxy. The relevant setting is: system.webServer.proxy:preserveHostHeaders true

Caveat: this has to be set on a server level so it might affect other applications on the same Server

This should also be added to the documentation (or 0.8 release notes)

aaclayton commented 3 years ago

Originally in GitLab by @thre-z

Thank you for the clarification!

I just wanted to keep both issues updated with each other.

aaclayton commented 3 years ago

Originally in GitLab by @felddy

An image update is not required. The container does not implement a proxy.

I documented the solution in the project discussions for anyone else that has failed to follow FoundryVTT knowledge base article, or the container's README section about nginx.

aaclayton commented 3 years ago

Originally in GitLab by @felddy

I was able to reproduce this issue.

If a user configured a proxy with a missing proxy_set_header Host $host; line it would function with v0.7.10.

As of v0.8.6 the same proxy configuration will generate a CORS error.

Adding the omitted nginx configuration line corrects the issue.

See also:

aaclayton commented 3 years ago

Originally in GitLab by @thre-z

FYI: I believe that felddy is updating their image to bring the proxy config in line with the directives in the KB: https://github.com/felddy/foundryvtt-docker/discussions/210#discussioncomment-811433

aaclayton commented 3 years ago

Originally in GitLab by @florad92

I cannot reproduce this issue either on Foundry 0.8.6.

My nginx comes from here https://foundryvtt.wiki/en/setup/hosting/Ubuntu-VM and includes the certbot SSL certs. As per my setup my main instance is running on localhost:8080 and is portforwarded to the foundry.<domain.tld> subdomain. If necessary I can post an anonymized copy of my config

aaclayton commented 3 years ago

Originally in GitLab by @thre-z

I'm not able to reproduce this issue.

I installed Foundry 0.8.6 on a fresh Ubuntu 20.04 VPS instance. Installed nginx (1.18.0). I copy and pasted the nginx configuration as on the KB (https://foundryvtt.com/article/nginx/) with no changes whatsoever except to include my own domain name.

The reverse proxy appears to be working fine. I was able to enter my license, install a system, a module, create a world, launch it, go back to set up, etc.

Edit to update: The default Caddy proxy config also seems to be working fine.

aaclayton commented 3 years ago

Thanks for sharing your findings @foresto and @Farwind. In the short term we'll make sure our KB article is up to date. I'll also look into stripping out the port number when determining same-origin status of requests.

aaclayton commented 3 years ago

Originally in GitLab by @foresto

More corrections: nginx should be configured to send $http_host rather than $host. Also, use proxy_set_header, not proxy_add_header. It should look like this:

proxy_set_header Host $http_host;

Using $host may break things for people using a different external port number. (I noticed this problem when module update checks never completed, because the corresponding xmlhttprequest was being quietly rejected.) Using proxy_add_header may cause nginx to reject the configuration with an "unknown directive" error.

Note that this means the knowledge base aticle is currently wrong. (It suggests using $host.)

aaclayton commented 3 years ago

Originally in GitLab by @foresto

It might be worth mentioning this new requirement in Foundry's release notes, to save other people the pain of troubleshooting.

aaclayton commented 3 years ago

Originally in GitLab by @foresto

@Farwind Thanks for the tip! That got my installation working.

One change that has occurred since previous version of nginx is that the Host header now needs to be added

One small correction: This new requirement to preserve the Host: header field does not come from a new version of nginx. (As noted earlier, my version of nginx has not changed.) Rather, it is a change in Foundry since version 0.7.10.

aaclayton commented 3 years ago

Originally in GitLab by @Farwind

Hi, I ran into this issue with my nginx configuration.

One change that has occurred since previous version of foundry is that the Host header now needs to be added in the nginx configuration

proxy_set_header Host $host;

Nginx previously would proxy without error without the above line, but won't in version 8.6

Looking at the documentation, this line is already mentioned in the nginx example: https://foundryvtt.com/article/nginx/

aaclayton commented 3 years ago

Originally in GitLab by @foresto

The 0.8.6 release announcement mentions this:

Support was added for HTTP2 which increases server side performance and networking transfer rates through the SPDY protocol

Could it be that it broke HTTP/1.1, and nobody noticed when testing because their browsers always negotiated HTTP2?