p11: allow p11_child to run completely unprivileged

To only operation of p11_child which requires special privileges is the
communication to pcscd which handles the Smartcard access. pcscd uses
policy-kit for access control so access can easily be configured by
dropping config snippets into the right directory.

If SSSD is configured to run as un-privileged user this patch creates
the needed config snippet for policy-kit and installs it in a suitable
directory. As a result p11_child does not have to be installed with
SETUID or SETGID bits set.

Resolves https://fedorahosted.org/sssd/ticket/2755 by making it obsolete

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

5 files changed, 38 insertions, 26 deletions

diff --git a/Makefile.am b/Makefile.am
index a0abb8fb3..212440c9b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -150,6 +150,10 @@ sssdlibexec_PROGRAMS += selinux_child
 endif
 if HAVE_NSS
 sssdlibexec_PROGRAMS += p11_child
+if SSSD_USER
+polkit_rulesdir = $(datadir)/polkit-1/rules.d
+dist_polkit_rules_DATA = contrib/sssd-pcsc.rules
+endif
 endif
 
 if BUILD_PAC_RESPONDER
@@ -3565,10 +3569,6 @@ if BUILD_SEMANAGE
 	-chgrp $(SSSD_USER) $(DESTDIR)$(sssdlibexecdir)/selinux_child
 	chmod 4750 $(DESTDIR)$(sssdlibexecdir)/selinux_child
 endif
-if HAVE_NSS
-	-chgrp $(SSSD_USER) $(DESTDIR)$(sssdlibexecdir)/p11_child
-	chmod 4750 $(DESTDIR)$(sssdlibexecdir)/p11_child
-endif
 endif
 
 install-data-hook:
diff --git a/configure.ac b/configure.ac
index 3fe824224..f7254c096 100644
--- a/configure.ac
+++ b/configure.ac
@@ -432,6 +432,7 @@ my_srcdir=`readlink -f $srcdir`
 AC_DEFINE_UNQUOTED([ABS_SRC_DIR], ["$my_srcdir"], [Absolute path to the source directory])
 
 AC_CONFIG_FILES([Makefile contrib/sssd.spec src/examples/rwtab src/doxy.config
+                 contrib/sssd-pcsc.rules
                  src/sysv/sssd src/sysv/gentoo/sssd src/sysv/SUSE/sssd
                  po/Makefile.in src/man/Makefile src/tests/cwrap/Makefile
                  src/tests/intg/Makefile
diff --git a/contrib/sssd-pcsc.rules.in b/contrib/sssd-pcsc.rules.in
new file mode 100644
index 000000000..31d2dbe4f
--- /dev/null
+++ b/contrib/sssd-pcsc.rules.in
@@ -0,0 +1,15 @@
+// Please put this file in /usr/share/polkit-1/rules.d/ if SSSD is running as
+// unprivileged user '@SSSD_USER@' to allow access to the Smartcard via pcscd.
+polkit.addRule(function(action, subject) {
+    if (action.id == "org.debian.pcsc-lite.access_card" &&
+        subject.user == "@SSSD_USER@") {
+            return polkit.Result.YES;
+    }
+});
+
+polkit.addRule(function(action, subject) {
+    if (action.id == "org.debian.pcsc-lite.access_pcsc" &&
+        subject.user == "@SSSD_USER@") {
+            return polkit.Result.YES;
+    }
+});
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index e0367e460..cff77b29e 100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -19,6 +19,12 @@
     %global use_systemd 1
 %endif
 
+# on Fedora and RHEL7 p11_child needs a polkit config snippet to be allowed to
+# talk to pcscd if SSSD runs as unpriviledged user
+%if (0%{?fedora} || 0%{?rhel} >= 7)
+    %global install_pcscd_polkit_rule 1
+%endif
+
 %if (0%{?use_systemd} == 1)
     %global with_initscript --with-initscript=systemd --with-systemdunitdir=%{_unitdir}
     %global with_syslog --with-syslog=journald
@@ -559,6 +565,7 @@ autoreconf -ivf
     --enable-nfsidmaplibdir=%{_libdir}/libnfsidmap \
     --disable-static \
     --disable-rpath \
+    --with-sssd-user=sssd \
     %{with_initscript} \
     %{?with_syslog} \
     %{?with_cifs_utils_plugin_option} \
@@ -684,7 +691,11 @@ rm -rf $RPM_BUILD_ROOT
 %{_libexecdir}/%{servicename}/sssd_autofs
 %{_libexecdir}/%{servicename}/sssd_ssh
 %{_libexecdir}/%{servicename}/sssd_sudo
-%attr(4750,root,sssd) %{_libexecdir}/%{servicename}/p11_child
+%{_libexecdir}/%{servicename}/p11_child
+
+%if (0%{?install_pcscd_polkit_rule} == 1)
+%{_datadir}/polkit-1/rules.d/*
+%endif
 
 %dir %{_libdir}/%{name}
 %{_libdir}/%{name}/libsss_simple.so
diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c
index 8a383a044..41d9fd11f 100644
--- a/src/p11_child/p11_child_nss.c
+++ b/src/p11_child/p11_child_nss.c
@@ -482,10 +482,13 @@ int main(int argc, const char *argv[])
     debug_level = SSSDBG_INVALID;
 
     /*
-     * This child runs as root (setuid(0)), so we need clear environment and
-     * set permissions for security reasons.
+     * This child can run as root or as sssd user relying on policy kit to
+     * grant access to pcscd. This means that no setuid or setgid bit must be
+     * set on the binary. We still should make sure to run with a restrictive
+     * umask but do not have to make additional precautions like clearing the
+     * environment. This would allow to use e.g. pkcs11-spy.so for further
+     * debugging.
      */
-    clearenv();
     umask(SSS_DFL_UMASK);
 
     pc = poptGetContext(argv[0], argc, argv, long_options, 0);
@@ -582,24 +585,6 @@ int main(int argc, const char *argv[])
           "Running with effective IDs: [%"SPRIuid"][%"SPRIgid"].\n",
           geteuid(), getegid());
 
-    if (getuid() != 0) {
-        ret = setuid(0);
-        if (ret == -1) {
-            ret = errno;
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "setuid failed: %d, p11_child might not work!\n", ret);
-        }
-    }
-
-    if (getgid() != 0) {
-        ret = setgid(0);
-        if (ret == -1) {
-            ret = errno;
-            DEBUG(SSSDBG_CRIT_FAILURE,
-                  "setgid failed: %d, p11_child might not work!\n", ret);
-        }
-    }
-
     DEBUG(SSSDBG_TRACE_INTERNAL,
           "Running with real IDs [%"SPRIuid"][%"SPRIgid"].\n",
           getuid(), getgid());