On Tue, 09.12.14 12:17, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote:
> When dbus client connects to systemd-bus-proxyd through > Unix domain socket proxy takes client's smack label and sets for itself. > > It is done before and independent of dropping privileges. > > The reason of such soluton is fact that tests of access rights > performed by lsm may take place inside kernel, not only > in userspace of recipient of message. > > The bus-proxyd needs CAP_MAC_ADMIN to manipulate its label. > > In case of systemd running in system mode, CAP_MAC_ADMIN > should be added to CapabilityBoundingSet in service file of bus-proxyd. > > In case of systemd running in user mode ('systemd --user') > it can be achieved by addition > Capabilities=cap_mac_admin=i and SecureBits=keep-caps > to user@.service file > and setting cap_mac_admin+ei on bus-proxyd binary. Looks good! Applied! (made some minor changes though, we try to avoid strerror() nowadays, and log_warning_errno() instead with %m) Thanks! > --- > Makefile.am | 11 +++++++++-- > configure.ac | 4 ++++ > src/bus-proxyd/bus-proxyd.c | 20 ++++++++++++++++++++ > src/shared/capability.c | 18 ++++++++++++++++++ > src/shared/capability.h | 2 ++ > units/systemd-bus-pro...@.service.in | 22 ---------------------- > units/systemd-bus-pro...@.service.m4.in | 22 ++++++++++++++++++++++ > units/u...@.service.in | 19 ------------------- > units/u...@.service.m4.in | 23 +++++++++++++++++++++++ > 9 files changed, 98 insertions(+), 43 deletions(-) > delete mode 100644 units/systemd-bus-pro...@.service.in > create mode 100644 units/systemd-bus-pro...@.service.m4.in > delete mode 100644 units/u...@.service.in > create mode 100644 units/u...@.service.m4.in > > diff --git a/Makefile.am b/Makefile.am > index 7b43733..78cf4a9 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -591,7 +591,7 @@ EXTRA_DIST += \ > units/systemd-f...@.service.in \ > units/systemd-fsck-root.service.in \ > units/systemd-machine-id-commit.service.in \ > - units/u...@.service.in \ > + units/u...@.service.m4.in \ > units/debug-shell.service.in \ > units/systemd-suspend.service.in \ > units/quotaon.service.in \ > @@ -2579,9 +2579,16 @@ dist_userunit_DATA += \ > endif > > EXTRA_DIST += \ > - units/systemd-bus-pro...@.service.in \ > + units/systemd-bus-pro...@.service.m4.in \ > units/user/systemd-bus-pro...@.service.in > > +if HAVE_SMACK > +bus-proxyd-set-cap-hook: > + $(SETCAP) cap_mac_admin+ei > $(DESTDIR)$(rootlibexecdir)/systemd-bus-proxyd > + > +INSTALL_EXEC_HOOKS += bus-proxyd-set-cap-hook > +endif > + > # > ------------------------------------------------------------------------------ > systemd_tty_ask_password_agent_SOURCES = \ > src/tty-ask-password-agent/tty-ask-password-agent.c > diff --git a/configure.ac b/configure.ac > index 356a3c3..94b4a02 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -90,6 +90,8 @@ AC_PATH_PROG([XSLTPROC], [xsltproc]) > AC_PATH_PROG([QUOTAON], [quotaon], [/usr/sbin/quotaon], > [$PATH:/usr/sbin:/sbin]) > AC_PATH_PROG([QUOTACHECK], [quotacheck], [/usr/sbin/quotacheck], > [$PATH:/usr/sbin:/sbin]) > > +AC_PATH_PROG([SETCAP], [setcap], [/usr/sbin/setcap], [$PATH:/usr/sbin:/sbin]) > + > AC_PATH_PROG([KILL], [kill], [/usr/bin/kill], [$PATH:/usr/sbin:/sbin]) > > AC_PATH_PROG([KMOD], [kmod], [/usr/bin/kmod], [$PATH:/usr/sbin:/sbin]) > @@ -674,6 +676,8 @@ if test "x${have_smack}" = xyes ; then > AC_DEFINE(HAVE_SMACK, 1, [Define if SMACK is available]) > fi > > +AM_CONDITIONAL([HAVE_SMACK], [test "x$have_smack" = "xyes"]) > + > # > ------------------------------------------------------------------------------ > AC_ARG_ENABLE([gcrypt], > AS_HELP_STRING([--disable-gcrypt],[Disable optional GCRYPT support]), > diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c > index 42fb0da..a9940f1 100644 > --- a/src/bus-proxyd/bus-proxyd.c > +++ b/src/bus-proxyd/bus-proxyd.c > @@ -46,6 +46,7 @@ > #include "capability.h" > #include "bus-policy.h" > #include "bus-control.h" > +#include "smack-util.h" > > static char *arg_address = NULL; > static char *arg_command_line_buffer = NULL; > @@ -1235,6 +1236,18 @@ static int patch_sender(sd_bus *a, sd_bus_message *m) { > return 0; > } > > +static int mac_smack_apply_label_and_drop_cap_mac_admin(pid_t its_pid, const > char *new_label) { > + > + int r1 = 0, r2 = 0; > + if (mac_smack_use()) { > + if (new_label && its_pid) > + r1 = mac_smack_apply_pid(its_pid, new_label); > + > + r2 = drop_capability(CAP_MAC_ADMIN); > + } > + return (r1 < 0) ? r1 : r2; > +} > + > int main(int argc, char *argv[]) { > > _cleanup_bus_close_unref_ sd_bus *a = NULL, *b = NULL; > @@ -1274,6 +1287,13 @@ int main(int argc, char *argv[]) { > if (is_unix) { > (void) getpeercred(in_fd, &ucred); > (void) getpeersec(in_fd, &peersec); > + > +#ifdef HAVE_SMACK > + r = mac_smack_apply_label_and_drop_cap_mac_admin(getpid(), > peersec); > + if (r < 0) > + log_warning("Failed to set SMACK label (%s) and drop" > + " CAP_MAC_ADMIN : %s", peersec, > strerror(-r)); > +#endif > } > > if (arg_drop_privileges) { > diff --git a/src/shared/capability.c b/src/shared/capability.c > index 5d156ab..65d7e03 100644 > --- a/src/shared/capability.c > +++ b/src/shared/capability.c > @@ -271,3 +271,21 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t > keep_capabilities) { > > return 0; > } > + > +int drop_capability(cap_value_t cv) { > + _cleanup_cap_free_ cap_t tmp_cap = NULL; > + > + tmp_cap = cap_get_proc(); > + if (!tmp_cap) > + return -errno; > + > + if ((cap_set_flag(tmp_cap, CAP_INHERITABLE, 1, &cv, CAP_CLEAR) < 0) > || > + (cap_set_flag(tmp_cap, CAP_PERMITTED, 1, &cv, CAP_CLEAR) < 0) || > + (cap_set_flag(tmp_cap, CAP_EFFECTIVE, 1, &cv, CAP_CLEAR) < 0)) > + return -errno; > + > + if (cap_set_proc(tmp_cap) < 0) > + return -errno; > + > + return 0; > +} > diff --git a/src/shared/capability.h b/src/shared/capability.h > index 3e6d999..6f2f6f9 100644 > --- a/src/shared/capability.h > +++ b/src/shared/capability.h > @@ -34,6 +34,8 @@ int capability_bounding_set_drop_usermode(uint64_t drop); > > int drop_privileges(uid_t uid, gid_t gid, uint64_t keep_capabilites); > > +int drop_capability(cap_value_t cv); > + > DEFINE_TRIVIAL_CLEANUP_FUNC(cap_t, cap_free); > #define _cleanup_cap_free_ _cleanup_(cap_freep) > > diff --git a/units/systemd-bus-pro...@.service.in > b/units/systemd-bus-pro...@.service.in > deleted file mode 100644 > index 23b5ffa..0000000 > --- a/units/systemd-bus-pro...@.service.in > +++ /dev/null > @@ -1,22 +0,0 @@ > -# This file is part of systemd. > -# > -# systemd is free software; you can redistribute it and/or modify it > -# under the terms of the GNU Lesser General Public License as published by > -# the Free Software Foundation; either version 2.1 of the License, or > -# (at your option) any later version. > - > -[Unit] > -Description=Legacy D-Bus Protocol Compatibility Daemon > - > -[Service] > -# The first argument will be replaced by the service by information on > -# the process requesting the proxy, we need a placeholder to keep the > -# space available for this. > -ExecStart=@rootlibexecdir@/systemd-bus-proxyd --drop-privileges > --address=kernel:path=/sys/fs/kdbus/0-system/bus > xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > -NotifyAccess=main > -CapabilityBoundingSet=CAP_IPC_OWNER CAP_SETUID CAP_SETGID CAP_SETPCAP > -PrivateTmp=yes > -PrivateDevices=yes > -PrivateNetwork=yes > -ProtectSystem=full > -ProtectHome=yes > diff --git a/units/systemd-bus-pro...@.service.m4.in > b/units/systemd-bus-pro...@.service.m4.in > new file mode 100644 > index 0000000..3f3ab64 > --- /dev/null > +++ b/units/systemd-bus-pro...@.service.m4.in > @@ -0,0 +1,22 @@ > +# This file is part of systemd. > +# > +# systemd is free software; you can redistribute it and/or modify it > +# under the terms of the GNU Lesser General Public License as published by > +# the Free Software Foundation; either version 2.1 of the License, or > +# (at your option) any later version. > + > +[Unit] > +Description=Legacy D-Bus Protocol Compatibility Daemon > + > +[Service] > +# The first argument will be replaced by the service by information on > +# the process requesting the proxy, we need a placeholder to keep the > +# space available for this. > +ExecStart=@rootlibexecdir@/systemd-bus-proxyd --drop-privileges > --address=kernel:path=/sys/fs/kdbus/0-system/bus > xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > +NotifyAccess=main > +CapabilityBoundingSet=CAP_IPC_OWNER CAP_SETUID CAP_SETGID CAP_SETPCAP > m4_ifdef(`HAVE_SMACK', CAP_MAC_ADMIN ) > +PrivateTmp=yes > +PrivateDevices=yes > +PrivateNetwork=yes > +ProtectSystem=full > +ProtectHome=yes > diff --git a/units/u...@.service.in b/units/u...@.service.in > deleted file mode 100644 > index 1e21d51..0000000 > --- a/units/u...@.service.in > +++ /dev/null > @@ -1,19 +0,0 @@ > -# This file is part of systemd. > -# > -# systemd is free software; you can redistribute it and/or modify it > -# under the terms of the GNU Lesser General Public License as published by > -# the Free Software Foundation; either version 2.1 of the License, or > -# (at your option) any later version. > - > -[Unit] > -Description=User Manager for UID %i > -After=systemd-user-sessions.service > - > -[Service] > -User=%i > -PAMName=systemd-user > -Type=notify > -ExecStart=-@rootlibexecdir@/systemd --user > -Slice=user-%i.slice > -KillMode=mixed > -Delegate=yes > diff --git a/units/u...@.service.m4.in b/units/u...@.service.m4.in > new file mode 100644 > index 0000000..340c02b > --- /dev/null > +++ b/units/u...@.service.m4.in > @@ -0,0 +1,23 @@ > +# This file is part of systemd. > +# > +# systemd is free software; you can redistribute it and/or modify it > +# under the terms of the GNU Lesser General Public License as published by > +# the Free Software Foundation; either version 2.1 of the License, or > +# (at your option) any later version. > + > +[Unit] > +Description=User Manager for UID %i > +After=systemd-user-sessions.service > + > +[Service] > +User=%i > +PAMName=systemd-user > +Type=notify > +ExecStart=-@rootlibexecdir@/systemd --user > +Slice=user-%i.slice > +KillMode=mixed > +Delegate=yes > +m4_ifdef(`HAVE_SMACK', > +Capabilities=cap_mac_admin=i > +SecureBits=keep-caps > +) > -- > 2.1.2 > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel