feat(acls): check acl owners and add bunch of tests

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 {
+		namespaces, err := h.expandGroup(alias)
-			return nil, errInvalidGroup
+		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 {
+		var ips []string
-			return nil, errInvalidTag
+		owners, err := h.expandTagOwners(alias)
-		}
+		if err != nil {
-
-		// 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 {
 			return nil, err
 		}
-		ips := []string{}
+		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 {
-			hostinfo := tailcfg.Hostinfo{}
+				if len(machine.HostInfo) == 0 {
-			if len(machine.HostInfo) != 0 {
+					continue
-				hi, err := machine.HostInfo.MarshalJSON()
+				}
+				hi, err := machine.GetHostInfo()
 				if err != nil {
 					return nil, err
 				}
-				err = json.Unmarshal(hi, &hostinfo)
+				for _, t := range hi.RequestTags {
-				if err != nil {
+					if alias == t {
-					return nil, err
-				}
-
-				// FIXME: Check TagOwners allows this
-				for _, t := range hostinfo.RequestTags {
-					if alias[4:] == 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{

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)

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,