Skip to content

Commit

Permalink
auditbeat/module/file_integrity: encode POSIXACLAccess as []byte (#36351
Browse files Browse the repository at this point in the history
)

When encoded as a string, the logic within the module works correctly,
but the logged data is passed through the JSON serialisation logic which
will ensure that all the unicode code points are valid. It is not
necessarily true that all the bytes in the data for this field are valid
(almost guaranteed not to be given that invalid qualifiers are
represented by 0xffffffff).

So use a []byte to hold this data. This has the additional advantage
that the representation in the logs will be almost exactly what getfattr
shows for this; the only difference being a "0s" prefix added by
getfattr.
  • Loading branch information
efd6 authored Aug 16, 2023
1 parent d771501 commit 7436895
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 17 deletions.
8 changes: 4 additions & 4 deletions auditbeat/module/file_integrity/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type Metadata struct {
SetGID bool `json:"setgid"` // setgid bit (POSIX only)
Origin []string `json:"origin"` // External origin info for the file (MacOS only)
SELinux string `json:"selinux"` // security.selinux xattr value (Linux only)
POSIXACLAccess string `json:"posix_acl_access"` // system.posix_acl_access xattr value (Linux only)
POSIXACLAccess []byte `json:"posix_acl_access"` // system.posix_acl_access xattr value (Linux only)
}

// NewEventFromFileInfo creates a new Event based on data from a os.FileInfo
Expand Down Expand Up @@ -326,8 +326,8 @@ func buildMetricbeatEvent(e *Event, existedBefore bool) mb.Event {
if info.SELinux != "" {
file["selinux"] = info.SELinux
}
if info.POSIXACLAccess != "" {
a, err := aclText([]byte(info.POSIXACLAccess))
if len(info.POSIXACLAccess) != 0 {
a, err := aclText(info.POSIXACLAccess)
if err == nil {
file["posix_acl_access"] = a
}
Expand Down Expand Up @@ -487,7 +487,7 @@ func diffEvents(old, new *Event) (Action, bool) {
// The owner and group names are ignored (they aren't persisted).
if o.Inode != n.Inode || o.UID != n.UID || o.GID != n.GID || o.SID != n.SID ||
o.Mode != n.Mode || o.Type != n.Type || o.SetUID != n.SetUID || o.SetGID != n.SetGID ||
o.SELinux != n.SELinux || o.POSIXACLAccess != n.POSIXACLAccess {
o.SELinux != n.SELinux || !bytes.Equal(o.POSIXACLAccess, n.POSIXACLAccess) {
result |= AttributesModified
}

Expand Down
14 changes: 8 additions & 6 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 @@ -69,16 +69,18 @@ func NewMetadata(path string, info os.FileInfo) (*Metadata, error) {
fileInfo.Owner = owner.Username
}

getExtendedAttributes(path, map[string]*string{
"security.selinux": &fileInfo.SELinux,
var selinux []byte
getExtendedAttributes(path, map[string]*[]byte{
"security.selinux": &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")
selinux, _, _ = bytes.Cut(selinux, []byte{0})
fileInfo.SELinux = string(selinux)

group, err := user.LookupGroupId(strconv.Itoa(int(fileInfo.GID)))
if err != nil {
Expand All @@ -92,7 +94,7 @@ func NewMetadata(path string, info os.FileInfo) (*Metadata, error) {
return fileInfo, errs.Err()
}

func getExtendedAttributes(path string, dst map[string]*string) {
func getExtendedAttributes(path string, dst map[string]*[]byte) {
f, err := os.Open(path)
if err != nil {
return
Expand All @@ -104,6 +106,6 @@ func getExtendedAttributes(path string, dst map[string]*string) {
if err != nil {
continue
}
*d = string(att)
*d = att
}
}
13 changes: 10 additions & 3 deletions auditbeat/module/file_integrity/flatbuffers.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ func fbWriteMetadata(b *flatbuffers.Builder, m *Metadata) flatbuffers.UOffsetT {
if m.SELinux != "" {
selinuxOffset = b.CreateString(m.SELinux)
}
if m.POSIXACLAccess != "" {
aclAccessOffset = b.CreateString(m.POSIXACLAccess)
if len(m.POSIXACLAccess) != 0 {
aclAccessOffset = b.CreateByteVector(m.POSIXACLAccess)
}
schema.MetadataStart(b)
schema.MetadataAddInode(b, m.Inode)
Expand Down Expand Up @@ -256,6 +256,13 @@ func fbDecodeMetadata(e *schema.Event) *Metadata {
return nil
}
mode := os.FileMode(info.Mode())
var posixACLAccess []byte
if n := info.PosixAclAccessLength(); n != 0 {
posixACLAccess = make([]byte, n)
for i := range posixACLAccess {
posixACLAccess[i] = byte(info.PosixAclAccess(i))
}
}
rtn := &Metadata{
Inode: info.Inode(),
UID: info.Uid(),
Expand All @@ -268,7 +275,7 @@ func fbDecodeMetadata(e *schema.Event) *Metadata {
SetUID: mode&os.ModeSetuid != 0,
SetGID: mode&os.ModeSetgid != 0,
SELinux: string(info.Selinux()),
POSIXACLAccess: string(info.PosixAclAccess()),
POSIXACLAccess: posixACLAccess,
}

switch info.Type() {
Expand Down
2 changes: 1 addition & 1 deletion auditbeat/module/file_integrity/schema.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ table Metadata {
ctime_ns:long;
type:Type = 1;
selinux:string;
posix_acl_access:string;
posix_acl_access:[byte];
}

table Hash {
Expand Down
27 changes: 24 additions & 3 deletions auditbeat/module/file_integrity/schema/Metadata.go

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

0 comments on commit 7436895

Please sign in to comment.