-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/22/2010 10:21 AM, Stephen Smalley wrote: > On Thu, 2010-07-22 at 08:22 -0400, Daniel J Walsh wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> Wanted to have you guys review before I send it up to systemd. >> >> This patch sets the socket context based on the domain of the daemon >> that systemd will start on connection. It also labels the fifo_file >> based off of the daemons label and the label of the directory the >> fifo_file will be created in. >> >> >> The patch does not handle, systemd creating the directories for the >> fifo_file. In the future, their is talk of making /var/run a tmpfs file >> system. This would mean systemd would create /var/run/mysqld/ before >> creating /var/run/mysqld/mysqld.socket. Additional SELinux controls >> would have to be added to systemd to get this correct. Not sure if the >> correct thing to do is at selabel or use >> selinux_getfileconfrompath(daemon, parentdir, "dir") > > selabel_lookup is likely safer, as the /var/run/mysqld directory might > be created by the package or by the init script rather than by the > daemon itself, so there might not be a type transition defined in policy > for it. A few comments below. > > diff --git a/configure.ac b/configure.ac > index 03feb43..4c75f66 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -105,6 +105,11 @@ PKG_CHECK_MODULES(DBUS, [ dbus-1 >= 1.3.2 ]) > AC_SUBST(DBUS_CFLAGS) > AC_SUBST(DBUS_LIBS) > > +PKG_CHECK_MODULES(SELINUX, [ libselinux >= 2.0.96 ]) > > Not sure you need this strict of a version check. The libselinux > interfaces that you are using have been around for a while. > > diff --git a/src/socket-util.c b/src/socket-util.c > index 442abfe..7712b8b 100644 > --- a/src/socket-util.c > +++ b/src/socket-util.c > @@ -315,8 +316,12 @@ int socket_address_listen( > if ((r = socket_address_verify(a)) < 0) > return r; > > - /* FIXME SELINUX: The socket() here should be done with the > - * right SELinux context set */ > + if (scon && setsockcreatecon(scon) < 0) { > > Why not unconditionally call setsockcreatecon(scon) here? > If scon is NULL, this will simply reset to the default policy behavior > for the socket so it does no harm and it will prevent you from > accidentally labeling a socket with the context used the last time > around. Alternatively you should call setsockcreatecon(NULL) after > calling socket() each time to reset it. > > diff --git a/src/socket.c b/src/socket.c > index b06ba09..f1f378c 100644 > --- a/src/socket.c > +++ b/src/socket.c > > static int fifo_address_create( > <snip> > - /* FIXME SELINUX: The mkfifo here should be done with > - * the right SELinux context set */ > + if (scon && ((r = selinux_getfileconfrompath(scon, path, "FIFO_FILE", &filecon)) == 0)) { > > Should be 'fifo_file' (lowercase) rather than FIFO_FILE. > > + r = setfscreatecon(filecon); > > Where do you reset the fscreate context to NULL so that other directories and files won't keep > being created in the prior fscreate context? > Updated with your comments. Strange the FIFO_FILE did not cause security_compute_create to fail when passing a 0 for the tclass? I though this should fail. I changed the patch to check the output of string_to_security_class. Will write the selabel patch after this is accepted. Not checking the return of setfscreatecon(NULL) or setsockcreatecon(NULL) Since I am not sure what to do if these fail and not likely to fail since the previous calls worked. Is there any way to see what a socket is labeled? netstat -aZ is just showing the process context, not the context of the label on the socket? -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkxIaHoACgkQrlYvE4MpobO//QCffAGKfiRksHExiSEy0nsJesI/ 8/oAoL1qZ62jdnOZueRIKgDvwoZPrULy =rt43 -----END PGP SIGNATURE-----
diff --git a/Makefile.am b/Makefile.am index 4dcecc5..31499ba 100644 --- a/Makefile.am +++ b/Makefile.am @@ -298,7 +298,8 @@ libsystemd_core_la_LIBADD = \ $(DBUS_LIBS) \ $(UDEV_LIBS) \ $(LIBWRAP_LIBS) \ - $(PAM_LIBS) + $(PAM_LIBS) \ + $(SELINUX_LIBS) # This is needed because automake is buggy in how it generates the # rules for C programs, but not Vala programs. We therefore can't diff --git a/configure.ac b/configure.ac index 03feb43..14622e4 100644 --- a/configure.ac +++ b/configure.ac @@ -105,6 +105,11 @@ PKG_CHECK_MODULES(DBUS, [ dbus-1 >= 1.3.2 ]) AC_SUBST(DBUS_CFLAGS) AC_SUBST(DBUS_LIBS) +PKG_CHECK_MODULES(SELINUX, [ libselinux ]) +AC_SUBST(SELINUX_CFLAGS) +AC_SUBST(SELINUX_LIBS) +AC_SEARCH_LIBS([is_selinux_enabled], [selinux], [], [AC_MSG_ERROR([*** libselinux library not found])]) + PKG_CHECK_MODULES(DBUSGLIB, [ dbus-glib-1 ]) AC_SUBST(DBUSGLIB_CFLAGS) AC_SUBST(DBUSGLIB_LIBS) diff --git a/src/socket-util.c b/src/socket-util.c index 442abfe..9247965 100644 --- a/src/socket-util.c +++ b/src/socket-util.c @@ -29,6 +29,7 @@ #include <net/if.h> #include <sys/types.h> #include <sys/stat.h> +#include <selinux/selinux.h> #include "macro.h" #include "util.h" @@ -305,7 +306,7 @@ int socket_address_listen( bool free_bind, mode_t directory_mode, mode_t socket_mode, - /* FIXME SELINUX: pass SELinux context object here */ + security_context_t scon, int *ret) { int r, fd, one; @@ -315,8 +316,12 @@ int socket_address_listen( if ((r = socket_address_verify(a)) < 0) return r; - /* FIXME SELINUX: The socket() here should be done with the - * right SELinux context set */ + if (setsockcreatecon(scon) < 0) { + log_error("Failed to set SELinux context (%s) on socket:", scon); + if (security_getenforce() == 1) { + return -errno; + } + } if ((fd = socket(socket_address_family(a), a->type | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)) < 0) return -errno; diff --git a/src/socket-util.h b/src/socket-util.h index 68c579b..841570f 100644 --- a/src/socket-util.h +++ b/src/socket-util.h @@ -26,6 +26,7 @@ #include <netinet/in.h> #include <sys/un.h> #include <net/if.h> +#include <selinux/selinux.h> #include "macro.h" #include "util.h" @@ -71,7 +72,7 @@ int socket_address_listen( bool free_bind, mode_t directory_mode, mode_t socket_mode, - /* FIXME SELINUX: pass SELinux context object here */ + security_context_t scon, int *ret); bool socket_address_is(const SocketAddress *a, const char *s, int type); diff --git a/src/socket.c b/src/socket.c index b06ba09..2816de4 100644 --- a/src/socket.c +++ b/src/socket.c @@ -27,6 +27,7 @@ #include <sys/epoll.h> #include <signal.h> #include <arpa/inet.h> +#include <selinux/selinux.h> #include "unit.h" #include "socket.h" @@ -642,24 +643,86 @@ static void socket_apply_fifo_options(Socket *s, int fd) { log_warning("F_SETPIPE_SZ: %m"); } +static int selinux_getconfromexe( + const char *exe, + security_context_t *newcon) { + + security_context_t mycon = NULL, fcon = NULL; + security_class_t sclass; + int r = 0; + + r = getcon(&mycon); + if (r < 0) + goto fail; + + r = getfilecon(exe, &fcon); + if (r < 0) + goto fail; + + sclass = string_to_security_class("process"); + r = security_compute_create(mycon, fcon, sclass, newcon); + +fail: + if (r < 0) + r = -errno; + + freecon(mycon); + freecon(fcon); + return r; +} + +static int selinux_getfileconfrompath( + const security_context_t scon, + const char *path, + const char *class, + security_context_t *fcon) { + + security_context_t dir_con = NULL; + security_class_t sclass; + int r = 0; + + r = getfilecon(path, &dir_con); + if (r >= 0) { + r = -1; + if ((sclass = string_to_security_class(class)) != 0) + r = security_compute_create(scon, dir_con, sclass, fcon); + } + if (r < 0) + r = -errno; + + freecon(dir_con); + return r; +} + static int fifo_address_create( const char *path, mode_t directory_mode, mode_t socket_mode, - /* FIXME SELINUX: pass SELinux context object here */ + security_context_t scon, int *_fd) { - int fd = -1, r; + int fd = -1, r = 0; struct stat st; mode_t old_mask; + security_context_t filecon = NULL; assert(path); assert(_fd); mkdir_parents(path, directory_mode); - /* FIXME SELINUX: The mkfifo here should be done with - * the right SELinux context set */ + if (scon && ((r = selinux_getfileconfrompath(scon, path, "fifo_file", &filecon)) == 0)) { + r = setfscreatecon(filecon); + if ( r < 0 ) { + log_error("Failed to set SELinux file context (%s) on %s:", scon, path); + r = -errno; + } + + freecon(filecon); + } + + if ( r < 0 && security_getenforce() == 1) + goto fail; /* Enforce the right access mode for the fifo */ old_mask = umask(~ socket_mode); @@ -680,6 +743,8 @@ static int fifo_address_create( goto fail; } + setfscreatecon(NULL); + if (fstat(fd, &st) < 0) { r = -errno; goto fail; @@ -698,6 +763,7 @@ static int fifo_address_create( return 0; fail: + setfscreatecon(NULL); if (fd >= 0) close_nointr_nofail(fd); @@ -707,6 +773,7 @@ fail: static int socket_open_fds(Socket *s) { SocketPort *p; int r; + security_context_t scon = NULL; assert(s); @@ -726,6 +793,13 @@ static int socket_open_fds(Socket *s) { s->service->exec_command[SERVICE_EXEC_START]->path); */ + if (selinux_getconfromexe(s->service->exec_command[SERVICE_EXEC_START]->path, &scon) < 0) { + log_error("Failed to get SELinux exec context for %s \n", s->service->exec_command[SERVICE_EXEC_START]->path); + if (security_getenforce() == 1) + return -errno; + } + + log_debug("SELinux Socket context for %s set to %s\n", s->service->exec_command[SERVICE_EXEC_START]->path, scon); LIST_FOREACH(port, p, s->ports) { if (p->fd >= 0) @@ -741,7 +815,7 @@ static int socket_open_fds(Socket *s) { s->free_bind, s->directory_mode, s->socket_mode, - /* FIXME SELINUX: Pass the SELinux context object here */ + scon, &p->fd)) < 0) goto rollback; @@ -753,7 +827,7 @@ static int socket_open_fds(Socket *s) { p->path, s->directory_mode, s->socket_mode, - /* FIXME SELINUX: Pass the SELinux context object here */ + scon, &p->fd)) < 0) goto rollback; @@ -763,10 +837,12 @@ static int socket_open_fds(Socket *s) { assert_not_reached("Unknown port type"); } + freecon(scon); return 0; rollback: socket_close_fds(s); + freecon(scon); return r; }
Attachment:
systemd-selinux.patch.sig
Description: PGP signature