diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 9b029b4616..69931e28bc 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -349,7 +349,7 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return err } - err = checkForNamedPortType(portKind, npmLiteToggle) + err = checkForNamedPortType(npmNetPol, portKind, npmLiteToggle, direction, &ports[i], "") if err != nil { return err } @@ -362,6 +362,52 @@ func peerAndPortRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Di return nil } +func directPeerAndPortAllowRule(npmNetPol *policies.NPMNetworkPolicy, direction policies.Direction, ports []networkingv1.NetworkPolicyPort, cidr string, npmLiteToggle bool) error { + if len(ports) == 0 { + acl := policies.NewACLPolicy(policies.Allowed, direction) + // bypasses ipset creation for /32 cidrs and directly creates an acl with the cidr + if direction == policies.Ingress { + acl.SrcDirectIPs = []string{cidr} + } else { + acl.DstDirectIPs = []string{cidr} + } + npmNetPol.ACLs = append(npmNetPol.ACLs, acl) + return nil + } else { + // handle each port separately + for i := range ports { + portKind, err := portType(ports[i]) + if err != nil { + return err + } + + err = checkForNamedPortType(npmNetPol, portKind, npmLiteToggle, direction, &ports[i], cidr) + if err != nil { + return err + } + + acl := policies.NewACLPolicy(policies.Allowed, direction) + + // Set direct IP based on direction + if direction == policies.Ingress { + acl.SrcDirectIPs = []string{cidr} + } else { + acl.DstDirectIPs = []string{cidr} + } + + // Handle ports + if portKind == numericPortType { + portInfo, protocol := numericPortRule(&ports[i]) + acl.DstPorts = portInfo + acl.Protocol = policies.Protocol(protocol) + } + npmNetPol.ACLs = append(npmNetPol.ACLs, acl) + + } + } + return nil +} + // translateRule translates ingress or egress rules and update npmNetPol object. func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, @@ -405,6 +451,14 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, // #2.1 Handle IPBlock and port if exist if peer.IPBlock != nil { if len(peer.IPBlock.CIDR) > 0 { + if npmLiteToggle { + err = directPeerAndPortAllowRule(npmNetPol, direction, ports, peer.IPBlock.CIDR, npmLiteToggle) + if err != nil { + return err + } + continue + } + ipBlockIPSet, ipBlockSetInfo, err := ipBlockRule(netPolName, npmNetPol.Namespace, direction, matchType, ruleIndex, peerIdx, peer.IPBlock) if err != nil { return err @@ -417,12 +471,6 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, } } - // if npm lite is configured, check network policy only consists of CIDR blocks - err := npmLiteValidPolicy(peer, npmLiteToggle) - if err != nil { - return err - } - // Do not need to run below code to translate PodSelector and NamespaceSelector // since IPBlock field is exclusive in NetworkPolicyPeer (i.e., peer in this code). @@ -642,17 +690,10 @@ func TranslatePolicy(npObj *networkingv1.NetworkPolicy, npmLiteToggle bool) (*po return npmNetPol, nil } -// validates only CIDR based peer is present + no combination of CIDR with pod/namespace selectors are present -func npmLiteValidPolicy(peer networkingv1.NetworkPolicyPeer, npmLiteEnabled bool) error { - if npmLiteEnabled && (peer.PodSelector != nil || peer.NamespaceSelector != nil) { - return ErrUnsupportedNonCIDR - } - return nil -} - -func checkForNamedPortType(portKind netpolPortType, npmLiteToggle bool) error { +func checkForNamedPortType(npmNetPol *policies.NPMNetworkPolicy, portKind netpolPortType, npmLiteToggle bool, direction policies.Direction, port *networkingv1.NetworkPolicyPort, cidr string) error { if npmLiteToggle && portKind == namedPortType { - return ErrUnsupportedNonCIDR + return fmt.Errorf("named port not supported in policy %s (namespace: %s, direction: %s, cidr: %s, port: %v, protocol: %v): %w", + npmNetPol.PolicyKey, npmNetPol.Namespace, direction, cidr, port.Port, port.Protocol, ErrUnsupportedNamedPort) } return nil } @@ -673,7 +714,7 @@ func checkOnlyPortRuleExists( if err != nil { return err } - err = checkForNamedPortType(portKind, npmLiteToggle) + err = checkForNamedPortType(npmNetPol, portKind, npmLiteToggle, direction, &ports[i], "") if err != nil { return err } diff --git a/npm/pkg/controlplane/translation/translatePolicy_test.go b/npm/pkg/controlplane/translation/translatePolicy_test.go index dc49c7bec3..634191a2a8 100644 --- a/npm/pkg/controlplane/translation/translatePolicy_test.go +++ b/npm/pkg/controlplane/translation/translatePolicy_test.go @@ -1458,6 +1458,195 @@ func TestPeerAndPortRule(t *testing.T) { } } +func TestDirectPeerAndPortAllowRule(t *testing.T) { + namedPort := intstr.FromString(namedPortStr) + port8000 := intstr.FromInt(8000) + var endPort int32 = 8100 + tcp := v1.ProtocolTCP + + tests := []struct { + name string + direction policies.Direction + ports []networkingv1.NetworkPolicyPort + cidr string + npmNetPol *policies.NPMNetworkPolicy + skipWindows bool + }{ + { + name: "egress tcp port 8000-8100 with /28 subnet", + direction: policies.Egress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &tcp, + Port: &port8000, + EndPort: &endPort, + }, + }, + cidr: "10.0.1.0/28", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstDirectIPs: []string{"10.0.1.0/28"}, + DstPorts: policies.Ports{ + Port: 8000, + EndPort: 8100, + }, + Protocol: "TCP", + }, + }, + }, + }, + { + name: "ingress no ports - single IP (/32)", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{}, + cidr: "10.226.0.49/32", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcDirectIPs: []string{"10.226.0.49/32"}, + }, + }, + }, + }, + { + name: "egress no ports - subnet (/24)", + direction: policies.Egress, + ports: []networkingv1.NetworkPolicyPort{}, + cidr: "192.168.1.0/24", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstDirectIPs: []string{"192.168.1.0/24"}, + }, + }, + }, + }, + { + name: "ingress no ports - large subnet (/16)", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{}, + cidr: "172.16.0.0/16", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcDirectIPs: []string{"172.16.0.0/16"}, + }, + }, + }, + }, + { + name: "egress tcp port 8000-8100 with /28 subnet", + direction: policies.Egress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &tcp, + Port: &port8000, + EndPort: &endPort, + }, + }, + cidr: "10.0.1.0/28", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Egress, + DstDirectIPs: []string{"10.0.1.0/28"}, + DstPorts: policies.Ports{ + Port: 8000, + EndPort: 8100, + }, + Protocol: "TCP", + }, + }, + }, + }, + { + name: "ingress udp port 53 with /32", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &[]v1.Protocol{v1.ProtocolUDP}[0], + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 53}, + }, + }, + cidr: "8.8.8.8/32", + npmNetPol: &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + ACLs: []*policies.ACLPolicy{ + { + Target: policies.Allowed, + Direction: policies.Ingress, + SrcDirectIPs: []string{"8.8.8.8/32"}, + DstPorts: policies.Ports{ + Port: 53, + EndPort: 0, + }, + Protocol: "UDP", + }, + }, + }, + }, + { + name: "named port should fail in NPM Lite", + direction: policies.Ingress, + ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &tcp, + Port: &namedPort, + }, + }, + cidr: "10.226.0.49/32", + skipWindows: true, // Should fail on both platforms + }, + } + + for _, tt := range tests { + tt := tt + npmLiteToggle := true + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + npmNetPol := &policies.NPMNetworkPolicy{ + Namespace: defaultNS, + PolicyKey: namedPortPolicyKey, + ACLPolicyID: fmt.Sprintf("azure-acl-%s-%s", defaultNS, namedPortPolicyKey), + } + err := directPeerAndPortAllowRule(npmNetPol, tt.direction, tt.ports, tt.cidr, npmLiteToggle) + if tt.skipWindows { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, tt.npmNetPol, npmNetPol) + } + }) + } +} + func TestIngressPolicy(t *testing.T) { tcp := v1.ProtocolTCP targetPodMatchType := policies.EitherMatch @@ -2921,169 +3110,6 @@ func TestEgressPolicy(t *testing.T) { } } -func TestNpmLiteCidrPolicy(t *testing.T) { - // Test 1) Npm lite enabled, CIDR + Namespace label Peers, returns error - // Test 2) NPM lite disabled, CIDR + Namespace label Peers, returns no error - // Test 3) Npm Lite enabled, CIDR Peers , returns no error - // Test 4) NPM Lite enabled, Combination of CIDR + Label in same peer, returns an error - // test 5) NPM Lite enabled, no peer, returns no error - // test 6) NPM Lite enabled, no cidr, no peer, only ports + protocol - - port8000 := intstr.FromInt(8000) - tcp := v1.ProtocolTCP - tests := []struct { - name string - targetSelector *metav1.LabelSelector - ports []networkingv1.NetworkPolicyPort - peersFrom []networkingv1.NetworkPolicyPeer - peersTo []networkingv1.NetworkPolicyPeer - npmLiteEnabled bool - wantErr bool - }{ - { - name: "CIDR + port + namespace", - targetSelector: nil, - ports: []networkingv1.NetworkPolicyPort{ - { - Protocol: &tcp, - Port: &port8000, - }, - }, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "peer-nsselector-kay": "peer-nsselector-value", - }, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: true, - }, - { - name: "cidr + namespace label + disabledLite ", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "peer-nsselector-kay": "peer-nsselector-value", - }, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: false, - wantErr: false, - }, - { - name: "CIDR Only", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - }, - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/16", - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: false, - }, - { - name: "CIDR + namespace labels", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{ - { - IPBlock: &networkingv1.IPBlock{ - CIDR: "172.17.0.0/17", - Except: []string{"172.17.1.0/24", "172.17.2.0/24"}, - }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "peer-nsselector-kay": "peer-nsselector-value", - }, - }, - }, - }, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: true, - }, - { - name: "no peers", - targetSelector: nil, - peersFrom: []networkingv1.NetworkPolicyPeer{}, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: false, - }, - { - name: "port only", - targetSelector: nil, - ports: []networkingv1.NetworkPolicyPort{ - { - Protocol: &tcp, - Port: &port8000, - }, - }, - peersFrom: []networkingv1.NetworkPolicyPeer{}, - peersTo: []networkingv1.NetworkPolicyPeer{}, - npmLiteEnabled: true, - wantErr: false, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - // run the function passing in peers and a flag indicating whether npm lite is enabled - var err error - for _, peer := range tt.peersFrom { - err = npmLiteValidPolicy(peer, tt.npmLiteEnabled) - if err != nil { - break - } - } - if tt.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - }) - } -} - func TestCheckForNamedPortType(t *testing.T) { port8000 := intstr.FromInt(8000) namedPort := intstr.FromString("namedPort") @@ -3127,8 +3153,28 @@ func TestCheckForNamedPortType(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - // run the function passing in peers and a flag indicating whether npm lite is enabled - err := checkForNamedPortType(tt.portKind, tt.npmLiteEnabled) + // Create a mock NPM network policy for testing + npmNetPol := &policies.NPMNetworkPolicy{ + PolicyKey: "test-policy/test", + Namespace: "test-namespace", + } + + // Use the first port from test data, or create a default one if ports are empty + var testPort *networkingv1.NetworkPolicyPort + if len(tt.ports) > 0 { + testPort = &tt.ports[0] + } else { + // Create a default port for tests without specific port data + port := intstr.FromInt(8080) + protocol := v1.ProtocolTCP + testPort = &networkingv1.NetworkPolicyPort{ + Protocol: &protocol, + Port: &port, + } + } + + // run the function passing in all required parameters + err := checkForNamedPortType(npmNetPol, tt.portKind, tt.npmLiteEnabled, policies.Ingress, testPort, "10.0.0.0/24") if tt.wantErr { require.Error(t, err) } else { diff --git a/npm/pkg/dataplane/policies/policy.go b/npm/pkg/dataplane/policies/policy.go index 646a03633a..d36b3530c5 100644 --- a/npm/pkg/dataplane/policies/policy.go +++ b/npm/pkg/dataplane/policies/policy.go @@ -114,6 +114,10 @@ type ACLPolicy struct { SrcList []SetInfo // DstList destination IPSets condition setinfos DstList []SetInfo + // SrcDirectIPs holds direct IPs for source matching (used for /32 CIDRs on Windows with npm lite enabled) + SrcDirectIPs []string + // DstDirectIPs holds direct IPs for destination matching (used for /32 CIDRs on Windows with npm lite enabled) + DstDirectIPs []string // Target defines a target in iptables for linux. i,e, Mark, Accept, Drop // in windows, this is either ALLOW or DENY Target Verdict @@ -282,7 +286,9 @@ func translatedIPSetsToString(items []*ipsets.TranslatedIPSet) string { // Included is false when match set have "!". // MatchType captures match direction flags. // For example match set in linux: -// ! azure-npm-123 src +// +// ! azure-npm-123 src +// // "!" this indicates a negative match (Included is false) of an azure-npm-123 // MatchType is "src" type SetInfo struct { diff --git a/npm/pkg/dataplane/policies/policy_windows.go b/npm/pkg/dataplane/policies/policy_windows.go index 5f4fbd16ff..9e23dcb92d 100644 --- a/npm/pkg/dataplane/policies/policy_windows.go +++ b/npm/pkg/dataplane/policies/policy_windows.go @@ -3,6 +3,7 @@ package policies import ( "errors" "fmt" + "strings" "github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets" "github.com/Microsoft/hcsshim/hcn" @@ -100,12 +101,7 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // Ignore adding ruletype for now as there is a bug // policySettings.RuleType = hcn.RuleTypeSwitch - // ACLPolicy settings uses ID field of SetPolicy in LocalAddresses or RemoteAddresses - srcListStr := getAddrListFromSetInfo(acl.SrcList) - dstListStr := getAddrListFromSetInfo(acl.DstList) - dstPortStr := getPortStrFromPorts(acl.DstPorts) - - // HNS has confusing Local and Remote address defintions + // HNS has confusing Local and Remote address definitions // For Traffic Direction INGRESS // LocalAddresses = Source Sets // RemoteAddresses = Destination Sets @@ -126,8 +122,28 @@ func (acl *ACLPolicy) convertToAclSettings(aclID string) (*NPMACLPolSettings, er // LocalAddresses = Destination IPs // RemoteAddresses = Source IPs - policySettings.LocalAddresses = srcListStr - policySettings.RemoteAddresses = dstListStr + var srcListStr, dstListStr string + // if direct IPs are used, we leave local addresses to be an empty string + if len(acl.SrcDirectIPs) > 0 || len(acl.DstDirectIPs) > 0 { + srcListStr = strings.Join(acl.SrcDirectIPs, ",") + dstListStr = strings.Join(acl.DstDirectIPs, ",") + policySettings.LocalAddresses = "" + if policySettings.Direction == hcn.DirectionTypeOut { + // EGRESS: Remote = Destination IPs from policy + policySettings.RemoteAddresses = dstListStr + } else { + // INGRESS: Remote = Source IPs from policy + policySettings.RemoteAddresses = srcListStr + } + } else { + // Original IPSet-based approach + srcListStr = getAddrListFromSetInfo(acl.SrcList) + dstListStr = getAddrListFromSetInfo(acl.DstList) + policySettings.LocalAddresses = srcListStr + policySettings.RemoteAddresses = dstListStr + } + + dstPortStr := getPortStrFromPorts(acl.DstPorts) // Switch ports based on direction policySettings.RemotePorts = ""