[pacman-dev] [PATCH] Add optional sandboxing when downloading files
[re-sending because my initial e-mail was rejected by spam filters, sorry!] Hi all, I would like to get some feedback on a optional sandboxing feature I would like to implement in pacman. The gist of it is to use a separate process for some sensitive operations, like downloading files, and to restrict the privileges of that process to reduce the risk of an exploitable bug turning into a code execution with full root privileges. I'm not too worried about the pacman code itself, but downloading files over the internet involves parsing HTTP payloads, as well as parsing X.509 certificates and doing cryptographic operations when HTTPS is used, which is more error-prone. For what it's worth, Debian's APT has similar options hidden behind the APT::Sandbox::Seccomp and APT::Sandbox::User parameters. The attached patch is clearly not in a finished state, but provides more details about the kind of changes needed to support this new feature. In a nutshell, what it currently does is that if the UseSandbox option is not set nothing changes. Otherwise internal and external downloads trigger the spawning of a new process, via fork(), whose result is communicated to the main process via a pipe, as is already done today for external downloads. Then: - If available, PR_SET_NO_NEW_PRIVS is called to ensure that the downloader will not be able to gain new privileges in the future, for example by executing a set-uid program. - If pacman has been built with libseccomp support and the kernel supports it, a list of system calls that are clearly of no use to a regular process downloading files is denied via a seccomp filter. That list is currently hard-coded but it would make more sense to make it configurable. - If the SandboxUser option is set to a valid user, the downloader process switches to that user via setgid, setgroups and setuid, dropping root privileges. This part requires making the download folder owned by that user. - If pacman has been built with libcap support and the downloader process still has privileged capabilities at this point, these are dropped. - If pacman has been built with landlock support (requires an updated linux-api-headers, >= 5.13) and the kernel supports it, the whole filesystem is set read-only with the exception of /tmp and DBPath. The list of writable paths is currently hard-coded but it would make sense to make it configurable as well. Any feedback will be welcome. Please be aware that I tried to keep the new code in line with the coding style and contributing guidelines but I'm not familiar with pacman's source code so I'm sure I did not do everything the right way, and that this code can be improved. What I would like to know first is whether there is any interest in that feature, then to get feedback about the "big picture" part of the design. If there is indeed interest and my approach is somewhat sane, then I can work on making the code less ugly, write the much needed documentation for the new feature and related options, and improve the logging of errors. Thanks! Remi Signed-off-by: Remi Gacogne <rgacogne@archlinux.org> --- lib/libalpm/alpm.h | 10 ++ lib/libalpm/alpm_sandbox.c | 341 +++++++++++++++++++++++++++++++++++++ lib/libalpm/alpm_sandbox.h | 31 ++++ lib/libalpm/dload.c | 83 ++++++++- lib/libalpm/handle.c | 20 +++ lib/libalpm/handle.h | 2 + lib/libalpm/meson.build | 1 + meson.build | 11 +- src/pacman/conf.c | 23 ++- src/pacman/conf.h | 2 + src/pacman/pacman-conf.c | 6 + 11 files changed, 526 insertions(+), 4 deletions(-) create mode 100644 lib/libalpm/alpm_sandbox.c create mode 100644 lib/libalpm/alpm_sandbox.h diff --git lib/libalpm/alpm.h lib/libalpm/alpm.h index a5f4a6ae..f414d811 100644 --- lib/libalpm/alpm.h +++ lib/libalpm/alpm.h @@ -1837,6 +1837,11 @@ int alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir); /* End of gpdir accessors */ /** @} */ +/** Sets the user to switch to for sensitive operations like downloading a file. + * @param handle the context handle + * @param sandboxuser the user to set + */ +int alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser); /** @name Accessors for use syslog * @@ -1859,6 +1864,11 @@ int alpm_option_set_usesyslog(alpm_handle_t *handle, int usesyslog); /* End of usesyslog accessors */ /** @} */ +/** Sets whether to use sandboxing for sensitive operations like downloading a file (0 is FALSE, TRUE otherwise). + * @param handle the context handle + * @param usesandbox whether to use the sandboxing (0 is FALSE, TRUE otherwise) + */ +int alpm_option_set_usesandbox(alpm_handle_t *handle, int usesandbox); /** @name Accessors to the list of no-upgrade files. * These functions modify the list of files which should diff --git lib/libalpm/alpm_sandbox.c lib/libalpm/alpm_sandbox.c new file mode 100644 index 00000000..29f46d58 --- /dev/null +++ lib/libalpm/alpm_sandbox.c @@ -0,0 +1,341 @@ +/* + * sandbox.c + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <errno.h> +#include <fcntl.h> +#include <grp.h> +#include <pwd.h> +#include <sys/capability.h> +#include <sys/syscall.h> +#include <sys/types.h> +#include <unistd.h> + +#ifdef HAVE_LINUX_LANDLOCK_H +# include <linux/landlock.h> +# include <sys/prctl.h> +# include <sys/syscall.h> +#endif /* HAVE_LINUX_LANDLOCK_H */ + +#ifdef HAVE_LIBSECCOMP +# include <seccomp.h> +#endif /* HAVE_LIBSECCOMP */ + +#include "alpm_sandbox.h" + +#ifdef HAVE_LINUX_LANDLOCK_H +#ifndef landlock_create_ruleset +static inline int landlock_create_ruleset(const struct landlock_ruleset_attr *const attr, + const size_t size, const __u32 flags) +{ + return syscall(__NR_landlock_create_ruleset, attr, size, flags); +} +#endif /* landlock_create_ruleset */ + +#ifndef landlock_add_rule +static inline int landlock_add_rule(const int ruleset_fd, + const enum landlock_rule_type rule_type, + const void *const rule_attr, const __u32 flags) +{ + return syscall(__NR_landlock_add_rule, ruleset_fd, rule_type, rule_attr, flags); +} +#endif /* landlock_add_rule */ + +#ifndef landlock_restrict_self +static inline int landlock_restrict_self(const int ruleset_fd, const __u32 flags) +{ + return syscall(__NR_landlock_restrict_self, ruleset_fd, flags); +} +#endif /* landlock_restrict_self */ + +#define _LANDLOCK_ACCESS_FS_WRITE ( \ + LANDLOCK_ACCESS_FS_WRITE_FILE | \ + LANDLOCK_ACCESS_FS_REMOVE_DIR | \ + LANDLOCK_ACCESS_FS_REMOVE_FILE | \ + LANDLOCK_ACCESS_FS_MAKE_CHAR | \ + LANDLOCK_ACCESS_FS_MAKE_DIR | \ + LANDLOCK_ACCESS_FS_MAKE_REG | \ + LANDLOCK_ACCESS_FS_MAKE_SOCK | \ + LANDLOCK_ACCESS_FS_MAKE_FIFO | \ + LANDLOCK_ACCESS_FS_MAKE_BLOCK | \ + LANDLOCK_ACCESS_FS_MAKE_SYM) + +#define _LANDLOCK_ACCESS_FS_READ ( \ + LANDLOCK_ACCESS_FS_READ_FILE | \ + LANDLOCK_ACCESS_FS_READ_DIR) + +static int sandbox_write_only_beneath_cwd(void) +{ + const struct landlock_ruleset_attr ruleset_attr = { + .handled_access_fs = \ + _LANDLOCK_ACCESS_FS_READ | \ + _LANDLOCK_ACCESS_FS_WRITE | \ + LANDLOCK_ACCESS_FS_EXECUTE, + }; + struct landlock_path_beneath_attr path_beneath = { + .allowed_access = _LANDLOCK_ACCESS_FS_WRITE, + }; + int result = 0; + int ruleset_fd; + + ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); + if(ruleset_fd < 0) { + return ruleset_fd; + } + + /* allow / as read-only */ + path_beneath.parent_fd = open("/", O_PATH | O_CLOEXEC | O_DIRECTORY); + path_beneath.allowed_access = _LANDLOCK_ACCESS_FS_READ | LANDLOCK_ACCESS_FS_EXECUTE; + + if(landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, &path_beneath, 0)) { + result = errno; + } + + close(path_beneath.parent_fd); + + if(result == 0) { + /* allow the current working directory as read-write */ + path_beneath.parent_fd = open(".", O_PATH | O_CLOEXEC | O_DIRECTORY); + path_beneath.allowed_access = _LANDLOCK_ACCESS_FS_READ | _LANDLOCK_ACCESS_FS_WRITE | LANDLOCK_ACCESS_FS_EXECUTE; + + if(!landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, &path_beneath, 0)) { + if(landlock_restrict_self(ruleset_fd, 0)) { + result = errno; + } + } else { + result = errno; + } + + close(path_beneath.parent_fd); + + if(result == 0) { + /* allow /tmp as well */ + path_beneath.parent_fd = open("/tmp", O_PATH | O_CLOEXEC | O_DIRECTORY); + path_beneath.allowed_access = _LANDLOCK_ACCESS_FS_READ | _LANDLOCK_ACCESS_FS_WRITE | LANDLOCK_ACCESS_FS_EXECUTE; + + if(!landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, &path_beneath, 0)) { + if(landlock_restrict_self(ruleset_fd, 0)) { + result = errno; + } + } else { + result = errno; + } + + close(path_beneath.parent_fd); + } + } + + close(ruleset_fd); + return result; +} +#endif /* HAVE_LINUX_LANDLOCK_H */ + +#ifdef HAVE_LIBSECCOMP + +static int sandbox_filter_syscalls(void) +{ + int ret = 0; + /* see https://docs.docker.com/engine/security/seccomp/ for inspiration, + as well as systemd's src/shared/seccomp-util.c */ + const char* denied_syscalls[] = { + /* kernel modules */ + "delete_module", + "finit_module", + "init_module", + /* mount */ + "chroot", + "fsconfig", + "fsmount", + "fsopen", + "fspick", + "mount", + "move_mount", + "open_tree", + "pivot_root", + "umount", + "umount2", + /* keyring */ + "add_key", + "keyctl", + "request_key", + /* CPU emulation */ + "modify_ldt", + "subpage_prot", + "switch_endian", + "vm86", + "vm86old", + /* debug */ + "kcmp", + "lookup_dcookie", + "perf_event_open", + "ptrace", + "rtas", + "sys_debug_setcontext", + /* set clock */ + "adjtimex", + "clock_adjtime", + "clock_adjtime64", + "clock_settime", + "clock_settime64", + "settimeofday", + /* raw IO */ + "ioperm", + "iopl", + "pciconfig_iobase", + "pciconfig_read", + "pciconfig_write", + /* kexec */ + "kexec_file_load", + "kexec_load", + /* reboot */ + "reboot", + /* privileged */ + "acct", + "bpf", + "personality", + /* obsolete */ + "_sysctl", + "afs_syscall", + "bdflush", + "break", + "create_module", + "ftime", + "get_kernel_syms", + "getpmsg", + "gtty", + "idle", + "lock", + "mpx", + "prof", + "profil", + "putpmsg", + "query_module", + "security", + "sgetmask", + "ssetmask", + "stime", + "stty", + "sysfs", + "tuxcall", + "ulimit", + "uselib", + "ustat", + "vserver", + /* swap */ + "swapon", + "swapoff", + }; + /* allow all syscalls that are not listed */ + size_t idx; + scmp_filter_ctx ctx = seccomp_init(SCMP_ACT_ALLOW); + if(ctx == NULL) { + return errno; + } + + for(idx = 0; idx < sizeof(denied_syscalls) / sizeof(*denied_syscalls); idx++) { + int syscall = seccomp_syscall_resolve_name(denied_syscalls[idx]); + if(syscall != __NR_SCMP_ERROR) { + if(seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), syscall, 0) != 0) { + /* Do not fail but log something like: _("Error blocking syscall %s\n"), denied_syscalls[idx]); */ + } + } + } + + if(seccomp_load(ctx) != 0) { + ret = errno; + } + + seccomp_release(ctx); + return ret; +} +#endif /* HAVE_LIBSECCOMP */ + +static int switch_to_user(const char *user) +{ + struct passwd const *pw = NULL; + if(getuid() != 0) { + return 1; + } + pw = getpwnam(user); + if(pw == NULL) { + return errno; + } + if(setgid(pw->pw_gid) != 0) { + return errno; + } + if(setgroups(0, NULL)) { + return errno; + } + if(setuid(pw->pw_uid) != 0) { + return errno; + } + return 0; +} + +/* check exported library symbols with: nm -C -D <lib> */ +#define SYMEXPORT __attribute__((visibility("default"))) + +int SYMEXPORT alpm_sandbox_child(const char* sandboxuser) +{ + int result = 0; +#ifdef HAVE_LINUX_LANDLOCK_H + int ret = 0; +#endif/* HAVE_LINUX_LANDLOCK_H */ + + /* make sure that we cannot gain more privileges later */ + if(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) != 0) { + result = errno; + } + +#ifdef HAVE_LIBSECCOMP + result = sandbox_filter_syscalls(); +#endif /* HAVE_LIBSECCOMP */ + + if(sandboxuser != NULL) { + result = switch_to_user(sandboxuser); + } + +#ifdef HAVE_LIBCAP + /* we might have some capabilities remaining, + * especially if sandboxuser is not set */ + cap_t caps = cap_get_proc(); + cap_clear(caps); + if(cap_set_mode(CAP_MODE_NOPRIV) != 0) { + cap_free(caps); + if(result == 0) { + result = errno; + } + } + if(cap_set_proc(caps) != 0) { + cap_free(caps); + if(result == 0) { + result = errno; + } + } + cap_free(caps); +#endif /* HAVE_LIBCAP */ + +#ifdef HAVE_LINUX_LANDLOCK_H + ret = sandbox_write_only_beneath_cwd(); + if(result == 0) { + result = ret; + } +#endif /* HAVE_LINUX_LANDLOCK_H */ + + return result; +} diff --git lib/libalpm/alpm_sandbox.h lib/libalpm/alpm_sandbox.h new file mode 100644 index 00000000..47611575 --- /dev/null +++ lib/libalpm/alpm_sandbox.h @@ -0,0 +1,31 @@ +/* + * sandbox.h + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef ALPM_SANDBOX_H +#define ALPM_SANDBOX_H + +#ifdef __cplusplus +extern "C" { +#endif + +int alpm_sandbox_child(const char *sandboxuser); + +#ifdef __cplusplus +} +#endif +#endif /* ALPM_SANDBOX_H */ diff --git lib/libalpm/dload.c lib/libalpm/dload.c index ca6be7b6..6bb06b4c 100644 --- lib/libalpm/dload.c +++ lib/libalpm/dload.c @@ -28,6 +28,7 @@ #include <sys/time.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/wait.h> #include <signal.h> #ifdef HAVE_NETINET_IN_H @@ -44,6 +45,7 @@ /* libalpm */ #include "dload.h" #include "alpm_list.h" +#include "alpm_sandbox.h" #include "alpm.h" #include "log.h" #include "util.h" @@ -898,6 +900,81 @@ static int curl_download_internal(alpm_handle_t *handle, #endif +static int curl_download_internal_sandboxed(alpm_handle_t *handle, + alpm_list_t *payloads /* struct dload_payload */, + const char *localpath) +{ + int pid, err = 0, ret = -1, err_fd[2]; + sigset_t oldblock; + struct sigaction sa_ign = { .sa_handler = SIG_IGN }, oldint, oldquit; + + if(pipe(err_fd) != 0) { + return -1; + } + + sigaction(SIGINT, &sa_ign, &oldint); + sigaction(SIGQUIT, &sa_ign, &oldquit); + sigaddset(&sa_ign.sa_mask, SIGCHLD); + sigprocmask(SIG_BLOCK, &sa_ign.sa_mask, &oldblock); + + pid = fork(); + + /* child */ + if(pid == 0) { + close(err_fd[0]); + fcntl(err_fd[1], F_SETFD, FD_CLOEXEC); + + /* restore signal handling for the child to inherit */ + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + /* cwd to the download directory */ + ret = chdir(localpath); + if(ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("could not chdir to download directory %s\n"), localpath); + ret = -1; + } else { + ret = alpm_sandbox_child(handle->sandboxuser); + if (ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("sandboxing failed!\n")); + } + + ret = curl_download_internal(handle, payloads, localpath); + } + + /* pass the result back to the parent */ + while(write(err_fd[1], &ret, sizeof(ret)) == -1 && errno == EINTR); + _Exit(ret < 0 ? 1 : 0); + } + + /* parent */ + close(err_fd[1]); + + if(pid != -1) { + int wret; + while((wret = waitpid(pid, &ret, 0)) == -1 && errno == EINTR); + if(wret > 0) { + while(read(err_fd[0], &ret, sizeof(ret)) == -1 && errno == EINTR); + } + } else { + /* fork failed, make sure errno is preserved after cleanup */ + err = errno; + } + + close(err_fd[0]); + + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + if(err) { + errno = err; + ret = -1; + } + return ret; +} + /* Returns -1 if an error happened for a required file * Returns 0 if a payload was actually downloaded * Returns 1 if no files were downloaded and all errors were non-fatal @@ -908,7 +985,11 @@ int _alpm_download(alpm_handle_t *handle, { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(handle, payloads, localpath); + if(handle->usesandbox) { + return curl_download_internal_sandboxed(handle, payloads, localpath); + } else { + return curl_download_internal(handle, payloads, localpath); + } #else RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1); #endif diff --git lib/libalpm/handle.c lib/libalpm/handle.c index e6b683cb..10ae44b2 100644 --- lib/libalpm/handle.c +++ lib/libalpm/handle.c @@ -573,6 +573,19 @@ int SYMEXPORT alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir) return 0; } +int SYMEXPORT alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser) +{ + CHECK_HANDLE(handle, return -1); + if(handle->sandboxuser) { + FREE(handle->sandboxuser); + } + + STRDUP(handle->sandboxuser, sandboxuser, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + + _alpm_log(handle, ALPM_LOG_DEBUG, "option 'sandboxuser' = %s\n", handle->sandboxuser); + return 0; +} + int SYMEXPORT alpm_option_set_usesyslog(alpm_handle_t *handle, int usesyslog) { CHECK_HANDLE(handle, return -1); @@ -580,6 +593,13 @@ int SYMEXPORT alpm_option_set_usesyslog(alpm_handle_t *handle, int usesyslog) return 0; } +int SYMEXPORT alpm_option_set_usesandbox(alpm_handle_t *handle, int usesandbox) +{ + CHECK_HANDLE(handle, return -1); + handle->usesandbox = usesandbox; + return 0; +} + static int _alpm_option_strlist_add(alpm_handle_t *handle, alpm_list_t **list, const char *str) { char *dup; diff --git lib/libalpm/handle.h lib/libalpm/handle.h index b1526c67..43550833 100644 --- lib/libalpm/handle.h +++ lib/libalpm/handle.h @@ -90,6 +90,7 @@ struct __alpm_handle_t { char *logfile; /* Name of the log file */ char *lockfile; /* Name of the lock file */ char *gpgdir; /* Directory where GnuPG files are stored */ + char *sandboxuser; /* User to switch to for sensitive operations like downloading files */ alpm_list_t *cachedirs; /* Paths to pacman cache directories */ alpm_list_t *hookdirs; /* Paths to hook directories */ alpm_list_t *overwrite_files; /* Paths that may be overwritten */ @@ -104,6 +105,7 @@ struct __alpm_handle_t { /* options */ alpm_list_t *architectures; /* Architectures of packages we should allow */ int usesyslog; /* Use syslog instead of logfile? */ /* TODO move to frontend */ + int usesandbox; /* Whether to enable sandboxing for sensitive operations like downloading files */ int checkspace; /* Check disk space before installing */ char *dbext; /* Sync DB extension */ int siglevel; /* Default signature verification level */ diff --git lib/libalpm/meson.build lib/libalpm/meson.build index 607e91a3..c36ae516 100644 --- lib/libalpm/meson.build +++ lib/libalpm/meson.build @@ -2,6 +2,7 @@ libalpm_sources = files(''' add.h add.c alpm.h alpm.c alpm_list.h alpm_list.c + alpm_sandbox.h alpm_sandbox.c backup.h backup.c base64.h base64.c be_local.c diff --git meson.build meson.build index 26c92b8e..88f0e3a1 100644 --- meson.build +++ meson.build @@ -91,6 +91,10 @@ libarchive = dependency('libarchive', version : '>=3.0.0', static : get_option('buildstatic')) +libcap = dependency('libcap', + static : get_option('buildstatic')) +conf.set('HAVE_LIBCAP', libcap.found()) + libcurl = dependency('libcurl', version : '>=7.55.0', required : get_option('curl'), @@ -120,7 +124,12 @@ else error('unhandled crypto value @0@'.format(want_crypto)) endif +libseccomp = dependency('libseccomp', + static : get_option('buildstatic')) +conf.set('HAVE_LIBSECCOMP', libseccomp.found()) + foreach header : [ + 'linux/landlock.h', 'mntent.h', 'sys/mnttab.h', 'sys/mount.h', @@ -305,7 +314,7 @@ libcommon = static_library( gnu_symbol_visibility : 'hidden', install : false) -alpm_deps = [crypto_provider, libarchive, libcurl, libintl, gpgme] +alpm_deps = [crypto_provider, libarchive, libcap, libcurl, libintl, libseccomp, gpgme] libalpm_a = static_library( 'alpm_objlib', diff --git src/pacman/conf.c src/pacman/conf.c index 12fee64c..9480a492 100644 --- src/pacman/conf.c +++ src/pacman/conf.c @@ -33,6 +33,8 @@ #include <unistd.h> #include <signal.h> +#include <alpm_sandbox.h> + /* pacman */ #include "conf.h" #include "ini.h" @@ -215,7 +217,7 @@ static char *get_tempfile(const char *path, const char *filename) * - not thread-safe * - errno may be set by fork(), pipe(), or execvp() */ -static int systemvp(const char *file, char *const argv[]) +static int systemvp(const char *file, char *const argv[], bool sandbox, const char *sandboxuser) { int pid, err = 0, ret = -1, err_fd[2]; sigset_t oldblock; @@ -242,6 +244,13 @@ static int systemvp(const char *file, char *const argv[]) sigaction(SIGQUIT, &oldquit, NULL); sigprocmask(SIG_SETMASK, &oldblock, NULL); + if (sandbox) { + ret = alpm_sandbox_child(sandboxuser); + if (ret != 0) { + pm_printf(ALPM_LOG_WARNING, _("sandboxing failed!\n")); + } + } + execvp(file, argv); /* execvp failed, pass the error back to the parent */ @@ -352,7 +361,7 @@ static int download_with_xfercommand(void *ctx, const char *url, free(cmd); } } - retval = systemvp(argv[0], (char**)argv); + retval = systemvp(argv[0], (char**)argv, config->usesandbox, config->sandboxuser); if(retval == -1) { pm_printf(ALPM_LOG_WARNING, _("running XferCommand: fork failed!\n")); @@ -601,6 +610,9 @@ static int _parse_options(const char *key, char *value, if(strcmp(key, "UseSyslog") == 0) { config->usesyslog = 1; pm_printf(ALPM_LOG_DEBUG, "config: usesyslog\n"); + } else if(strcmp(key, "UseSandbox") == 0) { + config->usesandbox = 1; + pm_printf(ALPM_LOG_DEBUG, "config: usesandbox\n"); } else if(strcmp(key, "ILoveCandy") == 0) { config->chomp = 1; pm_printf(ALPM_LOG_DEBUG, "config: chomp\n"); @@ -668,6 +680,11 @@ static int _parse_options(const char *key, char *value, config->logfile = strdup(value); pm_printf(ALPM_LOG_DEBUG, "config: logfile: %s\n", value); } + } else if(strcmp(key, "SandboxUser") == 0) { + if(!config->sandboxuser) { + config->sandboxuser = strdup(value); + pm_printf(ALPM_LOG_DEBUG, "config: sandboxuser: %s\n", value); + } } else if(strcmp(key, "XferCommand") == 0) { char **c; if((config->xfercommand_argv = wordsplit(value)) == NULL) { @@ -904,6 +921,8 @@ static int setup_libalpm(void) alpm_option_set_architectures(handle, config->architectures); alpm_option_set_checkspace(handle, config->checkspace); alpm_option_set_usesyslog(handle, config->usesyslog); + alpm_option_set_usesandbox(handle, config->usesandbox); + alpm_option_set_sandboxuser(handle, config->sandboxuser); alpm_option_set_ignorepkgs(handle, config->ignorepkg); alpm_option_set_ignoregroups(handle, config->ignoregrp); diff --git src/pacman/conf.h src/pacman/conf.h index 04350d39..8fced284 100644 --- src/pacman/conf.h +++ src/pacman/conf.h @@ -55,6 +55,7 @@ typedef struct __config_t { unsigned short print; unsigned short checkspace; unsigned short usesyslog; + unsigned short usesandbox; unsigned short color; unsigned short disable_dl_timeout; char *print_format; @@ -67,6 +68,7 @@ typedef struct __config_t { char *logfile; char *gpgdir; char *sysroot; + char *sandboxuser; alpm_list_t *hookdirs; alpm_list_t *cachedirs; alpm_list_t *architectures; diff --git src/pacman/pacman-conf.c src/pacman/pacman-conf.c index 600f1622..6da27937 100644 --- src/pacman/pacman-conf.c +++ src/pacman/pacman-conf.c @@ -251,6 +251,7 @@ static void dump_config(void) show_list_str("HookDir", config->hookdirs); show_str("GPGDir", config->gpgdir); show_str("LogFile", config->logfile); + show_str("SandboxUser", config->sandboxuser); show_list_str("HoldPkg", config->holdpkg); show_list_str("IgnorePkg", config->ignorepkg); @@ -268,6 +269,7 @@ static void dump_config(void) show_bool("DisableDownloadTimeout", config->disable_dl_timeout); show_bool("ILoveCandy", config->chomp); show_bool("NoProgressBar", config->noprogressbar); + show_bool("UseSandbox", config->usesandbox); show_int("ParallelDownloads", config->parallel_downloads); @@ -349,6 +351,8 @@ static int list_directives(void) show_str("GPGDir", config->gpgdir); } else if(strcasecmp(i->data, "LogFile") == 0) { show_str("LogFile", config->logfile); + } else if(strcasecmp(i->data, "SandboxUser") == 0) { + show_str("SandboxUser", config->sandboxuser); } else if(strcasecmp(i->data, "HoldPkg") == 0) { show_list_str("HoldPkg", config->holdpkg); @@ -369,6 +373,8 @@ static int list_directives(void) } else if(strcasecmp(i->data, "UseSyslog") == 0) { show_bool("UseSyslog", config->usesyslog); + } else if(strcasecmp(i->data, "UseSandbox") == 0) { + show_bool("UseSandbox", config->usesandbox); } else if(strcasecmp(i->data, "Color") == 0) { show_bool("Color", config->color); } else if(strcasecmp(i->data, "CheckSpace") == 0) { -- 2.33.0
On 08/30/21 at 11:37am, Remi Gacogne wrote:
[re-sending because my initial e-mail was rejected by spam filters, sorry!]
Hi all,
I would like to get some feedback on a optional sandboxing feature I would like to implement in pacman. The gist of it is to use a separate process for some sensitive operations, like downloading files, and to restrict the privileges of that process to reduce the risk of an exploitable bug turning into a code execution with full root privileges. I'm not too worried about the pacman code itself, but downloading files over the internet involves parsing HTTP payloads, as well as parsing X.509 certificates and doing cryptographic operations when HTTPS is used, which is more error-prone. For what it's worth, Debian's APT has similar options hidden behind the APT::Sandbox::Seccomp and APT::Sandbox::User parameters.
The attached patch is clearly not in a finished state, but provides more details about the kind of changes needed to support this new feature. In a nutshell, what it currently does is that if the UseSandbox option is not set nothing changes. Otherwise internal and external downloads trigger the spawning of a new process, via fork(), whose result is communicated to the main process via a pipe, as is already done today for external downloads. Then: - If available, PR_SET_NO_NEW_PRIVS is called to ensure that the downloader will not be able to gain new privileges in the future, for example by executing a set-uid program. - If pacman has been built with libseccomp support and the kernel supports it, a list of system calls that are clearly of no use to a regular process downloading files is denied via a seccomp filter. That list is currently hard-coded but it would make more sense to make it configurable. - If the SandboxUser option is set to a valid user, the downloader process switches to that user via setgid, setgroups and setuid, dropping root privileges. This part requires making the download folder owned by that user. - If pacman has been built with libcap support and the downloader process still has privileged capabilities at this point, these are dropped. - If pacman has been built with landlock support (requires an updated linux-api-headers, >= 5.13) and the kernel supports it, the whole filesystem is set read-only with the exception of /tmp and DBPath. The list of writable paths is currently hard-coded but it would make sense to make it configurable as well.
Any feedback will be welcome. Please be aware that I tried to keep the new code in line with the coding style and contributing guidelines but I'm not familiar with pacman's source code so I'm sure I did not do everything the right way, and that this code can be improved. What I would like to know first is whether there is any interest in that feature, then to get feedback about the "big picture" part of the design. If there is indeed interest and my approach is somewhat sane, then I can work on making the code less ugly, write the much needed documentation for the new feature and related options, and improve the logging of errors.
Thanks!
Remi
Signed-off-by: Remi Gacogne <rgacogne@archlinux.org> --- lib/libalpm/alpm.h | 10 ++ lib/libalpm/alpm_sandbox.c | 341 +++++++++++++++++++++++++++++++++++++ lib/libalpm/alpm_sandbox.h | 31 ++++ lib/libalpm/dload.c | 83 ++++++++- lib/libalpm/handle.c | 20 +++ lib/libalpm/handle.h | 2 + lib/libalpm/meson.build | 1 + meson.build | 11 +- src/pacman/conf.c | 23 ++- src/pacman/conf.h | 2 + src/pacman/pacman-conf.c | 6 + 11 files changed, 526 insertions(+), 4 deletions(-) create mode 100644 lib/libalpm/alpm_sandbox.c create mode 100644 lib/libalpm/alpm_sandbox.h
This is a lot. Let's focus on the portable user switching first; if that gets merged we can look at adding the extra Linux-specific stuff.
+ if(pid != -1) { + int wret; + while((wret = waitpid(pid, &ret, 0)) == -1 && errno == EINTR); + if(wret > 0) { + while(read(err_fd[0], &ret, sizeof(ret)) == -1 && errno == EINTR); + } + } else { + /* fork failed, make sure errno is preserved after cleanup */ + err = errno; + }
This is just returning an int, is there a need to pass it by pipe instead of just using the _Exit value? I'm also wondering if we could simplify the configuration. We could infer that a sandbox should be used if a sandbox user is set or infer the user from the cache folder being downloaded to. apg
On 3/9/21 11:58 am, Andrew Gregory wrote:
On 08/30/21 at 11:37am, Remi Gacogne wrote:
--- lib/libalpm/alpm.h | 10 ++ lib/libalpm/alpm_sandbox.c | 341 +++++++++++++++++++++++++++++++++++++ lib/libalpm/alpm_sandbox.h | 31 ++++ lib/libalpm/dload.c | 83 ++++++++- lib/libalpm/handle.c | 20 +++ lib/libalpm/handle.h | 2 + lib/libalpm/meson.build | 1 + meson.build | 11 +- src/pacman/conf.c | 23 ++- src/pacman/conf.h | 2 + src/pacman/pacman-conf.c | 6 + 11 files changed, 526 insertions(+), 4 deletions(-) create mode 100644 lib/libalpm/alpm_sandbox.c create mode 100644 lib/libalpm/alpm_sandbox.h
This is a lot. Let's focus on the portable user switching first; if that gets merged we can look at adding the extra Linux-specific stuff.
I was just writing the same thing! Other general comments: Rename alpm_sandbox.c to sandbox.c. We don't need the prefix for a file inside the library. Split out the libseccomp setup to sandbox-linux.c. I realise we mostly support Linux, but this will save this file becoming a mass of #ifdef if other operating systems add something similar. Allan
This second version contains only the portable part of the sandboxing, switching to a different user when a new option, 'SandboxUser', is set. To sum up the changes from the last version: - The non-portable parts (preventing new privileges from being gained, dropping capabilities, syscalls filtering and filesystem protection) are gone for now, I'll propose them via smaller patches if this change is accepted. - The value returned by the sub-process is now passed by the exit status only, instead of using a pipe. - The 'UseSandbox' option has been removed. - The alpm_sandbox.{c,h} files have been renamed to sandbox.{c,h}. - A short description of the 'SandboxUser' option has been added to pacman.conf.5. Cheers, Remi Signed-off-by: Remi Gacogne <rgacogne@archlinux.org> --- doc/pacman.conf.5.asciidoc | 5 ++ lib/libalpm/alpm.h | 5 ++ lib/libalpm/dload.c | 102 ++++++++++++++++++++++++++++++++++++- lib/libalpm/handle.c | 13 +++++ lib/libalpm/handle.h | 1 + lib/libalpm/meson.build | 1 + lib/libalpm/sandbox.c | 63 +++++++++++++++++++++++ lib/libalpm/sandbox.h | 31 +++++++++++ src/pacman/conf.c | 19 ++++++- src/pacman/conf.h | 1 + src/pacman/pacman-conf.c | 3 ++ 11 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 lib/libalpm/sandbox.c create mode 100644 lib/libalpm/sandbox.h diff --git doc/pacman.conf.5.asciidoc doc/pacman.conf.5.asciidoc index 77a3907f..5b20b177 100644 --- doc/pacman.conf.5.asciidoc +++ doc/pacman.conf.5.asciidoc @@ -207,6 +207,11 @@ Options positive integer. If this config option is not set then only one download stream is used (i.e. downloads happen sequentially). +*SandboxUser =* username:: + Specifies the user to switch to for sensitive operations, like downloading + files. That user should exist on the system and have the permissions to + write to the files located in `DBPath`. If this config option is not set + then these operations are done as the user running pacman. Repository Sections ------------------- diff --git lib/libalpm/alpm.h lib/libalpm/alpm.h index a5f4a6ae..5473558a 100644 --- lib/libalpm/alpm.h +++ lib/libalpm/alpm.h @@ -1837,6 +1837,11 @@ int alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir); /* End of gpdir accessors */ /** @} */ +/** Sets the user to switch to for sensitive operations like downloading a file. + * @param handle the context handle + * @param sandboxuser the user to set + */ +int alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser); /** @name Accessors for use syslog * diff --git lib/libalpm/dload.c lib/libalpm/dload.c index 4322318b..03e74ae2 100644 --- lib/libalpm/dload.c +++ lib/libalpm/dload.c @@ -28,6 +28,7 @@ #include <sys/time.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/wait.h> #include <signal.h> #ifdef HAVE_NETINET_IN_H @@ -46,6 +47,7 @@ #include "alpm_list.h" #include "alpm.h" #include "log.h" +#include "sandbox.h" #include "util.h" #include "handle.h" @@ -937,6 +939,100 @@ static int curl_download_internal(alpm_handle_t *handle, #endif +/* Download the requested files by launching a process inside a sandbox. + * Returns -1 if an error happened for a required file + * Returns 0 if a payload was actually downloaded + * Returns 1 if no files were downloaded and all errors were non-fatal + */ +static int curl_download_internal_sandboxed(alpm_handle_t *handle, + alpm_list_t *payloads /* struct dload_payload */, + const char *localpath) +{ + int pid, err = 0, ret = -1; + sigset_t oldblock; + struct sigaction sa_ign = { .sa_handler = SIG_IGN }, oldint, oldquit; + + sigaction(SIGINT, &sa_ign, &oldint); + sigaction(SIGQUIT, &sa_ign, &oldquit); + sigaddset(&sa_ign.sa_mask, SIGCHLD); + sigprocmask(SIG_BLOCK, &sa_ign.sa_mask, &oldblock); + + pid = fork(); + + /* child */ + if(pid == 0) { + /* restore signal handling for the child to inherit */ + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + /* cwd to the download directory */ + ret = chdir(localpath); + if(ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("could not chdir to download directory %s\n"), localpath); + ret = -1; + } else { + ret = alpm_sandbox_child(handle->sandboxuser); + if (ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("sandboxing failed!\n")); + } + + ret = curl_download_internal(handle, payloads, localpath); + } + + /* pass the result back to the parent */ + if(ret == 0) { + /* a payload was actually downloaded */ + _Exit(0); + } + else if(ret == 1) { + /* no files were downloaded and all errors were non-fatal */ + _Exit(1); + } + else { + /* an error happened for a required file */ + _Exit(2); + } + } + + /* parent */ + + if(pid != -1) { + int wret; + while((wret = waitpid(pid, &ret, 0)) == -1 && errno == EINTR); + if(wret > 0) { + if(!WIFEXITED(ret)) { + /* the child did not terminate normally */ + ret = -1; + } + else { + ret = WIFEXITED(ret); + if(ret != 0 && ret != 1) { + /* an error happened for a required file, or unexpected exit status */ + ret = -1; + } + } + } + else { + /* waitpid failed */ + err = errno; + } + } else { + /* fork failed, make sure errno is preserved after cleanup */ + err = errno; + } + + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + if(err) { + errno = err; + ret = -1; + } + return ret; +} + /* Returns -1 if an error happened for a required file * Returns 0 if a payload was actually downloaded * Returns 1 if no files were downloaded and all errors were non-fatal @@ -947,7 +1043,11 @@ int _alpm_download(alpm_handle_t *handle, { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(handle, payloads, localpath); + if(handle->sandboxuser) { + return curl_download_internal_sandboxed(handle, payloads, localpath); + } else { + return curl_download_internal(handle, payloads, localpath); + } #else RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1); #endif diff --git lib/libalpm/handle.c lib/libalpm/handle.c index e6b683cb..5c914faa 100644 --- lib/libalpm/handle.c +++ lib/libalpm/handle.c @@ -573,6 +573,19 @@ int SYMEXPORT alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir) return 0; } +int SYMEXPORT alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser) +{ + CHECK_HANDLE(handle, return -1); + if(handle->sandboxuser) { + FREE(handle->sandboxuser); + } + + STRDUP(handle->sandboxuser, sandboxuser, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + + _alpm_log(handle, ALPM_LOG_DEBUG, "option 'sandboxuser' = %s\n", handle->sandboxuser); + return 0; +} + int SYMEXPORT alpm_option_set_usesyslog(alpm_handle_t *handle, int usesyslog) { CHECK_HANDLE(handle, return -1); diff --git lib/libalpm/handle.h lib/libalpm/handle.h index b1526c67..e0587fc7 100644 --- lib/libalpm/handle.h +++ lib/libalpm/handle.h @@ -90,6 +90,7 @@ struct __alpm_handle_t { char *logfile; /* Name of the log file */ char *lockfile; /* Name of the lock file */ char *gpgdir; /* Directory where GnuPG files are stored */ + char *sandboxuser; /* User to switch to for sensitive operations like downloading files */ alpm_list_t *cachedirs; /* Paths to pacman cache directories */ alpm_list_t *hookdirs; /* Paths to hook directories */ alpm_list_t *overwrite_files; /* Paths that may be overwritten */ diff --git lib/libalpm/meson.build lib/libalpm/meson.build index 607e91a3..224fbf5f 100644 --- lib/libalpm/meson.build +++ lib/libalpm/meson.build @@ -24,6 +24,7 @@ libalpm_sources = files(''' pkghash.h pkghash.c rawstr.c remove.h remove.c + sandbox.h sandbox.c signing.c signing.h sync.h sync.c trans.h trans.c diff --git lib/libalpm/sandbox.c lib/libalpm/sandbox.c new file mode 100644 index 00000000..3c491176 --- /dev/null +++ lib/libalpm/sandbox.c @@ -0,0 +1,63 @@ +/* + * sandbox.c + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <errno.h> +#include <fcntl.h> +#include <grp.h> +#include <pwd.h> +#include <sys/types.h> +#include <unistd.h> + +#include "sandbox.h" + +static int switch_to_user(const char *user) +{ + struct passwd const *pw = NULL; + if(getuid() != 0) { + return 1; + } + pw = getpwnam(user); + if(pw == NULL) { + return errno; + } + if(setgid(pw->pw_gid) != 0) { + return errno; + } + if(setgroups(0, NULL)) { + return errno; + } + if(setuid(pw->pw_uid) != 0) { + return errno; + } + return 0; +} + +/* check exported library symbols with: nm -C -D <lib> */ +#define SYMEXPORT __attribute__((visibility("default"))) + +int SYMEXPORT alpm_sandbox_child(const char* sandboxuser) +{ + int result = 0; + + if(sandboxuser != NULL) { + result = switch_to_user(sandboxuser); + } + + return result; +} diff --git lib/libalpm/sandbox.h lib/libalpm/sandbox.h new file mode 100644 index 00000000..47611575 --- /dev/null +++ lib/libalpm/sandbox.h @@ -0,0 +1,31 @@ +/* + * sandbox.h + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef ALPM_SANDBOX_H +#define ALPM_SANDBOX_H + +#ifdef __cplusplus +extern "C" { +#endif + +int alpm_sandbox_child(const char *sandboxuser); + +#ifdef __cplusplus +} +#endif +#endif /* ALPM_SANDBOX_H */ diff --git src/pacman/conf.c src/pacman/conf.c index 12fee64c..15d05e8c 100644 --- src/pacman/conf.c +++ src/pacman/conf.c @@ -33,6 +33,8 @@ #include <unistd.h> #include <signal.h> +#include <sandbox.h> + /* pacman */ #include "conf.h" #include "ini.h" @@ -215,7 +217,7 @@ static char *get_tempfile(const char *path, const char *filename) * - not thread-safe * - errno may be set by fork(), pipe(), or execvp() */ -static int systemvp(const char *file, char *const argv[]) +static int systemvp(const char *file, char *const argv[], const char *sandboxuser) { int pid, err = 0, ret = -1, err_fd[2]; sigset_t oldblock; @@ -242,6 +244,13 @@ static int systemvp(const char *file, char *const argv[]) sigaction(SIGQUIT, &oldquit, NULL); sigprocmask(SIG_SETMASK, &oldblock, NULL); + if (sandboxuser) { + ret = alpm_sandbox_child(sandboxuser); + if (ret != 0) { + pm_printf(ALPM_LOG_WARNING, _("Switching to sandbox user %s failed!\n"), sandboxuser); + } + } + execvp(file, argv); /* execvp failed, pass the error back to the parent */ @@ -352,7 +361,7 @@ static int download_with_xfercommand(void *ctx, const char *url, free(cmd); } } - retval = systemvp(argv[0], (char**)argv); + retval = systemvp(argv[0], (char**)argv, config->sandboxuser); if(retval == -1) { pm_printf(ALPM_LOG_WARNING, _("running XferCommand: fork failed!\n")); @@ -668,6 +677,11 @@ static int _parse_options(const char *key, char *value, config->logfile = strdup(value); pm_printf(ALPM_LOG_DEBUG, "config: logfile: %s\n", value); } + } else if(strcmp(key, "SandboxUser") == 0) { + if(!config->sandboxuser) { + config->sandboxuser = strdup(value); + pm_printf(ALPM_LOG_DEBUG, "config: sandboxuser: %s\n", value); + } } else if(strcmp(key, "XferCommand") == 0) { char **c; if((config->xfercommand_argv = wordsplit(value)) == NULL) { @@ -904,6 +918,7 @@ static int setup_libalpm(void) alpm_option_set_architectures(handle, config->architectures); alpm_option_set_checkspace(handle, config->checkspace); alpm_option_set_usesyslog(handle, config->usesyslog); + alpm_option_set_sandboxuser(handle, config->sandboxuser); alpm_option_set_ignorepkgs(handle, config->ignorepkg); alpm_option_set_ignoregroups(handle, config->ignoregrp); diff --git src/pacman/conf.h src/pacman/conf.h index 04350d39..675e06fb 100644 --- src/pacman/conf.h +++ src/pacman/conf.h @@ -67,6 +67,7 @@ typedef struct __config_t { char *logfile; char *gpgdir; char *sysroot; + char *sandboxuser; alpm_list_t *hookdirs; alpm_list_t *cachedirs; alpm_list_t *architectures; diff --git src/pacman/pacman-conf.c src/pacman/pacman-conf.c index 600f1622..05a6bcd8 100644 --- src/pacman/pacman-conf.c +++ src/pacman/pacman-conf.c @@ -251,6 +251,7 @@ static void dump_config(void) show_list_str("HookDir", config->hookdirs); show_str("GPGDir", config->gpgdir); show_str("LogFile", config->logfile); + show_str("SandboxUser", config->sandboxuser); show_list_str("HoldPkg", config->holdpkg); show_list_str("IgnorePkg", config->ignorepkg); @@ -349,6 +350,8 @@ static int list_directives(void) show_str("GPGDir", config->gpgdir); } else if(strcasecmp(i->data, "LogFile") == 0) { show_str("LogFile", config->logfile); + } else if(strcasecmp(i->data, "SandboxUser") == 0) { + show_str("SandboxUser", config->sandboxuser); } else if(strcasecmp(i->data, "HoldPkg") == 0) { show_list_str("HoldPkg", config->holdpkg); -- 2.33.0
On 5/9/21 10:42 pm, Remi Gacogne wrote:
This second version contains only the portable part of the sandboxing, switching to a different user when a new option, 'SandboxUser', is set.
To sum up the changes from the last version: - The non-portable parts (preventing new privileges from being gained, dropping capabilities, syscalls filtering and filesystem protection) are gone for now, I'll propose them via smaller patches if this change is accepted. - The value returned by the sub-process is now passed by the exit status only, instead of using a pipe. - The 'UseSandbox' option has been removed. - The alpm_sandbox.{c,h} files have been renamed to sandbox.{c,h}. - A short description of the 'SandboxUser' option has been added to pacman.conf.5.
Cheers,
Remi
Signed-off-by: Remi Gacogne <rgacogne@archlinux.org>
I'm going to do a very quick review so I can get a good overview of the patch given it is still quite large. This will likely not be the final comments as I drill down into this in more detail later. Edit from Allan from 1 hour in the future! After reading the patch, I am fairly happy with its content. More time/eyes are needed on curl_download_internal_sandboxed().
--- doc/pacman.conf.5.asciidoc | 5 ++ lib/libalpm/alpm.h | 5 ++ lib/libalpm/dload.c | 102 ++++++++++++++++++++++++++++++++++++- lib/libalpm/handle.c | 13 +++++ lib/libalpm/handle.h | 1 + lib/libalpm/meson.build | 1 + lib/libalpm/sandbox.c | 63 +++++++++++++++++++++++ lib/libalpm/sandbox.h | 31 +++++++++++ src/pacman/conf.c | 19 ++++++- src/pacman/conf.h | 1 + src/pacman/pacman-conf.c | 3 ++ 11 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 lib/libalpm/sandbox.c create mode 100644 lib/libalpm/sandbox.h
diff --git doc/pacman.conf.5.asciidoc doc/pacman.conf.5.asciidoc index 77a3907f..5b20b177 100644 --- doc/pacman.conf.5.asciidoc +++ doc/pacman.conf.5.asciidoc @@ -207,6 +207,11 @@ Options positive integer. If this config option is not set then only one download stream is used (i.e. downloads happen sequentially).
+*SandboxUser =* username:: + Specifies the user to switch to for sensitive operations, like downloading + files. That user should exist on the system and have the permissions to + write to the files located in `DBPath`. If this config option is not set + then these operations are done as the user running pacman.
What are other process that you think we can add later? I think GPG checking would be a good addition.
Repository Sections ------------------- diff --git lib/libalpm/alpm.h lib/libalpm/alpm.h index a5f4a6ae..5473558a 100644 --- lib/libalpm/alpm.h +++ lib/libalpm/alpm.h @@ -1837,6 +1837,11 @@ int alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir); /* End of gpdir accessors */ /** @} */
+/** Sets the user to switch to for sensitive operations like downloading a file. + * @param handle the context handle + * @param sandboxuser the user to set + */ +int alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser);
I'd like a matching "get" option too. That is likely useful for configuration being done in graphical alpm frontends.
/** @name Accessors for use syslog * diff --git lib/libalpm/dload.c lib/libalpm/dload.c index 4322318b..03e74ae2 100644 --- lib/libalpm/dload.c +++ lib/libalpm/dload.c @@ -28,6 +28,7 @@ #include <sys/time.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/wait.h> #include <signal.h>
#ifdef HAVE_NETINET_IN_H @@ -46,6 +47,7 @@ #include "alpm_list.h" #include "alpm.h" #include "log.h" +#include "sandbox.h" #include "util.h" #include "handle.h"
OK
@@ -937,6 +939,100 @@ static int curl_download_internal(alpm_handle_t *handle,
#endif
+/* Download the requested files by launching a process inside a sandbox. + * Returns -1 if an error happened for a required file + * Returns 0 if a payload was actually downloaded + * Returns 1 if no files were downloaded and all errors were non-fatal + */ +static int curl_download_internal_sandboxed(alpm_handle_t *handle, + alpm_list_t *payloads /* struct dload_payload */, + const char *localpath) +{ + int pid, err = 0, ret = -1; + sigset_t oldblock; + struct sigaction sa_ign = { .sa_handler = SIG_IGN }, oldint, oldquit; + + sigaction(SIGINT, &sa_ign, &oldint); + sigaction(SIGQUIT, &sa_ign, &oldquit); + sigaddset(&sa_ign.sa_mask, SIGCHLD); + sigprocmask(SIG_BLOCK, &sa_ign.sa_mask, &oldblock); + + pid = fork(); + + /* child */ + if(pid == 0) { + /* restore signal handling for the child to inherit */ + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + /* cwd to the download directory */ + ret = chdir(localpath); + if(ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("could not chdir to download directory %s\n"), localpath); + ret = -1;
Seem error is more appropriate than warning here.
+ } else { + ret = alpm_sandbox_child(handle->sandboxuser); + if (ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("sandboxing failed!\n")); + } + + ret = curl_download_internal(handle, payloads, localpath);
If we fail to sandbox, we should definitely not continue. Error!
+ } + + /* pass the result back to the parent */ + if(ret == 0) { + /* a payload was actually downloaded */ + _Exit(0);
Hrm... exit() or _Exit()?
+ } + else if(ret == 1) { + /* no files were downloaded and all errors were non-fatal */ + _Exit(1); + } + else { + /* an error happened for a required file */ + _Exit(2); + } + } + + /* parent */ + + if(pid != -1) { + int wret; + while((wret = waitpid(pid, &ret, 0)) == -1 && errno == EINTR); + if(wret > 0) { + if(!WIFEXITED(ret)) { + /* the child did not terminate normally */ + ret = -1; + } + else { + ret = WIFEXITED(ret); + if(ret != 0 && ret != 1) { + /* an error happened for a required file, or unexpected exit status */ + ret = -1; + } + } + } + else { + /* waitpid failed */ + err = errno; + } + } else { + /* fork failed, make sure errno is preserved after cleanup */ + err = errno; + } + + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + if(err) { + errno = err; + ret = -1; + } + return ret; +} +
That looks fine on first glance.
/* Returns -1 if an error happened for a required file * Returns 0 if a payload was actually downloaded * Returns 1 if no files were downloaded and all errors were non-fatal @@ -947,7 +1043,11 @@ int _alpm_download(alpm_handle_t *handle, { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(handle, payloads, localpath); + if(handle->sandboxuser) { + return curl_download_internal_sandboxed(handle, payloads, localpath); + } else { + return curl_download_internal(handle, payloads, localpath); + } #else RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1); #endif
OK
diff --git lib/libalpm/handle.c lib/libalpm/handle.c index e6b683cb..5c914faa 100644 --- lib/libalpm/handle.c +++ lib/libalpm/handle.c @@ -573,6 +573,19 @@ int SYMEXPORT alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir) return 0; }
+int SYMEXPORT alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser) +{ + CHECK_HANDLE(handle, return -1); + if(handle->sandboxuser) { + FREE(handle->sandboxuser); + } + + STRDUP(handle->sandboxuser, sandboxuser, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + + _alpm_log(handle, ALPM_LOG_DEBUG, "option 'sandboxuser' = %s\n", handle->sandboxuser); + return 0; +} +
OK
int SYMEXPORT alpm_option_set_usesyslog(alpm_handle_t *handle, int usesyslog) { CHECK_HANDLE(handle, return -1); diff --git lib/libalpm/handle.h lib/libalpm/handle.h index b1526c67..e0587fc7 100644 --- lib/libalpm/handle.h +++ lib/libalpm/handle.h @@ -90,6 +90,7 @@ struct __alpm_handle_t { char *logfile; /* Name of the log file */ char *lockfile; /* Name of the lock file */ char *gpgdir; /* Directory where GnuPG files are stored */ + char *sandboxuser; /* User to switch to for sensitive operations like downloading files */ alpm_list_t *cachedirs; /* Paths to pacman cache directories */ alpm_list_t *hookdirs; /* Paths to hook directories */ alpm_list_t *overwrite_files; /* Paths that may be overwritten */
OK
diff --git lib/libalpm/meson.build lib/libalpm/meson.build index 607e91a3..224fbf5f 100644 --- lib/libalpm/meson.build +++ lib/libalpm/meson.build @@ -24,6 +24,7 @@ libalpm_sources = files(''' pkghash.h pkghash.c rawstr.c remove.h remove.c + sandbox.h sandbox.c signing.c signing.h sync.h sync.c trans.h trans.c
OK
diff --git lib/libalpm/sandbox.c lib/libalpm/sandbox.c new file mode 100644 index 00000000..3c491176 --- /dev/null +++ lib/libalpm/sandbox.c @@ -0,0 +1,63 @@ +/* + * sandbox.c + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <errno.h> +#include <fcntl.h> +#include <grp.h> +#include <pwd.h> +#include <sys/types.h> +#include <unistd.h> + +#include "sandbox.h" + +static int switch_to_user(const char *user) +{ + struct passwd const *pw = NULL; + if(getuid() != 0) { + return 1; + } + pw = getpwnam(user);
Does getpwnam handle a NULL user?
+ if(pw == NULL) { + return errno; + } + if(setgid(pw->pw_gid) != 0) { + return errno; + } + if(setgroups(0, NULL)) { + return errno; + } + if(setuid(pw->pw_uid) != 0) { + return errno; + }
this could make use of our ASSERT define ASSERT(getuid() != 0, return 1); ASSERT((pw = getpwnam(user)), return 1); ...
+ return 0; +} + +/* check exported library symbols with: nm -C -D <lib> */ +#define SYMEXPORT __attribute__((visibility("default"))) + +int SYMEXPORT alpm_sandbox_child(const char* sandboxuser) +{ + int result = 0; + + if(sandboxuser != NULL) { + result = switch_to_user(sandboxuser); + } + + return result; +}
OK
diff --git lib/libalpm/sandbox.h lib/libalpm/sandbox.h new file mode 100644 index 00000000..47611575 --- /dev/null +++ lib/libalpm/sandbox.h @@ -0,0 +1,31 @@ +/* + * sandbox.h + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef ALPM_SANDBOX_H +#define ALPM_SANDBOX_H + +#ifdef __cplusplus +extern "C" { +#endif +
No need for that - this header is not installed
+int alpm_sandbox_child(const char *sandboxuser); +
If this is needed externally to libalpm, it should be defined in alpm.h
+#ifdef __cplusplus +} +#endif +#endif /* ALPM_SANDBOX_H */ diff --git src/pacman/conf.c src/pacman/conf.c index 12fee64c..15d05e8c 100644 --- src/pacman/conf.c +++ src/pacman/conf.c @@ -33,6 +33,8 @@ #include <unistd.h> #include <signal.h>
+#include <sandbox.h> +
All public functions are available via alpm.h
/* pacman */ #include "conf.h" #include "ini.h" @@ -215,7 +217,7 @@ static char *get_tempfile(const char *path, const char *filename) * - not thread-safe * - errno may be set by fork(), pipe(), or execvp() */ -static int systemvp(const char *file, char *const argv[]) +static int systemvp(const char *file, char *const argv[], const char *sandboxuser) { int pid, err = 0, ret = -1, err_fd[2]; sigset_t oldblock; @@ -242,6 +244,13 @@ static int systemvp(const char *file, char *const argv[]) sigaction(SIGQUIT, &oldquit, NULL); sigprocmask(SIG_SETMASK, &oldblock, NULL);
+ if (sandboxuser) { + ret = alpm_sandbox_child(sandboxuser); + if (ret != 0) { + pm_printf(ALPM_LOG_WARNING, _("Switching to sandbox user %s failed!\n"), sandboxuser);
Error.
+ } + } + execvp(file, argv);
/* execvp failed, pass the error back to the parent */ @@ -352,7 +361,7 @@ static int download_with_xfercommand(void *ctx, const char *url, free(cmd); } } - retval = systemvp(argv[0], (char**)argv); + retval = systemvp(argv[0], (char**)argv, config->sandboxuser);
OK Which reminds me to split XferCommand support out of pacman/conf.h... Why is it there!
if(retval == -1) { pm_printf(ALPM_LOG_WARNING, _("running XferCommand: fork failed!\n")); @@ -668,6 +677,11 @@ static int _parse_options(const char *key, char *value, config->logfile = strdup(value); pm_printf(ALPM_LOG_DEBUG, "config: logfile: %s\n", value); } + } else if(strcmp(key, "SandboxUser") == 0) { + if(!config->sandboxuser) { + config->sandboxuser = strdup(value); + pm_printf(ALPM_LOG_DEBUG, "config: sandboxuser: %s\n", value); + } } else if(strcmp(key, "XferCommand") == 0) { char **c; if((config->xfercommand_argv = wordsplit(value)) == NULL) {
OK
@@ -904,6 +918,7 @@ static int setup_libalpm(void) alpm_option_set_architectures(handle, config->architectures); alpm_option_set_checkspace(handle, config->checkspace); alpm_option_set_usesyslog(handle, config->usesyslog); + alpm_option_set_sandboxuser(handle, config->sandboxuser);
alpm_option_set_ignorepkgs(handle, config->ignorepkg); alpm_option_set_ignoregroups(handle, config->ignoregrp); diff --git src/pacman/conf.h src/pacman/conf.h index 04350d39..675e06fb 100644 --- src/pacman/conf.h +++ src/pacman/conf.h @@ -67,6 +67,7 @@ typedef struct __config_t { char *logfile; char *gpgdir; char *sysroot; + char *sandboxuser; alpm_list_t *hookdirs; alpm_list_t *cachedirs; alpm_list_t *architectures; diff --git src/pacman/pacman-conf.c src/pacman/pacman-conf.c index 600f1622..05a6bcd8 100644 --- src/pacman/pacman-conf.c +++ src/pacman/pacman-conf.c @@ -251,6 +251,7 @@ static void dump_config(void) show_list_str("HookDir", config->hookdirs); show_str("GPGDir", config->gpgdir); show_str("LogFile", config->logfile); + show_str("SandboxUser", config->sandboxuser);
show_list_str("HoldPkg", config->holdpkg); show_list_str("IgnorePkg", config->ignorepkg); @@ -349,6 +350,8 @@ static int list_directives(void) show_str("GPGDir", config->gpgdir); } else if(strcasecmp(i->data, "LogFile") == 0) { show_str("LogFile", config->logfile); + } else if(strcasecmp(i->data, "SandboxUser") == 0) { + show_str("SandboxUser", config->sandboxuser);
} else if(strcasecmp(i->data, "HoldPkg") == 0) { show_list_str("HoldPkg", config->holdpkg);
OK
On 09/05/21 at 02:42pm, Remi Gacogne wrote:
This second version contains only the portable part of the sandboxing, switching to a different user when a new option, 'SandboxUser', is set.
To sum up the changes from the last version: - The non-portable parts (preventing new privileges from being gained, dropping capabilities, syscalls filtering and filesystem protection) are gone for now, I'll propose them via smaller patches if this change is accepted. - The value returned by the sub-process is now passed by the exit status only, instead of using a pipe. - The 'UseSandbox' option has been removed. - The alpm_sandbox.{c,h} files have been renamed to sandbox.{c,h}. - A short description of the 'SandboxUser' option has been added to pacman.conf.5.
Cheers,
Remi
Signed-off-by: Remi Gacogne <rgacogne@archlinux.org> ---
Put notes here to avoid including them in the commit message.
doc/pacman.conf.5.asciidoc | 5 ++ lib/libalpm/alpm.h | 5 ++ lib/libalpm/dload.c | 102 ++++++++++++++++++++++++++++++++++++- lib/libalpm/handle.c | 13 +++++ lib/libalpm/handle.h | 1 + lib/libalpm/meson.build | 1 + lib/libalpm/sandbox.c | 63 +++++++++++++++++++++++ lib/libalpm/sandbox.h | 31 +++++++++++ src/pacman/conf.c | 19 ++++++- src/pacman/conf.h | 1 + src/pacman/pacman-conf.c | 3 ++ 11 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 lib/libalpm/sandbox.c create mode 100644 lib/libalpm/sandbox.h
...
+/* Download the requested files by launching a process inside a sandbox. + * Returns -1 if an error happened for a required file + * Returns 0 if a payload was actually downloaded + * Returns 1 if no files were downloaded and all errors were non-fatal + */ +static int curl_download_internal_sandboxed(alpm_handle_t *handle, + alpm_list_t *payloads /* struct dload_payload */, + const char *localpath) +{ + int pid, err = 0, ret = -1; + sigset_t oldblock; + struct sigaction sa_ign = { .sa_handler = SIG_IGN }, oldint, oldquit; + + sigaction(SIGINT, &sa_ign, &oldint); + sigaction(SIGQUIT, &sa_ign, &oldquit); + sigaddset(&sa_ign.sa_mask, SIGCHLD); + sigprocmask(SIG_BLOCK, &sa_ign.sa_mask, &oldblock); + + pid = fork(); + + /* child */ + if(pid == 0) { + /* restore signal handling for the child to inherit */ + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + /* cwd to the download directory */ + ret = chdir(localpath); + if(ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("could not chdir to download directory %s\n"), localpath); + ret = -1; + } else { + ret = alpm_sandbox_child(handle->sandboxuser); + if (ret != 0) { + _alpm_log(handle, ALPM_LOG_WARNING, _("sandboxing failed!\n")); + } + + ret = curl_download_internal(handle, payloads, localpath); + } + + /* pass the result back to the parent */ + if(ret == 0) { + /* a payload was actually downloaded */ + _Exit(0); + } + else if(ret == 1) { + /* no files were downloaded and all errors were non-fatal */ + _Exit(1); + } + else { + /* an error happened for a required file */ + _Exit(2); + } + } + + /* parent */ + + if(pid != -1) { + int wret; + while((wret = waitpid(pid, &ret, 0)) == -1 && errno == EINTR); + if(wret > 0) { + if(!WIFEXITED(ret)) { + /* the child did not terminate normally */ + ret = -1; + } + else { + ret = WIFEXITED(ret); + if(ret != 0 && ret != 1) { + /* an error happened for a required file, or unexpected exit status */ + ret = -1; + } + } + } + else { + /* waitpid failed */ + err = errno; + } + } else { + /* fork failed, make sure errno is preserved after cleanup */ + err = errno; + } + + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + if(err) { + errno = err; + ret = -1; + } + return ret; +} +
After thinking about this some more, I think this is far too simple. Just running download_internal in an unprivileged fork will break anything that relies on side effects. download_internal sets pm_errno, tracks server errors, and calls a number of front-end callbacks. Losing server error tracking across multiple downloads isn't a big deal, but losing pm_errno is significant and we have no way of knowing what kind of state changes the front-end callbacks might be making. I suspect this would massively break GUI front-ends. apg
On 9/6/21 2:42 AM, Andrew Gregory wrote:
Put notes here to avoid including them in the commit message.
Understood, thanks!
After thinking about this some more, I think this is far too simple. Just running download_internal in an unprivileged fork will break anything that relies on side effects. download_internal sets pm_errno, tracks server errors, and calls a number of front-end callbacks. Losing server error tracking across multiple downloads isn't a big deal, but losing pm_errno is significant and we have no way of knowing what kind of state changes the front-end callbacks might be making. I suspect this would massively break GUI front-ends.
Right, that was my main worry when I started working on this, but since the 'XferCommand' option existed I was hopeful the download code was not too tightly coupled with the rest of the code. Do you think it would still make sense to keep working on this? It looks like we could pass the value of pm_errno back to the main process, using a pipe if needed. As for the front-end callbacks I guess we could detect that these are set and disable the sandboxing in that case, while documenting that this option does not play well with GUI front-ends. I'm guessing that's already the case with 'XferCommand?', since the same issues likely apply? Remi
On 9/6/21 09:34, Remi Gacogne wrote:
Right, that was my main worry when I started working on this, but since the 'XferCommand' option existed I was hopeful the download code was not too tightly coupled with the rest of the code. Do you think it would still make sense to keep working on this? It looks like we could pass the value of pm_errno back to the main process, using a pipe if needed. As for the front-end callbacks I guess we could detect that these are set and disable the sandboxing in that case, while documenting that this option does not play well with GUI front-ends. I'm guessing that's already the case with 'XferCommand?', since the same issues likely apply?
I looked a bit more into this. Passing pm_errno back is easy, and can even be done using the exit status without setting up a pipe. Unfortunately I realize now that I under-estimated the criticality of the front-ends callbacks. Reading that code more in depth quickly uncovered issues I did not notice earlier, like 'on_progress' being set during the downloads and leading to cb_log adding messages to a list that is never flushed when we fork. I'm not sure how to deal with that. Serializing the content of the dlcb events to pass them over a pipe could be done, but that would be a rather big patch, and would be cumbersome to maintain. I'll think about it a bit more, and in the meantime any ideas are welcome :) Remi
Signed-off-by: Remi Gacogne <rgacogne@archlinux.org> --- This is a new attempt at adding a new option, 'SandboxUser' to drop privileges for sensitive operations. This only applies to downloading files for now, but verifying signatures is a likely next candidate, hopefully a much easier one. This new version keeps the same logic than the previous one, with small changes addressing the comments from Allan and Andrew, and a big change to intercept the callbacks raised in the child process, serialize them before sending them over a pipe to finally execute them in the parent process. This is necessary to avoid breaking frontends. At the moment only the log and download callbacks are thus intercepted, as I did not find any other kind of callbacks being executed during a download operation while reviewing the code. One drawback of the current approach is that for the log callback the format string is applied in the child, before serializing, and the formatted result is passed to the callback instead of passing the format string and all the parameters. It is not obvious to me why a frontend would want to process the format string and parameters itself, but if there is a valid use-case for that this patch will break it. There is also two small changes in src/pacman/callback.c, as we can't assume anymore that the filename passed to the download callbacks will remain alive until the last of them has been called. It also looked like one of the "clean_filename" allocation was leaked. doc/pacman.conf.5.asciidoc | 5 + lib/libalpm/alpm.h | 17 ++ lib/libalpm/dload.c | 311 ++++++++++++++++++++++++++++++++++++- lib/libalpm/handle.c | 20 +++ lib/libalpm/handle.h | 1 + lib/libalpm/meson.build | 1 + lib/libalpm/sandbox.c | 54 +++++++ src/pacman/callback.c | 12 +- src/pacman/conf.c | 19 ++- src/pacman/conf.h | 1 + src/pacman/pacman-conf.c | 3 + 11 files changed, 437 insertions(+), 7 deletions(-) create mode 100644 lib/libalpm/sandbox.c diff --git doc/pacman.conf.5.asciidoc doc/pacman.conf.5.asciidoc index 77a3907f..b8126a67 100644 --- doc/pacman.conf.5.asciidoc +++ doc/pacman.conf.5.asciidoc @@ -207,6 +207,11 @@ Options positive integer. If this config option is not set then only one download stream is used (i.e. downloads happen sequentially). +*SandboxUser =* username:: + Specifies the user to switch to for sensitive operations, like downloading + files. That user should exist on the system and have the permissions to + write to the files located in `DBPath` and `CacheDir`. If this config option + is not set then these operations are done as the user running pacman. Repository Sections ------------------- diff --git lib/libalpm/alpm.h lib/libalpm/alpm.h index 8d8fe243..b206145b 100644 --- lib/libalpm/alpm.h +++ lib/libalpm/alpm.h @@ -1833,6 +1833,17 @@ int alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir); /* End of gpdir accessors */ /** @} */ +/** Returns the user to switch to for sensitive operations like downloading a file. + * @param handle the context handle + * @return the user name + */ +const char *alpm_option_get_sandboxuser(alpm_handle_t *handle); + +/** Sets the user to switch to for sensitive operations like downloading a file. + * @param handle the context handle + * @param sandboxuser the user to set + */ +int alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser); /** @name Accessors for use syslog * @@ -2881,6 +2892,12 @@ const char *alpm_version(void); * */ int alpm_capabilities(void); +/** Drop privileges by switching to a different user. + * @param sandboxuser the user to switch to + * @return 0 on success, the value of errno otherwise + */ +int alpm_sandbox_child(const char *sandboxuser); + /* End of libalpm_misc */ /** @} */ diff --git lib/libalpm/dload.c lib/libalpm/dload.c index 4322318b..582f024f 100644 --- lib/libalpm/dload.c +++ lib/libalpm/dload.c @@ -28,6 +28,7 @@ #include <sys/time.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/wait.h> #include <signal.h> #ifdef HAVE_NETINET_IN_H @@ -937,6 +938,310 @@ static int curl_download_internal(alpm_handle_t *handle, #endif +/** The type of callbacks that can happen during a sandboxed download */ +typedef enum _sandboxed_callbacks_t { + ALPM_SANDBOXED_LOG_CB, + ALPM_SANDBOXED_DOWNLOAD_CB +} sandboxed_callbacks_t; + +typedef struct _sandboxed_callbacks_context { + int callback_pipe; +} sandboxed_callbacks_context; + +__attribute__((format(printf, 3, 0))) +static void sandbox_log_cb(void *ctx, alpm_loglevel_t level, const char *fmt, va_list args) +{ + sandboxed_callbacks_t type = ALPM_SANDBOXED_LOG_CB; + sandboxed_callbacks_context *context = ctx; + char *string = NULL; + int string_size = 0; + + if(!context || context->callback_pipe == -1) { + return; + } + + string_size = vasprintf(&string, fmt, args); + if(string != NULL) { + write(context->callback_pipe, &type, sizeof(type)); + write(context->callback_pipe, &level, sizeof(level)); + write(context->callback_pipe, &string_size, sizeof(string_size)); + write(context->callback_pipe, string, string_size); + FREE(string); + } +} + +static void sandbox_dl_cb(void *ctx, const char *filename, alpm_download_event_type_t event, void *data) +{ + sandboxed_callbacks_t type = ALPM_SANDBOXED_DOWNLOAD_CB; + sandboxed_callbacks_context *context = ctx; + size_t filename_len; + + if(!context || context->callback_pipe == -1) { + return; + } + + if(!filename || (event != ALPM_DOWNLOAD_INIT && event != ALPM_DOWNLOAD_PROGRESS && event != ALPM_DOWNLOAD_RETRY && event != ALPM_DOWNLOAD_COMPLETED)) { + return; + } + + filename_len = strlen(filename); + + write(context->callback_pipe, &type, sizeof(type)); + write(context->callback_pipe, &event, sizeof(event)); + switch(event) { + case ALPM_DOWNLOAD_INIT: + write(context->callback_pipe, data, sizeof(alpm_download_event_init_t)); + break; + case ALPM_DOWNLOAD_PROGRESS: + write(context->callback_pipe, data, sizeof(alpm_download_event_progress_t)); + break; + case ALPM_DOWNLOAD_RETRY: + write(context->callback_pipe, data, sizeof(alpm_download_event_retry_t)); + break; + case ALPM_DOWNLOAD_COMPLETED: + write(context->callback_pipe, data, sizeof(alpm_download_event_completed_t)); + break; + } + write(context->callback_pipe, &filename_len, sizeof(filename_len)); + write(context->callback_pipe, filename, filename_len); +} + +static int sandbox_fetch_cb(void *, const char *, const char *, int ) +{ + fprintf(stderr, "Fetch callback called but not handled in sandboxed download\n"); + return -1; +} + +static void sandbox_question_cb(void *, alpm_question_t *) +{ + fprintf(stderr, "Question callback called but not handled in sandboxed download\n"); +} + +static void sandbox_event_cb(void *, alpm_event_t *) +{ + fprintf(stderr, "Event callback called but not handled in sandboxed download\n"); +} + +static void sandbox_progress_cb(void *, alpm_progress_t, const char *, int, size_t, size_t) +{ + fprintf(stderr, "Progress callback called but not handled in sandboxed download\n"); +} + +static bool handle_sandboxed_log_cb(alpm_handle_t *handle, int callback_pipe) { + alpm_loglevel_t level; + char *string = NULL; + int string_size = 0; + ssize_t got; + + got = read(callback_pipe, &level, sizeof(level)); + ASSERT(got > 0 && (size_t)got == sizeof(level), return false); + + got = read(callback_pipe, &string_size, sizeof(string_size)); + ASSERT(got > 0 && (size_t)got == sizeof(string_size), return false); + + MALLOC(string, string_size + 1, return false); + + got = read(callback_pipe, string, string_size); + if(got < 0 || got != string_size) { + FREE(string); + return false; + } + string[string_size] = 0; + + _alpm_log(handle, level, "%s", string); + FREE(string); + return true; +} + +static bool handle_sandboxed_download_cb(alpm_handle_t *handle, int callback_pipe) { + alpm_download_event_type_t type; + char *filename = NULL; + size_t filename_size, cb_data_size; + ssize_t got; + union { + alpm_download_event_init_t init; + alpm_download_event_progress_t progress; + alpm_download_event_retry_t retry; + alpm_download_event_completed_t completed; + } cb_data; + + got = read(callback_pipe, &type, sizeof(type)); + ASSERT(got > 0 && (size_t)got == sizeof(type), return false); + + switch (type) { + case ALPM_DOWNLOAD_INIT: + cb_data_size = sizeof(alpm_download_event_init_t); + got = read(callback_pipe, &cb_data.init, cb_data_size); + break; + case ALPM_DOWNLOAD_PROGRESS: + cb_data_size = sizeof(alpm_download_event_progress_t); + got = read(callback_pipe, &cb_data.progress, cb_data_size); + break; + case ALPM_DOWNLOAD_RETRY: + cb_data_size = sizeof(alpm_download_event_retry_t); + got = read(callback_pipe, &cb_data.retry, cb_data_size); + break; + case ALPM_DOWNLOAD_COMPLETED: + cb_data_size = sizeof(alpm_download_event_completed_t); + got = read(callback_pipe, &cb_data.completed, cb_data_size); + break; + default: + return false; + } + ASSERT(got > 0 && (size_t)got == cb_data_size, return false); + + got = read(callback_pipe, &filename_size, sizeof(filename_size)); + ASSERT(got > 0 && (size_t)got == sizeof(filename_size), return false); + + MALLOC(filename, filename_size + 1, return false); + + got = read(callback_pipe, filename, filename_size); + if(got < 0 || (size_t)got != filename_size) { + FREE(filename); + return false; + } + filename[filename_size] = 0; + + handle->dlcb(handle->dlcb_ctx, filename, type, &cb_data); + FREE(filename); + return true; +} + +/* Download the requested files by launching a process inside a sandbox. + * Returns -1 if an error happened for a required file + * Returns 0 if a payload was actually downloaded + * Returns 1 if no files were downloaded and all errors were non-fatal + */ +static int curl_download_internal_sandboxed(alpm_handle_t *handle, + alpm_list_t *payloads /* struct dload_payload */, + const char *localpath) +{ + int pid, err = 0, ret = -1, callbacks_fd[2]; + sigset_t oldblock; + struct sigaction sa_ign = { .sa_handler = SIG_IGN }, oldint, oldquit; + sandboxed_callbacks_context callbacks_ctx; + + if(pipe(callbacks_fd) != 0) { + return -1; + } + + sigaction(SIGINT, &sa_ign, &oldint); + sigaction(SIGQUIT, &sa_ign, &oldquit); + sigaddset(&sa_ign.sa_mask, SIGCHLD); + sigprocmask(SIG_BLOCK, &sa_ign.sa_mask, &oldblock); + + pid = fork(); + + /* child */ + if(pid == 0) { + close(callbacks_fd[0]); + fcntl(callbacks_fd[1], F_SETFD, FD_CLOEXEC); + callbacks_ctx.callback_pipe = callbacks_fd[1]; + alpm_option_set_logcb(handle, sandbox_log_cb, &callbacks_ctx); + alpm_option_set_dlcb(handle, sandbox_dl_cb, &callbacks_ctx); + alpm_option_set_fetchcb(handle, sandbox_fetch_cb, &callbacks_ctx); + alpm_option_set_eventcb(handle, sandbox_event_cb, &callbacks_ctx); + alpm_option_set_questioncb(handle, sandbox_question_cb, &callbacks_ctx); + alpm_option_set_progresscb(handle, sandbox_progress_cb, &callbacks_ctx); + + /* restore signal handling for the child to inherit */ + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + /* cwd to the download directory */ + ret = chdir(localpath); + if(ret != 0) { + _alpm_log(handle, ALPM_LOG_ERROR, _("could not chdir to download directory %s\n"), localpath); + ret = -1; + } else { + ret = alpm_sandbox_child(handle->sandboxuser); + if (ret != 0) { + _alpm_log(handle, ALPM_LOG_ERROR, _("sandboxing failed!\n")); + _Exit(ret | 128); + } + + ret = curl_download_internal(handle, payloads, localpath); + } + + /* pass the result back to the parent */ + if(ret == 0) { + /* a payload was actually downloaded */ + exit(0); + } + else if(ret == 1) { + /* no files were downloaded and all errors were non-fatal */ + _Exit(handle->pm_errno); + } + else { + /* an error happened for a required file */ + _Exit(handle->pm_errno | 128); + } + } + + /* parent */ + close(callbacks_fd[1]); + + if(pid != -1) { + bool done = false; + do { + sandboxed_callbacks_t callback_type; + ssize_t got = read(callbacks_fd[0], &callback_type, sizeof(callback_type)); + if(got < 0 || (size_t)got != sizeof(callback_type)) { + done = true; + break; + } + if(callback_type == ALPM_SANDBOXED_LOG_CB) { + if(!handle_sandboxed_log_cb(handle, callbacks_fd[0])) { + done = true; + } + } + else if(callback_type == ALPM_SANDBOXED_DOWNLOAD_CB) { + if(!handle_sandboxed_download_cb(handle, callbacks_fd[0])) { + done = true; + } + } + } + while(!done); + + int wret; + while((wret = waitpid(pid, &ret, 0)) == -1 && errno == EINTR); + if(wret > 0) { + if(!WIFEXITED(ret)) { + /* the child did not terminate normally */ + ret = -1; + } + else { + ret = WEXITSTATUS(ret); + if(ret & 128) { + /* an error happened for a required file, or unexpected exit status */ + handle->pm_errno = ret & ~128; + ret = -1; + } + } + } + else { + /* waitpid failed */ + err = errno; + } + } else { + /* fork failed, make sure errno is preserved after cleanup */ + err = errno; + } + + close(callbacks_fd[0]); + + sigaction(SIGINT, &oldint, NULL); + sigaction(SIGQUIT, &oldquit, NULL); + sigprocmask(SIG_SETMASK, &oldblock, NULL); + + if(err) { + errno = err; + ret = -1; + } + return ret; +} + /* Returns -1 if an error happened for a required file * Returns 0 if a payload was actually downloaded * Returns 1 if no files were downloaded and all errors were non-fatal @@ -947,7 +1252,11 @@ int _alpm_download(alpm_handle_t *handle, { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(handle, payloads, localpath); + if(handle->sandboxuser) { + return curl_download_internal_sandboxed(handle, payloads, localpath); + } else { + return curl_download_internal(handle, payloads, localpath); + } #else RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1); #endif diff --git lib/libalpm/handle.c lib/libalpm/handle.c index e6b683cb..1aeec85b 100644 --- lib/libalpm/handle.c +++ lib/libalpm/handle.c @@ -79,6 +79,7 @@ void _alpm_handle_free(alpm_handle_t *handle) FREE(handle->lockfile); FREELIST(handle->architectures); FREE(handle->gpgdir); + FREE(handle->sandboxuser); FREELIST(handle->noupgrade); FREELIST(handle->noextract); FREELIST(handle->ignorepkg); @@ -270,6 +271,12 @@ const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle) return handle->gpgdir; } +const char SYMEXPORT *alpm_option_get_sandboxuser(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return NULL); + return handle->sandboxuser; +} + int SYMEXPORT alpm_option_get_usesyslog(alpm_handle_t *handle) { CHECK_HANDLE(handle, return -1); @@ -573,6 +580,19 @@ int SYMEXPORT alpm_option_set_gpgdir(alpm_handle_t *handle, const char *gpgdir) return 0; } +int SYMEXPORT alpm_option_set_sandboxuser(alpm_handle_t *handle, const char *sandboxuser) +{ + CHECK_HANDLE(handle, return -1); + if(handle->sandboxuser) { + FREE(handle->sandboxuser); + } + + STRDUP(handle->sandboxuser, sandboxuser, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + + _alpm_log(handle, ALPM_LOG_DEBUG, "option 'sandboxuser' = %s\n", handle->sandboxuser); + return 0; +} + int SYMEXPORT alpm_option_set_usesyslog(alpm_handle_t *handle, int usesyslog) { CHECK_HANDLE(handle, return -1); diff --git lib/libalpm/handle.h lib/libalpm/handle.h index 2e85f283..0fff5374 100644 --- lib/libalpm/handle.h +++ lib/libalpm/handle.h @@ -91,6 +91,7 @@ struct _alpm_handle_t { char *logfile; /* Name of the log file */ char *lockfile; /* Name of the lock file */ char *gpgdir; /* Directory where GnuPG files are stored */ + char *sandboxuser; /* User to switch to for sensitive operations like downloading files */ alpm_list_t *cachedirs; /* Paths to pacman cache directories */ alpm_list_t *hookdirs; /* Paths to hook directories */ alpm_list_t *overwrite_files; /* Paths that may be overwritten */ diff --git lib/libalpm/meson.build lib/libalpm/meson.build index 607e91a3..224fbf5f 100644 --- lib/libalpm/meson.build +++ lib/libalpm/meson.build @@ -24,6 +24,7 @@ libalpm_sources = files(''' pkghash.h pkghash.c rawstr.c remove.h remove.c + sandbox.h sandbox.c signing.c signing.h sync.h sync.c trans.h trans.c diff --git lib/libalpm/sandbox.c lib/libalpm/sandbox.c new file mode 100644 index 00000000..0a2675c0 --- /dev/null +++ lib/libalpm/sandbox.c @@ -0,0 +1,54 @@ +/* + * sandbox.c + * + * Copyright (c) 2021 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <errno.h> +#include <fcntl.h> +#include <grp.h> +#include <pwd.h> +#include <sys/types.h> +#include <unistd.h> + +#include "alpm.h" +#include "util.h" + +static int switch_to_user(const char *user) +{ + struct passwd const *pw = NULL; + ASSERT(user != NULL, return 1); + ASSERT(getuid() == 0, return 1); + ASSERT((pw = getpwnam(user)), return errno); + ASSERT(setgid(pw->pw_gid) == 0, return errno); + ASSERT(setgroups(0, NULL) == 0, return errno); + ASSERT(setuid(pw->pw_uid) == 0, return errno); + return 0; +} + +/* check exported library symbols with: nm -C -D <lib> */ +#define SYMEXPORT __attribute__((visibility("default"))) + +int SYMEXPORT alpm_sandbox_child(const char* sandboxuser) +{ + int result = 0; + + if(sandboxuser != NULL) { + result = switch_to_user(sandboxuser); + } + + return result; +} diff --git src/pacman/callback.c src/pacman/callback.c index 2579b98a..da064114 100644 --- src/pacman/callback.c +++ src/pacman/callback.c @@ -57,7 +57,7 @@ static alpm_list_t *output = NULL; #endif struct pacman_progress_bar { - const char *filename; + char *filename; off_t xfered; /* Current amount of transferred data */ off_t total_size; size_t downloaded; @@ -252,7 +252,7 @@ void cb_event(void *ctx, alpm_event_t *event) alpm_event_hook_run_t *e = &event->hook_run; int digits = number_length(e->total); printf("(%*zu/%*zu) %s\n", digits, e->position, - digits, e->total, + digits, e->total, e->desc ? e->desc : e->name); } break; @@ -748,7 +748,8 @@ static void init_total_progressbar(void) { totalbar = calloc(1, sizeof(struct pacman_progress_bar)); assert(totalbar); - totalbar->filename = _("Total"); + totalbar->filename = strdup(_("Total")); + assert(totalbar->filename); totalbar->init_time = get_time_ms(); totalbar->total_size = list_total; totalbar->howmany = list_total_pkgs; @@ -888,7 +889,8 @@ static void dload_init_event(const char *filename, alpm_download_event_init_t *d struct pacman_progress_bar *bar = calloc(1, sizeof(struct pacman_progress_bar)); assert(bar); - bar->filename = filename; + bar->filename = strdup(filename); + assert(bar->filename); bar->init_time = get_time_ms(); bar->rate = 0.0; multibar_ui.active_downloads = alpm_list_add(multibar_ui.active_downloads, bar); @@ -904,6 +906,7 @@ static void dload_init_event(const char *filename, alpm_download_event_init_t *d printf("\n"); multibar_ui.cursor_lineno++; } + free(cleaned_filename); } /* Update progress bar rate/eta stats. @@ -1091,6 +1094,7 @@ static void dload_complete_event(const char *filename, alpm_download_event_compl multibar_ui.active_downloads = alpm_list_remove_item( multibar_ui.active_downloads, head); free(head); + free(j->filename); free(j); } else { break; diff --git src/pacman/conf.c src/pacman/conf.c index 12fee64c..3a170ff3 100644 --- src/pacman/conf.c +++ src/pacman/conf.c @@ -155,6 +155,7 @@ int config_free(config_t *oldconfig) free(oldconfig->dbpath); free(oldconfig->logfile); free(oldconfig->gpgdir); + free(oldconfig->sandboxuser); FREELIST(oldconfig->hookdirs); FREELIST(oldconfig->cachedirs); free(oldconfig->xfercommand); @@ -215,7 +216,7 @@ static char *get_tempfile(const char *path, const char *filename) * - not thread-safe * - errno may be set by fork(), pipe(), or execvp() */ -static int systemvp(const char *file, char *const argv[]) +static int systemvp(const char *file, char *const argv[], const char *sandboxuser) { int pid, err = 0, ret = -1, err_fd[2]; sigset_t oldblock; @@ -242,6 +243,14 @@ static int systemvp(const char *file, char *const argv[]) sigaction(SIGQUIT, &oldquit, NULL); sigprocmask(SIG_SETMASK, &oldblock, NULL); + if (sandboxuser) { + ret = alpm_sandbox_child(sandboxuser); + if (ret != 0) { + pm_printf(ALPM_LOG_ERROR, _("Switching to sandbox user %s failed!\n"), sandboxuser); + _Exit(ret); + } + } + execvp(file, argv); /* execvp failed, pass the error back to the parent */ @@ -352,7 +361,7 @@ static int download_with_xfercommand(void *ctx, const char *url, free(cmd); } } - retval = systemvp(argv[0], (char**)argv); + retval = systemvp(argv[0], (char**)argv, config->sandboxuser); if(retval == -1) { pm_printf(ALPM_LOG_WARNING, _("running XferCommand: fork failed!\n")); @@ -668,6 +677,11 @@ static int _parse_options(const char *key, char *value, config->logfile = strdup(value); pm_printf(ALPM_LOG_DEBUG, "config: logfile: %s\n", value); } + } else if(strcmp(key, "SandboxUser") == 0) { + if(!config->sandboxuser) { + config->sandboxuser = strdup(value); + pm_printf(ALPM_LOG_DEBUG, "config: sandboxuser: %s\n", value); + } } else if(strcmp(key, "XferCommand") == 0) { char **c; if((config->xfercommand_argv = wordsplit(value)) == NULL) { @@ -904,6 +918,7 @@ static int setup_libalpm(void) alpm_option_set_architectures(handle, config->architectures); alpm_option_set_checkspace(handle, config->checkspace); alpm_option_set_usesyslog(handle, config->usesyslog); + alpm_option_set_sandboxuser(handle, config->sandboxuser); alpm_option_set_ignorepkgs(handle, config->ignorepkg); alpm_option_set_ignoregroups(handle, config->ignoregrp); diff --git src/pacman/conf.h src/pacman/conf.h index 04350d39..675e06fb 100644 --- src/pacman/conf.h +++ src/pacman/conf.h @@ -67,6 +67,7 @@ typedef struct __config_t { char *logfile; char *gpgdir; char *sysroot; + char *sandboxuser; alpm_list_t *hookdirs; alpm_list_t *cachedirs; alpm_list_t *architectures; diff --git src/pacman/pacman-conf.c src/pacman/pacman-conf.c index 600f1622..05a6bcd8 100644 --- src/pacman/pacman-conf.c +++ src/pacman/pacman-conf.c @@ -251,6 +251,7 @@ static void dump_config(void) show_list_str("HookDir", config->hookdirs); show_str("GPGDir", config->gpgdir); show_str("LogFile", config->logfile); + show_str("SandboxUser", config->sandboxuser); show_list_str("HoldPkg", config->holdpkg); show_list_str("IgnorePkg", config->ignorepkg); @@ -349,6 +350,8 @@ static int list_directives(void) show_str("GPGDir", config->gpgdir); } else if(strcasecmp(i->data, "LogFile") == 0) { show_str("LogFile", config->logfile); + } else if(strcasecmp(i->data, "SandboxUser") == 0) { + show_str("SandboxUser", config->sandboxuser); } else if(strcasecmp(i->data, "HoldPkg") == 0) { show_list_str("HoldPkg", config->holdpkg); -- 2.33.0
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Remi Gacogne