--- Begin Message ---
Package: release.debian.org
Severity: normal
Tags: bookworm
User: release.debian....@packages.debian.org
Usertags: pu
X-Debbugs-Cc: debian.pack...@crowdsec.net
Hi,
[ Reason ]
I'd like to fix serious bug #1040976 in bookworm. In a nutshell, both
upstream and I missed the fact Debian 12 comes without rsyslog by
default now, meaning /var/log/auth.log stays empty and the detection of
failed SSH logins is ineffective.
[ Impact ]
There's still some protection provided by the crowd-generated decisions,
but for a tool that's commonly described as a fail2ban on steroids, this
is a very bad bug, giving users some illusion of security.
[ Tests ]
Both upstream and I have tested the package uploaded to unstable and the
one prepared with the attached debdiff in bookworm VMs, “attacking” them
over SSH, and checking that:
- `cscli metrics` reports SSH lines under both “Acquisition Metrics”
(confirming the update of the default config for the acquisition part
is fine) and “Parser Metrics” (confirming the relevant parser are
indeed able to parse and extract information from those lines).
Sample output can be found at the bottom of this mail — keeping in
mind it's fine to have unparsed lines, we're focussing on some
specific strings to identify unwanted activity.
- An online machine would report the unwanted activity to the Central
API, which confirms that we're indeed participating in the bad agent
reporting effort. (Upstream uses disposable VMs in the cloud to test
this: some whitelists are in place, and local IPs are never reported,
so I haven't tested that myself.) That's confirmed via such lines in
the logs: “Signal push: 1 signals to push”.
I also verified that installing bookworm's version and upgrading to the
proposed package doesn't trigger any undesired prompts about conffiles,
does deploy the updated config, and does activate the detection of
failed SSH logins immediately.
[ Risks ]
Trying to have some edits in /etc/crowdsec/acquis.yaml depending on
what's installed on a system definitely seemed riskier, and more
cumbersome for users (leading to prompts during upgrades etc.). That's
why we went for the 0017 patch instead (enabling journalctl
unconditionally). Upstream anticipated the obvious problem: the engine
would error out if it's not available, that's why there's a longer 0018
patch to allow for unavailable datasources (that part has been tested by
me as well, moving the journalctl executable out of the way).
Regarding patches:
- 0017 is only in a PR for the time being; some colleagues of my usual
contact (who is very well versed into Debian topics) would like some
more time to think about the impact for other distributions before
merging it into master, but it seems very clear to me we should go
this route for Debian.
- 0018 was merged into master, and backported to their releases/1.4.x
branch.
- 0019 is just about disabling a buggy test.
[ Checklist ]
[x] *all* changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in stable
[x] the issue is verified as fixed in unstable
[ Changes ]
Since we're talking about a configuration file (marked conffiles), the
easiest is to enable the journalctl datasource in all cases [0017], but
that required another patch to make sure the engine doesn't error out
when that datasource is present but not available [0018]; the resulting
1.4.6-5 package uploaded to unstable then uncovered flaws in a test case
during autopkgtest runs (the test seems fine during a normal build, but
errors out when autopkgtest runs build + test — which happens for most
if not all Go packages), hence another patch [0019]. The tracker page
doesn't list all autopkgtest runs yet, but that seems greener with
1.4.6-6 than it was with 1.4.6-5…
[ Other info ]
As mentioned above, some parts of `clsci metrics`'s output confirming
things are much better with the proposed package:
Acquisition Metrics:
╭─────────────────────────────────────────────────┬────────────┬──────────────┬────────────────┬────────────────────────╮
│ Source │ Lines read │ Lines
parsed │ Lines unparsed │ Lines poured to bucket │
├─────────────────────────────────────────────────┼────────────┼──────────────┼────────────────┼────────────────────────┤
│ journalctl:journalctl-_SYSTEMD_UNIT=ssh.service │ 14 │ 5
│ 9 │ - │
╰─────────────────────────────────────────────────┴────────────┴──────────────┴────────────────┴────────────────────────╯
Parser Metrics:
╭─────────────────────────────────┬──────┬────────┬──────────╮
│ Parsers │ Hits │ Parsed │ Unparsed │
├─────────────────────────────────┼──────┼────────┼──────────┤
│ crowdsecurity/sshd-logs │ 14 │ 5 │ 9 │
╰─────────────────────────────────┴──────┴────────┴──────────╯
Thanks for your time.
Cheers,
--
Cyril Brulebois -- Debian Consultant @ DEBAMAX -- https://debamax.com/
diff -Nru crowdsec-1.4.6/debian/changelog crowdsec-1.4.6/debian/changelog
--- crowdsec-1.4.6/debian/changelog 2023-05-31 18:54:17.000000000 +0200
+++ crowdsec-1.4.6/debian/changelog 2023-07-15 11:29:33.000000000 +0200
@@ -1,3 +1,32 @@
+crowdsec (1.4.6-6~deb12u1) bookworm; urgency=medium
+
+ * Rebuild for bookworm.
+ * Adjust gbp.conf accordingly.
+
+ -- Cyril Brulebois <cy...@debamax.com> Sat, 15 Jul 2023 11:29:33 +0200
+
+crowdsec (1.4.6-6) unstable; urgency=medium
+
+ * Disable unreliable TestStreaming test, as seen under autopkgtest
+ (https://github.com/crowdsecurity/crowdsec/issues/2352):
+ - 0019-disable-unreliable-test-TestStreaming.patch
+
+ -- Cyril Brulebois <cy...@debamax.com> Fri, 14 Jul 2023 20:54:27 +0200
+
+crowdsec (1.4.6-5) unstable; urgency=medium
+
+ * Fix default acquis.yaml to also include the journalctl datasource,
+ limited to the ssh.service unit, making sure acquisition works even
+ without the traditional auth.log file (Closes: #1040976):
+ - 0017-fix-default-acquisition.patch
+ * Make sure an invalid datasource doesn't make the engine error out,
+ making it possible to include the journalctl datasource in the default
+ config file unconditionally, without having to worry whether
+ journalctl is actually deployed and usable:
+ - 0018-non-fatal-errors-for-invalid-datasources.patch
+
+ -- Cyril Brulebois <cy...@debamax.com> Thu, 13 Jul 2023 17:23:01 +0200
+
crowdsec (1.4.6-4) unstable; urgency=medium
* Implement support for pending registration: since bouncers list crowdsec
diff -Nru crowdsec-1.4.6/debian/gbp.conf crowdsec-1.4.6/debian/gbp.conf
--- crowdsec-1.4.6/debian/gbp.conf 2023-03-20 23:10:51.000000000 +0100
+++ crowdsec-1.4.6/debian/gbp.conf 2023-07-15 11:28:38.000000000 +0200
@@ -1,3 +1,3 @@
[DEFAULT]
-debian-branch = debian/sid
+debian-branch = debian/bookworm
dist = DEP14
diff -Nru crowdsec-1.4.6/debian/patches/0017-fix-default-acquisition.patch
crowdsec-1.4.6/debian/patches/0017-fix-default-acquisition.patch
--- crowdsec-1.4.6/debian/patches/0017-fix-default-acquisition.patch
1970-01-01 01:00:00.000000000 +0100
+++ crowdsec-1.4.6/debian/patches/0017-fix-default-acquisition.patch
2023-07-13 17:20:18.000000000 +0200
@@ -0,0 +1,21 @@
+From: Manuel Sabban <git...@sabban.eu>
+Date: Wed Jun 28 10:23:40 2023 +0200
+Subject: Add journalctl for ssh by default
+Origin: https://github.com/crowdsecurity/crowdsec/pull/2316/
+diff --git a/config/acquis.yaml b/config/acquis.yaml
+index cc3631f3..69976b38 100644
+--- a/config/acquis.yaml
++++ b/config/acquis.yaml
+@@ -10,6 +10,12 @@ filenames:
+ - /var/log/syslog
+ labels:
+ type: syslog
++---
++source: journalctl
++journalctl_filter:
++ - "_SYSTEMD_UNIT=ssh.service"
++labels:
++ type: syslog
+ ---
+ filename: /var/log/apache2/*.log
+ labels:
diff -Nru
crowdsec-1.4.6/debian/patches/0018-non-fatal-errors-for-invalid-datasources.patch
crowdsec-1.4.6/debian/patches/0018-non-fatal-errors-for-invalid-datasources.patch
---
crowdsec-1.4.6/debian/patches/0018-non-fatal-errors-for-invalid-datasources.patch
1970-01-01 01:00:00.000000000 +0100
+++
crowdsec-1.4.6/debian/patches/0018-non-fatal-errors-for-invalid-datasources.patch
2023-07-13 17:20:20.000000000 +0200
@@ -0,0 +1,257 @@
+From 93d0beeb94641ee3f33ca78f3cdda229cfd95740 Mon Sep 17 00:00:00 2001
+From: mmetc <92726601+mm...@users.noreply.github.com>
+Date: Tue, 27 Jun 2023 10:46:25 +0200
+Subject: [PATCH] non-fatal error if some datasource can't be run (i.e.
+ journalctl but systemd is missing) (#2310)
+
+This on the other hand, gives a new fatal error when there are no valid
datasources.
+In the previous version, crowdsec kept running with just a warning if no
+acquisition yaml or dir were specified.
+---
+ cmd/crowdsec/crowdsec.go | 10 +++---
+ cmd/crowdsec/main.go | 4 +++
+ pkg/acquisition/acquisition.go | 31 ++++++++++++++---
+ pkg/acquisition/acquisition_test.go | 2 +-
+ tests/bats/01_crowdsec.bats | 49 +++++++++++++++++++++++----
+ tests/bin/assert-crowdsec-not-running | 2 +-
+ 6 files changed, 80 insertions(+), 18 deletions(-)
+
+Signed-off-by: Cyril Brulebois <cy...@debamax.com>
+
+diff --git a/cmd/crowdsec/crowdsec.go b/cmd/crowdsec/crowdsec.go
+index 84cf0838..a3b095ae 100644
+--- a/cmd/crowdsec/crowdsec.go
++++ b/cmd/crowdsec/crowdsec.go
+@@ -23,22 +23,22 @@ func initCrowdsec(cConfig *csconfig.Config)
(*parser.Parsers, error) {
+ var err error
+
+ // Populate cwhub package tools
+- if err := cwhub.GetHubIdx(cConfig.Hub); err != nil {
+- return &parser.Parsers{}, fmt.Errorf("Failed to load hub index
: %s", err)
++ if err = cwhub.GetHubIdx(cConfig.Hub); err != nil {
++ return &parser.Parsers{}, fmt.Errorf("while loading hub index :
%s", err)
+ }
+
+ // Start loading configs
+ csParsers := newParsers()
+ if csParsers, err = parser.LoadParsers(cConfig, csParsers); err != nil {
+- return &parser.Parsers{}, fmt.Errorf("Failed to load parsers:
%s", err)
++ return nil, fmt.Errorf("while loading parsers: %s", err)
+ }
+
+ if err := LoadBuckets(cConfig); err != nil {
+- return &parser.Parsers{}, fmt.Errorf("Failed to load scenarios:
%s", err)
++ return nil, fmt.Errorf("while loading scenarios: %s", err)
+ }
+
+ if err := LoadAcquisition(cConfig); err != nil {
+- return &parser.Parsers{}, fmt.Errorf("Error while loading
acquisition config : %s", err)
++ return nil, fmt.Errorf("while loading acquisition config: %s",
err)
+ }
+ return csParsers, nil
+ }
+diff --git a/cmd/crowdsec/main.go b/cmd/crowdsec/main.go
+index 43eb63ec..a25c372f 100644
+--- a/cmd/crowdsec/main.go
++++ b/cmd/crowdsec/main.go
+@@ -157,6 +157,10 @@ func LoadAcquisition(cConfig *csconfig.Config) error {
+ }
+ }
+
++ if len(dataSources) == 0 {
++ return fmt.Errorf("no datasource enabled")
++ }
++
+ return nil
+ }
+
+diff --git a/pkg/acquisition/acquisition.go b/pkg/acquisition/acquisition.go
+index 43093a50..2461064d 100644
+--- a/pkg/acquisition/acquisition.go
++++ b/pkg/acquisition/acquisition.go
+@@ -25,6 +25,20 @@ import (
+ tomb "gopkg.in/tomb.v2"
+ )
+
++type DataSourceUnavailableError struct {
++ Name string
++ Err error
++}
++
++func (e *DataSourceUnavailableError) Error() string {
++ return fmt.Sprintf("datasource '%s' is not available: %v", e.Name,
e.Err)
++}
++
++func (e *DataSourceUnavailableError) Unwrap() error {
++ return e.Err
++}
++
++
+ // The interface each datasource must implement
+ type DataSource interface {
+ GetMetrics() []prometheus.Collector // Returns
pointers to metrics that are managed by the module
+@@ -86,8 +100,11 @@ func GetDataSourceIface(dataSourceType string) DataSource {
+ return nil
+ }
+
++// DataSourceConfigure creates and returns a DataSource object from a
configuration,
++// if the configuration is not valid it returns an error.
++// If the datasource can't be run (eg. journalctl not available), it still
returns an error which
++// can be checked for the appropriate action.
+ func DataSourceConfigure(commonConfig configuration.DataSourceCommonCfg)
(*DataSource, error) {
+-
+ //we dump it back to []byte, because we want to decode the yaml blob
twice :
+ //once to DataSourceCommonCfg, and then later to the dedicated type of
the datasource
+ yamlConfig, err := yaml.Marshal(commonConfig)
+@@ -112,7 +129,7 @@ func DataSourceConfigure(commonConfig
configuration.DataSourceCommonCfg) (*DataS
+ subLogger := clog.WithFields(customLog)
+ /* check eventual dependencies are satisfied (ie. journald will
check journalctl availability) */
+ if err := dataSrc.CanRun(); err != nil {
+- return nil, errors.Wrapf(err, "datasource %s cannot be
run", commonConfig.Source)
++ return nil, &DataSourceUnavailableError{Name:
commonConfig.Source, Err: err}
+ }
+ /* configure the actual datasource */
+ if err := dataSrc.Configure(yamlConfig, subLogger); err != nil {
+@@ -179,10 +196,11 @@ func LoadAcquisitionFromFile(config
*csconfig.CrowdsecServiceCfg) ([]DataSource,
+ }
+ dec := yaml.NewDecoder(yamlFile)
+ dec.SetStrict(true)
++ idx := -1
+ for {
+ var sub configuration.DataSourceCommonCfg
+- var idx int
+ err = dec.Decode(&sub)
++ idx += 1
+ if err != nil {
+ if ! errors.Is(err, io.EOF) {
+ return nil, errors.Wrapf(err, "failed
to yaml decode %s", acquisFile)
+@@ -199,7 +217,6 @@ func LoadAcquisitionFromFile(config
*csconfig.CrowdsecServiceCfg) ([]DataSource,
+ if len(sub.Labels) == 0 {
+ if sub.Source == "" {
+ log.Debugf("skipping empty item in %s",
acquisFile)
+- idx += 1
+ continue
+ }
+ return nil, fmt.Errorf("missing labels in %s
(position: %d)", acquisFile, idx)
+@@ -212,10 +229,14 @@ func LoadAcquisitionFromFile(config
*csconfig.CrowdsecServiceCfg) ([]DataSource,
+ }
+ src, err := DataSourceConfigure(sub)
+ if err != nil {
++ var dserr *DataSourceUnavailableError
++ if errors.As(err, &dserr) {
++ log.Error(err)
++ continue
++ }
+ return nil, errors.Wrapf(err, "while
configuring datasource of type %s from %s (position: %d)", sub.Source,
acquisFile, idx)
+ }
+ sources = append(sources, *src)
+- idx += 1
+ }
+ }
+ return sources, nil
+diff --git a/pkg/acquisition/acquisition_test.go
b/pkg/acquisition/acquisition_test.go
+index a547970a..a96044ca 100644
+--- a/pkg/acquisition/acquisition_test.go
++++ b/pkg/acquisition/acquisition_test.go
+@@ -171,7 +171,7 @@ log_level: debug
+ source: mock_cant_run
+ wowo: ajsajasjas
+ `,
+- ExpectedError: "datasource mock_cant_run cannot be run:
can't run bro",
++ ExpectedError: "datasource 'mock_cant_run' is not
available: can't run bro",
+ },
+ }
+
+diff --git a/tests/bats/01_crowdsec.bats b/tests/bats/01_crowdsec.bats
+index a60b576d..f8272eb9 100644
+--- a/tests/bats/01_crowdsec.bats
++++ b/tests/bats/01_crowdsec.bats
+@@ -148,9 +148,10 @@ teardown() {
+ rm -f "$ACQUIS_DIR"
+
+ config_set '.common.log_media="stdout"'
+- run -124 --separate-stderr timeout 2s "${CROWDSEC}"
++ run -1 --separate-stderr timeout 2s "${CROWDSEC}"
+ # check warning
+- assert_stderr_line --partial "no acquisition file found"
++ assert_stderr --partial "no acquisition file found"
++ assert_stderr --partial "crowdsec init: while loading acquisition config:
no datasource enabled"
+ }
+
+ @test "crowdsec (error if acquisition_path and acquisition_dir are not
defined)" {
+@@ -163,19 +164,55 @@ teardown() {
+ config_set '.crowdsec_service.acquisition_dir=""'
+
+ config_set '.common.log_media="stdout"'
+- run -124 --separate-stderr timeout 2s "${CROWDSEC}"
++ run -1 --separate-stderr timeout 2s "${CROWDSEC}"
+ # check warning
+- assert_stderr_line --partial "no acquisition_path or acquisition_dir
specified"
++ assert_stderr --partial "no acquisition_path or acquisition_dir specified"
++ assert_stderr --partial "crowdsec init: while loading acquisition config:
no datasource enabled"
+ }
+
+ @test "crowdsec (no error if acquisition_path is empty string but
acquisition_dir is not empty)" {
+ ACQUIS_YAML=$(config_get '.crowdsec_service.acquisition_path')
+- rm -f "$ACQUIS_YAML"
+ config_set '.crowdsec_service.acquisition_path=""'
+
+ ACQUIS_DIR=$(config_get '.crowdsec_service.acquisition_dir')
+ mkdir -p "$ACQUIS_DIR"
+- touch "$ACQUIS_DIR"/foo.yaml
++ mv "$ACQUIS_YAML" "$ACQUIS_DIR"/foo.yaml
+
+ run -124 --separate-stderr timeout 2s "${CROWDSEC}"
++
++ # now, if foo.yaml is empty instead, there won't be valid datasources.
++
++ cat /dev/null >"$ACQUIS_DIR"/foo.yaml
++
++ run --separate-stderr -1 timeout 2s "${CROWDSEC}"
++ assert_stderr --partial "crowdsec init: while loading acquisition config:
no datasource enabled"
+ }
++
++@test "crowdsec (disabled datasources)" {
++ config_set '.common.log_media="stdout"'
++
++ # a datasource cannot run - missing journalctl command
++
++ ACQUIS_DIR=$(config_get '.crowdsec_service.acquisition_dir')
++ mkdir -p "$ACQUIS_DIR"
++ cat >"$ACQUIS_DIR"/foo.yaml <<-EOT
++ source: journalctl
++ journalctl_filter:
++ - "_SYSTEMD_UNIT=ssh.service"
++ labels:
++ type: syslog
++ EOT
++
++ run --separate-stderr -124 timeout 2s env PATH='' "${CROWDSEC}"
++ #shellcheck disable=SC2016
++ assert_stderr --partial 'datasource '\''journalctl'\'' is not available:
exec: "journalctl": executable file not found in $PATH'
++
++ # if all datasources are disabled, crowdsec should exit
++
++ ACQUIS_YAML=$(config_get '.crowdsec_service.acquisition_path')
++ rm -f "$ACQUIS_YAML"
++ config_set '.crowdsec_service.acquisition_path=""'
++
++ run --separate-stderr -1 timeout 2s env PATH='' "${CROWDSEC}"
++ assert_stderr --partial "crowdsec init: while loading acquisition config:
no datasource enabled"
++ }
+diff --git a/tests/bin/assert-crowdsec-not-running
b/tests/bin/assert-crowdsec-not-running
+index c6f381af..d678cb4f 100755
+--- a/tests/bin/assert-crowdsec-not-running
++++ b/tests/bin/assert-crowdsec-not-running
+@@ -1,7 +1,7 @@
+ #!/usr/bin/env bash
+
+ is_crowdsec_running() {
+- PIDS=$(pgrep -x 'crowdsec|crowdsec.test|crowdsec.cover')
++ PIDS=$(pgrep -x 'crowdsec|crowdsec.test|crowdsec.cover' 2>/dev/null)
+ }
+
+ # The process can be slow, especially on CI and during test coverage.
+--
+2.30.2
+
diff -Nru
crowdsec-1.4.6/debian/patches/0019-disable-unreliable-test-TestStreaming.patch
crowdsec-1.4.6/debian/patches/0019-disable-unreliable-test-TestStreaming.patch
---
crowdsec-1.4.6/debian/patches/0019-disable-unreliable-test-TestStreaming.patch
1970-01-01 01:00:00.000000000 +0100
+++
crowdsec-1.4.6/debian/patches/0019-disable-unreliable-test-TestStreaming.patch
2023-07-15 11:28:09.000000000 +0200
@@ -0,0 +1,19 @@
+From: Cyril Brulebois <cy...@debamax.com>
+Date: Fri, 14 Jul 2023 20:29:16 +0200
+Subject: Disable buggy test
+
+Adding the 0017 and 0018 patches had the side effect of uncovering
+reliability issues in TestStreaming(). Let's not block the bugfix for
+#1040976 on fixing that test: it's more important for the acquisition
+bugfix to make its way into testing and into stable.
+
+--- a/pkg/acquisition/modules/journalctl/journalctl_test.go
++++ b/pkg/acquisition/modules/journalctl/journalctl_test.go
+@@ -178,6 +178,7 @@ journalctl_filter:
+ }
+
+ func TestStreaming(t *testing.T) {
++ t.Skip("unreliable test:
https://github.com/crowdsecurity/crowdsec/issues/2352";)
+ if runtime.GOOS == "windows" {
+ t.Skip("Skipping test on windows")
+ }
diff -Nru crowdsec-1.4.6/debian/patches/series
crowdsec-1.4.6/debian/patches/series
--- crowdsec-1.4.6/debian/patches/series 2023-03-20 23:10:51.000000000
+0100
+++ crowdsec-1.4.6/debian/patches/series 2023-07-15 11:28:09.000000000
+0200
@@ -10,3 +10,6 @@
0014-silence-yaml-patching.patch
0015-silence-not-latest-version.patch
0016-try-to-make-reproducible-build-work-2119.patch
+0017-fix-default-acquisition.patch
+0018-non-fatal-errors-for-invalid-datasources.patch
+0019-disable-unreliable-test-TestStreaming.patch
--- End Message ---
