Skip to content

Commit

Permalink
address pr comment
Browse files Browse the repository at this point in the history
  • Loading branch information
efd6 committed Aug 15, 2023
1 parent 4303194 commit cced9cc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 15 deletions.
9 changes: 2 additions & 7 deletions auditbeat/module/file_integrity/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"crypto/sha1"
"crypto/sha256"
"crypto/sha512"
"encoding/base64"
"encoding/binary"
"encoding/hex"
"fmt"
Expand Down Expand Up @@ -328,7 +327,7 @@ func buildMetricbeatEvent(e *Event, existedBefore bool) mb.Event {
file["selinux"] = info.SELinux
}
if info.POSIXACLAccess != "" {
a, err := aclText(info.POSIXACLAccess)
a, err := aclText([]byte(info.POSIXACLAccess))
if err == nil {
file["posix_acl_access"] = a
}
Expand Down Expand Up @@ -370,11 +369,7 @@ func buildMetricbeatEvent(e *Event, existedBefore bool) mb.Event {
return out
}

func aclText(s string) ([]string, error) {
b, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(s, "0s"))
if err != nil {
return nil, err
}
func aclText(b []byte) ([]string, error) {
if (len(b)-4)%8 != 0 {
return nil, fmt.Errorf("unexpected ACL length: %d", len(b))
}
Expand Down
13 changes: 12 additions & 1 deletion auditbeat/module/file_integrity/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package file_integrity

import (
"bytes"
"encoding/base64"
"encoding/hex"
"fmt"
"io/ioutil"
Expand All @@ -27,6 +28,7 @@ import (
"os/user"
"reflect"
"runtime"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -557,6 +559,10 @@ func assertHasKey(t testing.TB, m mapstr.M, key string) bool {
}

func TestACLText(t *testing.T) {
// The xattr package returns raw bytes, but command line tools such as getfattr
// return a base64-encoded format, so use that here to make test validation
// easier.
//
// Depending on the system we are running this test on, we may or may not
// have a username associated with the user's UID in the xattr string, so
// dynamically determine the username here.
Expand All @@ -574,7 +580,12 @@ func TestACLText(t *testing.T) {
},
}
for i, test := range tests {
got, err := aclText(test.encoded)
b, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(test.encoded, "0s"))
if err != nil {
t.Errorf("invalid test: unexpected base64 encoding error for test %d: %v", i, err)
continue
}
got, err := aclText(b)
if err != nil {
t.Errorf("unexpected error for test %d: %v", i, err)
continue
Expand Down
15 changes: 8 additions & 7 deletions auditbeat/module/file_integrity/fileinfo_posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
package file_integrity

import (
"bytes"
"fmt"
"os"
"os/user"
"strconv"
"strings"
"syscall"

"github.com/joeshaw/multierror"
Expand Down Expand Up @@ -73,6 +73,12 @@ func NewMetadata(path string, info os.FileInfo) (*Metadata, error) {
"security.selinux": &fileInfo.SELinux,
"system.posix_acl_access": &fileInfo.POSIXACLAccess,
})
// The selinux attr may be null terminated. It would be cheaper
// to use strings.TrimRight, but absent documentation saying
// that there is only ever a final null terminator, take the
// guaranteed correct path of terminating at the first found
// null byte.
fileInfo.SELinux, _, _ = strings.Cut(fileInfo.SELinux, "\x00")

group, err := user.LookupGroupId(strconv.Itoa(int(fileInfo.GID)))
if err != nil {
Expand All @@ -98,11 +104,6 @@ func getExtendedAttributes(path string, dst map[string]*string) {
if err != nil {
continue
}
*d = string(trimNull(att))
*d = string(att)
}
}

func trimNull(b []byte) []byte {
b, _, _ = bytes.Cut(b, []byte{0})
return b
}

0 comments on commit cced9cc

Please sign in to comment.