[SSSD] [PATCH] p11: allow p11_child to run completely unprivileged

Sumit Bose sbose at redhat.com
Tue Nov 10 11:03:56 UTC 2015


Hi,

this patch removes the requirement to install p11_child with SETUID or
SETGID bit set. The needed privileges can be tuned with the help of
policy-kit so p11_child can either run as root or as SSSD user depending
on the SSSD configuration without the need to gain extra user or group
privileges.

With this patch the needed policy-kit configuration snippet is created
if the --with-sssd-user configure option is used. Since the contributed
spec file implicitly assumes that the SSSD user is 'sssd' I added the
option here. I added 'Requires: polkit' as well to make sure
%{_datadir}/polkit-1/rules.d/ exists. This might not be the best
solution for minimal installations where e.g. only the SSSD LDAP
provider is needed. But distribution can modify it to suit their needs.
I did it this way because I'm not sure if all distribution allow
directories to be owned by multiple packages. If someone can confirm
that this is allowed by the majority of the distribution I'd be happy to
modify the patch so that the directory is created and owned by the SSSD
package and drop the polkit requirement.

Since dropping the SETUID and SETGID bits improves the general security
it would be nice if this patch can be include in 1.13.x so the
distributions can pick it and improve already released packages.

Unfortunately I think to test the functionality, i.e. access a Smartcard
with p11_child as an unprivileged user, real hardware is needed. If you
want to test it, please ping me, maybe we can arrange something.

bye,
Sumit
-------------- next part --------------
From 38fb2635aaec4dca335227d532965dfbadbca2ef Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 30 Oct 2015 16:29:31 +0100
Subject: [PATCH] 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
---
 Makefile.am                   |  8 ++++----
 configure.ac                  |  1 +
 contrib/sssd-pcsc.rules.in    | 15 +++++++++++++++
 contrib/sssd.spec.in          |  6 +++++-
 src/p11_child/p11_child_nss.c | 27 ++++++---------------------
 5 files changed, 31 insertions(+), 26 deletions(-)
 create mode 100644 contrib/sssd-pcsc.rules.in

diff --git a/Makefile.am b/Makefile.am
index ee97d095e07f6d485232a1afd87f9e4057688f22..57bece64c5fca058dcf57efffb149770ac3b773e 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
@@ -3566,10 +3570,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 51c0b0da8a7cfb692eabf38a71e42fbb0e40b4b0..7f9b15979f608bef67a7813201b6d8b08b2c09d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -406,6 +406,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 0000000000000000000000000000000000000000..31d2dbe4f97bfb75071886a4d4ccb546a7674a80
--- /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 e0367e4608956917375e2e9f983c4397897d4284..ec8bc9ddbacbdd46107a1576e9c5301f52781244 100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -171,6 +171,7 @@ Group: Applications/System
 License: GPLv3+
 Requires: libldb >= 0.9.3
 Requires: libtdb >= 1.1.3
+Requires: polkit
 Requires: sssd-client%{?_isa} = %{version}-%{release}
 Requires: libsss_idmap = %{version}-%{release}
 Conflicts: sssd < %{version}-%{release}
@@ -559,6 +560,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 +686,9 @@ 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
+
+%{_datadir}/polkit-1/rules.d/*
 
 %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 de5494d03fbec2f66a882521638e94942e964f90..fb53150e5529ae6e79b590cbc4f8ad6e8356e9af 100644
--- a/src/p11_child/p11_child_nss.c
+++ b/src/p11_child/p11_child_nss.c
@@ -498,10 +498,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);
@@ -598,24 +601,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());
-- 
2.1.0



More information about the sssd-devel mailing list