From e7ecea764e99f873e1644b409d8eebec8e1c2630 Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Mon, 4 Mar 2024 14:22:53 +0100 Subject: [PATCH] pkg/csconfig: use yaml.v3; deprecate yaml.v2 for new code (#2867) * pkg/csconfig: use yaml.v3; deprecate yaml.v2 for new code * yaml.v3: handle empty files * Lint whitespace, errors --- .golangci.yml | 50 ++++++++++++++++++++++++++++++++ pkg/csconfig/api.go | 33 ++++++++++++++------- pkg/csconfig/api_test.go | 12 ++++++-- pkg/csconfig/config.go | 17 +++++++---- pkg/csconfig/config_test.go | 2 +- pkg/csconfig/console.go | 12 ++++++-- pkg/csconfig/crowdsec_service.go | 11 +++---- pkg/csconfig/database.go | 9 +++++- pkg/csconfig/profiles.go | 23 ++++++++++----- pkg/csconfig/simulation.go | 23 +++++++++++++-- pkg/csconfig/simulation_test.go | 4 +-- 11 files changed, 153 insertions(+), 43 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 29332447b..396da2141 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -72,6 +72,56 @@ linters-settings: deny: - pkg: "github.com/pkg/errors" desc: "errors.Wrap() is deprecated in favor of fmt.Errorf()" + yaml: + files: + - "!**/cmd/crowdsec-cli/alerts.go" + - "!**/cmd/crowdsec-cli/capi.go" + - "!**/cmd/crowdsec-cli/config_show.go" + - "!**/cmd/crowdsec-cli/hubtest.go" + - "!**/cmd/crowdsec-cli/lapi.go" + - "!**/cmd/crowdsec-cli/simulation.go" + - "!**/cmd/crowdsec/crowdsec.go" + - "!**/cmd/notification-dummy/main.go" + - "!**/cmd/notification-email/main.go" + - "!**/cmd/notification-http/main.go" + - "!**/cmd/notification-slack/main.go" + - "!**/cmd/notification-splunk/main.go" + - "!**/pkg/acquisition/acquisition.go" + - "!**/pkg/acquisition/acquisition_test.go" + - "!**/pkg/acquisition/modules/appsec/appsec.go" + - "!**/pkg/acquisition/modules/cloudwatch/cloudwatch.go" + - "!**/pkg/acquisition/modules/docker/docker.go" + - "!**/pkg/acquisition/modules/file/file.go" + - "!**/pkg/acquisition/modules/journalctl/journalctl.go" + - "!**/pkg/acquisition/modules/kafka/kafka.go" + - "!**/pkg/acquisition/modules/kinesis/kinesis.go" + - "!**/pkg/acquisition/modules/kubernetesaudit/k8s_audit.go" + - "!**/pkg/acquisition/modules/loki/loki.go" + - "!**/pkg/acquisition/modules/loki/timestamp_test.go" + - "!**/pkg/acquisition/modules/s3/s3.go" + - "!**/pkg/acquisition/modules/syslog/syslog.go" + - "!**/pkg/acquisition/modules/wineventlog/wineventlog_windows.go" + - "!**/pkg/appsec/appsec.go" + - "!**/pkg/appsec/loader.go" + - "!**/pkg/csplugin/broker.go" + - "!**/pkg/csplugin/broker_test.go" + - "!**/pkg/dumps/bucker_dump.go" + - "!**/pkg/dumps/bucket_dump.go" + - "!**/pkg/dumps/parser_dump.go" + - "!**/pkg/hubtest/coverage.go" + - "!**/pkg/hubtest/hubtest_item.go" + - "!**/pkg/hubtest/parser_assert.go" + - "!**/pkg/hubtest/scenario_assert.go" + - "!**/pkg/leakybucket/buckets_test.go" + - "!**/pkg/leakybucket/manager_load.go" + - "!**/pkg/metabase/metabase.go" + - "!**/pkg/parser/node.go" + - "!**/pkg/parser/node_test.go" + - "!**/pkg/parser/parsing_test.go" + - "!**/pkg/parser/stage.go" + deny: + - pkg: "gopkg.in/yaml.v2" + desc: "yaml.v2 is deprecated for new code in favor of yaml.v3" wsl: # Allow blocks to end with comments diff --git a/pkg/csconfig/api.go b/pkg/csconfig/api.go index de8ee4934..7fd1f5888 100644 --- a/pkg/csconfig/api.go +++ b/pkg/csconfig/api.go @@ -1,6 +1,7 @@ package csconfig import ( + "bytes" "crypto/tls" "crypto/x509" "errors" @@ -12,7 +13,7 @@ import ( "time" log "github.com/sirupsen/logrus" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/ptr" "github.com/crowdsecurity/go-cs-lib/yamlpatch" @@ -63,7 +64,7 @@ func (a *CTICfg) Load() error { } if a.Key != nil && *a.Key == "" { - return fmt.Errorf("empty cti key") + return errors.New("empty cti key") } if a.Enabled == nil { @@ -92,9 +93,14 @@ func (o *OnlineApiClientCfg) Load() error { return err } - err = yaml.UnmarshalStrict(fcontent, o.Credentials) + dec := yaml.NewDecoder(bytes.NewReader(fcontent)) + dec.KnownFields(true) + + err = dec.Decode(o.Credentials) if err != nil { - return fmt.Errorf("failed unmarshaling api server credentials configuration file '%s': %w", o.CredentialsFilePath, err) + if !errors.Is(err, io.EOF) { + return fmt.Errorf("failed unmarshaling api server credentials configuration file '%s': %w", o.CredentialsFilePath, err) + } } switch { @@ -120,9 +126,14 @@ func (l *LocalApiClientCfg) Load() error { return err } - err = yaml.UnmarshalStrict(fcontent, &l.Credentials) + dec := yaml.NewDecoder(bytes.NewReader(fcontent)) + dec.KnownFields(true) + + err = dec.Decode(&l.Credentials) if err != nil { - return fmt.Errorf("failed unmarshaling api client credential configuration file '%s': %w", l.CredentialsFilePath, err) + if !errors.Is(err, io.EOF) { + return fmt.Errorf("failed unmarshaling api client credential configuration file '%s': %w", l.CredentialsFilePath, err) + } } if l.Credentials == nil || l.Credentials.URL == "" { @@ -136,7 +147,7 @@ func (l *LocalApiClientCfg) Load() error { } if l.Credentials.Login != "" && (l.Credentials.CertPath != "" || l.Credentials.KeyPath != "") { - return fmt.Errorf("user/password authentication and TLS authentication are mutually exclusive") + return errors.New("user/password authentication and TLS authentication are mutually exclusive") } if l.InsecureSkipVerify == nil { @@ -263,7 +274,7 @@ func (c *Config) LoadAPIServer(inCli bool) error { } if c.API.Server.ListenURI == "" { - return fmt.Errorf("no listen_uri specified") + return errors.New("no listen_uri specified") } // inherit log level from common, then api->server @@ -350,7 +361,7 @@ func parseCapiWhitelists(fd io.Reader) (*CapiWhitelist, error) { decoder := yaml.NewDecoder(fd) if err := decoder.Decode(&fromCfg); err != nil { if errors.Is(err, io.EOF) { - return nil, fmt.Errorf("empty file") + return nil, errors.New("empty file") } return nil, err @@ -389,7 +400,7 @@ func (s *LocalApiServerCfg) LoadCapiWhitelists() error { fd, err := os.Open(s.CapiWhitelistsPath) if err != nil { - return fmt.Errorf("while opening capi whitelist file: %s", err) + return fmt.Errorf("while opening capi whitelist file: %w", err) } defer fd.Close() @@ -404,7 +415,7 @@ func (s *LocalApiServerCfg) LoadCapiWhitelists() error { func (c *Config) LoadAPIClient() error { if c.API == nil || c.API.Client == nil || c.API.Client.CredentialsFilePath == "" || c.DisableAgent { - return fmt.Errorf("no API client section in configuration") + return errors.New("no API client section in configuration") } if err := c.API.Client.Load(); err != nil { diff --git a/pkg/csconfig/api_test.go b/pkg/csconfig/api_test.go index e22c78204..b6febd4d4 100644 --- a/pkg/csconfig/api_test.go +++ b/pkg/csconfig/api_test.go @@ -9,7 +9,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/cstest" "github.com/crowdsecurity/go-cs-lib/ptr" @@ -68,6 +68,7 @@ func TestLoadLocalApiClientCfg(t *testing.T) { t.Run(tc.name, func(t *testing.T) { err := tc.input.Load() cstest.RequireErrorContains(t, err, tc.expectedErr) + if tc.expectedErr != "" { return } @@ -125,6 +126,7 @@ func TestLoadOnlineApiClientCfg(t *testing.T) { t.Run(tc.name, func(t *testing.T) { err := tc.input.Load() cstest.RequireErrorContains(t, err, tc.expectedErr) + if tc.expectedErr != "" { return } @@ -147,7 +149,11 @@ func TestLoadAPIServer(t *testing.T) { require.NoError(t, err) configData := os.ExpandEnv(string(fcontent)) - err = yaml.UnmarshalStrict([]byte(configData), &config) + + dec := yaml.NewDecoder(strings.NewReader(configData)) + dec.KnownFields(true) + + err = dec.Decode(&config) require.NoError(t, err) tests := []struct { @@ -242,6 +248,7 @@ func TestLoadAPIServer(t *testing.T) { t.Run(tc.name, func(t *testing.T) { err := tc.input.LoadAPIServer(false) cstest.RequireErrorContains(t, err, tc.expectedErr) + if tc.expectedErr != "" { return } @@ -305,6 +312,7 @@ func TestParseCapiWhitelists(t *testing.T) { t.Run(tc.name, func(t *testing.T) { wl, err := parseCapiWhitelists(strings.NewReader(tc.input)) cstest.RequireErrorContains(t, err, tc.expectedErr) + if tc.expectedErr != "" { return } diff --git a/pkg/csconfig/config.go b/pkg/csconfig/config.go index 2dc7ecc7d..0c960803e 100644 --- a/pkg/csconfig/config.go +++ b/pkg/csconfig/config.go @@ -1,14 +1,16 @@ // Package csconfig contains the configuration structures for crowdsec and cscli. - package csconfig import ( + "errors" "fmt" + "io" "os" "path/filepath" + "strings" log "github.com/sirupsen/logrus" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/csstring" "github.com/crowdsecurity/go-cs-lib/ptr" @@ -57,10 +59,15 @@ func NewConfig(configFile string, disableAgent bool, disableAPI bool, inCli bool DisableAPI: disableAPI, } - err = yaml.UnmarshalStrict([]byte(configData), &cfg) + dec := yaml.NewDecoder(strings.NewReader(configData)) + dec.KnownFields(true) + + err = dec.Decode(&cfg) if err != nil { - // this is actually the "merged" yaml - return nil, "", fmt.Errorf("%s: %w", configFile, err) + if !errors.Is(err, io.EOF) { + // this is actually the "merged" yaml + return nil, "", fmt.Errorf("%s: %w", configFile, err) + } } if cfg.Prometheus == nil { diff --git a/pkg/csconfig/config_test.go b/pkg/csconfig/config_test.go index 4843c2f70..56ecc2023 100644 --- a/pkg/csconfig/config_test.go +++ b/pkg/csconfig/config_test.go @@ -5,7 +5,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/cstest" ) diff --git a/pkg/csconfig/console.go b/pkg/csconfig/console.go index 1e8974154..01e74a94d 100644 --- a/pkg/csconfig/console.go +++ b/pkg/csconfig/console.go @@ -5,7 +5,7 @@ import ( "os" log "github.com/sirupsen/logrus" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/ptr" ) @@ -41,6 +41,7 @@ func (c *ConsoleConfig) IsPAPIEnabled() bool { if c == nil || c.ConsoleManagement == nil { return false } + return *c.ConsoleManagement } @@ -48,31 +49,36 @@ func (c *LocalApiServerCfg) LoadConsoleConfig() error { c.ConsoleConfig = &ConsoleConfig{} if _, err := os.Stat(c.ConsoleConfigPath); err != nil && os.IsNotExist(err) { log.Debugf("no console configuration to load") + c.ConsoleConfig.ShareCustomScenarios = ptr.Of(true) c.ConsoleConfig.ShareTaintedScenarios = ptr.Of(true) c.ConsoleConfig.ShareManualDecisions = ptr.Of(false) c.ConsoleConfig.ConsoleManagement = ptr.Of(false) c.ConsoleConfig.ShareContext = ptr.Of(false) + return nil } yamlFile, err := os.ReadFile(c.ConsoleConfigPath) if err != nil { - return fmt.Errorf("reading console config file '%s': %s", c.ConsoleConfigPath, err) + return fmt.Errorf("reading console config file '%s': %w", c.ConsoleConfigPath, err) } + err = yaml.Unmarshal(yamlFile, c.ConsoleConfig) if err != nil { - return fmt.Errorf("unmarshaling console config file '%s': %s", c.ConsoleConfigPath, err) + return fmt.Errorf("unmarshaling console config file '%s': %w", c.ConsoleConfigPath, err) } if c.ConsoleConfig.ShareCustomScenarios == nil { log.Debugf("no share_custom scenarios found, setting to true") c.ConsoleConfig.ShareCustomScenarios = ptr.Of(true) } + if c.ConsoleConfig.ShareTaintedScenarios == nil { log.Debugf("no share_tainted scenarios found, setting to true") c.ConsoleConfig.ShareTaintedScenarios = ptr.Of(true) } + if c.ConsoleConfig.ShareManualDecisions == nil { log.Debugf("no share_manual scenarios found, setting to false") c.ConsoleConfig.ShareManualDecisions = ptr.Of(false) diff --git a/pkg/csconfig/crowdsec_service.go b/pkg/csconfig/crowdsec_service.go index 36d38cf74..7820595b4 100644 --- a/pkg/csconfig/crowdsec_service.go +++ b/pkg/csconfig/crowdsec_service.go @@ -6,7 +6,7 @@ import ( "path/filepath" log "github.com/sirupsen/logrus" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/ptr" ) @@ -133,19 +133,16 @@ func (c *Config) LoadCrowdsec() error { } if err = c.LoadAPIClient(); err != nil { - return fmt.Errorf("loading api client: %s", err) + return fmt.Errorf("loading api client: %w", err) } return nil } func (c *CrowdsecServiceCfg) DumpContextConfigFile() error { - var out []byte - var err error - // XXX: MakeDirs - - if out, err = yaml.Marshal(c.ContextToSend); err != nil { + out, err := yaml.Marshal(c.ContextToSend) + if err != nil { return fmt.Errorf("while marshaling ConsoleConfig (for %s): %w", c.ConsoleContextPath, err) } diff --git a/pkg/csconfig/database.go b/pkg/csconfig/database.go index 5149b4ae3..2df220785 100644 --- a/pkg/csconfig/database.go +++ b/pkg/csconfig/database.go @@ -1,6 +1,7 @@ package csconfig import ( + "errors" "fmt" "time" @@ -45,6 +46,7 @@ type AuthGCCfg struct { type FlushDBCfg struct { MaxItems *int `yaml:"max_items,omitempty"` + // We could unmarshal as time.Duration, but alert filters right now are a map of strings MaxAge *string `yaml:"max_age,omitempty"` BouncersGC *AuthGCCfg `yaml:"bouncers_autodelete,omitempty"` AgentsGC *AuthGCCfg `yaml:"agents_autodelete,omitempty"` @@ -52,7 +54,7 @@ type FlushDBCfg struct { func (c *Config) LoadDBConfig(inCli bool) error { if c.DbConfig == nil { - return fmt.Errorf("no database configuration provided") + return errors.New("no database configuration provided") } if c.Cscli != nil { @@ -86,6 +88,7 @@ func (c *Config) LoadDBConfig(inCli bool) error { func (d *DatabaseCfg) ConnectionString() string { connString := "" + switch d.Type { case "sqlite": var sqliteConnectionStringParameters string @@ -94,6 +97,7 @@ func (d *DatabaseCfg) ConnectionString() string { } else { sqliteConnectionStringParameters = "_busy_timeout=100000&_fk=1" } + connString = fmt.Sprintf("file:%s?%s", d.DbPath, sqliteConnectionStringParameters) case "mysql": if d.isSocketConfig() { @@ -108,6 +112,7 @@ func (d *DatabaseCfg) ConnectionString() string { connString = fmt.Sprintf("host=%s port=%d user=%s dbname=%s password=%s sslmode=%s", d.Host, d.Port, d.User, d.DbName, d.Password, d.Sslmode) } } + return connString } @@ -121,8 +126,10 @@ func (d *DatabaseCfg) ConnectionDialect() (string, string, error) { if d.Type != "pgx" { log.Debugf("database type '%s' is deprecated, switching to 'pgx' instead", d.Type) } + return "pgx", dialect.Postgres, nil } + return "", "", fmt.Errorf("unknown database type '%s'", d.Type) } diff --git a/pkg/csconfig/profiles.go b/pkg/csconfig/profiles.go index ad3779ed1..6fbb8ed8b 100644 --- a/pkg/csconfig/profiles.go +++ b/pkg/csconfig/profiles.go @@ -6,7 +6,7 @@ import ( "fmt" "io" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/yamlpatch" @@ -23,43 +23,50 @@ import ( type ProfileCfg struct { Name string `yaml:"name,omitempty"` Debug *bool `yaml:"debug,omitempty"` - Filters []string `yaml:"filters,omitempty"` //A list of OR'ed expressions. the models.Alert object + Filters []string `yaml:"filters,omitempty"` // A list of OR'ed expressions. the models.Alert object Decisions []models.Decision `yaml:"decisions,omitempty"` DurationExpr string `yaml:"duration_expr,omitempty"` - OnSuccess string `yaml:"on_success,omitempty"` //continue or break - OnFailure string `yaml:"on_failure,omitempty"` //continue or break - OnError string `yaml:"on_error,omitempty"` //continue, break, error, report, apply, ignore + OnSuccess string `yaml:"on_success,omitempty"` // continue or break + OnFailure string `yaml:"on_failure,omitempty"` // continue or break + OnError string `yaml:"on_error,omitempty"` // continue, break, error, report, apply, ignore Notifications []string `yaml:"notifications,omitempty"` } func (c *LocalApiServerCfg) LoadProfiles() error { if c.ProfilesPath == "" { - return fmt.Errorf("empty profiles path") + return errors.New("empty profiles path") } patcher := yamlpatch.NewPatcher(c.ProfilesPath, ".local") + fcontent, err := patcher.PrependedPatchContent() if err != nil { return err } + reader := bytes.NewReader(fcontent) dec := yaml.NewDecoder(reader) - dec.SetStrict(true) + dec.KnownFields(true) + for { t := ProfileCfg{} + err = dec.Decode(&t) if err != nil { if errors.Is(err, io.EOF) { break } + return fmt.Errorf("while decoding %s: %w", c.ProfilesPath, err) } + c.Profiles = append(c.Profiles, &t) } if len(c.Profiles) == 0 { - return fmt.Errorf("zero profiles loaded for LAPI") + return errors.New("zero profiles loaded for LAPI") } + return nil } diff --git a/pkg/csconfig/simulation.go b/pkg/csconfig/simulation.go index 0d09aa478..bf121ef56 100644 --- a/pkg/csconfig/simulation.go +++ b/pkg/csconfig/simulation.go @@ -1,10 +1,13 @@ package csconfig import ( + "bytes" + "errors" "fmt" + "io" "path/filepath" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/crowdsecurity/go-cs-lib/yamlpatch" ) @@ -20,37 +23,51 @@ func (s *SimulationConfig) IsSimulated(scenario string) bool { if s.Simulation != nil && *s.Simulation { simulated = true } + for _, excluded := range s.Exclusions { if excluded == scenario { simulated = !simulated break } } + return simulated } func (c *Config) LoadSimulation() error { simCfg := SimulationConfig{} + if c.ConfigPaths.SimulationFilePath == "" { c.ConfigPaths.SimulationFilePath = filepath.Clean(c.ConfigPaths.ConfigDir + "/simulation.yaml") } patcher := yamlpatch.NewPatcher(c.ConfigPaths.SimulationFilePath, ".local") + rcfg, err := patcher.MergedPatchContent() if err != nil { return err } - if err := yaml.UnmarshalStrict(rcfg, &simCfg); err != nil { - return fmt.Errorf("while unmarshaling simulation file '%s' : %s", c.ConfigPaths.SimulationFilePath, err) + + dec := yaml.NewDecoder(bytes.NewReader(rcfg)) + dec.KnownFields(true) + + if err := dec.Decode(&simCfg); err != nil { + if !errors.Is(err, io.EOF) { + return fmt.Errorf("while unmarshaling simulation file '%s': %w", c.ConfigPaths.SimulationFilePath, err) + } } + if simCfg.Simulation == nil { simCfg.Simulation = new(bool) } + if c.Crowdsec != nil { c.Crowdsec.SimulationConfig = &simCfg } + if c.Cscli != nil { c.Cscli.SimulationConfig = &simCfg } + return nil } diff --git a/pkg/csconfig/simulation_test.go b/pkg/csconfig/simulation_test.go index 01f05e397..71b09ee39 100644 --- a/pkg/csconfig/simulation_test.go +++ b/pkg/csconfig/simulation_test.go @@ -60,7 +60,7 @@ func TestSimulationLoading(t *testing.T) { }, Crowdsec: &CrowdsecServiceCfg{}, }, - expectedErr: "while unmarshaling simulation file './testdata/config.yaml' : yaml: unmarshal errors", + expectedErr: "while unmarshaling simulation file './testdata/config.yaml': yaml: unmarshal errors", }, { name: "basic bad file content", @@ -71,7 +71,7 @@ func TestSimulationLoading(t *testing.T) { }, Crowdsec: &CrowdsecServiceCfg{}, }, - expectedErr: "while unmarshaling simulation file './testdata/config.yaml' : yaml: unmarshal errors", + expectedErr: "while unmarshaling simulation file './testdata/config.yaml': yaml: unmarshal errors", }, }