From 14a3f94f0cab3f88322350bc060f28c406754821 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 26 Jun 2024 13:44:40 +0200 Subject: [PATCH] fix search domains and remove username from magicdns (#1987) --- CHANGELOG.md | 4 ++ config-example.yaml | 9 +++ hscontrol/mapper/mapper.go | 52 ++++++++------- hscontrol/mapper/mapper_test.go | 11 +-- hscontrol/mapper/tail.go | 2 +- hscontrol/types/config.go | 29 ++++---- hscontrol/types/node.go | 25 ++++--- hscontrol/types/node_test.go | 114 ++++++++++++++++++++++++++++---- 8 files changed, 183 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 666e1670..fced0b6d 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 f1bc1631..4608317a 100644 --- a/config-example.yaml +++ b/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 diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index a6fa9ad6..adc49669 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 + } + + 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, diff --git a/hscontrol/mapper/mapper_test.go b/hscontrol/mapper/mapper_test.go index 2ba3d031..be48c6fa 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 ac39d35e..92fbed81 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 00934af6..8ac8dcc4 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 @@ -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"), diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index 3ccadc38..6bee5c42 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 157be89e..85857c3a 100644 --- a/hscontrol/types/node_test.go +++ b/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)