[systemd-devel] [PATCH 2/2 v3] socket: introduce SELinuxContextFromNet option
Michal Sekletar
msekleta at redhat.com
Tue Sep 2 06:06:28 PDT 2014
On Wed, Aug 27, 2014 at 04:45:32AM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> On Mon, Aug 25, 2014 at 10:02:58AM +0200, Michal Sekletar wrote:
> > <varlistentry>
> > + <term><varname>SELinuxContextFromNet=</varname></term>
> > + <listitem><para>Takes a boolean
> > + value. Controls whether systemd attempts to figure out
> > + SELinux label used for instantiated service from
> > + information handed by peer over the
> > + network. From information provided by
> > + peer we actually use only security level.
> > + Other parts of resulting SELinux context
> > + originate from either the target binary
> > + effectively triggered by socket unit or
> > + it is the value of
> > + <varname>SELinuxContext</varname>
> > + option. Configuration option has effect
> > + only on sockets with
> > + <varname>Accept</varname> mode set to
> > + <literal>yes</literal>. Also note that
> > + this option is usefull only when MLS/MCS
> > + SELinux policy is deployed. Defaults to
> > + <literal>false</literal>.
> > + </para></listitem>
> I think some prepositions are missing. Maybe something like this:
>
> Takes a boolean argument. When true systemd will attempt to figure
> out the SELinux label used for the instantiated service from
> the information handed by the peer over the network. Note
> that only the security label is used in the information provided
> by the peer. Other parts of the SELinux context originate from either
> the target binary that is effectively triggered by the socket unit
> or are taken from the value of the <varname>SELinuxContext=</varname>
> option. This configuration option only affects sockets with
> <varname>Accept=</varname> mode set to true. Also note that this
> option is useful only when MLS/MCS SELinux policy is deployed.
> Defaults to false.
Thanks for corrections, sounds a bit better than my original version.
>
> > + </varlistentry>
> > +
> > + <varlistentry>
> > <term><varname>PipeSize=</varname></term>
> > <listitem><para>Takes a size in
> > bytes. Controls the pipe buffer size
> > diff --git a/src/core/execute.c b/src/core/execute.c
> > index b5b2247..0e583ed 100644
> > --- a/src/core/execute.c
> > +++ b/src/core/execute.c
> > @@ -83,6 +83,7 @@
> > #include "af-list.h"
> > #include "mkdir.h"
> > #include "apparmor-util.h"
> > +#include "label.h"
> >
> > #ifdef HAVE_SECCOMP
> > #include "seccomp-util.h"
> > @@ -1232,6 +1233,7 @@ int exec_spawn(ExecCommand *command,
> > bool apply_chroot,
> > bool apply_tty_stdin,
> > bool confirm_spawn,
> > + bool selinux_context_net,
> > CGroupControllerMask cgroup_supported,
> > const char *cgroup_path,
> > const char *runtime_prefix,
> > @@ -1724,11 +1726,29 @@ int exec_spawn(ExecCommand *command,
> > #endif
> >
> > #ifdef HAVE_SELINUX
> > - if (context->selinux_context && use_selinux()) {
> > - err = setexeccon(context->selinux_context);
> > - if (err < 0 && !context->selinux_context_ignore) {
> > - r = EXIT_SELINUX_CONTEXT;
> > - goto fail_child;
> > + if (use_selinux()) {
> > + if (context->selinux_context) {
> > + err = setexeccon(context->selinux_context);
> > + if (err < 0 && !context->selinux_context_ignore) {
> > + r = EXIT_SELINUX_CONTEXT;
> > + goto fail_child;
> > + }
> > + }
> > +
> > + if (selinux_context_net && socket_fd >= 0) {
> > + _cleanup_free_ char *label = NULL;
> > +
> > + err = label_get_child_mls_label(socket_fd, command->path, &label);
> > + if (err < 0) {
> > + r = EXIT_SELINUX_CONTEXT;
> > + goto fail_child;
> > + }
> > +
> > + err = setexeccon(label);
> > + if (err < 0) {
> > + r = EXIT_SELINUX_CONTEXT;
> > + goto fail_child;
> > + }
> > }
> > }
> > #endif
> > diff --git a/src/core/execute.h b/src/core/execute.h
> > index 9d05d3a..23a466c 100644
> > --- a/src/core/execute.h
> > +++ b/src/core/execute.h
> > @@ -200,6 +200,7 @@ int exec_spawn(ExecCommand *command,
> > bool apply_chroot,
> > bool apply_tty_stdin,
> > bool confirm_spawn,
> > + bool selinux_context_net,
> > CGroupControllerMask cgroup_mask,
> > const char *cgroup_path,
> > const char *runtime_prefix,
> > diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
> > index 24aa80d..914d46e 100644
> > --- a/src/core/load-fragment-gperf.gperf.m4
> > +++ b/src/core/load-fragment-gperf.gperf.m4
> > @@ -262,6 +262,9 @@ Socket.SmackLabelIPOut, config_parse_string, 0,
> > `Socket.SmackLabel, config_parse_warn_compat, 0, 0
> > Socket.SmackLabelIPIn, config_parse_warn_compat, 0, 0
> > Socket.SmackLabelIPOut, config_parse_warn_compat, 0, 0')
> > +m4_ifdef(`HAVE_SELINUX',
> > +`Socket.SELinuxContextFromNet, config_parse_bool, 0, offsetof(Socket, selinux_context_from_net)',
> > +`Socket.SELinuxContextFromNet, config_parse_warn_compat, 0, 0')
> > EXEC_CONTEXT_CONFIG_ITEMS(Socket)m4_dnl
> > CGROUP_CONTEXT_CONFIG_ITEMS(Socket)m4_dnl
> > KILL_CONTEXT_CONFIG_ITEMS(Socket)m4_dnl
> > diff --git a/src/core/mount.c b/src/core/mount.c
> > index ec90b0a..223b9f7 100644
> > --- a/src/core/mount.c
> > +++ b/src/core/mount.c
> > @@ -715,6 +715,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) {
> > true,
> > true,
> > UNIT(m)->manager->confirm_spawn,
> > + false,
> > UNIT(m)->manager->cgroup_supported,
> > UNIT(m)->cgroup_path,
> > manager_get_runtime_prefix(UNIT(m)->manager),
> > diff --git a/src/core/service.c b/src/core/service.c
> > index 223e4b3..0fa1c6c 100644
> > --- a/src/core/service.c
> > +++ b/src/core/service.c
> > @@ -976,6 +976,7 @@ static int service_spawn(
> > apply_chroot,
> > apply_tty_stdin,
> > UNIT(s)->manager->confirm_spawn,
> > + s->socket_fd_selinux_context_net,
> > UNIT(s)->manager->cgroup_supported,
> > path,
> > manager_get_runtime_prefix(UNIT(s)->manager),
> > @@ -2705,7 +2706,7 @@ static void service_bus_name_owner_change(
> > }
> > }
> >
> > -int service_set_socket_fd(Service *s, int fd, Socket *sock) {
> > +int service_set_socket_fd(Service *s, int fd, Socket *sock, bool selinux_context_net) {
> > _cleanup_free_ char *peer = NULL;
> > int r;
> >
> > @@ -2743,6 +2744,8 @@ int service_set_socket_fd(Service *s, int fd, Socket *sock) {
> > }
> >
> > s->socket_fd = fd;
> > + s->socket_fd_selinux_context_net = selinux_context_net;
> > +
> >
> > unit_ref_set(&s->accept_socket, UNIT(sock));
> >
> > diff --git a/src/core/service.h b/src/core/service.h
> > index 5bcfd14..f9ec6dc 100644
> > --- a/src/core/service.h
> > +++ b/src/core/service.h
> > @@ -161,6 +161,7 @@ struct Service {
> >
> > pid_t main_pid, control_pid;
> > int socket_fd;
> > + bool socket_fd_selinux_context_net;
> >
> > bool permissions_start_only;
> > bool root_directory_start_only;
> > @@ -203,7 +204,7 @@ extern const UnitVTable service_vtable;
> >
> > struct Socket;
> >
> > -int service_set_socket_fd(Service *s, int fd, struct Socket *socket);
> > +int service_set_socket_fd(Service *s, int fd, struct Socket *socket, bool selinux_context_net);
> >
> > const char* service_state_to_string(ServiceState i) _const_;
> > ServiceState service_state_from_string(const char *s) _pure_;
> > diff --git a/src/core/socket.c b/src/core/socket.c
> > index 7ca8edb..eba17c0 100644
> > --- a/src/core/socket.c
> > +++ b/src/core/socket.c
> > @@ -31,6 +31,10 @@
> > #include <mqueue.h>
> > #include <sys/xattr.h>
> >
> > +#ifdef HAVE_SELINUX
> > +#include <selinux/selinux.h>
> > +#endif
> > +
> > #include "sd-event.h"
> > #include "log.h"
> > #include "load-dropin.h"
> > @@ -489,7 +493,8 @@ static void socket_dump(Unit *u, FILE *f, const char *prefix) {
> > "%sPassCredentials: %s\n"
> > "%sPassSecurity: %s\n"
> > "%sTCPCongestion: %s\n"
> > - "%sRemoveOnStop: %s\n",
> > + "%sRemoveOnStop: %s\n"
> > + "%sSELinuxContextFromNet: %s\n",
> > prefix, socket_state_to_string(s->state),
> > prefix, socket_result_to_string(s->result),
> > prefix, socket_address_bind_ipv6_only_to_string(s->bind_ipv6_only),
> > @@ -504,7 +509,8 @@ static void socket_dump(Unit *u, FILE *f, const char *prefix) {
> > prefix, yes_no(s->pass_cred),
> > prefix, yes_no(s->pass_sec),
> > prefix, strna(s->tcp_congestion),
> > - prefix, yes_no(s->remove_on_stop));
> > + prefix, yes_no(s->remove_on_stop),
> > + prefix, yes_no(s->selinux_context_from_net));
> >
> > if (s->control_pid > 0)
> > fprintf(f,
> > @@ -1128,8 +1134,12 @@ static int socket_open_fds(Socket *s) {
> > continue;
> >
> > if (p->type == SOCKET_SOCKET) {
> > -
> > - if (!know_label) {
> > + if (!know_label && s->selinux_context_from_net) {
> > + r = label_get_our_label(&label);
> > + if (r < 0)
> > + return r;
> > + know_label = true;
> > + } else if (!know_label) {
> >
> > r = socket_instantiate_service(s);
> > if (r < 0)
> > @@ -1386,6 +1396,7 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) {
> > true,
> > true,
> > UNIT(s)->manager->confirm_spawn,
> > + false,
> > UNIT(s)->manager->cgroup_supported,
> > UNIT(s)->cgroup_path,
> > manager_get_runtime_prefix(UNIT(s)->manager),
> > @@ -1820,7 +1831,7 @@ static void socket_enter_running(Socket *s, int cfd) {
> >
> > unit_choose_id(UNIT(service), name);
> >
> > - r = service_set_socket_fd(service, cfd, s);
> > + r = service_set_socket_fd(service, cfd, s, s->selinux_context_from_net);
> > if (r < 0)
> > goto fail;
> >
> > diff --git a/src/core/socket.h b/src/core/socket.h
> > index eede705..a2e0899 100644
> > --- a/src/core/socket.h
> > +++ b/src/core/socket.h
> > @@ -165,6 +165,8 @@ struct Socket {
> > char *smack_ip_in;
> > char *smack_ip_out;
> >
> > + bool selinux_context_from_net;
> > +
> > char *user, *group;
> > };
> >
> > diff --git a/src/core/swap.c b/src/core/swap.c
> > index 9f353af..00a33d1 100644
> > --- a/src/core/swap.c
> > +++ b/src/core/swap.c
> > @@ -643,6 +643,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) {
> > true,
> > true,
> > UNIT(s)->manager->confirm_spawn,
> > + false,
> > UNIT(s)->manager->cgroup_supported,
> > UNIT(s)->cgroup_path,
> > manager_get_runtime_prefix(UNIT(s)->manager),
> > diff --git a/src/shared/label.c b/src/shared/label.c
> > index 25a8b36..9aa8bf5 100644
> > --- a/src/shared/label.c
> > +++ b/src/shared/label.c
> > @@ -31,6 +31,7 @@
> > #ifdef HAVE_SELINUX
> > #include <selinux/selinux.h>
> > #include <selinux/label.h>
> > +#include <selinux/context.h>
> > #endif
> >
> > #include "label.h"
> > @@ -243,6 +244,91 @@ fail:
> > return r;
> > }
> >
> > +int label_get_our_label(char **label) {
> > + int r = 0;
> > + char *l = NULL;
> > +
> > +#ifdef HAVE_SELINUX
> > + r = getcon(&l);
> > + if (r < 0)
> > + return r;
> > +
> > + *label = l;
> > +#endif
> > +
> > + return r;
> > +}
> > +
> > +int label_get_child_mls_label(int socket_fd, const char *exe, char **label) {
> > + int r = 0;
> > +
> > +#ifdef HAVE_SELINUX
> > +
> > + _cleanup_security_context_free_ security_context_t mycon = NULL, peercon = NULL, fcon = NULL, ret = NULL;
> > + _cleanup_context_free_ context_t pcon = NULL, bcon = NULL;
> > + security_class_t sclass;
> > +
> > + const char *range = NULL;
> > +
> > + assert(socket_fd >= 0);
> > + assert(exe);
> > + assert(label);
> > +
> > + r = getcon(&mycon);
> > + if (r < 0)
> > + goto out;
> > +
> > + r = getpeercon(socket_fd, &peercon);
> > + if (r < 0)
> > + goto out;
> > +
> > + r = getexeccon(&fcon);
> > + if (r < 0)
> > + goto out;
> > +
> > + if (!fcon) {
> > + /* If there is no context set for next exec let's use context
> > + of target executable */
> > + r = getfilecon(exe, &fcon);
> > + if (r < 0)
> > + goto out;
> > + }
> > +
> > + bcon = context_new(mycon);
> > + if (!bcon)
> > + goto out;
> This will return 0. Should return log_oom() or ENOMEM instead?
Yep, will return ENOMEM.
>
> > + pcon = context_new(peercon);
> > + if (!pcon)
> > + goto out;
> Here too.
Same as above.
>
> > + range = context_range_get(pcon);
> > + if (!range)
> > + goto out;
> ...
>
> > + r = context_range_set(bcon, range);
> > + if (r)
> > + goto out;
> The check should be 'r < 0' like elsewhere.
Nope, libselinux is weird and context_range_set returns zero on success and
non-zero otherwise.
>
> > + freecon(mycon);
> > + mycon = context_str(bcon);
> > + if (!mycon)
> > + goto out;
> log_oom too.
>
> > + sclass = string_to_security_class("process");
> > + r = security_compute_create(mycon, fcon, sclass, &ret);
> > + if (r < 0)
> > + goto out;
> > +
> > + *label = ret;
> > +
> > +out:
> > + if (r && security_getenforce() == 1)
> > + r = -errno;
> > +#endif
>
> Please check for 'r < 0' and make sure that errno is set, to help the
> compiler (and readers).
> errno < 0 ? -errno : -EINVAL;
Will be fixed in next version of patch.
>
> Also, this might return -1 if security_getenforce() is false. Like
> Lennart said, should be something like -ENOSYS or something.
>
> Zbyszek
Michal
More information about the systemd-devel
mailing list