From 2070126c652a0b3be89a6c3d23bdd20c8b4082bf Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 20 Jun 2024 10:39:13 +0200 Subject: [PATCH 1/2] fix issue where search domains are ignored This commit fixes an issue where search domains was ignored if the user has not configured any external resolvers. Based on tailscale's documentation, there is no mention of this behaviour as far as I can see: https://tailscale.com/kb/1054/dns#search-domains In addition, it does add the missing base domain to the search domain. Signed-off-by: Kristoffer Dalby --- hscontrol/types/config.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index ab17cfb0ee..c1d9c52862 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -536,16 +536,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 @@ -571,8 +561,11 @@ 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 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 } From 8dfa1d3da81ba6ad44a87989137a579f67b8eef1 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 20 Jun 2024 10:45:23 +0200 Subject: [PATCH 2/2] remove username from magicdns names This commit removes the username from magicDNS names. This is in preparation of fixing tags, which currently is not correctly disassociated from users when added. With the current behaviour, fixing tagged devices would break the MagicDNS behaviour. This brings headscale to use the same behaviour of tailscale. Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 4 ++ config-example.yaml | 9 +++ hscontrol/mapper/mapper.go | 54 ++++++++------- hscontrol/mapper/mapper_test.go | 11 +-- hscontrol/mapper/tail.go | 2 +- hscontrol/types/config.go | 14 +++- hscontrol/types/node.go | 25 ++++--- hscontrol/types/node_test.go | 114 ++++++++++++++++++++++++++++---- 8 files changed, 180 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dce08f68f1..6740d83a10 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config-example.yaml b/config-example.yaml index 867f890330..86604df0f0 100644 --- a/config-example.yaml +++ b/config-example.yaml @@ -261,6 +261,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 diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index d4f4392a2e..bb36f02ce4 100644 --- a/hscontrol/mapper/mapper.go +++ b/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 + } - // 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, - ), - ) + dnsConfig := cfg.DNSConfig.Clone() - 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 + // if MagicDNS is enabled + 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) + } + 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, diff --git a/hscontrol/mapper/mapper_test.go b/hscontrol/mapper/mapper_test.go index 2ba3d0318f..be48c6fa95 100644 --- a/hscontrol/mapper/mapper_test.go +++ b/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{ { diff --git a/hscontrol/mapper/tail.go b/hscontrol/mapper/tail.go index ac39d35e9e..92fbed81ec 100644 --- a/hscontrol/mapper/tail.go +++ b/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) } diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index c1d9c52862..db39761645 100644 --- a/hscontrol/types/config.go +++ b/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 @@ -203,6 +204,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) @@ -561,6 +563,13 @@ func GetDNSConfig() (*tailcfg.DNSConfig, string) { baseDomain = "headscale.net" // does not really matter when MagicDNS is not enabled } + 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...) } @@ -708,7 +717,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"), diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 3ccadc3895..6bee5c42e7 100644 --- a/hscontrol/types/node.go +++ b/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", diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index 157be89e00..85857c3a1e 100644 --- a/hscontrol/types/node_test.go +++ b/hscontrol/types/node_test.go @@ -126,50 +126,135 @@ 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", + name: "all-set-with-username", node: Node{ GivenName: "test", User: User{ Name: "user", }, }, - dns: tailcfg.DNSConfig{ - Proxied: true, + cfg: Config{ + DNSConfig: &tailcfg.DNSConfig{ + Proxied: true, + }, + DNSUserNameInMagicDNS: true, }, domain: "example.com", want: "test.user.example.com", }, { - name: "no-given-name", + name: "no-given-name-with-username", node: Node{ User: User{ Name: "user", }, }, - dns: tailcfg.DNSConfig{ - Proxied: true, + 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", + name: "no-user-name-with-username", node: Node{ GivenName: "test", User: User{}, }, - dns: tailcfg.DNSConfig{ - Proxied: true, + 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{ + GivenName: "test", + User: User{ + Name: "user", + }, + }, + cfg: Config{ + DNSConfig: &tailcfg.DNSConfig{ + Proxied: true, + }, + DNSUserNameInMagicDNS: false, + }, + domain: "example.com", + want: "test.example.com", + }, + { + name: "no-given-name", + node: Node{ + User: User{ + Name: "user", + }, + }, + cfg: Config{ + DNSConfig: &tailcfg.DNSConfig{ + Proxied: true, + }, + DNSUserNameInMagicDNS: false, + }, + domain: "example.com", + wantErr: "failed to create valid FQDN: node has no given name", + }, + { + name: "no-user-name", + node: Node{ + GivenName: "test", + User: User{}, + }, + cfg: Config{ + DNSConfig: &tailcfg.DNSConfig{ + Proxied: true, + }, + DNSUserNameInMagicDNS: false, + }, + domain: "example.com", + want: "test.example.com", + }, { name: "no-magic-dns", node: Node{ @@ -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)