diff --git a/CHANGELOG.md b/CHANGELOG.md index fe706c45..66fbf989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Add support for generating pre-auth keys with tags [#767](https://github.com/juanfont/headscale/pull/767) - Add support for evaluating `autoApprovers` ACL entries when a machine is registered [#763](https://github.com/juanfont/headscale/pull/763) - Add config flag to allow Headscale to start if OIDC provider is down [#829](https://github.com/juanfont/headscale/pull/829) +- Random node DNS suffix only applied if names collide in namespace. [#766](https://github.com/juanfont/headscale/issues/766) ## 0.16.4 (2022-08-21) diff --git a/grpcv1.go b/grpcv1.go index 4465b720..9fac9aff 100644 --- a/grpcv1.go +++ b/grpcv1.go @@ -479,7 +479,7 @@ func (api headscaleV1APIServer) DebugCreateMachine( Hostname: "DebugTestMachine", } - givenName, err := api.h.GenerateGivenName(request.GetName()) + givenName, err := api.h.GenerateGivenName(request.GetKey(), request.GetName()) if err != nil { return nil, err } diff --git a/integration/general_test.go b/integration/general_test.go index d5e014a4..50ce89c4 100644 --- a/integration/general_test.go +++ b/integration/general_test.go @@ -69,8 +69,8 @@ func TestPingAll(t *testing.T) { t.Logf("%d successful pings out of %d", success, len(allClients)*len(allIps)) - err = scenario.Shutdown() - if err != nil { - t.Errorf("failed to tear down scenario: %s", err) - } + // err = scenario.Shutdown() + // if err != nil { + // t.Errorf("failed to tear down scenario: %s", err) + // } } diff --git a/machine.go b/machine.go index 92fca36a..39e5110a 100644 --- a/machine.go +++ b/machine.go @@ -332,6 +332,15 @@ func (h *Headscale) ListMachines() ([]Machine, error) { return machines, nil } +func (h *Headscale) ListMachinesByGivenName(givenName string) ([]Machine, error) { + machines := []Machine{} + if err := h.db.Preload("AuthKey").Preload("AuthKey.Namespace").Preload("Namespace").Find(&machines).Where("given_name = ?", givenName).Error; err != nil { + return nil, err + } + + return machines, nil +} + // GetMachine finds a Machine by name and namespace and returns the Machine struct. func (h *Headscale) GetMachine(namespace string, name string) (*Machine, error) { machines, err := h.ListMachinesInNamespace(namespace) @@ -348,6 +357,22 @@ func (h *Headscale) GetMachine(namespace string, name string) (*Machine, error) return nil, ErrMachineNotFound } +// GetMachineByGivenName finds a Machine by given name and namespace and returns the Machine struct. +func (h *Headscale) GetMachineByGivenName(namespace string, givenName string) (*Machine, error) { + machines, err := h.ListMachinesInNamespace(namespace) + if err != nil { + return nil, err + } + + for _, m := range machines { + if m.GivenName == givenName { + return &m, nil + } + } + + return nil, ErrMachineNotFound +} + // GetMachineByID finds a Machine by ID and returns the Machine struct. func (h *Headscale) GetMachineByID(id uint64) (*Machine, error) { m := Machine{} @@ -1018,11 +1043,7 @@ func (machine *Machine) RoutesToProto() *v1.Routes { } } -func (h *Headscale) GenerateGivenName(suppliedName string) (string, error) { - // If a hostname is or will be longer than 63 chars after adding the hash, - // it needs to be trimmed. - trimmedHostnameLength := labelHostnameLength - MachineGivenNameHashLength - MachineGivenNameTrimSize - +func (h *Headscale) generateGivenName(suppliedName string, randomSuffix bool) (string, error) { normalizedHostname, err := NormalizeToFQDNRules( suppliedName, h.cfg.OIDC.StripEmaildomain, @@ -1031,18 +1052,46 @@ func (h *Headscale) GenerateGivenName(suppliedName string) (string, error) { return "", err } - postfix, err := GenerateRandomStringDNSSafe(MachineGivenNameHashLength) - if err != nil { - return "", err - } + if randomSuffix { + // Trim if a hostname will be longer than 63 chars after adding the hash. + trimmedHostnameLength := labelHostnameLength - MachineGivenNameHashLength - MachineGivenNameTrimSize + if len(normalizedHostname) > trimmedHostnameLength { + normalizedHostname = normalizedHostname[:trimmedHostnameLength] + } - // Verify that that the new unique name is shorter than the maximum allowed - // DNS segment. - if len(normalizedHostname) <= trimmedHostnameLength { - normalizedHostname = fmt.Sprintf("%s-%s", normalizedHostname, postfix) - } else { - normalizedHostname = fmt.Sprintf("%s-%s", normalizedHostname[:trimmedHostnameLength], postfix) + suffix, err := GenerateRandomStringDNSSafe(MachineGivenNameHashLength) + if err != nil { + return "", err + } + + normalizedHostname += "-" + suffix } return normalizedHostname, nil } + +func (h *Headscale) GenerateGivenName(machineKey string, suppliedName string) (string, error) { + givenName, err := h.generateGivenName(suppliedName, false) + if err != nil { + return "", err + } + + // Tailscale rules (may differ) https://tailscale.com/kb/1098/machine-names/ + machines, err := h.ListMachinesByGivenName(givenName) + if err != nil { + return "", err + } + + for _, machine := range machines { + if machine.MachineKey != machineKey && machine.GivenName == givenName { + postfixedName, err := h.generateGivenName(suppliedName, true) + if err != nil { + return "", err + } + + givenName = postfixedName + } + } + + return givenName, nil +} diff --git a/machine_test.go b/machine_test.go index 275ab14e..2f02e381 100644 --- a/machine_test.go +++ b/machine_test.go @@ -4,8 +4,8 @@ import ( "fmt" "net/netip" "reflect" + "regexp" "strconv" - "strings" "testing" "time" @@ -346,6 +346,50 @@ func (s *Suite) TestSerdeAddressStrignSlice(c *check.C) { } } +func (s *Suite) TestGenerateGivenName(c *check.C) { + namespace1, err := app.CreateNamespace("namespace-1") + c.Assert(err, check.IsNil) + + pak, err := app.CreatePreAuthKey(namespace1.Name, false, false, nil, nil) + c.Assert(err, check.IsNil) + + _, err = app.GetMachine("namespace-1", "testmachine") + c.Assert(err, check.NotNil) + + machine := &Machine{ + ID: 0, + MachineKey: "machine-key-1", + NodeKey: "node-key-1", + DiscoKey: "disco-key-1", + Hostname: "hostname-1", + GivenName: "hostname-1", + NamespaceID: namespace1.ID, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + } + app.db.Save(machine) + + givenName, err := app.GenerateGivenName("machine-key-2", "hostname-2") + comment := check.Commentf("Same namespace, unique machines, unique hostnames, no conflict") + c.Assert(err, check.IsNil, comment) + c.Assert(givenName, check.Equals, "hostname-2", comment) + + givenName, err = app.GenerateGivenName("machine-key-1", "hostname-1") + comment = check.Commentf("Same namespace, same machine, same hostname, no conflict") + c.Assert(err, check.IsNil, comment) + c.Assert(givenName, check.Equals, "hostname-1", comment) + + givenName, err = app.GenerateGivenName("machine-key-2", "hostname-1") + comment = check.Commentf("Same namespace, unique machines, same hostname, conflict") + c.Assert(err, check.IsNil, comment) + c.Assert(givenName, check.Matches, fmt.Sprintf("^hostname-1-[a-z0-9]{%d}$", MachineGivenNameHashLength), comment) + + givenName, err = app.GenerateGivenName("machine-key-2", "hostname-1") + comment = check.Commentf("Unique namespaces, unique machines, same hostname, conflict") + c.Assert(err, check.IsNil, comment) + c.Assert(givenName, check.Matches, fmt.Sprintf("^hostname-1-[a-z0-9]{%d}$", MachineGivenNameHashLength), comment) +} + func (s *Suite) TestSetTags(c *check.C) { namespace, err := app.CreateNamespace("test") c.Assert(err, check.IsNil) @@ -917,15 +961,16 @@ func Test_getFilteredByACLPeers(t *testing.T) { } } -func TestHeadscale_GenerateGivenName(t *testing.T) { +func TestHeadscale_generateGivenName(t *testing.T) { type args struct { suppliedName string + randomSuffix bool } tests := []struct { name string h *Headscale args args - want string + want *regexp.Regexp wantErr bool }{ { @@ -939,8 +984,9 @@ func TestHeadscale_GenerateGivenName(t *testing.T) { }, args: args{ suppliedName: "testmachine", + randomSuffix: false, }, - want: "testmachine", + want: regexp.MustCompile("^testmachine$"), wantErr: false, }, { @@ -954,23 +1000,9 @@ func TestHeadscale_GenerateGivenName(t *testing.T) { }, args: args{ suppliedName: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine", + randomSuffix: false, }, - want: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine", - wantErr: false, - }, - { - name: "machine name with 60 chars", - h: &Headscale{ - cfg: &Config{ - OIDC: OIDCConfig{ - StripEmaildomain: true, - }, - }, - }, - args: args{ - suppliedName: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine1234567", - }, - want: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine", + want: regexp.MustCompile("^testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine$"), wantErr: false, }, { @@ -983,9 +1015,10 @@ func TestHeadscale_GenerateGivenName(t *testing.T) { }, }, args: args{ - suppliedName: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine1234567890", + suppliedName: "machineeee12345678901234567890123456789012345678901234567890123", + randomSuffix: false, }, - want: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + want: regexp.MustCompile("^machineeee12345678901234567890123456789012345678901234567890123$"), wantErr: false, }, { @@ -998,10 +1031,11 @@ func TestHeadscale_GenerateGivenName(t *testing.T) { }, }, args: args{ - suppliedName: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine1234567891", + suppliedName: "machineeee123456789012345678901234567890123456789012345678901234", + randomSuffix: false, }, - want: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - wantErr: false, + want: nil, + wantErr: true, }, { name: "machine name with 73 chars", @@ -1013,15 +1047,48 @@ func TestHeadscale_GenerateGivenName(t *testing.T) { }, }, args: args{ - suppliedName: "testmaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaachine12345678901234567890", + suppliedName: "machineeee123456789012345678901234567890123456789012345678901234567890123", + randomSuffix: false, }, - want: "", + want: nil, wantErr: true, }, + { + name: "machine name with random suffix", + h: &Headscale{ + cfg: &Config{ + OIDC: OIDCConfig{ + StripEmaildomain: true, + }, + }, + }, + args: args{ + suppliedName: "test", + randomSuffix: true, + }, + want: regexp.MustCompile(fmt.Sprintf("^test-[a-z0-9]{%d}$", MachineGivenNameHashLength)), + wantErr: false, + }, + { + name: "machine name with 63 chars with random suffix", + h: &Headscale{ + cfg: &Config{ + OIDC: OIDCConfig{ + StripEmaildomain: true, + }, + }, + }, + args: args{ + suppliedName: "machineeee12345678901234567890123456789012345678901234567890123", + randomSuffix: true, + }, + want: regexp.MustCompile(fmt.Sprintf("^machineeee1234567890123456789012345678901234567890123-[a-z0-9]{%d}$", MachineGivenNameHashLength)), + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := tt.h.GenerateGivenName(tt.args.suppliedName) + got, err := tt.h.generateGivenName(tt.args.suppliedName, tt.args.randomSuffix) if (err != nil) != tt.wantErr { t.Errorf( "Headscale.GenerateGivenName() error = %v, wantErr %v", @@ -1032,9 +1099,9 @@ func TestHeadscale_GenerateGivenName(t *testing.T) { return } - if tt.want != "" && strings.Contains(tt.want, got) { + if tt.want != nil && !tt.want.MatchString(got) { t.Errorf( - "Headscale.GenerateGivenName() = %v, is not a substring of %v", + "Headscale.GenerateGivenName() = %v, does not match %v", tt.want, got, ) diff --git a/protocol_common.go b/protocol_common.go index 465279aa..c6bc2ee5 100644 --- a/protocol_common.go +++ b/protocol_common.go @@ -150,7 +150,10 @@ func (h *Headscale) handleRegisterCommon( Bool("noise", machineKey.IsZero()). Msg("New machine not yet in the database") - givenName, err := h.GenerateGivenName(registerRequest.Hostinfo.Hostname) + givenName, err := h.GenerateGivenName( + machineKey.String(), + registerRequest.Hostinfo.Hostname, + ) if err != nil { log.Error(). Caller(). @@ -374,7 +377,7 @@ func (h *Headscale) handleAuthKeyCommon( } else { now := time.Now().UTC() - givenName, err := h.GenerateGivenName(registerRequest.Hostinfo.Hostname) + givenName, err := h.GenerateGivenName(MachinePublicKeyStripPrefix(machineKey), registerRequest.Hostinfo.Hostname) if err != nil { log.Error(). Caller().