-----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
