providers/linux: Fix linux Capabilities #196

haesbaert · 2023-12-04T10:34:03Z

I believe this never worked, as readCapabilities() expected a Capabilities line but was passed the full file as input, it also would allocate its own CapabilityInfo and return it, even though it would fill one member.

So change all this:

Scan the file line by line, as to avoid loading a full file into memory,
then parse out only the desired lines and pass down.
Pass the structure as a parameter and let each member be filled.
Error out if we failed to report at least one of the capabilities.
Add a test against init(1) as it will """always""" be run as root and have
all capabilities, test is a bit conservative but should be ok for now.

cla-checker-service · 2023-12-04T10:34:07Z

💚 CLA has been signed

I believe this never worked, as readCapabilities() expected a Capabilities line but was passed the full file as input, it also would allocate its own CapabilityInfo and return it, even though it would fill one member. So change all this: o Scan the file line by line, as to avoid loading a full file into memory, then parse out only the desired lines and pass down. o Pass the structure as a parameter and let each member be filled. o Error out if we failed to report at least one of the capabilities. o Add a test against init(1) as it will """always""" be run as root and have all capabilities, test is a bit conservative but should be ok for now.

pkoutsovasilis

two minor points which I don't deem as critical to action @haesbaert . Let me know your thoughts

pkoutsovasilis · 2023-12-04T11:16:36Z

providers/linux/capabilities_linux.go

-
-	err := parseKeyValue(content, ':', func(key, value []byte) error {
+func decodeCapabilityLine(content string, capInfo *types.CapabilityInfo) error {
+	return parseKeyValue([]byte(content), ':', func(key, value []byte) error {
 		var err error
 		switch string(key) {
 		case "CapInh":


although this code was already like that, I debate inside me if it would add more value having all Cap* strings defined as constants. Feel free to ignore

Ack, that makes sense, I was aiming to be conservative and using what's already there though.

I made a little snippet that gets all of the CAPs if you think it might be helpful here

package main import ( "fmt" "go/constant" "go/types" "os" // "golang.org/x/sys/unix" "golang.org/x/tools/go/packages" ) func main() { cfg := &packages.Config{Mode: packages.NeedTypes} pkgs, err := packages.Load(cfg, "golang.org/x/sys/unix") if err != nil { fmt.Println(err) os.Exit(1) } caps := map[string]uint64{} for _, pkg := range pkgs { scope := pkg.Types.Scope() for _, t := range scope.Names() { if len(t) > 4 && t[0:4] == "CAP_" { t_v, err := types.Eval(pkg.Fset, pkg.Types, scope.Pos(), t) if err == nil { if flagbits, ok := constant.Uint64Val(t_v.Value); ok { caps[t] = flagbits } } } } } for key, val := range caps { fmt.Printf("%v: %v\n", key, val) } }

pkoutsovasilis · 2023-12-04T11:21:22Z

providers/linux/process_linux.go

+			continue
+		}
+		err = decodeCapabilityLine(line, &capInfo)
+		if err != nil && gotErr == nil {


maybe we should do a "multierror" here? As it captures the error only for the first capability that fails and we lose the reason for any subsequent similar ones.

hmm I'm ok with just returning the first one though, as it's 99.999999% (number took out of my hat) the reason of the possible subsequent failures

Tacklebox

Points of discussion from slack aside, I think this looks good :) 🚢

Tacklebox · 2023-12-04T14:24:08Z

providers/linux/capabilities_linux.go

-
-	err := parseKeyValue(content, ':', func(key, value []byte) error {
+func decodeCapabilityLine(content string, capInfo *types.CapabilityInfo) error {
+	return parseKeyValue([]byte(content), ':', func(key, value []byte) error {
 		var err error
 		switch string(key) {
 		case "CapInh":


I made a little snippet that gets all of the CAPs if you think it might be helpful here

package main import ( "fmt" "go/constant" "go/types" "os" // "golang.org/x/sys/unix" "golang.org/x/tools/go/packages" ) func main() { cfg := &packages.Config{Mode: packages.NeedTypes} pkgs, err := packages.Load(cfg, "golang.org/x/sys/unix") if err != nil { fmt.Println(err) os.Exit(1) } caps := map[string]uint64{} for _, pkg := range pkgs { scope := pkg.Types.Scope() for _, t := range scope.Names() { if len(t) > 4 && t[0:4] == "CAP_" { t_v, err := types.Eval(pkg.Fset, pkg.Types, scope.Pos(), t) if err == nil { if flagbits, ok := constant.Uint64Val(t_v.Value); ok { caps[t] = flagbits } } } } } for key, val := range caps { fmt.Printf("%v: %v\n", key, val) } }

Include capabilities as described in: https://www.elastic.co/guide/en/ecs/master/ecs-process.html#field-process-thread-capabilities-effective Don't merge, this depends on two external PRs: elastic/go-sysinfo#196 elastic/go-sysinfo#197

@timestamp

Implements #36404 ECS: https://www.elastic.co/guide/en/ecs/master/ecs-process.html#field-process-thread-capabilities-effective Example output: ``` { "@timestamp": "2023-12-05T19:34:54.425Z", "@metadata": { "beat": "auditbeat", "type": "_doc", "version": "8.12.0" }, "process": { "thread": { "capabilities": { "effective": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ], "permitted": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ] } }, "entity_id": "DADEDQU03GoDNhc1", "pid": 2841325, "start": "2023-12-05T19:32:53.180Z", "args": [ "systemd-userwork: waiting..." ], ... ... ``` Don't merge, this depends on two external PRs: elastic/go-sysinfo#196 elastic/go-sysinfo#197 Next step is adding the same to add_process_metadata

@timestamp

Implements #36404 ECS: https://www.elastic.co/guide/en/ecs/master/ecs-process.html#field-process-thread-capabilities-effective Example output: ``` { "@timestamp": "2023-12-05T19:34:54.425Z", "@metadata": { "beat": "auditbeat", "type": "_doc", "version": "8.12.0" }, "process": { "thread": { "capabilities": { "effective": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ], "permitted": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ] } }, "entity_id": "DADEDQU03GoDNhc1", "pid": 2841325, "start": "2023-12-05T19:32:53.180Z", "args": [ "systemd-userwork: waiting..." ], ... ... ``` Implementation is pretty straightforward, go-sysinfo will parse /proc/$PID/status and fill in CapabilityInfo. Don't merge, this depends on two external PRs: elastic/go-sysinfo#196 elastic/go-sysinfo#197 Next step is adding the same to add_process_metadata

@timestamp

Implements #36404 ECS: https://www.elastic.co/guide/en/ecs/master/ecs-process.html#field-process-thread-capabilities-effective Example output: ``` { "@timestamp": "2023-12-05T19:34:54.425Z", "@metadata": { "beat": "auditbeat", "type": "_doc", "version": "8.12.0" }, "process": { "thread": { "capabilities": { "effective": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ], "permitted": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ] } }, "entity_id": "DADEDQU03GoDNhc1", "pid": 2841325, "start": "2023-12-05T19:32:53.180Z", "args": [ "systemd-userwork: waiting..." ], ... ... ``` Implementation is pretty straightforward, go-sysinfo will parse /proc/$PID/status and fill in CapabilityInfo. Don't merge, this depends on two external PRs: elastic/go-sysinfo#196 elastic/go-sysinfo#197 Next step is adding the same to add_process_metadata

@timestamp

Implements #36404 ECS: https://www.elastic.co/guide/en/ecs/master/ecs-process.html#field-process-thread-capabilities-effective Example output: ``` { "@timestamp": "2023-12-05T19:34:54.425Z", "@metadata": { "beat": "auditbeat", "type": "_doc", "version": "8.12.0" }, "process": { "thread": { "capabilities": { "effective": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ], "permitted": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ] } }, "entity_id": "DADEDQU03GoDNhc1", "pid": 2841325, "start": "2023-12-05T19:32:53.180Z", "args": [ "systemd-userwork: waiting..." ], ... ... ``` Implementation is pretty straightforward, go-sysinfo will parse /proc/$PID/status and fill in CapabilityInfo. Don't merge, this depends on two external PRs: elastic/go-sysinfo#196 elastic/go-sysinfo#197 Next step is adding the same to add_process_metadata

@timestamp

Implements #36404 ECS: https://www.elastic.co/guide/en/ecs/master/ecs-process.html#field-process-thread-capabilities-effective Example output: ``` { "@timestamp": "2023-12-05T19:34:54.425Z", "@metadata": { "beat": "auditbeat", "type": "_doc", "version": "8.12.0" }, "process": { "thread": { "capabilities": { "effective": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ], "permitted": [ "CAP_DAC_READ_SEARCH", "CAP_SYS_RESOURCE" ] } }, "entity_id": "DADEDQU03GoDNhc1", "pid": 2841325, "start": "2023-12-05T19:32:53.180Z", "args": [ "systemd-userwork: waiting..." ], ... ... ``` Implementation is pretty straightforward, go-sysinfo will parse /proc/$PID/status and fill in CapabilityInfo. Don't merge, this depends on two external PRs: elastic/go-sysinfo#196 elastic/go-sysinfo#197 Next step is adding the same to add_process_metadata

andrewkroh · 2023-12-06T17:28:22Z

I believe this never worked

😟 I checked the output of the tests on main and it reports a list of capabilities, so some part must have been working.

https://github.com/elastic/go-sysinfo/actions/runs/7030012626/job/19128722216#step:8:56

andrewkroh · 2023-12-06T17:30:57Z

providers/linux/process_linux.go

 func (p *process) Capabilities() (*types.CapabilityInfo, error) {
-	content, err := ioutil.ReadFile(p.path("status"))
+	var gotErr error
+	f, err := os.OpenFile(p.path("status"), os.O_RDONLY, 0644)


I think https://pkg.go.dev/os#Open would be more clear.

absolutely, I'm new to Go so expect weirdness, I'll fix it.

andrewkroh · 2023-12-06T17:31:49Z

providers/linux/process_linux.go

 func (p *process) Capabilities() (*types.CapabilityInfo, error) {
-	content, err := ioutil.ReadFile(p.path("status"))
+	var gotErr error


Recommend moving the declaration down closer to first usage to minimize scope.

andrewkroh · 2023-12-06T17:35:26Z

providers/linux/process_linux.go

+	var capInfo types.CapabilityInfo
+	for scanner.Scan() {
+		line := scanner.Text()
+		if len(line) != 24 || line[:3] != "Cap" {


Without ensuring the length of the line, then line[:3] could panic.

Agreed, but it does check: if len(line) != 24 || line[:3] != "Cap"

andrewkroh · 2023-12-06T17:37:58Z

providers/linux/process_linux.go

+		if err != nil && gotErr == nil {
+			gotErr = err
+		}
+	}



If the scanner encounters an error then Scan() will return false, and the you will only see the error if you call https://pkg.go.dev/bufio#Scanner.Err. So this needs to check for that error after the loop exits.

Well spot, will fix it, many thanks

andrewkroh · 2023-12-06T17:43:16Z

system_test.go

+	if err != nil {
+		t.Fatal(err)
+	}
+	totalCaps := 41


This seem brittle as it depends on the host operating system. The results could vary by kernel. Like if someone develops on an older OS then their tests might fail.

Agreed, I wasn't sure if this is worth or not

andrewkroh · 2023-12-06T17:46:39Z

.changelog/196.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+linux: fix linux capabilities


As a reader of the release notes, I think I would prefer to have a few more details about what was broken.

haesbaert · 2023-12-06T17:54:13Z

I believe this never worked

😟 I checked the output of the tests on main and it reports a list of capabilities, so some part must have been working.

https://github.com/elastic/go-sysinfo/actions/runs/7030012626/job/19128722216#step:8:56

This is strange, the caller just does:

func (p *process) Capabilities() (*types.CapabilityInfo, error) {
	content, err := ioutil.ReadFile(p.path("status"))
	if err != nil {
		return nil, err
	}

	return readCapabilities(content)
}

How would content be parseable by:

func readCapabilities(content []byte) (*types.CapabilityInfo, error) {
	var cap types.CapabilityInfo

	err := parseKeyValue(content, ':', func(key, value []byte) error {
		var err error
...

For sure I must be missing something

andrewkroh · 2023-12-06T18:02:17Z

My recommendation would be to create a failing testing that demonstrates the problem using main (before your changes).

How would content be parseable by:

The main branch implementation takes the full content, passes it to parseKeyValue which splits each line, then invokes the callback function for each key/value (as separated by :).

haesbaert · 2023-12-06T18:06:18Z

My recommendation would be to create a failing testing that demonstrates the problem using main (before your changes).

How would content be parseable by:

The main branch implementation takes the full content, passes it to parseKeyValue which splits each line, then invokes the callback function for each key/value (as separated by :).

hA! I'm a dummy, apologies. I did test it before and it didn't work, maybe I missed something else, I'll check it.

andrewkroh · 2023-12-06T18:08:51Z

Running your TestCapabilities against main appears to mostly work. (I'm running it in a container from my mac so I assume there are some caps that my PID 1 does not have).

% docker run -it --rm -v $(pwd):/go-sysinfo --privileged -w /go-sysinfo golang:1.21 go test -v -run TestCapabilities .
=== RUN   TestCapabilities
    system_test.go:331: 
                Error Trace:    system_test.go:331
                Error:          Not equal: 
                                expected: 38
                                actual  : 41
                Test:           TestCapabilities
    system_test.go:332: 
                Error Trace:    system_test.go:332
                Error:          Not equal: 
                                expected: 38
                                actual  : 41
                Test:           TestCapabilities
    system_test.go:333: 
                Error Trace:    system_test.go:333
                Error:          Not equal: 
                                expected: 38
                                actual  : 41
                Test:           TestCapabilities
--- FAIL: TestCapabilities (0.00s)

haesbaert · 2023-12-06T18:29:45Z

Running your TestCapabilities against main appears to mostly work. (I'm running it in a container from my mac so I assume there are some caps that my PID 1 does not have).

% docker run -it --rm -v $(pwd):/go-sysinfo --privileged -w /go-sysinfo golang:1.21 go test -v -run TestCapabilities .
=== RUN   TestCapabilities
    system_test.go:331: 
                Error Trace:    system_test.go:331
                Error:          Not equal: 
                                expected: 38
                                actual  : 41
                Test:           TestCapabilities
    system_test.go:332: 
                Error Trace:    system_test.go:332
                Error:          Not equal: 
                                expected: 38
                                actual  : 41
                Test:           TestCapabilities
    system_test.go:333: 
                Error Trace:    system_test.go:333
                Error:          Not equal: 
                                expected: 38
                                actual  : 41
                Test:           TestCapabilities
--- FAIL: TestCapabilities (0.00s)

I just ran main against my auditbeat change and indeed it works, I've been chasing a red herring, sorry for the noise.

haesbaert force-pushed the fixcap branch 2 times, most recently from 4c0b74d to ca253f7 Compare December 4, 2023 10:42

haesbaert force-pushed the fixcap branch from ca253f7 to 6ad5c63 Compare December 4, 2023 10:58

pkoutsovasilis reviewed Dec 4, 2023

View reviewed changes

pkoutsovasilis approved these changes Dec 4, 2023

View reviewed changes

Tacklebox approved these changes Dec 4, 2023

View reviewed changes

haesbaert mentioned this pull request Dec 5, 2023

providers/linux: Export capabilities as CAP_UPPERCASE #197

Draft

This was referenced Dec 5, 2023

[Auditbeat] system.process - report Linux capabilities elastic/beats#36404

Open

add_process_metadata - report linux capabilities elastic/beats#36403

Closed

haesbaert mentioned this pull request Dec 6, 2023

x-pack/auditbeat/module/system/process Report Linux capabilities elastic/beats#37303

Closed

6 tasks

andrewkroh reviewed Dec 6, 2023

View reviewed changes

haesbaert marked this pull request as draft December 7, 2023 09:42

andrewkroh closed this Mar 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

providers/linux: Fix linux Capabilities #196

providers/linux: Fix linux Capabilities #196

haesbaert commented Dec 4, 2023

cla-checker-service bot commented Dec 4, 2023 •

edited

Loading

pkoutsovasilis left a comment

pkoutsovasilis Dec 4, 2023

haesbaert Dec 4, 2023

Tacklebox Dec 4, 2023

pkoutsovasilis Dec 4, 2023

haesbaert Dec 4, 2023

Tacklebox left a comment

Tacklebox Dec 4, 2023

andrewkroh commented Dec 6, 2023

andrewkroh Dec 6, 2023

haesbaert Dec 6, 2023

andrewkroh Dec 6, 2023

andrewkroh Dec 6, 2023

haesbaert Dec 6, 2023

andrewkroh Dec 6, 2023

haesbaert Dec 6, 2023

andrewkroh Dec 6, 2023 •

edited

Loading

haesbaert Dec 6, 2023

andrewkroh Dec 6, 2023

haesbaert commented Dec 6, 2023 •

edited

Loading

andrewkroh commented Dec 6, 2023

haesbaert commented Dec 6, 2023

andrewkroh commented Dec 6, 2023

haesbaert commented Dec 6, 2023 •

edited

Loading

		@@ -0,0 +1,3 @@
		```release-note:bug
		linux: fix linux capabilities

providers/linux: Fix linux Capabilities #196

providers/linux: Fix linux Capabilities #196

Conversation

haesbaert commented Dec 4, 2023

cla-checker-service bot commented Dec 4, 2023 • edited Loading

pkoutsovasilis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tacklebox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haesbaert commented Dec 6, 2023 • edited Loading

andrewkroh commented Dec 6, 2023

haesbaert commented Dec 6, 2023

andrewkroh commented Dec 6, 2023

haesbaert commented Dec 6, 2023 • edited Loading

cla-checker-service bot commented Dec 4, 2023 •

edited

Loading

andrewkroh Dec 6, 2023 •

edited

Loading

haesbaert commented Dec 6, 2023 •

edited

Loading

haesbaert commented Dec 6, 2023 •

edited

Loading