Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On 2014-12-10 22:37, Lennart Poettering wrote: > On Tue, 09.12.14 18:26, Lennart Poettering (lenn...@poettering.net) wrote: > > Przemyslaw, > >>> +++ 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 >>> +) > > I have reverted the last bit above again, since it broke bootups in > nspawn machines. I figure the CAP_MAC_ADMIN capability is missing from > the bounding set in an nspawn, and that breaks the caps logic here. > > We should find another solution for this. I wanted to get 218 out of > the door, hence I reverted this bit for now, but we really should fine > a longer term solution for this. > > I build systemd with SMACK on, but turned off in the kernel. > > Any suggestions what we can do here? ConditionSecurity=smack instead of HAVE_SMACK would work, but it would also require separate unit for non-smack case, which is crap. No easy solutions come to my mind right now, unfortunately... Cheers, -- Karol Lewandowski, Samsung R&D Institute Poland ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On Tue, 09.12.14 18:26, Lennart Poettering (lenn...@poettering.net) wrote: Przemyslaw, > > +++ 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 > > +) I have reverted the last bit above again, since it broke bootups in nspawn machines. I figure the CAP_MAC_ADMIN capability is missing from the bounding set in an nspawn, and that breaks the caps logic here. We should find another solution for this. I wanted to get 218 out of the door, hence I reverted this bit for now, but we really should fine a longer term solution for this. I build systemd with SMACK on, but turned off in the kernel. Any suggestions what we can do here? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
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;
> @
[systemd-devel] [PATCH] bus-proxy: cloning smack label
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.
---
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_ADM
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On Mon, 01.12.14 11:47, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote: > Hello > Could you take a look at my patch? Sorry for the delay, I have a huge backlog of unreviewed patches, and am now working through them! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On Thu, 13.11.14 18:11, Przemyslaw Kedzierski (p.kedzier...@samsung.com) wrote:
Looks pretty good, but I coudln't apply it. There's something wrong
with the patch the deletion/renaming of the service files doesn't
work. Did you create this patch with git-format-patch?
> if (is_unix) {
> (void) getpeercred(in_fd, &ucred);
> (void) getpeersec(in_fd, &peersec);
> +
> +#ifdef HAVE_SMACK
> +if (mac_smack_use()) {
> +if (peersec) {
> +
> +r = mac_smack_apply_pid(getpid(), peersec);
> +if (r < 0)
> +log_warning("Failed to set SMACK
> label %s : %s", peersec, strerror(-r));
> +} else
> +log_warning("Invalid SMACK label");
> +
> +r = drop_capability(CAP_MAC_ADMIN);
> +if (r < 0)
> +log_warning("Failed to drop CAP_MAC_ADMIN:
> %s", strerror(-r));
> +}
> +#endif
> }
Hmm, could you make this bit a function of its own please?
> +m4_ifdef(`HAVE_SMACK',
> +Capabilities=cap_mac_admin=i
> +SecureBits=keep-caps
> +)
Hmm, it might be a good idea to also add some code to Makefile.am to
add the capability to the file after installation in case of
HAVE_SMACK. We used to do set a file cap like this on
systemd-detect-virt until a while back.
See commit fdd25311706bd32580ec4d43211cdf4665d2f9de for details about
the setcap lines we removed back then. It should be easy to just readd
those lines and adapt them to apply to systemd-bus-proxyd instead!
Thanks!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
Hello Could you take a look at my patch? Regards Przemyslaw Kedzierski ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] bus-proxy: cloning smack label
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.
Change-Id: I0acb5ec0d9ce4aecf25ddb0ca0e137b7a187a02f
---
Makefile.am | 4 ++--
src/bus-proxyd/bus-proxyd.c | 17 +
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 +++
8 files changed, 84 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 701666c..e9db1f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -588,7 +588,7 @@ EXTRA_DIST += \
units/user/systemd-exit.service.in \
units/systemd-f...@.service.in \
units/systemd-fsck-root.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 \
@@ -2528,7 +2528,7 @@ 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
#
--
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index d6607ed..08e2c7c 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -45,6 +45,7 @@
#include "def.h"
#include "capability.h"
#include "bus-policy.h"
+#include "smack-util.h"
static char *arg_address = NULL;
static char *arg_command_line_buffer = NULL;
@@ -1234,6 +1235,22 @@ int main(int argc, char *argv[]) {
if (is_unix) {
(void) getpeercred(in_fd, &ucred);
(void) getpeersec(in_fd, &peersec);
+
+#ifdef HAVE_SMACK
+if (mac_smack_use()) {
+if (peersec) {
+
+r = mac_smack_apply_pid(getpid(), peersec);
+if (r < 0)
+log_warning("Failed to set SMACK label
%s : %s", peersec, strerror(-r));
+} else
+log_warning("Invalid SMACK label");
+
+r = drop_capability(CAP_MAC_ADMIN);
+if (r < 0)
+log_warning("Failed to drop CAP_MAC_ADMIN:
%s", strerror(-r));
+}
+#endif
}
if (arg_drop_privileges) {
diff --git a/src/shared/capability.c b/src/shared/capability.c
index 0226542..9dc42ec 100644
--- a/src/shared/capability.c
+++ b/src/shared/capability.c
@@ -285,3 +285,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 eef703f..000
-
Re: [systemd-devel] [PATCH] bus-proxy: cloning smack label
On Thu, 06.11.14 11:44, 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.
Hmm, is this really the way this should work? I am a bit afraid of
including these lines on systems without SMACK. I figure this part at
least should be removed on non-SMACK builds, with m4 magic.
> Signed-off-by: Przemyslaw Kedzierski
We don't use S-o-b btw.
> (void) getpeercred(in_fd, &ucred);
> (void) getpeersec(in_fd, &peersec);
> +
> +if (mac_smack_use()) {
> +if (peersec) {
> +
> +r = mac_smack_set_process_label(peersec);
> +if (r < 0)
> +log_warning("Failed to set SMACK
> label %s : %s", peersec, strerror(-r));
Your colleage at Samsung, WaLyong Cho just posted a patch that added a
new call mac_smack_apply_pid() which is a superset of the
mac_smack_set_process_label(). I kinda like mac_smack_apply_pid()
better I must say, hence I'd love if we could get his patch in first,
and you could then rework your patch on top of his?
> +} else {
> +log_warning("Invalid SMACK label");
> +}
Please no {} on single-line if blocks.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] bus-proxy: cloning smack label
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.
Signed-off-by: Przemyslaw Kedzierski
---
src/bus-proxyd/bus-proxyd.c | 16
src/shared/capability.c | 18 ++
src/shared/capability.h | 2 ++
src/shared/smack-util.c | 18 ++
src/shared/smack-util.h | 1 +
units/systemd-bus-pro...@.service.in | 2 +-
units/u...@.service.in | 2 ++
7 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index d10de2f..066232e 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -45,6 +45,7 @@
#include "def.h"
#include "capability.h"
#include "bus-policy.h"
+#include "smack-util.h"
static char *arg_address = NULL;
static char *arg_command_line_buffer = NULL;
@@ -1168,6 +1169,21 @@ int main(int argc, char *argv[]) {
if (is_unix) {
(void) getpeercred(in_fd, &ucred);
(void) getpeersec(in_fd, &peersec);
+
+if (mac_smack_use()) {
+if (peersec) {
+
+r = mac_smack_set_process_label(peersec);
+if (r < 0)
+log_warning("Failed to set SMACK label
%s : %s", peersec, strerror(-r));
+} else {
+log_warning("Invalid SMACK label");
+}
+
+r = drop_capability(CAP_MAC_ADMIN);
+if (r < 0)
+log_warning("Failed to drop CAP_MAC_ADMIN:
%s", strerror(-r));
+}
}
if (arg_drop_privileges) {
diff --git a/src/shared/capability.c b/src/shared/capability.c
index 0226542..9dc42ec 100644
--- a/src/shared/capability.c
+++ b/src/shared/capability.c
@@ -285,3 +285,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/src/shared/smack-util.c b/src/shared/smack-util.c
index a8dccd1..3e9810c 100644
--- a/src/shared/smack-util.c
+++ b/src/shared/smack-util.c
@@ -181,3 +181,21 @@ int mac_smack_fix(const char *path, bool ignore_enoent,
bool ignore_erofs) {
return r;
}
+
+int mac_smack_set_process_label(char *label) {
+#ifdef HAVE_SMACK
+int ret;
+
+if (!label) {
+log_debug("No SMACK label given");
+return -1;
+}
+ret = write_string_file("/proc/self/attr/current", label);
+if (ret) {
+log_debug("Failed to set current SMACK label \"%s\" on self:
%s",
+ label, strerror(-ret));
+return ret;
+}
+#endif
+return 0;
+}
diff --git a/src/shared/smack-util.h b/src/shared/smack-util.h
index 68778da..a49405b 100644
--- a/src/shared/smack-util.h
+++ b/src/shared/smack-util.h
@@ -33,3 +33,4 @@ int mac_smack_apply(const char *path, const char *label);
int mac_smack_apply_fd(int fd, const char *label);
int mac_smack_apply_ip_in_fd(int fd, const char *label);
int mac_smack_apply_ip_out_fd(int fd, const char *label);
+int mac_smack_set_process_label(char *label);
diff --git a/units/systemd-bus-pro...@.serv
