Skip to content

Commit

Permalink
[24.11] 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.

[1]: juanfont/headscale#2210
[2]: juanfont/headscale#2248
  • Loading branch information
@motiejus
motiejus committed Nov 21, 2024
1 parent 057f63b commit d76c992
Show file tree
Hide file tree
Showing 2 changed files with 206 additions and 0 deletions.
2 changes: 2 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,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}"];
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?= <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

0 comments on commit d76c992

Please sign in to comment.