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
