* This is my first patch for systemd @ 2010-07-22 12:22 Daniel J Walsh 2010-07-22 14:21 ` Stephen Smalley 2010-07-23 2:33 ` Kyle Moffett 0 siblings, 2 replies; 9+ messages in thread From: Daniel J Walsh @ 2010-07-22 12:22 UTC (permalink / raw) To: Stephen Smalley, SELinux [-- Attachment #1: Type: text/plain, Size: 1057 bytes --] -----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") -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkxIN/IACgkQrlYvE4MpobNKsgCfRln4hH0J+gLEMyB39HiDbYHE kggAoM/i43XMzUffcwXAt15iCLHlsJOu =GN2k -----END PGP SIGNATURE----- [-- Attachment #2: systemd-selinux.patch --] [-- Type: text/plain, Size: 7727 bytes --] 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..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 ]) +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..7712b8b 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 (scon && 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..f1f378c 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,85 @@ 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) { + sclass = string_to_security_class(class); + 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); @@ -707,6 +769,7 @@ fail: static int socket_open_fds(Socket *s) { SocketPort *p; int r; + security_context_t scon = NULL; assert(s); @@ -726,6 +789,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("%s %s\n", s->service->exec_command[SERVICE_EXEC_START]->path, scon); LIST_FOREACH(port, p, s->ports) { if (p->fd >= 0) @@ -741,7 +811,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 +823,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 +833,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 #3: systemd-selinux.patch.sig --] [-- Type: application/pgp-signature, Size: 72 bytes --] ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-22 12:22 This is my first patch for systemd Daniel J Walsh @ 2010-07-22 14:21 ` Stephen Smalley 2010-07-22 15:49 ` Daniel J Walsh 2010-07-23 2:33 ` Kyle Moffett 1 sibling, 1 reply; 9+ messages in thread From: Stephen Smalley @ 2010-07-22 14:21 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SELinux 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? -- Stephen Smalley National Security Agency -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-22 14:21 ` Stephen Smalley @ 2010-07-22 15:49 ` Daniel J Walsh 2010-07-22 16:48 ` Stephen Smalley 0 siblings, 1 reply; 9+ messages in thread From: Daniel J Walsh @ 2010-07-22 15:49 UTC (permalink / raw) To: Stephen Smalley; +Cc: SELinux [-- Attachment #1: Type: text/plain, Size: 4011 bytes --] -----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----- [-- Attachment #2: systemd-selinux.patch --] [-- Type: text/plain, Size: 8217 bytes --] 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 #3: systemd-selinux.patch.sig --] [-- Type: application/pgp-signature, Size: 72 bytes --] ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-22 15:49 ` Daniel J Walsh @ 2010-07-22 16:48 ` Stephen Smalley 2010-07-22 19:37 ` Daniel J Walsh 0 siblings, 1 reply; 9+ messages in thread From: Stephen Smalley @ 2010-07-22 16:48 UTC (permalink / raw) To: Daniel J Walsh; +Cc: SELinux On Thu, 2010-07-22 at 11:49 -0400, Daniel J Walsh wrote: > 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. Yes, that's fine. > 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? netstat should be able to call fgetfilecon() on the socket fd after opening the /proc/pid/fd/<n> file to get the socket label. Not sure why it is using getpidcon() on the process instead. Who wrote that patch? -- Stephen Smalley National Security Agency -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-22 16:48 ` Stephen Smalley @ 2010-07-22 19:37 ` Daniel J Walsh 0 siblings, 0 replies; 9+ messages in thread From: Daniel J Walsh @ 2010-07-22 19:37 UTC (permalink / raw) To: Stephen Smalley; +Cc: SELinux -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 07/22/2010 12:48 PM, Stephen Smalley wrote: > On Thu, 2010-07-22 at 11:49 -0400, Daniel J Walsh wrote: >> 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. > > Yes, that's fine. > >> 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? > > netstat should be able to call fgetfilecon() on the socket fd after > opening the /proc/pid/fd/<n> file to get the socket label. Not sure why > it is using getpidcon() on the process instead. Who wrote that patch? > Probably me 8 years ago... :^( -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkxInekACgkQrlYvE4MpobPtBQCgyetTDlI5j2ZYp9uFItzJtZDR 4zAAoOrSPoHDYrdetSARtOWf3k2PjYgf =n9Yp -----END PGP SIGNATURE----- -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-22 12:22 This is my first patch for systemd Daniel J Walsh 2010-07-22 14:21 ` Stephen Smalley @ 2010-07-23 2:33 ` Kyle Moffett 2010-07-23 20:05 ` Stephen Smalley 1 sibling, 1 reply; 9+ messages in thread From: Kyle Moffett @ 2010-07-23 2:33 UTC (permalink / raw) To: Daniel J Walsh; +Cc: Stephen Smalley, SELinux On Thu, Jul 22, 2010 at 08:22, Daniel J Walsh <dwalsh@redhat.com> wrote: > 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") Hmm, one thing that I've been thinking about for a while now is a policy extension to type transitions allowing a match against the object name. So for example, assuming /var/run gets correctly labelled as var_run_t: typename_transition systemd_t var_run_t:file "mysqld" mysql_var_run_t; If some form of globbing or patterns was added, this could in theory even replace the current file_contexts file with a big list of named_type_transitions in the policy. Admittedly it could potentially make the policy larger, but I believe with careful encoding and compression it would be a net reduction in the size of /etc/selinux/$POLICYNAME/ (since the plain-text highly-repetitive file_contexts would go away). There are a few limitations that I can think of, though: * You could only match on whatever "name" field is available in the security manager, not the full path, although with proper labels on most of the path components this is not really an issue. * Since the name matching would need to be performed in the kernel, there would be a very strict limit on the complexity of the pattern or glob engine (probably NOT a bad thing :-D) As an example of where this would be useful, you could automatically assign separate labels to ".htpasswd" and ".htaccess" files created in www-data directories to prevent compromised web applications from creating, modifying, or even reading them. Cheers, Kyle Moffett -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-23 2:33 ` Kyle Moffett @ 2010-07-23 20:05 ` Stephen Smalley 2010-07-24 5:13 ` Kyle Moffett 0 siblings, 1 reply; 9+ messages in thread From: Stephen Smalley @ 2010-07-23 20:05 UTC (permalink / raw) To: Kyle Moffett; +Cc: Daniel J Walsh, SELinux, Eric Paris On Thu, 2010-07-22 at 22:33 -0400, Kyle Moffett wrote: > On Thu, Jul 22, 2010 at 08:22, Daniel J Walsh <dwalsh@redhat.com> wrote: > > 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") > > Hmm, one thing that I've been thinking about for a while now is a > policy extension to type transitions allowing a match against the > object name. > > So for example, assuming /var/run gets correctly labelled as var_run_t: > > typename_transition systemd_t var_run_t:file "mysqld" mysql_var_run_t; > > If some form of globbing or patterns was added, this could in theory > even replace the current file_contexts file with a big list of > named_type_transitions in the policy. Admittedly it could potentially > make the policy larger, but I believe with careful encoding and > compression it would be a net reduction in the size of > /etc/selinux/$POLICYNAME/ (since the plain-text highly-repetitive > file_contexts would go away). I don't think so. The file_contexts configuration is for the initial assignment of filesystem labeling state by userspace, not for determining how files are labeled at runtime. The file_contexts configuration would still be needed for applications like rpm and install in order to determine the right security context, even with an extended type_transition construct. So this would not eliminate the need for file_contexts. We did previously discuss enabling type_transition rules to match on last component name, but the initial forays on linux-fsdevel on the necessary changes to pass down the name to the right function were shot down by the vfs folks IIRC. restorecond is your best bet if you can't make use of the existing type transition rules. > There are a few limitations that I can think of, though: > > * You could only match on whatever "name" field is available in the > security manager, not the full path, although with proper labels on > most of the path components this is not really an issue. > > * Since the name matching would need to be performed in the kernel, > there would be a very strict limit on the complexity of the pattern or > glob engine (probably NOT a bad thing :-D) > > As an example of where this would be useful, you could automatically > assign separate labels to ".htpasswd" and ".htaccess" files created in > www-data directories to prevent compromised web applications from > creating, modifying, or even reading them. > > Cheers, > Kyle Moffett -- Stephen Smalley National Security Agency -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-23 20:05 ` Stephen Smalley @ 2010-07-24 5:13 ` Kyle Moffett 2010-07-27 13:24 ` Stephen Smalley 0 siblings, 1 reply; 9+ messages in thread From: Kyle Moffett @ 2010-07-24 5:13 UTC (permalink / raw) To: Stephen Smalley; +Cc: Daniel J Walsh, SELinux, Eric Paris On Fri, Jul 23, 2010 at 16:05, Stephen Smalley <sds@tycho.nsa.gov> wrote: > On Thu, 2010-07-22 at 22:33 -0400, Kyle Moffett wrote: >> Hmm, one thing that I've been thinking about for a while now is a >> policy extension to type transitions allowing a match against the >> object name. >> >> So for example, assuming /var/run gets correctly labelled as var_run_t: >> >> typename_transition systemd_t var_run_t:file "mysqld" mysql_var_run_t; >> >> If some form of globbing or patterns was added, this could in theory >> even replace the current file_contexts file with a big list of >> named_type_transitions in the policy. Admittedly it could potentially >> make the policy larger, but I believe with careful encoding and >> compression it would be a net reduction in the size of >> /etc/selinux/$POLICYNAME/ (since the plain-text highly-repetitive >> file_contexts would go away). > > I don't think so. The file_contexts configuration is for the initial > assignment of filesystem labeling state by userspace, not for > determining how files are labeled at runtime. The file_contexts > configuration would still be needed for applications like rpm and > install in order to determine the right security context, even with an > extended type_transition construct. So this would not eliminate the > need for file_contexts. I understand the problem of initial labeling, but it would still be possible to generate the entire set of initial labels from type transition rules if name-pattern-matching were introduced. Say you define 2 initial SID types in policy, an "fsroot_t" type and a "labelinit_t" type. Then to label the top-level directories, you have something along the lines of: typename_transition labelinit_t fsroot_t:dir "lib" lib_t; typename_transition labelinit_t fsroot_t:dir "usr" usr_t; typename_transition labelinit_t fsroot_t:dir "etc" etc_t; [...] typename_transition labelinit_t usr_t:dir "lib" lib_t; To label the /etc/passwd file, you would have: typename_transition labelinit_t etc_t:file "passwd" etc_passwd_t; Presumably you could do something similar with role-transitions (although would anything other than object_r be appropriate?) and with range-transitions in the MLS/MCS case. The downside is a whole lot of new transition rules, but I believe that we can improve the speed and performance of the policy compiler and libsepol/libsemanage enough that it won't be an issue. When you run "restorecon", "dpkg", "rpm", etc... all they would need to do while unpacking is maintain the current label for each directory in the path, and then check the transition from labelinit_t into the destination directory for the file. In practice I suspect that this would also rather dramatically cut down on the number of rules needed to secure certain filetypes, and furthermore it would make it much easier to customize the system. For example, let's say I'm developing on some GIT checkouts of X and installing them in /opt/xorgdev so they don't mess with my working distribution-provided X install. All I would need to do is ensure that the immediate subdirectories of /opt/xorgdev/ are labelled properly with lib_t, bin_t, etc. Then when I drop the suid X binaries into /opt/xorgdev/bin by hand and restorecon them, the pre-existing transition rule for "X" in a bin_t directory would trigger and set the context correctly. IE: When you put a suid program called "X" in some directory that's in the default path, it should get special SELinux privileges. > We did previously discuss enabling type_transition rules to match on > last component name, but the initial forays on linux-fsdevel on the > necessary changes to pass down the name to the right function were shot > down by the vfs folks IIRC. Hmm, that's depressing... Do you have a link to the patches that were posted? From what I understand some of the hooks for path-based security eventually got merged, so it's possible the information is now more readily available than it was before. The audit logs seem to contain the file or directory name *most* of the time, so it seems that it is frequently available somehow. Cheers, Kyle Moffett -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: This is my first patch for systemd 2010-07-24 5:13 ` Kyle Moffett @ 2010-07-27 13:24 ` Stephen Smalley 0 siblings, 0 replies; 9+ messages in thread From: Stephen Smalley @ 2010-07-27 13:24 UTC (permalink / raw) To: Kyle Moffett; +Cc: Daniel J Walsh, SELinux, Eric Paris On Sat, 2010-07-24 at 01:13 -0400, Kyle Moffett wrote: > On Fri, Jul 23, 2010 at 16:05, Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On Thu, 2010-07-22 at 22:33 -0400, Kyle Moffett wrote: > >> Hmm, one thing that I've been thinking about for a while now is a > >> policy extension to type transitions allowing a match against the > >> object name. > >> > >> So for example, assuming /var/run gets correctly labelled as var_run_t: > >> > >> typename_transition systemd_t var_run_t:file "mysqld" mysql_var_run_t; > >> > >> If some form of globbing or patterns was added, this could in theory > >> even replace the current file_contexts file with a big list of > >> named_type_transitions in the policy. Admittedly it could potentially > >> make the policy larger, but I believe with careful encoding and > >> compression it would be a net reduction in the size of > >> /etc/selinux/$POLICYNAME/ (since the plain-text highly-repetitive > >> file_contexts would go away). > > > > I don't think so. The file_contexts configuration is for the initial > > assignment of filesystem labeling state by userspace, not for > > determining how files are labeled at runtime. The file_contexts > > configuration would still be needed for applications like rpm and > > install in order to determine the right security context, even with an > > extended type_transition construct. So this would not eliminate the > > need for file_contexts. > > I understand the problem of initial labeling, but it would still be > possible to generate the entire set of initial labels from type > transition rules if name-pattern-matching were introduced. > > Say you define 2 initial SID types in policy, an "fsroot_t" type and a > "labelinit_t" type. > > Then to label the top-level directories, you have something along the lines of: > > typename_transition labelinit_t fsroot_t:dir "lib" lib_t; > typename_transition labelinit_t fsroot_t:dir "usr" usr_t; > typename_transition labelinit_t fsroot_t:dir "etc" etc_t; > [...] > typename_transition labelinit_t usr_t:dir "lib" lib_t; > > To label the /etc/passwd file, you would have: > > typename_transition labelinit_t etc_t:file "passwd" etc_passwd_t; > > Presumably you could do something similar with role-transitions > (although would anything other than object_r be appropriate?) and with > range-transitions in the MLS/MCS case. > > The downside is a whole lot of new transition rules, but I believe > that we can improve the speed and performance of the policy compiler > and libsepol/libsemanage enough that it won't be an issue. > > When you run "restorecon", "dpkg", "rpm", etc... all they would need > to do while unpacking is maintain the current label for each directory > in the path, and then check the transition from labelinit_t into the > destination directory for the file. > > In practice I suspect that this would also rather dramatically cut > down on the number of rules needed to secure certain filetypes, and > furthermore it would make it much easier to customize the system. > > For example, let's say I'm developing on some GIT checkouts of X and > installing them in /opt/xorgdev so they don't mess with my working > distribution-provided X install. All I would need to do is ensure > that the immediate subdirectories of /opt/xorgdev/ are labelled > properly with lib_t, bin_t, etc. Then when I drop the suid X binaries > into /opt/xorgdev/bin by hand and restorecon them, the pre-existing > transition rule for "X" in a bin_t directory would trigger and set the > context correctly. > > IE: When you put a suid program called "X" in some directory that's in > the default path, it should get special SELinux privileges. That sounds rather dangerous to me. And I really don't see it as being a full replacement for file_contexts (especially as I wouldn't want to try and support full regexes or even generic globs at that level), but only as a way for improving handling of runtime created files. > > We did previously discuss enabling type_transition rules to match on > > last component name, but the initial forays on linux-fsdevel on the > > necessary changes to pass down the name to the right function were shot > > down by the vfs folks IIRC. > > Hmm, that's depressing... Do you have a link to the patches that were > posted? From what I understand some of the hooks for path-based > security eventually got merged, so it's possible the information is > now more readily available than it was before. The audit logs seem to > contain the file or directory name *most* of the time, so it seems > that it is frequently available somehow. I couldn't quickly find the original discussion (maybe Eric has a reference to it), but the issue as I recall was that we'd need to pass the dentry down to the ext3_new_inode() function (and similarly for other filesystem types) so that it can be passed to ext3_init_security() and ultimately to the security_inode_init_security hook when computing the security attribute for a newly allocated inode, and someone (hch?) on -fsdevel opposed that change. One could always try again I suppose. -- Stephen Smalley National Security Agency -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with the words "unsubscribe selinux" without quotes as the message. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-07-27 13:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-07-22 12:22 This is my first patch for systemd Daniel J Walsh 2010-07-22 14:21 ` Stephen Smalley 2010-07-22 15:49 ` Daniel J Walsh 2010-07-22 16:48 ` Stephen Smalley 2010-07-22 19:37 ` Daniel J Walsh 2010-07-23 2:33 ` Kyle Moffett 2010-07-23 20:05 ` Stephen Smalley 2010-07-24 5:13 ` Kyle Moffett 2010-07-27 13:24 ` Stephen Smalley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.