From 01c55f0e69770a6ff35d0d8e4be5b9ffd1935a8e Mon Sep 17 00:00:00 2001 From: marco Date: Mon, 11 Mar 2024 11:28:19 +0100 Subject: [PATCH 1/3] fix ennvar expansion in configuration file --- cmd/root.go | 13 ++++++++----- pkg/cfg/config.go | 11 ++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 168834dc..5359703b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "context" "flag" "fmt" @@ -20,8 +19,10 @@ import ( csbouncer "github.com/crowdsecurity/go-cs-bouncer" "github.com/crowdsecurity/go-cs-lib/csdaemon" + "github.com/crowdsecurity/go-cs-lib/csstring" "github.com/crowdsecurity/go-cs-lib/version" + "github.com/crowdsecurity/crowdsec/pkg/models" "github.com/crowdsecurity/cs-firewall-bouncer/pkg/backend" @@ -152,17 +153,19 @@ func Execute() error { return fmt.Errorf("configuration file is required") } - configBytes, err := cfg.MergedConfig(*configPath) + configMerged, err := cfg.MergedConfig(*configPath) if err != nil { return fmt.Errorf("unable to read config file: %w", err) } if *showConfig { - fmt.Println(string(configBytes)) + fmt.Println(string(configMerged)) return nil } - config, err := cfg.NewConfig(bytes.NewReader(configBytes)) + configExpanded := csstring.StrictExpand(string(configMerged), os.LookupEnv) + + config, err := cfg.NewConfig(strings.NewReader(configExpanded)) if err != nil { return fmt.Errorf("unable to load configuration: %w", err) } @@ -186,7 +189,7 @@ func Execute() error { bouncer := &csbouncer.StreamBouncer{} - err = bouncer.ConfigReader(bytes.NewReader(configBytes)) + err = bouncer.ConfigReader(strings.NewReader(configExpanded)) if err != nil { return err } diff --git a/pkg/cfg/config.go b/pkg/cfg/config.go index 0251d913..6656876f 100644 --- a/pkg/cfg/config.go +++ b/pkg/cfg/config.go @@ -1,14 +1,13 @@ package cfg import ( + "errors" "fmt" "io" - "os" log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" - "github.com/crowdsecurity/go-cs-lib/csstring" "github.com/crowdsecurity/go-cs-lib/ptr" "github.com/crowdsecurity/go-cs-lib/yamlpatch" ) @@ -86,9 +85,7 @@ func NewConfig(reader io.Reader) (*BouncerConfig, error) { return nil, err } - configBuff := csstring.StrictExpand(string(fcontent), os.LookupEnv) - - err = yaml.Unmarshal([]byte(configBuff), &config) + err = yaml.Unmarshal(fcontent, &config) if err != nil { return nil, fmt.Errorf("failed to unmarshal: %w", err) } @@ -98,7 +95,7 @@ func NewConfig(reader io.Reader) (*BouncerConfig, error) { } if config.Mode == "" { - return nil, fmt.Errorf("config does not contain 'mode'") + return nil, errors.New("config does not contain 'mode'") } if len(config.SupportedDecisionsTypes) == 0 { @@ -191,7 +188,7 @@ func nftablesConfig(config *BouncerConfig) error { } if !*config.Nftables.Ipv4.Enabled && !*config.Nftables.Ipv6.Enabled { - return fmt.Errorf("both IPv4 and IPv6 disabled, doing nothing") + return errors.New("both IPv4 and IPv6 disabled, doing nothing") } if config.NftablesHooks == nil || len(config.NftablesHooks) == 0 { From f0fa6421f26c2aecfc80678bc6ec488bfb1e23e2 Mon Sep 17 00:00:00 2001 From: marco Date: Mon, 11 Mar 2024 11:32:15 +0100 Subject: [PATCH 2/3] CI: test variable expansion --- cmd/root.go | 4 +--- test/bouncer/test_yaml_local.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 5359703b..9348ae95 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -17,14 +17,12 @@ import ( "golang.org/x/exp/slices" "golang.org/x/sync/errgroup" + "github.com/crowdsecurity/crowdsec/pkg/models" csbouncer "github.com/crowdsecurity/go-cs-bouncer" "github.com/crowdsecurity/go-cs-lib/csdaemon" "github.com/crowdsecurity/go-cs-lib/csstring" "github.com/crowdsecurity/go-cs-lib/version" - - "github.com/crowdsecurity/crowdsec/pkg/models" - "github.com/crowdsecurity/cs-firewall-bouncer/pkg/backend" "github.com/crowdsecurity/cs-firewall-bouncer/pkg/cfg" "github.com/crowdsecurity/cs-firewall-bouncer/pkg/metrics" diff --git a/test/bouncer/test_yaml_local.py b/test/bouncer/test_yaml_local.py index 3cab695b..3c28b667 100644 --- a/test/bouncer/test_yaml_local.py +++ b/test/bouncer/test_yaml_local.py @@ -1,3 +1,5 @@ +import os + def test_yaml_local(bouncer, fw_cfg_factory): cfg = fw_cfg_factory() @@ -21,3 +23,18 @@ def test_yaml_local(bouncer, fw_cfg_factory): ]) fw.proc.wait(timeout=0.2) assert not fw.proc.is_running() + + # variable expansion + + config_local = { + 'mode': '$BOUNCER_MODE' + } + + os.environ['BOUNCER_MODE'] = 'fromenv' + + with bouncer(cfg, config_local=config_local) as fw: + fw.wait_for_lines_fnmatch([ + "*firewall 'fromenv' is not supported*", + ]) + fw.proc.wait(timeout=0.2) + assert not fw.proc.is_running() From 81523bc8d4d9e57a3eb6e1bd8139cafb262b0673 Mon Sep 17 00:00:00 2001 From: marco Date: Mon, 11 Mar 2024 14:05:13 +0100 Subject: [PATCH 3/3] lint --- cmd/root.go | 16 +++++++++++----- pkg/backend/backend.go | 5 +++-- pkg/cfg/config.go | 2 +- pkg/cfg/logging.go | 4 ++-- pkg/iptables/iptables.go | 11 ++++++----- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 9348ae95..076928d6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "flag" "fmt" "net" @@ -17,12 +18,13 @@ import ( "golang.org/x/exp/slices" "golang.org/x/sync/errgroup" - "github.com/crowdsecurity/crowdsec/pkg/models" csbouncer "github.com/crowdsecurity/go-cs-bouncer" "github.com/crowdsecurity/go-cs-lib/csdaemon" "github.com/crowdsecurity/go-cs-lib/csstring" "github.com/crowdsecurity/go-cs-lib/version" + "github.com/crowdsecurity/crowdsec/pkg/models" + "github.com/crowdsecurity/cs-firewall-bouncer/pkg/backend" "github.com/crowdsecurity/cs-firewall-bouncer/pkg/cfg" "github.com/crowdsecurity/cs-firewall-bouncer/pkg/metrics" @@ -46,9 +48,9 @@ func HandleSignals(ctx context.Context) error { case s := <-signalChan: switch s { case syscall.SIGTERM: - return fmt.Errorf("received SIGTERM") + return errors.New("received SIGTERM") case os.Interrupt: // cross-platform SIGINT - return fmt.Errorf("received interrupt") + return errors.New("received interrupt") } case <-ctx.Done(): return ctx.Err() @@ -75,6 +77,7 @@ func deleteDecisions(backend *backend.BackendCTX, decisions []*models.Decision, } log.Debugf("deleted %s", *d.Value) + nbDeletedDecisions++ } @@ -111,6 +114,7 @@ func addDecisions(backend *backend.BackendCTX, decisions []*models.Decision, con } log.Debugf("Adding '%s' for '%s'", *d.Value, *d.Duration) + nbNewDecisions++ } @@ -148,7 +152,7 @@ func Execute() error { } if configPath == nil || *configPath == "" { - return fmt.Errorf("configuration file is required") + return errors.New("configuration file is required") } configMerged, err := cfg.MergedConfig(*configPath) @@ -210,7 +214,7 @@ func Execute() error { g.Go(func() error { bouncer.Run(ctx) - return fmt.Errorf("bouncer stream halted") + return errors.New("bouncer stream halted") }) if config.PrometheusConfig.Enabled { @@ -235,6 +239,7 @@ func Execute() error { g.Go(func() error { log.Infof("Processing new and deleted decisions . . .") + for { select { case <-ctx.Done(): @@ -243,6 +248,7 @@ func Execute() error { if decisions == nil { continue } + deleteDecisions(backend, decisions.Deleted, config) addDecisions(backend, decisions.New, config) } diff --git a/pkg/backend/backend.go b/pkg/backend/backend.go index b7f10310..b66fb908 100644 --- a/pkg/backend/backend.go +++ b/pkg/backend/backend.go @@ -1,6 +1,7 @@ package backend import ( + "errors" "fmt" "runtime" @@ -72,7 +73,7 @@ func NewBackend(config *cfg.BouncerConfig) (*BackendCTX, error) { switch config.Mode { case cfg.IptablesMode, cfg.IpsetMode: if runtime.GOOS != "linux" { - return nil, fmt.Errorf("iptables and ipset is linux only") + return nil, errors.New("iptables and ipset is linux only") } b.firewall, err = iptables.NewIPTables(config) @@ -81,7 +82,7 @@ func NewBackend(config *cfg.BouncerConfig) (*BackendCTX, error) { } case cfg.NftablesMode: if runtime.GOOS != "linux" { - return nil, fmt.Errorf("nftables is linux only") + return nil, errors.New("nftables is linux only") } b.firewall, err = nftables.NewNFTables(config) diff --git a/pkg/cfg/config.go b/pkg/cfg/config.go index 6656876f..71d6014b 100644 --- a/pkg/cfg/config.go +++ b/pkg/cfg/config.go @@ -149,7 +149,7 @@ func NewConfig(reader io.Reader) (*BouncerConfig, error) { return config, nil } -func pfConfig(config *BouncerConfig) error { +func pfConfig(_ *BouncerConfig) error { return nil } diff --git a/pkg/cfg/logging.go b/pkg/cfg/logging.go index 1f6bc977..01afcc39 100644 --- a/pkg/cfg/logging.go +++ b/pkg/cfg/logging.go @@ -1,7 +1,7 @@ package cfg import ( - "fmt" + "errors" "io" "os" "path/filepath" @@ -75,7 +75,7 @@ func (c *LoggingConfig) setDefaults() { func (c *LoggingConfig) validate() error { if c.LogMode != "stdout" && c.LogMode != "file" { - return fmt.Errorf("log_mode should be either 'stdout' or 'file'") + return errors.New("log_mode should be either 'stdout' or 'file'") } return nil diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go index fcc31145..8541333d 100644 --- a/pkg/iptables/iptables.go +++ b/pkg/iptables/iptables.go @@ -4,6 +4,7 @@ package iptables import ( + "errors" "fmt" "os/exec" "strings" @@ -68,7 +69,7 @@ func NewIPTables(config *cfg.BouncerConfig) (types.Backend, error) { ipsetBin, err := exec.LookPath("ipset") if err != nil { - return nil, fmt.Errorf("unable to find ipset") + return nil, errors.New("unable to find ipset") } ipv4Ctx.ipsetBin = ipsetBin @@ -77,7 +78,7 @@ func NewIPTables(config *cfg.BouncerConfig) (types.Backend, error) { } else { ipv4Ctx.iptablesBin, err = exec.LookPath("iptables") if err != nil { - return nil, fmt.Errorf("unable to find iptables") + return nil, errors.New("unable to find iptables") } ipv4Ctx.Chains = config.IptablesChains for _, v := range config.IptablesChains { @@ -109,7 +110,7 @@ func NewIPTables(config *cfg.BouncerConfig) (types.Backend, error) { } else { ipv6Ctx.iptablesBin, err = exec.LookPath("ip6tables") if err != nil { - return nil, fmt.Errorf("unable to find ip6tables") + return nil, errors.New("unable to find ip6tables") } ipv6Ctx.Chains = config.IptablesChains for _, v := range config.IptablesChains { @@ -237,7 +238,7 @@ func (ipt *iptables) Delete(decision *models.Decision) error { } if err := ipt.v6.delete(decision); err != nil { - return fmt.Errorf("failed deleting ban") + return errors.New("failed deleting ban") } done = true @@ -245,7 +246,7 @@ func (ipt *iptables) Delete(decision *models.Decision) error { if strings.Contains(*decision.Value, ".") { if err := ipt.v4.delete(decision); err != nil { - return fmt.Errorf("failed deleting ban") + return errors.New("failed deleting ban") } done = true