From 401017e354c2e6e544dafc57aa049458c9653937 Mon Sep 17 00:00:00 2001 From: Tomas Hruby Date: Fri, 26 May 2023 12:40:34 -0700 Subject: [PATCH] [BPF] clasiffy correctly forwarded traffic Previously, anything that had source the same as the host was considered as "from host" by the policy engine and global policies with applyOnForward=false would apply to such traffic. However, for instance SNATed traffic due to nat-outgoing would be also considered "from host" eventhough it originated in a workload. This fixes it by first looking at a SEEN mark which indicates that the packet was seen by another endpoint and thus is forwarded with the exception of packets being routed via the bpf nat iface which is used by the host processes. --- felix/bpf-gpl/skb.h | 2 + felix/bpf-gpl/tc.c | 15 ++++++- felix/fv/bpf_test.go | 100 ++++++++++++++++++++++++++++++++++--------- 3 files changed, 96 insertions(+), 21 deletions(-) diff --git a/felix/bpf-gpl/skb.h b/felix/bpf-gpl/skb.h index 4eade1ef6bf..10c40ea8e72 100644 --- a/felix/bpf-gpl/skb.h +++ b/felix/bpf-gpl/skb.h @@ -172,4 +172,6 @@ static CALI_BPF_INLINE void skb_set_mark(struct __sk_buff *skb, __u32 mark) ); } +#define skb_mark_equals(skb, mask, val) (((skb)->mark & (mask)) == (val)) + #endif /* __SKB_H__ */ diff --git a/felix/bpf-gpl/tc.c b/felix/bpf-gpl/tc.c index 1ff05c8fdf5..e1e5eef5452 100644 --- a/felix/bpf-gpl/tc.c +++ b/felix/bpf-gpl/tc.c @@ -239,6 +239,19 @@ static CALI_BPF_INLINE void calico_tc_process_ct_lookup(struct cali_tc_ctx *ctx) { CALI_DEBUG("conntrack entry flags 0x%x\n", ctx->state->ct_result.flags); + /* We are forwarding a packet if it has a seen mark (that is another + * program has seen it already) and is either not routed through the + * bpfnat iface (which may be true for host traffic) or has the specific + * reasons set. + */ + bool forwarding = CALI_F_EGRESS && + skb_mark_equals(ctx->skb, CALI_SKB_MARK_SEEN_MASK, CALI_SKB_MARK_SEEN) && + (!skb_mark_equals(ctx->skb, CALI_SKB_MARK_FROM_NAT_IFACE_OUT, CALI_SKB_MARK_FROM_NAT_IFACE_OUT) || + (skb_mark_equals(ctx->skb, CALI_SKB_MARK_BYPASS_MASK, CALI_SKB_MARK_FALLTHROUGH) || + skb_mark_equals(ctx->skb, CALI_SKB_MARK_BYPASS_MASK, CALI_SKB_MARK_NAT_OUT) || + skb_mark_equals(ctx->skb, CALI_SKB_MARK_BYPASS_MASK, CALI_SKB_MARK_MASQ) || + skb_mark_equals(ctx->skb, CALI_SKB_MARK_BYPASS_MASK, CALI_SKB_MARK_SKIP_FIB))); + if (HAS_HOST_CONFLICT_PROG && (ctx->state->ct_result.flags & CALI_CT_FLAG_VIA_NAT_IF) && !(ctx->skb->mark & (CALI_SKB_MARK_FROM_NAT_IFACE_OUT | CALI_SKB_MARK_SEEN))) { @@ -467,7 +480,7 @@ static CALI_BPF_INLINE void calico_tc_process_ct_lookup(struct cali_tc_ctx *ctx) } } - if (rt_addr_is_local_host(ctx->state->ip_src)) { + if (!forwarding && rt_addr_is_local_host(ctx->state->ip_src)) { CALI_DEBUG("Source IP is local host.\n"); if (CALI_F_TO_HEP && is_failsafe_out(ctx->state->ip_proto, ctx->state->post_nat_dport, ctx->state->post_nat_ip_dst)) { CALI_DEBUG("Outbound failsafe port: %d. Skip policy.\n", ctx->state->post_nat_dport); diff --git a/felix/fv/bpf_test.go b/felix/fv/bpf_test.go index 344a8b9c1ef..392b8ba9c46 100644 --- a/felix/fv/bpf_test.go +++ b/felix/fv/bpf_test.go @@ -1124,30 +1124,90 @@ func describeBPFTests(opts ...bpfTestOpt) bool { _ = k8sClient }) - It("should handle NAT outgoing", func() { - By("SNATting outgoing traffic with the flag set") - cc.ExpectSNAT(w[0][0], felixes[0].IP, hostW[1]) - cc.CheckConnectivity(conntrackChecks(felixes)...) + Context("with both applyOnForward=true/false", func() { + BeforeEach(func() { + // The next two policies are to make sure that applyOnForward of a + // global policy is applied correctly to a host endpoint. The deny + // policy is not applied to forwarded traffic! - if testOpts.tunnel == "none" { - By("Leaving traffic alone with the flag clear") - pool, err := calicoClient.IPPools().Get(context.TODO(), "test-pool", options2.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - pool.Spec.NATOutgoing = false - pool, err = calicoClient.IPPools().Update(context.TODO(), pool, options2.SetOptions{}) - Expect(err).NotTo(HaveOccurred()) - cc.ResetExpectations() - cc.ExpectSNAT(w[0][0], w[0][0].IP, hostW[1]) - cc.CheckConnectivity(conntrackChecks(felixes)...) + By("global policy denies traffic to host 1 on host 0", func() { - By("SNATting again with the flag set") - pool.Spec.NATOutgoing = true - pool, err = calicoClient.IPPools().Update(context.TODO(), pool, options2.SetOptions{}) - Expect(err).NotTo(HaveOccurred()) - cc.ResetExpectations() + nets := []string{felixes[1].IP + "/32"} + switch testOpts.tunnel { + case "ipip": + nets = append(nets, felixes[1].ExpectedIPIPTunnelAddr+"/32") + } + + pol := api.NewGlobalNetworkPolicy() + pol.Namespace = "fv" + pol.Name = "host-0-1" + pol.Spec.Egress = []api.Rule{ + { + Action: "Deny", + Destination: api.EntityRule{ + Nets: nets, + }, + }, + } + pol.Spec.Selector = "node=='" + felixes[0].Name + "'" + pol.Spec.ApplyOnForward = false + + pol = createPolicy(pol) + }) + + By("global policy allows forwarded traffic to host 1 on host 0", func() { + + nets := []string{felixes[1].IP + "/32"} + switch testOpts.tunnel { + case "ipip": + nets = append(nets, felixes[1].ExpectedIPIPTunnelAddr+"/32") + } + + pol := api.NewGlobalNetworkPolicy() + pol.Namespace = "fv" + pol.Name = "host-0-1-forward" + pol.Spec.Egress = []api.Rule{ + { + Action: "Allow", + Destination: api.EntityRule{ + Nets: nets, + }, + }, + } + pol.Spec.Selector = "node=='" + felixes[0].Name + "'" + pol.Spec.ApplyOnForward = true + + pol = createPolicy(pol) + }) + + bpfWaitForPolicy(felixes[0], "eth0", "egress", "default.host-0-1") + + }) + It("should handle NAT outgoing", func() { + By("SNATting outgoing traffic with the flag set") cc.ExpectSNAT(w[0][0], felixes[0].IP, hostW[1]) cc.CheckConnectivity(conntrackChecks(felixes)...) - } + + if testOpts.tunnel == "none" { + By("Leaving traffic alone with the flag clear") + pool, err := calicoClient.IPPools().Get(context.TODO(), "test-pool", options2.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + pool.Spec.NATOutgoing = false + pool, err = calicoClient.IPPools().Update(context.TODO(), pool, options2.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + cc.ResetExpectations() + cc.ExpectSNAT(w[0][0], w[0][0].IP, hostW[1]) + cc.CheckConnectivity(conntrackChecks(felixes)...) + + By("SNATting again with the flag set") + pool.Spec.NATOutgoing = true + pool, err = calicoClient.IPPools().Update(context.TODO(), pool, options2.SetOptions{}) + Expect(err).NotTo(HaveOccurred()) + cc.ResetExpectations() + cc.ExpectSNAT(w[0][0], felixes[0].IP, hostW[1]) + cc.CheckConnectivity(conntrackChecks(felixes)...) + } + }) }) It("connectivity from all workloads via workload 0's main IP", func() {