Merge ~sergiodj/ubuntu/+source/sssd:bug1900642-condpathexists-focal into ubuntu/+source/sssd:ubuntu/focal-devel

[Sergio Durigan Junior]

This is a MP to address bug 1900642. It includes a patch that comes directly from upstream, which accepted my solution (after some tweaking).

In a nutshell, what the new patch does is to include some conditions in the sssd.service file that are responsible for checking whether the user as (a) a file named /etc/sssd/sssd.conf, or (b) some configuration snippet under /etc/sssd/conf.d/. If either is true, then the service can be started.

It's important to mention that these conditions will only be added to the service file if sssd has been compiled without --enable-files-domain support. This is true for the Debian/Ubuntu sssd packages.

The good thing is that the SRU template can be left unmodified.

autopkgtest is still happy:

autopkgtest [17:03:52]: @@@@@@@@@@@@@@@@@@@@ summary
ldap-user-group-ldap-auth PASS
ldap-user-group-krb5-auth PASS

[Bryce Harrington (bryce)]

Same as the groovy MP, approved for same reason and same condition that a fix for hirsute is likewise needed before this can be accepted for SRU.

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks for the review, Bryce.

I've just created an MP for hirsute, and tagged you as a reviewer:

https://code.launchpad.net/~sergiodj/ubuntu/+source/sssd/+git/sssd/+merge/396206

If you can, please have a look :-). Thanks!

[Sergio Durigan Junior (sergiodj)]

$ git push pkg upload/2.2.3-3ubuntu0.2
Enumerating objects: 16, done.
Counting objects: 100% (16/16), done.
Delta compression using up to 8 threads
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 2.86 KiB | 418.00 KiB/s, done.
Total 11 (delta 7), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/sssd
 * [new tag] upload/2.2.3-3ubuntu0.2 -> upload/2.2.3-3ubuntu0.2

$ dput sssd_2.2.3-3ubuntu0.2_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/sssd/sssd-focal/sssd_2.2.3-3ubuntu0.2_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/sssd/sssd-focal/sssd_2.2.3-3ubuntu0.2.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sssd_2.2.3-3ubuntu0.2.dsc: done.
  Uploading sssd_2.2.3-3ubuntu0.2.diff.gz: done.
  Uploading sssd_2.2.3-3ubuntu0.2_source.buildinfo: done.
  Uploading sssd_2.2.3-3ubuntu0.2_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 21cd451..b6366f5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+sssd (2.2.3-3ubuntu0.2) focal; urgency=medium
+
+  * d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch:
+    Upstream patch to make sssd.service only able to start when there
+    is a configuration file present.  (LP: #1900642)
+
+ -- Sergio Durigan Junior <[e-mail]>  Mon, 11 Jan 2021 14:33:54 -0500
+
 sssd (2.2.3-3ubuntu0.1) focal; urgency=medium
 
   * Enable support for "ad_use_ldaps" for new Active Directory 
diff --git a/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch b/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch
 new file mode 100644
index 0000000..622ddc3
--- /dev/null
+++ b/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch
@@ -0,0 +1,86 @@
+From: Sergio Durigan Junior <[e-mail]>
+Date: Wed, 9 Dec 2020 22:54:21 -0500
+Subject: Only start sssd.service if there's a configuration file present
+
+This commit is the follow-up of the discussion that is happening here:
+
+https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
+
+In a nutshell, SSSD is compile with --disable-files-domain and
+installed without a configuration file by default, which means that
+it's impossible to start it successfully unless the user has actively
+created/copied a sssd.conf inside /etc/sssd.
+
+There are two possible ways to have sssd.service successfully start:
+
+1) If SSSD is configured with --enable-files-domain, then no
+   configuration file is required, and the service can start normally.
+
+2) If SSSD is configured with --disable-files-domain, then a
+   configuration file is required.  This can be either
+   /etc/sssd/sssd.conf, or a snippet under /etc/sssd/conf.d/.
+
+For this reason, I'd like to suggest that we conditionally add the
+following lines to sssd.service:
+
+  ConditionPathExists=|/etc/sssd/sssd.conf
+  ConditionDirectoryNotEmpty=|/etc/sssd/conf.d/
+
+These lines will be added only if SSSD is not configured with
+--enable-files-domain.
+
+Signed-off-by: Sergio Durigan Junior <[e-mail]>
+
+Reviewed-by: Alexey Tikhonov <[e-mail]>
+
+Author: Sergio Durigan Junior <[e-mail]>
+Origin: upstream, https://github.com/SSSD/sssd/commit/a25256fe22dd0b976093d15a5c7c73e1dc64bbcc
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
+Last-Updated: 2021-01-11
+---
+ Makefile.am                      | 12 +++++++++++-
+ src/sysv/systemd/sssd.service.in |  1 +
+ 2 files changed, 12 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index 4bacabd..1e5c0e8 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -95,6 +95,15 @@ if HAVE_SYSTEMD_UNIT
+ ifp_exec_cmd = $(sssdlibexecdir)/sssd_ifp --uid 0 --gid 0 --dbus-activated
+ ifp_systemdservice = SystemdService=sssd-ifp.service
+ ifp_restart = Restart=on-failure
++# If sssd is configured with --enable-files-domain, the service is
++# able to start even without a configuration file.  Otherwise, sssd
++# requires a configuration file (either /etc/sssd/sssd.conf, or some
++# snippet under /etc/sssd/sssd.conf.d/) to be present.
++if ADD_FILES_DOMAIN
++condconfigexists =
++else
++condconfigexists = ConditionPathExists=\|/etc/sssd/sssd.conf\nConditionDirectoryNotEmpty=\|/etc/sssd/conf.d/
++endif
+ else
+ ifp_exec_cmd = $(sssdlibexecdir)/sss_signal
+ ifp_systemdservice =
+@@ -5128,7 +5137,8 @@ edit_cmd = $(SED) \
+         -e 's|@libexecdir[@]|$(libexecdir)|g' \
+         -e 's|@pipepath[@]|$(pipepath)|g' \
+         -e 's|@prefix[@]|$(prefix)|g' \
+-        -e 's|@SSSD_USER[@]|$(SSSD_USER)|g'
++        -e 's|@SSSD_USER[@]|$(SSSD_USER)|g' \
++        -e 's|@condconfigexists[@]|$(condconfigexists)|g'
+ 
+ replace_script = \
+     @rm -f $@ $@.tmp; \
+diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
+index 7a4b7c7..aae36f7 100644
+--- a/src/sysv/systemd/sssd.service.in
++++ b/src/sysv/systemd/sssd.service.in
+@@ -3,6 +3,7 @@ Description=System Security Services Daemon
+ # SSSD must be running before we permit user sessions
+ Before=systemd-user-sessions.service nss-user-lookup.target
+ Wants=nss-user-lookup.target
++@condconfigexists@
+ 
+ [Service]
+ Environment=DEBUG_LOGGER=--logger=files
diff --git a/debian/patches/series b/debian/patches/series
index 4b402cf..2ec352f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -6,3 +6,4 @@ lp-1868703-01-ad-allow-booleans-for-ad_inherit_opts_if_needed.patch
 lp-1868703-02-ad-add-ad_use_ldaps.patch
 lp-1868703-03-ldap-add-new-option-ldap_sasl_maxssf.patch
 lp-1868703-04-ad-set-min-and-max-ssf-for-ldaps.patch
+0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch