[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1037000: marked as done (unblock: crowdsec/1.4.6-4 and bouncers)

Your message dated Thu, 1 Jun 2023 13:56:10 +0200
with message-id <53079baa-38c9-98d8-6bfa-94874cae44f3@debian.org>
and subject line Re: Bug#1037000: unblock: crowdsec/1.4.6-4 and bouncers
has caused the Debian Bug report #1037000,
regarding unblock: crowdsec/1.4.6-4 and bouncers
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1037000: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1037000
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Hi!

Please unblock packages crowdsec, crowdsec-custom-bouncer, and
crowdsec-firewall-bouncer.

I'm filing a single unblock request since all three packages are
entangled, and Paul suggested this would be appropriate:
 - both bouncers get the exact same changes (only the context differs);
 - crowdsec gets an extra snippet to deal with the pending registration
   requested by either one or both bouncers.

[ Reason ]
RC bugs #1035499 and #1036985 on the bouncers: they might fail to
install depending on the dpkg-level ordering as far as configuration
goes, that is: if crowdsec (which is listed in Recommends) is unpacked
but not configured, the `cscli` call fails and the postinst errors out.

Back when the bouncers were prepared, both upstream and I verified that
we could just install either bouncer package without a pre-existing
crowdsec package installed, but as seen with Andreas' piuparts-based
testing, we might get a different order over time, or across bouncers…

In any case, at the moment, a freshly-deployed Bookworm VM can't get
either bouncer installed, and I'd like to get that fixed in time for
12.0.

The proposed changes keep the existing code paths, and add detection for
the problematic case, queueing bouncer registration, letting crowdsec
catch up when it's finally configured. To prevent bouncers from starting
before crowdsec has dealt with the registration, I've added a condition
to both their systemd units, and a `deb-systemd-invoke start` call once
everything is ready.

[ Impact ]
Abysmal user experience without those fixes.

[ Tests ]
Both upstream and I have tested updated packages, stashed in a custom
repository, installing 1 to 3 packages in various order, making sure the
new code does the right thing. I've also verified this under piuparts,
seeing that policy-rc.d is respected as it should (and the start request
is ignored without triggering an error).

[ Risks ]
There might be tricky situations I haven't imagined or encountered, but
since we're basically keeping existing code, and just adding detection
and solution for a specific bad situation during the first installation,
I'm not sure what could regress.

[ 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 testing


unblock crowdsec/1.4.6-4
unblock crowdsec-custom-bouncer/0.0.15-3
unblock crowdsec-firewall-bouncer/0.0.25-3


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-03-19 00:25:07.000000000 +0100
+++ crowdsec-1.4.6/debian/changelog	2023-05-31 18:54:17.000000000 +0200
@@ -1,3 +1,16 @@
+crowdsec (1.4.6-4) unstable; urgency=medium
+
+  * Implement support for pending registration: since bouncers list crowdsec
+    in Recommends, we cannot guarantee the order in which bouncers and
+    crowdsec are configured (See: #1035499, #1036985). Bouncers can now
+    queue triplets (systemd unit name, bouncer identifier and API key) in
+    /var/lib/crowdsec/pending-registration. crowdsec.postinst will register
+    those bouncers, and start their systemd units after removing that file
+    (satisfying their ConditionPathExists=! on it).
+  * Replace `exit 0` with `break` in the preceding code block.
+
+ -- Cyril Brulebois <cyril@debamax.com>  Wed, 31 May 2023 18:54:17 +0200
+
 crowdsec (1.4.6-3) unstable; urgency=medium
 
   * When performing an upgrade from pre-1.4.x versions, apply a workaround
diff -Nru crowdsec-1.4.6/debian/crowdsec.postinst crowdsec-1.4.6/debian/crowdsec.postinst
--- crowdsec-1.4.6/debian/crowdsec.postinst	2023-03-18 14:40:31.000000000 +0100
+++ crowdsec-1.4.6/debian/crowdsec.postinst	2023-05-31 17:01:15.000000000 +0200
@@ -280,15 +280,35 @@
   for _ in $(seq 1 $MAX); do
     # Getting decisions means we can happily exit:
     if grep -qs 'added [0-9][0-9]* entries, deleted [0-9][0-9]* entries' $LOG; then
-      exit 0
+      break
     fi
     # Getting 0 new entries means we can happily trigger a restart then exit:
     if grep -qs 'received 0 new entries (expected if you just installed crowdsec)' $LOG; then
       echo "W: Restarting manually to force a CAPI pull (upstream #2120)" >&2
       deb-systemd-invoke restart 'crowdsec.service' >/dev/null || true
-      exit 0
+      break
     fi
     # Don't poll too aggressively:
     sleep 1
   done
 fi
+
+# Bouncer registration: they have crowdsec in Recommends only, so ordering isn't
+# guaranteed (#1035499, #1036985). Process pending registration if any, then
+# kick relevant systemd units once their ConditionPathExists is satisfied.
+PENDING=/var/lib/crowdsec/pending-registration
+if [ -f $PENDING ]; then
+  while read unit name key; do
+    units="${units:+$units }$unit"
+    bouncers="${bouncers:+$bouncers }$name"
+    # We don't need the API key to be echo'd back:
+    cscli --error -oraw bouncers add "$name" -k "$key" > /dev/null
+  done < $PENDING
+  rm -f $PENDING
+  echo "I: Registered bouncers: $bouncers" >&2
+
+  for unit in $units; do
+    deb-systemd-invoke start "$unit"
+  done
+  echo "I: Restarts units: $units" >&2
+fi

diff -Nru crowdsec-custom-bouncer-0.0.15/debian/changelog crowdsec-custom-bouncer-0.0.15/debian/changelog
--- crowdsec-custom-bouncer-0.0.15/debian/changelog	2023-03-21 01:02:17.000000000 +0100
+++ crowdsec-custom-bouncer-0.0.15/debian/changelog	2023-05-31 19:01:19.000000000 +0200
@@ -1,3 +1,21 @@
+crowdsec-custom-bouncer (0.0.15-3) unstable; urgency=medium
+
+  * Fix failure to install if crowdsec is unpacked but not configured
+    (Closes: #1035499):
+     - If both cscli and /etc/crowdsec/config.yaml exist, register as
+       previously, via `cscli bouncers add`.
+     - If cscli exists but /etc/crowdsec/config.yaml doesn't, create an
+       API key similarly to what cscli would do (32 hexadecimal digits),
+       and queue registration in /var/lib/crowdsec/pending-registration.
+  * Add ConditionPathExists=!/var/lib/crowdsec/pending-registration to the
+    systemd unit accordingly, so that it's skipped until crowdsec's
+    postinst deals with the pending-registration file, removes it, and
+    starts the units.
+  * Use mode 600 when creating crowdsec-custom-bouncer.yaml.local, since
+    it contains an API key.
+
+ -- Cyril Brulebois <cyril@debamax.com>  Wed, 31 May 2023 19:01:19 +0200
+
 crowdsec-custom-bouncer (0.0.15-2) unstable; urgency=medium
 
   * Add patch to honor log-related settings (upstream PR #54):
diff -Nru crowdsec-custom-bouncer-0.0.15/debian/crowdsec-custom-bouncer.postinst crowdsec-custom-bouncer-0.0.15/debian/crowdsec-custom-bouncer.postinst
--- crowdsec-custom-bouncer-0.0.15/debian/crowdsec-custom-bouncer.postinst	2023-03-21 00:58:41.000000000 +0100
+++ crowdsec-custom-bouncer-0.0.15/debian/crowdsec-custom-bouncer.postinst	2023-05-31 17:16:45.000000000 +0200
@@ -2,6 +2,7 @@
 set -e
 
 CONFIG=/etc/crowdsec/bouncers/crowdsec-custom-bouncer.yaml
+PENDING=/var/lib/crowdsec/pending-registration
 
 # Create a configuration override file only once, see README.Debian:
 if [ "$1" = configure ] && [ ! -f "$CONFIG.local" ]; then
@@ -13,10 +14,21 @@
     # machines; reuse the logic from crowdsec's postinst:
     unique=$(tr -dc 'a-zA-Z0-9' < /dev/urandom | fold -w 32 | head -n 1)
     id="CustomBouncer-$unique"
-    api_key=$(cscli --error -oraw bouncers add "$id")
-    if [ -z "$api_key" ]; then
-      echo "E: Local registration failed to yield an API key" >&2
-      exit 1
+
+    # crowdsec might be unpacked but not configured (#1035499):
+    if [ -f /etc/crowdsec/config.yaml ]; then
+      api_key=$(cscli --error -oraw bouncers add "$id")
+      if [ -z "$api_key" ]; then
+        echo "E: Local registration failed to yield an API key" >&2
+        exit 1
+      fi
+    else
+      api_key=$(tr -dc 'a-f0-9' < /dev/urandom | fold -w 32 | head -n 1)
+      if [ ! -f $PENDING ]; then
+        touch $PENDING
+        chmod 600 $PENDING
+      fi
+      echo "crowdsec-custom-bouncer $id $api_key" >> $PENDING
     fi
 
     # Store it so that it can be unregistered when purging the package:
@@ -28,6 +40,9 @@
   DEFAULT_SCRIPT=/usr/bin/true
   echo "W: Using $DEFAULT_SCRIPT as default custom script" >&2
 
+  touch "$CONFIG.local"
+  chmod 600 "$CONFIG.local"
+
   if [ -n "$api_key" ]; then
     cat > "$CONFIG.local" <<EOF
 bin_path: $DEFAULT_SCRIPT
diff -Nru crowdsec-custom-bouncer-0.0.15/debian/patches/0002-adjust-service.patch crowdsec-custom-bouncer-0.0.15/debian/patches/0002-adjust-service.patch
--- crowdsec-custom-bouncer-0.0.15/debian/patches/0002-adjust-service.patch	2023-03-21 00:58:41.000000000 +0100
+++ crowdsec-custom-bouncer-0.0.15/debian/patches/0002-adjust-service.patch	2023-05-31 17:15:02.000000000 +0200
@@ -9,12 +9,13 @@
 
 --- a/config/crowdsec-custom-bouncer.service
 +++ b/config/crowdsec-custom-bouncer.service
-@@ -1,14 +1,11 @@
+@@ -1,14 +1,12 @@
  [Unit]
  Description=The custom bouncer for CrowdSec
 -After=syslog.target network.target remote-fs.target nss-lookup.target crowdsec.service
 -
 +After=network.target remote-fs.target nss-lookup.target crowdsec.service
++ConditionPathExists=!/var/lib/crowdsec/pending-registration
  
  [Service]
  Type=notify

diff -Nru crowdsec-firewall-bouncer-0.0.25/debian/changelog crowdsec-firewall-bouncer-0.0.25/debian/changelog
--- crowdsec-firewall-bouncer-0.0.25/debian/changelog	2023-03-20 23:27:07.000000000 +0100
+++ crowdsec-firewall-bouncer-0.0.25/debian/changelog	2023-05-31 18:57:41.000000000 +0200
@@ -1,3 +1,21 @@
+crowdsec-firewall-bouncer (0.0.25-3) unstable; urgency=medium
+
+  * Fix failure to install if crowdsec is unpacked but not configured
+    (Closes: #1036985):
+     - If both cscli and /etc/crowdsec/config.yaml exist, register as
+       previously, via `cscli bouncers add`.
+     - If cscli exists but /etc/crowdsec/config.yaml doesn't, create an
+       API key similarly to what cscli would do (32 hexadecimal digits),
+       and queue registration in /var/lib/crowdsec/pending-registration.
+  * Add ConditionPathExists=!/var/lib/crowdsec/pending-registration to the
+    systemd unit accordingly, so that it's skipped until crowdsec's
+    postinst deals with the pending-registration file, removes it, and
+    starts the units.
+  * Use mode 600 when creating crowdsec-firewall-bouncer.yaml.local, since
+    it contains an API key.
+
+ -- Cyril Brulebois <cyril@debamax.com>  Wed, 31 May 2023 18:57:41 +0200
+
 crowdsec-firewall-bouncer (0.0.25-2) unstable; urgency=medium
 
   * Stop shipping a logrotate configuration snippet, as the bouncer rotates
diff -Nru crowdsec-firewall-bouncer-0.0.25/debian/crowdsec-firewall-bouncer.postinst crowdsec-firewall-bouncer-0.0.25/debian/crowdsec-firewall-bouncer.postinst
--- crowdsec-firewall-bouncer-0.0.25/debian/crowdsec-firewall-bouncer.postinst	2022-12-15 03:37:41.000000000 +0100
+++ crowdsec-firewall-bouncer-0.0.25/debian/crowdsec-firewall-bouncer.postinst	2023-05-31 16:49:51.000000000 +0200
@@ -2,6 +2,7 @@
 set -e
 
 CONFIG=/etc/crowdsec/bouncers/crowdsec-firewall-bouncer.yaml
+PENDING=/var/lib/crowdsec/pending-registration
 
 # Create a configuration override file only once, see README.Debian:
 if [ "$1" = configure ] && [ ! -f "$CONFIG.local" ]; then
@@ -13,10 +14,21 @@
     # machines; reuse the logic from crowdsec's postinst:
     unique=$(tr -dc 'a-zA-Z0-9' < /dev/urandom | fold -w 32 | head -n 1)
     id="FirewallBouncer-$unique"
-    api_key=$(cscli --error -oraw bouncers add "$id")
-    if [ -z "$api_key" ]; then
-      echo "E: Local registration failed to yield an API key" >&2
-      exit 1
+
+    # crowdsec might be unpacked but not configured (#1036985):
+    if [ -f /etc/crowdsec/config.yaml ]; then
+      api_key=$(cscli --error -oraw bouncers add "$id")
+      if [ -z "$api_key" ]; then
+        echo "E: Local registration failed to yield an API key" >&2
+        exit 1
+      fi
+    else
+      api_key=$(tr -dc 'a-f0-9' < /dev/urandom | fold -w 32 | head -n 1)
+      if [ ! -f $PENDING ]; then
+        touch $PENDING
+        chmod 600 $PENDING
+      fi
+      echo "crowdsec-firewall-bouncer $id $api_key" >> $PENDING
     fi
 
     # Store it so that it can be unregistered when purging the package:
@@ -37,6 +49,9 @@
     firewall=nftables
   fi
 
+  touch "$CONFIG.local"
+  chmod 600 "$CONFIG.local"
+
   # Generate the override:
   if [ -n "$api_key" ]; then
     cat > "$CONFIG.local" <<EOF
diff -Nru crowdsec-firewall-bouncer-0.0.25/debian/patches/0001-adjust-service.patch crowdsec-firewall-bouncer-0.0.25/debian/patches/0001-adjust-service.patch
--- crowdsec-firewall-bouncer-0.0.25/debian/patches/0001-adjust-service.patch	2022-11-30 21:35:51.000000000 +0100
+++ crowdsec-firewall-bouncer-0.0.25/debian/patches/0001-adjust-service.patch	2023-05-31 16:49:51.000000000 +0200
@@ -14,12 +14,13 @@
 
 --- a/config/crowdsec-firewall-bouncer.service
 +++ b/config/crowdsec-firewall-bouncer.service
-@@ -1,12 +1,12 @@
+@@ -1,12 +1,13 @@
  [Unit]
  Description=The firewall bouncer for CrowdSec
 -After=syslog.target network.target remote-fs.target nss-lookup.target crowdsec.service
 +After=network.target remote-fs.target nss-lookup.target crowdsec.service
  Before=netfilter-persistent.service
++ConditionPathExists=!/var/lib/crowdsec/pending-registration
  
  [Service]
  Type=notify

--- End Message ---
--- Begin Message --- 
Hi,

On 31-05-2023 22:38, Cyril Brulebois wrote:

unblock crowdsec/1.4.6-4
unblock crowdsec-custom-bouncer/0.0.15-3
unblock crowdsec-firewall-bouncer/0.0.25-3


unblocked.

Paul

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


--- End Message ---
Reply to: