From b5d0b102fe1f0bdcb0399d67606d1beb829d591f Mon Sep 17 00:00:00 2001 From: Antonin Bas Date: Tue, 23 Jan 2024 21:03:57 -0800 Subject: [PATCH] Remove invalid flow when WireGuard is enabled After merging #5885, the OVS tunnel port is no longer created when enabling WireGuard, as it is not being used. However, we were still trying to install flows referencing the tunnel port. Even though the port was non-existent, flow creation would succeed, but the condition matching on the tunnel port was being dropped silently. This would lead to invalid datapath behavior. We update the code to prevent installing these invalid flows when WireGuard is enabled. Fixes #5905 Signed-off-by: Antonin Bas --- pkg/agent/openflow/client.go | 4 -- pkg/agent/openflow/client_test.go | 26 +++++++--- pkg/agent/openflow/pipeline.go | 26 +++++----- pkg/agent/openflow/pod_connectivity.go | 2 +- pkg/agent/openflow/pod_connectivity_test.go | 56 +++++++++++++++------ 5 files changed, 75 insertions(+), 39 deletions(-) diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go index fe46c981b40..025d87e2d08 100644 --- a/pkg/agent/openflow/client.go +++ b/pkg/agent/openflow/client.go @@ -1625,10 +1625,6 @@ func (c *client) InstallMulticlusterClassifierFlows(tunnelOFPort uint32, isGatew c.featurePodConnectivity.l2ForwardCalcFlow(GlobalVirtualMACForMulticluster, tunnelOFPort), } - if c.networkConfig.TrafficEncapMode != config.TrafficEncapModeEncap { - flows = append(flows, c.featurePodConnectivity.tunnelClassifierFlow(tunnelOFPort)) - } - if isGateway { flows = append(flows, c.featureMulticluster.tunnelClassifierFlow(tunnelOFPort), diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go index 0dd6f5ec878..f76b589b6e9 100644 --- a/pkg/agent/openflow/client_test.go +++ b/pkg/agent/openflow/client_test.go @@ -94,6 +94,7 @@ type clientOptions struct { enableMulticluster bool enableL7NetworkPolicy bool enableL7FlowExporter bool + trafficEncryptionMode config.TrafficEncryptionModeType } type clientOptionsFn func(*clientOptions) @@ -167,6 +168,12 @@ func enableMulticluster(o *clientOptions) { o.enableMulticluster = true } +func setTrafficEncryptionMode(trafficEncryptionMode config.TrafficEncryptionModeType) clientOptionsFn { + return func(o *clientOptions) { + o.trafficEncryptionMode = trafficEncryptionMode + } +} + func installNodeFlows(ofClient Client, cacheKey string) (int, error) { gwIP, ipNet, _ := net.ParseCIDR("10.10.0.1/24") hostName := cacheKey @@ -457,9 +464,19 @@ func newFakeClientWithBridge( MAC: fakeGatewayMAC, OFPort: uint32(2), } + networkConfig := &config.NetworkConfig{ + IPv4Enabled: enableIPv4, + IPv6Enabled: enableIPv6, + TrafficEncapMode: trafficEncapMode, + TrafficEncryptionMode: o.trafficEncryptionMode, + } + tunnelOFPort := uint32(0) + if networkConfig.NeedsTunnelInterface() { + tunnelOFPort = 1 + } nodeConfig := &config.NodeConfig{ GatewayConfig: gatewayConfig, - TunnelOFPort: uint32(1), + TunnelOFPort: tunnelOFPort, WireGuardConfig: &config.WireGuardConfig{}, PodIPv4CIDR: fakePodIPv4CIDR, PodIPv6CIDR: fakePodIPv6CIDR, @@ -474,11 +491,6 @@ func newFakeClientWithBridge( OFPort: uint32(4), }, } - networkConfig := &config.NetworkConfig{ - IPv4Enabled: enableIPv4, - IPv6Enabled: enableIPv6, - TrafficEncapMode: trafficEncapMode, - } egressConfig := &config.EgressConfig{ ExceptCIDRs: egressExceptCIDRs, } @@ -2708,7 +2720,7 @@ func Test_client_ReplayFlows(t *testing.T) { expectedFlows := append(pipelineDefaultFlows(true /* egressTrafficShapingEnabled */, false /* externalNodeEnabled */, true /* isEncap */, true /* isIPv4 */), egressInitFlows(true)...) expectedFlows = append(expectedFlows, multicastInitFlows(true)...) expectedFlows = append(expectedFlows, networkPolicyInitFlows(true, false, false)...) - expectedFlows = append(expectedFlows, podConnectivityInitFlows(config.TrafficEncapModeEncap, false, true, true, true)...) + expectedFlows = append(expectedFlows, podConnectivityInitFlows(config.TrafficEncapModeEncap, config.TrafficEncryptionModeNone, false, true, true, true)...) expectedFlows = append(expectedFlows, serviceInitFlows(true, true, false, false)...) addFlowInCache := func(cache *flowCategoryCache, cacheKey string, flows []binding.Flow) { diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go index d6247594ce4..f536e51959b 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -1087,21 +1087,23 @@ func (f *featurePodConnectivity) flowsToTrace(dataplaneTag uint8, // L2 forwarding calculation. for _, ipProtocol := range f.ipProtocols { if f.networkConfig.TrafficEncapMode.SupportsEncap() { - // SendToController and Output if output port is tunnel port. - fb := OutputTable.ofTable.BuildFlow(priorityNormal+3). - Cookie(cookieID). - MatchRegFieldWithValue(TargetOFPortField, f.tunnelPort). - MatchProtocol(ipProtocol). - MatchRegMark(OutputToOFPortRegMark). - MatchIPDSCP(dataplaneTag). - SetHardTimeout(timeout). - Action().OutputToRegField(TargetOFPortField) - fb = ifDroppedOnly(fb) - flows = append(flows, fb.Done()) + if f.tunnelPort != 0 { + // SendToController and Output if output port is tunnel port. + fb := OutputTable.ofTable.BuildFlow(priorityNormal+3). + Cookie(cookieID). + MatchRegFieldWithValue(TargetOFPortField, f.tunnelPort). + MatchProtocol(ipProtocol). + MatchRegMark(OutputToOFPortRegMark). + MatchIPDSCP(dataplaneTag). + SetHardTimeout(timeout). + Action().OutputToRegField(TargetOFPortField) + fb = ifDroppedOnly(fb) + flows = append(flows, fb.Done()) + } // For injected packets, only SendToController if output port is local gateway. In encapMode, a Traceflow // packet going out of the gateway port (i.e. exiting the overlay) essentially means that the Traceflow // request is complete. - fb = OutputTable.ofTable.BuildFlow(priorityNormal+2). + fb := OutputTable.ofTable.BuildFlow(priorityNormal+2). Cookie(cookieID). MatchRegFieldWithValue(TargetOFPortField, f.gatewayPort). MatchProtocol(ipProtocol). diff --git a/pkg/agent/openflow/pod_connectivity.go b/pkg/agent/openflow/pod_connectivity.go index 856ca571785..2b1fe44c1e7 100644 --- a/pkg/agent/openflow/pod_connectivity.go +++ b/pkg/agent/openflow/pod_connectivity.go @@ -168,7 +168,7 @@ func (f *featurePodConnectivity) initFlows() []*openflow15.FlowMod { // Add flow to ensure the liveliness check packet could be forwarded correctly. flows = append(flows, f.localProbeFlows()...) - if f.networkConfig.TrafficEncapMode.SupportsEncap() { + if f.tunnelPort != 0 { flows = append(flows, f.tunnelClassifierFlow(f.tunnelPort)) flows = append(flows, f.l2ForwardCalcFlow(GlobalVirtualMAC, f.tunnelPort)) } diff --git a/pkg/agent/openflow/pod_connectivity_test.go b/pkg/agent/openflow/pod_connectivity_test.go index 916cec44016..e80a821c51d 100644 --- a/pkg/agent/openflow/pod_connectivity_test.go +++ b/pkg/agent/openflow/pod_connectivity_test.go @@ -23,7 +23,11 @@ import ( "antrea.io/antrea/pkg/util/runtime" ) -func podConnectivityInitFlows(trafficEncapMode config.TrafficEncapModeType, connectUplinkToBridge, isIPv4, trafficControlEnabled, multicastEnabled bool) []string { +func podConnectivityInitFlows( + trafficEncapMode config.TrafficEncapModeType, + trafficEncryptionMode config.TrafficEncryptionModeType, + connectUplinkToBridge, isIPv4, trafficControlEnabled, multicastEnabled bool, +) []string { var flows []string switch trafficEncapMode { case config.TrafficEncapModeEncap: @@ -59,7 +63,6 @@ func podConnectivityInitFlows(trafficEncapMode config.TrafficEncapModeType, conn "cookie=0x1010000000000, table=ARPResponder, priority=190,arp actions=NORMAL", "cookie=0x1010000000000, table=Classifier, priority=210,ip,in_port=2,nw_src=10.10.0.1 actions=set_field:0x2/0xf->reg0,goto_table:SpoofGuard", "cookie=0x1010000000000, table=Classifier, priority=200,in_port=2 actions=set_field:0x2/0xf->reg0,set_field:0x8000000/0x8000000->reg4,goto_table:SpoofGuard", - "cookie=0x1010000000000, table=Classifier, priority=200,in_port=1 actions=set_field:0x1/0xf->reg0,set_field:0x200/0x200->reg0,goto_table:UnSNAT", "cookie=0x1010000000000, table=ConntrackZone, priority=200,ip actions=ct(table=ConntrackState,zone=65520,nat)", "cookie=0x1010000000000, table=ConntrackState, priority=200,ct_state=+inv+trk,ip actions=drop", "cookie=0x1010000000000, table=ConntrackState, priority=190,ct_state=-new+trk,ct_mark=0x0/0x10,ip actions=goto_table:AntreaPolicyEgressRule", @@ -73,6 +76,11 @@ func podConnectivityInitFlows(trafficEncapMode config.TrafficEncapModeType, conn "cookie=0x1010000000000, table=ConntrackCommit, priority=200,ct_state=+new+trk-snat,ct_mark=0x0/0x10,ip actions=ct(commit,table=Output,zone=65520,exec(move:NXM_NX_REG0[0..3]->NXM_NX_CT_MARK[0..3]))", "cookie=0x1010000000000, table=Output, priority=200,reg0=0x200000/0x600000 actions=output:NXM_NX_REG1[]", } + if trafficEncryptionMode != config.TrafficEncryptionModeWireGuard { + flows = append(flows, + "cookie=0x1010000000000, table=Classifier, priority=200,in_port=1 actions=set_field:0x1/0xf->reg0,set_field:0x200/0x200->reg0,goto_table:UnSNAT", + ) + } if !multicastEnabled { flows = append(flows, "cookie=0x1010000000000, table=SpoofGuard, priority=200,ip,in_port=2 actions=goto_table:UnSNAT") } else { @@ -94,16 +102,18 @@ func podConnectivityInitFlows(trafficEncapMode config.TrafficEncapModeType, conn if trafficControlEnabled { flows = append(flows, "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=0a:00:00:00:00:01 actions=set_field:0x2->reg1,set_field:0x200000/0x600000->reg0,goto_table:TrafficControl", - "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=aa:bb:cc:dd:ee:ff actions=set_field:0x1->reg1,set_field:0x200000/0x600000->reg0,goto_table:TrafficControl", "cookie=0x1010000000000, table=TrafficControl, priority=210,reg0=0x200006/0x60000f actions=goto_table:Output", "cookie=0x1010000000000, table=Output, priority=211,reg0=0x200000/0x600000,reg4=0x400000/0xc00000 actions=output:NXM_NX_REG1[],output:NXM_NX_REG9[]", "cookie=0x1010000000000, table=Output, priority=211,reg0=0x200000/0x600000,reg4=0x800000/0xc00000 actions=output:NXM_NX_REG9[]", ) + if trafficEncryptionMode != config.TrafficEncryptionModeWireGuard { + flows = append(flows, "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=aa:bb:cc:dd:ee:ff actions=set_field:0x1->reg1,set_field:0x200000/0x600000->reg0,goto_table:TrafficControl") + } } else { - flows = append(flows, - "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=0a:00:00:00:00:01 actions=set_field:0x2->reg1,set_field:0x200000/0x600000->reg0,goto_table:IngressSecurityClassifier", - "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=aa:bb:cc:dd:ee:ff actions=set_field:0x1->reg1,set_field:0x200000/0x600000->reg0,goto_table:IngressSecurityClassifier", - ) + flows = append(flows, "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=0a:00:00:00:00:01 actions=set_field:0x2->reg1,set_field:0x200000/0x600000->reg0,goto_table:IngressSecurityClassifier") + if trafficEncryptionMode != config.TrafficEncryptionModeWireGuard { + flows = append(flows, "cookie=0x1010000000000, table=L2ForwardingCalc, priority=200,dl_dst=aa:bb:cc:dd:ee:ff actions=set_field:0x1->reg1,set_field:0x200000/0x600000->reg0,goto_table:IngressSecurityClassifier") + } } case config.TrafficEncapModeNoEncap: if !isIPv4 { @@ -284,13 +294,13 @@ func Test_featurePodConnectivity_initFlows(t *testing.T) { name: "IPv4 Encap", enableIPv4: true, trafficEncapMode: config.TrafficEncapModeEncap, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, false, true, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, config.TrafficEncryptionModeNone, false, true, false, false), }, { name: "IPv4 NoEncap", enableIPv4: true, trafficEncapMode: config.TrafficEncapModeNoEncap, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNoEncap, false, true, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNoEncap, config.TrafficEncryptionModeNone, false, true, false, false), }, { name: "IPv4 NoEncap with Antrea IPAM", @@ -298,35 +308,35 @@ func Test_featurePodConnectivity_initFlows(t *testing.T) { skipWindows: true, trafficEncapMode: config.TrafficEncapModeNoEncap, clientOptions: []clientOptionsFn{enableConnectUplinkToBridge}, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNoEncap, true, true, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNoEncap, config.TrafficEncryptionModeNone, true, true, false, false), }, { name: "IPv4 NetworkPolicyOnly Linux", enableIPv4: true, skipWindows: true, trafficEncapMode: config.TrafficEncapModeNetworkPolicyOnly, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNetworkPolicyOnly, false, true, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNetworkPolicyOnly, config.TrafficEncryptionModeNone, false, true, false, false), }, { name: "IPv6 Encap", enableIPv6: true, skipWindows: true, trafficEncapMode: config.TrafficEncapModeEncap, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, false, false, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, config.TrafficEncryptionModeNone, false, false, false, false), }, { name: "IPv6 NoEncap", enableIPv6: true, skipWindows: true, trafficEncapMode: config.TrafficEncapModeNoEncap, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNoEncap, false, false, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNoEncap, config.TrafficEncryptionModeNone, false, false, false, false), }, { name: "IPv6 NetworkPolicyOnly", enableIPv6: true, skipWindows: true, trafficEncapMode: config.TrafficEncapModeNetworkPolicyOnly, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNetworkPolicyOnly, false, false, false, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeNetworkPolicyOnly, config.TrafficEncryptionModeNone, false, false, false, false), }, { name: "IPv4 Encap with TrafficControl", @@ -334,7 +344,23 @@ func Test_featurePodConnectivity_initFlows(t *testing.T) { skipWindows: true, trafficEncapMode: config.TrafficEncapModeEncap, clientOptions: []clientOptionsFn{enableTrafficControl}, - expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, false, true, true, false), + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, config.TrafficEncryptionModeNone, false, true, true, false), + }, + { + name: "IPv4 Encap with WireGuard", + enableIPv4: true, + skipWindows: true, + trafficEncapMode: config.TrafficEncapModeEncap, + clientOptions: []clientOptionsFn{setTrafficEncryptionMode(config.TrafficEncryptionModeWireGuard)}, + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, config.TrafficEncryptionModeWireGuard, false, true, false, false), + }, + { + name: "IPv4 Encap with IPsec", + enableIPv4: true, + skipWindows: true, + trafficEncapMode: config.TrafficEncapModeEncap, + clientOptions: []clientOptionsFn{setTrafficEncryptionMode(config.TrafficEncryptionModeIPSec)}, + expectedFlows: podConnectivityInitFlows(config.TrafficEncapModeEncap, config.TrafficEncryptionModeIPSec, false, true, false, false), }, } for _, tc := range testCases {