[openssh-commits] [openssh] 01/03: upstream: move subprocess() from auth.c to misc.c

git+noreply at mindrot.org git+noreply at mindrot.org
Tue Dec 22 15:44:21 AEDT 2020


This is an automated email from the git hooks/post-receive script.

djm pushed a commit to branch master
in repository openssh.

commit a34e14a5a0071de2036826a00197ce38c8b4ba8b
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Tue Dec 22 00:12:22 2020 +0000

    upstream: move subprocess() from auth.c to misc.c
    
    make privilege dropping optional but allow it via callbacks (to avoid
    need to link uidswap.c everywhere)
    
    add some other flags (keep environment, disable strict path safety check)
    that make this more useful for client-side use.
    
    feedback & ok markus@
    
    OpenBSD-Commit-ID: a80ea9fdcc156f1a18e9c166122c759fae1637bf
---
 auth.c         | 154 +--------------------------------------------------
 auth.h         |   8 +--
 auth2-pubkey.c |  12 ++--
 misc.c         | 171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 misc.h         |  12 +++-
 5 files changed, 190 insertions(+), 167 deletions(-)

diff --git a/auth.c b/auth.c
index e1bf4e75..2b77abca 100644
--- a/auth.c
+++ b/auth.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth.c,v 1.150 2020/12/20 23:36:51 djm Exp $ */
+/* $OpenBSD: auth.c,v 1.151 2020/12/22 00:12:22 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -841,158 +841,6 @@ auth_get_canonical_hostname(struct ssh *ssh, int use_dns)
 	}
 }
 
-/*
- * Runs command in a subprocess with a minimal environment.
- * Returns pid on success, 0 on failure.
- * The child stdout and stderr maybe captured, left attached or sent to
- * /dev/null depending on the contents of flags.
- * "tag" is prepended to log messages.
- * NB. "command" is only used for logging; the actual command executed is
- * av[0].
- */
-pid_t
-subprocess(const char *tag, struct passwd *pw, const char *command,
-    int ac, char **av, FILE **child, u_int flags)
-{
-	FILE *f = NULL;
-	struct stat st;
-	int fd, devnull, p[2], i;
-	pid_t pid;
-	char *cp, errmsg[512];
-	u_int envsize;
-	char **child_env;
-
-	if (child != NULL)
-		*child = NULL;
-
-	debug3_f("%s command \"%s\" running as %s (flags 0x%x)",
-	    tag, command, pw->pw_name, flags);
-
-	/* Check consistency */
-	if ((flags & SSH_SUBPROCESS_STDOUT_DISCARD) != 0 &&
-	    (flags & SSH_SUBPROCESS_STDOUT_CAPTURE) != 0) {
-		error_f("inconsistent flags");
-		return 0;
-	}
-	if (((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) == 0) != (child == NULL)) {
-		error_f("inconsistent flags/output");
-		return 0;
-	}
-
-	/*
-	 * If executing an explicit binary, then verify the it exists
-	 * and appears safe-ish to execute
-	 */
-	if (!path_absolute(av[0])) {
-		error("%s path is not absolute", tag);
-		return 0;
-	}
-	temporarily_use_uid(pw);
-	if (stat(av[0], &st) == -1) {
-		error("Could not stat %s \"%s\": %s", tag,
-		    av[0], strerror(errno));
-		restore_uid();
-		return 0;
-	}
-	if (safe_path(av[0], &st, NULL, 0, errmsg, sizeof(errmsg)) != 0) {
-		error("Unsafe %s \"%s\": %s", tag, av[0], errmsg);
-		restore_uid();
-		return 0;
-	}
-	/* Prepare to keep the child's stdout if requested */
-	if (pipe(p) == -1) {
-		error("%s: pipe: %s", tag, strerror(errno));
-		restore_uid();
-		return 0;
-	}
-	restore_uid();
-
-	switch ((pid = fork())) {
-	case -1: /* error */
-		error("%s: fork: %s", tag, strerror(errno));
-		close(p[0]);
-		close(p[1]);
-		return 0;
-	case 0: /* child */
-		/* Prepare a minimal environment for the child. */
-		envsize = 5;
-		child_env = xcalloc(sizeof(*child_env), envsize);
-		child_set_env(&child_env, &envsize, "PATH", _PATH_STDPATH);
-		child_set_env(&child_env, &envsize, "USER", pw->pw_name);
-		child_set_env(&child_env, &envsize, "LOGNAME", pw->pw_name);
-		child_set_env(&child_env, &envsize, "HOME", pw->pw_dir);
-		if ((cp = getenv("LANG")) != NULL)
-			child_set_env(&child_env, &envsize, "LANG", cp);
-
-		for (i = 0; i < NSIG; i++)
-			ssh_signal(i, SIG_DFL);
-
-		if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
-			error("%s: open %s: %s", tag, _PATH_DEVNULL,
-			    strerror(errno));
-			_exit(1);
-		}
-		if (dup2(devnull, STDIN_FILENO) == -1) {
-			error("%s: dup2: %s", tag, strerror(errno));
-			_exit(1);
-		}
-
-		/* Set up stdout as requested; leave stderr in place for now. */
-		fd = -1;
-		if ((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) != 0)
-			fd = p[1];
-		else if ((flags & SSH_SUBPROCESS_STDOUT_DISCARD) != 0)
-			fd = devnull;
-		if (fd != -1 && dup2(fd, STDOUT_FILENO) == -1) {
-			error("%s: dup2: %s", tag, strerror(errno));
-			_exit(1);
-		}
-		closefrom(STDERR_FILENO + 1);
-
-		/* Don't use permanently_set_uid() here to avoid fatal() */
-		if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1) {
-			error("%s: setresgid %u: %s", tag, (u_int)pw->pw_gid,
-			    strerror(errno));
-			_exit(1);
-		}
-		if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) {
-			error("%s: setresuid %u: %s", tag, (u_int)pw->pw_uid,
-			    strerror(errno));
-			_exit(1);
-		}
-		/* stdin is pointed to /dev/null at this point */
-		if ((flags & SSH_SUBPROCESS_STDOUT_DISCARD) != 0 &&
-		    dup2(STDIN_FILENO, STDERR_FILENO) == -1) {
-			error("%s: dup2: %s", tag, strerror(errno));
-			_exit(1);
-		}
-
-		execve(av[0], av, child_env);
-		error("%s exec \"%s\": %s", tag, command, strerror(errno));
-		_exit(127);
-	default: /* parent */
-		break;
-	}
-
-	close(p[1]);
-	if ((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) == 0)
-		close(p[0]);
-	else if ((f = fdopen(p[0], "r")) == NULL) {
-		error("%s: fdopen: %s", tag, strerror(errno));
-		close(p[0]);
-		/* Don't leave zombie child */
-		kill(pid, SIGTERM);
-		while (waitpid(pid, NULL, 0) == -1 && errno == EINTR)
-			;
-		return 0;
-	}
-	/* Success */
-	debug3_f("%s pid %ld", tag, (long)pid);
-	if (child != NULL)
-		*child = f;
-	return pid;
-}
-
 /* These functions link key/cert options to the auth framework */
 
 /* Log sshauthopt options locally and (optionally) for remote transmission */
diff --git a/auth.h b/auth.h
index becc672b..43c7d3d4 100644
--- a/auth.h
+++ b/auth.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth.h,v 1.100 2019/09/06 05:23:55 djm Exp $ */
+/* $OpenBSD: auth.h,v 1.101 2020/12/22 00:12:22 djm Exp $ */
 
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
@@ -225,12 +225,6 @@ void	 auth_debug_reset(void);
 
 struct passwd *fakepw(void);
 
-#define	SSH_SUBPROCESS_STDOUT_DISCARD  (1)     /* Discard stdout */
-#define	SSH_SUBPROCESS_STDOUT_CAPTURE  (1<<1)  /* Redirect stdout */
-#define	SSH_SUBPROCESS_STDERR_DISCARD  (1<<2)  /* Discard stderr */
-pid_t	subprocess(const char *, struct passwd *,
-    const char *, int, char **, FILE **, u_int flags);
-
 int	 sys_auth_passwd(struct ssh *, const char *);
 
 #if defined(KRB5) && !defined(HEIMDAL)
diff --git a/auth2-pubkey.c b/auth2-pubkey.c
index 307afa56..14863cbf 100644
--- a/auth2-pubkey.c
+++ b/auth2-pubkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-pubkey.c,v 1.102 2020/12/17 23:28:50 djm Exp $ */
+/* $OpenBSD: auth2-pubkey.c,v 1.103 2020/12/22 00:12:22 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -530,9 +530,10 @@ match_principals_command(struct ssh *ssh, struct passwd *user_pw,
 	/* Prepare a printable command for logs, etc. */
 	command = argv_assemble(ac, av);
 
-	if ((pid = subprocess("AuthorizedPrincipalsCommand", runas_pw, command,
+	if ((pid = subprocess("AuthorizedPrincipalsCommand", command,
 	    ac, av, &f,
-	    SSH_SUBPROCESS_STDOUT_CAPTURE|SSH_SUBPROCESS_STDERR_DISCARD)) == 0)
+	    SSH_SUBPROCESS_STDOUT_CAPTURE|SSH_SUBPROCESS_STDERR_DISCARD,
+	    runas_pw, temporarily_use_uid, restore_uid)) == 0)
 		goto out;
 
 	uid_swapped = 1;
@@ -968,9 +969,10 @@ user_key_command_allowed2(struct ssh *ssh, struct passwd *user_pw,
 		xasprintf(&command, "%s %s", av[0], av[1]);
 	}
 
-	if ((pid = subprocess("AuthorizedKeysCommand", runas_pw, command,
+	if ((pid = subprocess("AuthorizedKeysCommand", command,
 	    ac, av, &f,
-	    SSH_SUBPROCESS_STDOUT_CAPTURE|SSH_SUBPROCESS_STDERR_DISCARD)) == 0)
+	    SSH_SUBPROCESS_STDOUT_CAPTURE|SSH_SUBPROCESS_STDERR_DISCARD,
+	    runas_pw, temporarily_use_uid, restore_uid)) == 0)
 		goto out;
 
 	uid_swapped = 1;
diff --git a/misc.c b/misc.c
index 7c9460e8..68feebf7 100644
--- a/misc.c
+++ b/misc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.c,v 1.156 2020/11/27 00:49:58 djm Exp $ */
+/* $OpenBSD: misc.c,v 1.157 2020/12/22 00:12:22 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2005-2020 Damien Miller.  All rights reserved.
@@ -2479,3 +2479,172 @@ stdfd_devnull(int do_stdin, int do_stdout, int do_stderr)
 		close(devnull);
 	return ret;
 }
+
+/*
+ * Runs command in a subprocess with a minimal environment.
+ * Returns pid on success, 0 on failure.
+ * The child stdout and stderr maybe captured, left attached or sent to
+ * /dev/null depending on the contents of flags.
+ * "tag" is prepended to log messages.
+ * NB. "command" is only used for logging; the actual command executed is
+ * av[0].
+ */
+pid_t
+subprocess(const char *tag, const char *command,
+    int ac, char **av, FILE **child, u_int flags,
+    struct passwd *pw, privdrop_fn *drop_privs, privrestore_fn *restore_privs)
+{
+	FILE *f = NULL;
+	struct stat st;
+	int fd, devnull, p[2], i;
+	pid_t pid;
+	char *cp, errmsg[512];
+	u_int nenv = 0;
+	char **env = NULL;
+
+	/* If dropping privs, then must specify user and restore function */
+	if (drop_privs != NULL && (pw == NULL || restore_privs == NULL)) {
+		error("%s: inconsistent arguments", tag); /* XXX fatal? */
+		return 0;
+	}
+	if (pw == NULL && (pw = getpwuid(getuid())) == NULL) {
+		error("%s: no user for current uid", tag);
+		return 0;
+	}
+	if (child != NULL)
+		*child = NULL;
+
+	debug3_f("%s command \"%s\" running as %s (flags 0x%x)",
+	    tag, command, pw->pw_name, flags);
+
+	/* Check consistency */
+	if ((flags & SSH_SUBPROCESS_STDOUT_DISCARD) != 0 &&
+	    (flags & SSH_SUBPROCESS_STDOUT_CAPTURE) != 0) {
+		error_f("inconsistent flags");
+		return 0;
+	}
+	if (((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) == 0) != (child == NULL)) {
+		error_f("inconsistent flags/output");
+		return 0;
+	}
+
+	/*
+	 * If executing an explicit binary, then verify the it exists
+	 * and appears safe-ish to execute
+	 */
+	if (!path_absolute(av[0])) {
+		error("%s path is not absolute", tag);
+		return 0;
+	}
+	if (drop_privs != NULL)
+		drop_privs(pw);
+	if (stat(av[0], &st) == -1) {
+		error("Could not stat %s \"%s\": %s", tag,
+		    av[0], strerror(errno));
+		goto restore_return;
+	}
+	if ((flags & SSH_SUBPROCESS_UNSAFE_PATH) == 0 &&
+	    safe_path(av[0], &st, NULL, 0, errmsg, sizeof(errmsg)) != 0) {
+		error("Unsafe %s \"%s\": %s", tag, av[0], errmsg);
+		goto restore_return;
+	}
+	/* Prepare to keep the child's stdout if requested */
+	if (pipe(p) == -1) {
+		error("%s: pipe: %s", tag, strerror(errno));
+ restore_return:
+		if (restore_privs != NULL)
+			restore_privs();
+		return 0;
+	}
+	if (restore_privs != NULL)
+		restore_privs();
+
+	switch ((pid = fork())) {
+	case -1: /* error */
+		error("%s: fork: %s", tag, strerror(errno));
+		close(p[0]);
+		close(p[1]);
+		return 0;
+	case 0: /* child */
+		/* Prepare a minimal environment for the child. */
+		if ((flags & SSH_SUBPROCESS_PRESERVE_ENV) == 0) {
+			nenv = 5;
+			env = xcalloc(sizeof(*env), nenv);
+			child_set_env(&env, &nenv, "PATH", _PATH_STDPATH);
+			child_set_env(&env, &nenv, "USER", pw->pw_name);
+			child_set_env(&env, &nenv, "LOGNAME", pw->pw_name);
+			child_set_env(&env, &nenv, "HOME", pw->pw_dir);
+			if ((cp = getenv("LANG")) != NULL)
+				child_set_env(&env, &nenv, "LANG", cp);
+		}
+
+		for (i = 0; i < NSIG; i++)
+			ssh_signal(i, SIG_DFL);
+
+		if ((devnull = open(_PATH_DEVNULL, O_RDWR)) == -1) {
+			error("%s: open %s: %s", tag, _PATH_DEVNULL,
+			    strerror(errno));
+			_exit(1);
+		}
+		if (dup2(devnull, STDIN_FILENO) == -1) {
+			error("%s: dup2: %s", tag, strerror(errno));
+			_exit(1);
+		}
+
+		/* Set up stdout as requested; leave stderr in place for now. */
+		fd = -1;
+		if ((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) != 0)
+			fd = p[1];
+		else if ((flags & SSH_SUBPROCESS_STDOUT_DISCARD) != 0)
+			fd = devnull;
+		if (fd != -1 && dup2(fd, STDOUT_FILENO) == -1) {
+			error("%s: dup2: %s", tag, strerror(errno));
+			_exit(1);
+		}
+		closefrom(STDERR_FILENO + 1);
+
+		if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1) {
+			error("%s: setresgid %u: %s", tag, (u_int)pw->pw_gid,
+			    strerror(errno));
+			_exit(1);
+		}
+		if (setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) {
+			error("%s: setresuid %u: %s", tag, (u_int)pw->pw_uid,
+			    strerror(errno));
+			_exit(1);
+		}
+		/* stdin is pointed to /dev/null at this point */
+		if ((flags & SSH_SUBPROCESS_STDOUT_DISCARD) != 0 &&
+		    dup2(STDIN_FILENO, STDERR_FILENO) == -1) {
+			error("%s: dup2: %s", tag, strerror(errno));
+			_exit(1);
+		}
+		if (env != NULL)
+			execve(av[0], av, env);
+		else
+			execv(av[0], av);
+		error("%s %s \"%s\": %s", tag, env == NULL ? "execv" : "execve",
+		    command, strerror(errno));
+		_exit(127);
+	default: /* parent */
+		break;
+	}
+
+	close(p[1]);
+	if ((flags & SSH_SUBPROCESS_STDOUT_CAPTURE) == 0)
+		close(p[0]);
+	else if ((f = fdopen(p[0], "r")) == NULL) {
+		error("%s: fdopen: %s", tag, strerror(errno));
+		close(p[0]);
+		/* Don't leave zombie child */
+		kill(pid, SIGTERM);
+		while (waitpid(pid, NULL, 0) == -1 && errno == EINTR)
+			;
+		return 0;
+	}
+	/* Success */
+	debug3_f("%s pid %ld", tag, (long)pid);
+	if (child != NULL)
+		*child = f;
+	return pid;
+}
diff --git a/misc.h b/misc.h
index b8120a14..c60fe202 100644
--- a/misc.h
+++ b/misc.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.h,v 1.90 2020/11/27 00:49:58 djm Exp $ */
+/* $OpenBSD: misc.h,v 1.91 2020/12/22 00:12:22 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo at cs.hut.fi>
@@ -99,6 +99,16 @@ void	 sock_set_v6only(int);
 struct passwd *pwcopy(struct passwd *);
 const char *ssh_gai_strerror(int);
 
+typedef void privdrop_fn(struct passwd *);
+typedef void privrestore_fn(void);
+#define	SSH_SUBPROCESS_STDOUT_DISCARD	(1)     /* Discard stdout */
+#define	SSH_SUBPROCESS_STDOUT_CAPTURE	(1<<1)  /* Redirect stdout */
+#define	SSH_SUBPROCESS_STDERR_DISCARD	(1<<2)  /* Discard stderr */
+#define	SSH_SUBPROCESS_UNSAFE_PATH	(1<<3)	/* Don't check for safe cmd */
+#define	SSH_SUBPROCESS_PRESERVE_ENV	(1<<4)	/* Keep parent environment */
+pid_t subprocess(const char *, const char *, int, char **, FILE **, u_int,
+    struct passwd *, privdrop_fn *, privrestore_fn *);
+
 typedef struct arglist arglist;
 struct arglist {
 	char    **list;

-- 
To stop receiving notification emails like this one, please contact
djm at mindrot.org.


More information about the openssh-commits mailing list