From fb45138fc19fffeb336e46c0a859ee9c711240be Mon Sep 17 00:00:00 2001 From: Adrien Raffin Date: Sat, 5 Feb 2022 17:18:39 +0100 Subject: [PATCH] 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,