From 8586e1fd908ddf24054ba239f7b73dd77828b13e Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 12 Jul 2021 12:10:29 +0200 Subject: [PATCH] add_process_metadata: Support different integer types for pid field (#26829) (#26836) 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 430e4ce363e8..c58ba074a2b9 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -184,6 +184,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Omit full index template from errors that occur while loading the template. {pull}25743[25743] - In the script processor, the `decode_xml` and `decode_xml_wineventlog` processors are now available as `DecodeXML` and `DecodeXMLWineventlog` respectively. - Fix encoding errors when using the disk queue on nested data with multi-byte characters {pull}26484[26484] +- 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 01e2cf1e9fe9..5b8c9b4fdfdd 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 493034098cfe..d60b1fc4bc9c 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) + } + }) + } +}