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 juanfont#2210

[1]: juanfont#2210 (comment)

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 a suffix of the base_domain, this will cause the headscale server and embedded DERP to become unreachable from the Tailscale node.")
 )
 
 type IPAllocationStrategy string
@@ -787,17 +788,11 @@ func GetHeadscaleConfig() (*Config, error) {
 
 	serverURL := viper.GetString("server_url")
 
-	// BaseDomain cannot be the same as 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)
-	//
 	// 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{
@@ -894,6 +889,37 @@ func IsCLIConfigured() bool {
 	return viper.GetString("cli.address") != "" && viper.GetString("cli.api_key") != ""
 }
 
+// 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 := 1; i < len(baseDomainParts)-1; i++ {
+		if serverDomainParts[s-i] != baseDomainParts[b-i] {
+			return nil
+		}
+	}
+
+	return errServerURLSuffix
+}
+
 type deprecator struct {
 	warns  set.Set[string]
 	fatals set.Set[string]

hscontrol/types/config_test.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -139,7 +140,7 @@ func TestReadConfig(t *testing.T) {
 				return GetHeadscaleConfig()
 			},
 			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: "server_url cannot be a suffix of the base_domain, this will cause the headscale server and embedded DERP to become unreachable from the Tailscale node.",
 		},
 		{
 			name:       "base-domain-not-in-server-url",
@@ -289,3 +290,59 @@ func TestReadConfigFromEnv(t *testing.T) {
 		})
 	}
 }
+
+// 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://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)
+		})
+	}
+}

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