Update embeded derp host to requested host

[kev-the-dev]

DRAFT implementation of #1423. Not super happy with the implementation, but this may help illustrate the desired feature.

• read the CONTRIBUTING guidelines
• raised a GitHub issue or discussed it on the projects chat beforehand
• added unit tests
• added integration tests
• updated documentation if needed
• updated CHANGELOG.md

[juanfont]

First of all, thank you so much for the PR!

I see a problem, though. One biggest sources of issues for headscale users are misconfigured reverse proxies. If we start depending on the user configuring also the Host header, that'd be a problem...

[kev-the-dev]

One biggest sources of issues for headscale users are misconfigured reverse proxies. If we start depending on the user configuring also the Host header, that'd be a problem...

The concern is that someone would configure a reverse proxy to override the Host header incorrectly? I'm reading through the golang Request documentation and it sounds like perhaps request.URL would be more robust for this issue, since it isn't a header and generally will not be re-written by a reverse proxy? https://pkg.go.dev/net/http#Request

[kev-the-dev]

@juanfont it looks like request.URL actually won't work. From go docs:

// For server requests, the URL is parsed from the URI
// supplied on the Request-Line as stored in RequestURI.  For
// most requests, fields other than Path and RawQuery will be
// empty. (See [RFC 7230, Section 5.3](https://rfc-editor.org/rfc/rfc7230.html#section-5.3))
//

Given that there may be some risk for reverse proxies, would you accept the original feature (using Host) as a configuration option (default off)?

[juanfont]

But Host will depend on how you configure your reverse proxy.

For instance, in a typical nginx configuration users usually have something like proxy_pass http://127.0.0.1:8000 or proxy_pass http://foobar:8000 towards Headscale.

This will cause that the default Host header that Headscale will see will be something like '127.0.0.1:8000' or 'foobar:8000' - because that's the host nginx is calling to.

Only if the user remembers to overwrite the Host header to whatever real FQDN would your proposed solution work :(

[kev-the-dev]

I'm a bit confused about this because my setup seems fairly "normal" and my patch is working for this use case without either nginx or headscale ever needing to know the FQDN. I wonder if golang's http library is doing something unexpected to fill out Host.

upstream headscale {
    server 127.0.0.1:8080;
}

map $http_upgrade $connection_upgrade {
  default upgrade;
  ''      close;
}

server {
    listen 443 ssl default_server;
    server_name localhost;

    ssl_certificate     <cert>;
    ssl_certificate_key <key>;

    add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;

    access_log /dev/stdout main;
    error_log /dev/stderr warn;

    location ~* ^\/(headscale|derp|bootstrap-dns|ts2021).*$ {
        proxy_pass https://headscale;
        proxy_ssl_trusted_certificate <headscale_crt>;
        proxy_http_version 1.1;
        proxy_set_header Upgrade $http_upgrade;
        proxy_set_header Connection $connection_upgrade;
        proxy_buffering off;
    }
}

If I then set an /etc/hosts for 127.0.0.1 foo.bar, I can connect with --login-server foo.bar with no problem. Adding a debug line to my patch, I see headscale is receiving the correct Host. Somehow headscale seems to get the correct request.Host from behind the proxy.

7:50PM TRC setting machine.RequestedHost noise host=foo.bar

Regardless, as an optional configuration parameter, we can provide documentation on how to configure your reverse proxy to use this config.

[kev-the-dev]

Ah, after adding some more debug lines it looks like this works because of the noise protocol. Even though the reverse proxy may have the wrong host, inside the noise hijack HTTP connection, the Host is whatever was set by the tailscale client.

So this should actually work regardless of reverse proxy settings, assuming the client is using noise :)

Does this change your view on the risk of this change, either as a default config or optional?

Edit: we can have this feature only apply to noise connections to further reducer risk

[juanfont]

Edit: we can have this feature only apply to noise connections to further reducer risk

But this adds complexity. And we still need server_url for those users that do not use reverse proxies so Headscale can get a SSL cert.

And to be fair, having both server_url and listen_addr is an industry-practice that you can find in many many products.

[kev-the-dev]

Yes, this options adds some code complexity and that should always be weighed as a tradeoff against what is accomplishes.

I believe the complexity is worthwhile as it will simplify requirements for reverse proxy users, by no longer requiring the FQDN to be known at startup in server_url, and may allow new use cases like a headscale server which can be reached at multiple FQDN's (this may be particularly useful for multi-tenant scenarios making use of the namespaces/users feature).

This is a judgment call for the maintainers and community. Perhaps we can wait to see if other users on discord/github chime in with a desire for this feature. I'm happy to iterate on the implementation details and documentation in the future.

[kradalby]

Hi! as part of #1473, we have reorganised a lot of the code.

To clear PRs that needs to be rebased or redone, we are closing open PRs that will require significant code change to be merged.

In addition, the issue of the PR might in some cases have been fixed, change or no longer relevant, so it would be great if this is considered as well.

Thank you for your contribution!

If it is still relevant and the PR is reopened, we will aim at getting the changes into the next release after the reorg if accepted.