[openssh-commits] [openssh] 01/02: upstream: Allow some keywords to expand shell-style ${ENV}

git+noreply at mindrot.org git+noreply at mindrot.org
Fri May 29 15:46:54 AEST 2020


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

djm pushed a commit to branch master
in repository openssh.

commit 4a1b46e6d032608b7ec00ae51c4e25b82f460b05
Author: dtucker at openbsd.org <dtucker at openbsd.org>
Date:   Fri May 29 04:25:40 2020 +0000

    upstream: 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@
    
    OpenBSD-Commit-ID: a4a2e801fc2d4df2fe0e58f50d9c81b03822dffa
---
 misc.c       | 163 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
 misc.h       |   5 +-
 readconf.c   |  20 ++++++--
 ssh.c        |  40 ++++++++++++---
 ssh_config.5 |  53 +++++++++++++++----
 5 files changed, 229 insertions(+), 52 deletions(-)

diff --git a/misc.c b/misc.c
index 5a34107f..3ec02d79 100644
--- a/misc.c
+++ b/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.
@@ -1084,45 +1084,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__);
+	if (parseerror == NULL)
+		fatal("%s: null parseerror arg", __func__);
+	*parseerror = 1;
 
-	/* 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__);
+	/* 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",
@@ -1134,8 +1179,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,
@@ -1146,16 +1193,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/misc.h b/misc.h
index 8e4234b6..7e9f5ac1 100644
--- a/misc.h
+++ b/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 at 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/readconf.c b/readconf.c
index 63ed7fd5..c0595a52 100644
--- a/readconf.c
+++ b/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 at cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo at cs.hut.fi>, Espoo, Finland
@@ -1809,7 +1809,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);
 		}
@@ -2355,12 +2360,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/ssh.c b/ssh.c
index 98b6ce78..9d61e2e6 100644
--- a/ssh.c
+++ b/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 at cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo at cs.hut.fi>, Espoo, Finland
@@ -259,6 +259,31 @@ default_client_percent_expand(const char *str, const char *homedir,
 	    (char *)NULL);
 }
 
+/*
+ * 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.
@@ -1378,14 +1403,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);
@@ -1395,7 +1420,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);
@@ -1573,7 +1598,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);
@@ -2201,7 +2227,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),
@@ -2251,7 +2277,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/ssh_config.5 b/ssh_config.5
index dc010ccb..001544dd 100644
--- a/ssh_config.5
+++ b/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,
@@ -1152,8 +1158,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
@@ -1423,8 +1431,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
@@ -1875,6 +1885,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

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


More information about the openssh-commits mailing list