From f621b8d77311a076498516864685e9aba371d00f Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 20 Sep 2021 12:29:45 +0100 Subject: [PATCH 1/4] socket-util: const-ify iov parameter in send_one_fd_iov_sa --- src/basic/socket-util.c | 4 ++-- src/basic/socket-util.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 9fc89a5f6e..b5defd10f6 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -921,7 +921,7 @@ int getpeergroups(int fd, gid_t **ret) { ssize_t send_one_fd_iov_sa( int transport_fd, int fd, - struct iovec *iov, size_t iovlen, + const struct iovec *iov, size_t iovlen, const struct sockaddr *sa, socklen_t len, int flags) { @@ -929,7 +929,7 @@ ssize_t send_one_fd_iov_sa( struct msghdr mh = { .msg_name = (struct sockaddr*) sa, .msg_namelen = len, - .msg_iov = iov, + .msg_iov = (struct iovec *)iov, .msg_iovlen = iovlen, }; ssize_t k; diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 0d76eace7d..cb4a92236f 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -154,7 +154,7 @@ int getpeergroups(int fd, gid_t **ret); ssize_t send_one_fd_iov_sa( int transport_fd, int fd, - struct iovec *iov, size_t iovlen, + const struct iovec *iov, size_t iovlen, const struct sockaddr *sa, socklen_t len, int flags); int send_one_fd_sa(int transport_fd, From 6f7ccbcc747e7006eba250a38684d7f7bf52b334 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 17 Sep 2021 12:42:18 +0100 Subject: [PATCH 2/4] selinux: add mac_selinux_create_file_prepare_label helper Allow to use setfscreatecon with a custom label rather than the default --- src/shared/selinux-util.c | 15 +++++++++++++++ src/shared/selinux-util.h | 1 + 2 files changed, 16 insertions(+) diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c index 34e78e6792..ca92a65efc 100644 --- a/src/shared/selinux-util.c +++ b/src/shared/selinux-util.c @@ -562,6 +562,21 @@ int mac_selinux_create_file_prepare(const char *path, mode_t mode) { #endif } +int mac_selinux_create_file_prepare_label(const char *path, const char *label) { +#if HAVE_SELINUX + + if (!label) + return 0; + + if (!mac_selinux_use()) + return 0; + + if (setfscreatecon_raw(label) < 0) + return log_enforcing_errno(errno, "Failed to set specified SELinux security context '%s' for '%s': %m", label, strna(path)); +#endif + return 0; +} + void mac_selinux_create_file_clear(void) { #if HAVE_SELINUX diff --git a/src/shared/selinux-util.h b/src/shared/selinux-util.h index 1095bdef0e..4147a3ad50 100644 --- a/src/shared/selinux-util.h +++ b/src/shared/selinux-util.h @@ -43,6 +43,7 @@ char* mac_selinux_free(char *label); int mac_selinux_create_file_prepare(const char *path, mode_t mode); int mac_selinux_create_file_prepare_at(int dirfd, const char *path, mode_t mode); +int mac_selinux_create_file_prepare_label(const char *path, const char *label); void mac_selinux_create_file_clear(void); int mac_selinux_create_socket_prepare(const char *label); From 447d46879ca2b7c001a3d27d6a89ef05fb77c297 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 16 Sep 2021 12:43:51 +0100 Subject: [PATCH 3/4] portable: use send_one_fd_iov/receive_one_fd_iov instead of custom helpers Will be useful for the next commit switching to multiple IOVs --- src/portable/portable.c | 113 +++++++++------------------------------- 1 file changed, 26 insertions(+), 87 deletions(-) diff --git a/src/portable/portable.c b/src/portable/portable.c index 3f841b9258..8af7fe2aef 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -134,96 +134,23 @@ int portable_metadata_hashmap_to_sorted_array(Hashmap *unit_files, PortableMetad return 0; } -static int send_item( +static int send_one_fd_iov_with_data_fd( int socket_fd, - const char *name, + const struct iovec *iov, + size_t iovlen, int fd) { - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control = {}; - struct iovec iovec; - struct msghdr mh = { - .msg_control = &control, - .msg_controllen = sizeof(control), - .msg_iov = &iovec, - .msg_iovlen = 1, - }; - struct cmsghdr *cmsg; _cleanup_close_ int data_fd = -1; + assert(iov || iovlen == 0); assert(socket_fd >= 0); - assert(name); assert(fd >= 0); data_fd = copy_data_fd(fd); if (data_fd < 0) return data_fd; - cmsg = CMSG_FIRSTHDR(&mh); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - cmsg->cmsg_len = CMSG_LEN(sizeof(int)); - memcpy(CMSG_DATA(cmsg), &data_fd, sizeof(int)); - - iovec = IOVEC_MAKE_STRING(name); - - if (sendmsg(socket_fd, &mh, MSG_NOSIGNAL) < 0) - return -errno; - - return 0; -} - -static int recv_item( - int socket_fd, - char **ret_name, - int *ret_fd) { - - CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(int))) control; - char buffer[PATH_MAX+2]; - struct iovec iov = IOVEC_INIT(buffer, sizeof(buffer)-1); - struct msghdr mh = { - .msg_control = &control, - .msg_controllen = sizeof(control), - .msg_iov = &iov, - .msg_iovlen = 1, - }; - struct cmsghdr *cmsg; - _cleanup_close_ int found_fd = -1; - char *copy; - ssize_t n; - - assert(socket_fd >= 0); - assert(ret_name); - assert(ret_fd); - - n = recvmsg_safe(socket_fd, &mh, MSG_CMSG_CLOEXEC); - if (n < 0) - return (int) n; - - CMSG_FOREACH(cmsg, &mh) { - if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SCM_RIGHTS) { - - if (cmsg->cmsg_len == CMSG_LEN(sizeof(int))) { - assert(found_fd < 0); - found_fd = *(int*) CMSG_DATA(cmsg); - break; - } - - cmsg_close_all(&mh); - return -EIO; - } - } - - buffer[n] = 0; - - copy = strdup(buffer); - if (!copy) - return -ENOMEM; - - *ret_name = copy; - *ret_fd = TAKE_FD(found_fd); - - return 0; + return send_one_fd_iov(socket_fd, data_fd, iov, iovlen, 0); } DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(portable_metadata_hash_ops, char, string_hash_func, string_compare_func, @@ -272,7 +199,11 @@ static int extract_now( path_is_extension ? "extension-release " : "os-release"); else { if (socket_fd >= 0) { - r = send_item(socket_fd, os_release_id, os_release_fd); + struct iovec iov[] = { + IOVEC_MAKE_STRING(os_release_id), + }; + + r = send_one_fd_iov_with_data_fd(socket_fd, iov, ELEMENTSOF(iov), os_release_fd); if (r < 0) return log_debug_errno(r, "Failed to send os-release file: %m"); } @@ -333,7 +264,11 @@ static int extract_now( } if (socket_fd >= 0) { - r = send_item(socket_fd, de->d_name, fd); + struct iovec iov[] = { + IOVEC_MAKE_STRING(de->d_name), + }; + + r = send_one_fd_iov_with_data_fd(socket_fd, iov, ELEMENTSOF(iov), fd); if (r < 0) return log_debug_errno(r, "Failed to send unit metadata to parent: %m"); } @@ -465,23 +400,27 @@ static int portable_extract_by_path( for (;;) { _cleanup_(portable_metadata_unrefp) PortableMetadata *add = NULL; - _cleanup_free_ char *name = NULL; _cleanup_close_ int fd = -1; + char iov_buffer[PATH_MAX + 2]; + struct iovec iov = IOVEC_INIT(iov_buffer, sizeof(iov_buffer)); - r = recv_item(seq[0], &name, &fd); - if (r < 0) - return log_debug_errno(r, "Failed to receive item: %m"); + ssize_t n = receive_one_fd_iov(seq[0], &iov, 1, 0, &fd); + if (n == -EIO) + break; + if (n < 0) + return log_debug_errno(n, "Failed to receive item: %m"); + iov_buffer[n] = 0; /* We can't really distinguish a zero-length datagram without any fds from EOF (both are signalled the * same way by recvmsg()). Hence, accept either as end notification. */ - if (isempty(name) && fd < 0) + if (isempty(iov_buffer) && fd < 0) break; - if (isempty(name) || fd < 0) + if (isempty(iov_buffer) || fd < 0) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid item sent from child."); - add = portable_metadata_new(name, path, fd); + add = portable_metadata_new(iov_buffer, path, fd); if (!add) return -ENOMEM; fd = -1; From 409f4cf1f2bc797bb6a73eab310b707308a8352e Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 16 Sep 2021 12:47:42 +0100 Subject: [PATCH 4/4] portable: copy SELinux label when extracting units from images Units are copied out via sendmsg datafd from images, but that means the SELinux labels get lost in transit. Extract them and copy them over. Given recvmsg cannot use multiple IOV transparently when the sizes are variable, use a '\0' as a separator between the filename and the label. --- meson.build | 2 +- src/portable/portable.c | 46 ++++++++++++++++++++++++++++++++++++----- src/portable/portable.h | 1 + 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/meson.build b/meson.build index 8d5a1055fe..4c4e9512ea 100644 --- a/meson.build +++ b/meson.build @@ -2291,7 +2291,7 @@ if conf.get('ENABLE_PORTABLED') == 1 systemd_portabled_sources, include_directories : includes, link_with : [libshared], - dependencies : [threads], + dependencies : [threads, libselinux], install_rpath : rootlibexecdir, install : true, install_dir : rootlibexecdir) diff --git a/src/portable/portable.c b/src/portable/portable.c index 8af7fe2aef..4daafea3d3 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -28,6 +28,7 @@ #include "path-lookup.h" #include "portable.h" #include "process-util.h" +#include "selinux-util.h" #include "set.h" #include "signal-util.h" #include "socket-util.h" @@ -78,7 +79,7 @@ static bool unit_match(const char *unit, char **matches) { return false; } -static PortableMetadata *portable_metadata_new(const char *name, const char *path, int fd) { +static PortableMetadata *portable_metadata_new(const char *name, const char *path, const char *selinux_label, int fd) { PortableMetadata *m; m = malloc0(offsetof(PortableMetadata, name) + strlen(name) + 1); @@ -92,6 +93,15 @@ static PortableMetadata *portable_metadata_new(const char *name, const char *pat return mfree(m); } + /* The metadata file might have SELinux labels, we need to carry them and reapply them */ + if (!isempty(selinux_label)) { + m->selinux_label = strdup(selinux_label); + if (!m->selinux_label) { + free(m->image_path); + return mfree(m); + } + } + strcpy(m->name, name); m->fd = fd; @@ -105,6 +115,7 @@ PortableMetadata *portable_metadata_unref(PortableMetadata *i) { safe_close(i->fd); free(i->source); free(i->image_path); + free(i->selinux_label); return mfree(i); } @@ -201,6 +212,7 @@ static int extract_now( if (socket_fd >= 0) { struct iovec iov[] = { IOVEC_MAKE_STRING(os_release_id), + IOVEC_MAKE((char *)"\0", sizeof(char)), }; r = send_one_fd_iov_with_data_fd(socket_fd, iov, ELEMENTSOF(iov), os_release_fd); @@ -209,7 +221,7 @@ static int extract_now( } if (ret_os_release) { - os_release = portable_metadata_new(os_release_id, NULL, os_release_fd); + os_release = portable_metadata_new(os_release_id, NULL, NULL, os_release_fd); if (!os_release) return -ENOMEM; @@ -264,8 +276,19 @@ static int extract_now( } if (socket_fd >= 0) { + _cleanup_(mac_selinux_freep) char *con = NULL; +#if HAVE_SELINUX + /* The units will be copied on the host's filesystem, so if they had a SELinux label + * we have to preserve it. Copy it out so that it can be applied later. */ + + r = fgetfilecon_raw(fd, &con); + if (r < 0 && errno != ENODATA) + log_debug_errno(errno, "Failed to get SELinux file context from '%s', ignoring: %m", de->d_name); +#endif struct iovec iov[] = { IOVEC_MAKE_STRING(de->d_name), + IOVEC_MAKE((char *)"\0", sizeof(char)), + IOVEC_MAKE_STRING(strempty(con)), }; r = send_one_fd_iov_with_data_fd(socket_fd, iov, ELEMENTSOF(iov), fd); @@ -273,7 +296,7 @@ static int extract_now( return log_debug_errno(r, "Failed to send unit metadata to parent: %m"); } - m = portable_metadata_new(de->d_name, NULL, fd); + m = portable_metadata_new(de->d_name, NULL, NULL, fd); if (!m) return -ENOMEM; fd = -1; @@ -401,7 +424,11 @@ static int portable_extract_by_path( for (;;) { _cleanup_(portable_metadata_unrefp) PortableMetadata *add = NULL; _cleanup_close_ int fd = -1; - char iov_buffer[PATH_MAX + 2]; + /* We use NAME_MAX space for the SELinux label here. The kernel currently enforces no limit, but + * according to suggestions from the SELinux people this will change and it will probably be + * identical to NAME_MAX. For now we use that, but this should be updated one day when the final + * limit is known. */ + char iov_buffer[PATH_MAX + NAME_MAX + 2]; struct iovec iov = IOVEC_INIT(iov_buffer, sizeof(iov_buffer)); ssize_t n = receive_one_fd_iov(seq[0], &iov, 1, 0, &fd); @@ -420,7 +447,13 @@ static int portable_extract_by_path( return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid item sent from child."); - add = portable_metadata_new(iov_buffer, path, fd); + /* Given recvmsg cannot be used with multiple io vectors if you don't know the size in advance, + * use a marker to separate the name and the optional SELinux context. */ + char *selinux_label = memchr(iov_buffer, 0, n); + assert(selinux_label); + selinux_label++; + + add = portable_metadata_new(iov_buffer, path, selinux_label, fd); if (!add) return -ENOMEM; fd = -1; @@ -1065,7 +1098,10 @@ static int attach_unit_file( _cleanup_(unlink_and_freep) char *tmp = NULL; _cleanup_close_ int fd = -1; + (void) mac_selinux_create_file_prepare_label(path, m->selinux_label); + fd = open_tmpfile_linkable(path, O_WRONLY|O_CLOEXEC, &tmp); + mac_selinux_create_file_clear(); /* Clear immediately in case of errors */ if (fd < 0) return log_debug_errno(fd, "Failed to create unit file '%s': %m", path); diff --git a/src/portable/portable.h b/src/portable/portable.h index 94144287ae..077ab3333f 100644 --- a/src/portable/portable.h +++ b/src/portable/portable.h @@ -12,6 +12,7 @@ typedef struct PortableMetadata { int fd; char *source; char *image_path; + char *selinux_label; char name[]; } PortableMetadata;