From d3486548523da377fa79dc11fd0a89c56bc3a09c Mon Sep 17 00:00:00 2001 From: "Alex K." <8418476+fearful-symmetry@users.noreply.github.com> Date: Thu, 8 Aug 2024 08:43:41 -0700 Subject: [PATCH] Move around librpm calls, add `setreuid()` safety net (#40436) * tinkering with suid * cleanup setuid work * clean up debugging code * docs, experimenting with setreuid * remove go thread * final cleanup * crossbuild issues * fighting with ci * linter... * tinker with CI packges * more linter tinkering * cleanup * deref pointer * fix seccomp --- .github/workflows/golangci-lint.yml | 2 +- x-pack/auditbeat/auditbeat.reference.yml | 8 ++ .../module/system/_meta/config.yml.tmpl | 8 ++ .../auditbeat/module/system/package/config.go | 3 + .../module/system/package/package.go | 94 +++++++++++++++---- .../system/package/package_linux_test.go | 91 ++++++++++++++++++ .../module/system/package/package_test.go | 1 + .../module/system/package/rpm_linux.go | 52 +++++++--- .../module/system/package/rpm_linux_test.go | 2 +- .../module/system/package/rpm_others.go | 2 +- x-pack/auditbeat/seccomp_linux.go | 1 + 11 files changed, 226 insertions(+), 38 deletions(-) create mode 100644 x-pack/auditbeat/module/system/package/package_linux_test.go diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 19f343602e87..94059ab8d963 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -33,7 +33,7 @@ jobs: go-version-file: .go-version - name: Install Apt Package - run: sudo apt-get update && sudo apt-get install -y libpcap-dev + run: sudo apt-get update && sudo apt-get install -y libpcap-dev librpm-dev - name: golangci-lint env: diff --git a/x-pack/auditbeat/auditbeat.reference.yml b/x-pack/auditbeat/auditbeat.reference.yml index 438ae307b80d..191c993d6d7d 100644 --- a/x-pack/auditbeat/auditbeat.reference.yml +++ b/x-pack/auditbeat/auditbeat.reference.yml @@ -205,6 +205,14 @@ auditbeat.modules: # socket.state.period: 12h # user.state.period: 12h + # Use setreuid() to drop out of root before making any calls to the RPM backend. + # This is exclusively useful for older rpm versions that rely on BDB as a backend; + # BDB does not parallelize well, and multiple applications attempting to connect to + # RPM's BDB backend may result in crashes and database corruption. By dropping out of root, + # we eliminate the chances of any corruption happening should auditbeat conflict with any other + # application attempting to use RPM/BDB + # package.rpm_drop_to_uid: 1000 + # Average file read rate for hashing of the process executable. Default is "50 MiB". process.hash.scan_rate_per_sec: 50 MiB diff --git a/x-pack/auditbeat/module/system/_meta/config.yml.tmpl b/x-pack/auditbeat/module/system/_meta/config.yml.tmpl index 5f19ccf0ce37..a1907f7e4b07 100644 --- a/x-pack/auditbeat/module/system/_meta/config.yml.tmpl +++ b/x-pack/auditbeat/module/system/_meta/config.yml.tmpl @@ -45,6 +45,14 @@ # user.state.period: 12h {{- end }} + # Use setreuid() to drop out of root before making any calls to the RPM backend. + # This is exclusively useful for older rpm versions that rely on BDB as a backend; + # BDB does not parallelize well, and multiple applications attempting to connect to + # RPM's BDB backend may result in crashes and database corruption. By dropping out of root, + # we eliminate the chances of any corruption happening should auditbeat conflict with any other + # application attempting to use RPM/BDB + # package.rpm_drop_to_uid: 1000 + # Average file read rate for hashing of the process executable. Default is "50 MiB". process.hash.scan_rate_per_sec: 50 MiB diff --git a/x-pack/auditbeat/module/system/package/config.go b/x-pack/auditbeat/module/system/package/config.go index 556f0e991454..ecd2ba8052a5 100644 --- a/x-pack/auditbeat/module/system/package/config.go +++ b/x-pack/auditbeat/module/system/package/config.go @@ -14,6 +14,9 @@ import ( type config struct { StatePeriod time.Duration `config:"state.period"` PackageStatePeriod time.Duration `config:"package.state.period"` + // PackageSuidDrop runs RPM queries with suid to drop out of root + // see the comment in package.go for more context + PackageSuidDrop *int64 `config:"package.rpm_drop_to_uid"` } func (c *config) effectiveStatePeriod() time.Duration { diff --git a/x-pack/auditbeat/module/system/package/package.go b/x-pack/auditbeat/module/system/package/package.go index 66d85bb7f17a..944bc083b332 100644 --- a/x-pack/auditbeat/module/system/package/package.go +++ b/x-pack/auditbeat/module/system/package/package.go @@ -17,8 +17,10 @@ import ( "io/fs" "os" "path/filepath" + "runtime" "strconv" "strings" + "syscall" "time" "github.com/cespare/xxhash/v2" @@ -233,6 +235,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { ms.log.Debug("No state timestamp found.") } + if config.PackageSuidDrop != nil { + if os.Getuid() != 0 && int(*ms.config.PackageSuidDrop) != os.Getuid() { + return nil, fmt.Errorf("package.rpm_drop_to_suid is set to %d, but we're running as a different non-root user", *config.PackageSuidDrop) + } + ms.log.Debugf("Dropping to EUID %d from UID %d for RPM API calls", *ms.config.PackageSuidDrop, os.Getuid()) + } + packages, err := loadPackages(ms.bucket) if err != nil { return nil, fmt.Errorf("failed to load persisted package metadata from disk: %w", err) @@ -293,7 +302,7 @@ func (ms *MetricSet) reportState(report mb.ReporterV2) error { for _, pkg := range packages { event := ms.packageEvent(pkg, eventTypeState, eventActionExistingPackage) - event.RootFields.Put("event.id", stateID.String()) //nolint:errcheck // This will not return an error as long as 'event' remains as a map. + _, _ = event.RootFields.Put("event.id", stateID.String()) report.Event(event) } @@ -481,26 +490,71 @@ func storePackages(bucket datastore.Bucket, packages []*Package) error { return nil } -func (ms *MetricSet) getPackages() (packages []*Package, err error) { +func (ms *MetricSet) getPackages() ([]*Package, error) { + packages := []*Package{} var foundPackageManager bool - - _, err = os.Stat(rpmPath) - if err == nil { + _, statErr := os.Stat(rpmPath) + if statErr == nil { foundPackageManager = true + if ms.config.PackageSuidDrop != nil { + + // This is rather ugly. + // Basically, older RPM setups will use BDB as a database for the RPM state, and + // BDB is incredibly easy to corrupt and does not handle parallel operations well. + // see https://github.com/rpm-software-management/rpm/issues/232 + // The easiest way around this is to drop perms to non-root, so librpm can't write to any of the DB files. + // this means we can't corrupt anything, and it also means that BDB won't perform any of the failchk() + // operations that exibit some parallel access issues + // HOWEVER this is technically non-POSIX-compliant, as posix expects all threads in the process to + // have identical perms. + + // lock our setreuid to one thread + runtime.LockOSThread() + doUnlock := true + defer func() { + // if for some reason the second setreuid call fails, we don't + // want to release the OS thread, as we'll have a non-root thread floating around that + // the go scheduler could assign to something that expects root permissions. + if doUnlock { + runtime.UnlockOSThread() + } else { + ms.log.Debugf("setreuid has failed; package query thread will remain locked") + } + }() - rpmPackages, err := listRPMPackages() - if err != nil { - return nil, fmt.Errorf("error getting RPM packages: %w", err) + minus1 := -1 + currentUID := os.Getuid() + _, _, serr := syscall.Syscall(syscall.SYS_SETREUID, uintptr(minus1), uintptr(*ms.config.PackageSuidDrop), uintptr(minus1)) + if serr != 0 { + return nil, fmt.Errorf("got error from setreuid trying to drop out of root: %w", serr) + } + + rpmPackages, err := listRPMPackages(true) + if err != nil { + return nil, fmt.Errorf("got error listing RPM packages: %w", err) + } + + _, _, serr = syscall.Syscall(syscall.SYS_SETREUID, uintptr(minus1), uintptr(currentUID), uintptr(minus1)) + if serr != 0 { + doUnlock = false + return nil, fmt.Errorf("got error from setreuid trying to reset euid: %w", serr) + } + + packages = append(packages, rpmPackages...) + } else { + rpmPackages, err := listRPMPackages(false) + if err != nil { + return nil, fmt.Errorf("error listing RPM packages: %w", err) + } + packages = append(packages, rpmPackages...) } - ms.log.Debugf("RPM packages: %v", len(rpmPackages)) - packages = append(packages, rpmPackages...) - } else if err != nil && !os.IsNotExist(err) { - return nil, fmt.Errorf("error opening %v: %w", rpmPath, err) + } else if !os.IsNotExist(statErr) { + return nil, fmt.Errorf("error opening %v: %w", rpmPath, statErr) } - _, err = os.Stat(dpkgPath) - if err == nil { + _, statErr = os.Stat(dpkgPath) + if statErr == nil { foundPackageManager = true dpkgPackages, err := ms.listDebPackages() @@ -510,17 +564,17 @@ func (ms *MetricSet) getPackages() (packages []*Package, err error) { ms.log.Debugf("DEB packages: %v", len(dpkgPackages)) packages = append(packages, dpkgPackages...) - } else if err != nil && !os.IsNotExist(err) { - return nil, fmt.Errorf("error opening %v: %w", dpkgPath, err) + } else if !os.IsNotExist(statErr) { + return nil, fmt.Errorf("error opening %v: %w", dpkgPath, statErr) } for _, path := range homebrewCellarPath { - _, err = os.Stat(path) - if err != nil { - if errors.Is(err, fs.ErrNotExist) { + _, statErr = os.Stat(path) + if statErr != nil { + if errors.Is(statErr, fs.ErrNotExist) { continue } - return nil, fmt.Errorf("error opening %v: %w", path, err) + return nil, fmt.Errorf("error opening %v: %w", path, statErr) } foundPackageManager = true homebrewPackages, err := listBrewPackages(path) diff --git a/x-pack/auditbeat/module/system/package/package_linux_test.go b/x-pack/auditbeat/module/system/package/package_linux_test.go new file mode 100644 index 000000000000..16d67b1b43c0 --- /dev/null +++ b/x-pack/auditbeat/module/system/package/package_linux_test.go @@ -0,0 +1,91 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +//go:build linux + +package pkg + +import ( + "os" + "strconv" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/elastic/beats/v7/x-pack/auditbeat/module/system/user" + "github.com/elastic/elastic-agent-libs/logp" +) + +func TestRPMParallel(t *testing.T) { + currentUID := os.Getuid() + if currentUID != 0 { + t.Skipf("can only run as root") + } + logp.DevelopmentSetup() + + count := 20 + waiter := sync.WaitGroup{} + waiter.Add(count) + + useUID := getUser(t) + + t.Logf("Starting...") + for i := 0; i < count; i++ { + inner := i + go func() { + defer waiter.Done() + testMs := MetricSet{ + log: logp.L(), + config: config{ + PackageSuidDrop: &useUID, + }, + } + + pkgList, err := testMs.getPackages() + require.NoError(t, err) + + t.Logf("got %d packages from %d", len(pkgList), inner) + }() + + } + waiter.Wait() +} + +func TestWithSuid(t *testing.T) { + currentUID := os.Getuid() + if currentUID != 0 { + t.Skipf("can only run as root") + } + useUID := getUser(t) + testMs := MetricSet{ + log: logp.L(), + config: config{ + PackageSuidDrop: &useUID, + }, + } + + packages, err := testMs.getPackages() + require.NoError(t, err) + + require.NotZero(t, packages) + t.Logf("got %d packages", len(packages)) +} + +func getUser(t *testing.T) int64 { + // pick a user to drop to + userList, err := user.GetUsers(false) + require.NoError(t, err) + + var useUID int64 + for _, user := range userList { + if user.UID != "0" { + newUID, err := strconv.ParseInt(user.UID, 10, 64) + require.NoError(t, err) + useUID = newUID + break + } + } + return useUID +} diff --git a/x-pack/auditbeat/module/system/package/package_test.go b/x-pack/auditbeat/module/system/package/package_test.go index f01c94aed710..4163ad20b436 100644 --- a/x-pack/auditbeat/module/system/package/package_test.go +++ b/x-pack/auditbeat/module/system/package/package_test.go @@ -21,6 +21,7 @@ import ( "github.com/elastic/beats/v7/auditbeat/core" "github.com/elastic/beats/v7/auditbeat/datastore" abtest "github.com/elastic/beats/v7/auditbeat/testing" + "github.com/elastic/beats/v7/metricbeat/mb" mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing" "github.com/elastic/beats/v7/x-pack/auditbeat/module/system" diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index e2b7489675de..75384ffe7f61 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -19,6 +19,7 @@ import ( ) /* + #include #include @@ -27,6 +28,15 @@ import ( #include #include + +int +my_rpmtsSetRootDir(void *f, rpmts ts) { + int (*rpmtsSetRootDir)(rpmts, const char*); + rpmtsSetRootDir = (int (*)(rpmts, const char*))f; + + return rpmtsSetRootDir(ts, "/"); +} + rpmts my_rpmtsCreate(void *f) { rpmts (*rpmtsCreate)(); @@ -196,6 +206,7 @@ type librpm struct { rpmsqSetInterruptSafety unsafe.Pointer rpmFreeRpmrc unsafe.Pointer rpmFreeMacros unsafe.Pointer + rpmtsSetRootDir unsafe.Pointer } func (lib *librpm) close() error { @@ -255,7 +266,7 @@ func openLibrpm() (*librpm, error) { librpm.handle, err = dlopen.GetHandle(librpmNames) if err != nil { - return nil, fmt.Errorf("couldn't open %v: %v", librpmNames, err) + return nil, fmt.Errorf("couldn't open %v: %w", librpmNames, err) } librpm.rpmtsCreate, err = librpm.handle.GetSymbolPointer("rpmtsCreate") @@ -314,24 +325,32 @@ func openLibrpm() (*librpm, error) { } // Only available in librpm>=4.13.0 - librpm.rpmsqSetInterruptSafety, err = librpm.handle.GetSymbolPointer("rpmsqSetInterruptSafety") + librpm.rpmsqSetInterruptSafety, _ = librpm.handle.GetSymbolPointer("rpmsqSetInterruptSafety") // no error check // Only available in librpm>=4.6.0 - librpm.rpmFreeMacros, err = librpm.handle.GetSymbolPointer("rpmFreeMacros") + librpm.rpmFreeMacros, _ = librpm.handle.GetSymbolPointer("rpmFreeMacros") // no error check + librpm.rpmtsSetRootDir, err = librpm.handle.GetSymbolPointer("rpmtsSetRootDir") + if err != nil { + return nil, err + } + return &librpm, nil } -func listRPMPackages() ([]*Package, error) { +func listRPMPackages(ownsThread bool) ([]*Package, error) { // In newer versions, librpm is using the thread-local variable // `disableInterruptSafety` in rpmio/rpmsq.c to disable signal // traps. To make sure our settings remain in effect throughout // our function calls we have to lock the OS thread here, since // Golang can otherwise use any thread it likes for each C.* call. - runtime.LockOSThread() - defer runtime.UnlockOSThread() + + if !ownsThread { + runtime.LockOSThread() + defer runtime.UnlockOSThread() + } if openedLibrpm == nil { var err error @@ -341,6 +360,15 @@ func listRPMPackages() ([]*Package, error) { } } + res := C.my_rpmReadConfigFiles(openedLibrpm.rpmReadConfigFiles) + if int(res) != 0 { + return nil, fmt.Errorf("Error: %d", int(res)) + } + defer C.my_rpmFreeRpmrc(openedLibrpm.rpmFreeRpmrc) + if openedLibrpm.rpmFreeMacros != nil { + defer C.my_rpmFreeMacros(openedLibrpm.rpmFreeMacros) + } + if openedLibrpm.rpmsqSetInterruptSafety != nil { C.my_rpmsqSetInterruptSafety(openedLibrpm.rpmsqSetInterruptSafety, 0) } @@ -351,14 +379,8 @@ func listRPMPackages() ([]*Package, error) { } defer C.my_rpmtsFree(openedLibrpm.rpmtsFree, rpmts) - res := C.my_rpmReadConfigFiles(openedLibrpm.rpmReadConfigFiles) - if int(res) != 0 { - return nil, fmt.Errorf("Error: %d", int(res)) - } - defer C.my_rpmFreeRpmrc(openedLibrpm.rpmFreeRpmrc) - if openedLibrpm.rpmFreeMacros != nil { - defer C.my_rpmFreeMacros(openedLibrpm.rpmFreeMacros) - } + // setup root dir, used if librpm has to resolve a macro that contains a path + C.my_rpmtsSetRootDir(openedLibrpm.rpmtsSetRootDir, rpmts) mi := C.my_rpmtsInitIterator(openedLibrpm.rpmtsInitIterator, rpmts) if mi == nil { @@ -366,7 +388,7 @@ func listRPMPackages() ([]*Package, error) { } defer C.my_rpmdbFreeIterator(openedLibrpm.rpmdbFreeIterator, mi) - var packages []*Package + packages := make([]*Package, 0) for header := C.my_rpmdbNextIterator(openedLibrpm.rpmdbNextIterator, mi); header != nil; header = C.my_rpmdbNextIterator(openedLibrpm.rpmdbNextIterator, mi) { pkg, err := packageFromHeader(header, openedLibrpm) diff --git a/x-pack/auditbeat/module/system/package/rpm_linux_test.go b/x-pack/auditbeat/module/system/package/rpm_linux_test.go index ff0c50129037..20d5ccd2f6c8 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux_test.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux_test.go @@ -27,7 +27,7 @@ func TestRPMPackages(t *testing.T) { t.Fatal(err) } - packages, err := listRPMPackages() + packages, err := listRPMPackages(false) if err != nil { t.Fatal(err) } diff --git a/x-pack/auditbeat/module/system/package/rpm_others.go b/x-pack/auditbeat/module/system/package/rpm_others.go index dc2755eebf65..331f5988c57f 100644 --- a/x-pack/auditbeat/module/system/package/rpm_others.go +++ b/x-pack/auditbeat/module/system/package/rpm_others.go @@ -8,7 +8,7 @@ package pkg import "errors" -func listRPMPackages() ([]*Package, error) { +func listRPMPackages(_ bool) ([]*Package, error) { return nil, errors.New("listing RPM packages is only supported on Linux") } diff --git a/x-pack/auditbeat/seccomp_linux.go b/x-pack/auditbeat/seccomp_linux.go index 9bde50c77d03..709d973465d6 100644 --- a/x-pack/auditbeat/seccomp_linux.go +++ b/x-pack/auditbeat/seccomp_linux.go @@ -19,6 +19,7 @@ func init() { if err := seccomp.ModifyDefaultPolicy(seccomp.AddSyscall, "mremap", "umask", + "setreuid", ); err != nil { panic(err) }