policy/v2: validate that no undefined group or tag is used

Fixes #2570

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

hscontrol/policy/v2/types.go

@@ -720,6 +720,20 @@ type Usernames []Username
 // Groups are a map of Group to a list of Username.
 type Groups map[Group]Usernames
 
+func (g Groups) Contains(group *Group) error {
+	if group == nil {
+		return nil
+	}
+
+	for defined := range map[Group]Usernames(g) {
+		if defined == *group {
+			return nil
+		}
+	}
+
+	return fmt.Errorf(`Group %q is not defined in the Policy, please define or remove the reference to it`, group)
+}
+
 // UnmarshalJSON overrides the default JSON unmarshalling for Groups to ensure
 // that each group name is validated using the isGroup function. This ensures
 // that all group names conform to the expected format, which is always prefixed
@@ -791,6 +805,20 @@ func (h Hosts) exist(name Host) bool {
 // TagOwners are a map of Tag to a list of the UserEntities that own the tag.
 type TagOwners map[Tag]Owners
 
+func (to TagOwners) Contains(tagOwner *Tag) error {
+	if tagOwner == nil {
+		return nil
+	}
+
+	for defined := range map[Tag]Owners(to) {
+		if defined == *tagOwner {
+			return nil
+		}
+	}
+
+	return fmt.Errorf(`Tag %q is not defined in the Policy, please define or remove the reference to it`, tagOwner)
+}
+
 // resolveTagOwners resolves the TagOwners to a map of Tag to netipx.IPSet.
 // The resulting map can be used to quickly look up the IPSet for a given Tag.
 // It is intended for internal use in a PolicyManager.
@@ -1047,6 +1075,16 @@ func (p *Policy) validate() error {
 					errs = append(errs, err)
 					continue
 				}
+			case *Group:
+				g := src.(*Group)
+				if err := p.Groups.Contains(g); err != nil {
+					errs = append(errs, err)
+				}
+			case *Tag:
+				tagOwner := src.(*Tag)
+				if err := p.TagOwners.Contains(tagOwner); err != nil {
+					errs = append(errs, err)
+				}
 			}
 		}
 
@@ -1069,6 +1107,16 @@ func (p *Policy) validate() error {
 					errs = append(errs, err)
 					continue
 				}
+			case *Group:
+				g := dst.Alias.(*Group)
+				if err := p.Groups.Contains(g); err != nil {
+					errs = append(errs, err)
+				}
+			case *Tag:
+				tagOwner := dst.Alias.(*Tag)
+				if err := p.TagOwners.Contains(tagOwner); err != nil {
+					errs = append(errs, err)
+				}
 			}
 		}
 	}
@@ -1102,6 +1150,16 @@ func (p *Policy) validate() error {
 					errs = append(errs, err)
 					continue
 				}
+			case *Group:
+				g := src.(*Group)
+				if err := p.Groups.Contains(g); err != nil {
+					errs = append(errs, err)
+				}
+			case *Tag:
+				tagOwner := src.(*Tag)
+				if err := p.TagOwners.Contains(tagOwner); err != nil {
+					errs = append(errs, err)
+				}
 			}
 		}
 		for _, dst := range ssh.Destinations {
@@ -1117,6 +1175,55 @@ func (p *Policy) validate() error {
 					errs = append(errs, err)
 					continue
 				}
+			case *Tag:
+				tagOwner := dst.(*Tag)
+				if err := p.TagOwners.Contains(tagOwner); err != nil {
+					errs = append(errs, err)
+				}
+			}
+		}
+	}
+
+	for _, tagOwners := range p.TagOwners {
+		for _, tagOwner := range tagOwners {
+			switch tagOwner.(type) {
+			case *Group:
+				g := tagOwner.(*Group)
+				if err := p.Groups.Contains(g); err != nil {
+					errs = append(errs, err)
+				}
+			}
+		}
+	}
+
+	for _, approvers := range p.AutoApprovers.Routes {
+		for _, approver := range approvers {
+			switch approver.(type) {
+			case *Group:
+				g := approver.(*Group)
+				if err := p.Groups.Contains(g); err != nil {
+					errs = append(errs, err)
+				}
+			case *Tag:
+				tagOwner := approver.(*Tag)
+				if err := p.TagOwners.Contains(tagOwner); err != nil {
+					errs = append(errs, err)
+				}
+			}
+		}
+	}
+
+	for _, approver := range p.AutoApprovers.ExitNode {
+		switch approver.(type) {
+		case *Group:
+			g := approver.(*Group)
+			if err := p.Groups.Contains(g); err != nil {
+				errs = append(errs, err)
+			}
+		case *Tag:
+			tagOwner := approver.(*Tag)
+			if err := p.TagOwners.Contains(tagOwner); err != nil {
+				errs = append(errs, err)
 			}
 		}
 	}

hscontrol/policy/v2/types_test.go

@@ -511,6 +511,201 @@ func TestUnmarshalPolicy(t *testing.T) {
 `,
 			wantErr: `"autogroup:internet" used in SSH destination, it can only be used in ACL destinations`,
 		},
+		{
+			name: "group-must-be-defined-acl-src",
+			input: `
+{
+  "acls": [
+    {
+      "action": "accept",
+      "src": [
+        "group:notdefined"
+      ],
+      "dst": [
+        "autogroup:internet:*"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `Group "group:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "group-must-be-defined-acl-dst",
+			input: `
+{
+  "acls": [
+    {
+      "action": "accept",
+      "src": [
+        "*"
+      ],
+      "dst": [
+        "group:notdefined:*"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `Group "group:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "group-must-be-defined-acl-ssh-src",
+			input: `
+{
+  "ssh": [
+    {
+      "action": "accept",
+      "src": [
+        "group:notdefined"
+      ],
+      "dst": [
+        "user@"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `Group "group:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "group-must-be-defined-acl-tagOwner",
+			input: `
+{
+  "tagOwners": {
+    "tag:test": ["group:notdefined"],
+  },
+}
+`,
+			wantErr: `Group "group:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "group-must-be-defined-acl-autoapprover-route",
+			input: `
+{
+  "autoApprovers": {
+    "routes": {
+      "10.0.0.0/16": ["group:notdefined"]
+    }
+  },
+}
+`,
+			wantErr: `Group "group:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "group-must-be-defined-acl-autoapprover-exitnode",
+			input: `
+{
+  "autoApprovers": {
+    "exitNode": ["group:notdefined"]
+   },
+}
+`,
+			wantErr: `Group "group:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "tag-must-be-defined-acl-src",
+			input: `
+{
+  "acls": [
+    {
+      "action": "accept",
+      "src": [
+        "tag:notdefined"
+      ],
+      "dst": [
+        "autogroup:internet:*"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "tag-must-be-defined-acl-dst",
+			input: `
+{
+  "acls": [
+    {
+      "action": "accept",
+      "src": [
+        "*"
+      ],
+      "dst": [
+        "tag:notdefined:*"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "tag-must-be-defined-acl-ssh-src",
+			input: `
+{
+  "ssh": [
+    {
+      "action": "accept",
+      "src": [
+        "tag:notdefined"
+      ],
+      "dst": [
+        "user@"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "tag-must-be-defined-acl-ssh-dst",
+			input: `
+{
+  "groups": {
+  	"group:defined": ["user@"],
+  },
+  "ssh": [
+    {
+      "action": "accept",
+      "src": [
+        "group:defined"
+      ],
+      "dst": [
+        "tag:notdefined",
+      ],
+    }
+  ]
+}
+`,
+			wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "tag-must-be-defined-acl-autoapprover-route",
+			input: `
+{
+  "autoApprovers": {
+    "routes": {
+      "10.0.0.0/16": ["tag:notdefined"]
+    }
+  },
+}
+`,
+			wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
+		{
+			name: "tag-must-be-defined-acl-autoapprover-exitnode",
+			input: `
+{
+  "autoApprovers": {
+    "exitNode": ["tag:notdefined"]
+   },
+}
+`,
+			wantErr: `Tag "tag:notdefined" is not defined in the Policy, please define or remove the reference to it`,
+		},
 	}
 
 	cmps := append(util.Comparers, cmp.Comparer(func(x, y Prefix) bool {