From 8d31ef19b0a17d895aa80fb5338e6b69327d570b Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 13 Jul 2021 11:30:55 +0200 Subject: [PATCH] add_process_metadata: Support different integer types for pid field (#26829) (#26837) Fixes a bug with the add_process_metadata processor where sometimes the lookup PID specified in match_pids cannot be parsed even if it's a valid integer. This is caused by the processor expecting the field to be of int type, while depending on how the field is populated, it can be other types, usually an int64 if the source is a json document. Fixes #26830 (cherry picked from commit 7be6e5e4b6451e5cc72c8a2f0b67b30516bb268d) Co-authored-by: Adrian Serrano --- CHANGELOG.next.asciidoc | 1 + .../add_process_metadata.go | 38 +++-- .../add_process_metadata_test.go | 160 +++++++++++++++++- 3 files changed, 187 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index ecc3dddf720..ad54989e9c4 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -110,6 +110,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Add service resource in k8s cluster role. {pull}20546[20546] - [Metricbeat][Kubernetes] Change cluster_ip field from ip to keyword. {pull}20571[20571] - The `o365input` and `o365` module now recover from an authentication problem or other fatal errors, instead of terminating. {pull}21258[21258] +- Fix `add_process_metadata` processor complaining about valid pid fields not being valid integers. {pull}26829[26829] {issue}26830[26830] *Auditbeat* diff --git a/libbeat/processors/add_process_metadata/add_process_metadata.go b/libbeat/processors/add_process_metadata/add_process_metadata.go index 01e2cf1e9fe..5b8c9b4fdfd 100644 --- a/libbeat/processors/add_process_metadata/add_process_metadata.go +++ b/libbeat/processors/add_process_metadata/add_process_metadata.go @@ -19,6 +19,7 @@ package add_process_metadata import ( "fmt" + "reflect" "strconv" "time" @@ -182,23 +183,40 @@ func (p *addProcessMetadata) Run(event *beat.Event) (*beat.Event, error) { return event, ErrNoMatch } -func (p *addProcessMetadata) enrich(event common.MapStr, pidField string) (result common.MapStr, err error) { - pidIf, err := event.GetValue(pidField) - if err != nil { - return nil, err - } - - var pid int - switch v := pidIf.(type) { +func pidToInt(value interface{}) (pid int, err error) { + switch v := value.(type) { case string: pid, err = strconv.Atoi(v) if err != nil { - return nil, errors.Wrapf(err, "cannot convert string field '%s' to an integer", pidField) + return 0, errors.Wrap(err, "error converting string to integer") } case int: pid = v + case int8, int16, int32, int64: + pid64 := reflect.ValueOf(v).Int() + if pid = int(pid64); int64(pid) != pid64 { + return 0, errors.Errorf("integer out of range: %d", pid64) + } + case uint, uintptr, uint8, uint16, uint32, uint64: + pidu64 := reflect.ValueOf(v).Uint() + if pid = int(pidu64); pid < 0 || uint64(pid) != pidu64 { + return 0, errors.Errorf("integer out of range: %d", pidu64) + } default: - return nil, errors.Errorf("cannot parse field '%s' (not an integer or string)", pidField) + return 0, errors.Errorf("not an integer or string, but %T", v) + } + return pid, nil +} + +func (p *addProcessMetadata) enrich(event common.MapStr, pidField string) (result common.MapStr, err error) { + pidIf, err := event.GetValue(pidField) + if err != nil { + return nil, err + } + + pid, err := pidToInt(pidIf) + if err != nil { + return nil, errors.Wrapf(err, "cannot parse pid field '%s'", pidField) } var meta common.MapStr diff --git a/libbeat/processors/add_process_metadata/add_process_metadata_test.go b/libbeat/processors/add_process_metadata/add_process_metadata_test.go index 493034098cf..d60b1fc4bc9 100644 --- a/libbeat/processors/add_process_metadata/add_process_metadata_test.go +++ b/libbeat/processors/add_process_metadata/add_process_metadata_test.go @@ -18,9 +18,11 @@ package add_process_metadata import ( + "math" "os" "testing" "time" + "unsafe" "github.com/pkg/errors" "github.com/stretchr/testify/assert" @@ -403,7 +405,7 @@ func TestAddProcessMetadata(t *testing.T) { expected: common.MapStr{ "ppid": "a", }, - err: errors.New("error applying add_process_metadata processor: cannot convert string field 'ppid' to an integer: strconv.Atoi: parsing \"a\": invalid syntax"), + err: errors.New("error applying add_process_metadata processor: cannot parse pid field 'ppid': error converting string to integer: strconv.Atoi: parsing \"a\": invalid syntax"), }, { description: "bad PID field type", @@ -416,7 +418,7 @@ func TestAddProcessMetadata(t *testing.T) { expected: common.MapStr{ "ppid": false, }, - err: errors.New("error applying add_process_metadata processor: cannot parse field 'ppid' (not an integer or string)"), + err: errors.New("error applying add_process_metadata processor: cannot parse pid field 'ppid': not an integer or string, but bool"), }, { description: "process not found", @@ -824,3 +826,157 @@ func TestBadProcess(t *testing.T) { assert.NotNil(t, result.Fields) assert.Equal(t, ev.Fields, result.Fields) } + +func TestPIDToInt(t *testing.T) { + const intIs64bit = unsafe.Sizeof(int(0)) == unsafe.Sizeof(int64(0)) + for _, test := range []struct { + name string + pid interface{} + fail bool + }{ + { + name: "numeric string", + pid: "1234", + }, + { + name: "numeric string ignore octal", + pid: "008", + }, + { + name: "numeric string invalid hex", + pid: "0x10", + fail: true, + }, + { + name: "non-numeric string", + pid: "abcd", + fail: true, + }, + { + name: "int", + pid: 0, + }, + { + name: "int min", + pid: math.MaxInt32, + }, + { + name: "int max", + pid: math.MaxInt32, + }, + { + name: "uint min", + pid: uint(0), + }, + { + name: "uint max", + pid: uint(math.MaxUint32), + fail: !intIs64bit, + }, + { + name: "int8", + pid: int8(0), + }, + { + name: "int8 min", + pid: int8(math.MinInt8), + }, + { + name: "int8 max", + pid: int8(math.MaxInt8), + }, + { + name: "uint8 min", + pid: uint8(0), + }, + { + name: "uint8 max", + pid: uint8(math.MaxUint8), + }, + { + name: "int16", + pid: int16(0), + }, + { + name: "int16 min", + pid: int16(math.MinInt16), + }, + { + name: "int16 max", + pid: int16(math.MaxInt16), + }, + { + name: "uint16 min", + pid: uint16(0), + }, + { + name: "uint16 max", + pid: uint16(math.MaxUint16), + }, + { + name: "int32", + pid: int32(0), + }, + { + name: "int32 min", + pid: int32(math.MinInt32), + }, + { + name: "int32 max", + pid: int32(math.MaxInt32), + }, + { + name: "uint32 min", + pid: uint32(0), + }, + { + name: "uint32 max", + pid: uint32(math.MaxUint32), + fail: !intIs64bit, + }, + { + name: "int64", + pid: int64(0), + fail: false, + }, + { + name: "int64 min", + pid: int64(math.MinInt64), + fail: !intIs64bit, + }, + { + name: "int64 max", + pid: int64(math.MaxInt64), + fail: !intIs64bit, + }, + { + name: "uint64 min", + pid: uint64(0), + fail: false, + }, + { + name: "uint64 max", + pid: uint64(math.MaxUint64), + fail: true, + }, + { + name: "uintptr", + pid: uintptr(0), + fail: false, + }, + { + name: "boolean", + pid: false, + fail: true, + }, + } { + t.Run(test.name, func(t *testing.T) { + _, err := pidToInt(test.pid) + if test.fail { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +}