diff --git a/.gitignore b/.gitignore index 727bfdb6ec..f3193173d6 100644 --- a/.gitignore +++ b/.gitignore @@ -183,6 +183,7 @@ /tests/test_conf /tools/libvirt-guests.sh /tools/virt-login-shell +/tools/virt-login-shell-helper /tools/virsh /tools/virsh-*-edit.c /tools/virt-admin diff --git a/cfg.mk b/cfg.mk index 694ce2076f..9130b4560b 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1203,7 +1203,7 @@ exclude_file_name_regexp--sc_avoid_write = \ exclude_file_name_regexp--sc_bindtextdomain = .* -exclude_file_name_regexp--sc_gettext_init = ^(tests|examples)/ +exclude_file_name_regexp--sc_gettext_init = ^((tests|examples)/|tools/virt-login-shell.c) exclude_file_name_regexp--sc_copyright_format = \ ^cfg\.mk$$ @@ -1229,7 +1229,7 @@ exclude_file_name_regexp--sc_prohibit_access_xok = \ ^(cfg\.mk|src/util/virutil\.c)$$ exclude_file_name_regexp--sc_prohibit_asprintf = \ - ^(cfg\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c$$) + ^(cfg\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c|tools/virt-login-shell\.c$$) exclude_file_name_regexp--sc_prohibit_strdup = \ ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c|tests/commandhelper\.c$$) @@ -1256,7 +1256,7 @@ exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \ ^src/rpc/gendispatch\.pl$$ exclude_file_name_regexp--sc_prohibit_nonreentrant = \ - ^((po|tests|examples)/|docs/.*(py|js|html\.in)|run.in$$|tools/wireshark/util/genxdrstub\.pl$$) + ^((po|tests|examples)/|docs/.*(py|js|html\.in)|run.in$$|tools/wireshark/util/genxdrstub\.pl|tools/virt-login-shell\.c$$) exclude_file_name_regexp--sc_prohibit_select = \ ^cfg\.mk$$ @@ -1270,7 +1270,7 @@ exclude_file_name_regexp--sc_prohibit_raw_allocation = \ exclude_file_name_regexp--sc_prohibit_readlink = \ ^src/(util/virutil|lxc/lxc_container)\.c$$ -exclude_file_name_regexp--sc_prohibit_setuid = ^src/util/virutil\.c$$ +exclude_file_name_regexp--sc_prohibit_setuid = ^src/util/virutil\.c|tools/virt-login-shell\.c$$ exclude_file_name_regexp--sc_prohibit_sprintf = \ ^(cfg\.mk|docs/hacking\.html\.in|.*\.stp|.*\.pl)$$ @@ -1317,7 +1317,7 @@ exclude_file_name_regexp--sc_prohibit_unsigned_pid = \ ^(include/libvirt/.*\.h|src/(qemu/qemu_driver\.c|driver-hypervisor\.h|libvirt(-[a-z]*)?\.c|.*\.x|util/vir(polkit|systemd)\.c)|tests/virpolkittest\.c|tools/virsh-domain\.c)$$ exclude_file_name_regexp--sc_prohibit_getenv = \ - ^tests/.*\.[ch]$$ + ^tests/.*\.[ch]|tools/virt-login-shell\.c$$ exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \ ^(src/util/virlog\.h|src/network/bridge_driver\.h)$$ diff --git a/libvirt.spec.in b/libvirt.spec.in index 045c0fed1a..6f96fbec33 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1847,6 +1847,7 @@ exit 0 %if %{with_lxc} %files login-shell %attr(4750, root, virtlogin) %{_bindir}/virt-login-shell +%{_libexecdir}/virt-login-shell-helper %config(noreplace) %{_sysconfdir}/libvirt/virt-login-shell.conf %{_mandir}/man1/virt-login-shell.1* %endif diff --git a/tools/Makefile.am b/tools/Makefile.am index 125540d313..3d9461ba65 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -105,6 +105,7 @@ endif WITH_SANLOCK if WITH_LOGIN_SHELL conf_DATA += virt-login-shell.conf bin_PROGRAMS += virt-login-shell +libexec_PROGRAMS = virt-login-shell-helper man1_MANS += virt-login-shell.1 endif WITH_LOGIN_SHELL @@ -192,25 +193,25 @@ virt_host_validate_CFLAGS = \ $(AM_CFLAGS) \ $(NULL) -# Since virt-login-shell will be setuid, we must do everything -# we can to avoid linking to other libraries. Many of them do -# unsafe things in functions marked __atttribute__((constructor)). -# This we statically link to a library containing only the minimal -# libvirt client code, not libvirt.so itself. +# virt-login-shell will be setuid, and must not link to anything +# except glibc. It wil scrub the environment and then invoke the +# real virt-login-shell-helper binary. virt_login_shell_SOURCES = \ + virt-login-shell.c + +virt_login_shell_helper_SOURCES = \ virt-login-shell-helper.c -virt_login_shell_LDFLAGS = \ +virt_login_shell_helper_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(NULL) -virt_login_shell_LDADD = \ - $(STATIC_BINARIES) \ - ../src/libvirt-setuid-rpc-client.la \ +virt_login_shell_helper_LDADD = \ + ../src/libvirt.la \ + ../src/libvirt-lxc.la \ ../gnulib/lib/libgnu.la -virt_login_shell_CFLAGS = \ - -DLIBVIRT_SETUID_RPC_CLIENT \ +virt_login_shell_helper_CFLAGS = \ $(AM_CFLAGS) \ $(NULL) diff --git a/tools/virt-login-shell-helper.c b/tools/virt-login-shell-helper.c index f06eb1464a..8dc3bedaa0 100644 --- a/tools/virt-login-shell-helper.c +++ b/tools/virt-login-shell-helper.c @@ -157,8 +157,10 @@ main(int argc, char **argv) pid_t cpid = -1; int ret = EXIT_CANCELED; int status; - uid_t uid = getuid(); - gid_t gid = getgid(); + unsigned long long uidval; + unsigned long long gidval; + uid_t uid; + gid_t gid; char *name = NULL; char **shargv = NULL; size_t shargvlen = 0; @@ -199,6 +201,16 @@ main(int argc, char **argv) if (virGettextInitialize() < 0) return ret; + if (geteuid() != 0) { + fprintf(stderr, _("%s: must be run as root\n"), argv[0]); + return ret; + } + + if (getuid() != 0) { + fprintf(stderr, _("%s: must not be run setuid root\n"), argv[0]); + return ret; + } + while ((arg = getopt_long(argc, argv, "hVc:", opt, &longindex)) != -1) { switch (arg) { case 'h': @@ -220,17 +232,29 @@ main(int argc, char **argv) } } - if (argc > optind) { - virReportSystemError(EINVAL, _("%s takes no options"), progname); + if (optind != (argc - 2)) { + virReportSystemError(EINVAL, _("%s expects UID and GID parameters"), progname); goto cleanup; } - if (uid == 0) { - virReportSystemError(EPERM, _("%s must be run by non root users"), - progname); + if (virStrToLong_ull(argv[optind], NULL, 10, &uidval) < 0 || + ((uid_t)uidval) != uidval) { + virReportSystemError(EINVAL, _("%s cannot parse UID '%s'"), + progname, argv[optind]); goto cleanup; } + optind++; + if (virStrToLong_ull(argv[optind], NULL, 10, &gidval) < 0 || + ((gid_t)gidval) != gidval) { + virReportSystemError(EINVAL, _("%s cannot parse GID '%s'"), + progname, argv[optind]); + goto cleanup; + } + + uid = (uid_t)uidval; + gid = (gid_t)gidval; + name = virGetUserName(uid); if (!name) goto cleanup; diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c new file mode 100644 index 0000000000..41d0b349aa --- /dev/null +++ b/tools/virt-login-shell.c @@ -0,0 +1,84 @@ +/* + * virt-login-shell.c: a setuid shell to connect to a container + * + * Copyright (C) 2019 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * . + */ + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "configmake.h" +#include "intprops.h" + +int main(int argc, char **argv) { + char uidstr[INT_BUFSIZE_BOUND(uid_t)]; + char gidstr[INT_BUFSIZE_BOUND(gid_t)]; + const char *const newargv[] = { + LIBEXECDIR "/virt-login-shell-helper", + uidstr, + gidstr, + NULL, + }; + char *newenv[] = { + NULL, + NULL, + }; + char *term = getenv("TERM"); + + if (getuid() == 0 || getgid() == 0) { + fprintf(stderr, "%s: must not be run as root\n", argv[0]); + exit(EXIT_FAILURE); + } + + if (geteuid() != 0) { + fprintf(stderr, "%s: must be run as setuid root\n", argv[0]); + exit(EXIT_FAILURE); + } + + if (argc != 1) { + fprintf(stderr, "%s: no arguments expected\n", argv[0]); + exit(EXIT_FAILURE); + } + + if (term && + asprintf(&(newenv[0]), "TERM=%s", term) < 0) { + fprintf(stderr, "%s: cannot set TERM env variable: %s\n", + argv[0], strerror(errno)); + exit(EXIT_FAILURE); + } + + assert(snprintf(uidstr, sizeof(uidstr), "%d", getuid()) < sizeof(uidstr)); + assert(snprintf(gidstr, sizeof(gidstr), "%d", getgid()) < sizeof(gidstr)); + + if (setuid(0) < 0) { + fprintf(stderr, "%s: unable to set real UID to root: %s\n", + argv[0], strerror(errno)); + exit(EXIT_FAILURE); + } + + execve(newargv[0], (char *const*)newargv, newenv); + fprintf(stderr, "%s: failed to run %s/virt-login-shell-helper: %s\n", + argv[0], LIBEXECDIR, strerror(errno)); + exit(EXIT_FAILURE); +}