tag 786555 + patch
tag 870456 + patch
thanks

Hi,

Le 17/07/17 à 12:38, Laurent Bigonville a écrit :
Le 17/07/17 à 11:11, Laurent Bigonville a écrit :
Le 17/07/17 à 01:59, Michael Biebl a écrit :
Am 17.07.2017 um 01:46 schrieb Bdale Garbee:

Sure, sounds good.  I'm personally ambivalent about selinux since I
don't use it, but I'm always in favor of making things work for as many
users in as many contexts as possible.
bigon, is there a way sudo could do the selinux relabeling itself when
creating /run/sudo/ts so we don't need an init script/tmpfile to do
that? That would be a more elegant solution. I see sudo does already
link againt libselinux. So having that dependency is not a concern.

There are multiple ways of doing that:

 1. via policy: this should be already the case for confined users,
    but apparently not for unconfined ones. This might be a bug in
    the policy, I'll poke upstream about that.
 2. via an explicit call to libselinux: call selabel_lookup_raw() to
    retrieve which context should be used and then call
    setfscreatecon_raw() so the next file create will atomically have
    the correct context. sudo is already selinux-aware so that could
    be a solution as well.
 3. Pre-create the directory during the startup with the correct context.

The easiest being of course 3)

Apparently the sudo tarball already contains a tmpfile snippet (init.d/sudo.conf.in) that is already shipped in the package. So that solves the systemd case.

For the !systemd case, the only thing that needs to be added in the initscript to replicate the tmpfiles snippet is something like:
mkdir /run/sudo /run/sudo/ts
chmod 0711 /run/sudo
chmod 0700 /run/sudo/ts
[ -x /sbin/restorecon ] && /sbin/restorecon /run/sudo /run/sudo/ts
I wouldn't recursively relabel the files in there to avoid potentially breaking things for confined users.

I've attached here a series of patch that should fix this bug and #870456 as well.

The patch are NOT migrating the timestamp files though
>From e88e77ce2eb30bc0708658cbd831792e8a653af5 Mon Sep 17 00:00:00 2001
From: Laurent Bigonville <bi...@debian.org>
Date: Wed, 2 Aug 2017 10:55:37 +0200
Subject: [PATCH 3/3] debian/sudo*.postinst: Move the debhelper marker before
 the creation of the sudo group, this way the snippets added by debhelper will
 be executed even if the group already exists. (Closes: #870456)

---
 debian/changelog          | 5 ++++-
 debian/sudo-ldap.postinst | 2 ++
 debian/sudo.postinst      | 4 ++--
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 3bdc097..cd344e0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -5,8 +5,11 @@ sudo (1.8.20p2-2) UNRELEASED; urgency=medium
   * Move timestamp files to /run/sudo, with systemd the directory is
     created/cleaned by tmpfiles.d now, the sudo initscript/service is not
     doing anything in that case anymore (Closes: #786555)
+  * debian/sudo*.postinst: Move the debhelper marker before the creation of
+    the sudo group, this way the snippets added by debhelper will be executed
+    even if the group already exists. (Closes: #870456)
 
- -- Laurent Bigonville <bi...@debian.org>  Wed, 02 Aug 2017 10:31:39 +0200
+ -- Laurent Bigonville <bi...@debian.org>  Wed, 02 Aug 2017 10:53:39 +0200
 
 sudo (1.8.20p2-1) unstable; urgency=medium
 
diff --git a/debian/sudo-ldap.postinst b/debian/sudo-ldap.postinst
index b27a2c1..bb2b206 100644
--- a/debian/sudo-ldap.postinst
+++ b/debian/sudo-ldap.postinst
@@ -34,6 +34,8 @@ fi
 # if we've gotten this far .. remove the saved, unchanged old sudoers file
 rm -f /etc/sudoers.pre-conffile
 
+#DEBHELPER#
+
 # make sure we have a sudo group
 
 [ -n "`getent group sudo`" ] && exit 0   # we're finished if there is a group sudo:
diff --git a/debian/sudo.postinst b/debian/sudo.postinst
index c49d096..a29c2c7 100644
--- a/debian/sudo.postinst
+++ b/debian/sudo.postinst
@@ -22,6 +22,8 @@ fi
 # if we've gotten this far .. remove the saved, unchanged old sudoers file
 rm -f /etc/sudoers.pre-conffile
 
+#DEBHELPER#
+
 # make sure we have a sudo group
 
 [ -n "`getent group sudo`" ] && exit 0   # we're finished if there is a group sudo:
@@ -55,5 +57,3 @@ echo "Creating group 'sudo' with gid = $gid";
 groupadd -g $gid sudo
 
 echo ""
-
-#DEBHELPER#
-- 
2.13.3

>From 18d312d2241bb4ac6aac68f71356fd4a4954ce18 Mon Sep 17 00:00:00 2001
From: Laurent Bigonville <bi...@debian.org>
Date: Wed, 2 Aug 2017 10:33:26 +0200
Subject: [PATCH 2/3] Move timestamp files to /run/sudo, with systemd the
 directory is created/cleaned by tmpfiles.d now, the sudo initscript/service
 is not doing anything in that case anymore (Closes: #786555)

---
 debian/changelog                |  5 ++++-
 debian/rules                    | 19 ++++++++++++-------
 debian/sudo-ldap.postrm         |  1 +
 debian/sudo-ldap.sudo-ldap.init | 12 ++++++++++--
 debian/sudo.postrm              |  1 +
 debian/sudo.service             | 10 ----------
 debian/sudo.sudo.init           | 12 ++++++++++--
 7 files changed, 38 insertions(+), 22 deletions(-)
 delete mode 100644 debian/sudo.service

diff --git a/debian/changelog b/debian/changelog
index 8587993..3bdc097 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,8 +2,11 @@ sudo (1.8.20p2-2) UNRELEASED; urgency=medium
 
   * debian/sudo*.postinst: Drop /var/run/sudo -> /var/lib/sudo migration code,
     this migration happend in 2010 and that code is not necessary anymore
+  * Move timestamp files to /run/sudo, with systemd the directory is
+    created/cleaned by tmpfiles.d now, the sudo initscript/service is not
+    doing anything in that case anymore (Closes: #786555)
 
- -- Laurent Bigonville <bi...@debian.org>  Wed, 02 Aug 2017 10:21:01 +0200
+ -- Laurent Bigonville <bi...@debian.org>  Wed, 02 Aug 2017 10:31:39 +0200
 
 sudo (1.8.20p2-1) unstable; urgency=medium
 
diff --git a/debian/rules b/debian/rules
index 8cc7acf..17a4c6e 100755
--- a/debian/rules
+++ b/debian/rules
@@ -11,7 +11,7 @@ CPPFLAGS = `dpkg-buildflags --get CPPFLAGS`
 DEB_HOST_MULTIARCH ?= $(shell dpkg-architecture -qDEB_HOST_MULTIARCH)
 DEB_HOST_ARCH_OS ?= $(shell dpkg-architecture -qDEB_HOST_ARCH_OS)
 ifeq ($(DEB_HOST_ARCH_OS),linux)
-  configure_args += --with-selinux --with-linux-audit
+  configure_args += --with-selinux --with-linux-audit --enable-tmpfiles.d=yes
 endif
 
 reconf-stamp:
@@ -44,7 +44,7 @@ configure-stamp: reconf-stamp
 		--with-passprompt="[sudo] password for %p: " \
 		--disable-root-mailer \
 		--with-sendmail=/usr/sbin/sendmail \
-		--with-rundir=/var/lib/sudo \
+		--with-rundir=/run/sudo \
 		--libexecdir=/usr/lib/sudo \
 		--with-sssd --with-sssd-lib=/usr/lib/$(DEB_HOST_MULTIARCH) \
 		$(configure_args)
@@ -69,7 +69,7 @@ configure-stamp: reconf-stamp
 		--disable-root-mailer \
 		--disable-setresuid \
 		--with-sendmail=/usr/sbin/sendmail \
-		--with-rundir=/var/lib/sudo \
+		--with-rundir=/run/sudo \
 		--with-ldap-conf-file=/etc/sudo-ldap.conf \
 		--libexecdir=/usr/lib/sudo \
 		$(configure_args)
@@ -113,6 +113,10 @@ install: build-stamp
 		debian/sudo*/usr/share/doc/sudo/LICENSE* \
 		debian/sudo*/usr/share/doc/sudo/ChangeLog
 
+	# /run/sudo directory is created at boot time and shouldn't be in the
+	# package
+	rm -rf debian/sudo*/run
+
 	# move upstream-installed docs to the right place for ldap package
 	mv	debian/sudo-ldap/usr/share/doc/sudo/* \
 		debian/sudo-ldap/usr/share/doc/sudo-ldap/
@@ -139,10 +143,11 @@ install: build-stamp
 	install -o root -g root -m 0440 debian/README \
 		debian/sudo-ldap/etc/sudoers.d/README
 
-	install -o root -g root -m 0644 debian/sudo.service \
-		debian/sudo/lib/systemd/system/sudo.service
-	install -o root -g root -m 0644 debian/sudo.service \
-		debian/sudo-ldap/lib/systemd/system/sudo.service
+	# we don't want the initscript to run, the creation of the rundir and
+	# the cleanup the stamp files is now done by tmpfiles when using
+	# systemd
+	ln -s /dev/null debian/sudo/lib/systemd/system/sudo.service
+	ln -s /dev/null debian/sudo-ldap/lib/systemd/system/sudo.service
 
 binary-indep: build install
 
diff --git a/debian/sudo-ldap.postrm b/debian/sudo-ldap.postrm
index 246f99d..c3b48c8 100644
--- a/debian/sudo-ldap.postrm
+++ b/debian/sudo-ldap.postrm
@@ -4,6 +4,7 @@ case "$1" in
   purge)
 	rm -f /etc/sudo-ldap.conf
 	rm -rf /var/lib/sudo
+	rm -rf /run/sudo
   ;;
 
   remove|upgrade|deconfigure)
diff --git a/debian/sudo-ldap.sudo-ldap.init b/debian/sudo-ldap.sudo-ldap.init
index 1726dae..5080db8 100644
--- a/debian/sudo-ldap.sudo-ldap.init
+++ b/debian/sudo-ldap.sudo-ldap.init
@@ -20,9 +20,17 @@ set -e
 case "$1" in
   start)
 	# make sure privileges don't persist across reboots
-	if [ -d /var/lib/sudo ]
+	# if the /run/sudo directory doesn't exist, let's create it with the
+	# correct permissions and SELinux label
+	if [ -d /run/sudo ]
 	then
-                find /var/lib/sudo -exec touch -d @0 '{}' \;
+                find /run/sudo -exec touch -d @0 '{}' \;
+	else
+		mkdir /run/sudo /run/sudo/ts
+		chown root:root /run/sudo /run/sudo/ts
+		chmod 0711 /run/sudo
+		chmod 0700 /run/sudo/ts
+		[ -x /sbin/restorecon ] && /sbin/restorecon /run/sudo /run/sudo/ts
 	fi
 	;;
   stop|reload|restart|force-reload|status)
diff --git a/debian/sudo.postrm b/debian/sudo.postrm
index ab1425a..f683170 100644
--- a/debian/sudo.postrm
+++ b/debian/sudo.postrm
@@ -3,6 +3,7 @@
 case "$1" in
   purge)
 	rm -rf /var/lib/sudo
+	rm -rf /run/sudo
   ;;
 
   remove|upgrade|deconfigure)
diff --git a/debian/sudo.service b/debian/sudo.service
deleted file mode 100644
index a8c5460..0000000
--- a/debian/sudo.service
+++ /dev/null
@@ -1,10 +0,0 @@
-[Unit]
-Description=Provide limited super user privileges to specific users
-
-[Service]
-Type=oneshot
-# \073 is ';' which needs to be part of the find parameters
-ExecStart=/usr/bin/find /var/lib/sudo -exec /usr/bin/touch -d @0 '{}' \073
-
-[Install]
-WantedBy=multi-user.target
diff --git a/debian/sudo.sudo.init b/debian/sudo.sudo.init
index 0f01b97..c971310 100644
--- a/debian/sudo.sudo.init
+++ b/debian/sudo.sudo.init
@@ -20,9 +20,17 @@ set -e
 case "$1" in
   start)
 	# make sure privileges don't persist across reboots
-	if [ -d /var/lib/sudo ]
+	# if the /run/sudo directory doesn't exist, let's create it with the
+	# correct permissions and SELinux label
+	if [ -d /run/sudo ]
 	then
-                find /var/lib/sudo -exec touch -d @0 '{}' \;
+                find /run/sudo -exec touch -d @0 '{}' \;
+	else
+		mkdir /run/sudo /run/sudo/ts
+		chown root:root /run/sudo /run/sudo/ts
+		chmod 0711 /run/sudo
+		chmod 0700 /run/sudo/ts
+		[ -x /sbin/restorecon ] && /sbin/restorecon /run/sudo /run/sudo/ts
 	fi
 	;;
   stop|reload|restart|force-reload|status)
-- 
2.13.3

>From 1f2ed5b979eb77928c9fde402164f238851b71ee Mon Sep 17 00:00:00 2001
From: Laurent Bigonville <bi...@debian.org>
Date: Wed, 2 Aug 2017 10:22:21 +0200
Subject: [PATCH 1/3] debian/sudo*.postinst: Drop /var/run/sudo ->
 /var/lib/sudo migration code, this migration happend in 2010 and that code is
 not necessary anymore

---
 debian/changelog          | 7 +++++++
 debian/sudo-ldap.postinst | 8 --------
 debian/sudo.postinst      | 8 --------
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index f49679f..8587993 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+sudo (1.8.20p2-2) UNRELEASED; urgency=medium
+
+  * debian/sudo*.postinst: Drop /var/run/sudo -> /var/lib/sudo migration code,
+    this migration happend in 2010 and that code is not necessary anymore
+
+ -- Laurent Bigonville <bi...@debian.org>  Wed, 02 Aug 2017 10:21:01 +0200
+
 sudo (1.8.20p2-1) unstable; urgency=medium
 
   * new upstream version
diff --git a/debian/sudo-ldap.postinst b/debian/sudo-ldap.postinst
index be913a1..b27a2c1 100644
--- a/debian/sudo-ldap.postinst
+++ b/debian/sudo-ldap.postinst
@@ -19,14 +19,6 @@ then
 	echo "sudoers:	files ldap" >> /etc/nsswitch.conf
 fi
 
-# handle state directory transition from /var/run/sudo to /var/lib/sudo,
-# moving any existing content over to avoid re-lecturing existing users
-if [ -d "/var/run/sudo" ];then
-    mkdir -p /var/lib/sudo
-    (cd /var/run/sudo ; tar cf - .) | (cd /var/lib/sudo ; tar xf -)
-    rm -rf /var/run/sudo
-fi
-
 # make sure sudoers has the correct permissions and owner/group
 if [ -f /etc/sudoers ];then
     chown root:root /etc/sudoers
diff --git a/debian/sudo.postinst b/debian/sudo.postinst
index 7f94155..c49d096 100644
--- a/debian/sudo.postinst
+++ b/debian/sudo.postinst
@@ -13,14 +13,6 @@ if [ ! -f /etc/sudoers ];then
 	echo "WARNING:  /etc/sudoers not present!";
 fi
 
-# handle state directory transition from /var/run/sudo to /var/lib/sudo,
-# moving any existing content over to avoid re-lecturing existing users
-if [ -d "/var/run/sudo" ];then
-    mkdir -p /var/lib/sudo
-    (cd /var/run/sudo ; tar cf - .) | (cd /var/lib/sudo ; tar xf -)
-    rm -rf /var/run/sudo
-fi
-
 # make sure sudoers has the correct permissions and owner/group
 if [ -f /etc/sudoers ];then
     chown root:root /etc/sudoers
-- 
2.13.3

Reply via email to