Skip to content

Commit

Permalink
headscale: backport BaseDomain and ServerURL checks
Browse files Browse the repository at this point in the history 
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 or unstable.

The patch does not apply cleanly on v0.23.0, so putting it here instead.

Supersedes NixOS#357969, this will be backported to 24.11 with a tag.

[1]: juanfont/headscale#2210
[2]: juanfont/headscale#2248
  • Loading branch information
@motiejus
motiejus committed Dec 10, 2024
1 parent b93b330 commit f1bdc12
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkgs/servers/headscale/default.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}"];
Expand Down
204 changes: 204 additions & 0 deletions pkgs/servers/headscale/patches/config-loosen-up-BaseDomain-and-ServerURL-checks.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
From 6ba8990b0b982b261b0b549080a2f7f780cc70d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Motiejus=20Jak=C5=A1tys?= <[email protected]>
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

0 comments on commit f1bdc12

Please sign in to comment.