headscale: backport BaseDomain and ServerURL checks

pkgs/servers/headscale/default.nix

@@ -16,6 +16,9 @@ buildGoModule rec {
     hash = "sha256-5tlnVNpn+hJayxHjTpbOO3kRInOYOFz0pe9pwjXZlBE=";
   };
 
+  # Merged post-v0.23.0, so should be removed with next release.
+  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}"];

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?= <motiejus@jakstys.lt>
+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
+