From 9b7d657cbe4ae7f648798a7080819f3863f6b7c8 Mon Sep 17 00:00:00 2001 From: Jamie Greeff Date: Wed, 29 Dec 2021 09:58:10 +0000 Subject: [PATCH 01/31] Return all peers instead of peers in same namespace --- dns.go | 6 +++- machine.go | 87 ++++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 67 insertions(+), 26 deletions(-) diff --git a/dns.go b/dns.go index 8ecd993c..37daa884 100644 --- a/dns.go +++ b/dns.go @@ -163,7 +163,11 @@ func getMapResponseDNSConfig( dnsConfig = dnsConfigOrig.Clone() dnsConfig.Domains = append( dnsConfig.Domains, - fmt.Sprintf("%s.%s", machine.Namespace.Name, baseDomain), + fmt.Sprintf( + "%s.%s", + strings.Replace(machine.Namespace.Name, "@", ".", -1), // Replace @ with . for valid domain for machine + baseDomain, + ), ) namespaceSet := set.New(set.ThreadSafe) diff --git a/machine.go b/machine.go index 2b462985..aef310c5 100644 --- a/machine.go +++ b/machine.go @@ -119,6 +119,33 @@ func (machine Machine) isExpired() bool { return time.Now().UTC().After(*machine.Expiry) } +// Our Pineapple fork of Headscale ignores namespaces when dealing with peers +// and instead passes ALL peers across all namespaces to each client. Access between clients +// is then enforced with ACL policies. +func (h *Headscale) getAllPeers(machine *Machine) (Machines, error) { + log.Trace(). + Caller(). + Str("machine", machine.Name). + Msg("Finding all peers") + + machines := Machines{} + if err := h.db.Preload("Namespace").Where("machine_key <> ? AND registered", + machine.MachineKey).Find(&machines).Error; err != nil { + log.Error().Err(err).Msg("Error accessing db") + + return Machines{}, err + } + + sort.Slice(machines, func(i, j int) bool { return machines[i].ID < machines[j].ID }) + + log.Trace(). + Caller(). + Str("machine", machine.Name). + Msgf("Found all machines: %s", machines.String()) + + return machines, nil +} + func (h *Headscale) getDirectPeers(machine *Machine) (Machines, error) { log.Trace(). Caller(). @@ -206,7 +233,40 @@ func (h *Headscale) getSharedTo(machine *Machine) (Machines, error) { } func (h *Headscale) getPeers(machine *Machine) (Machines, error) { - direct, err := h.getDirectPeers(machine) + // direct, err := h.getDirectPeers(machine) + // if err != nil { + // log.Error(). + // Caller(). + // Err(err). + // Msg("Cannot fetch peers") + + // return Machines{}, err + // } + + // shared, err := h.getShared(machine) + // if err != nil { + // log.Error(). + // Caller(). + // Err(err). + // Msg("Cannot fetch peers") + + // return Machines{}, err + // } + + // sharedTo, err := h.getSharedTo(machine) + // if err != nil { + // log.Error(). + // Caller(). + // Err(err). + // Msg("Cannot fetch peers") + + // return Machines{}, err + // } + + // peers := append(direct, shared...) + // peers = append(peers, sharedTo...) + + peers, err := h.getAllPeers(machine) if err != nil { log.Error(). Caller(). @@ -216,29 +276,6 @@ func (h *Headscale) getPeers(machine *Machine) (Machines, error) { return Machines{}, err } - shared, err := h.getShared(machine) - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Cannot fetch peers") - - return Machines{}, err - } - - sharedTo, err := h.getSharedTo(machine) - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Cannot fetch peers") - - return Machines{}, err - } - - peers := append(direct, shared...) - peers = append(peers, sharedTo...) - sort.Slice(peers, func(i, j int) bool { return peers[i].ID < peers[j].ID }) log.Trace(). @@ -597,7 +634,7 @@ func (machine Machine) toNode( hostname = fmt.Sprintf( "%s.%s.%s", machine.Name, - machine.Namespace.Name, + strings.Replace(machine.Namespace.Name, "@", ".", -1), // Replace @ with . for valid domain for machine baseDomain, ) } else { From e482dfeed4a06e7d9be966a8068dd06b826f686d Mon Sep 17 00:00:00 2001 From: Adrien Raffin Date: Thu, 3 Feb 2022 19:53:11 +0100 Subject: [PATCH 02/31] feat(machine): add ACLFilter if ACL's are enabled. This commit change the default behaviour and remove the notion of namespaces between the hosts. It allows all namespaces to be only filtered by the ACLs. This behavior is closer to tailsnet. --- machine.go | 133 ++++++++++++++++++++++++++++++++---------------- machine_test.go | 84 ++++++++++++++++++++++++++++++ utils.go | 9 ++++ 3 files changed, 183 insertions(+), 43 deletions(-) diff --git a/machine.go b/machine.go index aef310c5..231c179d 100644 --- a/machine.go +++ b/machine.go @@ -119,14 +119,21 @@ func (machine Machine) isExpired() bool { return time.Now().UTC().After(*machine.Expiry) } -// Our Pineapple fork of Headscale ignores namespaces when dealing with peers -// and instead passes ALL peers across all namespaces to each client. Access between clients -// is then enforced with ACL policies. -func (h *Headscale) getAllPeers(machine *Machine) (Machines, error) { +func containsAddresses(inputs []string, addrs MachineAddresses) bool { + for _, addr := range addrs.ToStringSlice() { + if containsString(inputs, addr) { + return true + } + } + return false +} + +// getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. +func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { log.Trace(). Caller(). Str("machine", machine.Name). - Msg("Finding all peers") + Msg("Finding peers filtered by ACLs") machines := Machines{} if err := h.db.Preload("Namespace").Where("machine_key <> ? AND registered", @@ -135,15 +142,50 @@ func (h *Headscale) getAllPeers(machine *Machine) (Machines, error) { return Machines{}, err } + mMachines := make(map[uint64]Machine) - sort.Slice(machines, func(i, j int) bool { return machines[i].ID < machines[j].ID }) + // Aclfilter peers here. We are itering through machines in all namespaces and search through the computed aclRules + // for match between rule SrcIPs and DstPorts. If the rule is a match we allow the machine to be viewable. + + // FIXME: On official control plane if a rule allow user A to talk to user B but NO rule allows user B to talk to + // user A. The behaviour is the following + // + // On official tailscale control plane: + // on first `tailscale status`` on node A we can see node B. The `tailscale status` command on node B doesn't show node A + // We can successfully establish a communication from A to B. When it's done, if we run the `tailscale status` command + // on node B again we can now see node A. It's not possible to establish a communication from node B to node A. + // On this implementation of the feature + // on any `tailscale status` command on node A we can see node B. The `tailscale status` command on node B DOES show A. + // + // I couldn't find a way to not clutter the output of `tailscale status` with all nodes that we could be talking to. + // In order to do this we would need to be able to identify that node A want to talk to node B but that Node B doesn't know + // how to talk to node A and then add the peering resource. + + for _, m := range machines { + for _, rule := range h.aclRules { + var dst []string + for _, d := range rule.DstPorts { + dst = append(dst, d.IP) + } + if (containsAddresses(rule.SrcIPs, machine.IPAddresses) && (containsAddresses(dst, m.IPAddresses) || containsString(dst, "*"))) || + (containsAddresses(rule.SrcIPs, m.IPAddresses) && containsAddresses(dst, machine.IPAddresses)) { + mMachines[m.ID] = m + } + } + } + + var authorizedMachines Machines + for _, m := range mMachines { + authorizedMachines = append(authorizedMachines, m) + } + sort.Slice(authorizedMachines, func(i, j int) bool { return authorizedMachines[i].ID < authorizedMachines[j].ID }) log.Trace(). Caller(). Str("machine", machine.Name). - Msgf("Found all machines: %s", machines.String()) + Msgf("Found some machines: %s", machines.String()) - return machines, nil + return authorizedMachines, nil } func (h *Headscale) getDirectPeers(machine *Machine) (Machines, error) { @@ -233,47 +275,52 @@ func (h *Headscale) getSharedTo(machine *Machine) (Machines, error) { } func (h *Headscale) getPeers(machine *Machine) (Machines, error) { - // direct, err := h.getDirectPeers(machine) - // if err != nil { - // log.Error(). - // Caller(). - // Err(err). - // Msg("Cannot fetch peers") + var peers Machines + var err error + // If ACLs rules are defined, filter visible host list with the ACLs + // else use the classic namespace scope + if h.aclPolicy != nil { + peers, err = h.getFilteredByACLPeers(machine) + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("Cannot fetch peers") - // return Machines{}, err - // } + return Machines{}, err + } + } else { + direct, err := h.getDirectPeers(machine) + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("Cannot fetch peers") - // shared, err := h.getShared(machine) - // if err != nil { - // log.Error(). - // Caller(). - // Err(err). - // Msg("Cannot fetch peers") + return Machines{}, err + } - // return Machines{}, err - // } + shared, err := h.getShared(machine) + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("Cannot fetch peers") - // sharedTo, err := h.getSharedTo(machine) - // if err != nil { - // log.Error(). - // Caller(). - // Err(err). - // Msg("Cannot fetch peers") + return Machines{}, err + } - // return Machines{}, err - // } + sharedTo, err := h.getSharedTo(machine) + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("Cannot fetch peers") - // peers := append(direct, shared...) - // peers = append(peers, sharedTo...) - - peers, err := h.getAllPeers(machine) - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Cannot fetch peers") - - return Machines{}, err + return Machines{}, err + } + peers = append(direct, shared...) + peers = append(peers, sharedTo...) } sort.Slice(peers, func(i, j int) bool { return peers[i].ID < peers[j].ID }) diff --git a/machine_test.go b/machine_test.go index ff1dc914..f84603a4 100644 --- a/machine_test.go +++ b/machine_test.go @@ -1,6 +1,7 @@ package headscale import ( + "fmt" "strconv" "time" @@ -154,6 +155,89 @@ func (s *Suite) TestGetDirectPeers(c *check.C) { c.Assert(peersOfMachine0[8].Name, check.Equals, "testmachine10") } +func (s *Suite) TestGetACLFilteredPeers(c *check.C) { + type base struct { + namespace *Namespace + key *PreAuthKey + } + + var stor []base + + for _, name := range []string{"test", "admin"} { + namespace, err := app.CreateNamespace(name) + c.Assert(err, check.IsNil) + pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) + c.Assert(err, check.IsNil) + stor = append(stor, base{namespace, pak}) + + } + + _, err := app.GetMachineByID(0) + c.Assert(err, check.NotNil) + + for index := 0; index <= 10; index++ { + machine := Machine{ + ID: uint64(index), + MachineKey: "foo" + strconv.Itoa(index), + NodeKey: "bar" + strconv.Itoa(index), + DiscoKey: "faa" + strconv.Itoa(index), + IPAddress: fmt.Sprintf("100.64.0.%v", strconv.Itoa(index+1)), + Name: "testmachine" + strconv.Itoa(index), + NamespaceID: stor[index%2].namespace.ID, + Registered: true, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(stor[index%2].key.ID), + } + app.db.Save(&machine) + } + + app.aclPolicy = &ACLPolicy{ + Groups: map[string][]string{ + "group:test": {"admin"}, + }, + Hosts: map[string]netaddr.IPPrefix{}, + TagOwners: map[string][]string{}, + ACLs: []ACL{ + {Action: "accept", Users: []string{"admin"}, Ports: []string{"*:*"}}, + {Action: "accept", Users: []string{"test"}, Ports: []string{"test:*"}}, + }, + Tests: []ACLTest{}, + } + + rules, err := app.generateACLRules() + c.Assert(err, check.IsNil) + app.aclRules = rules + + adminMachine, err := app.GetMachineByID(1) + c.Logf("Machine(%v), namespace: %v", adminMachine.Name, adminMachine.Namespace) + c.Assert(err, check.IsNil) + + testMachine, err := app.GetMachineByID(2) + c.Logf("Machine(%v), namespace: %v", testMachine.Name, testMachine.Namespace) + c.Assert(err, check.IsNil) + + _, err = testMachine.GetHostInfo() + c.Assert(err, check.IsNil) + + peersOfTestMachine, err := app.getFilteredByACLPeers(testMachine) + c.Assert(err, check.IsNil) + + peersOfAdminMachine, err := app.getFilteredByACLPeers(adminMachine) + c.Assert(err, check.IsNil) + + c.Log(peersOfTestMachine) + c.Assert(len(peersOfTestMachine), check.Equals, 4) + c.Assert(peersOfTestMachine[0].Name, check.Equals, "testmachine4") + c.Assert(peersOfTestMachine[1].Name, check.Equals, "testmachine6") + c.Assert(peersOfTestMachine[3].Name, check.Equals, "testmachine10") + + c.Log(peersOfAdminMachine) + c.Assert(len(peersOfAdminMachine), check.Equals, 9) + c.Assert(peersOfAdminMachine[0].Name, check.Equals, "testmachine2") + c.Assert(peersOfAdminMachine[2].Name, check.Equals, "testmachine4") + c.Assert(peersOfAdminMachine[5].Name, check.Equals, "testmachine7") +} + func (s *Suite) TestExpireMachine(c *check.C) { namespace, err := app.CreateNamespace("test") c.Assert(err, check.IsNil) diff --git a/utils.go b/utils.go index a6be8cc4..794971a1 100644 --- a/utils.go +++ b/utils.go @@ -212,6 +212,15 @@ func (h *Headscale) getUsedIPs() ([]netaddr.IP, error) { return ips, nil } +func containsString(ss []string, s string) bool { + for _, v := range ss { + if v == s { + return true + } + } + return false +} + func containsIPs(ips []netaddr.IP, ip netaddr.IP) bool { for _, v := range ips { if v == ip { From e9949b4c70e5a0bc0599e10d854ece73976c06e4 Mon Sep 17 00:00:00 2001 From: Adrien Raffin Date: Thu, 3 Feb 2022 20:00:41 +0100 Subject: [PATCH 03/31] feat(acls): simplify updating rules --- acls.go | 8 +++++--- machine_test.go | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/acls.go b/acls.go index 326625b9..e2b2a27c 100644 --- a/acls.go +++ b/acls.go @@ -69,14 +69,16 @@ func (h *Headscale) LoadACLPolicy(path string) error { } h.aclPolicy = &policy + return h.UpdateACLRules() +} + +func (h *Headscale) UpdateACLRules() error { rules, err := h.generateACLRules() if err != nil { return err } - h.aclRules = rules - log.Trace().Interface("ACL", rules).Msg("ACL rules generated") - + h.aclRules = rules return nil } diff --git a/machine_test.go b/machine_test.go index f84603a4..56fdf1f3 100644 --- a/machine_test.go +++ b/machine_test.go @@ -204,9 +204,8 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { Tests: []ACLTest{}, } - rules, err := app.generateACLRules() + err = app.UpdateACLRules() c.Assert(err, check.IsNil) - app.aclRules = rules adminMachine, err := app.GetMachineByID(1) c.Logf("Machine(%v), namespace: %v", adminMachine.Name, adminMachine.Namespace) From fb45138fc19fffeb336e46c0a859ee9c711240be Mon Sep 17 00:00:00 2001 From: Adrien Raffin Date: Sat, 5 Feb 2022 17:18:39 +0100 Subject: [PATCH 04/31] feat(acls): check acl owners and add bunch of tests --- acls.go | 96 ++++++++++++++------- acls_test.go | 220 ++++++++++++++++++++++++++++++++++++++++++++++++ machine_test.go | 2 +- 3 files changed, 287 insertions(+), 31 deletions(-) diff --git a/acls.go b/acls.go index e2b2a27c..c86e315f 100644 --- a/acls.go +++ b/acls.go @@ -2,6 +2,7 @@ package headscale import ( "encoding/json" + "errors" "fmt" "io" "os" @@ -90,8 +91,6 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { return nil, errInvalidAction } - filterRule := tailcfg.FilterRule{} - srcIPs := []string{} for innerIndex, user := range acl.Users { srcs, err := h.generateACLPolicySrcIP(user) @@ -103,7 +102,6 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { } srcIPs = append(srcIPs, srcs...) } - filterRule.SrcIPs = srcIPs destPorts := []tailcfg.NetPortRange{} for innerIndex, ports := range acl.Ports { @@ -174,17 +172,23 @@ func (h *Headscale) generateACLPolicyDestPorts( return dests, nil } +// expandalias has an input of either +// - a namespace +// - a group +// - a tag +// and transform these in IPAddresses func (h *Headscale) expandAlias(alias string) ([]string, error) { if alias == "*" { return []string{"*"}, nil } if strings.HasPrefix(alias, "group:") { - if _, ok := h.aclPolicy.Groups[alias]; !ok { - return nil, errInvalidGroup + namespaces, err := h.expandGroup(alias) + if err != nil { + return nil, err } ips := []string{} - for _, n := range h.aclPolicy.Groups[alias] { + for _, n := range namespaces { nodes, err := h.ListMachinesInNamespace(n) if err != nil { return nil, errInvalidNamespace @@ -198,40 +202,35 @@ func (h *Headscale) expandAlias(alias string) ([]string, error) { } if strings.HasPrefix(alias, "tag:") { - if _, ok := h.aclPolicy.TagOwners[alias]; !ok { - return nil, errInvalidTag - } - - // This will have HORRIBLE performance. - // We need to change the data model to better store tags - machines := []Machine{} - if err := h.db.Where("registered").Find(&machines).Error; err != nil { + var ips []string + owners, err := h.expandTagOwners(alias) + if err != nil { return nil, err } - ips := []string{} - for _, machine := range machines { - hostinfo := tailcfg.Hostinfo{} - if len(machine.HostInfo) != 0 { - hi, err := machine.HostInfo.MarshalJSON() + for _, namespace := range owners { + machines, err := h.ListMachinesInNamespace(namespace) + if err != nil { + if errors.Is(err, errNamespaceNotFound) { + continue + } else { + return nil, err + } + } + for _, machine := range machines { + if len(machine.HostInfo) == 0 { + continue + } + hi, err := machine.GetHostInfo() if err != nil { return nil, err } - err = json.Unmarshal(hi, &hostinfo) - if err != nil { - return nil, err - } - - // FIXME: Check TagOwners allows this - for _, t := range hostinfo.RequestTags { - if alias[4:] == t { + for _, t := range hi.RequestTags { + if alias == t { ips = append(ips, machine.IPAddresses.ToStringSlice()...) - - break } } } } - return ips, nil } @@ -266,6 +265,43 @@ func (h *Headscale) expandAlias(alias string) ([]string, error) { return nil, errInvalidUserSection } +// expandTagOwners will return a list of namespace. An owner can be either a namespace or a group +// a group cannot be composed of groups +func (h *Headscale) expandTagOwners(owner string) ([]string, error) { + var owners []string + ows, ok := h.aclPolicy.TagOwners[owner] + if !ok { + return []string{}, fmt.Errorf("%w. %v isn't owned by a TagOwner. Please add one first. https://tailscale.com/kb/1018/acls/#tag-owners", errInvalidTag, owner) + } + for _, ow := range ows { + if strings.HasPrefix(ow, "group:") { + gs, err := h.expandGroup(ow) + if err != nil { + return []string{}, err + } + owners = append(owners, gs...) + } else { + owners = append(owners, ow) + } + } + return owners, nil +} + +// expandGroup will return the list of namespace inside the group +// after some validation +func (h *Headscale) expandGroup(group string) ([]string, error) { + gs, ok := h.aclPolicy.Groups[group] + if !ok { + return []string{}, fmt.Errorf("group %v isn't registered. %w", group, errInvalidGroup) + } + for _, g := range gs { + if strings.HasPrefix(g, "group:") { + return []string{}, fmt.Errorf("%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", errInvalidGroup) + } + } + return gs, nil +} + func (h *Headscale) expandPorts(portsStr string) (*[]tailcfg.PortRange, error) { if portsStr == "*" { return &[]tailcfg.PortRange{ diff --git a/acls_test.go b/acls_test.go index c35f4f84..f2fb6a03 100644 --- a/acls_test.go +++ b/acls_test.go @@ -1,7 +1,11 @@ package headscale import ( + "errors" + "gopkg.in/check.v1" + "gorm.io/datatypes" + "inet.af/netaddr" ) func (s *Suite) TestWrongPath(c *check.C) { @@ -52,6 +56,222 @@ func (s *Suite) TestBasicRule(c *check.C) { c.Assert(rules, check.NotNil) } +func (s *Suite) TestInvalidAction(c *check.C) { + app.aclPolicy = &ACLPolicy{ + ACLs: []ACL{ + {Action: "invalidAction", Users: []string{"*"}, Ports: []string{"*:*"}}, + }, + } + err := app.UpdateACLRules() + c.Assert(errors.Is(err, errInvalidAction), check.Equals, true) +} + +func (s *Suite) TestInvalidGroupInGroup(c *check.C) { + // this ACL is wrong because the group in users sections doesn't exist + app.aclPolicy = &ACLPolicy{ + Groups: Groups{"group:test": []string{"foo"}, "group:error": []string{"foo", "group:test"}}, + ACLs: []ACL{ + {Action: "accept", Users: []string{"group:error"}, Ports: []string{"*:*"}}, + }, + } + err := app.UpdateACLRules() + c.Assert(errors.Is(err, errInvalidGroup), check.Equals, true) +} + +func (s *Suite) TestInvalidTagOwners(c *check.C) { + // this ACL is wrong because no tagOwners own the requested tag for the server + app.aclPolicy = &ACLPolicy{ + ACLs: []ACL{ + {Action: "accept", Users: []string{"tag:foo"}, Ports: []string{"*:*"}}, + }, + } + err := app.UpdateACLRules() + c.Assert(errors.Is(err, errInvalidTag), check.Equals, true) +} + +// this test should validate that we can expand a group in a TagOWner section and +// match properly the IP's of the related hosts. The owner is valid and the tag is also valid. +// the tag is matched in the Users section +func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { + namespace, err := app.CreateNamespace("foo") + c.Assert(err, check.IsNil) + + pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) + c.Assert(err, check.IsNil) + + _, err = app.GetMachine("foo", "testmachine") + c.Assert(err, check.NotNil) + b := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") + machine := Machine{ + ID: 0, + MachineKey: "foo", + NodeKey: "bar", + DiscoKey: "faa", + Name: "testmachine", + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + NamespaceID: namespace.ID, + Registered: true, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + HostInfo: datatypes.JSON(b), + } + app.db.Save(&machine) + + app.aclPolicy = &ACLPolicy{ + Groups: Groups{"group:test": []string{"foo", "foobar"}}, + TagOwners: TagOwners{"tag:test": []string{"bar", "group:test"}}, + ACLs: []ACL{ + {Action: "accept", Users: []string{"tag:test"}, Ports: []string{"*:*"}}, + }, + } + err = app.UpdateACLRules() + c.Assert(err, check.IsNil) + c.Assert(app.aclRules, check.HasLen, 1) + c.Assert(app.aclRules[0].SrcIPs, check.HasLen, 1) + c.Assert(app.aclRules[0].SrcIPs[0], check.Equals, "100.64.0.1") +} + +// this test should validate that we can expand a group in a TagOWner section and +// match properly the IP's of the related hosts. The owner is valid and the tag is also valid. +// the tag is matched in the Ports section +func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { + namespace, err := app.CreateNamespace("foo") + c.Assert(err, check.IsNil) + + pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) + c.Assert(err, check.IsNil) + + _, err = app.GetMachine("foo", "testmachine") + c.Assert(err, check.NotNil) + b := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") + machine := Machine{ + ID: 1, + MachineKey: "foo", + NodeKey: "bar", + DiscoKey: "faa", + Name: "testmachine", + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + NamespaceID: namespace.ID, + Registered: true, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + HostInfo: datatypes.JSON(b), + } + app.db.Save(&machine) + + app.aclPolicy = &ACLPolicy{ + Groups: Groups{"group:test": []string{"foo", "foobar"}}, + TagOwners: TagOwners{"tag:test": []string{"bar", "group:test"}}, + ACLs: []ACL{ + {Action: "accept", Users: []string{"*"}, Ports: []string{"tag:test:*"}}, + }, + } + err = app.UpdateACLRules() + c.Assert(err, check.IsNil) + c.Assert(app.aclRules, check.HasLen, 1) + c.Assert(app.aclRules[0].DstPorts, check.HasLen, 1) + c.Assert(app.aclRules[0].DstPorts[0].IP, check.Equals, "100.64.0.1") +} + +// need a test with: +// tag on a host that isn't owned by a tag owners. So the namespace +// of the host should be valid +func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { + namespace, err := app.CreateNamespace("foo") + c.Assert(err, check.IsNil) + + pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) + c.Assert(err, check.IsNil) + + _, err = app.GetMachine("foo", "testmachine") + c.Assert(err, check.NotNil) + b := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:foo\"]}") + machine := Machine{ + ID: 1, + MachineKey: "foo", + NodeKey: "bar", + DiscoKey: "faa", + Name: "testmachine", + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + NamespaceID: namespace.ID, + Registered: true, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + HostInfo: datatypes.JSON(b), + } + app.db.Save(&machine) + + app.aclPolicy = &ACLPolicy{ + TagOwners: TagOwners{"tag:test": []string{"foo"}}, + ACLs: []ACL{ + {Action: "accept", Users: []string{"foo"}, Ports: []string{"*:*"}}, + }, + } + err = app.UpdateACLRules() + c.Assert(err, check.IsNil) + c.Assert(app.aclRules, check.HasLen, 1) + c.Assert(app.aclRules[0].SrcIPs, check.HasLen, 1) + c.Assert(app.aclRules[0].SrcIPs[0], check.Equals, "100.64.0.1") +} + +// tag on a host is owned by a tag owner, the tag is valid. +// an ACL rule is matching the tag to a namespace. It should not be valid since the +// host should be tied to the tag now. +func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { + namespace, err := app.CreateNamespace("foo") + c.Assert(err, check.IsNil) + + pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) + c.Assert(err, check.IsNil) + + _, err = app.GetMachine("foo", "webserver") + c.Assert(err, check.NotNil) + b := []byte("{\"OS\":\"centos\",\"Hostname\":\"webserver\",\"RequestTags\":[\"tag:webapp\"]}") + machine := Machine{ + ID: 1, + MachineKey: "foo", + NodeKey: "bar", + DiscoKey: "faa", + Name: "webserver", + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + NamespaceID: namespace.ID, + Registered: true, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + HostInfo: datatypes.JSON(b), + } + app.db.Save(&machine) + _, err = app.GetMachine("foo", "user") + b = []byte("{\"OS\":\"debian\",\"Hostname\":\"user\"}") + c.Assert(err, check.NotNil) + machine = Machine{ + ID: 2, + MachineKey: "foo2", + NodeKey: "bar2", + DiscoKey: "faab", + Name: "user", + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + NamespaceID: namespace.ID, + Registered: true, + RegisterMethod: RegisterMethodAuthKey, + AuthKeyID: uint(pak.ID), + HostInfo: datatypes.JSON(b), + } + app.db.Save(&machine) + + app.aclPolicy = &ACLPolicy{ + TagOwners: TagOwners{"tag:webapp": []string{"foo"}}, + ACLs: []ACL{ + {Action: "accept", Users: []string{"foo"}, Ports: []string{"tag:webapp:80,443"}}, + }, + } + err = app.UpdateACLRules() + c.Assert(err, check.IsNil) + c.Logf("Rules: %v", app.aclRules) + c.Assert(app.aclRules, check.HasLen, 1) + c.Assert(app.aclRules[0].SrcIPs, check.HasLen, 0) +} + func (s *Suite) TestPortRange(c *check.C) { err := app.LoadACLPolicy("./tests/acls/acl_policy_basic_range.hujson") c.Assert(err, check.IsNil) diff --git a/machine_test.go b/machine_test.go index 56fdf1f3..203289ac 100644 --- a/machine_test.go +++ b/machine_test.go @@ -181,7 +181,7 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { MachineKey: "foo" + strconv.Itoa(index), NodeKey: "bar" + strconv.Itoa(index), DiscoKey: "faa" + strconv.Itoa(index), - IPAddress: fmt.Sprintf("100.64.0.%v", strconv.Itoa(index+1)), + IPAddresses: MachineAddresses{netaddr.MustParseIP(fmt.Sprintf("100.64.0.%v", strconv.Itoa(index+1)))}, Name: "testmachine" + strconv.Itoa(index), NamespaceID: stor[index%2].namespace.ID, Registered: true, From 97eac3b9389e5eb17dc901f15bdb4882d0e7a167 Mon Sep 17 00:00:00 2001 From: Adrien Raffin Date: Sun, 6 Feb 2022 17:55:12 +0100 Subject: [PATCH 05/31] feat(acl): update frequently the aclRules This call should be done quite at each modification of a server resources like RequestTags. When a server changes it's tag we should rebuild the ACL rules. When a server is added to headscale we also should update the ACLRules. --- poll.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/poll.go b/poll.go index dd3956fb..c2a51d11 100644 --- a/poll.go +++ b/poll.go @@ -91,6 +91,12 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { machine.DiscoKey = DiscoPublicKeyStripPrefix(req.DiscoKey) now := time.Now().UTC() + // update ACLRules with peer informations (to update server tags if necessary) + err = h.UpdateACLRules() + if err != nil { + log.Error().Caller().Str("func", "handleAuthKey").Str("machine", machine.Name).Err(err) + } + // From Tailscale client: // // ReadOnly is whether the client just wants to fetch the MapResponse, From de5994644790b6948d1d39b12a59dfb9931df797 Mon Sep 17 00:00:00 2001 From: Adrien Raffin Date: Mon, 7 Feb 2022 16:12:05 +0100 Subject: [PATCH 06/31] feat(acls): rewrite functions to be testable Rewrite some function to get rid of the dependency on Headscale object. This allows us to write succinct test that are more easy to review and implement. The improvements of the tests allowed to write the removal of the tagged hosts from the namespace as specified here: https://tailscale.com/kb/1068/acl-tags/ --- acls.go | 187 ++++++++++-------- acls_test.go | 521 ++++++++++++++++++++++++++++++++++++++++++++++++++- machine.go | 13 ++ 3 files changed, 646 insertions(+), 75 deletions(-) diff --git a/acls.go b/acls.go index c86e315f..9dd1260d 100644 --- a/acls.go +++ b/acls.go @@ -2,7 +2,6 @@ package headscale import ( "encoding/json" - "errors" "fmt" "io" "os" @@ -86,6 +85,11 @@ func (h *Headscale) UpdateACLRules() error { func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { rules := []tailcfg.FilterRule{} + machines, err := h.ListAllMachines() + if err != nil { + return nil, err + } + for index, acl := range h.aclPolicy.ACLs { if acl.Action != "accept" { return nil, errInvalidAction @@ -93,7 +97,7 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { srcIPs := []string{} for innerIndex, user := range acl.Users { - srcs, err := h.generateACLPolicySrcIP(user) + srcs, err := h.generateACLPolicySrcIP(machines, *h.aclPolicy, user) if err != nil { log.Error(). Msgf("Error parsing ACL %d, User %d", index, innerIndex) @@ -105,7 +109,7 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { destPorts := []tailcfg.NetPortRange{} for innerIndex, ports := range acl.Ports { - dests, err := h.generateACLPolicyDestPorts(ports) + dests, err := h.generateACLPolicyDestPorts(machines, *h.aclPolicy, ports) if err != nil { log.Error(). Msgf("Error parsing ACL %d, Port %d", index, innerIndex) @@ -124,11 +128,13 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { return rules, nil } -func (h *Headscale) generateACLPolicySrcIP(u string) ([]string, error) { - return h.expandAlias(u) +func (h *Headscale) generateACLPolicySrcIP(machines []Machine, aclPolicy ACLPolicy, u string) ([]string, error) { + return expandAlias(machines, aclPolicy, u) } func (h *Headscale) generateACLPolicyDestPorts( + machines []Machine, + aclPolicy ACLPolicy, d string, ) ([]tailcfg.NetPortRange, error) { tokens := strings.Split(d, ":") @@ -149,11 +155,11 @@ func (h *Headscale) generateACLPolicyDestPorts( alias = fmt.Sprintf("%s:%s", tokens[0], tokens[1]) } - expanded, err := h.expandAlias(alias) + expanded, err := expandAlias(machines, aclPolicy, alias) if err != nil { return nil, err } - ports, err := h.expandPorts(tokens[len(tokens)-1]) + ports, err := expandPorts(tokens[len(tokens)-1]) if err != nil { return nil, err } @@ -177,52 +183,40 @@ func (h *Headscale) generateACLPolicyDestPorts( // - a group // - a tag // and transform these in IPAddresses -func (h *Headscale) expandAlias(alias string) ([]string, error) { +func expandAlias(machines []Machine, aclPolicy ACLPolicy, alias string) ([]string, error) { + ips := []string{} if alias == "*" { return []string{"*"}, nil } if strings.HasPrefix(alias, "group:") { - namespaces, err := h.expandGroup(alias) + namespaces, err := expandGroup(aclPolicy, alias) if err != nil { - return nil, err + return ips, err } - ips := []string{} for _, n := range namespaces { - nodes, err := h.ListMachinesInNamespace(n) - if err != nil { - return nil, errInvalidNamespace - } + nodes := listMachinesInNamespace(machines, n) for _, node := range nodes { ips = append(ips, node.IPAddresses.ToStringSlice()...) } } - return ips, nil } if strings.HasPrefix(alias, "tag:") { - var ips []string - owners, err := h.expandTagOwners(alias) + owners, err := expandTagOwners(aclPolicy, alias) if err != nil { - return nil, err + return ips, err } for _, namespace := range owners { - machines, err := h.ListMachinesInNamespace(namespace) - if err != nil { - if errors.Is(err, errNamespaceNotFound) { - continue - } else { - return nil, err - } - } + machines := listMachinesInNamespace(machines, namespace) for _, machine := range machines { if len(machine.HostInfo) == 0 { continue } hi, err := machine.GetHostInfo() if err != nil { - return nil, err + return ips, err } for _, t := range hi.RequestTags { if alias == t { @@ -234,75 +228,75 @@ func (h *Headscale) expandAlias(alias string) ([]string, error) { return ips, nil } - n, err := h.GetNamespace(alias) - if err == nil { - nodes, err := h.ListMachinesInNamespace(n.Name) - if err != nil { - return nil, err - } - ips := []string{} - for _, n := range nodes { - ips = append(ips, n.IPAddresses.ToStringSlice()...) - } - + // if alias is a namespace + nodes := listMachinesInNamespace(machines, alias) + nodes, err := excludeCorrectlyTaggedNodes(aclPolicy, nodes, alias) + if err != nil { + return ips, err + } + for _, n := range nodes { + ips = append(ips, n.IPAddresses.ToStringSlice()...) + } + if len(ips) > 0 { return ips, nil } - if h, ok := h.aclPolicy.Hosts[alias]; ok { + // if alias is an host + if h, ok := aclPolicy.Hosts[alias]; ok { return []string{h.String()}, nil } + // if alias is an IP ip, err := netaddr.ParseIP(alias) if err == nil { return []string{ip.String()}, nil } + // if alias is an CIDR cidr, err := netaddr.ParseIPPrefix(alias) if err == nil { return []string{cidr.String()}, nil } - return nil, errInvalidUserSection + return ips, errInvalidUserSection } -// expandTagOwners will return a list of namespace. An owner can be either a namespace or a group -// a group cannot be composed of groups -func (h *Headscale) expandTagOwners(owner string) ([]string, error) { - var owners []string - ows, ok := h.aclPolicy.TagOwners[owner] - if !ok { - return []string{}, fmt.Errorf("%w. %v isn't owned by a TagOwner. Please add one first. https://tailscale.com/kb/1018/acls/#tag-owners", errInvalidTag, owner) +// excludeCorrectlyTaggedNodes will remove from the list of input nodes the ones +// that are correctly tagged since they should not be listed as being in the namespace +// we assume in this function that we only have nodes from 1 namespace. +func excludeCorrectlyTaggedNodes(aclPolicy ACLPolicy, nodes []Machine, namespace string) ([]Machine, error) { + out := []Machine{} + tags := []string{} + for tag, ns := range aclPolicy.TagOwners { + if containsString(ns, namespace) { + tags = append(tags, tag) + } } - for _, ow := range ows { - if strings.HasPrefix(ow, "group:") { - gs, err := h.expandGroup(ow) - if err != nil { - return []string{}, err + // for each machine if tag is in tags list, don't append it. + for _, machine := range nodes { + if len(machine.HostInfo) == 0 { + out = append(out, machine) + continue + } + hi, err := machine.GetHostInfo() + if err != nil { + return out, err + } + found := false + for _, t := range hi.RequestTags { + if containsString(tags, t) { + found = true + break } - owners = append(owners, gs...) - } else { - owners = append(owners, ow) + } + if !found { + out = append(out, machine) } } - return owners, nil + return out, nil } -// expandGroup will return the list of namespace inside the group -// after some validation -func (h *Headscale) expandGroup(group string) ([]string, error) { - gs, ok := h.aclPolicy.Groups[group] - if !ok { - return []string{}, fmt.Errorf("group %v isn't registered. %w", group, errInvalidGroup) - } - for _, g := range gs { - if strings.HasPrefix(g, "group:") { - return []string{}, fmt.Errorf("%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", errInvalidGroup) - } - } - return gs, nil -} - -func (h *Headscale) expandPorts(portsStr string) (*[]tailcfg.PortRange, error) { +func expandPorts(portsStr string) (*[]tailcfg.PortRange, error) { if portsStr == "*" { return &[]tailcfg.PortRange{ {First: portRangeBegin, Last: portRangeEnd}, @@ -344,3 +338,50 @@ func (h *Headscale) expandPorts(portsStr string) (*[]tailcfg.PortRange, error) { return &ports, nil } + +func listMachinesInNamespace(machines []Machine, namespace string) []Machine { + out := []Machine{} + for _, machine := range machines { + if machine.Namespace.Name == namespace { + out = append(out, machine) + } + } + return out +} + +// expandTagOwners will return a list of namespace. An owner can be either a namespace or a group +// a group cannot be composed of groups +func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { + var owners []string + ows, ok := aclPolicy.TagOwners[tag] + if !ok { + return []string{}, fmt.Errorf("%w. %v isn't owned by a TagOwner. Please add one first. https://tailscale.com/kb/1018/acls/#tag-owners", errInvalidTag, tag) + } + for _, ow := range ows { + if strings.HasPrefix(ow, "group:") { + gs, err := expandGroup(aclPolicy, ow) + if err != nil { + return []string{}, err + } + owners = append(owners, gs...) + } else { + owners = append(owners, ow) + } + } + return owners, nil +} + +// expandGroup will return the list of namespace inside the group +// after some validation +func expandGroup(aclPolicy ACLPolicy, group string) ([]string, error) { + gs, ok := aclPolicy.Groups[group] + if !ok { + return []string{}, fmt.Errorf("group %v isn't registered. %w", group, errInvalidGroup) + } + for _, g := range gs { + if strings.HasPrefix(g, "group:") { + return []string{}, fmt.Errorf("%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", errInvalidGroup) + } + } + return gs, nil +} diff --git a/acls_test.go b/acls_test.go index f2fb6a03..8bedb47d 100644 --- a/acls_test.go +++ b/acls_test.go @@ -2,10 +2,13 @@ package headscale import ( "errors" + "reflect" + "testing" "gopkg.in/check.v1" "gorm.io/datatypes" "inet.af/netaddr" + "tailscale.com/tailcfg" ) func (s *Suite) TestWrongPath(c *check.C) { @@ -267,9 +270,16 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { } err = app.UpdateACLRules() c.Assert(err, check.IsNil) - c.Logf("Rules: %v", app.aclRules) c.Assert(app.aclRules, check.HasLen, 1) - c.Assert(app.aclRules[0].SrcIPs, check.HasLen, 0) + c.Assert(app.aclRules[0].SrcIPs, check.HasLen, 1) + c.Assert(app.aclRules[0].SrcIPs[0], check.Equals, "100.64.0.2") + c.Assert(app.aclRules[0].DstPorts, check.HasLen, 2) + c.Assert(app.aclRules[0].DstPorts[0].Ports.First, check.Equals, uint16(80)) + c.Assert(app.aclRules[0].DstPorts[0].Ports.Last, check.Equals, uint16(80)) + c.Assert(app.aclRules[0].DstPorts[0].IP, check.Equals, "100.64.0.1") + c.Assert(app.aclRules[0].DstPorts[1].Ports.First, check.Equals, uint16(443)) + c.Assert(app.aclRules[0].DstPorts[1].Ports.Last, check.Equals, uint16(443)) + c.Assert(app.aclRules[0].DstPorts[1].IP, check.Equals, "100.64.0.1") } func (s *Suite) TestPortRange(c *check.C) { @@ -385,3 +395,510 @@ func (s *Suite) TestPortGroup(c *check.C) { c.Assert(len(ips), check.Equals, 1) c.Assert(rules[0].SrcIPs[0], check.Equals, ips[0].String()) } + +func Test_expandGroup(t *testing.T) { + type args struct { + aclPolicy ACLPolicy + group string + } + tests := []struct { + name string + args args + want []string + wantErr bool + }{ + { + name: "simple test", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{"group:test": []string{"g1", "foo", "test"}, "group:foo": []string{"foo", "test"}}, + }, + group: "group:test", + }, + want: []string{"g1", "foo", "test"}, + wantErr: false, + }, + { + name: "InexistantGroup", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{"group:test": []string{"g1", "foo", "test"}, "group:foo": []string{"foo", "test"}}, + }, + group: "group:bar", + }, + want: []string{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := expandGroup(tt.args.aclPolicy, tt.args.group) + if (err != nil) != tt.wantErr { + t.Errorf("expandGroup() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("expandGroup() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_expandTagOwners(t *testing.T) { + type args struct { + aclPolicy ACLPolicy + tag string + } + tests := []struct { + name string + args args + want []string + wantErr bool + }{ + { + name: "simple tag", + args: args{ + aclPolicy: ACLPolicy{ + TagOwners: TagOwners{"tag:test": []string{"namespace1"}}, + }, + tag: "tag:test", + }, + want: []string{"namespace1"}, + wantErr: false, + }, + { + name: "tag and group", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{"group:foo": []string{"n1", "bar"}}, + TagOwners: TagOwners{"tag:test": []string{"group:foo"}}, + }, + tag: "tag:test", + }, + want: []string{"n1", "bar"}, + wantErr: false, + }, + { + name: "namespace and group", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{"group:foo": []string{"n1", "bar"}}, + TagOwners: TagOwners{"tag:test": []string{"group:foo", "home"}}, + }, + tag: "tag:test", + }, + want: []string{"n1", "bar", "home"}, + wantErr: false, + }, + { + name: "invalid tag", + args: args{ + aclPolicy: ACLPolicy{ + TagOwners: TagOwners{"tag:foo": []string{"group:foo", "home"}}, + }, + tag: "tag:test", + }, + want: []string{}, + wantErr: true, + }, + { + name: "invalid group", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{"group:bar": []string{"n1", "foo"}}, + TagOwners: TagOwners{"tag:test": []string{"group:foo", "home"}}, + }, + tag: "tag:test", + }, + want: []string{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := expandTagOwners(tt.args.aclPolicy, tt.args.tag) + if (err != nil) != tt.wantErr { + t.Errorf("expandTagOwners() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("expandTagOwners() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_expandPorts(t *testing.T) { + type args struct { + portsStr string + } + tests := []struct { + name string + args args + want *[]tailcfg.PortRange + wantErr bool + }{ + { + name: "wildcard", + args: args{portsStr: "*"}, + want: &[]tailcfg.PortRange{ + {First: portRangeBegin, Last: portRangeEnd}, + }, + wantErr: false, + }, + { + name: "two ports", + args: args{portsStr: "80,443"}, + want: &[]tailcfg.PortRange{ + {First: 80, Last: 80}, + {First: 443, Last: 443}, + }, + wantErr: false, + }, + { + name: "a range and a port", + args: args{portsStr: "80-1024,443"}, + want: &[]tailcfg.PortRange{ + {First: 80, Last: 1024}, + {First: 443, Last: 443}, + }, + wantErr: false, + }, + { + name: "out of bounds", + args: args{portsStr: "854038"}, + want: nil, + wantErr: true, + }, + { + name: "wrong port", + args: args{portsStr: "85a38"}, + want: nil, + wantErr: true, + }, + { + name: "wrong port in first", + args: args{portsStr: "a-80"}, + want: nil, + wantErr: true, + }, + { + name: "wrong port in last", + args: args{portsStr: "80-85a38"}, + want: nil, + wantErr: true, + }, + { + name: "wrong port format", + args: args{portsStr: "80-85a38-3"}, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := expandPorts(tt.args.portsStr) + if (err != nil) != tt.wantErr { + t.Errorf("expandPorts() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("expandPorts() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_listMachinesInNamespace(t *testing.T) { + type args struct { + machines []Machine + namespace string + } + tests := []struct { + name string + args args + want []Machine + }{ + { + name: "1 machine in namespace", + args: args{ + machines: []Machine{ + {Namespace: Namespace{Name: "test"}}, + }, + namespace: "test", + }, + want: []Machine{ + {Namespace: Namespace{Name: "test"}}, + }, + }, + { + name: "3 machines, 2 in namespace", + args: args{ + machines: []Machine{ + {ID: 1, Namespace: Namespace{Name: "test"}}, + {ID: 2, Namespace: Namespace{Name: "foo"}}, + {ID: 3, Namespace: Namespace{Name: "foo"}}, + }, + namespace: "foo", + }, + want: []Machine{ + {ID: 2, Namespace: Namespace{Name: "foo"}}, + {ID: 3, Namespace: Namespace{Name: "foo"}}, + }, + }, + { + name: "5 machines, 0 in namespace", + args: args{ + machines: []Machine{ + {ID: 1, Namespace: Namespace{Name: "test"}}, + {ID: 2, Namespace: Namespace{Name: "foo"}}, + {ID: 3, Namespace: Namespace{Name: "foo"}}, + {ID: 4, Namespace: Namespace{Name: "foo"}}, + {ID: 5, Namespace: Namespace{Name: "foo"}}, + }, + namespace: "bar", + }, + want: []Machine{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := listMachinesInNamespace(tt.args.machines, tt.args.namespace); !reflect.DeepEqual(got, tt.want) { + t.Errorf("listMachinesInNamespace() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_expandAlias(t *testing.T) { + type args struct { + machines []Machine + aclPolicy ACLPolicy + alias string + } + tests := []struct { + name string + args args + want []string + wantErr bool + }{ + { + name: "wildcard", + args: args{ + alias: "*", + machines: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.78.84.227")}}, + }, + aclPolicy: ACLPolicy{}, + }, + want: []string{"*"}, + wantErr: false, + }, + { + name: "simple group", + args: args{ + alias: "group:foo", + machines: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "test"}}, + }, + aclPolicy: ACLPolicy{ + Groups: Groups{"group:foo": []string{"foo", "bar"}}, + }, + }, + want: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, + wantErr: false, + }, + { + name: "wrong group", + args: args{ + alias: "group:test", + machines: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "test"}}, + }, + aclPolicy: ACLPolicy{ + Groups: Groups{"group:foo": []string{"foo", "bar"}}, + }, + }, + want: []string{}, + wantErr: true, + }, + { + name: "simple ipaddress", + args: args{ + alias: "10.0.0.3", + machines: []Machine{}, + aclPolicy: ACLPolicy{}, + }, + want: []string{"10.0.0.3"}, + wantErr: false, + }, + { + name: "private network", + args: args{ + alias: "homeNetwork", + machines: []Machine{}, + aclPolicy: ACLPolicy{ + Hosts: Hosts{"homeNetwork": netaddr.MustParseIPPrefix("192.168.1.0/24")}, + }, + }, + want: []string{"192.168.1.0/24"}, + wantErr: false, + }, + { + name: "simple host", + args: args{ + alias: "10.0.0.1", + machines: []Machine{}, + aclPolicy: ACLPolicy{}, + }, + want: []string{"10.0.0.1"}, + wantErr: false, + }, + { + name: "simple CIDR", + args: args{ + alias: "10.0.0.0/16", + machines: []Machine{}, + aclPolicy: ACLPolicy{}, + }, + want: []string{"10.0.0.0/16"}, + wantErr: false, + }, + { + name: "simple tag", + args: args{ + alias: "tag:test", + machines: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + }, + aclPolicy: ACLPolicy{ + TagOwners: TagOwners{"tag:test": []string{"foo"}}, + }, + }, + want: []string{"100.64.0.1", "100.64.0.2"}, + wantErr: false, + }, + { + name: "No tag defined", + args: args{ + alias: "tag:foo", + machines: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "test"}}, + }, + aclPolicy: ACLPolicy{ + Groups: Groups{"group:foo": []string{"foo", "bar"}}, + TagOwners: TagOwners{"tag:test": []string{"group:foo"}}, + }, + }, + want: []string{}, + wantErr: true, + }, + { + name: "list host in namespace without correctly tagged servers", + args: args{ + alias: "foo", + machines: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + }, + aclPolicy: ACLPolicy{ + TagOwners: TagOwners{"tag:test": []string{"foo"}}, + }, + }, + want: []string{"100.64.0.4"}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := expandAlias(tt.args.machines, tt.args.aclPolicy, tt.args.alias) + if (err != nil) != tt.wantErr { + t.Errorf("expandAlias() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("expandAlias() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_excludeCorrectlyTaggedNodes(t *testing.T) { + type args struct { + aclPolicy ACLPolicy + nodes []Machine + namespace string + } + tests := []struct { + name string + args args + want []Machine + wantErr bool + }{ + { + name: "exclude nodes with valid tags", + args: args{ + aclPolicy: ACLPolicy{ + TagOwners: TagOwners{"tag:test": []string{"foo"}}, + }, + nodes: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + }, + namespace: "foo", + }, + want: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + }, + wantErr: false, + }, + { + name: "all nodes have invalid tags, don't exclude them", + args: args{ + aclPolicy: ACLPolicy{ + TagOwners: TagOwners{"tag:foo": []string{"foo"}}, + }, + nodes: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + }, + namespace: "foo", + }, + want: []Machine{ + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, + {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := excludeCorrectlyTaggedNodes(tt.args.aclPolicy, tt.args.nodes, tt.args.namespace) + if (err != nil) != tt.wantErr { + t.Errorf("excludeCorrectlyTaggedNodes() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("excludeCorrectlyTaggedNodes() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/machine.go b/machine.go index 231c179d..01fc9276 100644 --- a/machine.go +++ b/machine.go @@ -119,6 +119,19 @@ func (machine Machine) isExpired() bool { return time.Now().UTC().After(*machine.Expiry) } +func (h *Headscale) ListAllMachines() ([]Machine, error) { + machines := []Machine{} + if err := h.db.Preload("AuthKey"). + Preload("AuthKey.Namespace"). + Preload("Namespace"). + Where("registered"). + Find(&machines).Error; err != nil { + return nil, err + } + + return machines, nil +} + func containsAddresses(inputs []string, addrs MachineAddresses) bool { for _, addr := range addrs.ToStringSlice() { if containsString(inputs, addr) { From 7b5ba9f78156b7ef1abe3abf9bc0d731396488b5 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 14 Feb 2022 13:54:44 +0100 Subject: [PATCH 07/31] docs(acl): add configuration example to explain acls --- docs/README.md | 8 ++++ docs/acls.md | 126 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 docs/acls.md diff --git a/docs/README.md b/docs/README.md index 1ac15221..4b5d0842 100644 --- a/docs/README.md +++ b/docs/README.md @@ -39,6 +39,14 @@ use namespaces (which are the equivalent to user/logins in Tailscale.com). Please check https://tailscale.com/kb/1018/acls/, and `./tests/acls/` in this repo for working examples. +When using ACL's the Namespace borders are no longer applied. All machines +whichever the Namespace have the ability to communicate with other hosts as +long as the ACL's permits this exchange. + +The [ACLs](acls.md) document should help understand a fictional case of setting +up ACLs in a small company. All concepts presented in this document could be +applied outside of business oriented usage. + ### Apple devices An endpoint with information on how to connect your Apple devices (currently macOS only) is available at `/apple` on your running instance. diff --git a/docs/acls.md b/docs/acls.md new file mode 100644 index 00000000..89e8b557 --- /dev/null +++ b/docs/acls.md @@ -0,0 +1,126 @@ + +# ACLs use case example + +Let's build an example use case for a small business (It may be the place where +ACL's are the most useful). + +We have a small company with a boss, an admin, two developers and an intern. + +The boss should have access to all servers but not to the users hosts. Admin +should also have access to all hosts except that their permissions should be +limited to maintaining the hosts (for example purposes). The developers can do +anything they want on dev hosts, but only watch on productions hosts. Intern +can only interact with the development servers. + +Each user have at least a device connected to the network and we have some +servers. + +- database.prod +- database.dev +- app-server1.prod +- app-server1.dev +- billing.internal + +## Setup of the network + +Let's create the namespaces. Each user should have his own namespace. The users +here are represented as namespaces. + +```bash +headscale namespaces create boss +headscale namespaces create admin1 +headscale namespaces create dev1 +headscale namespaces create dev2 +headscale namespaces create intern1 +``` + +We don't need to create namespaces for the servers because the servers will be +tagged. When registering the servers we will need to add the flag +`--advertised-tags=tag:,tag:`, and the user (namespace) that is +registering the server should be allowed to do it. Since anyone can add tags to +a server they can register, the check of the tags is done on headscale server +and only valid tags are applied. A tag is valid if the namespace that is +registering it is allowed to do it. + +Here are the ACL's to implement the same permissions as above: + +```json +{ + // groups are collections of users having a common scope. A user can be in multiple groups + // groups cannot be composed of groups + "groups": { + "group:boss": ["boss"], + "group:dev": ["dev1","dev2"], + "group:admin": ["admin1"], + "group:intern": ["intern1"], + }, + // tagOwners in tailscale is an association between a TAG and the people allowed to set this TAG on a server. + // This is documented [here](https://tailscale.com/kb/1068/acl-tags#defining-a-tag) + // and explained [here](https://tailscale.com/blog/rbac-like-it-was-meant-to-be/) + "tagOwners": { + // the administrators can add servers in production + "tag:prod-databases": ["group:admin"], + "tag:prod-app-servers": ["group:admin"], + + // the boss can tag any server as internal + "tag:internal": ["group:boss"], + + // dev can add servers for dev purposes as well as admins + "tag:dev-databases": ["group:admin","group:dev"], + "tag:dev-app-servers": ["group:admin", "group:dev"], + + // interns cannot add servers + }, + "acls": [ + // boss have access to all servers + {"action":"accept", + "users":["group:boss"], + "ports":[ + "tag:prod-databases:*", + "tag:prod-app-servers:*", + "tag:internal:*", + "tag:dev-databases:*", + "tag:dev-app-servers:*", + ] + }, + + // admin have only access to administrative ports of the servers + {"action":"accept", + "users":["group:admin"], + "ports":[ + "tag:prod-databases:22", + "tag:prod-app-servers:22", + "tag:internal:22", + "tag:dev-databases:22", + "tag:dev-app-servers:22", + ] + }, + + // developers have access to databases servers and application servers on all ports + // they can only view the applications servers in prod and have no access to databases servers in production + {"action":"accept", "users":["group:dev"], "ports":[ + "tag:dev-databases:*", + "tag:dev-app-servers:*", + "tag:prod-app-servers:80,443", + ] + }, + + // servers should be able to talk to database. Database should not be able to initiate connections to + // applications servers + {"action":"accept", "users":["tag:dev-app-servers"], "ports":["tag:dev-databases:5432"]}, + {"action":"accept", "users":["tag:prod-app-servers"], "ports":["tag:prod-databases:5432"]}, + + // interns have access to dev-app-servers only in reading mode + {"action":"accept", "users":["group:intern"], "ports":["tag:dev-app-servers:80,443"]}, + + // We still have to allow internal namespaces communications since nothing guarantees that each user have + // their own namespaces. + {"action":"accept", "users":["boss"], "ports":["boss:*"]}, + {"action":"accept", "users":["dev1"], "ports":["dev1:*"]}, + {"action":"accept", "users":["dev2"], "ports":["dev2:*"]}, + {"action":"accept", "users":["admin1"], "ports":["admin1:*"]}, + {"action":"accept", "users":["intern1"], "ports":["intern1:*"]}, + ] +} +``` + From aceaba60f174703eedefa2b9775e8c4026000988 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 14 Feb 2022 14:02:18 +0100 Subject: [PATCH 08/31] docs(changelog): bump changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e44161..16427017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ **TBD (TBD):** +**BREAKING**: +- ACLs have been rewritten and the behavior is different from before. It's now more aligned to tailscale's view of the feature. Namespaces are viewed as users and can communicate with each others. Tags should now work correctly and adding a host to Headscale should now reload the rules. The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features. + + **0.13.0 (2022-xx-xx):** **Features**: From 9cedbbafd46ad3034967a25f1901f8fe04ee4bf7 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 14 Feb 2022 15:26:54 +0100 Subject: [PATCH 09/31] chore(all): update some files for linter --- acls.go | 31 +++++++++------ acls_test.go | 101 +++++++++++++++++++++++++----------------------- api.go | 22 ++++++++++- dns.go | 4 +- machine.go | 13 ++++--- machine_test.go | 3 +- poll.go | 17 +++++++- 7 files changed, 118 insertions(+), 73 deletions(-) diff --git a/acls.go b/acls.go index 9dd1260d..3d6b1945 100644 --- a/acls.go +++ b/acls.go @@ -20,7 +20,6 @@ const ( errInvalidUserSection = Error("invalid user section") errInvalidGroup = Error("invalid group") errInvalidTag = Error("invalid tag") - errInvalidNamespace = Error("invalid namespace") errInvalidPortFormat = Error("invalid port format") ) @@ -69,6 +68,7 @@ func (h *Headscale) LoadACLPolicy(path string) error { } h.aclPolicy = &policy + return h.UpdateACLRules() } @@ -79,6 +79,7 @@ func (h *Headscale) UpdateACLRules() error { } log.Trace().Interface("ACL", rules).Msg("ACL rules generated") h.aclRules = rules + return nil } @@ -182,7 +183,7 @@ func (h *Headscale) generateACLPolicyDestPorts( // - a namespace // - a group // - a tag -// and transform these in IPAddresses +// and transform these in IPAddresses. func expandAlias(machines []Machine, aclPolicy ACLPolicy, alias string) ([]string, error) { ips := []string{} if alias == "*" { @@ -200,6 +201,7 @@ func expandAlias(machines []Machine, aclPolicy ACLPolicy, alias string) ([]strin ips = append(ips, node.IPAddresses.ToStringSlice()...) } } + return ips, nil } @@ -225,6 +227,7 @@ func expandAlias(machines []Machine, aclPolicy ACLPolicy, alias string) ([]strin } } } + return ips, nil } @@ -276,6 +279,7 @@ func excludeCorrectlyTaggedNodes(aclPolicy ACLPolicy, nodes []Machine, namespace for _, machine := range nodes { if len(machine.HostInfo) == 0 { out = append(out, machine) + continue } hi, err := machine.GetHostInfo() @@ -286,6 +290,7 @@ func excludeCorrectlyTaggedNodes(aclPolicy ACLPolicy, nodes []Machine, namespace for _, t := range hi.RequestTags { if containsString(tags, t) { found = true + break } } @@ -293,6 +298,7 @@ func excludeCorrectlyTaggedNodes(aclPolicy ACLPolicy, nodes []Machine, namespace out = append(out, machine) } } + return out, nil } @@ -346,42 +352,45 @@ func listMachinesInNamespace(machines []Machine, namespace string) []Machine { out = append(out, machine) } } + return out } // expandTagOwners will return a list of namespace. An owner can be either a namespace or a group -// a group cannot be composed of groups +// a group cannot be composed of groups. func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { var owners []string ows, ok := aclPolicy.TagOwners[tag] if !ok { return []string{}, fmt.Errorf("%w. %v isn't owned by a TagOwner. Please add one first. https://tailscale.com/kb/1018/acls/#tag-owners", errInvalidTag, tag) } - for _, ow := range ows { - if strings.HasPrefix(ow, "group:") { - gs, err := expandGroup(aclPolicy, ow) + for _, owner := range ows { + if strings.HasPrefix(owner, "group:") { + gs, err := expandGroup(aclPolicy, owner) if err != nil { return []string{}, err } owners = append(owners, gs...) } else { - owners = append(owners, ow) + owners = append(owners, owner) } } + return owners, nil } // expandGroup will return the list of namespace inside the group -// after some validation +// after some validation. func expandGroup(aclPolicy ACLPolicy, group string) ([]string, error) { - gs, ok := aclPolicy.Groups[group] + groups, ok := aclPolicy.Groups[group] if !ok { return []string{}, fmt.Errorf("group %v isn't registered. %w", group, errInvalidGroup) } - for _, g := range gs { + for _, g := range groups { if strings.HasPrefix(g, "group:") { return []string{}, fmt.Errorf("%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", errInvalidGroup) } } - return gs, nil + + return groups, nil } diff --git a/acls_test.go b/acls_test.go index 8bedb47d..786de0fc 100644 --- a/acls_test.go +++ b/acls_test.go @@ -94,7 +94,7 @@ func (s *Suite) TestInvalidTagOwners(c *check.C) { // this test should validate that we can expand a group in a TagOWner section and // match properly the IP's of the related hosts. The owner is valid and the tag is also valid. -// the tag is matched in the Users section +// the tag is matched in the Users section. func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { namespace, err := app.CreateNamespace("foo") c.Assert(err, check.IsNil) @@ -104,7 +104,7 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { _, err = app.GetMachine("foo", "testmachine") c.Assert(err, check.NotNil) - b := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") + hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") machine := Machine{ ID: 0, MachineKey: "foo", @@ -116,7 +116,7 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), - HostInfo: datatypes.JSON(b), + HostInfo: datatypes.JSON(hostInfo), } app.db.Save(&machine) @@ -136,7 +136,7 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { // this test should validate that we can expand a group in a TagOWner section and // match properly the IP's of the related hosts. The owner is valid and the tag is also valid. -// the tag is matched in the Ports section +// the tag is matched in the Ports section. func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { namespace, err := app.CreateNamespace("foo") c.Assert(err, check.IsNil) @@ -146,7 +146,7 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { _, err = app.GetMachine("foo", "testmachine") c.Assert(err, check.NotNil) - b := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") + hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") machine := Machine{ ID: 1, MachineKey: "foo", @@ -158,7 +158,7 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), - HostInfo: datatypes.JSON(b), + HostInfo: datatypes.JSON(hostInfo), } app.db.Save(&machine) @@ -178,7 +178,7 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { // need a test with: // tag on a host that isn't owned by a tag owners. So the namespace -// of the host should be valid +// of the host should be valid. func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { namespace, err := app.CreateNamespace("foo") c.Assert(err, check.IsNil) @@ -188,7 +188,7 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { _, err = app.GetMachine("foo", "testmachine") c.Assert(err, check.NotNil) - b := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:foo\"]}") + hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:foo\"]}") machine := Machine{ ID: 1, MachineKey: "foo", @@ -200,7 +200,7 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), - HostInfo: datatypes.JSON(b), + HostInfo: datatypes.JSON(hostInfo), } app.db.Save(&machine) @@ -229,7 +229,7 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { _, err = app.GetMachine("foo", "webserver") c.Assert(err, check.NotNil) - b := []byte("{\"OS\":\"centos\",\"Hostname\":\"webserver\",\"RequestTags\":[\"tag:webapp\"]}") + hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"webserver\",\"RequestTags\":[\"tag:webapp\"]}") machine := Machine{ ID: 1, MachineKey: "foo", @@ -241,11 +241,11 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), - HostInfo: datatypes.JSON(b), + HostInfo: datatypes.JSON(hostInfo), } app.db.Save(&machine) _, err = app.GetMachine("foo", "user") - b = []byte("{\"OS\":\"debian\",\"Hostname\":\"user\"}") + hostInfo = []byte("{\"OS\":\"debian\",\"Hostname\":\"user\"}") c.Assert(err, check.NotNil) machine = Machine{ ID: 2, @@ -258,7 +258,7 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), - HostInfo: datatypes.JSON(b), + HostInfo: datatypes.JSON(hostInfo), } app.db.Save(&machine) @@ -430,15 +430,16 @@ func Test_expandGroup(t *testing.T) { wantErr: true, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := expandGroup(tt.args.aclPolicy, tt.args.group) - if (err != nil) != tt.wantErr { - t.Errorf("expandGroup() error = %v, wantErr %v", err, tt.wantErr) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := expandGroup(test.args.aclPolicy, test.args.group) + if (err != nil) != test.wantErr { + t.Errorf("expandGroup() error = %v, wantErr %v", err, test.wantErr) + return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("expandGroup() = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, test.want) { + t.Errorf("expandGroup() = %v, want %v", got, test.want) } }) } @@ -514,15 +515,16 @@ func Test_expandTagOwners(t *testing.T) { wantErr: true, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := expandTagOwners(tt.args.aclPolicy, tt.args.tag) - if (err != nil) != tt.wantErr { - t.Errorf("expandTagOwners() error = %v, wantErr %v", err, tt.wantErr) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := expandTagOwners(test.args.aclPolicy, test.args.tag) + if (err != nil) != test.wantErr { + t.Errorf("expandTagOwners() error = %v, wantErr %v", err, test.wantErr) + return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("expandTagOwners() = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, test.want) { + t.Errorf("expandTagOwners() = %v, want %v", got, test.want) } }) } @@ -595,15 +597,16 @@ func Test_expandPorts(t *testing.T) { wantErr: true, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := expandPorts(tt.args.portsStr) - if (err != nil) != tt.wantErr { - t.Errorf("expandPorts() error = %v, wantErr %v", err, tt.wantErr) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := expandPorts(test.args.portsStr) + if (err != nil) != test.wantErr { + t.Errorf("expandPorts() error = %v, wantErr %v", err, test.wantErr) + return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("expandPorts() = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, test.want) { + t.Errorf("expandPorts() = %v, want %v", got, test.want) } }) } @@ -824,15 +827,16 @@ func Test_expandAlias(t *testing.T) { wantErr: false, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := expandAlias(tt.args.machines, tt.args.aclPolicy, tt.args.alias) - if (err != nil) != tt.wantErr { - t.Errorf("expandAlias() error = %v, wantErr %v", err, tt.wantErr) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := expandAlias(test.args.machines, test.args.aclPolicy, test.args.alias) + if (err != nil) != test.wantErr { + t.Errorf("expandAlias() error = %v, wantErr %v", err, test.wantErr) + return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("expandAlias() = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, test.want) { + t.Errorf("expandAlias() = %v, want %v", got, test.want) } }) } @@ -889,15 +893,16 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { wantErr: false, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := excludeCorrectlyTaggedNodes(tt.args.aclPolicy, tt.args.nodes, tt.args.namespace) - if (err != nil) != tt.wantErr { - t.Errorf("excludeCorrectlyTaggedNodes() error = %v, wantErr %v", err, tt.wantErr) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := excludeCorrectlyTaggedNodes(test.args.aclPolicy, test.args.nodes, test.args.namespace) + if (err != nil) != test.wantErr { + t.Errorf("excludeCorrectlyTaggedNodes() error = %v, wantErr %v", err, test.wantErr) + return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("excludeCorrectlyTaggedNodes() = %v, want %v", got, tt.want) + if !reflect.DeepEqual(got, test.want) { + t.Errorf("excludeCorrectlyTaggedNodes() = %v, want %v", got, test.want) } }) } diff --git a/api.go b/api.go index 020ded01..073be5e2 100644 --- a/api.go +++ b/api.go @@ -261,7 +261,16 @@ func (h *Headscale) getMapResponse( var respBody []byte if req.Compress == "zstd" { - src, _ := json.Marshal(resp) + src, err := json.Marshal(resp) + if err != nil { + log.Error(). + Caller(). + Str("func", "getMapResponse"). + Err(err). + Msg("Failed to marshal response for the client") + + return nil, err + } encoder, _ := zstd.NewWriter(nil) srcCompressed := encoder.EncodeAll(src, nil) @@ -290,7 +299,16 @@ func (h *Headscale) getMapKeepAliveResponse( var respBody []byte var err error if mapRequest.Compress == "zstd" { - src, _ := json.Marshal(mapResponse) + src, err := json.Marshal(mapResponse) + if err != nil { + log.Error(). + Caller(). + Str("func", "getMapKeepAliveResponse"). + Err(err). + Msg("Failed to marshal keepalive response for the client") + + return nil, err + } encoder, _ := zstd.NewWriter(nil) srcCompressed := encoder.EncodeAll(src, nil) respBody = h.privateKey.SealTo(machineKey, srcCompressed) diff --git a/dns.go b/dns.go index 37daa884..085a14e2 100644 --- a/dns.go +++ b/dns.go @@ -165,7 +165,7 @@ func getMapResponseDNSConfig( dnsConfig.Domains, fmt.Sprintf( "%s.%s", - strings.Replace(machine.Namespace.Name, "@", ".", -1), // Replace @ with . for valid domain for machine + strings.ReplaceAll(machine.Namespace.Name, "@", "."), // Replace @ with . for valid domain for machine baseDomain, ), ) @@ -176,7 +176,7 @@ func getMapResponseDNSConfig( namespaceSet.Add(p.Namespace) } for _, namespace := range namespaceSet.List() { - dnsRoute := fmt.Sprintf("%s.%s", namespace.(Namespace).Name, baseDomain) + var dnsRoute string = fmt.Sprintf("%v.%v", namespace.(Namespace).Name, baseDomain) dnsConfig.Routes[dnsRoute] = nil } } else { diff --git a/machine.go b/machine.go index 01fc9276..cb70bf12 100644 --- a/machine.go +++ b/machine.go @@ -138,6 +138,7 @@ func containsAddresses(inputs []string, addrs MachineAddresses) bool { return true } } + return false } @@ -174,20 +175,20 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { // In order to do this we would need to be able to identify that node A want to talk to node B but that Node B doesn't know // how to talk to node A and then add the peering resource. - for _, m := range machines { + for _, mchn := range machines { for _, rule := range h.aclRules { var dst []string for _, d := range rule.DstPorts { dst = append(dst, d.IP) } - if (containsAddresses(rule.SrcIPs, machine.IPAddresses) && (containsAddresses(dst, m.IPAddresses) || containsString(dst, "*"))) || - (containsAddresses(rule.SrcIPs, m.IPAddresses) && containsAddresses(dst, machine.IPAddresses)) { - mMachines[m.ID] = m + if (containsAddresses(rule.SrcIPs, machine.IPAddresses) && (containsAddresses(dst, mchn.IPAddresses) || containsString(dst, "*"))) || + (containsAddresses(rule.SrcIPs, mchn.IPAddresses) && containsAddresses(dst, machine.IPAddresses)) { + mMachines[mchn.ID] = mchn } } } - var authorizedMachines Machines + authorizedMachines := make([]Machine, 0, len(mMachines)) for _, m := range mMachines { authorizedMachines = append(authorizedMachines, m) } @@ -694,7 +695,7 @@ func (machine Machine) toNode( hostname = fmt.Sprintf( "%s.%s.%s", machine.Name, - strings.Replace(machine.Namespace.Name, "@", ".", -1), // Replace @ with . for valid domain for machine + strings.ReplaceAll(machine.Namespace.Name, "@", "."), // Replace @ with . for valid domain for machine baseDomain, ) } else { diff --git a/machine_test.go b/machine_test.go index 203289ac..df00067b 100644 --- a/machine_test.go +++ b/machine_test.go @@ -161,7 +161,7 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { key *PreAuthKey } - var stor []base + stor := make([]base, 0) for _, name := range []string{"test", "admin"} { namespace, err := app.CreateNamespace(name) @@ -169,7 +169,6 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) c.Assert(err, check.IsNil) stor = append(stor, base{namespace, pak}) - } _, err := app.GetMachineByID(0) diff --git a/poll.go b/poll.go index c2a51d11..f00f7484 100644 --- a/poll.go +++ b/poll.go @@ -85,7 +85,10 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { Str("machine", machine.Name). Msg("Found machine in database") - hostinfo, _ := json.Marshal(req.Hostinfo) + hostinfo, err := json.Marshal(req.Hostinfo) + if err != nil { + return + } machine.Name = req.Hostinfo.Hostname machine.HostInfo = datatypes.JSON(hostinfo) machine.DiscoKey = DiscoPublicKeyStripPrefix(req.DiscoKey) @@ -106,7 +109,17 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { // The intended use is for clients to discover the DERP map at start-up // before their first real endpoint update. if !req.ReadOnly { - endpoints, _ := json.Marshal(req.Endpoints) + endpoints, err := json.Marshal(req.Endpoints) + if err != nil { + log.Error(). + Caller(). + Str("func", "PollNetMapHandler"). + Err(err). + Msg("Failed to mashal requested endpoints for the client") + ctx.String(http.StatusInternalServerError, ":(") + + return + } machine.Endpoints = datatypes.JSON(endpoints) machine.LastSeen = &now } From d8c4c3163b6fe9bba05fa2dd2566910ebf749c31 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 14 Feb 2022 15:54:51 +0100 Subject: [PATCH 10/31] chore(fmt): apply make fmt command --- CHANGELOG.md | 2 +- acls.go | 35 +++++- acls_test.go | 302 +++++++++++++++++++++++++++++++++++++++++------- dns.go | 6 +- docs/acls.md | 155 ++++++++++++++----------- machine.go | 11 +- machine_test.go | 12 +- poll.go | 6 +- 8 files changed, 399 insertions(+), 130 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16427017..62808b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,8 @@ **TBD (TBD):** **BREAKING**: -- ACLs have been rewritten and the behavior is different from before. It's now more aligned to tailscale's view of the feature. Namespaces are viewed as users and can communicate with each others. Tags should now work correctly and adding a host to Headscale should now reload the rules. The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features. +- ACLs have been rewritten and the behavior is different from before. It's now more aligned to tailscale's view of the feature. Namespaces are viewed as users and can communicate with each others. Tags should now work correctly and adding a host to Headscale should now reload the rules. The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features. **0.13.0 (2022-xx-xx):** diff --git a/acls.go b/acls.go index 3d6b1945..d5d39889 100644 --- a/acls.go +++ b/acls.go @@ -129,7 +129,11 @@ func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { return rules, nil } -func (h *Headscale) generateACLPolicySrcIP(machines []Machine, aclPolicy ACLPolicy, u string) ([]string, error) { +func (h *Headscale) generateACLPolicySrcIP( + machines []Machine, + aclPolicy ACLPolicy, + u string, +) ([]string, error) { return expandAlias(machines, aclPolicy, u) } @@ -184,7 +188,11 @@ func (h *Headscale) generateACLPolicyDestPorts( // - a group // - a tag // and transform these in IPAddresses. -func expandAlias(machines []Machine, aclPolicy ACLPolicy, alias string) ([]string, error) { +func expandAlias( + machines []Machine, + aclPolicy ACLPolicy, + alias string, +) ([]string, error) { ips := []string{} if alias == "*" { return []string{"*"}, nil @@ -267,7 +275,11 @@ func expandAlias(machines []Machine, aclPolicy ACLPolicy, alias string) ([]strin // excludeCorrectlyTaggedNodes will remove from the list of input nodes the ones // that are correctly tagged since they should not be listed as being in the namespace // we assume in this function that we only have nodes from 1 namespace. -func excludeCorrectlyTaggedNodes(aclPolicy ACLPolicy, nodes []Machine, namespace string) ([]Machine, error) { +func excludeCorrectlyTaggedNodes( + aclPolicy ACLPolicy, + nodes []Machine, + namespace string, +) ([]Machine, error) { out := []Machine{} tags := []string{} for tag, ns := range aclPolicy.TagOwners { @@ -362,7 +374,11 @@ func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { var owners []string ows, ok := aclPolicy.TagOwners[tag] if !ok { - return []string{}, fmt.Errorf("%w. %v isn't owned by a TagOwner. Please add one first. https://tailscale.com/kb/1018/acls/#tag-owners", errInvalidTag, tag) + return []string{}, fmt.Errorf( + "%w. %v isn't owned by a TagOwner. Please add one first. https://tailscale.com/kb/1018/acls/#tag-owners", + errInvalidTag, + tag, + ) } for _, owner := range ows { if strings.HasPrefix(owner, "group:") { @@ -384,11 +400,18 @@ func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { func expandGroup(aclPolicy ACLPolicy, group string) ([]string, error) { groups, ok := aclPolicy.Groups[group] if !ok { - return []string{}, fmt.Errorf("group %v isn't registered. %w", group, errInvalidGroup) + return []string{}, fmt.Errorf( + "group %v isn't registered. %w", + group, + errInvalidGroup, + ) } for _, g := range groups { if strings.HasPrefix(g, "group:") { - return []string{}, fmt.Errorf("%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", errInvalidGroup) + return []string{}, fmt.Errorf( + "%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", + errInvalidGroup, + ) } } diff --git a/acls_test.go b/acls_test.go index 786de0fc..f8407d42 100644 --- a/acls_test.go +++ b/acls_test.go @@ -72,7 +72,10 @@ func (s *Suite) TestInvalidAction(c *check.C) { func (s *Suite) TestInvalidGroupInGroup(c *check.C) { // this ACL is wrong because the group in users sections doesn't exist app.aclPolicy = &ACLPolicy{ - Groups: Groups{"group:test": []string{"foo"}, "group:error": []string{"foo", "group:test"}}, + Groups: Groups{ + "group:test": []string{"foo"}, + "group:error": []string{"foo", "group:test"}, + }, ACLs: []ACL{ {Action: "accept", Users: []string{"group:error"}, Ports: []string{"*:*"}}, }, @@ -104,7 +107,9 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { _, err = app.GetMachine("foo", "testmachine") c.Assert(err, check.NotNil) - hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") + hostInfo := []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ) machine := Machine{ ID: 0, MachineKey: "foo", @@ -146,7 +151,9 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { _, err = app.GetMachine("foo", "testmachine") c.Assert(err, check.NotNil) - hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}") + hostInfo := []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ) machine := Machine{ ID: 1, MachineKey: "foo", @@ -188,7 +195,9 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { _, err = app.GetMachine("foo", "testmachine") c.Assert(err, check.NotNil) - hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:foo\"]}") + hostInfo := []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:foo\"]}", + ) machine := Machine{ ID: 1, MachineKey: "foo", @@ -229,7 +238,9 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { _, err = app.GetMachine("foo", "webserver") c.Assert(err, check.NotNil) - hostInfo := []byte("{\"OS\":\"centos\",\"Hostname\":\"webserver\",\"RequestTags\":[\"tag:webapp\"]}") + hostInfo := []byte( + "{\"OS\":\"centos\",\"Hostname\":\"webserver\",\"RequestTags\":[\"tag:webapp\"]}", + ) machine := Machine{ ID: 1, MachineKey: "foo", @@ -265,7 +276,11 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { app.aclPolicy = &ACLPolicy{ TagOwners: TagOwners{"tag:webapp": []string{"foo"}}, ACLs: []ACL{ - {Action: "accept", Users: []string{"foo"}, Ports: []string{"tag:webapp:80,443"}}, + { + Action: "accept", + Users: []string{"foo"}, + Ports: []string{"tag:webapp:80,443"}, + }, }, } err = app.UpdateACLRules() @@ -411,7 +426,10 @@ func Test_expandGroup(t *testing.T) { name: "simple test", args: args{ aclPolicy: ACLPolicy{ - Groups: Groups{"group:test": []string{"g1", "foo", "test"}, "group:foo": []string{"foo", "test"}}, + Groups: Groups{ + "group:test": []string{"g1", "foo", "test"}, + "group:foo": []string{"foo", "test"}, + }, }, group: "group:test", }, @@ -422,7 +440,10 @@ func Test_expandGroup(t *testing.T) { name: "InexistantGroup", args: args{ aclPolicy: ACLPolicy{ - Groups: Groups{"group:test": []string{"g1", "foo", "test"}, "group:foo": []string{"foo", "test"}}, + Groups: Groups{ + "group:test": []string{"g1", "foo", "test"}, + "group:foo": []string{"foo", "test"}, + }, }, group: "group:bar", }, @@ -666,7 +687,10 @@ func Test_listMachinesInNamespace(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := listMachinesInNamespace(tt.args.machines, tt.args.namespace); !reflect.DeepEqual(got, tt.want) { + if got := listMachinesInNamespace(tt.args.machines, tt.args.namespace); !reflect.DeepEqual( + got, + tt.want, + ) { t.Errorf("listMachinesInNamespace() = %v, want %v", got, tt.want) } }) @@ -691,7 +715,11 @@ func Test_expandAlias(t *testing.T) { alias: "*", machines: []Machine{ {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.78.84.227")}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.78.84.227"), + }, + }, }, aclPolicy: ACLPolicy{}, }, @@ -703,10 +731,30 @@ func Test_expandAlias(t *testing.T) { args: args{ alias: "group:foo", machines: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "test"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "bar"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "test"}, + }, }, aclPolicy: ACLPolicy{ Groups: Groups{"group:foo": []string{"foo", "bar"}}, @@ -720,10 +768,30 @@ func Test_expandAlias(t *testing.T) { args: args{ alias: "group:test", machines: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "test"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "bar"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "test"}, + }, }, aclPolicy: ACLPolicy{ Groups: Groups{"group:foo": []string{"foo", "bar"}}, @@ -748,7 +816,9 @@ func Test_expandAlias(t *testing.T) { alias: "homeNetwork", machines: []Machine{}, aclPolicy: ACLPolicy{ - Hosts: Hosts{"homeNetwork": netaddr.MustParseIPPrefix("192.168.1.0/24")}, + Hosts: Hosts{ + "homeNetwork": netaddr.MustParseIPPrefix("192.168.1.0/24"), + }, }, }, want: []string{"192.168.1.0/24"}, @@ -779,10 +849,36 @@ func Test_expandAlias(t *testing.T) { args: args{ alias: "tag:test", machines: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "bar"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "foo"}, + }, }, aclPolicy: ACLPolicy{ TagOwners: TagOwners{"tag:test": []string{"foo"}}, @@ -796,10 +892,30 @@ func Test_expandAlias(t *testing.T) { args: args{ alias: "tag:foo", machines: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "test"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "bar"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "test"}, + }, }, aclPolicy: ACLPolicy{ Groups: Groups{"group:foo": []string{"foo", "bar"}}, @@ -814,10 +930,36 @@ func Test_expandAlias(t *testing.T) { args: args{ alias: "foo", machines: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, Namespace: Namespace{Name: "bar"}}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "bar"}, + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "foo"}, + }, }, aclPolicy: ACLPolicy{ TagOwners: TagOwners{"tag:test": []string{"foo"}}, @@ -829,7 +971,11 @@ func Test_expandAlias(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, err := expandAlias(test.args.machines, test.args.aclPolicy, test.args.alias) + got, err := expandAlias( + test.args.machines, + test.args.aclPolicy, + test.args.alias, + ) if (err != nil) != test.wantErr { t.Errorf("expandAlias() error = %v, wantErr %v", err, test.wantErr) @@ -861,14 +1007,38 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { TagOwners: TagOwners{"tag:test": []string{"foo"}}, }, nodes: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "foo"}, + }, }, namespace: "foo", }, want: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + { + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, + Namespace: Namespace{Name: "foo"}, + }, }, wantErr: false, }, @@ -879,25 +1049,69 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { TagOwners: TagOwners{"tag:foo": []string{"foo"}}, }, nodes: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "foo"}, + }, }, namespace: "foo", }, want: []Machine{ - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "foo"}, HostInfo: []byte("{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}")}, - {IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, Namespace: Namespace{Name: "foo"}}, + { + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "foo"}, + HostInfo: []byte( + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + ), + }, + { + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, + Namespace: Namespace{Name: "foo"}, + }, }, wantErr: false, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, err := excludeCorrectlyTaggedNodes(test.args.aclPolicy, test.args.nodes, test.args.namespace) + got, err := excludeCorrectlyTaggedNodes( + test.args.aclPolicy, + test.args.nodes, + test.args.namespace, + ) if (err != nil) != test.wantErr { - t.Errorf("excludeCorrectlyTaggedNodes() error = %v, wantErr %v", err, test.wantErr) + t.Errorf( + "excludeCorrectlyTaggedNodes() error = %v, wantErr %v", + err, + test.wantErr, + ) return } diff --git a/dns.go b/dns.go index 085a14e2..be6238f8 100644 --- a/dns.go +++ b/dns.go @@ -165,7 +165,11 @@ func getMapResponseDNSConfig( dnsConfig.Domains, fmt.Sprintf( "%s.%s", - strings.ReplaceAll(machine.Namespace.Name, "@", "."), // Replace @ with . for valid domain for machine + strings.ReplaceAll( + machine.Namespace.Name, + "@", + ".", + ), // Replace @ with . for valid domain for machine baseDomain, ), ) diff --git a/docs/acls.md b/docs/acls.md index 89e8b557..63e7c6b8 100644 --- a/docs/acls.md +++ b/docs/acls.md @@ -1,4 +1,3 @@ - # ACLs use case example Let's build an example use case for a small business (It may be the place where @@ -46,81 +45,97 @@ Here are the ACL's to implement the same permissions as above: ```json { - // groups are collections of users having a common scope. A user can be in multiple groups - // groups cannot be composed of groups - "groups": { - "group:boss": ["boss"], - "group:dev": ["dev1","dev2"], - "group:admin": ["admin1"], - "group:intern": ["intern1"], + // groups are collections of users having a common scope. A user can be in multiple groups + // groups cannot be composed of groups + "groups": { + "group:boss": ["boss"], + "group:dev": ["dev1", "dev2"], + "group:admin": ["admin1"], + "group:intern": ["intern1"] + }, + // tagOwners in tailscale is an association between a TAG and the people allowed to set this TAG on a server. + // This is documented [here](https://tailscale.com/kb/1068/acl-tags#defining-a-tag) + // and explained [here](https://tailscale.com/blog/rbac-like-it-was-meant-to-be/) + "tagOwners": { + // the administrators can add servers in production + "tag:prod-databases": ["group:admin"], + "tag:prod-app-servers": ["group:admin"], + + // the boss can tag any server as internal + "tag:internal": ["group:boss"], + + // dev can add servers for dev purposes as well as admins + "tag:dev-databases": ["group:admin", "group:dev"], + "tag:dev-app-servers": ["group:admin", "group:dev"] + + // interns cannot add servers + }, + "acls": [ + // boss have access to all servers + { + "action": "accept", + "users": ["group:boss"], + "ports": [ + "tag:prod-databases:*", + "tag:prod-app-servers:*", + "tag:internal:*", + "tag:dev-databases:*", + "tag:dev-app-servers:*" + ] }, - // tagOwners in tailscale is an association between a TAG and the people allowed to set this TAG on a server. - // This is documented [here](https://tailscale.com/kb/1068/acl-tags#defining-a-tag) - // and explained [here](https://tailscale.com/blog/rbac-like-it-was-meant-to-be/) - "tagOwners": { - // the administrators can add servers in production - "tag:prod-databases": ["group:admin"], - "tag:prod-app-servers": ["group:admin"], - // the boss can tag any server as internal - "tag:internal": ["group:boss"], - - // dev can add servers for dev purposes as well as admins - "tag:dev-databases": ["group:admin","group:dev"], - "tag:dev-app-servers": ["group:admin", "group:dev"], - - // interns cannot add servers + // admin have only access to administrative ports of the servers + { + "action": "accept", + "users": ["group:admin"], + "ports": [ + "tag:prod-databases:22", + "tag:prod-app-servers:22", + "tag:internal:22", + "tag:dev-databases:22", + "tag:dev-app-servers:22" + ] }, - "acls": [ - // boss have access to all servers - {"action":"accept", - "users":["group:boss"], - "ports":[ - "tag:prod-databases:*", - "tag:prod-app-servers:*", - "tag:internal:*", - "tag:dev-databases:*", - "tag:dev-app-servers:*", - ] - }, - // admin have only access to administrative ports of the servers - {"action":"accept", - "users":["group:admin"], - "ports":[ - "tag:prod-databases:22", - "tag:prod-app-servers:22", - "tag:internal:22", - "tag:dev-databases:22", - "tag:dev-app-servers:22", - ] - }, + // developers have access to databases servers and application servers on all ports + // they can only view the applications servers in prod and have no access to databases servers in production + { + "action": "accept", + "users": ["group:dev"], + "ports": [ + "tag:dev-databases:*", + "tag:dev-app-servers:*", + "tag:prod-app-servers:80,443" + ] + }, - // developers have access to databases servers and application servers on all ports - // they can only view the applications servers in prod and have no access to databases servers in production - {"action":"accept", "users":["group:dev"], "ports":[ - "tag:dev-databases:*", - "tag:dev-app-servers:*", - "tag:prod-app-servers:80,443", - ] - }, + // servers should be able to talk to database. Database should not be able to initiate connections to + // applications servers + { + "action": "accept", + "users": ["tag:dev-app-servers"], + "ports": ["tag:dev-databases:5432"] + }, + { + "action": "accept", + "users": ["tag:prod-app-servers"], + "ports": ["tag:prod-databases:5432"] + }, - // servers should be able to talk to database. Database should not be able to initiate connections to - // applications servers - {"action":"accept", "users":["tag:dev-app-servers"], "ports":["tag:dev-databases:5432"]}, - {"action":"accept", "users":["tag:prod-app-servers"], "ports":["tag:prod-databases:5432"]}, + // interns have access to dev-app-servers only in reading mode + { + "action": "accept", + "users": ["group:intern"], + "ports": ["tag:dev-app-servers:80,443"] + }, - // interns have access to dev-app-servers only in reading mode - {"action":"accept", "users":["group:intern"], "ports":["tag:dev-app-servers:80,443"]}, - - // We still have to allow internal namespaces communications since nothing guarantees that each user have - // their own namespaces. - {"action":"accept", "users":["boss"], "ports":["boss:*"]}, - {"action":"accept", "users":["dev1"], "ports":["dev1:*"]}, - {"action":"accept", "users":["dev2"], "ports":["dev2:*"]}, - {"action":"accept", "users":["admin1"], "ports":["admin1:*"]}, - {"action":"accept", "users":["intern1"], "ports":["intern1:*"]}, - ] + // We still have to allow internal namespaces communications since nothing guarantees that each user have + // their own namespaces. + { "action": "accept", "users": ["boss"], "ports": ["boss:*"] }, + { "action": "accept", "users": ["dev1"], "ports": ["dev1:*"] }, + { "action": "accept", "users": ["dev2"], "ports": ["dev2:*"] }, + { "action": "accept", "users": ["admin1"], "ports": ["admin1:*"] }, + { "action": "accept", "users": ["intern1"], "ports": ["intern1:*"] } + ] } ``` - diff --git a/machine.go b/machine.go index cb70bf12..ba677f15 100644 --- a/machine.go +++ b/machine.go @@ -192,7 +192,10 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { for _, m := range mMachines { authorizedMachines = append(authorizedMachines, m) } - sort.Slice(authorizedMachines, func(i, j int) bool { return authorizedMachines[i].ID < authorizedMachines[j].ID }) + sort.Slice( + authorizedMachines, + func(i, j int) bool { return authorizedMachines[i].ID < authorizedMachines[j].ID }, + ) log.Trace(). Caller(). @@ -695,7 +698,11 @@ func (machine Machine) toNode( hostname = fmt.Sprintf( "%s.%s.%s", machine.Name, - strings.ReplaceAll(machine.Namespace.Name, "@", "."), // Replace @ with . for valid domain for machine + strings.ReplaceAll( + machine.Namespace.Name, + "@", + ".", + ), // Replace @ with . for valid domain for machine baseDomain, ) } else { diff --git a/machine_test.go b/machine_test.go index df00067b..f9e58be9 100644 --- a/machine_test.go +++ b/machine_test.go @@ -176,11 +176,13 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { for index := 0; index <= 10; index++ { machine := Machine{ - ID: uint64(index), - MachineKey: "foo" + strconv.Itoa(index), - NodeKey: "bar" + strconv.Itoa(index), - DiscoKey: "faa" + strconv.Itoa(index), - IPAddresses: MachineAddresses{netaddr.MustParseIP(fmt.Sprintf("100.64.0.%v", strconv.Itoa(index+1)))}, + ID: uint64(index), + MachineKey: "foo" + strconv.Itoa(index), + NodeKey: "bar" + strconv.Itoa(index), + DiscoKey: "faa" + strconv.Itoa(index), + IPAddresses: MachineAddresses{ + netaddr.MustParseIP(fmt.Sprintf("100.64.0.%v", strconv.Itoa(index+1))), + }, Name: "testmachine" + strconv.Itoa(index), NamespaceID: stor[index%2].namespace.ID, Registered: true, diff --git a/poll.go b/poll.go index f00f7484..96db43f2 100644 --- a/poll.go +++ b/poll.go @@ -97,7 +97,11 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { // update ACLRules with peer informations (to update server tags if necessary) err = h.UpdateACLRules() if err != nil { - log.Error().Caller().Str("func", "handleAuthKey").Str("machine", machine.Name).Err(err) + log.Error(). + Caller(). + Str("func", "handleAuthKey"). + Str("machine", machine.Name). + Err(err) } // From Tailscale client: From 5f642eef76e497c03e0409aefa691d7602822bf3 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 14 Feb 2022 18:24:37 +0100 Subject: [PATCH 11/31] chore(lint): more lint fixing --- acls_test.go | 10 +++++----- dns.go | 10 ++++++++-- utils.go | 1 + 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/acls_test.go b/acls_test.go index f8407d42..2bd5bf96 100644 --- a/acls_test.go +++ b/acls_test.go @@ -685,13 +685,13 @@ func Test_listMachinesInNamespace(t *testing.T) { want: []Machine{}, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := listMachinesInNamespace(tt.args.machines, tt.args.namespace); !reflect.DeepEqual( + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := listMachinesInNamespace(test.args.machines, test.args.namespace); !reflect.DeepEqual( got, - tt.want, + test.want, ) { - t.Errorf("listMachinesInNamespace() = %v, want %v", got, tt.want) + t.Errorf("listMachinesInNamespace() = %v, want %v", got, test.want) } }) } diff --git a/dns.go b/dns.go index be6238f8..45e0faeb 100644 --- a/dns.go +++ b/dns.go @@ -179,8 +179,14 @@ func getMapResponseDNSConfig( for _, p := range peers { namespaceSet.Add(p.Namespace) } - for _, namespace := range namespaceSet.List() { - var dnsRoute string = fmt.Sprintf("%v.%v", namespace.(Namespace).Name, baseDomain) + for _, ns := range namespaceSet.List() { + namespace, ok := ns.(Namespace) + if !ok { + dnsConfig = dnsConfigOrig + + continue + } + dnsRoute := fmt.Sprintf("%v.%v", namespace.Name, baseDomain) dnsConfig.Routes[dnsRoute] = nil } } else { diff --git a/utils.go b/utils.go index 794971a1..3cee5e35 100644 --- a/utils.go +++ b/utils.go @@ -218,6 +218,7 @@ func containsString(ss []string, s string) bool { return true } } + return false } From f073d8f43c0334db46dccdb66083e7c3a07925a5 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 14 Feb 2022 18:32:20 +0100 Subject: [PATCH 12/31] chore(lint): ignore linting on test_expandalias This is a false positive on the way the function is built. Small tests cases are all inside this functions, making it big. --- acls_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/acls_test.go b/acls_test.go index 2bd5bf96..e68a01f6 100644 --- a/acls_test.go +++ b/acls_test.go @@ -697,6 +697,7 @@ func Test_listMachinesInNamespace(t *testing.T) { } } +// nolint func Test_expandAlias(t *testing.T) { type args struct { machines []Machine From 8798efd3536bb7ee7883de909f69c12b3715f06a Mon Sep 17 00:00:00 2001 From: ohdearaugustin Date: Sat, 19 Feb 2022 21:04:01 +0100 Subject: [PATCH 13/31] contributor: set specific version --- .github/workflows/contributors.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/contributors.yml b/.github/workflows/contributors.yml index e27b6eda..4a73009b 100644 --- a/.github/workflows/contributors.yml +++ b/.github/workflows/contributors.yml @@ -4,13 +4,13 @@ on: push: branches: - main - + workflow_dispatch: jobs: add-contributors: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: BobAnkh/add-contributors@master + - uses: BobAnkh/add-contributors@v0.2.2 with: CONTRIBUTOR: "## Contributors" COLUMN_PER_ROW: "6" From 5245f1accc5fd7f85ecbe6f4e59599659f3b72d8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 19 Feb 2022 22:48:26 +0000 Subject: [PATCH 14/31] docs(README): update contributors --- README.md | 181 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 168 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 1d86380e..e7ba6649 100644 --- a/README.md +++ b/README.md @@ -122,13 +122,6 @@ make build - + + + + + - - + + + + + + + + + + + + + - - + + + + + + + + + + - - + + + + + + + +
- - Juan -
- Juan Font - - 		Kristoffer @@ -136,6 +129,13 @@ make build Kristoffer Dalby + + Juan +
+ Juan Font + + 		Ward @@ -150,6 +150,13 @@ make build ohdearaugustin + + Alessandro +
+ Alessandro (Ale) Segala + + 		unreality/ @@ -157,6 +164,15 @@ make build unreality
+ + Eugen +
+ Eugen Biegler + + 		Aaron @@ -164,8 +180,27 @@ make build Aaron Bieber
+ + Fernando +
+ Fernando De Lucchi + + 		+ + Hoàng +
+ Hoàng Đức Hiếu + + 		+ + Michael +
+ Michael G. + + 		Paul @@ -173,6 +208,8 @@ make build Paul Tötterman
Casey @@ -187,6 +224,20 @@ make build Silver Bullet + + Stefan +
+ Stefan Majer + + 		+ + lachy-2849/ +
+ lachy-2849 + + 		thomas/ @@ -194,6 +245,29 @@ make build thomas + + Abraham +
+ Abraham Ingersoll + +
+ + Adrien +
+ Adrien Raffin-Caboisse + + 		+ + Artem +
+ Artem Klevtsov + + 		Arthur @@ -201,6 +275,13 @@ make build Arthur Woimbée + + Bryan +
+ Bryan Stenson + + 		Felix @@ -208,8 +289,6 @@ make build Felix Kronlage-Dammers
Felix @@ -217,6 +296,43 @@ make build Felix Yan
+ + JJGadgets/ +
+ JJGadgets + + 		+ + Jim +
+ Jim Tittsler + + 		+ + Pierre +
+ Pierre Carru + + 		+ + rcursaru/ +
+ rcursaru + + 		+ + Ryan +
+ Ryan Fowler + + 		Shaanan @@ -224,6 +340,15 @@ make build Shaanan Cohney
+ + Tanner/ +
+ Tanner + + 		Teteros/ @@ -252,8 +377,6 @@ make build Tjerk Woudsma
Zakhar @@ -261,6 +384,15 @@ make build Zakhar Bessarab
+ + ZiYuan/ +
+ ZiYuan + + 		derelm/ @@ -268,6 +400,13 @@ make build derelm + + e-zk/ +
+ e-zk + + 		ignoramous/ @@ -275,6 +414,22 @@ make build ignoramous + + lion24/ +
+ lion24 + + 		+ + Wakeful-Cloud/ +
+ Wakeful-Cloud + +
zy/ From 4f9ece14c56db2b6baa0767a546b19506b90d832 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Sun, 20 Feb 2022 20:47:12 +0100 Subject: [PATCH 15/31] Apply suggestions from code review on changelog Co-authored-by: Kristoffer Dalby --- CHANGELOG.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62808b84..cd582afd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,24 @@ **TBD (TBD):** +**0.14.0 (2022-xx-xx):** + +**UPCOMING BREAKING**: +From the **next** version (`0.15.0`), all machines will be able to communicate regardless of +if they are in the same namespace. This means that the behaviour currently limited to ACLs +will become default. From version `0.15.0`, all limitation of communications must be done +with ACLs. + +This is a part of aligning `headscale`'s behaviour with Tailscale's upstream behaviour. + **BREAKING**: -- ACLs have been rewritten and the behavior is different from before. It's now more aligned to tailscale's view of the feature. Namespaces are viewed as users and can communicate with each others. Tags should now work correctly and adding a host to Headscale should now reload the rules. The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features. +- ACLs have been rewritten to align with the bevaviour Tailscale Control Panel provides. **NOTE:** This is only active if you use ACLs + - Namespaces are now treated as Users + - All machines can communicate with all machines by default + - Tags should now work correctly and adding a host to Headscale should now reload the rules. + - The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features + **0.13.0 (2022-xx-xx):** From d00251c63e3eb8c93c0ea21acda1040af5971331 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Sun, 20 Feb 2022 21:24:02 +0100 Subject: [PATCH 16/31] fix(acls,machines): apply code review suggestions --- acls.go | 8 ++++---- acls_test.go | 2 +- machine.go | 37 ++++++++++++++++++++----------------- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/acls.go b/acls.go index d5d39889..f9ed09da 100644 --- a/acls.go +++ b/acls.go @@ -204,7 +204,7 @@ func expandAlias( return ips, err } for _, n := range namespaces { - nodes := listMachinesInNamespace(machines, n) + nodes := filterMachinesByNamespace(machines, n) for _, node := range nodes { ips = append(ips, node.IPAddresses.ToStringSlice()...) } @@ -219,7 +219,7 @@ func expandAlias( return ips, err } for _, namespace := range owners { - machines := listMachinesInNamespace(machines, namespace) + machines := filterMachinesByNamespace(machines, namespace) for _, machine := range machines { if len(machine.HostInfo) == 0 { continue @@ -240,7 +240,7 @@ func expandAlias( } // if alias is a namespace - nodes := listMachinesInNamespace(machines, alias) + nodes := filterMachinesByNamespace(machines, alias) nodes, err := excludeCorrectlyTaggedNodes(aclPolicy, nodes, alias) if err != nil { return ips, err @@ -357,7 +357,7 @@ func expandPorts(portsStr string) (*[]tailcfg.PortRange, error) { return &ports, nil } -func listMachinesInNamespace(machines []Machine, namespace string) []Machine { +func filterMachinesByNamespace(machines []Machine, namespace string) []Machine { out := []Machine{} for _, machine := range machines { if machine.Namespace.Name == namespace { diff --git a/acls_test.go b/acls_test.go index e68a01f6..32dd5726 100644 --- a/acls_test.go +++ b/acls_test.go @@ -687,7 +687,7 @@ func Test_listMachinesInNamespace(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if got := listMachinesInNamespace(test.args.machines, test.args.namespace); !reflect.DeepEqual( + if got := filterMachinesByNamespace(test.args.machines, test.args.namespace); !reflect.DeepEqual( got, test.want, ) { diff --git a/machine.go b/machine.go index ba677f15..4c984d64 100644 --- a/machine.go +++ b/machine.go @@ -142,6 +142,16 @@ func containsAddresses(inputs []string, addrs MachineAddresses) bool { return false } +// matchSourceAndDestinationWithRule will check if source is authorized to communicate with destination through +// the given rule. +func matchSourceAndDestinationWithRule(rule tailcfg.FilterRule, source Machine, destination Machine) bool { + var dst []string + for _, d := range rule.DstPorts { + dst = append(dst, d.IP) + } + return (containsAddresses(rule.SrcIPs, source.IPAddresses) && containsAddresses(dst, destination.IPAddresses)) || containsString(dst, "*") +} + // getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { log.Trace(). @@ -149,14 +159,12 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { Str("machine", machine.Name). Msg("Finding peers filtered by ACLs") - machines := Machines{} - if err := h.db.Preload("Namespace").Where("machine_key <> ? AND registered", - machine.MachineKey).Find(&machines).Error; err != nil { - log.Error().Err(err).Msg("Error accessing db") - + machines, err := h.ListAllMachines() + if err != nil { + log.Error().Err(err).Msg("Error retrieving list of machines") return Machines{}, err } - mMachines := make(map[uint64]Machine) + peers := make(map[uint64]Machine) // Aclfilter peers here. We are itering through machines in all namespaces and search through the computed aclRules // for match between rule SrcIPs and DstPorts. If the rule is a match we allow the machine to be viewable. @@ -175,21 +183,16 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { // In order to do this we would need to be able to identify that node A want to talk to node B but that Node B doesn't know // how to talk to node A and then add the peering resource. - for _, mchn := range machines { + for _, peer := range machines { for _, rule := range h.aclRules { - var dst []string - for _, d := range rule.DstPorts { - dst = append(dst, d.IP) - } - if (containsAddresses(rule.SrcIPs, machine.IPAddresses) && (containsAddresses(dst, mchn.IPAddresses) || containsString(dst, "*"))) || - (containsAddresses(rule.SrcIPs, mchn.IPAddresses) && containsAddresses(dst, machine.IPAddresses)) { - mMachines[mchn.ID] = mchn + if matchSourceAndDestinationWithRule(rule, *machine, peer) || matchSourceAndDestinationWithRule(rule, peer, *machine) { + peers[peer.ID] = peer } } } - authorizedMachines := make([]Machine, 0, len(mMachines)) - for _, m := range mMachines { + authorizedMachines := make([]Machine, 0, len(peers)) + for _, m := range peers { authorizedMachines = append(authorizedMachines, m) } sort.Slice( @@ -200,7 +203,7 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { log.Trace(). Caller(). Str("machine", machine.Name). - Msgf("Found some machines: %s", machines.String()) + Msgf("Found some machines: %v", machines) return authorizedMachines, nil } From 5e167cc00ad5aa312c1d0c50b289a5479d88b867 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Sun, 20 Feb 2022 23:00:31 +0100 Subject: [PATCH 17/31] fix(tests): fix naming issues related to code review --- acls_test.go | 240 ++++++++++++++++++++++++++------------------------- 1 file changed, 123 insertions(+), 117 deletions(-) diff --git a/acls_test.go b/acls_test.go index 32dd5726..cb333090 100644 --- a/acls_test.go +++ b/acls_test.go @@ -99,16 +99,16 @@ func (s *Suite) TestInvalidTagOwners(c *check.C) { // match properly the IP's of the related hosts. The owner is valid and the tag is also valid. // the tag is matched in the Users section. func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { - namespace, err := app.CreateNamespace("foo") + namespace, err := app.CreateNamespace("user1") c.Assert(err, check.IsNil) pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) c.Assert(err, check.IsNil) - _, err = app.GetMachine("foo", "testmachine") + _, err = app.GetMachine("user1", "testmachine") c.Assert(err, check.NotNil) hostInfo := []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"testmachine\",\"RequestTags\":[\"tag:test\"]}", ) machine := Machine{ ID: 0, @@ -126,8 +126,8 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { app.db.Save(&machine) app.aclPolicy = &ACLPolicy{ - Groups: Groups{"group:test": []string{"foo", "foobar"}}, - TagOwners: TagOwners{"tag:test": []string{"bar", "group:test"}}, + Groups: Groups{"group:test": []string{"user1", "user2"}}, + TagOwners: TagOwners{"tag:test": []string{"user3", "group:test"}}, ACLs: []ACL{ {Action: "accept", Users: []string{"tag:test"}, Ports: []string{"*:*"}}, }, @@ -143,20 +143,20 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { // match properly the IP's of the related hosts. The owner is valid and the tag is also valid. // the tag is matched in the Ports section. func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { - namespace, err := app.CreateNamespace("foo") + namespace, err := app.CreateNamespace("user1") c.Assert(err, check.IsNil) pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) c.Assert(err, check.IsNil) - _, err = app.GetMachine("foo", "testmachine") + _, err = app.GetMachine("user1", "testmachine") c.Assert(err, check.NotNil) hostInfo := []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"testmachine\",\"RequestTags\":[\"tag:test\"]}", ) machine := Machine{ ID: 1, - MachineKey: "foo", + MachineKey: "12345", NodeKey: "bar", DiscoKey: "faa", Name: "testmachine", @@ -170,8 +170,8 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { app.db.Save(&machine) app.aclPolicy = &ACLPolicy{ - Groups: Groups{"group:test": []string{"foo", "foobar"}}, - TagOwners: TagOwners{"tag:test": []string{"bar", "group:test"}}, + Groups: Groups{"group:test": []string{"user1", "user2"}}, + TagOwners: TagOwners{"tag:test": []string{"user3", "group:test"}}, ACLs: []ACL{ {Action: "accept", Users: []string{"*"}, Ports: []string{"tag:test:*"}}, }, @@ -187,20 +187,20 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { // tag on a host that isn't owned by a tag owners. So the namespace // of the host should be valid. func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { - namespace, err := app.CreateNamespace("foo") + namespace, err := app.CreateNamespace("user1") c.Assert(err, check.IsNil) pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) c.Assert(err, check.IsNil) - _, err = app.GetMachine("foo", "testmachine") + _, err = app.GetMachine("user1", "testmachine") c.Assert(err, check.NotNil) hostInfo := []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:foo\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"testmachine\",\"RequestTags\":[\"tag:foo\"]}", ) machine := Machine{ ID: 1, - MachineKey: "foo", + MachineKey: "12345", NodeKey: "bar", DiscoKey: "faa", Name: "testmachine", @@ -214,9 +214,9 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { app.db.Save(&machine) app.aclPolicy = &ACLPolicy{ - TagOwners: TagOwners{"tag:test": []string{"foo"}}, + TagOwners: TagOwners{"tag:test": []string{"user1"}}, ACLs: []ACL{ - {Action: "accept", Users: []string{"foo"}, Ports: []string{"*:*"}}, + {Action: "accept", Users: []string{"user1"}, Ports: []string{"*:*"}}, }, } err = app.UpdateACLRules() @@ -230,20 +230,20 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { // an ACL rule is matching the tag to a namespace. It should not be valid since the // host should be tied to the tag now. func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { - namespace, err := app.CreateNamespace("foo") + namespace, err := app.CreateNamespace("user1") c.Assert(err, check.IsNil) pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) c.Assert(err, check.IsNil) - _, err = app.GetMachine("foo", "webserver") + _, err = app.GetMachine("user1", "webserver") c.Assert(err, check.NotNil) hostInfo := []byte( "{\"OS\":\"centos\",\"Hostname\":\"webserver\",\"RequestTags\":[\"tag:webapp\"]}", ) machine := Machine{ ID: 1, - MachineKey: "foo", + MachineKey: "12345", NodeKey: "bar", DiscoKey: "faa", Name: "webserver", @@ -255,12 +255,12 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { HostInfo: datatypes.JSON(hostInfo), } app.db.Save(&machine) - _, err = app.GetMachine("foo", "user") + _, err = app.GetMachine("user1", "user") hostInfo = []byte("{\"OS\":\"debian\",\"Hostname\":\"user\"}") c.Assert(err, check.NotNil) machine = Machine{ ID: 2, - MachineKey: "foo2", + MachineKey: "56789", NodeKey: "bar2", DiscoKey: "faab", Name: "user", @@ -274,11 +274,11 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { app.db.Save(&machine) app.aclPolicy = &ACLPolicy{ - TagOwners: TagOwners{"tag:webapp": []string{"foo"}}, + TagOwners: TagOwners{"tag:webapp": []string{"user1"}}, ACLs: []ACL{ { Action: "accept", - Users: []string{"foo"}, + Users: []string{"user1"}, Ports: []string{"tag:webapp:80,443"}, }, }, @@ -339,7 +339,7 @@ func (s *Suite) TestPortNamespace(c *check.C) { ips, _ := app.getAvailableIPs() machine := Machine{ ID: 0, - MachineKey: "foo", + MachineKey: "12345", NodeKey: "bar", DiscoKey: "faa", Name: "testmachine", @@ -427,13 +427,13 @@ func Test_expandGroup(t *testing.T) { args: args{ aclPolicy: ACLPolicy{ Groups: Groups{ - "group:test": []string{"g1", "foo", "test"}, - "group:foo": []string{"foo", "test"}, + "group:test": []string{"user1", "user2", "user3"}, + "group:foo": []string{"user2", "user3"}, }, }, group: "group:test", }, - want: []string{"g1", "foo", "test"}, + want: []string{"user1", "user2", "user3"}, wantErr: false, }, { @@ -441,11 +441,11 @@ func Test_expandGroup(t *testing.T) { args: args{ aclPolicy: ACLPolicy{ Groups: Groups{ - "group:test": []string{"g1", "foo", "test"}, - "group:foo": []string{"foo", "test"}, + "group:test": []string{"user1", "user2", "user3"}, + "group:foo": []string{"user2", "user3"}, }, }, - group: "group:bar", + group: "group:undefined", }, want: []string{}, wantErr: true, @@ -478,45 +478,45 @@ func Test_expandTagOwners(t *testing.T) { wantErr bool }{ { - name: "simple tag", + name: "simple tag expansion", args: args{ aclPolicy: ACLPolicy{ - TagOwners: TagOwners{"tag:test": []string{"namespace1"}}, + TagOwners: TagOwners{"tag:test": []string{"user1"}}, }, tag: "tag:test", }, - want: []string{"namespace1"}, + want: []string{"user1"}, wantErr: false, }, { - name: "tag and group", + name: "expand with tag and group", args: args{ aclPolicy: ACLPolicy{ - Groups: Groups{"group:foo": []string{"n1", "bar"}}, + Groups: Groups{"group:foo": []string{"user1", "user2"}}, TagOwners: TagOwners{"tag:test": []string{"group:foo"}}, }, tag: "tag:test", }, - want: []string{"n1", "bar"}, + want: []string{"user1", "user2"}, wantErr: false, }, { - name: "namespace and group", + name: "expand with namespace and group", args: args{ aclPolicy: ACLPolicy{ - Groups: Groups{"group:foo": []string{"n1", "bar"}}, - TagOwners: TagOwners{"tag:test": []string{"group:foo", "home"}}, + Groups: Groups{"group:foo": []string{"user1", "user2"}}, + TagOwners: TagOwners{"tag:test": []string{"group:foo", "user3"}}, }, tag: "tag:test", }, - want: []string{"n1", "bar", "home"}, + want: []string{"user1", "user2", "user3"}, wantErr: false, }, { name: "invalid tag", args: args{ aclPolicy: ACLPolicy{ - TagOwners: TagOwners{"tag:foo": []string{"group:foo", "home"}}, + TagOwners: TagOwners{"tag:foo": []string{"group:foo", "user1"}}, }, tag: "tag:test", }, @@ -527,8 +527,8 @@ func Test_expandTagOwners(t *testing.T) { name: "invalid group", args: args{ aclPolicy: ACLPolicy{ - Groups: Groups{"group:bar": []string{"n1", "foo"}}, - TagOwners: TagOwners{"tag:test": []string{"group:foo", "home"}}, + Groups: Groups{"group:bar": []string{"user1", "user2"}}, + TagOwners: TagOwners{"tag:test": []string{"group:foo", "user2"}}, }, tag: "tag:test", }, @@ -647,40 +647,40 @@ func Test_listMachinesInNamespace(t *testing.T) { name: "1 machine in namespace", args: args{ machines: []Machine{ - {Namespace: Namespace{Name: "test"}}, + {Namespace: Namespace{Name: "joe"}}, }, - namespace: "test", + namespace: "joe", }, want: []Machine{ - {Namespace: Namespace{Name: "test"}}, + {Namespace: Namespace{Name: "joe"}}, }, }, { name: "3 machines, 2 in namespace", args: args{ machines: []Machine{ - {ID: 1, Namespace: Namespace{Name: "test"}}, - {ID: 2, Namespace: Namespace{Name: "foo"}}, - {ID: 3, Namespace: Namespace{Name: "foo"}}, + {ID: 1, Namespace: Namespace{Name: "joe"}}, + {ID: 2, Namespace: Namespace{Name: "marc"}}, + {ID: 3, Namespace: Namespace{Name: "marc"}}, }, - namespace: "foo", + namespace: "marc", }, want: []Machine{ - {ID: 2, Namespace: Namespace{Name: "foo"}}, - {ID: 3, Namespace: Namespace{Name: "foo"}}, + {ID: 2, Namespace: Namespace{Name: "marc"}}, + {ID: 3, Namespace: Namespace{Name: "marc"}}, }, }, { name: "5 machines, 0 in namespace", args: args{ machines: []Machine{ - {ID: 1, Namespace: Namespace{Name: "test"}}, - {ID: 2, Namespace: Namespace{Name: "foo"}}, - {ID: 3, Namespace: Namespace{Name: "foo"}}, - {ID: 4, Namespace: Namespace{Name: "foo"}}, - {ID: 5, Namespace: Namespace{Name: "foo"}}, + {ID: 1, Namespace: Namespace{Name: "joe"}}, + {ID: 2, Namespace: Namespace{Name: "marc"}}, + {ID: 3, Namespace: Namespace{Name: "marc"}}, + {ID: 4, Namespace: Namespace{Name: "marc"}}, + {ID: 5, Namespace: Namespace{Name: "marc"}}, }, - namespace: "bar", + namespace: "mickael", }, want: []Machine{}, }, @@ -730,35 +730,35 @@ func Test_expandAlias(t *testing.T) { { name: "simple group", args: args{ - alias: "group:foo", + alias: "group:accountant", machines: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.3"), }, - Namespace: Namespace{Name: "bar"}, + Namespace: Namespace{Name: "marc"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "test"}, + Namespace: Namespace{Name: "mickael"}, }, }, aclPolicy: ACLPolicy{ - Groups: Groups{"group:foo": []string{"foo", "bar"}}, + Groups: Groups{"group:accountant": []string{"joe", "marc"}}, }, }, want: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, @@ -767,35 +767,35 @@ func Test_expandAlias(t *testing.T) { { name: "wrong group", args: args{ - alias: "group:test", + alias: "group:hr", machines: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.3"), }, - Namespace: Namespace{Name: "bar"}, + Namespace: Namespace{Name: "marc"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "test"}, + Namespace: Namespace{Name: "mickael"}, }, }, aclPolicy: ACLPolicy{ - Groups: Groups{"group:foo": []string{"foo", "bar"}}, + Groups: Groups{"group:accountant": []string{"joe", "marc"}}, }, }, want: []string{}, @@ -848,41 +848,41 @@ func Test_expandAlias(t *testing.T) { { name: "simple tag", args: args{ - alias: "tag:test", + alias: "tag:hr-webserver", machines: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:hr-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:hr-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.3"), }, - Namespace: Namespace{Name: "bar"}, + Namespace: Namespace{Name: "marc"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, }, aclPolicy: ACLPolicy{ - TagOwners: TagOwners{"tag:test": []string{"foo"}}, + TagOwners: TagOwners{"tag:hr-webserver": []string{"joe"}}, }, }, want: []string{"100.64.0.1", "100.64.0.2"}, @@ -891,36 +891,36 @@ func Test_expandAlias(t *testing.T) { { name: "No tag defined", args: args{ - alias: "tag:foo", + alias: "tag:hr-webserver", machines: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.3"), }, - Namespace: Namespace{Name: "bar"}, + Namespace: Namespace{Name: "marc"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "test"}, + Namespace: Namespace{Name: "mickael"}, }, }, aclPolicy: ACLPolicy{ - Groups: Groups{"group:foo": []string{"foo", "bar"}}, - TagOwners: TagOwners{"tag:test": []string{"group:foo"}}, + Groups: Groups{"group:accountant": []string{"joe", "marc"}}, + TagOwners: TagOwners{"tag:accountant-webserver": []string{"group:accountant"}}, }, }, want: []string{}, @@ -929,41 +929,41 @@ func Test_expandAlias(t *testing.T) { { name: "list host in namespace without correctly tagged servers", args: args{ - alias: "foo", + alias: "joe", machines: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:accountant-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:accountant-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.3"), }, - Namespace: Namespace{Name: "bar"}, + Namespace: Namespace{Name: "marc"}, }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, }, aclPolicy: ACLPolicy{ - TagOwners: TagOwners{"tag:test": []string{"foo"}}, + TagOwners: TagOwners{"tag:accountant-webserver": []string{"joe"}}, }, }, want: []string{"100.64.0.4"}, @@ -1005,40 +1005,40 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { name: "exclude nodes with valid tags", args: args{ aclPolicy: ACLPolicy{ - TagOwners: TagOwners{"tag:test": []string{"foo"}}, + TagOwners: TagOwners{"tag:accountant-webserver": []string{"joe"}}, }, nodes: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:accountant-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:accountant-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, }, - namespace: "foo", + namespace: "joe", }, want: []Machine{ { IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, }, wantErr: false, @@ -1047,54 +1047,60 @@ func Test_excludeCorrectlyTaggedNodes(t *testing.T) { name: "all nodes have invalid tags, don't exclude them", args: args{ aclPolicy: ACLPolicy{ - TagOwners: TagOwners{"tag:foo": []string{"foo"}}, + TagOwners: TagOwners{"tag:accountant-webserver": []string{"joe"}}, }, nodes: []Machine{ { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.1"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"hr-web1\",\"RequestTags\":[\"tag:hr-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.2"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"hr-web2\",\"RequestTags\":[\"tag:hr-webserver\"]}", ), }, { IPAddresses: MachineAddresses{ netaddr.MustParseIP("100.64.0.4"), }, - Namespace: Namespace{Name: "foo"}, + Namespace: Namespace{Name: "joe"}, }, }, - namespace: "foo", + namespace: "joe", }, want: []Machine{ { - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, - Namespace: Namespace{Name: "foo"}, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"hr-web1\",\"RequestTags\":[\"tag:hr-webserver\"]}", ), }, { - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, - Namespace: Namespace{Name: "foo"}, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "joe"}, HostInfo: []byte( - "{\"OS\":\"centos\",\"Hostname\":\"foo\",\"RequestTags\":[\"tag:test\"]}", + "{\"OS\":\"centos\",\"Hostname\":\"hr-web2\",\"RequestTags\":[\"tag:hr-webserver\"]}", ), }, { - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.4")}, - Namespace: Namespace{Name: "foo"}, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.4"), + }, + Namespace: Namespace{Name: "joe"}, }, }, wantErr: false, From b3d0fb7a939ceabd4c1f7743149fe41329e29e7b Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Sun, 20 Feb 2022 23:47:04 +0100 Subject: [PATCH 18/31] fix(machine): revert modifications Using h.ListAllMachines also listed the current machine in the result. It's unnecessary (I don't know if it's harmful). Breaking the check with the `matchSourceAndDestinationWithRule` broke the tests. We have a specificity with the '*' destination that isn't symetrical. I need to think of a better way to do this. It too hard to read. --- machine.go | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/machine.go b/machine.go index 4c984d64..46f20daa 100644 --- a/machine.go +++ b/machine.go @@ -142,16 +142,6 @@ func containsAddresses(inputs []string, addrs MachineAddresses) bool { return false } -// matchSourceAndDestinationWithRule will check if source is authorized to communicate with destination through -// the given rule. -func matchSourceAndDestinationWithRule(rule tailcfg.FilterRule, source Machine, destination Machine) bool { - var dst []string - for _, d := range rule.DstPorts { - dst = append(dst, d.IP) - } - return (containsAddresses(rule.SrcIPs, source.IPAddresses) && containsAddresses(dst, destination.IPAddresses)) || containsString(dst, "*") -} - // getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { log.Trace(). @@ -159,9 +149,10 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { Str("machine", machine.Name). Msg("Finding peers filtered by ACLs") - machines, err := h.ListAllMachines() - if err != nil { - log.Error().Err(err).Msg("Error retrieving list of machines") + machines := Machines{} + if err := h.db.Preload("Namespace").Where("machine_key <> ? AND registered", + machine.MachineKey).Find(&machines).Error; err != nil { + log.Error().Err(err).Msg("Error accessing db") return Machines{}, err } peers := make(map[uint64]Machine) @@ -185,7 +176,13 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { for _, peer := range machines { for _, rule := range h.aclRules { - if matchSourceAndDestinationWithRule(rule, *machine, peer) || matchSourceAndDestinationWithRule(rule, peer, *machine) { + var dst []string + for _, d := range rule.DstPorts { + dst = append(dst, d.IP) + } + if (containsAddresses(rule.SrcIPs, machine.IPAddresses) && (containsAddresses(dst, peer.IPAddresses) || containsString(dst, "*"))) || ( + // open return path + containsAddresses(rule.SrcIPs, peer.IPAddresses) && containsAddresses(dst, machine.IPAddresses)) { peers[peer.ID] = peer } } From 5242025ab3cc803c01932d7d903f747a8319913b Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Sun, 20 Feb 2022 23:50:08 +0100 Subject: [PATCH 19/31] fix(machines): renaming following review comments --- machine.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/machine.go b/machine.go index 46f20daa..edebe031 100644 --- a/machine.go +++ b/machine.go @@ -188,13 +188,13 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { } } - authorizedMachines := make([]Machine, 0, len(peers)) + authorizedPeers := make([]Machine, 0, len(peers)) for _, m := range peers { - authorizedMachines = append(authorizedMachines, m) + authorizedPeers = append(authorizedPeers, m) } sort.Slice( - authorizedMachines, - func(i, j int) bool { return authorizedMachines[i].ID < authorizedMachines[j].ID }, + authorizedPeers, + func(i, j int) bool { return authorizedPeers[i].ID < authorizedPeers[j].ID }, ) log.Trace(). @@ -202,7 +202,7 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { Str("machine", machine.Name). Msgf("Found some machines: %v", machines) - return authorizedMachines, nil + return authorizedPeers, nil } func (h *Headscale) getDirectPeers(machine *Machine) (Machines, error) { From 960412a335729869b063b7101ece86367f35c882 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 09:02:27 +0100 Subject: [PATCH 20/31] fix(machines): simplify complex if check This should fix the performance issue with computation of `dst` variable. It's also easier to read now. --- machine.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/machine.go b/machine.go index edebe031..45434075 100644 --- a/machine.go +++ b/machine.go @@ -132,8 +132,8 @@ func (h *Headscale) ListAllMachines() ([]Machine, error) { return machines, nil } -func containsAddresses(inputs []string, addrs MachineAddresses) bool { - for _, addr := range addrs.ToStringSlice() { +func containsAddresses(inputs []string, addrs []string) bool { + for _, addr := range addrs { if containsString(inputs, addr) { return true } @@ -142,6 +142,11 @@ func containsAddresses(inputs []string, addrs MachineAddresses) bool { return false } +// matchSourceAndDestinationWithRule +func matchSourceAndDestinationWithRule(ruleSources []string, ruleDestinations []string, source []string, destination []string) bool { + return containsAddresses(ruleSources, source) && containsAddresses(ruleDestinations, destination) +} + // getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { log.Trace(). @@ -180,9 +185,9 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { for _, d := range rule.DstPorts { dst = append(dst, d.IP) } - if (containsAddresses(rule.SrcIPs, machine.IPAddresses) && (containsAddresses(dst, peer.IPAddresses) || containsString(dst, "*"))) || ( - // open return path - containsAddresses(rule.SrcIPs, peer.IPAddresses) && containsAddresses(dst, machine.IPAddresses)) { + if matchSourceAndDestinationWithRule(rule.SrcIPs, dst, machine.IPAddresses.ToStringSlice(), peer.IPAddresses.ToStringSlice()) || // match source and destination + matchSourceAndDestinationWithRule(rule.SrcIPs, dst, machine.IPAddresses.ToStringSlice(), []string{"*"}) || // match source and all destination + matchSourceAndDestinationWithRule(rule.SrcIPs, dst, peer.IPAddresses.ToStringSlice(), machine.IPAddresses.ToStringSlice()) { // match return path peers[peer.ID] = peer } } From 9c6ce02554020ae114089eecc5e431c0d36dffe4 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 09:05:04 +0100 Subject: [PATCH 21/31] fix(machines): use ListAllMachines function added a simple filter to remove the current node --- machine.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/machine.go b/machine.go index 45434075..8a33e1d8 100644 --- a/machine.go +++ b/machine.go @@ -154,10 +154,9 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { Str("machine", machine.Name). Msg("Finding peers filtered by ACLs") - machines := Machines{} - if err := h.db.Preload("Namespace").Where("machine_key <> ? AND registered", - machine.MachineKey).Find(&machines).Error; err != nil { - log.Error().Err(err).Msg("Error accessing db") + machines, err := h.ListAllMachines() + if err != nil { + log.Error().Err(err).Msg("Error retrieving list of machines") return Machines{}, err } peers := make(map[uint64]Machine) @@ -180,6 +179,9 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { // how to talk to node A and then add the peering resource. for _, peer := range machines { + if peer.ID == machine.ID { + continue + } for _, rule := range h.aclRules { var dst []string for _, d := range rule.DstPorts { From f0068601364890852fde36a425299608ec82cca7 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 09:15:34 +0100 Subject: [PATCH 22/31] feat(machines): untie dependency with class for filter func The dependency to the `headscale` struct makes tests harder to do. This change allow to easily add some tests for this quite sensible function. --- machine.go | 19 ++++++++++--------- machine_test.go | 7 +++++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/machine.go b/machine.go index 8a33e1d8..30c4ad92 100644 --- a/machine.go +++ b/machine.go @@ -148,19 +148,13 @@ func matchSourceAndDestinationWithRule(ruleSources []string, ruleDestinations [] } // getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. -func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { +func getFilteredByACLPeers(machines []Machine, rules []tailcfg.FilterRule, machine *Machine) (Machines, error) { log.Trace(). Caller(). Str("machine", machine.Name). Msg("Finding peers filtered by ACLs") - machines, err := h.ListAllMachines() - if err != nil { - log.Error().Err(err).Msg("Error retrieving list of machines") - return Machines{}, err - } peers := make(map[uint64]Machine) - // Aclfilter peers here. We are itering through machines in all namespaces and search through the computed aclRules // for match between rule SrcIPs and DstPorts. If the rule is a match we allow the machine to be viewable. @@ -182,7 +176,7 @@ func (h *Headscale) getFilteredByACLPeers(machine *Machine) (Machines, error) { if peer.ID == machine.ID { continue } - for _, rule := range h.aclRules { + for _, rule := range rules { var dst []string for _, d := range rule.DstPorts { dst = append(dst, d.IP) @@ -301,10 +295,17 @@ func (h *Headscale) getSharedTo(machine *Machine) (Machines, error) { func (h *Headscale) getPeers(machine *Machine) (Machines, error) { var peers Machines var err error + // If ACLs rules are defined, filter visible host list with the ACLs // else use the classic namespace scope if h.aclPolicy != nil { - peers, err = h.getFilteredByACLPeers(machine) + var machines []Machine + machines, err = h.ListAllMachines() + if err != nil { + log.Error().Err(err).Msg("Error retrieving list of machines") + return Machines{}, err + } + peers, err = getFilteredByACLPeers(machines, h.aclRules, machine) if err != nil { log.Error(). Caller(). diff --git a/machine_test.go b/machine_test.go index f9e58be9..90caf9d2 100644 --- a/machine_test.go +++ b/machine_test.go @@ -219,10 +219,13 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { _, err = testMachine.GetHostInfo() c.Assert(err, check.IsNil) - peersOfTestMachine, err := app.getFilteredByACLPeers(testMachine) + machines, err := app.ListAllMachines() c.Assert(err, check.IsNil) - peersOfAdminMachine, err := app.getFilteredByACLPeers(adminMachine) + peersOfTestMachine, err := getFilteredByACLPeers(machines, app.aclRules, testMachine) + c.Assert(err, check.IsNil) + + peersOfAdminMachine, err := getFilteredByACLPeers(machines, app.aclRules, adminMachine) c.Assert(err, check.IsNil) c.Log(peersOfTestMachine) From 5ab62378aef28cfadf0739b891b0cd7b38d8c033 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 09:57:31 +0100 Subject: [PATCH 23/31] tests(machines): test all combinations of peer filtering --- machine_test.go | 168 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/machine_test.go b/machine_test.go index 90caf9d2..fb104f7d 100644 --- a/machine_test.go +++ b/machine_test.go @@ -2,11 +2,14 @@ package headscale import ( "fmt" + "reflect" "strconv" + "testing" "time" "gopkg.in/check.v1" "inet.af/netaddr" + "tailscale.com/tailcfg" ) func (s *Suite) TestGetMachine(c *check.C) { @@ -295,3 +298,168 @@ func (s *Suite) TestSerdeAddressStrignSlice(c *check.C) { c.Assert(deserialized[i], check.Equals, input[i]) } } + +func Test_getFilteredByACLPeers(t *testing.T) { + type args struct { + machines []Machine + rules []tailcfg.FilterRule + machine *Machine + } + tests := []struct { + name string + args args + want Machines + wantErr bool + }{ + { + name: "all hosts can talk to each other", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + {SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "*"}, + }, + }, + }, + machine: &Machine{ // current machine + ID: 1, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + Namespace: Namespace{Name: "joe"}, + }, + }, + want: Machines{ + { + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, + Namespace: Namespace{Name: "mickael"}, + }, + }, + wantErr: false, + }, + { + name: "One host can talk to another, but not all hosts", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + {SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "100.64.0.2"}, + }, + }, + }, + machine: &Machine{ // current machine + ID: 1, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + Namespace: Namespace{Name: "joe"}, + }, + }, + want: Machines{ + { + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + }, + wantErr: false, + }, + { + name: "host cannot directly talk to destination, but return path is authorized", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + {SrcIPs: []string{"100.64.0.3"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "100.64.0.2"}, + }, + }, + }, + machine: &Machine{ // current machine + ID: 1, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + }, + want: Machines{ + { + ID: 3, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, + Namespace: Namespace{Name: "mickael"}, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getFilteredByACLPeers(tt.args.machines, tt.args.rules, tt.args.machine) + if (err != nil) != tt.wantErr { + t.Errorf("getFilteredByACLPeers() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getFilteredByACLPeers() = %v, want %v", got, tt.want) + } + }) + } +} + +var getFilteredByACLPeersTestRules = []tailcfg.FilterRule{ + { + SrcIPs: []string{"100.64.0.1"}, + DstPorts: []tailcfg.NetPortRange{{IP: "*"}}, + }, +} From 4bbe0051f6410b934b4585a4d690d442cbb21791 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 10:02:59 +0100 Subject: [PATCH 24/31] chore(machines): apply lint --- machine.go | 15 ++++----------- machine_test.go | 39 ++++++++++++--------------------------- 2 files changed, 16 insertions(+), 38 deletions(-) diff --git a/machine.go b/machine.go index 30c4ad92..0ec7b814 100644 --- a/machine.go +++ b/machine.go @@ -142,13 +142,13 @@ func containsAddresses(inputs []string, addrs []string) bool { return false } -// matchSourceAndDestinationWithRule +// matchSourceAndDestinationWithRule. func matchSourceAndDestinationWithRule(ruleSources []string, ruleDestinations []string, source []string, destination []string) bool { return containsAddresses(ruleSources, source) && containsAddresses(ruleDestinations, destination) } // getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. -func getFilteredByACLPeers(machines []Machine, rules []tailcfg.FilterRule, machine *Machine) (Machines, error) { +func getFilteredByACLPeers(machines []Machine, rules []tailcfg.FilterRule, machine *Machine) Machines { log.Trace(). Caller(). Str("machine", machine.Name). @@ -203,7 +203,7 @@ func getFilteredByACLPeers(machines []Machine, rules []tailcfg.FilterRule, machi Str("machine", machine.Name). Msgf("Found some machines: %v", machines) - return authorizedPeers, nil + return authorizedPeers } func (h *Headscale) getDirectPeers(machine *Machine) (Machines, error) { @@ -303,17 +303,10 @@ func (h *Headscale) getPeers(machine *Machine) (Machines, error) { machines, err = h.ListAllMachines() if err != nil { log.Error().Err(err).Msg("Error retrieving list of machines") - return Machines{}, err - } - peers, err = getFilteredByACLPeers(machines, h.aclRules, machine) - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Cannot fetch peers") return Machines{}, err } + peers = getFilteredByACLPeers(machines, h.aclRules, machine) } else { direct, err := h.getDirectPeers(machine) if err != nil { diff --git a/machine_test.go b/machine_test.go index fb104f7d..3be3332b 100644 --- a/machine_test.go +++ b/machine_test.go @@ -225,11 +225,8 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { machines, err := app.ListAllMachines() c.Assert(err, check.IsNil) - peersOfTestMachine, err := getFilteredByACLPeers(machines, app.aclRules, testMachine) - c.Assert(err, check.IsNil) - - peersOfAdminMachine, err := getFilteredByACLPeers(machines, app.aclRules, adminMachine) - c.Assert(err, check.IsNil) + peersOfTestMachine := getFilteredByACLPeers(machines, app.aclRules, testMachine) + peersOfAdminMachine := getFilteredByACLPeers(machines, app.aclRules, adminMachine) c.Log(peersOfTestMachine) c.Assert(len(peersOfTestMachine), check.Equals, 4) @@ -306,10 +303,9 @@ func Test_getFilteredByACLPeers(t *testing.T) { machine *Machine } tests := []struct { - name string - args args - want Machines - wantErr bool + name string + args args + want Machines }{ { name: "all hosts can talk to each other", @@ -332,7 +328,8 @@ func Test_getFilteredByACLPeers(t *testing.T) { }, }, rules: []tailcfg.FilterRule{ // list of all ACLRules registered - {SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, + { + SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, DstPorts: []tailcfg.NetPortRange{ {IP: "*"}, }, @@ -356,7 +353,6 @@ func Test_getFilteredByACLPeers(t *testing.T) { Namespace: Namespace{Name: "mickael"}, }, }, - wantErr: false, }, { name: "One host can talk to another, but not all hosts", @@ -379,7 +375,8 @@ func Test_getFilteredByACLPeers(t *testing.T) { }, }, rules: []tailcfg.FilterRule{ // list of all ACLRules registered - {SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, + { + SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, DstPorts: []tailcfg.NetPortRange{ {IP: "100.64.0.2"}, }, @@ -398,7 +395,6 @@ func Test_getFilteredByACLPeers(t *testing.T) { Namespace: Namespace{Name: "marc"}, }, }, - wantErr: false, }, { name: "host cannot directly talk to destination, but return path is authorized", @@ -421,7 +417,8 @@ func Test_getFilteredByACLPeers(t *testing.T) { }, }, rules: []tailcfg.FilterRule{ // list of all ACLRules registered - {SrcIPs: []string{"100.64.0.3"}, + { + SrcIPs: []string{"100.64.0.3"}, DstPorts: []tailcfg.NetPortRange{ {IP: "100.64.0.2"}, }, @@ -440,26 +437,14 @@ func Test_getFilteredByACLPeers(t *testing.T) { Namespace: Namespace{Name: "mickael"}, }, }, - wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := getFilteredByACLPeers(tt.args.machines, tt.args.rules, tt.args.machine) - if (err != nil) != tt.wantErr { - t.Errorf("getFilteredByACLPeers() error = %v, wantErr %v", err, tt.wantErr) - return - } + got := getFilteredByACLPeers(tt.args.machines, tt.args.rules, tt.args.machine) if !reflect.DeepEqual(got, tt.want) { t.Errorf("getFilteredByACLPeers() = %v, want %v", got, tt.want) } }) } } - -var getFilteredByACLPeersTestRules = []tailcfg.FilterRule{ - { - SrcIPs: []string{"100.64.0.1"}, - DstPorts: []tailcfg.NetPortRange{{IP: "*"}}, - }, -} From 25550f886663c35c4a5c063f31ba93ef6beef89c Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 16:06:20 +0100 Subject: [PATCH 25/31] chore(format): run prettier on repo --- CHANGELOG.md | 8 ++--- acls_test.go | 6 ++-- machine.go | 23 ++++++++++++--- machine_test.go | 78 +++++++++++++++++++++++++++++++------------------ 4 files changed, 77 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cf99d9f..6c04a170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,9 @@ **0.14.0 (2022-xx-xx):** **UPCOMING BREAKING**: -From the **next** version (`0.15.0`), all machines will be able to communicate regardless of -if they are in the same namespace. This means that the behaviour currently limited to ACLs -will become default. From version `0.15.0`, all limitation of communications must be done +From the **next** version (`0.15.0`), all machines will be able to communicate regardless of +if they are in the same namespace. This means that the behaviour currently limited to ACLs +will become default. From version `0.15.0`, all limitation of communications must be done with ACLs. This is a part of aligning `headscale`'s behaviour with Tailscale's upstream behaviour. @@ -17,7 +17,7 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - ACLs have been rewritten to align with the bevaviour Tailscale Control Panel provides. **NOTE:** This is only active if you use ACLs - Namespaces are now treated as Users - All machines can communicate with all machines by default - - Tags should now work correctly and adding a host to Headscale should now reload the rules. + - Tags should now work correctly and adding a host to Headscale should now reload the rules. - The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features **0.13.0 (2022-02-18):** diff --git a/acls_test.go b/acls_test.go index cb333090..44eb2e15 100644 --- a/acls_test.go +++ b/acls_test.go @@ -919,8 +919,10 @@ func Test_expandAlias(t *testing.T) { }, }, aclPolicy: ACLPolicy{ - Groups: Groups{"group:accountant": []string{"joe", "marc"}}, - TagOwners: TagOwners{"tag:accountant-webserver": []string{"group:accountant"}}, + Groups: Groups{"group:accountant": []string{"joe", "marc"}}, + TagOwners: TagOwners{ + "tag:accountant-webserver": []string{"group:accountant"}, + }, }, }, want: []string{}, diff --git a/machine.go b/machine.go index 0ec7b814..b1dd0d27 100644 --- a/machine.go +++ b/machine.go @@ -143,12 +143,22 @@ func containsAddresses(inputs []string, addrs []string) bool { } // matchSourceAndDestinationWithRule. -func matchSourceAndDestinationWithRule(ruleSources []string, ruleDestinations []string, source []string, destination []string) bool { - return containsAddresses(ruleSources, source) && containsAddresses(ruleDestinations, destination) +func matchSourceAndDestinationWithRule( + ruleSources []string, + ruleDestinations []string, + source []string, + destination []string, +) bool { + return containsAddresses(ruleSources, source) && + containsAddresses(ruleDestinations, destination) } // getFilteredByACLPeerss should return the list of peers authorized to be accessed from machine. -func getFilteredByACLPeers(machines []Machine, rules []tailcfg.FilterRule, machine *Machine) Machines { +func getFilteredByACLPeers( + machines []Machine, + rules []tailcfg.FilterRule, + machine *Machine, +) Machines { log.Trace(). Caller(). Str("machine", machine.Name). @@ -181,7 +191,12 @@ func getFilteredByACLPeers(machines []Machine, rules []tailcfg.FilterRule, machi for _, d := range rule.DstPorts { dst = append(dst, d.IP) } - if matchSourceAndDestinationWithRule(rule.SrcIPs, dst, machine.IPAddresses.ToStringSlice(), peer.IPAddresses.ToStringSlice()) || // match source and destination + if matchSourceAndDestinationWithRule( + rule.SrcIPs, + dst, + machine.IPAddresses.ToStringSlice(), + peer.IPAddresses.ToStringSlice(), + ) || // match source and destination matchSourceAndDestinationWithRule(rule.SrcIPs, dst, machine.IPAddresses.ToStringSlice(), []string{"*"}) || // match source and all destination matchSourceAndDestinationWithRule(rule.SrcIPs, dst, peer.IPAddresses.ToStringSlice(), machine.IPAddresses.ToStringSlice()) { // match return path peers[peer.ID] = peer diff --git a/machine_test.go b/machine_test.go index 3be3332b..b1cd3419 100644 --- a/machine_test.go +++ b/machine_test.go @@ -312,19 +312,25 @@ func Test_getFilteredByACLPeers(t *testing.T) { args: args{ machines: []Machine{ // list of all machines in the database { - ID: 1, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, - Namespace: Namespace{Name: "joe"}, + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, }, { - ID: 2, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, - Namespace: Namespace{Name: "marc"}, + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, }, { - ID: 3, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, - Namespace: Namespace{Name: "mickael"}, + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, }, }, rules: []tailcfg.FilterRule{ // list of all ACLRules registered @@ -359,19 +365,25 @@ func Test_getFilteredByACLPeers(t *testing.T) { args: args{ machines: []Machine{ // list of all machines in the database { - ID: 1, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, - Namespace: Namespace{Name: "joe"}, + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, }, { - ID: 2, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, - Namespace: Namespace{Name: "marc"}, + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, }, { - ID: 3, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, - Namespace: Namespace{Name: "mickael"}, + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, }, }, rules: []tailcfg.FilterRule{ // list of all ACLRules registered @@ -401,19 +413,25 @@ func Test_getFilteredByACLPeers(t *testing.T) { args: args{ machines: []Machine{ // list of all machines in the database { - ID: 1, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, - Namespace: Namespace{Name: "joe"}, + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, }, { - ID: 2, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, - Namespace: Namespace{Name: "marc"}, + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, }, { - ID: 3, - IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, - Namespace: Namespace{Name: "mickael"}, + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, }, }, rules: []tailcfg.FilterRule{ // list of all ACLRules registered @@ -441,7 +459,11 @@ func Test_getFilteredByACLPeers(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := getFilteredByACLPeers(tt.args.machines, tt.args.rules, tt.args.machine) + got := getFilteredByACLPeers( + tt.args.machines, + tt.args.rules, + tt.args.machine, + ) if !reflect.DeepEqual(got, tt.want) { t.Errorf("getFilteredByACLPeers() = %v, want %v", got, tt.want) } From 211fe4034a0b32cc1c86c60370a744642ad2e5d1 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 16:10:20 +0100 Subject: [PATCH 26/31] chore(linter): ignore tt var as it's generated code (vscode) --- .golangci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yaml b/.golangci.yaml index 965f5496..153cd7c3 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -48,6 +48,7 @@ linters-settings: - ip - ok - c + - tt gocritic: disabled-checks: From 50af44bc2fa7a578b3e141518ffcef2e32976325 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 20:06:31 +0100 Subject: [PATCH 27/31] fix: add error checking in acl and poll If aclPolicy is not defined, in updateAclPolicy, return an error. --- acls.go | 4 ++++ poll.go | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/acls.go b/acls.go index f9ed09da..db2fc588 100644 --- a/acls.go +++ b/acls.go @@ -86,6 +86,10 @@ func (h *Headscale) UpdateACLRules() error { func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { rules := []tailcfg.FilterRule{} + if h.aclPolicy == nil { + return nil, errEmptyPolicy + } + machines, err := h.ListAllMachines() if err != nil { return nil, err diff --git a/poll.go b/poll.go index 96db43f2..21aa3b33 100644 --- a/poll.go +++ b/poll.go @@ -95,15 +95,16 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { now := time.Now().UTC() // update ACLRules with peer informations (to update server tags if necessary) - err = h.UpdateACLRules() - if err != nil { - log.Error(). - Caller(). - Str("func", "handleAuthKey"). - Str("machine", machine.Name). - Err(err) + if h.aclPolicy != nil { + err = h.UpdateACLRules() + if err != nil { + log.Error(). + Caller(). + Str("func", "handleAuthKey"). + Str("machine", machine.Name). + Err(err) + } } - // From Tailscale client: // // ReadOnly is whether the client just wants to fetch the MapResponse, From baae266db015297ff304c1547fec6eed2fc6c45b Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 20:25:41 +0100 Subject: [PATCH 28/31] Update acls_test.go Co-authored-by: Kristoffer Dalby --- acls_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/acls_test.go b/acls_test.go index 44eb2e15..965c0076 100644 --- a/acls_test.go +++ b/acls_test.go @@ -59,6 +59,7 @@ func (s *Suite) TestBasicRule(c *check.C) { c.Assert(rules, check.NotNil) } +# TODO(kradalby): Make tests values safe, independent and descriptive. func (s *Suite) TestInvalidAction(c *check.C) { app.aclPolicy = &ACLPolicy{ ACLs: []ACL{ From 650108c7c7fc7fdf84cd5cd2fbe6be043ef74dd9 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 21:45:15 +0100 Subject: [PATCH 29/31] chore(fmt): apply fmt --- machine.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/machine.go b/machine.go index b1dd0d27..ee483422 100644 --- a/machine.go +++ b/machine.go @@ -197,8 +197,18 @@ func getFilteredByACLPeers( machine.IPAddresses.ToStringSlice(), peer.IPAddresses.ToStringSlice(), ) || // match source and destination - matchSourceAndDestinationWithRule(rule.SrcIPs, dst, machine.IPAddresses.ToStringSlice(), []string{"*"}) || // match source and all destination - matchSourceAndDestinationWithRule(rule.SrcIPs, dst, peer.IPAddresses.ToStringSlice(), machine.IPAddresses.ToStringSlice()) { // match return path + matchSourceAndDestinationWithRule( + rule.SrcIPs, + dst, + machine.IPAddresses.ToStringSlice(), + []string{"*"}, + ) || // match source and all destination + matchSourceAndDestinationWithRule( + rule.SrcIPs, + dst, + peer.IPAddresses.ToStringSlice(), + machine.IPAddresses.ToStringSlice(), + ) { // match return path peers[peer.ID] = peer } } From d971f0f0e62ec3ca3cf58f236db62fa9d09405a4 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Mon, 21 Feb 2022 21:48:05 +0100 Subject: [PATCH 30/31] fix(acls_test): fix comment in go code --- acls_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acls_test.go b/acls_test.go index 965c0076..5534257d 100644 --- a/acls_test.go +++ b/acls_test.go @@ -59,7 +59,7 @@ func (s *Suite) TestBasicRule(c *check.C) { c.Assert(rules, check.NotNil) } -# TODO(kradalby): Make tests values safe, independent and descriptive. +// TODO(kradalby): Make tests values safe, independent and descriptive. func (s *Suite) TestInvalidAction(c *check.C) { app.aclPolicy = &ACLPolicy{ ACLs: []ACL{ From f2f8d834e81bd1818e7f96a9446b356276927164 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 22 Feb 2022 11:26:21 +0100 Subject: [PATCH 31/31] fix(machine): remove comment After some more tests in tailscale I couldn't replicate the behavior described in there. When adding a rule, allowing A to talk to B the reverse connection was instantly added to B to allow communication to B. The previous assumption was probably wrong. --- machine.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/machine.go b/machine.go index ee483422..3c704ad3 100644 --- a/machine.go +++ b/machine.go @@ -167,21 +167,6 @@ func getFilteredByACLPeers( peers := make(map[uint64]Machine) // Aclfilter peers here. We are itering through machines in all namespaces and search through the computed aclRules // for match between rule SrcIPs and DstPorts. If the rule is a match we allow the machine to be viewable. - - // FIXME: On official control plane if a rule allow user A to talk to user B but NO rule allows user B to talk to - // user A. The behaviour is the following - // - // On official tailscale control plane: - // on first `tailscale status`` on node A we can see node B. The `tailscale status` command on node B doesn't show node A - // We can successfully establish a communication from A to B. When it's done, if we run the `tailscale status` command - // on node B again we can now see node A. It's not possible to establish a communication from node B to node A. - // On this implementation of the feature - // on any `tailscale status` command on node A we can see node B. The `tailscale status` command on node B DOES show A. - // - // I couldn't find a way to not clutter the output of `tailscale status` with all nodes that we could be talking to. - // In order to do this we would need to be able to identify that node A want to talk to node B but that Node B doesn't know - // how to talk to node A and then add the peering resource. - for _, peer := range machines { if peer.ID == machine.ID { continue