From d76c99251f2733a6e468c2875542ee1b51c14120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Motiejus=20Jak=C5=A1tys?= Date: Thu, 21 Nov 2024 21:11:12 +0200 Subject: [PATCH] [24.11] headscale: backport BaseDomain and ServerURL checks Currently users upgrading from 24.05 to 24.11 may stumble across an overly-restrictive BaseURL and ServerURL check in headscale[1]. A fix has been merged upstream[2], this is backport, so users can have it easier upgrading from 24.05 to 24.11. [1]: https://github.com/juanfont/headscale/issues/2210 [2]: https://github.com/juanfont/headscale/pull/2248 --- pkgs/servers/headscale/default.nix | 2 + ...n-up-BaseDomain-and-ServerURL-checks.patch | 204 ++++++++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 pkgs/servers/headscale/patches/config-loosen-up-BaseDomain-and-ServerURL-checks.patch diff --git a/pkgs/servers/headscale/default.nix b/pkgs/servers/headscale/default.nix index 4d3b1921f9ae4..292a8c69fb963 100644 --- a/pkgs/servers/headscale/default.nix +++ b/pkgs/servers/headscale/default.nix @@ -16,6 +16,8 @@ buildGoModule rec { hash = "sha256-5tlnVNpn+hJayxHjTpbOO3kRInOYOFz0pe9pwjXZlBE="; }; + patches = [ ./patches/config-loosen-up-BaseDomain-and-ServerURL-checks.patch ]; + vendorHash = "sha256-+8dOxPG/Q+wuHgRwwWqdphHOuop0W9dVyClyQuh7aRc="; ldflags = ["-s" "-w" "-X github.com/juanfont/headscale/cmd/headscale/cli.Version=v${version}"]; diff --git a/pkgs/servers/headscale/patches/config-loosen-up-BaseDomain-and-ServerURL-checks.patch b/pkgs/servers/headscale/patches/config-loosen-up-BaseDomain-and-ServerURL-checks.patch new file mode 100644 index 0000000000000..d226bc99ceffe --- /dev/null +++ b/pkgs/servers/headscale/patches/config-loosen-up-BaseDomain-and-ServerURL-checks.patch @@ -0,0 +1,204 @@ +From 6ba8990b0b982b261b0b549080a2f7f780cc70d6 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Motiejus=20Jak=C5=A1tys?= +Date: Thu, 21 Nov 2024 06:28:45 +0200 +Subject: [PATCH] config: loosen up BaseDomain and ServerURL checks + +Requirements [here][1]: + +> OK: +> server_url: headscale.com, base: clients.headscale.com +> server_url: headscale.com, base: headscale.net +> +> Not OK: +> server_url: server.headscale.com, base: headscale.com +> +> Essentially we have to prevent the possibility where the headscale +> server has a URL which can also be assigned to a node. +> +> So for the Not OK scenario: +> +> if the server is: server.headscale.com, and a node joins with the name +> server, it will be assigned server.headscale.com and that will break +> the connection for nodes which will now try to connect to that node +> instead of the headscale server. + +Fixes #2210 + +[1]: https://github.com/juanfont/headscale/issues/2210#issuecomment-2488165187 +--- + hscontrol/types/config.go | 44 +++++++++++-- + hscontrol/types/config_test.go | 64 ++++++++++++++++++- + .../testdata/base-domain-in-server-url.yaml | 2 +- + 3 files changed, 102 insertions(+), 8 deletions(-) + +diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go +index 50ce2f075f4c..b10118aaeade 100644 +--- a/hscontrol/types/config.go ++++ b/hscontrol/types/config.go +@@ -28,8 +28,9 @@ const ( + maxDuration time.Duration = 1<<63 - 1 + ) + +-var errOidcMutuallyExclusive = errors.New( +- "oidc_client_secret and oidc_client_secret_path are mutually exclusive", ++var ( ++ errOidcMutuallyExclusive = errors.New("oidc_client_secret and oidc_client_secret_path are mutually exclusive") ++ errServerURLSuffix = errors.New("server_url cannot be part of base_domain in a way that could make the DERP and headscale server unreachable") + ) + + type IPAllocationStrategy string +@@ -814,10 +815,10 @@ func LoadServerConfig() (*Config, error) { + // - DERP run on their own domains + // - Control plane runs on login.tailscale.com/controlplane.tailscale.com + // - MagicDNS (BaseDomain) for users is on a *.ts.net domain per tailnet (e.g. tail-scale.ts.net) +- // +- // TODO(kradalby): remove dnsConfig.UserNameInMagicDNS check when removed. +- if !dnsConfig.UserNameInMagicDNS && dnsConfig.BaseDomain != "" && strings.Contains(serverURL, dnsConfig.BaseDomain) { +- return nil, errors.New("server_url cannot contain the base_domain, this will cause the headscale server and embedded DERP to become unreachable from the Tailscale node.") ++ if !dnsConfig.UserNameInMagicDNS && dnsConfig.BaseDomain != "" { ++ if err := isSafeServerURL(serverURL, dnsConfig.BaseDomain); err != nil { ++ return nil, err ++ } + } + + return &Config{ +@@ -910,6 +911,37 @@ func LoadServerConfig() (*Config, error) { + }, nil + } + ++// BaseDomain cannot be a suffix of the server URL. ++// This is because Tailscale takes over the domain in BaseDomain, ++// causing the headscale server and DERP to be unreachable. ++// For Tailscale upstream, the following is true: ++// - DERP run on their own domains. ++// - Control plane runs on login.tailscale.com/controlplane.tailscale.com. ++// - MagicDNS (BaseDomain) for users is on a *.ts.net domain per tailnet (e.g. tail-scale.ts.net). ++func isSafeServerURL(serverURL, baseDomain string) error { ++ server, err := url.Parse(serverURL) ++ if err != nil { ++ return err ++ } ++ ++ serverDomainParts := strings.Split(server.Host, ".") ++ baseDomainParts := strings.Split(baseDomain, ".") ++ ++ if len(serverDomainParts) <= len(baseDomainParts) { ++ return nil ++ } ++ ++ s := len(serverDomainParts) ++ b := len(baseDomainParts) ++ for i := range len(baseDomainParts) { ++ if serverDomainParts[s-i-1] != baseDomainParts[b-i-1] { ++ return nil ++ } ++ } ++ ++ return errServerURLSuffix ++} ++ + type deprecator struct { + warns set.Set[string] + fatals set.Set[string] +diff --git a/hscontrol/types/config_test.go b/hscontrol/types/config_test.go +index e6e8d6c2e0b1..68a13f6c0f40 100644 +--- a/hscontrol/types/config_test.go ++++ b/hscontrol/types/config_test.go +@@ -1,6 +1,7 @@ + package types + + import ( ++ "fmt" + "os" + "path/filepath" + "testing" +@@ -141,7 +142,7 @@ func TestReadConfig(t *testing.T) { + return LoadServerConfig() + }, + want: nil, +- wantErr: "server_url cannot contain the base_domain, this will cause the headscale server and embedded DERP to become unreachable from the Tailscale node.", ++ wantErr: errServerURLSuffix.Error(), + }, + { + name: "base-domain-not-in-server-url", +@@ -337,3 +338,64 @@ tls_letsencrypt_challenge_type: TLS-ALPN-01 + err = LoadConfig(tmpDir, false) + assert.NoError(t, err) + } ++ ++// OK ++// server_url: headscale.com, base: clients.headscale.com ++// server_url: headscale.com, base: headscale.net ++// ++// NOT OK ++// server_url: server.headscale.com, base: headscale.com. ++func TestSafeServerURL(t *testing.T) { ++ tests := []struct { ++ serverURL, baseDomain, ++ wantErr string ++ }{ ++ { ++ serverURL: "https://example.com", ++ baseDomain: "example.org", ++ }, ++ { ++ serverURL: "https://headscale.com", ++ baseDomain: "headscale.com", ++ }, ++ { ++ serverURL: "https://headscale.com", ++ baseDomain: "clients.headscale.com", ++ }, ++ { ++ serverURL: "https://headscale.com", ++ baseDomain: "clients.subdomain.headscale.com", ++ }, ++ { ++ serverURL: "https://headscale.kristoffer.com", ++ baseDomain: "mybase", ++ }, ++ { ++ serverURL: "https://server.headscale.com", ++ baseDomain: "headscale.com", ++ wantErr: errServerURLSuffix.Error(), ++ }, ++ { ++ serverURL: "https://server.subdomain.headscale.com", ++ baseDomain: "headscale.com", ++ wantErr: errServerURLSuffix.Error(), ++ }, ++ { ++ serverURL: "http://foo\x00", ++ wantErr: `parse "http://foo\x00": net/url: invalid control character in URL`, ++ }, ++ } ++ ++ for _, tt := range tests { ++ testName := fmt.Sprintf("server=%s domain=%s", tt.serverURL, tt.baseDomain) ++ t.Run(testName, func(t *testing.T) { ++ err := isSafeServerURL(tt.serverURL, tt.baseDomain) ++ if tt.wantErr != "" { ++ assert.EqualError(t, err, tt.wantErr) ++ ++ return ++ } ++ assert.NoError(t, err) ++ }) ++ } ++} +diff --git a/hscontrol/types/testdata/base-domain-in-server-url.yaml b/hscontrol/types/testdata/base-domain-in-server-url.yaml +index 683e021837c9..2d6a4694a09a 100644 +--- a/hscontrol/types/testdata/base-domain-in-server-url.yaml ++++ b/hscontrol/types/testdata/base-domain-in-server-url.yaml +@@ -8,7 +8,7 @@ prefixes: + database: + type: sqlite3 + +-server_url: "https://derp.no" ++server_url: "https://server.derp.no" + + dns: + magic_dns: true +-- +2.47.0 +