From cf7b643bfd56bbfcf5241ab2c555a501f606fbc0 Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Wed, 3 Jun 2020 18:27:03 -0400 Subject: [PATCH] Fix gob encoding of system/package struct data (#18887) The system/package dataset uses encoding/gob to encode and decode the Package struct data that it persists. The struct was updated to contain an error which is an interface type that's not serializable by default. I have made the field unexpected so that it is ignored by encoding/gob. Fixes #18536 --- CHANGELOG.next.asciidoc | 1 + .../module/system/package/package.go | 7 +- .../module/system/package/package_homebrew.go | 8 +- .../module/system/package/package_test.go | 73 ++++++++++++++++++ .../module/system/package/rpm_linux.go | 2 +- .../system/package/testdata/package.v1.gob | Bin 0 -> 241 bytes 6 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 x-pack/auditbeat/module/system/package/testdata/package.v1.gob diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 5ccd4b7d8ff..e8191fa04f7 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -120,6 +120,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - system/socket: Fixed compatibility issue with kernel 5.x. {pull}15771[15771] - system/package: Fix parsing of Installed-Size field of DEB packages. {issue}16661[16661] {pull}17188[17188] - system module: Fix panic during initialisation when /proc/stat can't be read. {pull}17569[17569] +- system/package: Fix an error that can occur while trying to persist package metadata. {issue}18536[18536] {pull}18887[18887] *Filebeat* diff --git a/x-pack/auditbeat/module/system/package/package.go b/x-pack/auditbeat/module/system/package/package.go index b6b11593520..f988030b8d0 100644 --- a/x-pack/auditbeat/module/system/package/package.go +++ b/x-pack/auditbeat/module/system/package/package.go @@ -121,8 +121,9 @@ type Package struct { Size uint64 Summary string URL string - Error error Type string + + error error } // Hash creates a hash for Package. @@ -389,8 +390,8 @@ func (ms *MetricSet) packageEvent(pkg *Package, eventType string, action eventAc event.MetricSetFields.Put("entity_id", pkg.entityID(ms.HostID())) } - if pkg.Error != nil { - event.RootFields.Put("error.message", pkg.Error.Error()) + if pkg.error != nil { + event.RootFields.Put("error.message", pkg.error.Error()) } return event diff --git a/x-pack/auditbeat/module/system/package/package_homebrew.go b/x-pack/auditbeat/module/system/package/package_homebrew.go index 063c99187a7..c3ec3dc5565 100644 --- a/x-pack/auditbeat/module/system/package/package_homebrew.go +++ b/x-pack/auditbeat/module/system/package/package_homebrew.go @@ -60,12 +60,12 @@ func listBrewPackages() ([]*Package, error) { installReceiptPath := path.Join(homebrewCellarPath, pkg.Name, pkg.Version, "INSTALL_RECEIPT.json") contents, err := ioutil.ReadFile(installReceiptPath) if err != nil { - pkg.Error = errors.Wrapf(err, "error reading %v", installReceiptPath) + pkg.error = errors.Wrapf(err, "error reading %v", installReceiptPath) } else { var installReceipt InstallReceipt err = json.Unmarshal(contents, &installReceipt) if err != nil { - pkg.Error = errors.Wrapf(err, "error unmarshalling JSON in %v", installReceiptPath) + pkg.error = errors.Wrapf(err, "error unmarshalling JSON in %v", installReceiptPath) } else { formulaPath = installReceipt.Source.Path } @@ -78,7 +78,7 @@ func listBrewPackages() ([]*Package, error) { file, err := os.Open(formulaPath) if err != nil { - pkg.Error = errors.Wrapf(err, "error reading %v", formulaPath) + pkg.error = errors.Wrapf(err, "error reading %v", formulaPath) } else { defer file.Close() @@ -97,7 +97,7 @@ func listBrewPackages() ([]*Package, error) { } } if err = scanner.Err(); err != nil { - pkg.Error = errors.Wrapf(err, "error parsing %v", formulaPath) + pkg.error = errors.Wrapf(err, "error parsing %v", formulaPath) } } diff --git a/x-pack/auditbeat/module/system/package/package_test.go b/x-pack/auditbeat/module/system/package/package_test.go index f25c40e9cb0..14d0abd03ab 100644 --- a/x-pack/auditbeat/module/system/package/package_test.go +++ b/x-pack/auditbeat/module/system/package/package_test.go @@ -7,9 +7,15 @@ package pkg import ( + "bytes" + "encoding/gob" + "errors" + "flag" + "io/ioutil" "path/filepath" "runtime" "testing" + "time" "github.com/stretchr/testify/assert" @@ -19,6 +25,8 @@ import ( mbtest "github.com/elastic/beats/v7/metricbeat/mb/testing" ) +var flagUpdateGob = flag.Bool("update-gob", false, "update persisted gob testdata") + func TestData(t *testing.T) { if runtime.GOOS == "darwin" { t.Skip("FIXME: https://github.com/elastic/beats/issues/18855") @@ -160,3 +168,68 @@ func getConfig() map[string]interface{} { "datasets": []string{"package"}, } } + +func TestPackageGobEncodeDecode(t *testing.T) { + pkg := Package{ + Name: "foo", + Version: "1.2.3", + Release: "1", + Arch: "amd64", + License: "bar", + InstallTime: time.Unix(1591021924, 0).UTC(), + Size: 1234, + Summary: "Foo stuff", + URL: "http://foo.example.com", + Type: "rpm", + } + + buf := new(bytes.Buffer) + if err := gob.NewEncoder(buf).Encode(pkg); err != nil { + t.Fatal(err) + } + + const gobTestFile = "testdata/package.v1.gob" + if *flagUpdateGob { + // NOTE: If you are updating this file then you may have introduced a + // a breaking change. + if err := ioutil.WriteFile(gobTestFile, buf.Bytes(), 0644); err != nil { + t.Fatal(err) + } + } + + t.Run("decode", func(t *testing.T) { + var pkgDecoded Package + if err := gob.NewDecoder(buf).Decode(&pkgDecoded); err != nil { + t.Fatal(err) + } + assert.Equal(t, pkg, pkgDecoded) + }) + + // Validate that we get the same result when decoding an earlier saved version. + // This detects breakages to the struct or to the encoding/decoding pkgs. + t.Run("decode_from_file", func(t *testing.T) { + contents, err := ioutil.ReadFile(gobTestFile) + if err != nil { + t.Fatal(err) + } + + var pkgDecoded Package + if err := gob.NewDecoder(bytes.NewReader(contents)).Decode(&pkgDecoded); err != nil { + t.Fatal(err) + } + assert.Equal(t, pkg, pkgDecoded) + }) +} + +// Regression test for https://github.com/elastic/beats/issues/18536 to verify +// that error isn't made public. +func TestPackageWithErrorGobEncode(t *testing.T) { + pkg := Package{ + error: errors.New("test"), + } + + buf := new(bytes.Buffer) + if err := gob.NewEncoder(buf).Encode(pkg); err != nil { + t.Fatal(err) + } +} diff --git a/x-pack/auditbeat/module/system/package/rpm_linux.go b/x-pack/auditbeat/module/system/package/rpm_linux.go index 76e10fd8164..e3a1e6f78c5 100644 --- a/x-pack/auditbeat/module/system/package/rpm_linux.go +++ b/x-pack/auditbeat/module/system/package/rpm_linux.go @@ -372,7 +372,7 @@ func packageFromHeader(header C.Header, openedLibrpm *librpm) (*Package, error) if version != nil { pkg.Version = C.GoString(version) } else { - pkg.Error = errors.New("Failed to get package version") + pkg.error = errors.New("failed to get package version") } pkg.Release = C.GoString(C.my_headerGetString(openedLibrpm.headerGetString, header, RPMTAG_RELEASE)) diff --git a/x-pack/auditbeat/module/system/package/testdata/package.v1.gob b/x-pack/auditbeat/module/system/package/testdata/package.v1.gob new file mode 100644 index 0000000000000000000000000000000000000000..f7c6a42f48c7f798c45971873033d723f12073ee GIT binary patch literal 241 zcmXYpyH3ME5Jl%|*CadyRMhzbM*&1ZM}veEA(U5>YH-Ghh1YAX9f{Ck_(F<&4x6>1 zx-&;Q$2a~{0lgPa-gqtW4dDd(nT_C#&|8SBGFx=_L~`M(- zY*u%-P_OK8fgqjl*Zoyk@*cnX2gi@r4yt;arnuVFb$LG=hC(dwY|32X#2P498ae