From e4d10ad9640031ac38c98aaed1ce3459733585c2 Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@dalby.cc>
Date: Fri, 2 May 2025 13:58:12 +0300
Subject: [PATCH] policy/v2: validate autogroup:interet only in dst (#2552)

---
 hscontrol/policy/v2/types.go      | 40 +++++++++++++-
 hscontrol/policy/v2/types_test.go | 91 +++++++++++++++++++++++++++++++
 2 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go
index 0e292f3a..2ee998b6 100644
--- a/hscontrol/policy/v2/types.go
+++ b/hscontrol/policy/v2/types.go
@@ -382,13 +382,13 @@ func (p Prefix) Resolve(_ *Policy, _ types.Users, nodes types.Nodes) (*netipx.IP
 type AutoGroup string
 
 const (
-	AutoGroupInternet = "autogroup:internet"
+	AutoGroupInternet AutoGroup = "autogroup:internet"
 )
 
-var autogroups = []string{AutoGroupInternet}
+var autogroups = []AutoGroup{AutoGroupInternet}
 
 func (ag AutoGroup) Validate() error {
-	if slices.Contains(autogroups, string(ag)) {
+	if slices.Contains(autogroups, ag) {
 		return nil
 	}
 
@@ -412,6 +412,14 @@ func (ag AutoGroup) Resolve(_ *Policy, _ types.Users, _ types.Nodes) (*netipx.IP
 	return nil, nil
 }
 
+func (ag *AutoGroup) Is(c AutoGroup) bool {
+	if ag == nil {
+		return false
+	}
+
+	return *ag == c
+}
+
 type Alias interface {
 	Validate() error
 	UnmarshalJSON([]byte) error
@@ -928,6 +936,32 @@ func (p *Policy) validate() error {
 				if !p.Hosts.exist(*h) {
 					errs = append(errs, fmt.Errorf(`Host %q is not defined in the Policy, please define or remove the reference to it`, *h))
 				}
+			case *AutoGroup:
+				ag := src.(*AutoGroup)
+				if ag.Is(AutoGroupInternet) {
+					errs = append(errs, fmt.Errorf(`"autogroup:internet" used in source, it can only be used in ACL destinations`))
+				}
+			}
+		}
+	}
+
+	for _, ssh := range p.SSHs {
+		for _, src := range ssh.Sources {
+			switch src.(type) {
+			case *AutoGroup:
+				ag := src.(*AutoGroup)
+				if ag.Is(AutoGroupInternet) {
+					errs = append(errs, fmt.Errorf(`"autogroup:internet" used in SSH source, it can only be used in ACL destinations`))
+				}
+			}
+		}
+		for _, dst := range ssh.Destinations {
+			switch dst.(type) {
+			case *AutoGroup:
+				ag := dst.(*AutoGroup)
+				if ag.Is(AutoGroupInternet) {
+					errs = append(errs, fmt.Errorf(`"autogroup:internet" used in SSH destination, it can only be used in ACL destinations`))
+				}
 			}
 		}
 	}
diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go
index 6a89efd3..b428c55a 100644
--- a/hscontrol/policy/v2/types_test.go
+++ b/hscontrol/policy/v2/types_test.go
@@ -420,6 +420,97 @@ func TestUnmarshalPolicy(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "autogroup:internet-in-dst-allowed",
+			input: `
+{
+  "acls": [
+    {
+      "action": "accept",
+      "src": [
+        "10.0.0.1"
+      ],
+      "dst": [
+        "autogroup:internet:*"
+      ]
+    }
+  ]
+}
+`,
+			want: &Policy{
+				ACLs: []ACL{
+					{
+						Action: "accept",
+						Sources: Aliases{
+							pp("10.0.0.1/32"),
+						},
+						Destinations: []AliasWithPorts{
+							{
+								Alias: ptr.To(AutoGroup("autogroup:internet")),
+								Ports: []tailcfg.PortRange{tailcfg.PortRangeAny},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "autogroup:internet-in-src-not-allowed",
+			input: `
+{
+  "acls": [
+    {
+      "action": "accept",
+      "src": [
+        "autogroup:internet"
+      ],
+      "dst": [
+        "10.0.0.1:*"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `"autogroup:internet" used in source, it can only be used in ACL destinations`,
+		},
+		{
+			name: "autogroup:internet-in-ssh-src-not-allowed",
+			input: `
+{
+  "ssh": [
+    {
+      "action": "accept",
+      "src": [
+        "autogroup:internet"
+      ],
+      "dst": [
+        "tag:test"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `"autogroup:internet" used in SSH source, it can only be used in ACL destinations`,
+		},
+		{
+			name: "autogroup:internet-in-ssh-dst-not-allowed",
+			input: `
+{
+  "ssh": [
+    {
+      "action": "accept",
+      "src": [
+        "tag:test"
+      ],
+      "dst": [
+        "autogroup:internet"
+      ]
+    }
+  ]
+}
+`,
+			wantErr: `"autogroup:internet" used in SSH destination, it can only be used in ACL destinations`,
+		},
 	}
 
 	cmps := append(util.Comparers, cmp.Comparer(func(x, y Prefix) bool {