Allow some keywords to expand shell-style ${ENV} environment

variables on the client side.  The supported keywords are
CertificateFile, ControlPath, IdentityAgent and IdentityFile, plus
LocalForward and RemoteForward when used for Unix domain socket
paths.  This would for example allow forwarding of Unix domain
socket paths that change at runtime.  bz#3140, ok djm@

5 files changed, 230 insertions, 53 deletions

diff --git a/usr.bin/ssh/misc.c b/usr.bin/ssh/misc.c
index e52656cad21..73c86438904 100644
--- a/usr.bin/ssh/misc.c
+++ b/usr.bin/ssh/misc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.c,v 1.149 2020/05/29 01:20:46 dtucker Exp $ */
+/* $OpenBSD: misc.c,v 1.150 2020/05/29 04:25:40 dtucker Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2005-2020 Damien Miller.  All rights reserved.
@@ -1049,45 +1049,90 @@ tilde_expand_filename(const char *filename, uid_t uid)
 }
 
 /*
- * Expand a string with a set of %[char] escapes. A number of escapes may be
- * specified as (char *escape_chars, char *replacement) pairs. The list must
- * be terminated by a NULL escape_char. Returns replaced string in memory
- * allocated by xmalloc.
+ * Expand a string with a set of %[char] escapes and/or ${ENVIRONMENT}
+ * substitutions.  A number of escapes may be specified as
+ * (char *escape_chars, char *replacement) pairs. The list must be terminated
+ * by a NULL escape_char. Returns replaced string in memory allocated by
+ * xmalloc which the caller must free.
  */
-char *
-percent_expand(const char *string, ...)
+static char *
+vdollar_percent_expand(int *parseerror, int dollar, int percent,
+    const char *string, va_list ap)
 {
 #define EXPAND_MAX_KEYS	16
-	u_int num_keys, i;
+	u_int num_keys = 0, i;
 	struct {
 		const char *key;
 		const char *repl;
 	} keys[EXPAND_MAX_KEYS];
 	struct sshbuf *buf;
-	va_list ap;
-	int r;
-	char *ret;
+	int r, missingvar = 0;
+	char *ret = NULL, *var, *varend, *val;
+	size_t len;
 
 	if ((buf = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __func__);
-
-	/* Gather keys */
-	va_start(ap, string);
-	for (num_keys = 0; num_keys < EXPAND_MAX_KEYS; num_keys++) {
-		keys[num_keys].key = va_arg(ap, char *);
-		if (keys[num_keys].key == NULL)
-			break;
-		keys[num_keys].repl = va_arg(ap, char *);
-		if (keys[num_keys].repl == NULL)
-			fatal("%s: NULL replacement", __func__);
+	if (parseerror == NULL)
+		fatal("%s: null parseerror arg", __func__);
+	*parseerror = 1;
+
+	/* Gather keys if we're doing percent expansion. */
+	if (percent) {
+		for (num_keys = 0; num_keys < EXPAND_MAX_KEYS; num_keys++) {
+			keys[num_keys].key = va_arg(ap, char *);
+			if (keys[num_keys].key == NULL)
+				break;
+			keys[num_keys].repl = va_arg(ap, char *);
+			if (keys[num_keys].repl == NULL)
+				fatal("%s: NULL replacement for token %s", __func__, keys[num_keys].key);
+		}
+		if (num_keys == EXPAND_MAX_KEYS && va_arg(ap, char *) != NULL)
+			fatal("%s: too many keys", __func__);
+		if (num_keys == 0)
+			fatal("%s: percent expansion without token list",
+			    __func__);
 	}
-	if (num_keys == EXPAND_MAX_KEYS && va_arg(ap, char *) != NULL)
-		fatal("%s: too many keys", __func__);
-	va_end(ap);
 
 	/* Expand string */
 	for (i = 0; *string != '\0'; string++) {
-		if (*string != '%') {
+		/* Optionally process ${ENVIRONMENT} expansions. */
+		if (dollar && string[0] == '$' && string[1] == '{') {
+			string += 2;  /* skip over '${' */
+			if ((varend = strchr(string, '}')) == NULL) {
+				error("%s: environment variable '%s' missing "
+				   "closing '}'", __func__, string);
+				goto out;
+			}
+			len = varend - string;
+			if (len == 0) {
+				error("%s: zero-length environment variable",
+				    __func__);
+				goto out;
+			}
+			var = xmalloc(len + 1);
+			(void)strlcpy(var, string, len + 1);
+			if ((val = getenv(var)) == NULL) {
+				error("%s: env var ${%s} has no value",
+				    __func__, var);
+				missingvar = 1;
+			} else {
+				debug3("%s: expand ${%s} -> '%s'", __func__,
+				    var, val);
+				if ((r = sshbuf_put(buf, val, strlen(val))) !=0)
+					fatal("%s: sshbuf_put: %s", __func__,
+					    ssh_err(r));
+			}
+			free(var);
+			string += len;
+			continue;
+		}
+
+		/*
+		 * Process percent expansions if we have a list of TOKENs.
+		 * If we're not doing percent expansion everything just gets
+		 * appended here.
+		 */
+		if (*string != '%' || !percent) {
  append:
 			if ((r = sshbuf_put_u8(buf, *string)) != 0) {
 				fatal("%s: sshbuf_put_u8: %s",
@@ -1099,8 +1144,10 @@ percent_expand(const char *string, ...)
 		/* %% case */
 		if (*string == '%')
 			goto append;
-		if (*string == '\0')
-			fatal("%s: invalid format", __func__);
+		if (*string == '\0') {
+			error("%s: invalid format", __func__);
+			goto out;
+		}
 		for (i = 0; i < num_keys; i++) {
 			if (strchr(keys[i].key, *string) != NULL) {
 				if ((r = sshbuf_put(buf, keys[i].repl,
@@ -1111,16 +1158,72 @@ percent_expand(const char *string, ...)
 				break;
 			}
 		}
-		if (i >= num_keys)
-			fatal("%s: unknown key %%%c", __func__, *string);
+		if (i >= num_keys) {
+			error("%s: unknown key %%%c", __func__, *string);
+			goto out;
+		}
 	}
-	if ((ret = sshbuf_dup_string(buf)) == NULL)
+	if (!missingvar && (ret = sshbuf_dup_string(buf)) == NULL)
 		fatal("%s: sshbuf_dup_string failed", __func__);
+	*parseerror = 0;
+ out:
 	sshbuf_free(buf);
-	return ret;
+	return *parseerror ? NULL : ret;
 #undef EXPAND_MAX_KEYS
 }
 
+char *
+dollar_expand(int *parseerr, const char *string)
+{
+	char *ret;
+	int err;
+	va_list ap;
+
+	memset(ap, 0, sizeof(ap));  /* unused */
+	ret = vdollar_percent_expand(&err, 1, 0, string, ap);
+	if (parseerr != NULL)
+		*parseerr = err;
+	return ret;
+}
+
+/*
+ * Returns expanded string or NULL if a specified environment variable is
+ * not defined, or calls fatal if the string is invalid.
+ */
+char *
+percent_expand(const char *string, ...)
+{
+	char *ret;
+	int err;
+	va_list ap;
+
+	va_start(ap, string);
+	ret = vdollar_percent_expand(&err, 0, 1, string, ap);
+	va_end(ap);
+	if (err)
+		fatal("%s failed", __func__);
+	return ret;
+}
+
+/*
+ * Returns expanded string or NULL if a specified environment variable is
+ * not defined, or calls fatal if the string is invalid.
+ */
+char *
+percent_dollar_expand(const char *string, ...)
+{
+	char *ret;
+	int err;
+	va_list ap;
+
+	va_start(ap, string);
+	ret = vdollar_percent_expand(&err, 1, 1, string, ap);
+	va_end(ap);
+	if (err)
+		fatal("%s failed", __func__);
+	return ret;
+}
+
 int
 tun_open(int tun, int mode, char **ifname)
 {
diff --git a/usr.bin/ssh/misc.h b/usr.bin/ssh/misc.h
index 64d81a6f529..79e6ab50c1c 100644
--- a/usr.bin/ssh/misc.h
+++ b/usr.bin/ssh/misc.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: misc.h,v 1.85 2020/05/26 01:06:52 djm Exp $ */
+/* $OpenBSD: misc.h,v 1.86 2020/05/29 04:25:40 dtucker Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -68,7 +68,10 @@ int	 parse_uri(const char *, const char *, char **, char **, int *, char **);
 long	 convtime(const char *);
 const char *fmt_timeframe(time_t t);
 char	*tilde_expand_filename(const char *, uid_t);
+
+char	*dollar_expand(int *, const char *string);
 char	*percent_expand(const char *, ...) __attribute__((__sentinel__));
+char	*percent_dollar_expand(const char *, ...) __attribute__((__sentinel__));
 char	*tohex(const void *, size_t);
 void	 xextendf(char **s, const char *sep, const char *fmt, ...)
     __attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ (3)));
diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c
index 7fbe9ecedae..cd83522f48b 100644
--- a/usr.bin/ssh/readconf.c
+++ b/usr.bin/ssh/readconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.c,v 1.330 2020/05/27 21:25:18 djm Exp $ */
+/* $OpenBSD: readconf.c,v 1.331 2020/05/29 04:25:40 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1795,7 +1795,12 @@ parse_keytypes:
 			    filename, linenum);
   parse_agent_path:
 		/* Extra validation if the string represents an env var. */
-		if (arg[0] == '$' && !valid_env_name(arg + 1)) {
+		if ((arg2 = dollar_expand(&r, arg)) == NULL || r)
+			fatal("%.200s line %d: Invalid environment expansion "
+			    "%s.", filename, linenum, arg);
+		free(arg2);
+		/* check for legacy environment format */
+		if (arg[0] == '$' && arg[1] != '{' && !valid_env_name(arg + 1)) {
 			fatal("%.200s line %d: Invalid environment name %s.",
 			    filename, linenum, arg);
 		}
@@ -2334,12 +2339,19 @@ parse_forward(struct Forward *fwd, const char *fwdspec, int dynamicfwd, int remo
 {
 	struct fwdarg fwdargs[4];
 	char *p, *cp;
-	int i;
+	int i, err;
 
 	memset(fwd, 0, sizeof(*fwd));
 	memset(fwdargs, 0, sizeof(fwdargs));
 
-	cp = p = xstrdup(fwdspec);
+	/*
+	 * We expand environment variables before checking if we think they're
+	 * paths so that if ${VAR} expands to a fully qualified path it is
+	 * treated as a path.
+	 */
+	cp = p = dollar_expand(&err, fwdspec);
+	if (p == NULL || err)
+		return 0;
 
 	/* skip leading spaces */
 	while (isspace((u_char)*cp))
diff --git a/usr.bin/ssh/ssh.c b/usr.bin/ssh/ssh.c
index 8d701c56ba9..dd7648ac00f 100644
--- a/usr.bin/ssh/ssh.c
+++ b/usr.bin/ssh/ssh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.c,v 1.527 2020/04/10 00:52:07 dtucker Exp $ */
+/* $OpenBSD: ssh.c,v 1.528 2020/05/29 04:25:40 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -244,6 +244,31 @@ default_client_percent_expand(const char *str, const char *homedir,
 }
 
 /*
+ * Expands the set of percent_expand options used by the majority of keywords
+ * AND perform environment variable substitution.
+ * Caller must free returned string.
+ */
+static char *
+default_client_percent_dollar_expand(const char *str, const char *homedir,
+    const char *remhost, const char *remuser, const char *locuser)
+{
+	char *ret;
+
+	ret = percent_dollar_expand(str,
+	    /* values from statics above */
+	    DEFAULT_CLIENT_PERCENT_EXPAND_ARGS,
+	    /* values from arguments */
+	    "d", homedir,
+	    "h", remhost,
+	    "r", remuser,
+	    "u", locuser,
+	    (char *)NULL);
+	if (ret == NULL)
+		fatal("invalid environment variable expansion");
+	return ret;
+}
+
+/*
  * Attempt to resolve a host name / port to a set of addresses and
  * optionally return any CNAMEs encountered along the way.
  * Returns NULL on failure.
@@ -1357,14 +1382,14 @@ main(int ac, char **av)
 	if (options.control_path != NULL) {
 		cp = tilde_expand_filename(options.control_path, getuid());
 		free(options.control_path);
-		options.control_path = default_client_percent_expand(cp,
+		options.control_path = default_client_percent_dollar_expand(cp,
 		    pw->pw_dir, host, options.user, pw->pw_name);
 		free(cp);
 	}
 
 	if (options.identity_agent != NULL) {
 		p = tilde_expand_filename(options.identity_agent, getuid());
-		cp = default_client_percent_expand(p,
+		cp = default_client_percent_dollar_expand(p,
 		    pw->pw_dir, host, options.user, pw->pw_name);
 		free(p);
 		free(options.identity_agent);
@@ -1374,7 +1399,7 @@ main(int ac, char **av)
 	if (options.forward_agent_sock_path != NULL) {
 		p = tilde_expand_filename(options.forward_agent_sock_path,
 		    getuid());
-		cp = default_client_percent_expand(p,
+		cp = default_client_percent_dollar_expand(p,
 		    pw->pw_dir, host, options.user, pw->pw_name);
 		free(p);
 		free(options.forward_agent_sock_path);
@@ -1546,7 +1571,8 @@ main(int ac, char **av)
 			unsetenv(SSH_AUTHSOCKET_ENV_NAME);
 		} else {
 			cp = options.identity_agent;
-			if (cp[0] == '$') {
+			/* legacy (limited) format */
+			if (cp[0] == '$' && cp[1] != '{') {
 				if (!valid_env_name(cp + 1)) {
 					fatal("Invalid IdentityAgent "
 					    "environment variable name %s", cp);
@@ -2174,7 +2200,7 @@ load_public_identity_files(struct passwd *pw)
 			continue;
 		}
 		cp = tilde_expand_filename(options.identity_files[i], getuid());
-		filename = default_client_percent_expand(cp,
+		filename = default_client_percent_dollar_expand(cp,
 		    pw->pw_dir, host, options.user, pw->pw_name);
 		free(cp);
 		check_load(sshkey_load_public(filename, &public, NULL),
@@ -2224,7 +2250,7 @@ load_public_identity_files(struct passwd *pw)
 	for (i = 0; i < options.num_certificate_files; i++) {
 		cp = tilde_expand_filename(options.certificate_files[i],
 		    getuid());
-		filename = default_client_percent_expand(cp,
+		filename = default_client_percent_dollar_expand(cp,
 		    pw->pw_dir, host, options.user, pw->pw_name);
 		free(cp);
 
diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5
index f5f553b6e77..5d7060211c8 100644
--- a/usr.bin/ssh/ssh_config.5
+++ b/usr.bin/ssh/ssh_config.5
@@ -33,8 +33,8 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: ssh_config.5,v 1.325 2020/04/11 20:20:09 jmc Exp $
-.Dd $Mdocdate: April 11 2020 $
+.\" $OpenBSD: ssh_config.5,v 1.326 2020/05/29 04:25:40 dtucker Exp $
+.Dd $Mdocdate: May 29 2020 $
 .Dt SSH_CONFIG 5
 .Os
 .Sh NAME
@@ -389,9 +389,11 @@ or
 .Pp
 Arguments to
 .Cm CertificateFile
-may use the tilde syntax to refer to a user's home directory
-or the tokens described in the
+may use the tilde syntax to refer to a user's home directory,
+the tokens described in the
 .Sx TOKENS
+section and environment variables as described in the
+.Sx ENVIRONMENT VARIABLES
 section.
 .Pp
 It is possible to have multiple certificate files specified in
@@ -551,9 +553,11 @@ section above or the string
 to disable connection sharing.
 Arguments to
 .Cm ControlPath
-may use the tilde syntax to refer to a user's home directory
-or the tokens described in the
+may use the tilde syntax to refer to a user's home directory,
+the tokens described in the
 .Sx TOKENS
+section and environment variables as described in the
+.Sx ENVIRONMENT VARIABLES
 section.
 It is recommended that any
 .Cm ControlPath
@@ -934,9 +938,11 @@ the location of the socket.
 .Pp
 Arguments to
 .Cm IdentityAgent
-may use the tilde syntax to refer to a user's home directory
-or the tokens described in the
+may use the tilde syntax to refer to a user's home directory,
+the tokens described in the
 .Sx TOKENS
+section and environment variables as described in the
+.Sx ENVIRONMENT VARIABLES
 section.
 .It Cm IdentityFile
 Specifies a file from which the user's DSA, ECDSA, authenticator-hosted ECDSA,
@@ -1153,8 +1159,10 @@ indicates that the listening port be bound for local use only, while an
 empty address or
 .Sq *
 indicates that the port should be available from all interfaces.
-Unix domain socket paths accept the tokens described in the
+Unix domain socket paths may use the tokens described in the
 .Sx TOKENS
+section and environment variables as described in the
+.Sx ENVIRONMENT VARIABLES
 section.
 .It Cm LogLevel
 Gives the verbosity level that is used when logging messages from
@@ -1424,8 +1432,10 @@ Multiple forwardings may be specified, and additional
 forwardings can be given on the command line.
 Privileged ports can be forwarded only when
 logging in as root on the remote machine.
-Unix domain socket paths accept the tokens described in the
+Unix domain socket paths may use the tokens described in the
 .Sx TOKENS
+section and environment variables as described in the
+.Sx ENVIRONMENT VARIABLES
 section.
 .Pp
 If the
@@ -1876,6 +1886,29 @@ accepts all tokens.
 .Pp
 .Cm ProxyCommand
 accepts the tokens %%, %h, %n, %p, and %r.
+.Sh ENVIRONMENT VARIABLES
+Arguments to some keywords can be expanded at runtime from environment
+variables on the client by enclosing them in
+.Ic ${} ,
+for example
+.Ic ${HOME}/.ssh
+would refer to the user's .ssh directory.
+If a specified environment variable does not exist then an error will be
+returned and the setting for that keyword will be ignored.
+.Pp
+The keywords
+.El
+.Cm CertificateFile ,
+.Cm ControlPath ,
+.Cm IdentityAgent
+and
+.Cm IdentityFile
+support environment variables.
+The keywords
+.Cm LocalForward
+and
+.Cm RemoteForward
+support environment variables only for Unix domain socket paths.
 .Sh FILES
 .Bl -tag -width Ds
 .It Pa ~/.ssh/config