drop both IPv4 and IPv6 traffic in networkpolicy drop acl (#3940)

* drop both IPv4 and IPv6 traffic in networkpolicy drop acl Signed-off-by: 马洪贞 <[email protected]> * exec mockgen cmd Signed-off-by: 马洪贞 <[email protected]> --------- Signed-off-by: 马洪贞 <[email protected]>
kubeovn · Apr 22, 2024 · cd9df5d · cd9df5d
1 parent cb6f229
commit cd9df5d
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 79 deletions.
diff --git a/mocks/pkg/ovs/interface.go b/mocks/pkg/ovs/interface.go
diff --git a/pkg/controller/network_policy.go b/pkg/controller/network_policy.go
@@ -298,17 +298,16 @@ func (c *Controller) handleUpdateNp(key string) error {
 
 					ingressACLOps = append(ingressACLOps, ops...)
 				}
-
-				if err = c.OVNNbClient.Transact("add-ingress-acls", ingressACLOps); err != nil {
-					return fmt.Errorf("add ingress acls to %s: %v", pgName, err)
-				}
-
-				if err = c.OVNNbClient.SetACLLog(pgName, protocol, logEnable, true); err != nil {
-					// just log and do not return err here
-					klog.Errorf("failed to set ingress acl log for np %s, %v", key, err)
-				}
 			}
 		}
+		if err := c.OVNNbClient.Transact("add-ingress-acls", ingressACLOps); err != nil {
+			return fmt.Errorf("add ingress acls to %s: %v", pgName, err)
+		}
+
+		if err := c.OVNNbClient.SetACLLog(pgName, logEnable, true); err != nil {
+			// just log and do not return err here
+			klog.Errorf("failed to set ingress acl log for np %s, %v", key, err)
+		}
 
 		ass, err := c.OVNNbClient.ListAddressSets(map[string]string{
 			networkPolicyKey: fmt.Sprintf("%s/%s/%s", np.Namespace, npName, "ingress"),
@@ -427,16 +426,16 @@ func (c *Controller) handleUpdateNp(key string) error {
 					egressACLOps = append(egressACLOps, ops...)
 				}
 
-				if err = c.OVNNbClient.Transact("add-egress-acls", egressACLOps); err != nil {
-					return fmt.Errorf("add egress acls to %s: %v", pgName, err)
-				}
-
-				if err = c.OVNNbClient.SetACLLog(pgName, protocol, logEnable, false); err != nil {
-					// just log and do not return err here
-					klog.Errorf("failed to set egress acl log for np %s, %v", key, err)
-				}
 			}
 		}
+		if err := c.OVNNbClient.Transact("add-egress-acls", egressACLOps); err != nil {
+			return fmt.Errorf("add egress acls to %s: %v", pgName, err)
+		}
+
+		if err := c.OVNNbClient.SetACLLog(pgName, logEnable, false); err != nil {
+			// just log and do not return err here
+			klog.Errorf("failed to set egress acl log for np %s, %v", key, err)
+		}
 
 		ass, err := c.OVNNbClient.ListAddressSets(map[string]string{
 			networkPolicyKey: fmt.Sprintf("%s/%s/%s", np.Namespace, npName, "egress"),

diff --git a/pkg/ovs/interface.go b/pkg/ovs/interface.go
@@ -144,7 +144,7 @@ type ACL interface {
 	CreateSgBaseACL(sgName, direction string) error
 	UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string) error
 	UpdateLogicalSwitchACL(lsName, cidrBlock string, subnetAcls []kubeovnv1.ACL, allowEWTraffic bool) error
-	SetACLLog(pgName, protocol string, logEnable, isIngress bool) error
+	SetACLLog(pgName string, logEnable, isIngress bool) error
 	SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error
 	SGLostACL(sg *kubeovnv1.SecurityGroup) (bool, error)
 	DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error

diff --git a/pkg/ovs/ovn-nb-acl.go b/pkg/ovs/ovn-nb-acl.go
@@ -25,15 +25,10 @@ func (c *OVNNbClient) UpdateIngressACLOps(pgName, asIngressName, asExceptName, p
 
 	if strings.HasSuffix(asIngressName, ".0") || strings.HasSuffix(asIngressName, ".all") {
 		// create the default drop rule for only once
-		ipSuffix := "ip4"
-		if protocol == kubeovnv1.ProtocolIPv6 {
-			ipSuffix = "ip6"
-		}
-
-		/* default drop acl */
+		// both IPv4 and IPv6 traffic should be forbade in dual-stack situation
 		allIPMatch := NewAndACLMatch(
 			NewACLMatch("outport", "==", "@"+pgName, ""),
-			NewACLMatch(ipSuffix, "", "", ""),
+			NewACLMatch("ip", "", "", ""),
 		)
 		options := func(acl *ovnnb.ACL) {
 			if logEnable {
@@ -75,15 +70,10 @@ func (c *OVNNbClient) UpdateEgressACLOps(pgName, asEgressName, asExceptName, pro
 
 	if strings.HasSuffix(asEgressName, ".0") || strings.HasSuffix(asEgressName, ".all") {
 		// create the default drop rule for only once
-		ipSuffix := "ip4"
-		if protocol == kubeovnv1.ProtocolIPv6 {
-			ipSuffix = "ip6"
-		}
-
-		/* default drop acl */
+		// both IPv4 and IPv6 traffic should be forbade in dual-stack situation
 		allIPMatch := NewAndACLMatch(
 			NewACLMatch("inport", "==", "@"+pgName, ""),
-			NewACLMatch(ipSuffix, "", "", ""),
+			NewACLMatch("ip", "", "", ""),
 		)
 		options := func(acl *ovnnb.ACL) {
 			if logEnable {
@@ -621,23 +611,18 @@ func (c *OVNNbClient) SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR
 	return nil
 }
 
-func (c *OVNNbClient) SetACLLog(pgName, protocol string, logEnable, isIngress bool) error {
+func (c *OVNNbClient) SetACLLog(pgName string, logEnable, isIngress bool) error {
 	direction := ovnnb.ACLDirectionToLport
 	portDirection := "outport"
 	if !isIngress {
 		direction = ovnnb.ACLDirectionFromLport
 		portDirection = "inport"
 	}
 
-	ipSuffix := "ip4"
-	if protocol == kubeovnv1.ProtocolIPv6 {
-		ipSuffix = "ip6"
-	}
-
 	// match all traffic to or from pgName
 	allIPMatch := NewAndACLMatch(
 		NewACLMatch(portDirection, "==", "@"+pgName, ""),
-		NewACLMatch(ipSuffix, "", "", ""),
+		NewACLMatch("ip", "", "", ""),
 	)
 
 	acl, err := c.GetACL(pgName, direction, util.IngressDefaultDrop, allIPMatch.String(), true)
@@ -650,6 +635,9 @@ func (c *OVNNbClient) SetACLLog(pgName, protocol string, logEnable, isIngress bo
 		return nil // skip if acl not found
 	}
 
+	if acl.Log == logEnable {
+		return nil
+	}
 	acl.Log = logEnable
 
 	err = c.UpdateACL(acl, &acl.Log)

diff --git a/pkg/ovs/ovn-nb-acl_test.go b/pkg/ovs/ovn-nb-acl_test.go
@@ -94,7 +94,7 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() {
 		require.NoError(t, err)
 		require.Len(t, ops, 4)
 
-		expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip4", pgName), util.IngressDefaultDrop)
+		expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop)
 
 		matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, npp, nil)
 		i := 1
@@ -120,7 +120,7 @@ func (suite *OvnClientTestSuite) testUpdateIngressACLOps() {
 		require.NoError(t, err)
 		require.Len(t, ops, 3)
 
-		expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip6", pgName), util.IngressDefaultDrop)
+		expect(ops[0].Row, "drop", ovnnb.ACLDirectionToLport, fmt.Sprintf("outport == @%s && ip", pgName), util.IngressDefaultDrop)
 
 		matches := newNetworkPolicyACLMatch(pgName, asIngressName, asExceptName, protocol, ovnnb.ACLDirectionToLport, nil, nil)
 		i := 1
@@ -164,7 +164,7 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() {
 		require.NoError(t, err)
 		require.Len(t, ops, 4)
 
-		expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip4", pgName), util.EgressDefaultDrop)
+		expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop)
 
 		matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, npp, nil)
 		i := 1
@@ -190,7 +190,7 @@ func (suite *OvnClientTestSuite) testUpdateEgressACLOps() {
 		require.NoError(t, err)
 		require.Len(t, ops, 3)
 
-		expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip6", pgName), util.EgressDefaultDrop)
+		expect(ops[0].Row, "drop", ovnnb.ACLDirectionFromLport, fmt.Sprintf("inport == @%s && ip", pgName), util.EgressDefaultDrop)
 
 		matches := newNetworkPolicyACLMatch(pgName, asEgressName, asExceptName, protocol, ovnnb.ACLDirectionFromLport, nil, nil)
 		i := 1
@@ -719,7 +719,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
 	require.NoError(t, err)
 
 	t.Run("set ingress acl log to false", func(t *testing.T) {
-		match := fmt.Sprintf("outport == @%s && ip4", pgName)
+		match := fmt.Sprintf("outport == @%s && ip", pgName)
 		acl := newACL(pgName, ovnnb.ACLDirectionToLport, util.IngressDefaultDrop, match, ovnnb.ACLActionDrop, func(acl *ovnnb.ACL) {
 			acl.Name = &pgName
 			acl.Log = true
@@ -729,7 +729,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
 		err = ovnClient.CreateAcls(pgName, portGroupKey, acl)
 		require.NoError(t, err)
 
-		err = ovnClient.SetACLLog(pgName, kubeovnv1.ProtocolIPv4, false, true)
+		err = ovnClient.SetACLLog(pgName, false, true)
 		require.NoError(t, err)
 
 		acl, err = ovnClient.GetACL(pgName, ovnnb.ACLDirectionToLport, util.IngressDefaultDrop, match, false)
@@ -738,7 +738,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
 	})
 
 	t.Run("set egress acl log to false", func(t *testing.T) {
-		match := fmt.Sprintf("inport == @%s && ip4", pgName)
+		match := fmt.Sprintf("inport == @%s && ip", pgName)
 		acl := newACL(pgName, ovnnb.ACLDirectionFromLport, util.IngressDefaultDrop, match, ovnnb.ACLActionDrop, func(acl *ovnnb.ACL) {
 			acl.Name = &pgName
 			acl.Log = false
@@ -748,7 +748,7 @@ func (suite *OvnClientTestSuite) testSetACLLog() {
 		err = ovnClient.CreateAcls(pgName, portGroupKey, acl)
 		require.NoError(t, err)
 
-		err = ovnClient.SetACLLog(pgName, kubeovnv1.ProtocolIPv4, true, false)
+		err = ovnClient.SetACLLog(pgName, true, false)
 		require.NoError(t, err)
 
 		acl, err = ovnClient.GetACL(pgName, ovnnb.ACLDirectionFromLport, util.IngressDefaultDrop, match, false)