From 7e2e0f52dba33412862e56496ddf995ba27bfa54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A9=AC=E6=B4=AA=E8=B4=9E?= Date: Fri, 19 Apr 2024 18:30:59 +0800 Subject: [PATCH 1/2] drop both IPv4 and IPv6 traffic in networkpolicy drop acl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 马洪贞 --- pkg/controller/network_policy.go | 33 ++++++++++++++++---------------- pkg/ovs/interface.go | 2 +- pkg/ovs/ovn-nb-acl.go | 30 +++++++++-------------------- pkg/ovs/ovn-nb-acl_test.go | 16 ++++++++-------- 4 files changed, 34 insertions(+), 47 deletions(-) diff --git a/pkg/controller/network_policy.go b/pkg/controller/network_policy.go index d9c4ee768c4..e1c53c8662b 100644 --- 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 index 091e9e884e1..1bb1b104299 100644 --- 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 index 30407b9c21f..3b9948a0776 100644 --- 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,7 +611,7 @@ 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 { @@ -629,15 +619,10 @@ func (c *OVNNbClient) SetACLLog(pgName, protocol string, logEnable, isIngress bo 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 index b1f5b03deb7..718f366dcfe 100644 --- 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) From 20bf3ad9430a97239cd48c90235ddaaa849a3343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A9=AC=E6=B4=AA=E8=B4=9E?= Date: Mon, 22 Apr 2024 14:42:35 +0800 Subject: [PATCH 2/2] exec mockgen cmd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 马洪贞 --- mocks/pkg/ovs/interface.go | 64 +++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/mocks/pkg/ovs/interface.go b/mocks/pkg/ovs/interface.go index 23a133dfb45..ec4550df6d1 100644 --- a/mocks/pkg/ovs/interface.go +++ b/mocks/pkg/ovs/interface.go @@ -102,32 +102,32 @@ func (mr *MockNBGlobalMockRecorder) SetLsCtSkipDstLportIPs(enabled any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLsCtSkipDstLportIPs", reflect.TypeOf((*MockNBGlobal)(nil).SetLsCtSkipDstLportIPs), enabled) } -// SetNodeLocalDNSIP mocks base method. -func (m *MockNBGlobal) SetNodeLocalDNSIP(nodeLocalDNSIP string) error { +// SetLsDnatModDlDst mocks base method. +func (m *MockNBGlobal) SetLsDnatModDlDst(enabled bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetNodeLocalDNSIP", nodeLocalDNSIP) + ret := m.ctrl.Call(m, "SetLsDnatModDlDst", enabled) ret0, _ := ret[0].(error) return ret0 } -// SetNodeLocalDNSIP indicates an expected call of SetNodeLocalDNSIP. -func (mr *MockNBGlobalMockRecorder) SetNodeLocalDNSIP(nodeLocalDNSIP any) *gomock.Call { +// SetLsDnatModDlDst indicates an expected call of SetLsDnatModDlDst. +func (mr *MockNBGlobalMockRecorder) SetLsDnatModDlDst(enabled any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetNodeLocalDNSIP", reflect.TypeOf((*MockNBGlobal)(nil).SetNodeLocalDNSIP), nodeLocalDNSIP) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLsDnatModDlDst", reflect.TypeOf((*MockNBGlobal)(nil).SetLsDnatModDlDst), enabled) } -// SetLsDnatModDlDst mocks base method. -func (m *MockNBGlobal) SetLsDnatModDlDst(enabled bool) error { +// SetNodeLocalDNSIP mocks base method. +func (m *MockNBGlobal) SetNodeLocalDNSIP(nodeLocalDNSIP string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetLsDnatModDlDst", enabled) + ret := m.ctrl.Call(m, "SetNodeLocalDNSIP", nodeLocalDNSIP) ret0, _ := ret[0].(error) return ret0 } -// SetLsDnatModDlDst indicates an expected call of SetLsDnatModDlDst. -func (mr *MockNBGlobalMockRecorder) SetLsDnatModDlDst(enabled any) *gomock.Call { +// SetNodeLocalDNSIP indicates an expected call of SetNodeLocalDNSIP. +func (mr *MockNBGlobalMockRecorder) SetNodeLocalDNSIP(nodeLocalDNSIP any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLsDnatModDlDst", reflect.TypeOf((*MockNBGlobal)(nil).SetLsDnatModDlDst), enabled) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetNodeLocalDNSIP", reflect.TypeOf((*MockNBGlobal)(nil).SetNodeLocalDNSIP), nodeLocalDNSIP) } // SetUseCtInvMatch mocks base method. @@ -1728,17 +1728,17 @@ func (mr *MockACLMockRecorder) SGLostACL(sg any) *gomock.Call { } // SetACLLog mocks base method. -func (m *MockACL) SetACLLog(pgName, protocol string, logEnable, isIngress bool) error { +func (m *MockACL) SetACLLog(pgName string, logEnable, isIngress bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetACLLog", pgName, protocol, logEnable, isIngress) + ret := m.ctrl.Call(m, "SetACLLog", pgName, logEnable, isIngress) ret0, _ := ret[0].(error) return ret0 } // SetACLLog indicates an expected call of SetACLLog. -func (mr *MockACLMockRecorder) SetACLLog(pgName, protocol, logEnable, isIngress any) *gomock.Call { +func (mr *MockACLMockRecorder) SetACLLog(pgName, logEnable, isIngress any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetACLLog", reflect.TypeOf((*MockACL)(nil).SetACLLog), pgName, protocol, logEnable, isIngress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetACLLog", reflect.TypeOf((*MockACL)(nil).SetACLLog), pgName, logEnable, isIngress) } // SetLogicalSwitchPrivate mocks base method. @@ -4103,17 +4103,17 @@ func (mr *MockNbClientMockRecorder) SGLostACL(sg any) *gomock.Call { } // SetACLLog mocks base method. -func (m *MockNbClient) SetACLLog(pgName, protocol string, logEnable, isIngress bool) error { +func (m *MockNbClient) SetACLLog(pgName string, logEnable, isIngress bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetACLLog", pgName, protocol, logEnable, isIngress) + ret := m.ctrl.Call(m, "SetACLLog", pgName, logEnable, isIngress) ret0, _ := ret[0].(error) return ret0 } // SetACLLog indicates an expected call of SetACLLog. -func (mr *MockNbClientMockRecorder) SetACLLog(pgName, protocol, logEnable, isIngress any) *gomock.Call { +func (mr *MockNbClientMockRecorder) SetACLLog(pgName, logEnable, isIngress any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetACLLog", reflect.TypeOf((*MockNbClient)(nil).SetACLLog), pgName, protocol, logEnable, isIngress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetACLLog", reflect.TypeOf((*MockNbClient)(nil).SetACLLog), pgName, logEnable, isIngress) } // SetAzName mocks base method. @@ -4289,32 +4289,32 @@ func (mr *MockNbClientMockRecorder) SetLsCtSkipDstLportIPs(enabled any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLsCtSkipDstLportIPs", reflect.TypeOf((*MockNbClient)(nil).SetLsCtSkipDstLportIPs), enabled) } -// SetNodeLocalDNSIP mocks base method. -func (m *MockNbClient) SetNodeLocalDNSIP(nodeLocalDNSIP string) error { +// SetLsDnatModDlDst mocks base method. +func (m *MockNbClient) SetLsDnatModDlDst(enabled bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetNodeLocalDNSIP", nodeLocalDNSIP) + ret := m.ctrl.Call(m, "SetLsDnatModDlDst", enabled) ret0, _ := ret[0].(error) return ret0 } -// SetNodeLocalDNSIP indicates an expected call of SetNodeLocalDNSIP. -func (mr *MockNbClientMockRecorder) SetNodeLocalDNSIP(nodeLocalDNSIP any) *gomock.Call { +// SetLsDnatModDlDst indicates an expected call of SetLsDnatModDlDst. +func (mr *MockNbClientMockRecorder) SetLsDnatModDlDst(enabled any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetNodeLocalDNSIP", reflect.TypeOf((*MockNbClient)(nil).SetNodeLocalDNSIP), nodeLocalDNSIP) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLsDnatModDlDst", reflect.TypeOf((*MockNbClient)(nil).SetLsDnatModDlDst), enabled) } -// SetLsDnatModDlDst mocks base method. -func (m *MockNbClient) SetLsDnatModDlDst(enabled bool) error { +// SetNodeLocalDNSIP mocks base method. +func (m *MockNbClient) SetNodeLocalDNSIP(nodeLocalDNSIP string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetLsDnatModDlDst", enabled) + ret := m.ctrl.Call(m, "SetNodeLocalDNSIP", nodeLocalDNSIP) ret0, _ := ret[0].(error) return ret0 } -// SetLsDnatModDlDst indicates an expected call of SetLsDnatModDlDst. -func (mr *MockNbClientMockRecorder) SetLsDnatModDlDst(enabled any) *gomock.Call { +// SetNodeLocalDNSIP indicates an expected call of SetNodeLocalDNSIP. +func (mr *MockNbClientMockRecorder) SetNodeLocalDNSIP(nodeLocalDNSIP any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLsDnatModDlDst", reflect.TypeOf((*MockNbClient)(nil).SetLsDnatModDlDst), enabled) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetNodeLocalDNSIP", reflect.TypeOf((*MockNbClient)(nil).SetNodeLocalDNSIP), nodeLocalDNSIP) } // SetUseCtInvMatch mocks base method.