From ad4f06e7cafb420864187e60389fa17474181040 Mon Sep 17 00:00:00 2001 From: Hongliang Liu Date: Wed, 13 Apr 2022 15:14:25 +0800 Subject: [PATCH 1/2] Bump antrea.io/ofnet from v0.5.5 to v0.5.7 Fix #3583 Signed-off-by: Hongliang Liu --- go.mod | 2 +- go.sum | 4 ++-- plugins/octant/go.sum | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index ad8d8289322..3d322530ca8 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.17 require ( antrea.io/libOpenflow v0.6.2 - antrea.io/ofnet v0.5.5 + antrea.io/ofnet v0.5.7 github.com/ClickHouse/clickhouse-go v1.5.1 github.com/DATA-DOG/go-sqlmock v1.5.0 github.com/Mellanox/sriovnet v1.0.2 diff --git a/go.sum b/go.sum index a73e2933519..cbb880d2140 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ antrea.io/libOpenflow v0.6.2 h1:1JMSJ7Lp7yOhKybHey9VDtRI6JuIgkhUWJBX5GIFY9I= antrea.io/libOpenflow v0.6.2/go.mod h1:CzEJZxDNAupiGxeL5VOw92PsxfyvehEAvE3PiC6gr8o= -antrea.io/ofnet v0.5.5 h1:4CLYWqQE4/XyuIUaTSqB83Zj+YnuplrlEXPvVm/r0JE= -antrea.io/ofnet v0.5.5/go.mod h1:8TJVF6MLe9/gZ/KbhGUvULs9/TxssepEaYEe+o1SEgs= +antrea.io/ofnet v0.5.7 h1:x0q0lZqp05wu01gk1+S5S15FmIpmTGPhi/z/aIDmuMw= +antrea.io/ofnet v0.5.7/go.mod h1:8TJVF6MLe9/gZ/KbhGUvULs9/TxssepEaYEe+o1SEgs= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.38.0/go.mod h1:990N+gfupTy94rShfmMCWGDn0LpTmnzTp2qbd1dvSRU= diff --git a/plugins/octant/go.sum b/plugins/octant/go.sum index ad7eab5135b..004b58dddf2 100644 --- a/plugins/octant/go.sum +++ b/plugins/octant/go.sum @@ -1,5 +1,5 @@ antrea.io/libOpenflow v0.6.2/go.mod h1:CzEJZxDNAupiGxeL5VOw92PsxfyvehEAvE3PiC6gr8o= -antrea.io/ofnet v0.5.5/go.mod h1:8TJVF6MLe9/gZ/KbhGUvULs9/TxssepEaYEe+o1SEgs= +antrea.io/ofnet v0.5.7/go.mod h1:8TJVF6MLe9/gZ/KbhGUvULs9/TxssepEaYEe+o1SEgs= bazil.org/fuse v0.0.0-20160811212531-371fbbdaa898/go.mod h1:Xbm+BRKSBEpa4q4hTSxohYNQpsxXPbPry4JJWOB3LB8= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= From 824fc53333536af1a253d51986c7295d181dfcea Mon Sep 17 00:00:00 2001 From: Hongliang Liu Date: Wed, 23 Mar 2022 16:52:59 +0800 Subject: [PATCH 2/2] Fix the issue of local probe bypassing flows on Windows When proxyAll is enabled, kube-proxy can be replaced by AntreaProxy, then Service traffic and non-Service traffic can be distinguished by ServiceCTMark and NotServiceCTMark. Service traffic with ServiceCTMark should not bypass Network Policies, and non-Service traffic generated by kubelet with NotServiceCTMark should bypass Network Policies. Signed-off-by: Hongliang Liu --- pkg/agent/openflow/client.go | 5 +-- pkg/agent/openflow/fields.go | 9 ++--- pkg/agent/openflow/pipeline.go | 47 ++++++++++++++++--------- pkg/agent/openflow/pod_connectivity.go | 5 ++- test/integration/agent/openflow_test.go | 2 +- 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go index ef097e9d778..d03f73f5662 100644 --- a/pkg/agent/openflow/client.go +++ b/pkg/agent/openflow/client.go @@ -688,7 +688,7 @@ func (c *client) InstallGatewayFlows() error { } // Add flow to ensure the liveness check packet could be forwarded correctly. - flows = append(flows, c.featurePodConnectivity.localProbeFlow(c.ovsDatapathType)...) + flows = append(flows, c.featurePodConnectivity.localProbeFlows(c.ovsDatapathType)...) flows = append(flows, c.featurePodConnectivity.l3FwdFlowToGateway()...) if err := c.ofEntryOperations.AddAll(flows); err != nil { @@ -786,7 +786,8 @@ func (c *client) generatePipelines() { c.nodeConfig, c.networkConfig, c.connectUplinkToBridge, - c.enableMulticast) + c.enableMulticast, + c.proxyAll) c.activatedFeatures = append(c.activatedFeatures, c.featurePodConnectivity) c.traceableFeatures = append(c.traceableFeatures, c.featurePodConnectivity) diff --git a/pkg/agent/openflow/fields.go b/pkg/agent/openflow/fields.go index f6e756d5f5d..2ce0c313c78 100644 --- a/pkg/agent/openflow/fields.go +++ b/pkg/agent/openflow/fields.go @@ -173,11 +173,12 @@ var ( FromGatewayCTMark = binding.NewCTMark(ConnSourceCTMarkField, gatewayVal) FromBridgeCTMark = binding.NewCTMark(ConnSourceCTMarkField, bridgeVal) - // CTMark[4]: Mark to indicate DNAT is performed on the connection for Service. - // This CT mark is used in CtZone / CtZoneV6 and SNATCtZone / SNATCtZoneV6. - ServiceCTMark = binding.NewOneBitCTMark(4) + // CTMark[4]: Marks to indicate whether DNAT is performed on the connection for Service. + // These CT marks are used in CtZone / CtZoneV6 and SNATCtZone / SNATCtZoneV6. + ServiceCTMark = binding.NewOneBitCTMark(4) + NotServiceCTMark = binding.NewOneBitZeroCTMark(4) - // CTMark[5]: Mark to indicate SNAT should be performed on the connection for Service. + // CTMark[5]: Mark to indicate SNAT is performed on the connection for Service. // This CT mark is only used in CtZone / CtZoneV6. ConnSNATCTMark = binding.NewOneBitCTMark(5) diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go index 6c8c3a79a9a..df3fee29dcb 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -2112,34 +2112,47 @@ func (f *featureNetworkPolicy) dnsPacketInFlow(conjunctionID uint32) binding.Flo Done() } -// localProbeFlow generates the flow to forward locally generated packets to ConntrackCommitTable, bypassing ingress -// rules of Network Policies. The packets are sent by kubelet to probe the liveness/readiness of local Pods. -// On Linux and when OVS kernel datapath is used, it identifies locally generated packets by matching the -// HostLocalSourceMark, otherwise it matches the source IP. The difference is because: -// 1. On Windows, kube-proxy userspace mode is used, and currently there is no way to distinguish kubelet generated -// traffic from kube-proxy proxied traffic. +// localProbeFlows generates the flows to forward locally generated request packets to stageConntrack directly, bypassing +// ingress rules of Network Policies. The packets are sent by kubelet to probe the liveness/readiness of local Pods. +// On Linux and when OVS kernel datapath is used, the probe packets are identified by matching the HostLocalSourceMark. +// On Windows or when OVS userspace (netdev) datapath is used, we need a different approach because: +// 1. On Windows, kube-proxy userspace mode is used, and currently there is no way to distinguish kubelet generated traffic +// from kube-proxy proxied traffic. // 2. pkt_mark field is not properly supported for OVS userspace (netdev) datapath. -// Note that there is a defect in the latter way that NodePort Service access by external clients will be masqueraded as -// a local gateway IP to bypass Network Policies. See https://github.com/antrea-io/antrea/issues/280. -// TODO: Fix it after replacing kube-proxy with AntreaProxy. -func (f *featurePodConnectivity) localProbeFlow(ovsDatapathType ovsconfig.OVSDatapathType) []binding.Flow { +// When proxyAll is disabled, the probe packets are identified by matching the source IP is the Antrea gateway IP; +// otherwise, the packets are identified by matching both the Antrea gateway IP and NotServiceCTMark. Note that, when +// proxyAll is disabled, currently there is no way to distinguish kubelet generated traffic from kube-proxy proxied traffic +// only by matching the Antrea gateway IP. There is a defect that NodePort Service access by external clients will be +// masqueraded as the Antrea gateway IP to bypass NetworkPolicies. See https://github.com/antrea-io/antrea/issues/280. +func (f *featurePodConnectivity) localProbeFlows(ovsDatapathType ovsconfig.OVSDatapathType) []binding.Flow { cookieID := f.cookieAllocator.Request(f.category).Raw() var flows []binding.Flow if runtime.IsWindowsPlatform() || ovsDatapathType == ovsconfig.OVSDatapathNetdev { for ipProtocol, gatewayIP := range f.gatewayIPs { + fb := IngressSecurityClassifierTable.ofTable.BuildFlow(priorityHigh). + Cookie(cookieID). + MatchProtocol(ipProtocol). + MatchCTStateRpl(false). + MatchCTStateTrk(true). + MatchSrcIP(gatewayIP) + if f.proxyAll { + fb = fb.MatchCTMark(NotServiceCTMark) + } + flows = append(flows, + fb.Action().GotoStage(stageConntrack). + Done()) + } + } else { + for _, ipProtocol := range f.ipProtocols { flows = append(flows, IngressSecurityClassifierTable.ofTable.BuildFlow(priorityHigh). Cookie(cookieID). MatchProtocol(ipProtocol). - MatchSrcIP(gatewayIP). + MatchCTStateRpl(false). + MatchCTStateTrk(true). + MatchPktMark(types.HostLocalSourceMark, &types.HostLocalSourceMark). Action().GotoStage(stageConntrack). Done()) } - } else { - flows = append(flows, IngressSecurityClassifierTable.ofTable.BuildFlow(priorityHigh). - Cookie(cookieID). - MatchPktMark(types.HostLocalSourceMark, &types.HostLocalSourceMark). - Action().GotoStage(stageConntrack). - Done()) } return flows } diff --git a/pkg/agent/openflow/pod_connectivity.go b/pkg/agent/openflow/pod_connectivity.go index 8e8354d6430..3cfde50caf8 100644 --- a/pkg/agent/openflow/pod_connectivity.go +++ b/pkg/agent/openflow/pod_connectivity.go @@ -40,6 +40,7 @@ type featurePodConnectivity struct { connectUplinkToBridge bool ctZoneSrcField *binding.RegField enableMulticast bool + proxyAll bool category cookie.Category } @@ -54,7 +55,8 @@ func newFeaturePodConnectivity( nodeConfig *config.NodeConfig, networkConfig *config.NetworkConfig, connectUplinkToBridge bool, - enableMulticast bool) *featurePodConnectivity { + enableMulticast bool, + proxyAll bool) *featurePodConnectivity { ctZones := make(map[binding.Protocol]int) gatewayIPs := make(map[binding.Protocol]net.IP) localCIDRs := make(map[binding.Protocol]net.IPNet) @@ -91,6 +93,7 @@ func newFeaturePodConnectivity( connectUplinkToBridge: connectUplinkToBridge, ctZoneSrcField: getZoneSrcField(connectUplinkToBridge), enableMulticast: enableMulticast, + proxyAll: proxyAll, category: cookie.PodConnectivity, } } diff --git a/test/integration/agent/openflow_test.go b/test/integration/agent/openflow_test.go index 834af6536fd..c148cfb04f6 100644 --- a/test/integration/agent/openflow_test.go +++ b/test/integration/agent/openflow_test.go @@ -1237,7 +1237,7 @@ func prepareGatewayFlows(gwIPs []net.IP, gwMAC net.HardwareAddr, vMAC net.Hardwa tableName: "IngressSecurityClassifier", flows: []*ofTestUtils.ExpectFlow{ { - MatchStr: fmt.Sprintf("priority=210,%s,%s=%s", ipProtoStr, nwSrcStr, gwIP.String()), + MatchStr: fmt.Sprintf("priority=210,ct_state=-rpl+trk,%s,%s=%s", ipProtoStr, nwSrcStr, gwIP.String()), ActStr: "goto_table:ConntrackCommit", }, },