From 3af55c5cb3cfec6d0618a33bcb12b0dec6de070a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 31 Oct 2014 01:07:49 +0100 Subject: [PATCH] qrexec: use PAM directly instead of calling su to setup the session Instead of calling 'su' to switch the user, use own implementation of this. Thanks to PAM it's pretty simple. The main reason is to have control over process waiting for session termination (to call pam_close_sesion/pam_end). Especially we don't want it to keep std* fds open, which would prevent qrexec-agent from receiving EOF when one of them will be closed. Also, this will preserve QREXEC_AGENT_PID environment variable. Fixes QubesOS/qubes-issues#2851 --- debian/control | 1 + debian/qubes-core-agent-qrexec.install | 1 + qrexec/Makefile | 3 +- qrexec/qrexec-agent.c | 144 +++++++++++++++++++++++++ qrexec/qrexec.pam | 9 ++ rpm_spec/core-agent.spec | 2 + 6 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 qrexec/qrexec.pam diff --git a/debian/control b/debian/control index 18545c4..3ec505a 100644 --- a/debian/control +++ b/debian/control @@ -3,6 +3,7 @@ Section: admin Priority: extra Maintainer: unman Build-Depends: + libpam0g-dev, libqrexec-utils-dev, libqubes-rpc-filecopy-dev (>= 3.1.3), libvchan-xen-dev, diff --git a/debian/qubes-core-agent-qrexec.install b/debian/qubes-core-agent-qrexec.install index 1c61677..0696b42 100644 --- a/debian/qubes-core-agent-qrexec.install +++ b/debian/qubes-core-agent-qrexec.install @@ -1,3 +1,4 @@ +etc/pam.d/qrexec lib/systemd/system/qubes-qrexec-agent.service usr/bin/qrexec-client-vm usr/bin/qrexec-fork-server diff --git a/qrexec/Makefile b/qrexec/Makefile index 96c9c78..40ddc3a 100644 --- a/qrexec/Makefile +++ b/qrexec/Makefile @@ -1,7 +1,7 @@ CC=gcc CFLAGS+=-I. -g -O2 -Wall -Wextra -Werror -pie -fPIC `pkg-config --cflags vchan-$(BACKEND_VMM)` LDFLAGS=-pie -LDLIBS=`pkg-config --libs vchan-$(BACKEND_VMM)` -lqrexec-utils +LDLIBS=`pkg-config --libs vchan-$(BACKEND_VMM)` -lqrexec-utils -lpam all: qrexec-agent qrexec-client-vm qrexec-fork-server qrexec-agent: qrexec-agent.o qrexec-agent-data.o @@ -19,4 +19,5 @@ install: ln -s ../../bin/qrexec-client-vm $(DESTDIR)/usr/lib/qubes/qrexec_client_vm install qrexec-fork-server $(DESTDIR)/usr/bin install qubes-rpc-multiplexer $(DESTDIR)/usr/lib/qubes + install -D -m 0644 qrexec.pam $(DESTDIR)/etc/pam.d/qrexec diff --git a/qrexec/qrexec-agent.c b/qrexec/qrexec-agent.c index fba2f47..6a5a139 100644 --- a/qrexec/qrexec-agent.c +++ b/qrexec/qrexec-agent.c @@ -20,6 +20,8 @@ */ #define _GNU_SOURCE +#define HAVE_PAM + #include #include #include @@ -35,6 +37,9 @@ #include #include #include +#ifdef HAVE_PAM +#include +#endif #include "qrexec.h" #include #include "libqrexec-utils.h" @@ -65,6 +70,36 @@ void no_colon_in_cmd() exit(1); } +#ifdef HAVE_PAM +int pam_conv_callback(int num_msg, const struct pam_message **msg, + struct pam_response **resp, void *appdata_ptr __attribute__((__unused__))) +{ + int i; + struct pam_response *resp_array = + calloc(sizeof(struct pam_response), num_msg); + + if (resp_array == NULL) + return PAM_BUF_ERR; + + for (i=0; imsg_style == PAM_ERROR_MSG) + fprintf(stderr, "%s", msg[i]->msg); + if (msg[i]->msg_style == PAM_PROMPT_ECHO_OFF || + msg[i]->msg_style == PAM_PROMPT_ECHO_ON) { + resp_array[i].resp = strdup(""); + resp_array[i].resp_retcode = 0; + } + } + *resp = resp_array; + return PAM_SUCCESS; +} + +static struct pam_conv conv = { + pam_conv_callback, + NULL +}; +#endif + /* Start program requested by dom0 in already prepared process * (stdin/stdout/stderr already set, etc) * Called in two cases: @@ -86,6 +121,16 @@ void do_exec(const char *cmd) { char buf[strlen(QUBES_RPC_MULTIPLEXER_PATH) + strlen(cmd) - RPC_REQUEST_COMMAND_LEN + 1]; char *realcmd = index(cmd, ':'), *user; +#ifdef HAVE_PAM + int retval, status; + pam_handle_t *pamh=NULL; + struct passwd *pw; + struct passwd pw_copy; + pid_t child, pid; + char **env; + char pid_s[32]; +#endif + if (!realcmd) no_colon_in_cmd(); /* mark end of username and move to command */ @@ -103,9 +148,108 @@ void do_exec(const char *cmd) signal(SIGCHLD, SIG_DFL); signal(SIGPIPE, SIG_DFL); +#ifdef HAVE_PAM + pw = getpwnam (user); + if (! (pw && pw->pw_name && pw->pw_name[0] && pw->pw_dir && pw->pw_dir[0] + && pw->pw_passwd)) { + fprintf(stderr, "user %s does not exist", user); + exit(1); + } + + /* Make a copy of the password information and point pw at the local + * copy instead. Otherwise, some systems (e.g. Linux) would clobber + * the static data through the getlogin call. + */ + pw_copy = *pw; + pw = &pw_copy; + pw->pw_name = strdup(pw->pw_name); + pw->pw_passwd = strdup(pw->pw_passwd); + pw->pw_dir = strdup(pw->pw_dir); + pw->pw_shell = strdup(pw->pw_shell); + endpwent(); + + retval = pam_start("qrexec", user, &conv, &pamh); + if (retval != PAM_SUCCESS) + goto error; + + retval = pam_authenticate(pamh, 0); + if (retval != PAM_SUCCESS) + goto error; + + retval = initgroups(pw->pw_name, pw->pw_gid); + if (retval == -1) { + perror("initgroups"); + goto error; + } + + retval = pam_setcred(pamh, PAM_ESTABLISH_CRED); + if (retval != PAM_SUCCESS) + goto error; + + retval = pam_open_session(pamh, 0); + if (retval != PAM_SUCCESS) + goto error; + + /* provide this variable to child process */ + snprintf(pid_s, sizeof(pid_s), "QREXEC_AGENT_PID=%d", getppid()); + retval = pam_putenv(pamh, pid_s); + if (retval != PAM_SUCCESS) + goto error; + + /* FORK HERE */ + child = fork (); + + switch (child) { + case -1: + goto error; + case 0: + /* child */ + if (setgid (pw->pw_gid)) + exit(126); + if (setuid (pw->pw_uid)) + exit(126); + setsid(); + /* This is a copy but don't care to free as we exec later anyways. */ + env = pam_getenvlist (pamh); + execle("/bin/sh", "sh", "-c", realcmd, (char*)NULL, env); + exit(127); + default: + /* parent */ + /* close std*, so when child process closes them, qrexec-agent will receive EOF */ + /* this is the main purpose of this reimplementation of /bin/su... */ + close(0); + close(1); + close(2); + } + + /* reachable only in parent */ + pid = waitpid (child, &status, 0); + if (pid != (pid_t)-1) { + if (WIFSIGNALED (status)) + status = WTERMSIG (status) + 128; + else + status = WEXITSTATUS (status); + } else + status = 1; + + retval = pam_close_session (pamh, 0); + + retval = pam_setcred (pamh, PAM_DELETE_CRED | PAM_SILENT); + + if (pam_end(pamh, retval) != PAM_SUCCESS) { /* close Linux-PAM */ + pamh = NULL; + exit(1); + } + exit(status); +error: + pam_end(pamh, PAM_ABORT); + exit(1); +#else execl("/bin/su", "su", "-", user, "-c", realcmd, NULL); perror("execl"); exit(1); +#endif + } void handle_vchan_error(const char *op) diff --git a/qrexec/qrexec.pam b/qrexec/qrexec.pam new file mode 100644 index 0000000..c6896bc --- /dev/null +++ b/qrexec/qrexec.pam @@ -0,0 +1,9 @@ +#%PAM-1.0 +auth sufficient pam_rootok.so +auth substack system-auth +auth include postlogin +account sufficient pam_succeed_if.so uid = 0 use_uid quiet +account include system-auth +password include system-auth +session include system-auth +session include postlogin diff --git a/rpm_spec/core-agent.spec b/rpm_spec/core-agent.spec index 3136883..07e1c8c 100644 --- a/rpm_spec/core-agent.spec +++ b/rpm_spec/core-agent.spec @@ -157,6 +157,7 @@ BuildRequires: xen-devel BuildRequires: libX11-devel BuildRequires: qubes-utils-devel >= 3.1.3 BuildRequires: qubes-libvchan-%{backend_vmm}-devel +BuildRequires: pam-devel %description The Qubes core files for installation inside a Qubes VM. @@ -611,6 +612,7 @@ rm -f %{name}-%{version} %{python3_sitelib}/dnf-plugins/* %files qrexec +%config(noreplace) /etc/pam.d/qrexec /usr/bin/qrexec-fork-server /usr/bin/qrexec-client-vm /usr/lib/qubes/qrexec-agent