From 4d20bd367852045f531332e414d2782a66a6589c Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:32:32 -0400 Subject: [PATCH 01/24] gosec: allow loading user defined file --- agent/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/config.go b/agent/config.go index e5d8587bf..38eb69f6d 100644 --- a/agent/config.go +++ b/agent/config.go @@ -16,7 +16,7 @@ type Config struct { // NewConfigFromFile will create a new agent config from a YAML file. func NewConfigFromFile(file string) (*Config, error) { - contents, err := ioutil.ReadFile(file) + contents, err := ioutil.ReadFile(file) // #nosec - configs load based on user specified directory if err != nil { return nil, fmt.Errorf("could not find config file: %s", err) } From 0e598360e20fe4e3a95294e3e4559c0d05cabdb5 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:32:40 -0400 Subject: [PATCH 02/24] gosec: limit stanza log to the user that is running stanza. stanza error log can expose sensative information --- cmd/stanza/logging.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/stanza/logging.go b/cmd/stanza/logging.go index f4d3eb7de..a16abfe19 100644 --- a/cmd/stanza/logging.go +++ b/cmd/stanza/logging.go @@ -59,5 +59,6 @@ func registerWindowsSink() { } func newWinFileSink(u *url.URL) (zap.Sink, error) { - return os.OpenFile(u.Path[1:], os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644) + // Ensure permissions restrict access to the running user only + return os.OpenFile(u.Path[1:], os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0600) } From b55c96cf61f56377143fb6d511b1c78d11b929d0 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:33:11 -0400 Subject: [PATCH 03/24] handle errors returned by stderr / stdout --- cmd/stanza/offsets.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/cmd/stanza/offsets.go b/cmd/stanza/offsets.go index 31c2b2e53..b60cdeb6b 100644 --- a/cmd/stanza/offsets.go +++ b/cmd/stanza/offsets.go @@ -9,6 +9,7 @@ import ( "github.com/observiq/stanza/operator/helper" "github.com/spf13/cobra" "go.etcd.io/bbolt" + "go.uber.org/zap" ) var stdout io.Writer = os.Stdout @@ -19,9 +20,6 @@ func NewOffsetsCmd(rootFlags *RootFlags) *cobra.Command { Use: "offsets", Short: "Manage input operator offsets", Args: cobra.NoArgs, - Run: func(command *cobra.Command, args []string) { - stdout.Write([]byte("No offsets subcommand specified. See `stanza offsets help` for details\n")) - }, } offsets.AddCommand(NewOffsetsClearCmd(rootFlags)) @@ -46,7 +44,10 @@ func NewOffsetsClearCmd(rootFlags *RootFlags) *cobra.Command { if all { if len(args) != 0 { - stdout.Write([]byte("Providing a list of operator IDs does nothing with the --all flag\n")) + _, err := stdout.Write([]byte("Providing a list of operator IDs does nothing with the --all flag\n")) + if err != nil { + exitOnErr("", err) + } } err := db.Update(func(tx *bbolt.Tx) error { @@ -59,7 +60,10 @@ func NewOffsetsClearCmd(rootFlags *RootFlags) *cobra.Command { exitOnErr("Failed to delete offsets", err) } else { if len(args) == 0 { - stdout.Write([]byte("Must either specify a list of operators or the --all flag\n")) + _, err := stdout.Write([]byte("Must either specify a list of operators or the --all flag\n")) + if err != nil { + exitOnErr("", err) + } os.Exit(1) } @@ -101,8 +105,8 @@ func NewOffsetsListCmd(rootFlags *RootFlags) *cobra.Command { } return offsetBucket.ForEach(func(key, value []byte) error { - stdout.Write(append(key, '\n')) - return nil + _, err := stdout.Write(append(key, '\n')) + return err }) }) if err != nil { @@ -115,8 +119,12 @@ func NewOffsetsListCmd(rootFlags *RootFlags) *cobra.Command { } func exitOnErr(msg string, err error) { + var sugaredLogger *zap.SugaredLogger if err != nil { - os.Stderr.WriteString(fmt.Sprintf("%s: %s\n", msg, err)) + _, err := os.Stderr.WriteString(fmt.Sprintf("%s: %s\n", msg, err)) + if err != nil { + sugaredLogger.Errorw("Failed to write to stdout", zap.Any("error", err)) + } os.Exit(1) } } From a9578515b0dbda1a886d96ac44b28afa16777845 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:33:46 -0400 Subject: [PATCH 04/24] gosec: allow net/http/pprof's http webhook. handle errors from closing files --- cmd/stanza/root.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/cmd/stanza/root.go b/cmd/stanza/root.go index 708276656..ca6bec2dd 100644 --- a/cmd/stanza/root.go +++ b/cmd/stanza/root.go @@ -7,7 +7,7 @@ import ( "net/http" // This package registers its HTTP endpoints for profiling using an init hook - _ "net/http/pprof" + _ "net/http/pprof" // #nosec "os" "runtime" "runtime/pprof" @@ -156,7 +156,11 @@ func startProfiling(ctx context.Context, flags *RootFlags, logger *zap.SugaredLo if err != nil { logger.Errorw("Failed to create CPU profile", zap.Error(err)) } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + logger.Errorf(err.Error()) + } + }() if err := pprof.StartCPUProfile(f); err != nil { log.Fatal("could not start CPU profile: ", err) @@ -185,7 +189,11 @@ func startProfiling(ctx context.Context, flags *RootFlags, logger *zap.SugaredLo if err != nil { logger.Errorw("Failed to create memory profile", zap.Error(err)) } - defer f.Close() // error handling omitted for example + defer func() { + if err := f.Close(); err != nil { + logger.Errorw("Failed to close file", zap.Error(err)) + } + }() runtime.GC() // get up-to-date statistics if err := pprof.WriteHeapProfile(f); err != nil { From c9e3b01988937d6052a3d58d0225a9f2f2cf8130 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:34:16 -0400 Subject: [PATCH 05/24] gosec: limit stanza database to user that is running stanza. stanza database can contain sensative information --- database/database.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/database/database.go b/database/database.go index 53304523a..9419f7982 100644 --- a/database/database.go +++ b/database/database.go @@ -49,7 +49,8 @@ func OpenDatabase(file string) (Database, error) { if _, err := os.Stat(filepath.Dir(file)); err != nil { if os.IsNotExist(err) { - err := os.MkdirAll(filepath.Dir(file), 0755) + // Ensure permissions restrict access to the running user only + err := os.MkdirAll(filepath.Dir(file), 0600) if err != nil { return nil, fmt.Errorf("creating database directory: %s", err) } From 71a511aa0cdb502cad00929ef3fd388a620d47ee Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:35:25 -0400 Subject: [PATCH 06/24] gosec: allow user to pass in files. restrict buffer file to user running stanza. handle errors from closing files --- operator/buffer/disk.go | 3 ++- operator/buffer/disk_metadata.go | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/operator/buffer/disk.go b/operator/buffer/disk.go index 94ea1e9d1..cc6ee5125 100644 --- a/operator/buffer/disk.go +++ b/operator/buffer/disk.go @@ -125,7 +125,8 @@ func (d *DiskBuffer) Open(path string, sync bool) error { if sync { flags |= os.O_SYNC } - if d.data, err = os.OpenFile(dataPath, flags, 0755); err != nil { + // #nosec - configs load based on user specified directory + if d.data, err = os.OpenFile(dataPath, flags, 0600); err != nil { return err } diff --git a/operator/buffer/disk_metadata.go b/operator/buffer/disk_metadata.go index fd9328791..856811b35 100644 --- a/operator/buffer/disk_metadata.go +++ b/operator/buffer/disk_metadata.go @@ -56,7 +56,8 @@ func OpenMetadata(path string, sync bool) (*Metadata, error) { if sync { flags |= os.O_SYNC } - if m.file, err = os.OpenFile(path, flags, 0755); err != nil { + // #nosec - configs load based on user specified directory + if m.file, err = os.OpenFile(path, flags, 0600); err != nil { return &Metadata{}, err } @@ -108,8 +109,7 @@ func (m *Metadata) Close() error { if err != nil { return err } - m.file.Close() - return nil + return m.file.Close() } // setDeadRange sets the dead range start and length, then persists it to disk From 316448c28be2bd12074f46afab4ab61ab9543f18 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:35:51 -0400 Subject: [PATCH 07/24] handle error from closing connection --- operator/builtin/input/udp/udp.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/operator/builtin/input/udp/udp.go b/operator/builtin/input/udp/udp.go index 68d050514..00cac513e 100644 --- a/operator/builtin/input/udp/udp.go +++ b/operator/builtin/input/udp/udp.go @@ -143,7 +143,9 @@ func (u *UDPInput) readMessage() (string, net.Addr, error) { // Stop will stop listening for udp messages. func (u *UDPInput) Stop() error { u.cancel() - u.connection.Close() + if err := u.connection.Close(); err != nil { + u.Errorf("failed to close connection, got error: %s", err) + } u.wg.Wait() return nil } From 12f3f5180345ef828ad2cfa71f6eba1bb01ad9b0 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:36:24 -0400 Subject: [PATCH 08/24] gosec: limit permissions for file output file to the user running stanza --- operator/builtin/output/file/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/builtin/output/file/file.go b/operator/builtin/output/file/file.go index d74df7c68..4cfdf202a 100644 --- a/operator/builtin/output/file/file.go +++ b/operator/builtin/output/file/file.go @@ -74,7 +74,7 @@ type FileOutput struct { // Start will open the output file. func (fo *FileOutput) Start() error { var err error - fo.file, err = os.OpenFile(fo.path, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0660) + fo.file, err = os.OpenFile(fo.path, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0600) if err != nil { return err } From 3072f523ee50886208d6c6cfe4663bc79abcb8fe Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:36:48 -0400 Subject: [PATCH 09/24] handle errors from closing body --- operator/builtin/output/forward/forward.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/operator/builtin/output/forward/forward.go b/operator/builtin/output/forward/forward.go index b89af6d3c..85022eac3 100644 --- a/operator/builtin/output/forward/forward.go +++ b/operator/builtin/output/forward/forward.go @@ -165,10 +165,11 @@ func (f *ForwardOutput) handleResponse(res *http.Response) error { if err != nil { return errors.NewError("unexpected status code", "", "status", res.Status) } else { - res.Body.Close() + if err := res.Body.Close(); err != nil { + f.Errorf(err.Error()) + } return errors.NewError("unexpected status code", "", "status", res.Status, "body", string(body)) } } - res.Body.Close() - return nil + return res.Body.Close() } From c0ee888830163c6b4bddca8d6133d4ec125bc1e7 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:37:14 -0400 Subject: [PATCH 10/24] handle errors from closing body --- operator/builtin/output/newrelic/newrelic.go | 7 ++++--- operator/builtin/output/otlp/otlp.go | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/operator/builtin/output/newrelic/newrelic.go b/operator/builtin/output/newrelic/newrelic.go index 0dbc8ca64..4c5fa7345 100644 --- a/operator/builtin/output/newrelic/newrelic.go +++ b/operator/builtin/output/newrelic/newrelic.go @@ -237,10 +237,11 @@ func (nro *NewRelicOutput) handleResponse(res *http.Response) error { if err != nil { return errors.NewError("unexpected status code", "", "status", res.Status) } else { - res.Body.Close() + if err := res.Body.Close(); err != nil { + nro.Errorf(err.Error()) + } return errors.NewError("unexpected status code", "", "status", res.Status, "body", string(body)) } } - res.Body.Close() - return nil + return res.Body.Close() } diff --git a/operator/builtin/output/otlp/otlp.go b/operator/builtin/output/otlp/otlp.go index 4ea748db9..f36086fdb 100644 --- a/operator/builtin/output/otlp/otlp.go +++ b/operator/builtin/output/otlp/otlp.go @@ -181,10 +181,11 @@ func (o *OTLPOutput) handleResponse(res *http.Response) error { if err != nil { return errors.NewError("non-success status code", "", "status", fmt.Sprint(res.StatusCode)) } else { - res.Body.Close() + if err := res.Body.Close(); err != nil { + o.Errorf(err.Error()) + } return errors.NewError("non-success status code", "", "status", fmt.Sprint(res.StatusCode), "body", string(body)) } } - res.Body.Close() - return nil + return res.Body.Close() } From 1e53f3508bc834e4c27d3080ac9211333ccaa064 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:37:41 -0400 Subject: [PATCH 11/24] return the error from t.ProcessWith --- operator/builtin/parser/time/time.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/operator/builtin/parser/time/time.go b/operator/builtin/parser/time/time.go index a424018f6..b1ac6f33f 100644 --- a/operator/builtin/parser/time/time.go +++ b/operator/builtin/parser/time/time.go @@ -58,6 +58,5 @@ func (t *TimeParserOperator) CanOutput() bool { // Process will parse time from an entry. func (t *TimeParserOperator) Process(ctx context.Context, entry *entry.Entry) error { - t.ProcessWith(ctx, entry, t.TimeParser.Parse) - return nil + return t.ProcessWith(ctx, entry, t.TimeParser.Parse) } From d25c7885b3e395ace8b7d27e9a630deae0e9509b Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:38:16 -0400 Subject: [PATCH 12/24] handle error from base.Set --- operator/builtin/transformer/recombine/recombine.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/operator/builtin/transformer/recombine/recombine.go b/operator/builtin/transformer/recombine/recombine.go index 2fed40a13..c3b4b298b 100644 --- a/operator/builtin/transformer/recombine/recombine.go +++ b/operator/builtin/transformer/recombine/recombine.go @@ -3,14 +3,15 @@ package recombine import ( "context" "fmt" + "strings" + "sync" + "time" + "github.com/antonmedv/expr" "github.com/antonmedv/expr/vm" "github.com/observiq/stanza/entry" "github.com/observiq/stanza/operator" "github.com/observiq/stanza/operator/helper" - "strings" - "sync" - "time" ) func init() { @@ -233,7 +234,10 @@ func (r *RecombineOperator) flushCombined() error { } // Set the recombined field on the entry - base.Set(r.combineField, recombined.String()) + err := base.Set(r.combineField, recombined.String()) + if err != nil { + return err + } r.Write(context.Background(), base) r.batch = r.batch[:0] From 018a561d966d426e0e4d2f8a5895840313e59990 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:38:35 -0400 Subject: [PATCH 13/24] gosec: allow loading user defined file --- operator/helper/operatortest/operatortest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/helper/operatortest/operatortest.go b/operator/helper/operatortest/operatortest.go index c48ef6939..fb7189a64 100644 --- a/operator/helper/operatortest/operatortest.go +++ b/operator/helper/operatortest/operatortest.go @@ -32,7 +32,7 @@ type ConfigUnmarshalTest struct { } func configFromFileViaYaml(file string, config interface{}) error { - bytes, err := ioutil.ReadFile(file) + bytes, err := ioutil.ReadFile(file) // #nosec - configs load based on user specified directory if err != nil { return fmt.Errorf("could not find config file: %s", err) } From 559bd13cfa315f30dfa022f68f21b404748b17bb Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:38:46 -0400 Subject: [PATCH 14/24] gosec: allow loading user defined file --- plugin/plugin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/plugin.go b/plugin/plugin.go index 94f81feb5..5d62f8576 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -177,7 +177,7 @@ func splitPluginFile(text []byte) (metadata, template []byte, err error) { // NewPluginFromFile builds a new plugin from a file func NewPluginFromFile(path string) (*Plugin, error) { - contents, err := ioutil.ReadFile(path) + contents, err := ioutil.ReadFile(path) // #nosec - configs load based on user specified directory if err != nil { return nil, fmt.Errorf("could not read plugin file: %s", err) } From 39b0e1eedba61a3058589381e6358eb89341067e Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:39:03 -0400 Subject: [PATCH 15/24] gosec: handle errors --- testutil/util.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/testutil/util.go b/testutil/util.go index 078dd5b8b..c4c6f31d9 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -22,7 +22,10 @@ func NewTempDir(t testing.TB) string { } t.Cleanup(func() { - os.RemoveAll(tempDir) + if err := os.RemoveAll(tempDir); err != nil { + t.Errorf(err.Error()) + t.FailNow() + } }) return tempDir @@ -37,7 +40,10 @@ func NewTestDatabase(t testing.TB) *bbolt.DB { } t.Cleanup(func() { - os.RemoveAll(tempDir) + if err := os.RemoveAll(tempDir); err != nil { + t.Errorf(err.Error()) + t.FailNow() + } }) db, err := bbolt.Open(filepath.Join(tempDir, "test.db"), 0666, nil) @@ -47,7 +53,10 @@ func NewTestDatabase(t testing.TB) *bbolt.DB { } t.Cleanup(func() { - db.Close() + if err := db.Close(); err != nil { + t.Errorf(err.Error()) + t.FailNow() + } }) return db From df00c720feee658fdbbb5442434c0b55ca972b92 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 23 Jun 2021 19:52:11 -0400 Subject: [PATCH 16/24] database directory is fine with 0755 permissions but the database file should be 0600 --- database/database.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/database/database.go b/database/database.go index 9419f7982..24445860f 100644 --- a/database/database.go +++ b/database/database.go @@ -49,8 +49,7 @@ func OpenDatabase(file string) (Database, error) { if _, err := os.Stat(filepath.Dir(file)); err != nil { if os.IsNotExist(err) { - // Ensure permissions restrict access to the running user only - err := os.MkdirAll(filepath.Dir(file), 0600) + err := os.MkdirAll(filepath.Dir(file), 0755) // #nosec - 0755 directory permissions are okay if err != nil { return nil, fmt.Errorf("creating database directory: %s", err) } @@ -60,5 +59,5 @@ func OpenDatabase(file string) (Database, error) { } options := &bbolt.Options{Timeout: 1 * time.Second} - return bbolt.Open(file, 0666, options) + return bbolt.Open(file, 0600, options) } From 623a6e30321d8591e137393c3d83dcea1a103c52 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 30 Jun 2021 18:53:39 -0400 Subject: [PATCH 17/24] remove return type error from handleEvent and handleEvents as the error is never checked. Instead, log the error because we want to continue --- operator/builtin/input/aws/cloudwatch/cloudwatch.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/operator/builtin/input/aws/cloudwatch/cloudwatch.go b/operator/builtin/input/aws/cloudwatch/cloudwatch.go index 921c88575..54430d390 100644 --- a/operator/builtin/input/aws/cloudwatch/cloudwatch.go +++ b/operator/builtin/input/aws/cloudwatch/cloudwatch.go @@ -10,7 +10,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/cloudwatchlogs" - "github.com/observiq/stanza/errors" "github.com/observiq/stanza/operator" "github.com/observiq/stanza/operator/helper" ) @@ -302,14 +301,14 @@ func (c *CloudwatchInput) filterLogEventsInputBuilder(nextToken string) cloudwat } // handleEvent is the handler for a AWS Cloudwatch Logs Filtered Event. -func (c *CloudwatchInput) handleEvent(ctx context.Context, event *cloudwatchlogs.FilteredLogEvent) error { +func (c *CloudwatchInput) handleEvent(ctx context.Context, event *cloudwatchlogs.FilteredLogEvent) { e := map[string]interface{}{ "message": event.Message, "ingestion_time": event.IngestionTime, } entry, err := c.NewEntry(e) if err != nil { - return errors.Wrap(err, "Failed to create new entry from record") + c.Errorf("Failed to create new entry from record: %s", err) } entry.AddResourceKey("log_group", c.logGroupName) @@ -327,15 +326,15 @@ func (c *CloudwatchInput) handleEvent(ctx context.Context, event *cloudwatchlogs c.Debugf("Writing start time %d to database", *event.IngestionTime) c.persist.Write(c.logGroupName, c.startTime) } - return nil } -func (c *CloudwatchInput) handleEvents(ctx context.Context, events []*cloudwatchlogs.FilteredLogEvent) error { +func (c *CloudwatchInput) handleEvents(ctx context.Context, events []*cloudwatchlogs.FilteredLogEvent) { for _, event := range events { c.handleEvent(ctx, event) } - c.persist.DB.Sync() - return nil + if err := c.persist.DB.Sync(); err != nil { + c.Errorf("Failed to sync offset database: %s", err) + } } // Returns time.Now() as Unix Time in Milliseconds From 7e0396b60c1869f0694d99f387af53902fed0eca Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 30 Jun 2021 18:57:51 -0400 Subject: [PATCH 18/24] return file.Close() error --- operator/builtin/output/file/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/builtin/output/file/file.go b/operator/builtin/output/file/file.go index 4cfdf202a..729223d20 100644 --- a/operator/builtin/output/file/file.go +++ b/operator/builtin/output/file/file.go @@ -88,7 +88,7 @@ func (fo *FileOutput) Start() error { // Stop will close the output file. func (fo *FileOutput) Stop() error { if fo.file != nil { - fo.file.Close() + return fo.file.Close() } return nil } From db20e4acb89a6fd53076d9c233968af849f1e24f Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 30 Jun 2021 18:58:04 -0400 Subject: [PATCH 19/24] log errors returned by file.Close() --- operator/builtin/input/file/file.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/operator/builtin/input/file/file.go b/operator/builtin/input/file/file.go index a7a4734bd..025f44de4 100644 --- a/operator/builtin/input/file/file.go +++ b/operator/builtin/input/file/file.go @@ -203,7 +203,9 @@ OUTER: for i := 0; i < len(fps); { fp := fps[i] if len(fp.FirstBytes) == 0 { - files[i].Close() + if err := files[i].Close(); err != nil { + f.Errorf("problem closing file", "file", files[i].Name()) + } // Empty file, don't read it until we can compare its fingerprint fps = append(fps[:i], fps[i+1:]...) files = append(files[:i], files[i+1:]...) From 955dab0d16dd3bd28b16baf8fe7a9111acd491e5 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 30 Jun 2021 19:02:08 -0400 Subject: [PATCH 20/24] nosec, user defines the input file --- operator/builtin/input/file/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator/builtin/input/file/file.go b/operator/builtin/input/file/file.go index 025f44de4..11e8e7874 100644 --- a/operator/builtin/input/file/file.go +++ b/operator/builtin/input/file/file.go @@ -179,7 +179,7 @@ func (f *InputOperator) makeReaders(filePaths []string) []*Reader { } f.SeenPaths[path] = struct{}{} } - file, err := os.Open(path) + file, err := os.Open(path) // #nosec - operator must read in files defined by user if err != nil { f.Errorw("Failed to open file", zap.Error(err)) continue From 67c0ce17fae9beba9ad9ad1699e5af35ec3d885b Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 30 Jun 2021 19:03:07 -0400 Subject: [PATCH 21/24] journalctl is an executable that is required for this operator to function --- operator/builtin/input/journald/journald.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operator/builtin/input/journald/journald.go b/operator/builtin/input/journald/journald.go index cf808da97..eee33a4af 100644 --- a/operator/builtin/input/journald/journald.go +++ b/operator/builtin/input/journald/journald.go @@ -82,7 +82,8 @@ func (c JournaldInputConfig) Build(buildContext operator.BuildContext) ([]operat if cursor != nil { args = append(args, "--after-cursor", string(cursor)) } - return exec.CommandContext(ctx, "journalctl", args...) + return exec.CommandContext(ctx, "journalctl", args...) // #nosec - ... + // journalctl is an executable that is required for this operator to function }, json: jsoniter.ConfigFastest, } From 79df13f0cdb1f334d28967b68e87886ffdcd432e Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 30 Jun 2021 19:14:03 -0400 Subject: [PATCH 22/24] gosec wants TLS 1.2/1.3 but Go defaults to 1.0. Some users may require old versions of TLS, so we will support 1.0 and newer. An issue has been filed to make the min version a configuratin option for environments that require a higher min version --- operator/builtin/input/tcp/tcp.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/operator/builtin/input/tcp/tcp.go b/operator/builtin/input/tcp/tcp.go index 3a3a1d934..6125ee27e 100644 --- a/operator/builtin/input/tcp/tcp.go +++ b/operator/builtin/input/tcp/tcp.go @@ -158,7 +158,17 @@ func (t *TCPInput) configureListener() error { return nil } - config := tls.Config{Certificates: []tls.Certificate{t.tlsKeyPair}} + // TLS 1.0 is the package default since Go 1.2 + // https://golang.org/pkg/crypto/tls/ + // An issue has been filed to support modifyingn the minimum version + // https://github.com/observIQ/stanza/issues/349 + var tlsVersion uint16 = tls.VersionTLS10 + + // #nosec - Go defaults to TLS 1.0, and some users may require it + config := tls.Config{ + Certificates: []tls.Certificate{t.tlsKeyPair}, + MinVersion: tlsVersion, + } config.Time = func() time.Time { return time.Now() } config.Rand = rand.Reader From 53954d1485e153d4082f8dd43f82819eb5d602af Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 7 Jul 2021 22:07:48 -0400 Subject: [PATCH 23/24] remove inappropriate use of failnow --- testutil/util.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/testutil/util.go b/testutil/util.go index 5d8a57b49..ad3761c73 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -25,7 +25,6 @@ func NewTempDir(t testing.TB) string { t.Cleanup(func() { if err := os.RemoveAll(tempDir); err != nil { t.Errorf(err.Error()) - t.FailNow() } }) @@ -43,7 +42,6 @@ func NewTestDatabase(t testing.TB) *bbolt.DB { t.Cleanup(func() { if err := os.RemoveAll(tempDir); err != nil { t.Errorf(err.Error()) - t.FailNow() } }) @@ -56,7 +54,6 @@ func NewTestDatabase(t testing.TB) *bbolt.DB { t.Cleanup(func() { if err := db.Close(); err != nil { t.Errorf(err.Error()) - t.FailNow() } }) From e0ca03133145a1a7b84c36e4e27f098c5a0be8e1 Mon Sep 17 00:00:00 2001 From: jsirianni Date: Wed, 7 Jul 2021 22:54:25 -0400 Subject: [PATCH 24/24] close open files before test finishes --- operator/buffer/disk_test.go | 19 +++++++++++++++++++ .../aws/cloudwatch/cloudwatch_persist_test.go | 5 +++++ operator/helper/persister_test.go | 5 +++++ 3 files changed, 29 insertions(+) diff --git a/operator/buffer/disk_test.go b/operator/buffer/disk_test.go index ae7c8a53c..303739a5c 100644 --- a/operator/buffer/disk_test.go +++ b/operator/buffer/disk_test.go @@ -142,6 +142,8 @@ func TestDiskBuffer(t *testing.T) { err = b2.Open(dir, false) require.NoError(t, err) readN(t, b2, 20, 0) + err = b2.Close() + require.NoError(t, err) }) t.Run("Write20Flush10CloseRead20", func(t *testing.T) { @@ -160,6 +162,8 @@ func TestDiskBuffer(t *testing.T) { err = b2.Open(dir, false) require.NoError(t, err) readN(t, b2, 10, 10) + err = b2.Close() + require.NoError(t, err) }) t.Run("ReadWaitTimesOut", func(t *testing.T) { @@ -178,6 +182,11 @@ func TestDiskBuffer(t *testing.T) { b := NewDiskBuffer(100) // Enough space for 1, but not 2 entries dir := testutil.NewTempDir(t) err := b.Open(dir, false) + defer func() { + if err := b.Close(); err != nil { + t.Error(err.Error()) + } + }() require.NoError(t, err) // Add a first entry @@ -214,6 +223,11 @@ func TestDiskBuffer(t *testing.T) { b := NewDiskBuffer(1 << 30) dir := testutil.NewTempDir(t) err := b.Open(dir, false) + defer func() { + if err := b.Close(); err != nil { + t.Error(err.Error()) + } + }() require.NoError(t, err) writes := 0 @@ -247,6 +261,11 @@ func TestDiskBufferBuild(t *testing.T) { cfg.Path = testutil.NewTempDir(t) b, err := cfg.Build(testutil.NewBuildContext(t), "test") require.NoError(t, err) + defer func() { + if err := b.Close(); err != nil { + t.Error(err.Error()) + } + }() diskBuffer := b.(*DiskBuffer) require.Equal(t, diskBuffer.atEnd, false) require.Len(t, diskBuffer.entryAdded, 1) diff --git a/operator/builtin/input/aws/cloudwatch/cloudwatch_persist_test.go b/operator/builtin/input/aws/cloudwatch/cloudwatch_persist_test.go index 90a68b8cb..387921745 100644 --- a/operator/builtin/input/aws/cloudwatch/cloudwatch_persist_test.go +++ b/operator/builtin/input/aws/cloudwatch/cloudwatch_persist_test.go @@ -25,6 +25,11 @@ func TestPersisterLoad(t *testing.T) { tempDir := testutil.NewTempDir(t) db, openDbErr := database.OpenDatabase(filepath.Join(tempDir, "test.db")) require.NoError(t, openDbErr) + defer func() { + if err := db.Close(); err != nil { + t.Error(err.Error()) + } + }() persister := Persister{ DB: helper.NewScopedDBPersister(db, "test"), } diff --git a/operator/helper/persister_test.go b/operator/helper/persister_test.go index e93247a55..b73a3d939 100644 --- a/operator/helper/persister_test.go +++ b/operator/helper/persister_test.go @@ -20,6 +20,11 @@ func TestPersisterCache(t *testing.T) { func TestPersisterLoad(t *testing.T) { tempDir := testutil.NewTempDir(t) db, err := database.OpenDatabase(filepath.Join(tempDir, "test.db")) + defer func() { + if err := db.Close(); err != nil { + t.Error(err.Error()) + } + }() persister := NewScopedDBPersister(db, "test") persister.Set("key", []byte("value"))