Merge ~kstenerud/ubuntu/+source/sssd:xenial-sssd-pidfile-1777860 into ubuntu/+source/sssd:ubuntu/xenial-devel

[Karl Stenerud]

Merge Proposal:

The PIDFILE entry in /lib/systemd/system/sssd.service was incorrectly removed. As a result, there's nothing to check if the pidfile is still valid, and sssd will fail to start again after an abnormal termination.

  * d/p/add-back-pidfile.patch: Re-add PIDFILE entry to
    /lib/systemd/system/sssd.service (LP: #1777860)

PPA: ppa:kstenerud/xenial-sssd-pidfile-1777860

Steps to test:

# lxc launch ubuntu-daily:xenial tester
# lxc exec tester bash
# apt update
# apt dist-upgrade -y
# apt install -y sssd
# echo "[nss]
filter_groups = root
filter_users = root
reconnection_retries = 3
[pam]
reconnection_retries = 3
[sssd]
config_file_version = 2
reconnection_retries = 3
sbus_timeout = 30
services = nss, pam
domains = europe.example.com,asia.example.com
[domain/europe.example.com]
#With this as false, a simple "getent passwd" for testing won't work. You must do getent passwd <email address hidden>
enumerate = false
cache_credentials = true
id_provider = ldap
access_provider = ldap
auth_provider = krb5
chpass_provider = krb5
ldap_uri = ldaps://dc1.europe.example.com,ldaps://dc2.europe.example.com
ldap_search_base = dc=europe,dc=example,dc=com
ldap_tls_cacert = /etc/ssl/certs/ca-certificates.crt
#This parameter requires that the DC present a completely validated certificate chain. If you're testing or don't care, use 'allow' or 'never'.
ldap_tls_reqcert = demand
krb5_realm = EUROPE.EXAMPLE.COM
dns_discovery_domain = EUROPE.EXAMPLE.COM
ldap_schema = rfc2307bis
ldap_access_order = expire
ldap_account_expire_policy = ad
ldap_force_upper_case_realm = true
ldap_user_search_base = dc=europe,dc=example,dc=com
ldap_group_search_base = dc=europe,dc=example,dc=com
ldap_user_object_class = user
ldap_user_name = sAMAccountName
ldap_user_fullname = displayName
ldap_user_home_directory = unixHomeDirectory
ldap_user_principal = userPrincipalName
ldap_group_object_class = group
ldap_group_name = sAMAccountName
#Bind credentials
ldap_default_bind_dn = cn=europe-ldap-reader,cn=Users,dc=europe,dc=example,dc=com
ldap_default_authtok = secret
[domain/asia.example.com]
#With this as false, a simple "getent passwd" for testing won't work. You must do getent passwd <email address hidden>
enumerate = false
cache_credentials = true
id_provider = ldap
access_provider = ldap
auth_provider = krb5
chpass_provider = krb5
ldap_uri = ldaps://dc1.asia.example.com,ldaps://dc2.asia.example.com
ldap_search_base = dc=asia,dc=example,dc=com
ldap_tls_cacert = /etc/ssl/certs/ca-certificates.crt
#This parameter requires that the DC present a completely validated certificate chain. If you're testing or don't care, use 'allow' or 'never'.
ldap_tls_reqcert = demand
krb5_realm = ASIA.EXAMPLE.COM
dns_discovery_domain = ASIA.EXAMPLE.COM
ldap_schema = rfc2307bis
ldap_access_order = expire
ldap_account_expire_policy = ad
ldap_force_upper_case_realm = true
ldap_user_search_base = dc=asia,dc=example,dc=com
ldap_group_search_base = dc=asia,dc=example,dc=com
ldap_user_object_class = user
ldap_user_name = sAMAccountName
ldap_user_fullname = displayName
ldap_user_home_directory = unixHomeDirectory
ldap_user_principal = userPrincipalName
ldap_group_object_class = group
ldap_group_name = sAMAccountName
#Bind credentials
ldap_default_bind_dn = cn=asia-ldap-reader,cn=Users,dc=asia,dc=example,dc=com
ldap_default_authtok = secret" >/etc/sssd/sssd.conf
# chmod 600 /etc/sssd/sssd.conf
# service sssd start
# pkill -KILL -F /var/run/sssd.pid
# service sssd start
Job for sssd.service failed because the control process exited with error code. See "systemctl status sssd.service" and "journalctl -xe" for details.
# journalctl -xe
...
Oct 30 10:25:46 xtest sssd[7110]: SSSD is already running

# add-apt-repository -y ppa:kstenerud/xenial-sssd-pidfile-1777860
# apt update
# apt dist-upgrade
# service sssd start

Package Tests:

There are no tests in this package.

[Andreas Hasenack (ahasenack)]

Thanks for this, some changes requested inline.

review: Needs Fixing

[Karl Stenerud (kstenerud)]

Sorry, didn't realize this one-line fix existed upstream already. Fixed.

[Andreas Hasenack (ahasenack)]

One last set of changes :)

review: Needs Fixing

[Karl Stenerud (kstenerud)]

Updated!

[Andreas Hasenack (ahasenack)]

Thanks, +1. Let me know if/when you want sponsoring.

review: Approve

[Karl Stenerud (kstenerud)]

Please sponsor this mp :)

[Andreas Hasenack (ahasenack)]

Tagged and uploaded.

diff --git a/debian/changelog b/debian/changelog
index 3bff41d..4dee60b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+sssd (1.13.4-1ubuntu1.12) xenial; urgency=medium
+
+  * d/p/add-back-pidfile.patch: Re-add PIDFILE entry to
+    /lib/systemd/system/sssd.service (LP: #1777860)
+
+ -- Karl Stenerud <[e-mail]>  Wed, 31 Oct 2018 15:41:19 +0100
+
 sssd (1.13.4-1ubuntu1.11) xenial; urgency=medium
 
   * d/p/fix-ad-passwd-renewal-fd-leak.diff: Fix fd leak triggered by the AD
diff --git a/debian/patches/add-back-pidfile.patch b/debian/patches/add-back-pidfile.patch
new file mode 100644
index 0000000..7206910
--- /dev/null
+++ b/debian/patches/add-back-pidfile.patch
@@ -0,0 +1,26 @@
+Description: SYSTEMD: Clean pid file in corner cases
+ .
+ SSSD can cleanup pid file in case of standard stopping of daemon.
+ It's done in function monitor_cleanup. However monitor does not have a
+ change to cleanup file in case of OOM or sending SIGKILL to monitor.
+ .
+ Even though PIDFile is not necessary for services with Type notify
+ we should let systemd to clean this file in unexpected situations.
+Origin: upstream, https://pagure.io/SSSD/sssd/c/0d34f9df39978a2a2a6fea02b5e2f8db0ce48228
+Bug: https://pagure.io/SSSD/sssd/issue/3528
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/xenial/+source/sssd/+bug/1777860
+Last-Update: 2018-10-31
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
+index 62fdbd5d..e97f15a6 100644
+--- a/src/sysv/systemd/sssd.service.in
++++ b/src/sysv/systemd/sssd.service.in
+@@ -8,6 +8,7 @@ Wants=nss-user-lookup.target
+ ExecStart=@sbindir@/sssd -i -f
+ Type=notify
+ NotifyAccess=main
++PIDFile=@localstatedir@/run/sssd.pid
+ 
+ [Install]
+ WantedBy=multi-user.target
diff --git a/debian/patches/series b/debian/patches/series
index c9f128d..99ab3a0 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,4 @@ attempt_ptr_update_on_nonzero_return.diff
 bad-initgroups-results-3045.patch
 CVE-2017-12173.patch
 fix-ad-passwd-renewal-fd-leak.diff
+add-back-pidfile.patch