Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions bpf/flows.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
*/
#include "dns_tracker.h"

/*
* Defines the TLS tracker,
*/
#include "tls_tracker.h"

/*
* Defines an rtt tracker,
* which runs inside flow_monitor. Is optional.
Expand Down Expand Up @@ -93,6 +98,17 @@ static __always_inline void update_existing_flow(flow_metrics *aggregate_flow, p
aggregate_flow->flags |= pkt->flags;
aggregate_flow->dscp = pkt->dscp;
aggregate_flow->sampling = sampling;
if (pkt->ssl_version != 0 && aggregate_flow->ssl_version != pkt->ssl_version) {
if (aggregate_flow->ssl_version == 0) {
aggregate_flow->ssl_version = pkt->ssl_version;
} else {
// If several SSL versions are found, keep just the smallest and set the "mismatch" flag
if (pkt->ssl_version < aggregate_flow->ssl_version) {
aggregate_flow->ssl_version = pkt->ssl_version;
}
aggregate_flow->misc_flags |= MISC_FLAGS_SSL_MISMATCH;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it better to have mismatch counter instead of flag ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the flag allows us to correlate precisely with a particular flow, and report that up to the UI

}
}
} else if (if_index != 0) {
// Only add info that we've seen this interface (we can also update end time & flags)
aggregate_flow->end_mono_time_ts = pkt->current_ts;
Expand Down Expand Up @@ -178,6 +194,7 @@ static inline int flow_monitor(struct __sk_buff *skb, u8 direction) {
if (enable_dns_tracking) {
dns_errno = track_dns_packet(skb, &pkt);
}
track_tls_version(skb, &pkt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think its better if we have feature config for tracker like we do with all other features ? instead of enabling it by default ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I'll do that (although I would like to have it enabled by default if it doesn't show visible impact on perfs)

flow_metrics *aggregate_flow = (flow_metrics *)bpf_map_lookup_elem(&aggregated_flows, &id);
if (aggregate_flow != NULL) {
update_existing_flow(aggregate_flow, &pkt, len, flow_sampling, skb->ifindex, direction);
Expand All @@ -197,6 +214,7 @@ static inline int flow_monitor(struct __sk_buff *skb, u8 direction) {
new_flow.sampling = flow_sampling;
__builtin_memcpy(new_flow.dst_mac, eth->h_dest, ETH_ALEN);
__builtin_memcpy(new_flow.src_mac, eth->h_source, ETH_ALEN);
new_flow.ssl_version = pkt.ssl_version;

long ret = bpf_map_update_elem(&aggregated_flows, &id, &new_flow, BPF_NOEXIST);
if (ret != 0) {
Expand Down
82 changes: 82 additions & 0 deletions bpf/tls_tracker.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
light weight TLS tracker.
*/

#ifndef __TLS_TRACKER_H__
#define __TLS_TRACKER_H__
#include "utils.h"

#define CONTENT_TYPE_CHANGE_CIPHER 0x14
#define CONTENT_TYPE_ALERT 0x15
#define CONTENT_TYPE_HANDSHAKE 0x16
#define CONTENT_TYPE_APP_DATA 0x17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on this gist it seems there also heartbeat
https://gist.github.com/coin8086/1cd0411447066a5a02be6a3e493479e2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe something for the feature could be measuring the handshake latency ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that would require a new map like we do for DNS, it's probably more impact on performance? If so, it should probably be a separate feature

#define HANDSHAKE_CLIENT_HELLO 0x01
#define HANDSHAKE_SERVER_HELLO 0x02

// https://www.rfc-editor.org/rfc/rfc5246
struct tls_record {
u8 content_type; // handshake, alert, change cipher, app data
u8 major;
u8 minor;
u16 length;
};

struct tls_handshake_header {
u8 content_type; // client hello, server hello ...
u8 len[3];
};

struct tls_handshake_version {
u8 major;
u8 minor;
};

// Extract TLS info
static inline void track_tls_version(struct __sk_buff *skb, pkt_info *pkt) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be good idea to add return code and track failure at the caller since there many conditions check in this function and if things doesn't work it will be hard to debug.
might be good adding BPF_PRINTK() on failing checks ?

if (pkt->id->transport_protocol == IPPROTO_TCP) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do u think this work for STCP protocol too ?, not sure what is the story for DTLS ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't digged into other protocols.. might be something to do if someone asks for it, but I believe TCP covers most of the expectations

void *data_end = (void *)(long)skb->data_end;
struct tcphdr *tcp = (struct tcphdr *)pkt->l4_hdr;
if (!tcp || ((void *)tcp + sizeof(*tcp) > data_end)) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the verifier needs the above check given how late we call this tracker all headers should have been sane already ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I had a verifier error without those checks


u8 len = tcp->doff * sizeof(u32);
if (!len) {
return;
}

struct tls_record rec;
u32 offset = (long)pkt->l4_hdr - (long)skb->data + len;

if ((bpf_skb_load_bytes(skb, offset, &rec, sizeof(rec))) < 0) {
return;
}

switch (rec.content_type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic assume there is always tls record after tcp header this is not safe assumption we could have dns header here or any other header ??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, I should harden that a bit more; I've found unexpected TLSVersion during my tests probably because of that.

case CONTENT_TYPE_HANDSHAKE: {
pkt->ssl_version = ((u16)rec.major) << 8 | rec.minor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why setting it here if u really want handshake ssl version ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a fallback for the case where handshake header couldn't be read ... But I'm going to change that, actually perhaps we don't want the client-hello version at all, it doesn't tell what's actually going to be used

struct tls_handshake_header handshake;
if (bpf_skb_load_bytes(skb, offset + sizeof(rec), &handshake, sizeof(handshake)) < 0) {
return;
}
if (handshake.content_type == HANDSHAKE_CLIENT_HELLO ||
handshake.content_type == HANDSHAKE_SERVER_HELLO) {
struct tls_handshake_version handshake_version;
if (bpf_skb_load_bytes(skb, offset + sizeof(rec) + sizeof(handshake),
&handshake_version, sizeof(handshake_version)) < 0) {
return;
}
pkt->ssl_version = ((u16)handshake_version.major) << 8 | handshake_version.minor;
}
break;
}
case CONTENT_TYPE_CHANGE_CIPHER:
case CONTENT_TYPE_ALERT:
case CONTENT_TYPE_APP_DATA:
pkt->ssl_version = ((u16)rec.major) << 8 | rec.minor;
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do u think adding SSL record type to the follow will be adding value to end user ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean adding it to the flow, like, as a bitfield? hmm why not ..

}
}
}

#endif // __TLS_TRACKER_H__
5 changes: 5 additions & 0 deletions bpf/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ typedef __u64 u64;

#define MAX_PAYLOAD_SIZE 256

#define MISC_FLAGS_SSL_MISMATCH 0x01

// according to field 61 in https://www.iana.org/assignments/ipfix/ipfix.xhtml
typedef enum direction_t {
INGRESS,
Expand Down Expand Up @@ -110,6 +112,8 @@ typedef struct flow_metrics_t {
u8 nb_observed_intf;
u8 observed_direction[MAX_OBSERVED_INTERFACES];
u32 observed_intf[MAX_OBSERVED_INTERFACES];
u16 ssl_version;
u8 misc_flags;
} flow_metrics;

// Force emitting enums/structs into the ELF
Expand Down Expand Up @@ -192,6 +196,7 @@ typedef struct pkt_info_t {
u16 dns_id;
u16 dns_flags;
u64 dns_latency;
u16 ssl_version;
} pkt_info;

// Structure for payload metadata
Expand Down
4 changes: 4 additions & 0 deletions pkg/decode/decode_protobuf.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ func RecordToMap(fr *model.Record) config.GenericMap {
out["Sampling"] = fr.Metrics.Sampling
}

if tlsVersion := fr.Metrics.SSLVersionToString(); tlsVersion != "" {
out["TLSVersion"] = tlsVersion
}

if fr.Metrics.EthProtocol == uint16(ethernet.EtherTypeIPv4) || fr.Metrics.EthProtocol == uint16(ethernet.EtherTypeIPv6) {
out["SrcAddr"] = model.IP(fr.ID.SrcIp).String()
out["DstAddr"] = model.IP(fr.ID.DstIp).String()
Expand Down
2 changes: 2 additions & 0 deletions pkg/decode/decode_protobuf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func TestPBFlowToMap(t *testing.T) {
},
IpsecEncrypted: 1,
IpsecEncryptedRet: 0,
SslVersion: 0x0303,
}

out := PBFlowToMap(flow)
Expand Down Expand Up @@ -155,5 +156,6 @@ func TestPBFlowToMap(t *testing.T) {
"ZoneId": uint16(100),
"IPSecRetCode": int32(0),
"IPSecStatus": "success",
"TLSVersion": "TLS 1.2",
}, out)
}
4 changes: 3 additions & 1 deletion pkg/ebpf/bpf_arm64_bpfel.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified pkg/ebpf/bpf_arm64_bpfel.o
Binary file not shown.
4 changes: 3 additions & 1 deletion pkg/ebpf/bpf_powerpc_bpfel.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified pkg/ebpf/bpf_powerpc_bpfel.o
Binary file not shown.
4 changes: 3 additions & 1 deletion pkg/ebpf/bpf_s390_bpfeb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified pkg/ebpf/bpf_s390_bpfeb.o
Binary file not shown.
4 changes: 3 additions & 1 deletion pkg/ebpf/bpf_x86_bpfel.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified pkg/ebpf/bpf_x86_bpfel.o
Binary file not shown.
58 changes: 58 additions & 0 deletions pkg/exporter/converters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func TestConversions(t *testing.T) {
Flags: 0x100,
Dscp: 64,
Sampling: 1,
SslVersion: 0x0303,
},
AdditionalMetrics: &ebpf.BpfAdditionalMetrics{
DnsRecord: ebpf.BpfDnsRecordT{
Expand Down Expand Up @@ -83,6 +84,7 @@ func TestConversions(t *testing.T) {
"AgentIP": "10.11.12.13",
"IPSecRetCode": 0,
"IPSecStatus": "success",
"TLSVersion": "TLS 1.2",
},
},
{
Expand Down Expand Up @@ -333,6 +335,7 @@ func TestConversions(t *testing.T) {
Packets: 123,
Flags: 0x100,
Dscp: 64,
SslVersion: 0x0200,
},
AdditionalMetrics: &ebpf.BpfAdditionalMetrics{
DnsRecord: ebpf.BpfDnsRecordT{
Expand Down Expand Up @@ -389,6 +392,7 @@ func TestConversions(t *testing.T) {
"TimeFlowRttNs": someDuration.Nanoseconds(),
"IPSecRetCode": 0,
"IPSecStatus": "success",
"TLSVersion": "0x0200",
},
},
{
Expand All @@ -410,6 +414,7 @@ func TestConversions(t *testing.T) {
Packets: 1,
Flags: 0x100,
Dscp: 64,
SslVersion: 0x0303,
},
AdditionalMetrics: &ebpf.BpfAdditionalMetrics{
DnsRecord: ebpf.BpfDnsRecordT{
Expand Down Expand Up @@ -447,6 +452,59 @@ func TestConversions(t *testing.T) {
"AgentIP": "10.11.12.13",
"IPSecRetCode": 0,
"IPSecStatus": "success",
"TLSVersion": "TLS 1.2",
},
},
{
name: "SSL Mismatch",
flow: &model.Record{
ID: ebpf.BpfFlowId{
SrcIp: model.IPAddr{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0x06, 0x07, 0x08, 0x09},
DstIp: model.IPAddr{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0x0a, 0x0b, 0x0c, 0x0d},
SrcPort: 23000,
DstPort: 443,
TransportProtocol: 6,
},
Metrics: model.BpfFlowContent{
BpfFlowMetrics: &ebpf.BpfFlowMetrics{
EthProtocol: 2048,
SrcMac: model.MacAddr{0x04, 0x05, 0x06, 0x07, 0x08, 0x09},
DstMac: model.MacAddr{0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f},
Bytes: 64,
Packets: 1,
Flags: 0x100,
Dscp: 64,
SslVersion: 0x0303,
MiscFlags: model.MiscFlagsSSLMismatch,
},
},
Interfaces: []model.IntfDirUdn{
model.NewIntfDirUdn("eth0", model.DirectionEgress, nil),
},
TimeFlowStart: someTime,
TimeFlowEnd: someTime,
AgentIP: net.IPv4(0x0a, 0x0b, 0x0c, 0x0d),
},
expected: &config.GenericMap{
"IfDirections": []int{1},
"Bytes": 64,
"SrcAddr": "6.7.8.9",
"DstAddr": "10.11.12.13",
"Dscp": 64,
"DstMac": "0a:0b:0c:0d:0e:0f",
"SrcMac": "04:05:06:07:08:09",
"Etype": 2048,
"Packets": 1,
"Proto": 6,
"SrcPort": 23000,
"DstPort": 443,
"Flags": 0x100,
"TimeFlowStartMs": someTime.UnixMilli(),
"TimeFlowEndMs": someTime.UnixMilli(),
"Interfaces": []string{"eth0"},
"Udns": []string{""},
"AgentIP": "10.11.12.13",
"TLSVersion": "~ TLS 1.2",
},
},
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/model/record.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package model

import (
"crypto/tls"
"encoding/binary"
"fmt"
"io"
Expand All @@ -25,6 +26,8 @@ const (
NetworkEventsMaxEventsMD = 8
MaxNetworkEvents = 4
MaxObservedInterfaces = 6

MiscFlagsSSLMismatch = 0x01
)

var recordLog = logrus.WithField("component", "model")
Expand Down Expand Up @@ -222,3 +225,18 @@ func AllZeroIP(ip net.IP) bool {
}
return false
}

func (r *BpfFlowContent) SSLVersionToString() string {
if r.SslVersion == 0 {
return ""
}
v := tls.VersionName(r.SslVersion)
if r.HasSSLMismatch() {
return "~ " + v
}
return v
}

func (r *BpfFlowContent) HasSSLMismatch() bool {
return r.MiscFlags&MiscFlagsSSLMismatch > 0
}
Loading
Loading