Merge ~ddstreet/ubuntu/+source/systemd:revert-sysctl-conf-patch into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-impish

[Dan Streetman]

[Lukas Märdian (slyon)]

Hi Dan! In general this looks good to me and I favor removing delta as much as possible.

Checking the history of this patch, it was introduced in 2017 to set the *.promote_secondaries=1 and *.default_qdisc=fq_codel values, which are the defaults today. (LP: #1721223) – So it's fine keeping those upstream values IMO.

It was then modified in 2019 to explicitly drop upstream's default fs.protected_regular=1 and fs.protected_fifos=1 values. (LP: #1845637) – Checking a current Ubuntu Hirsute system shows:

$ sudo sysctl fs.protected_regular
fs.protected_regular = 2
$ sudo sysctl fs.protected_fifos
fs.protected_fifos = 1

Who sets the fs.protected_regular=2 value (is it the kernel?) – Would upstream systemd's default of fs.protected_regular=1 override this current value, and thus degrate security?

What are your thoughts on this?

[Dan Streetman (ddstreet)]

> Who sets the fs.protected_regular=2 value

that was set by the procps package, in this commit:
https://salsa.debian.org/debian/procps/-/commit/299f4a1a10810e2995e666374b880b543af8e8e4

for debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=914859

I removed that file from Ubuntu via bug:
https://bugs.launchpad.net/ubuntu/+source/procps/+bug/1938585

As you can see from the Debian bug, there's no reasoning for setting the default to 2 other than 'probably better', but only protected_regular is increased to 2. I don't have any objection to increasing the protection, but it should happen in upstream systemd, or at least we should have specific reasoning to adjust it in Ubuntu if upstream has reasoning to not increase it (after someone proposes it upstream).

> (is it the kernel?)

the kernel defaults to 0 for both protected_regular and protected_fifo.

[Lukas Märdian (slyon)]

Thank you Dan for providing those references and background information!

I agree and think we should put the upstream defaults back in place, then.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 912567b..fde93c5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -16,6 +16,9 @@ systemd (248.3-1ubuntu4) UNRELEASED; urgency=medium
     Fix sleep verb used by logind during suspend-then-hibernate
     (LP: #1934981)
     https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=b09083d520fecd57ebe28319ad7638442db2e2bd
+  * d/p/debian/UBUNTU-drop-kernel.-settings-from-sysctl-defaults-shipped.patch:
+    Revert patch, no reason to diverge from upstream on these sysctl settings
+    https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=8959a2640c631f026988f1fef6b1a825af6c97dd
 
  -- Dan Streetman <[e-mail]>  Tue, 20 Jul 2021 16:53:38 -0400
 
diff --git a/debian/patches/debian/UBUNTU-drop-kernel.-settings-from-sysctl-defaults-shipped.patch b/debian/patches/debian/UBUNTU-drop-kernel.-settings-from-sysctl-defaults-shipped.patch
 deleted file mode 100644
index 1284009..0000000
--- a/debian/patches/debian/UBUNTU-drop-kernel.-settings-from-sysctl-defaults-shipped.patch
+++ /dev/null
@@ -1,51 +0,0 @@
-From: Dimitri John Ledkov <[e-mail]>
-Date: Wed, 11 Oct 2017 12:17:03 +0100
-Subject: UBUNTU: drop unrelated settings from sysctl defaults shipped by
- systemd.
-
----
- sysctl.d/50-default.conf | 24 ------------------------
- 1 file changed, 24 deletions(-)
-
-diff --git a/sysctl.d/50-default.conf b/sysctl.d/50-default.conf
-index f41e24b..87e95d4 100644
---- a/sysctl.d/50-default.conf
-+++ b/sysctl.d/50-default.conf
-@@ -11,26 +11,6 @@
- # (e.g. /etc/sysctl.d/90-override.conf), and put any assignments
- # there.
- 
--# System Request functionality of the kernel (SYNC)
--#
--# Use kernel.sysrq = 1 to allow all keys.
--# See https://www.kernel.org/doc/html/latest/admin-guide/sysrq.html for a list
--# of values and keys.
--kernel.sysrq = 16
--
--# Append the PID to the core filename
--kernel.core_uses_pid = 1
--
--# Source route verification
--net.ipv4.conf.default.rp_filter = 2
--net.ipv4.conf.*.rp_filter = 2
---net.ipv4.conf.all.rp_filter
--
--# Do not accept source routing
--net.ipv4.conf.default.accept_source_route = 0
--net.ipv4.conf.*.accept_source_route = 0
---net.ipv4.conf.all.accept_source_route
--
- # Promote secondary addresses when the primary address is removed
- net.ipv4.conf.default.promote_secondaries = 1
- net.ipv4.conf.*.promote_secondaries = 1
-@@ -47,10 +27,6 @@ net.ipv4.conf.*.promote_secondaries = 1
- # Fair Queue CoDel packet scheduler to fight bufferbloat
- -net.core.default_qdisc = fq_codel
- 
--# Enable hard and soft link protection
--fs.protected_hardlinks = 1
--fs.protected_symlinks = 1
--
- # Enable regular file and FIFO protection
- fs.protected_regular = 1
- fs.protected_fifos = 1
diff --git a/debian/patches/series b/debian/patches/series
index c0a06a3..b0c5483 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -27,7 +27,6 @@ debian/Revert-pager-stop-disabling-urlification-under-a-pager.patch
 debian/Ubuntu-UseDomains-by-default.patch
 debian/Ubuntu-core-in-execute-soft-fail-setting-Nice-priority-when.patch
 debian/Ubuntu-units-set-ConditionVirtualization-private-users-on-j.patch
-debian/UBUNTU-drop-kernel.-settings-from-sysctl-defaults-shipped.patch
 debian/UBUNTU-Add-AssumedApparmorLabel-unconfined-to-timedate1-dbus.patch
 debian/UBUNTU-test-test-functions-launch-qemu-with-vga-none.patch
 debian/UBUNTU-wait-online-exit-if-no-links-are-managed.patch