Skip to content

Commit

Permalink
[BPF] clasiffy correctly forwarded traffic
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tomastigera
tomastigera committed May 29, 2023
1 parent 03b7805 commit e647557
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 54 deletions.
2 changes: 2 additions & 0 deletions felix/bpf-gpl/skb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__ */
15 changes: 14 additions & 1 deletion felix/bpf-gpl/tc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand Down Expand Up @@ -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);
Expand Down
79 changes: 46 additions & 33 deletions felix/bpf/ut/failsafes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ var failsafeTests = []failsafeTest{
DstIP: fsafeDstIP,
Protocol: layers.IPProtocolUDP,
},
Outbound: true,
Allowed: true,
Outbound: true,
Allowed: true,
FromLocalHost: true,
},
{
Description: "Packets from localhost to non-failsafe IP are denied",
Expand All @@ -106,8 +107,9 @@ var failsafeTests = []failsafeTest{
DstIP: net.IPv4(4, 4, 4, 4),
Protocol: layers.IPProtocolUDP,
},
Outbound: false,
Allowed: false,
Outbound: false,
Allowed: false,
FromLocalHost: true,
},
{
Description: "Packets from outbound failsafes to inbound failsafes are denied",
Expand Down Expand Up @@ -179,8 +181,9 @@ var failsafeTests = []failsafeTest{
IPHeaderUDP: &layers.UDP{
DstPort: 161,
},
Outbound: true,
Allowed: false,
Outbound: true,
Allowed: false,
FromLocalHost: true,
},
}

Expand Down Expand Up @@ -213,37 +216,47 @@ func TestFailsafes(t *testing.T) {
Expect(err).NotTo(HaveOccurred())

for _, test := range failsafeTests {
_, _, _, _, pktBytes, err := testPacket(nil, test.IPHeaderIPv4, test.IPHeaderUDP, nil)
Expect(err).NotTo(HaveOccurred())

prog := "calico_from_host_ep"
skbMark = 0
if test.Outbound {
skbMark = tcdefs.MarkSeen
prog = "calico_to_host_ep"
}

result := "TC_ACT_SHOT"
if test.Allowed {
result = "TC_ACT_UNSPEC"
}

runBpfTest(t, prog, test.Rules, func(bpfrun bpfProgRunFn) {
res, err := bpfrun(pktBytes)
t.Run(test.Description, func(t *testing.T) {
_, _, _, _, pktBytes, err := testPacket(nil, test.IPHeaderIPv4, test.IPHeaderUDP, nil)
Expect(err).NotTo(HaveOccurred())
Expect(res.RetvalStr()).To(Equal(result), fmt.Sprintf("expected program to return %s", result))

prog := "calico_from_host_ep"
skbMark = 0

var opts []testOption

if test.Outbound {
if !test.FromLocalHost {
skbMark = tcdefs.MarkSeen
} else {
opts = append(opts, withHostNetworked())
}
prog = "calico_to_host_ep"
}

result := "TC_ACT_SHOT"
if test.Allowed {
result = "TC_ACT_UNSPEC"
}

runBpfTest(t, prog, test.Rules, func(bpfrun bpfProgRunFn) {
res, err := bpfrun(pktBytes)
Expect(err).NotTo(HaveOccurred())
Expect(res.RetvalStr()).To(Equal(result), fmt.Sprintf("expected program to return %s", result))
}, opts...)
if !test.Outbound && test.Allowed {
expectMark(tcdefs.MarkSeen)
}
})
if !test.Outbound && test.Allowed {
expectMark(tcdefs.MarkSeen)
}
}
}

type failsafeTest struct {
Description string
Rules *polprog.Rules
IPHeaderIPv4 *layers.IPv4
IPHeaderUDP gopacket.Layer
Outbound bool
Allowed bool
Description string
Rules *polprog.Rules
IPHeaderIPv4 *layers.IPv4
IPHeaderUDP gopacket.Layer
Outbound bool
Allowed bool
FromLocalHost bool
}
100 changes: 80 additions & 20 deletions felix/fv/bpf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit e647557

Please sign in to comment.