Skip to content

Commit

Permalink
Revert "[USMON-417] gotls: Restore support on fedora (#27437)" (#29128)
Browse files Browse the repository at this point in the history
  • Loading branch information
guyarb authored Sep 9, 2024
1 parent f5b5729 commit e9cee3d
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 83 deletions.
4 changes: 0 additions & 4 deletions pkg/network/ebpf/c/protocols/tls/go-tls-maps.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
// offsets_data map contains the information about the locations of structs in the inspected binary, mapped by the binary's inode number.
BPF_HASH_MAP(offsets_data, go_tls_offsets_data_key_t, tls_offsets_data_t, 1024)

// Maps PID to the <device id>-<inode> tuple, that is used to find the offsets_data map for the binary.
// Size is a 10 times the size of the offsets_data map, to have enough space for all the binaries.
BPF_HASH_MAP(pid_to_device_inode, u32, go_tls_offsets_data_key_t, 10240)

/* go_tls_read_args is used to get the read function info when running in the read-return uprobe.
The key contains the go routine id and the pid. */
BPF_LRU_MAP(go_tls_read_args, go_tls_function_args_key_t, go_tls_read_args_data_t, 2048)
Expand Down
46 changes: 36 additions & 10 deletions pkg/network/ebpf/c/protocols/tls/https.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

#ifdef COMPILE_CORE
#include "ktypes.h"
#define MINORBITS 20
#define MINORMASK ((1U << MINORBITS) - 1)
#define MAJOR(dev) ((unsigned int) ((dev) >> MINORBITS))
#define MINOR(dev) ((unsigned int) ((dev) & MINORMASK))
#else
#include <linux/dcache.h>
#include <linux/fs.h>
Expand Down Expand Up @@ -283,19 +287,41 @@ static __always_inline void map_ssl_ctx_to_sock(struct sock *skp) {
bpf_map_update_with_telemetry(ssl_sock_by_ctx, &ssl_ctx, &ssl_sock, BPF_ANY);
}


// Retrieves the result of binary analysis for the current task binary's inode number.
// For the current PID, we retrieve the inode number of the binary and then we look up the binary's analysis result.
/**
* get_offsets_data retrieves the result of binary analysis for the
* current task binary's inode number.
*/
static __always_inline tls_offsets_data_t* get_offsets_data() {
u64 pid_tgid = bpf_get_current_pid_tgid();
u32 pid = pid_tgid >> 32;
go_tls_offsets_data_key_t *key = bpf_map_lookup_elem(&pid_to_device_inode, &pid);
if (key == NULL) {
log_debug("get_offsets_data: could not find key for pid %u", pid);
struct task_struct *t = (struct task_struct *) bpf_get_current_task();
struct inode *inode;
go_tls_offsets_data_key_t key;
dev_t dev_id;

inode = BPF_CORE_READ(t, mm, exe_file, f_inode);
if (!inode) {
log_debug("get_offsets_data: could not read f_inode field");
return NULL;
}

int err;
err = BPF_CORE_READ_INTO(&key.ino, inode, i_ino);
if (err) {
log_debug("get_offsets_data: could not read i_ino field");
return NULL;
}
go_tls_offsets_data_key_t key_copy = *key;
return bpf_map_lookup_elem(&offsets_data, &key_copy);

err = BPF_CORE_READ_INTO(&dev_id, inode, i_sb, s_dev);
if (err) {
log_debug("get_offsets_data: could not read s_dev field");
return NULL;
}

key.device_id_major = MAJOR(dev_id);
key.device_id_minor = MINOR(dev_id);

log_debug("get_offsets_data: task binary inode number: %llu; device ID %x:%x", key.ino, key.device_id_major, key.device_id_minor);

return bpf_map_lookup_elem(&offsets_data, &key);
}

#endif
27 changes: 25 additions & 2 deletions pkg/network/protocols/tls/gotls/testutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,34 @@
package testutil

import (
"slices"
"testing"

"github.com/stretchr/testify/require"

"github.com/DataDog/datadog-agent/pkg/network/config"
usmconfig "github.com/DataDog/datadog-agent/pkg/network/usm/config"
"github.com/DataDog/datadog-agent/pkg/util/kernel"
)

const (
fedoraPlatform = "fedora"
)

var fedoraUnsupportedVersions = []string{"35", "36", "37", "38"}

// isFedora returns true if the current OS is Fedora.
// go-tls does not work correctly on Fedora 35, 36, 37 and 38.
func isFedora(t *testing.T) bool {
platform, err := kernel.Platform()
require.NoError(t, err)
platformVersion, err := kernel.PlatformVersion()
require.NoError(t, err)

return platform == fedoraPlatform && slices.Contains(fedoraUnsupportedVersions, platformVersion)
}

// GoTLSSupported returns true if GO-TLS monitoring is supported on the current OS.
func GoTLSSupported(cfg *config.Config) bool {
return usmconfig.TLSSupported(cfg) && (cfg.EnableRuntimeCompiler || cfg.EnableCORE)
func GoTLSSupported(t *testing.T, cfg *config.Config) bool {
return usmconfig.TLSSupported(cfg) && (cfg.EnableRuntimeCompiler || cfg.EnableCORE) && !isFedora(t)
}
73 changes: 19 additions & 54 deletions pkg/network/usm/ebpf_gotls.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import (

const (
offsetsDataMap = "offsets_data"
pidToDeviceInodeMap = "pid_to_device_inode"
goTLSReadArgsMap = "go_tls_read_args"
goTLSWriteArgsMap = "go_tls_write_args"
connectionTupleByGoTLSMap = "conn_tup_by_go_tls_conn"
Expand Down Expand Up @@ -116,11 +115,6 @@ type goTLSProgram struct {
// inodes.
offsetsDataMap *ebpf.Map

// eBPF map holding the mapping of PIDs to device/inode numbers.
// On some filesystems (like btrfs), the device-id in the task-struct can be different from the device-id extracted
// in the user-mode. This map is used to ensure the eBPF probes are getting the correct device/inode numbers.
pidToDeviceInodeMap *ebpf.Map

// binAnalysisMetric handles telemetry on the time spent doing binary
// analysis
binAnalysisMetric *libtelemetry.Counter
Expand All @@ -137,7 +131,6 @@ var _ utils.Attacher = &goTLSProgram{}
var goTLSSpec = &protocols.ProtocolSpec{
Maps: []*manager.Map{
{Name: offsetsDataMap},
{Name: pidToDeviceInodeMap},
{Name: goTLSReadArgsMap},
{Name: goTLSWriteArgsMap},
{Name: connectionTupleByGoTLSMap},
Expand Down Expand Up @@ -223,10 +216,6 @@ func (p *goTLSProgram) PreStart(m *manager.Manager) error {
if err != nil {
return fmt.Errorf("could not get offsets_data map: %s", err)
}
p.pidToDeviceInodeMap, _, err = m.GetMap(pidToDeviceInodeMap)
if err != nil {
return fmt.Errorf("could not get %s map: %s", pidToDeviceInodeMap, err)
}

procMonitor := monitor.GetProcessMonitor()
cleanupExec := procMonitor.SubscribeExec(p.handleProcessStart)
Expand All @@ -253,7 +242,7 @@ func (p *goTLSProgram) PreStart(m *manager.Manager) error {
processSet := p.registry.GetRegisteredProcesses()
deletedPids := monitor.FindDeletedProcesses(processSet)
for deletedPid := range deletedPids {
_ = p.DetachPID(deletedPid)
_ = p.registry.Unregister(deletedPid)
}
}
}
Expand Down Expand Up @@ -289,7 +278,6 @@ var (

// DetachPID detaches the provided PID from the eBPF program.
func (p *goTLSProgram) DetachPID(pid uint32) error {
_ = p.pidToDeviceInodeMap.Delete(unsafe.Pointer(&pid))
return p.registry.Unregister(pid)
}

Expand Down Expand Up @@ -350,13 +338,12 @@ func (p *goTLSProgram) AttachPID(pid uint32) error {

// Check go process
probeList := make([]manager.ProbeIdentificationPair, 0)
return p.registry.Register(binPath, pid,
registerCBCreator(p.manager, p.offsetsDataMap, p.pidToDeviceInodeMap, &probeList, p.binAnalysisMetric, p.binNoSymbolsMetric),
unregisterCBCreator(p.manager, &probeList, p.offsetsDataMap, p.pidToDeviceInodeMap),
alreadyCBCreator(p.pidToDeviceInodeMap))
return p.registry.Register(binPath, pid, registerCBCreator(p.manager, p.offsetsDataMap, &probeList, p.binAnalysisMetric, p.binNoSymbolsMetric),
unregisterCBCreator(p.manager, &probeList, p.offsetsDataMap),
utils.IgnoreCB)
}

func registerCBCreator(mgr *manager.Manager, offsetsDataMap, pidToDeviceInodeMap *ebpf.Map, probeIDs *[]manager.ProbeIdentificationPair, binAnalysisMetric, binNoSymbolsMetric *libtelemetry.Counter) func(path utils.FilePath) error {
func registerCBCreator(mgr *manager.Manager, offsetsDataMap *ebpf.Map, probeIDs *[]manager.ProbeIdentificationPair, binAnalysisMetric, binNoSymbolsMetric *libtelemetry.Counter) func(path utils.FilePath) error {
return func(filePath utils.FilePath) error {
start := time.Now()

Expand All @@ -379,13 +366,13 @@ func registerCBCreator(mgr *manager.Manager, offsetsDataMap, pidToDeviceInodeMap
return fmt.Errorf("error extracting inspectoin data from %s: %w", filePath.HostPath, err)
}

if err := addInspectionResultToMap(offsetsDataMap, pidToDeviceInodeMap, filePath, inspectionResult); err != nil {
if err := addInspectionResultToMap(offsetsDataMap, filePath.ID, inspectionResult); err != nil {
return fmt.Errorf("failed adding inspection rules: %w", err)
}

pIDs, err := attachHooks(mgr, inspectionResult, filePath.HostPath, filePath.ID)
if err != nil {
removeInspectionResultFromMap(offsetsDataMap, pidToDeviceInodeMap, filePath)
removeInspectionResultFromMap(offsetsDataMap, filePath.ID)
return fmt.Errorf("error while attaching hooks to %s: %w", filePath.HostPath, err)
}
*probeIDs = pIDs
Expand All @@ -398,21 +385,6 @@ func registerCBCreator(mgr *manager.Manager, offsetsDataMap, pidToDeviceInodeMap
}
}

// alreadyCBCreator handles the case where a binary is already registered. In such a case the registry callback won't
// be called, so we need to add a mapping from the PID to the device/inode of the binary.
func alreadyCBCreator(pidToDeviceInodeMap *ebpf.Map) func(utils.FilePath) error {
return func(filePath utils.FilePath) error {
if filePath.PID == 0 {
return nil
}
return pidToDeviceInodeMap.Put(unsafe.Pointer(&filePath.PID), unsafe.Pointer(&gotls.TlsBinaryId{
Id_major: unix.Major(filePath.ID.Dev),
Id_minor: unix.Minor(filePath.ID.Dev),
Ino: filePath.ID.Inode,
}))
}
}

func (p *goTLSProgram) handleProcessExit(pid pid) {
_ = p.DetachPID(pid)
}
Expand All @@ -423,39 +395,32 @@ func (p *goTLSProgram) handleProcessStart(pid pid) {

// addInspectionResultToMap runs a binary inspection and adds the result to the
// map that's being read by the probes, indexed by the binary's inode number `ino`.
func addInspectionResultToMap(offsetsDataMap, pidToDeviceInodeMap *ebpf.Map, filePath utils.FilePath, result *bininspect.Result) error {
func addInspectionResultToMap(offsetsDataMap *ebpf.Map, binID utils.PathIdentifier, result *bininspect.Result) error {
offsetsData, err := inspectionResultToProbeData(result)
if err != nil {
return fmt.Errorf("error while parsing inspection result: %w", err)
}

key := &gotls.TlsBinaryId{
Id_major: unix.Major(filePath.ID.Dev),
Id_minor: unix.Minor(filePath.ID.Dev),
Ino: filePath.ID.Inode,
Id_major: unix.Major(binID.Dev),
Id_minor: unix.Minor(binID.Dev),
Ino: binID.Inode,
}
if err := offsetsDataMap.Put(unsafe.Pointer(key), unsafe.Pointer(&offsetsData)); err != nil {
return fmt.Errorf("could not write binary inspection result to map for binID %v (pid %v): %w", filePath.ID, filePath.PID, err)
return fmt.Errorf("could not write binary inspection result to map for binID %v: %w", binID, err)
}

if err := pidToDeviceInodeMap.Put(unsafe.Pointer(&filePath.PID), unsafe.Pointer(key)); err != nil {
return fmt.Errorf("could not write pid to device/inode (%s) map for pid %v: %w", filePath.ID.String(), filePath.PID, err)
}
return nil
}

func removeInspectionResultFromMap(offsetsDataMap, pidToDeviceInodeMap *ebpf.Map, filePath utils.FilePath) {
func removeInspectionResultFromMap(offsetsDataMap *ebpf.Map, binID utils.PathIdentifier) {
key := &gotls.TlsBinaryId{
Id_major: unix.Major(filePath.ID.Dev),
Id_minor: unix.Minor(filePath.ID.Dev),
Ino: filePath.ID.Inode,
}
if filePath.PID != 0 {
_ = pidToDeviceInodeMap.Delete(unsafe.Pointer(&filePath.PID))
Id_major: unix.Major(binID.Dev),
Id_minor: unix.Minor(binID.Dev),
Ino: binID.Inode,
}
if err := offsetsDataMap.Delete(unsafe.Pointer(key)); err != nil {
log.Errorf("could not remove inspection result from map for ino %v: %s", filePath.ID, err)
return
log.Errorf("could not remove inspection result from map for ino %v: %s", binID, err)
}
}

Expand Down Expand Up @@ -510,12 +475,12 @@ func attachHooks(mgr *manager.Manager, result *bininspect.Result, binPath string
return probeIDs, nil
}

func unregisterCBCreator(mgr *manager.Manager, probeIDs *[]manager.ProbeIdentificationPair, offsetsDataMap, pidToDeviceInodeMap *ebpf.Map) func(path utils.FilePath) error {
func unregisterCBCreator(mgr *manager.Manager, probeIDs *[]manager.ProbeIdentificationPair, offsetsDataMap *ebpf.Map) func(path utils.FilePath) error {
return func(path utils.FilePath) error {
if len(*probeIDs) == 0 {
return nil
}
removeInspectionResultFromMap(offsetsDataMap, pidToDeviceInodeMap, path)
removeInspectionResultFromMap(offsetsDataMap, path.ID)
for _, probeID := range *probeIDs {
err := mgr.DetachHook(probeID)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions pkg/network/usm/kafka_monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (s *KafkaProtocolParsingSuite) TestKafkaProtocolParsing() {

for mode, name := range map[bool]string{false: "without TLS", true: "with TLS"} {
t.Run(name, func(t *testing.T) {
if mode && !gotlsutils.GoTLSSupported(config.New()) {
if mode && !gotlsutils.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}
for _, version := range versions {
Expand Down Expand Up @@ -1244,7 +1244,7 @@ func (s *KafkaProtocolParsingSuite) TestKafkaFetchRaw() {
})

t.Run("with TLS", func(t *testing.T) {
if !gotlsutils.GoTLSSupported(config.New()) {
if !gotlsutils.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}

Expand Down Expand Up @@ -1470,7 +1470,7 @@ func (s *KafkaProtocolParsingSuite) TestKafkaProduceRaw() {
})

t.Run("with TLS", func(t *testing.T) {
if !gotlsutils.GoTLSSupported(config.New()) {
if !gotlsutils.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}

Expand Down
12 changes: 7 additions & 5 deletions pkg/network/usm/monitor_tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,9 +548,11 @@ func (s *tlsSuite) TestJavaInjection() {
}

func TestHTTPGoTLSAttachProbes(t *testing.T) {
t.Skip("skipping GoTLS tests while we investigate their flakiness")

modes := []ebpftest.BuildMode{ebpftest.RuntimeCompiled, ebpftest.CORE}
ebpftest.TestBuildModes(t, modes, "", func(t *testing.T) {
if !gotlstestutil.GoTLSSupported(config.New()) {
if !gotlstestutil.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}

Expand All @@ -569,7 +571,7 @@ func testHTTP2GoTLSAttachProbes(t *testing.T, cfg *config.Config) {
if !http2.Supported() {
t.Skip("HTTP2 not supported for this setup")
}
if !gotlstestutil.GoTLSSupported(cfg) {
if !gotlstestutil.GoTLSSupported(t, cfg) {
t.Skip("GoTLS not supported for this setup")
}

Expand Down Expand Up @@ -601,7 +603,7 @@ func TestHTTPSGoTLSAttachProbesOnContainer(t *testing.T) {
t.Skip("Skipping a flaky test")
modes := []ebpftest.BuildMode{ebpftest.RuntimeCompiled, ebpftest.CORE}
ebpftest.TestBuildModes(t, modes, "", func(t *testing.T) {
if !gotlstestutil.GoTLSSupported(config.New()) {
if !gotlstestutil.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}

Expand All @@ -619,7 +621,7 @@ func TestOldConnectionRegression(t *testing.T) {

modes := []ebpftest.BuildMode{ebpftest.RuntimeCompiled, ebpftest.CORE}
ebpftest.TestBuildModes(t, modes, "", func(t *testing.T) {
if !gotlstestutil.GoTLSSupported(config.New()) {
if !gotlstestutil.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}

Expand Down Expand Up @@ -694,7 +696,7 @@ func TestOldConnectionRegression(t *testing.T) {
func TestLimitListenerRegression(t *testing.T) {
modes := []ebpftest.BuildMode{ebpftest.RuntimeCompiled, ebpftest.CORE}
ebpftest.TestBuildModes(t, modes, "", func(t *testing.T) {
if !gotlstestutil.GoTLSSupported(config.New()) {
if !gotlstestutil.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/network/usm/postgres_monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (s *postgresProtocolParsingSuite) TestDecoding() {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.isTLS && !gotlstestutil.GoTLSSupported(config.New()) {
if tt.isTLS && !gotlstestutil.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}
testDecoding(t, tt.isTLS)
Expand Down
4 changes: 2 additions & 2 deletions pkg/network/usm/tests/tracer_usm_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func skipIfUsingNAT(t *testing.T, ctx testContext) {

// skipIfGoTLSNotSupported skips the test if GoTLS is not supported.
func skipIfGoTLSNotSupported(t *testing.T, _ testContext) {
if !gotlstestutil.GoTLSSupported(config.New()) {
if !gotlstestutil.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS is not supported")
}
}
Expand Down Expand Up @@ -183,7 +183,7 @@ func (s *USMSuite) TestProtocolClassification() {
cfg.EnableNativeTLSMonitoring = true
cfg.EnableHTTPMonitoring = true
cfg.EnablePostgresMonitoring = true
cfg.EnableGoTLSSupport = gotlstestutil.GoTLSSupported(cfg)
cfg.EnableGoTLSSupport = gotlstestutil.GoTLSSupported(t, cfg)
cfg.BypassEnabled = true
tr, err := tracer.NewTracer(cfg, nil)
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/usm/usm_grpc_monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestGRPCScenarios(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
if tc.isTLS && !gotlsutils.GoTLSSupported(config.New()) {
if tc.isTLS && !gotlsutils.GoTLSSupported(t, config.New()) {
t.Skip("GoTLS not supported for this setup")
}
suite.Run(t, &usmGRPCSuite{isTLS: tc.isTLS})
Expand Down
Loading

0 comments on commit e9cee3d

Please sign in to comment.