fix search domains and remove username from magicdns (#1987)

CHANGELOG.md

@@ -40,6 +40,10 @@ after improving the test harness as part of adopting [#1460](https://github.com/
 - Prefixes are now defined per v4 and v6 range. [#1756](https://github.com/juanfont/headscale/pull/1756)
   - `ip_prefixes` option is now `prefixes.v4` and `prefixes.v6`
   - `prefixes.allocation` can be set to assign IPs at `sequential` or `random`. [#1869](https://github.com/juanfont/headscale/pull/1869)
+- MagicDNS domains no longer contain usernames []()
+  - This is in preperation to fix Headscales implementation of tags which currently does not correctly remove the link between a tagged device and a user. As tagged devices will not have a user, this will require a change to the DNS generation, removing the username, see [#1369](https://github.com/juanfont/headscale/issues/1369) for more information.
+  - `use_username_in_magic_dns` can be used to turn this behaviour on again, but note that this option _will be removed_ when tags are fixed.
+  - This option brings Headscales behaviour in line with Tailscale.
 
 ### Changes
 

config-example.yaml

@@ -265,6 +265,15 @@ dns_config:
   # Only works if there is at least a nameserver defined.
   magic_dns: true
 
+  # DEPRECATED
+  # Use the username as part of the DNS name for nodes, with this option enabled:
+  # node1.username.example.com
+  # while when this is disabled:
+  # node1.example.com
+  # This is a legacy option as Headscale has have this wrongly implemented
+  # while in upstream Tailscale, the username is not included.
+  use_username_in_magic_dns: false
+
   # Defines the base domain to create the hostnames for MagicDNS.
   # `base_domain` must be a FQDNs, without the trailing dot.
   # The FQDN of the hosts will be

hscontrol/mapper/mapper.go

@@ -122,37 +122,41 @@ func generateUserProfiles(
 }
 
 func generateDNSConfig(
-	base *tailcfg.DNSConfig,
+	cfg *types.Config,
 	baseDomain string,
 	node *types.Node,
 	peers types.Nodes,
 ) *tailcfg.DNSConfig {
-	dnsConfig := base.Clone()
+	if cfg.DNSConfig == nil {
+		return nil
+	}
+
+	dnsConfig := cfg.DNSConfig.Clone()
 
 	// if MagicDNS is enabled
-	if base != nil && base.Proxied {
-		// Only inject the Search Domain of the current user
-		// shared nodes should use their full FQDN
-		dnsConfig.Domains = append(
-			dnsConfig.Domains,
-			fmt.Sprintf(
-				"%s.%s",
-				node.User.Name,
-				baseDomain,
-			),
-		)
+	if dnsConfig.Proxied {
+		if cfg.DNSUserNameInMagicDNS {
+			// Only inject the Search Domain of the current user
+			// shared nodes should use their full FQDN
+			dnsConfig.Domains = append(
+				dnsConfig.Domains,
+				fmt.Sprintf(
+					"%s.%s",
+					node.User.Name,
+					baseDomain,
+				),
+			)
 
-		userSet := mapset.NewSet[types.User]()
-		userSet.Add(node.User)
-		for _, p := range peers {
-			userSet.Add(p.User)
+			userSet := mapset.NewSet[types.User]()
+			userSet.Add(node.User)
+			for _, p := range peers {
+				userSet.Add(p.User)
+			}
+			for _, user := range userSet.ToSlice() {
+				dnsRoute := fmt.Sprintf("%v.%v", user.Name, baseDomain)
+				dnsConfig.Routes[dnsRoute] = nil
+			}
 		}
-		for _, user := range userSet.ToSlice() {
-			dnsRoute := fmt.Sprintf("%v.%v", user.Name, baseDomain)
-			dnsConfig.Routes[dnsRoute] = nil
-		}
-	} else {
-		dnsConfig = base
 	}
 
 	addNextDNSMetadata(dnsConfig.Resolvers, node)
@@ -568,7 +572,7 @@ func appendPeerChanges(
 	profiles := generateUserProfiles(node, changed, cfg.BaseDomain)
 
 	dnsConfig := generateDNSConfig(
-		cfg.DNSConfig,
+		cfg,
 		cfg.BaseDomain,
 		node,
 		peers,

hscontrol/mapper/mapper_test.go

@@ -127,7 +127,10 @@ func TestDNSConfigMapResponse(t *testing.T) {
 			}
 
 			got := generateDNSConfig(
-				&dnsConfigOrig,
+				&types.Config{
+					DNSConfig:             &dnsConfigOrig,
+					DNSUserNameInMagicDNS: true,
+				},
 				baseDomain,
 				nodeInShared1,
 				peersOfNodeInShared1,
@@ -187,9 +190,9 @@ func Test_fullMapResponse(t *testing.T) {
 		UserID:     0,
 		User:       types.User{Name: "mini"},
 		ForcedTags: []string{},
-		AuthKey:     &types.PreAuthKey{},
-		LastSeen:    &lastSeen,
-		Expiry:      &expire,
+		AuthKey:    &types.PreAuthKey{},
+		LastSeen:   &lastSeen,
+		Expiry:     &expire,
 		Hostinfo:   &tailcfg.Hostinfo{},
 		Routes: []types.Route{
 			{

hscontrol/mapper/tail.go

@@ -77,7 +77,7 @@ func tailNode(
 		keyExpiry = time.Time{}
 	}
 
-	hostname, err := node.GetFQDN(cfg.DNSConfig, cfg.BaseDomain)
+	hostname, err := node.GetFQDN(cfg, cfg.BaseDomain)
 	if err != nil {
 		return nil, fmt.Errorf("tailNode, failed to create FQDN: %s", err)
 	}

hscontrol/types/config.go

@@ -63,7 +63,8 @@ type Config struct {
 	ACMEURL   string
 	ACMEEmail string
 
-	DNSConfig *tailcfg.DNSConfig
+	DNSConfig             *tailcfg.DNSConfig
+	DNSUserNameInMagicDNS bool
 
 	UnixSocket           string
 	UnixSocketPermission fs.FileMode
@@ -204,6 +205,7 @@ func LoadConfig(path string, isFile bool) error {
 
 	viper.SetDefault("dns_config", nil)
 	viper.SetDefault("dns_config.override_local_dns", true)
+	viper.SetDefault("dns_config.use_username_in_magic_dns", false)
 
 	viper.SetDefault("derp.server.enabled", false)
 	viper.SetDefault("derp.server.stun.enabled", true)
@@ -540,16 +542,6 @@ func GetDNSConfig() (*tailcfg.DNSConfig, string) {
 			dnsConfig.Domains = domains
 		}
 
-		if viper.IsSet("dns_config.domains") {
-			domains := viper.GetStringSlice("dns_config.domains")
-			if len(dnsConfig.Resolvers) > 0 {
-				dnsConfig.Domains = domains
-			} else if domains != nil {
-				log.Warn().
-					Msg("Warning: dns_config.domains is set, but no nameservers are configured. Ignoring domains.")
-			}
-		}
-
 		if viper.IsSet("dns_config.extra_records") {
 			var extraRecords []tailcfg.DNSRecord
 
@@ -575,8 +567,18 @@ func GetDNSConfig() (*tailcfg.DNSConfig, string) {
 			baseDomain = "headscale.net" // does not really matter when MagicDNS is not enabled
 		}
 
-		log.Trace().Interface("dns_config", dnsConfig).Msg("DNS configuration loaded")
+		if !viper.GetBool("dns_config.use_username_in_magic_dns") {
+			dnsConfig.Domains = []string{baseDomain}
+		} else {
+			log.Warn().Msg("DNS: Usernames in DNS has been deprecated, this option will be remove in future versions")
+			log.Warn().Msg("DNS: see 0.23.0 changelog for more information.")
+		}
 
+		if domains := viper.GetStringSlice("dns_config.domains"); len(domains) > 0 {
+			dnsConfig.Domains = append(dnsConfig.Domains, domains...)
+		}
+
+		log.Trace().Interface("dns_config", dnsConfig).Msg("DNS configuration loaded")
 		return dnsConfig, baseDomain
 	}
 
@@ -719,7 +721,8 @@ func GetHeadscaleConfig() (*Config, error) {
 
 		TLS: GetTLSConfig(),
 
-		DNSConfig: dnsConfig,
+		DNSConfig:             dnsConfig,
+		DNSUserNameInMagicDNS: viper.GetBool("dns_config.use_username_in_magic_dns"),
 
 		ACMEEmail: viper.GetString("acme_email"),
 		ACMEURL:   viper.GetString("acme_url"),

hscontrol/types/node.go

@@ -394,23 +394,32 @@ func (node *Node) Proto() *v1.Node {
 	return nodeProto
 }
 
-func (node *Node) GetFQDN(dnsConfig *tailcfg.DNSConfig, baseDomain string) (string, error) {
+func (node *Node) GetFQDN(cfg *Config, baseDomain string) (string, error) {
 	var hostname string
-	if dnsConfig != nil && dnsConfig.Proxied { // MagicDNS
+	if cfg.DNSConfig != nil && cfg.DNSConfig.Proxied { // MagicDNS
 		if node.GivenName == "" {
 			return "", fmt.Errorf("failed to create valid FQDN: %w", ErrNodeHasNoGivenName)
 		}
 
-		if node.User.Name == "" {
-			return "", fmt.Errorf("failed to create valid FQDN: %w", ErrNodeUserHasNoName)
-		}
-
 		hostname = fmt.Sprintf(
-			"%s.%s.%s",
+			"%s.%s",
 			node.GivenName,
-			node.User.Name,
 			baseDomain,
 		)
+
+		if cfg.DNSUserNameInMagicDNS {
+			if node.User.Name == "" {
+				return "", fmt.Errorf("failed to create valid FQDN: %w", ErrNodeUserHasNoName)
+			}
+
+			hostname = fmt.Sprintf(
+				"%s.%s.%s",
+				node.GivenName,
+				node.User.Name,
+				baseDomain,
+			)
+		}
+
 		if len(hostname) > MaxHostnameLength {
 			return "", fmt.Errorf(
 				"failed to create valid FQDN (%s): %w",

hscontrol/types/node_test.go

@@ -126,11 +126,87 @@ func TestNodeFQDN(t *testing.T) {
 	tests := []struct {
 		name    string
 		node    Node
-		dns     tailcfg.DNSConfig
+		cfg     Config
 		domain  string
 		want    string
 		wantErr string
 	}{
+		{
+			name: "all-set-with-username",
+			node: Node{
+				GivenName: "test",
+				User: User{
+					Name: "user",
+				},
+			},
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: true,
+				},
+				DNSUserNameInMagicDNS: true,
+			},
+			domain: "example.com",
+			want:   "test.user.example.com",
+		},
+		{
+			name: "no-given-name-with-username",
+			node: Node{
+				User: User{
+					Name: "user",
+				},
+			},
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: true,
+				},
+				DNSUserNameInMagicDNS: true,
+			},
+			domain:  "example.com",
+			wantErr: "failed to create valid FQDN: node has no given name",
+		},
+		{
+			name: "no-user-name-with-username",
+			node: Node{
+				GivenName: "test",
+				User:      User{},
+			},
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: true,
+				},
+				DNSUserNameInMagicDNS: true,
+			},
+			domain:  "example.com",
+			wantErr: "failed to create valid FQDN: node user has no name",
+		},
+		{
+			name: "no-magic-dns-with-username",
+			node: Node{
+				GivenName: "test",
+				User: User{
+					Name: "user",
+				},
+			},
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: false,
+				},
+				DNSUserNameInMagicDNS: true,
+			},
+			domain: "example.com",
+			want:   "test",
+		},
+		{
+			name: "no-dnsconfig-with-username",
+			node: Node{
+				GivenName: "test",
+				User: User{
+					Name: "user",
+				},
+			},
+			domain: "example.com",
+			want:   "test",
+		},
 		{
 			name: "all-set",
 			node: Node{
@@ -139,11 +215,14 @@ func TestNodeFQDN(t *testing.T) {
 					Name: "user",
 				},
 			},
-			dns: tailcfg.DNSConfig{
-				Proxied: true,
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: true,
+				},
+				DNSUserNameInMagicDNS: false,
 			},
 			domain: "example.com",
-			want:   "test.user.example.com",
+			want:   "test.example.com",
 		},
 		{
 			name: "no-given-name",
@@ -152,8 +231,11 @@ func TestNodeFQDN(t *testing.T) {
 					Name: "user",
 				},
 			},
-			dns: tailcfg.DNSConfig{
-				Proxied: true,
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: true,
+				},
+				DNSUserNameInMagicDNS: false,
 			},
 			domain:  "example.com",
 			wantErr: "failed to create valid FQDN: node has no given name",
@@ -164,11 +246,14 @@ func TestNodeFQDN(t *testing.T) {
 				GivenName: "test",
 				User:      User{},
 			},
-			dns: tailcfg.DNSConfig{
-				Proxied: true,
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: true,
+				},
+				DNSUserNameInMagicDNS: false,
 			},
-			domain:  "example.com",
-			wantErr: "failed to create valid FQDN: node user has no name",
+			domain: "example.com",
+			want:   "test.example.com",
 		},
 		{
 			name: "no-magic-dns",
@@ -178,8 +263,11 @@ func TestNodeFQDN(t *testing.T) {
 					Name: "user",
 				},
 			},
-			dns: tailcfg.DNSConfig{
-				Proxied: false,
+			cfg: Config{
+				DNSConfig: &tailcfg.DNSConfig{
+					Proxied: false,
+				},
+				DNSUserNameInMagicDNS: false,
 			},
 			domain: "example.com",
 			want:   "test",
@@ -199,7 +287,7 @@ func TestNodeFQDN(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := tc.node.GetFQDN(&tc.dns, tc.domain)
+			got, err := tc.node.GetFQDN(&tc.cfg, tc.domain)
 
 			if (err != nil) && (err.Error() != tc.wantErr) {
 				t.Errorf("GetFQDN() error = %s, wantErr %s", err, tc.wantErr)