Merge branch 'main' into test/multiple-peer-logging

client/internal/acl/manager.go

@@ -398,11 +398,15 @@ func (d *DefaultManager) squashAcceptRules(
 	//
 	// We zeroed this to notify squash function that this protocol can't be squashed.
 	addRuleToCalculationMap := func(i int, r *mgmProto.FirewallRule, protocols map[mgmProto.RuleProtocol]*protoMatch) {
-		drop := r.Action == mgmProto.RuleAction_DROP || r.Port != ""
+		hasPortRestrictions := r.Action == mgmProto.RuleAction_DROP ||
-		if drop {
+			r.Port != "" || !portInfoEmpty(r.PortInfo)
+
+		if hasPortRestrictions {
+			// Don't squash rules with port restrictions
 			protocols[r.Protocol] = &protoMatch{ips: map[string]int{}}
 			return
 		}
+
 		if _, ok := protocols[r.Protocol]; !ok {
 			protocols[r.Protocol] = &protoMatch{
 				ips: map[string]int{},

client/internal/acl/manager_test.go

@@ -330,6 +330,434 @@ func TestDefaultManagerSquashRulesNoAffect(t *testing.T) {
 	assert.Equal(t, len(networkMap.FirewallRules), len(rules))
 }
 
+func TestDefaultManagerSquashRulesWithPortRestrictions(t *testing.T) {
+	tests := []struct {
+		name          string
+		rules         []*mgmProto.FirewallRule
+		expectedCount int
+		description   string
+	}{
+		{
+			name: "should not squash rules with port ranges",
+			rules: []*mgmProto.FirewallRule{
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Range_{
+							Range: &mgmProto.PortInfo_Range{
+								Start: 8080,
+								End:   8090,
+							},
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Range_{
+							Range: &mgmProto.PortInfo_Range{
+								Start: 8080,
+								End:   8090,
+							},
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Range_{
+							Range: &mgmProto.PortInfo_Range{
+								Start: 8080,
+								End:   8090,
+							},
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Range_{
+							Range: &mgmProto.PortInfo_Range{
+								Start: 8080,
+								End:   8090,
+							},
+						},
+					},
+				},
+			},
+			expectedCount: 4,
+			description:   "Rules with port ranges should not be squashed even if they cover all peers",
+		},
+		{
+			name: "should not squash rules with specific ports",
+			rules: []*mgmProto.FirewallRule{
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Port{
+							Port: 80,
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Port{
+							Port: 80,
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Port{
+							Port: 80,
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Port{
+							Port: 80,
+						},
+					},
+				},
+			},
+			expectedCount: 4,
+			description:   "Rules with specific ports should not be squashed even if they cover all peers",
+		},
+		{
+			name: "should not squash rules with legacy port field",
+			rules: []*mgmProto.FirewallRule{
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+			},
+			expectedCount: 4,
+			description:   "Rules with legacy port field should not be squashed",
+		},
+		{
+			name: "should not squash rules with DROP action",
+			rules: []*mgmProto.FirewallRule{
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_DROP,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_DROP,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_DROP,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_DROP,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+			},
+			expectedCount: 4,
+			description:   "Rules with DROP action should not be squashed",
+		},
+		{
+			name: "should squash rules without port restrictions",
+			rules: []*mgmProto.FirewallRule{
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+			},
+			expectedCount: 1,
+			description:   "Rules without port restrictions should be squashed into a single 0.0.0.0 rule",
+		},
+		{
+			name: "mixed rules should not squash protocol with port restrictions",
+			rules: []*mgmProto.FirewallRule{
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					PortInfo: &mgmProto.PortInfo{
+						PortSelection: &mgmProto.PortInfo_Port{
+							Port: 80,
+						},
+					},
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+				},
+			},
+			expectedCount: 4,
+			description:   "TCP should not be squashed because one rule has port restrictions",
+		},
+		{
+			name: "should squash UDP but not TCP when TCP has port restrictions",
+			rules: []*mgmProto.FirewallRule{
+				// TCP rules with port restrictions - should NOT be squashed
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_TCP,
+					Port:      "443",
+				},
+				// UDP rules without port restrictions - SHOULD be squashed
+				{
+					PeerIP:    "10.93.0.1",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_UDP,
+				},
+				{
+					PeerIP:    "10.93.0.2",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_UDP,
+				},
+				{
+					PeerIP:    "10.93.0.3",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_UDP,
+				},
+				{
+					PeerIP:    "10.93.0.4",
+					Direction: mgmProto.RuleDirection_IN,
+					Action:    mgmProto.RuleAction_ACCEPT,
+					Protocol:  mgmProto.RuleProtocol_UDP,
+				},
+			},
+			expectedCount: 5, // 4 TCP rules + 1 squashed UDP rule (0.0.0.0)
+			description:   "UDP should be squashed to 0.0.0.0 rule, but TCP should remain as individual rules due to port restrictions",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			networkMap := &mgmProto.NetworkMap{
+				RemotePeers: []*mgmProto.RemotePeerConfig{
+					{AllowedIps: []string{"10.93.0.1"}},
+					{AllowedIps: []string{"10.93.0.2"}},
+					{AllowedIps: []string{"10.93.0.3"}},
+					{AllowedIps: []string{"10.93.0.4"}},
+				},
+				FirewallRules: tt.rules,
+			}
+
+			manager := &DefaultManager{}
+			rules, _ := manager.squashAcceptRules(networkMap)
+
+			assert.Equal(t, tt.expectedCount, len(rules), tt.description)
+
+			// For squashed rules, verify we get the expected 0.0.0.0 rule
+			if tt.expectedCount == 1 {
+				assert.Equal(t, "0.0.0.0", rules[0].PeerIP)
+				assert.Equal(t, mgmProto.RuleDirection_IN, rules[0].Direction)
+				assert.Equal(t, mgmProto.RuleAction_ACCEPT, rules[0].Action)
+			}
+		})
+	}
+}
+
+func TestPortInfoEmpty(t *testing.T) {
+	tests := []struct {
+		name     string
+		portInfo *mgmProto.PortInfo
+		expected bool
+	}{
+		{
+			name:     "nil PortInfo should be empty",
+			portInfo: nil,
+			expected: true,
+		},
+		{
+			name: "PortInfo with zero port should be empty",
+			portInfo: &mgmProto.PortInfo{
+				PortSelection: &mgmProto.PortInfo_Port{
+					Port: 0,
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "PortInfo with valid port should not be empty",
+			portInfo: &mgmProto.PortInfo{
+				PortSelection: &mgmProto.PortInfo_Port{
+					Port: 80,
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "PortInfo with nil range should be empty",
+			portInfo: &mgmProto.PortInfo{
+				PortSelection: &mgmProto.PortInfo_Range_{
+					Range: nil,
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "PortInfo with zero start range should be empty",
+			portInfo: &mgmProto.PortInfo{
+				PortSelection: &mgmProto.PortInfo_Range_{
+					Range: &mgmProto.PortInfo_Range{
+						Start: 0,
+						End:   100,
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "PortInfo with zero end range should be empty",
+			portInfo: &mgmProto.PortInfo{
+				PortSelection: &mgmProto.PortInfo_Range_{
+					Range: &mgmProto.PortInfo_Range{
+						Start: 80,
+						End:   0,
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "PortInfo with valid range should not be empty",
+			portInfo: &mgmProto.PortInfo{
+				PortSelection: &mgmProto.PortInfo_Range_{
+					Range: &mgmProto.PortInfo_Range{
+						Start: 8080,
+						End:   8090,
+					},
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := portInfoEmpty(tt.portInfo)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
 func TestDefaultManagerEnableSSHRules(t *testing.T) {
 	networkMap := &mgmProto.NetworkMap{
 		PeerConfig: &mgmProto.PeerConfig{