From ef321d42a7063c2f8c5b56cef5ed1421c1591cd2 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 --- build/yamls/antrea.yml | 2 +- ci/kind/config-3nodes.yml | 2 +- pkg/agent/openflow/client.go | 4 -- pkg/agent/openflow/client_test.go | 26 +++++++--- pkg/agent/openflow/pipeline.go | 24 +++++---- pkg/agent/openflow/pod_connectivity.go | 2 +- pkg/agent/openflow/pod_connectivity_test.go | 56 +++++++++++++++------ 7 files changed, 76 insertions(+), 40 deletions(-) diff --git a/build/yamls/antrea.yml b/build/yamls/antrea.yml index d3c980a7ff8..da9a24e06d0 100644 --- a/build/yamls/antrea.yml +++ b/build/yamls/antrea.yml @@ -5692,7 +5692,7 @@ data: # the PSK value must be passed to Antrea Agent through an environment # variable: ANTREA_IPSEC_PSK. # - wireGuard: Enable WireGuard for tunnel traffic encryption. - trafficEncryptionMode: "none" + trafficEncryptionMode: "wireGuard" # Enable bridging mode of Pod network on Nodes, in which the Node's transport interface is connected # to the OVS bridge, and cross-Node/VLAN traffic of AntreaIPAM Pods (Pods whose IP addresses are diff --git a/ci/kind/config-3nodes.yml b/ci/kind/config-3nodes.yml index 7e35238b73a..a73c193abc1 100644 --- a/ci/kind/config-3nodes.yml +++ b/ci/kind/config-3nodes.yml @@ -1,8 +1,8 @@ kind: Cluster apiVersion: kind.x-k8s.io/v1alpha4 networking: + ipFamily: dual disableDefaultCNI: true - podSubnet: 10.10.0.0/16 nodes: - role: control-plane - role: worker 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..756b8e09326 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -1087,17 +1087,19 @@ 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. 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 {