Skip to content

Commit 56a1a67

Browse files
smagnani96joestringer
authored andcommitted
bpf:hubble: support policy verdict from L3 devices
When `bpf_host` is attached to a L3 device and there are policies (e.g., when HostFw is enabled), verdict notification are emitted, but monitor/Hubble incorrectly decodes the packet, starting from a non-existent L2 header. Let's fix this by adding a flag, similarly to what we did for drop/trace notifications. From v1.18 onwards, `bpf_wireguard` has its own program rather than attaching `bpf_host` to the `cilium_wg0@ingress`. Though, it still can happen that `bpf_host` is attached to some L3 device (TUN). Example of PolicyVerdict incorrectly decoded: ``` Policy verdict log: flow 0x0 local EP ID 285, remote ID remote-node, proto 1, ingress, action allow, auth: disabled, match L4-Only, 40:00:40:01:90:a3 -> 45:c0:00:34:51:3b UnknownEthernetType ``` Example of PolicyVerdict correctly decoded after PR: ``` Policy verdict log: flow 0x0 local EP ID 285, remote ID remote-node, proto 1, ingress, action allow, auth: disabled, match L4-Only, 172.18.0.3 -> 172.18.0.4 EchoRequest ``` Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
1 parent ef1cfdd commit 56a1a67

4 files changed

Lines changed: 66 additions & 29 deletions

File tree

bpf/lib/policy_log.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct policy_verdict_notify {
3030
ipv6:1,
3131
match_type:3,
3232
audited:1,
33-
pad0:1;
33+
l3:1;
3434
__u8 auth_type;
3535
__u8 pad1[3]; /* align with 64 bits */
3636
__u32 cookie;
@@ -107,6 +107,7 @@ send_policy_verdict_notify(struct __ctx_buff *ctx, __u32 remote_label, __u16 dst
107107
.audited = is_audited,
108108
.auth_type = auth_type,
109109
.cookie = cookie,
110+
.l3 = THIS_IS_L3_DEV,
110111
};
111112

112113
ctx_event_output(ctx, &cilium_events,

pkg/hubble/parser/threefour/parser.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ func (p *Parser) Decode(data []byte, decoded *pb.Flow) error {
214214
return fmt.Errorf("not enough bytes to decode %d", data)
215215
}
216216

217-
isL3Device := tn != nil && tn.IsL3Device() || dn != nil && dn.IsL3Device()
218-
isIPv6 := tn != nil && tn.IsIPv6() || dn != nil && dn.IsIPv6()
217+
isL3Device := tn != nil && tn.IsL3Device() || dn != nil && dn.IsL3Device() || pvn != nil && pvn.IsTrafficL3Device()
218+
isIPv6 := tn != nil && tn.IsIPv6() || dn != nil && dn.IsIPv6() || pvn != nil && pvn.IsTrafficIPv6()
219219
isVXLAN := tn != nil && tn.IsVXLAN() || dn != nil && dn.IsVXLAN()
220220
isGeneve := tn != nil && tn.IsGeneve() || dn != nil && dn.IsGeneve()
221221
ether, ip, l4, tunnel, srcIP, dstIP, srcPort, dstPort, summary, err := decodeLayers(data[packetOffset:], p.packet, isL3Device, isIPv6, isVXLAN, isGeneve)

pkg/hubble/parser/threefour/parser_test.go

Lines changed: 52 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2345,23 +2345,29 @@ func TestDecode_PolicyVerdictNotify(t *testing.T) {
23452345
parser, err := New(hivetest.Logger(t), defaultEndpointGetter, nil, nil, nil, nil, nil)
23462346
require.NoError(t, err)
23472347

2348-
template := &flowpb.Flow{
2349-
EventType: &flowpb.CiliumEventType{Type: 5},
2350-
Summary: flowpb.IPVersion_IPv4.String(),
2351-
Type: flowpb.FlowType_L3_L4,
2352-
Verdict: flowpb.Verdict_FORWARDED,
2353-
Source: &flowpb.Endpoint{},
2354-
Destination: &flowpb.Endpoint{},
2355-
Ethernet: &flowpb.Ethernet{
2356-
Source: srcMAC.String(),
2357-
Destination: dstMAC.String(),
2358-
},
2359-
IP: &flowpb.IP{
2360-
IpVersion: flowpb.IPVersion_IPv4,
2361-
Source: localIP.String(),
2362-
Destination: remoteIP.String(),
2363-
},
2364-
IsReply: wrapperspb.Bool(false),
2348+
getTemplate := func(isL3Device bool) *flowpb.Flow {
2349+
template := &flowpb.Flow{
2350+
EventType: &flowpb.CiliumEventType{Type: 5},
2351+
Summary: flowpb.IPVersion_IPv4.String(),
2352+
Type: flowpb.FlowType_L3_L4,
2353+
Verdict: flowpb.Verdict_FORWARDED,
2354+
Source: &flowpb.Endpoint{},
2355+
Destination: &flowpb.Endpoint{},
2356+
Ethernet: &flowpb.Ethernet{
2357+
Source: srcMAC.String(),
2358+
Destination: dstMAC.String(),
2359+
},
2360+
IP: &flowpb.IP{
2361+
IpVersion: flowpb.IPVersion_IPv4,
2362+
Source: localIP.String(),
2363+
Destination: remoteIP.String(),
2364+
},
2365+
IsReply: wrapperspb.Bool(false),
2366+
}
2367+
if isL3Device {
2368+
template.Ethernet = nil
2369+
}
2370+
return template
23652371
}
23662372

23672373
testCases := []struct {
@@ -2386,7 +2392,7 @@ func TestDecode_PolicyVerdictNotify(t *testing.T) {
23862392
},
23872393
},
23882394
{
2389-
name: "ingresss",
2395+
name: "ingress",
23902396
event: monitor.PolicyVerdictNotify{
23912397
Type: byte(monitorAPI.MessageTypePolicyVerdict),
23922398
Source: localEP,
@@ -2398,20 +2404,41 @@ func TestDecode_PolicyVerdictNotify(t *testing.T) {
23982404
TrafficDirection: flowpb.TrafficDirection_INGRESS,
23992405
},
24002406
},
2407+
{
2408+
name: "ingress_l3_device",
2409+
event: monitor.PolicyVerdictNotify{
2410+
Type: byte(monitorAPI.MessageTypePolicyVerdict),
2411+
Source: localEP,
2412+
Flags: monitorAPI.PolicyIngress | monitor.PolicyVerdictNotifyFlagIsL3,
2413+
},
2414+
ipTuple: egressTuple,
2415+
want: &flowpb.Flow{
2416+
Source: &flowpb.Endpoint{ID: 1234},
2417+
TrafficDirection: flowpb.TrafficDirection_INGRESS,
2418+
},
2419+
},
24012420
}
24022421
for _, tc := range testCases {
24032422
t.Run(tc.name, func(t *testing.T) {
2404-
want := proto.Clone(template)
2405-
proto.Merge(want, tc.want)
2423+
isL3Device := false
2424+
if ev, ok := tc.event.(monitor.PolicyVerdictNotify); ok {
2425+
isL3Device = ev.IsTrafficL3Device()
2426+
}
24062427

2407-
data, err := testutils.CreateL3L4Payload(tc.event,
2408-
&layers.Ethernet{
2428+
var l []gopacket.SerializableLayer
2429+
if !isL3Device {
2430+
l = append(l, &layers.Ethernet{
24092431
SrcMAC: srcMAC,
24102432
DstMAC: dstMAC,
24112433
EthernetType: layers.EthernetTypeIPv4,
2412-
},
2413-
&layers.IPv4{SrcIP: tc.ipTuple.src.AsSlice(), DstIP: tc.ipTuple.dst.AsSlice()},
2414-
)
2434+
})
2435+
}
2436+
l = append(l, &layers.IPv4{SrcIP: tc.ipTuple.src.AsSlice(), DstIP: tc.ipTuple.dst.AsSlice()})
2437+
2438+
want := proto.Clone(getTemplate(isL3Device))
2439+
proto.Merge(want, tc.want)
2440+
2441+
data, err := testutils.CreateL3L4Payload(tc.event, l...)
24152442
if err != nil {
24162443
t.Fatalf("Unexpected error from CreateL3L4Payload(%T, ...): %v", tc.event, err)
24172444
}

pkg/monitor/datapath_policy.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ const (
3737
// corresponds to whether the traffic was allowed due to the audit mode
3838
PolicyVerdictNotifyFlagIsAudited = 0x40
3939

40+
// PolicyVerdictNotifyFlagIsL3 is the bit mask in Flags that
41+
// corresponds to whether the traffic is from a L3 device or not
42+
PolicyVerdictNotifyFlagIsL3 = 0x80
43+
4044
// PolicyVerdictNotifyFlagMatchTypeBitOffset is the bit offset in Flags that
4145
// corresponds to the policy match type
4246
PolicyVerdictNotifyFlagMatchTypeBitOffset = 3
@@ -114,6 +118,11 @@ func (n *PolicyVerdictNotify) IsTrafficIPv6() bool {
114118
return (n.Flags&PolicyVerdictNotifyFlagIsIPv6 > 0)
115119
}
116120

121+
// IsTrafficL3Device returns true if this notify is from a L3 device
122+
func (n *PolicyVerdictNotify) IsTrafficL3Device() bool {
123+
return (n.Flags&PolicyVerdictNotifyFlagIsL3 > 0)
124+
}
125+
117126
// GetPolicyMatchType returns how the traffic matched the policy
118127
func (n *PolicyVerdictNotify) GetPolicyMatchType() api.PolicyMatchType {
119128
return api.PolicyMatchType((n.Flags & PolicyVerdictNotifyFlagMatchType) >>
@@ -161,5 +170,5 @@ func (n *PolicyVerdictNotify) DumpInfo(buf *bufio.Writer, data []byte, numeric a
161170
fmt.Fprintf(buf, ", proto %d, %s, action %s, auth: %s, match %s, %s\n", n.Proto, dir,
162171
GetPolicyActionString(n.Verdict, n.IsTrafficAudited()),
163172
n.GetAuthType(), n.GetPolicyMatchType(),
164-
GetConnectionSummary(data[PolicyVerdictNotifyLen:], nil))
173+
GetConnectionSummary(data[PolicyVerdictNotifyLen:], &decodeOpts{IsL3Device: n.IsTrafficL3Device(), IsIPv6: n.IsTrafficIPv6()}))
165174
}

0 commit comments

Comments
 (0)