Attachment 3375 Details for Bug 3140 – allow environment variables in path config keywords

[patch] allow environment variables in path config keywords

ssh-keyword-environment-vars.patch (text/plain), 10.79 KB, created by Darren Tucker on 2020-04-10 18:56:06 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Darren Tucker

Created: 2020-04-10 18:56:06 AEST

Size: 10.79 KB

patch

obsolete

>Index: usr.bin/ssh/misc.c
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/misc.c,v
>retrieving revision 1.146
>diff -u -p -r1.146 misc.c
>--- usr.bin/ssh/misc.c	28 Jan 2020 01:49:36 -0000	1.146
>+++ usr.bin/ssh/misc.c	10 Apr 2020 08:25:51 -0000
>@@ -1089,6 +1089,68 @@ percent_expand(const char *string, ...)
> #undef EXPAND_MAX_KEYS
> }
> 
>+/*
>+ * Expand a string with a set of ${ENVIRONMENT_VARIABLES}.  Returns a
>+ * dynamically allocated expanded string which caller must free or NULL in
>+ * the case of an error.
>+ */
>+char *
>+dollar_env_expand(const char *string)
>+{
>+	u_int i;
>+	struct sshbuf *buf;
>+	int r;
>+	char *ret = NULL, *var, *varend, *val;
>+	size_t len;
>+
>+	if ((buf = sshbuf_new()) == NULL)
>+		fatal("%s: sshbuf_new failed", __func__);
>+
>+	for (i = 0; *string != '\0'; string++) {
>+		if (*string != '$' || (string[0] == '$' && string[1] != '{')) {
>+			if ((r = sshbuf_put_u8(buf, *string)) != 0) {
>+				fatal("%s: sshbuf_put_u8: %s",
>+				    __func__, ssh_err(r));
>+			}
>+			continue;
>+		}
>+		string += 2;  /* skip over '${' */
>+		if (*string == '\0') {
>+			error("%s: invalid environment variable", __func__);
>+			goto out;
>+		}
>+		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);
>+			free(var);
>+			goto out;
>+		} 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;
>+	}
>+	if ((ret = sshbuf_dup_string(buf)) == NULL)
>+		fatal("%s: sshbuf_dup_string failed", __func__);
>+ out:
>+	sshbuf_free(buf);
>+	return ret;
>+}
>+
> int
> tun_open(int tun, int mode, char **ifname)
> {
>Index: usr.bin/ssh/misc.h
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/misc.h,v
>retrieving revision 1.84
>diff -u -p -r1.84 misc.h
>--- usr.bin/ssh/misc.h	24 Jan 2020 23:54:40 -0000	1.84
>+++ usr.bin/ssh/misc.h	10 Apr 2020 08:25:51 -0000
>@@ -68,6 +68,7 @@ int	 parse_uri(const char *, const char 
> long	 convtime(const char *);
> char	*tilde_expand_filename(const char *, uid_t);
> char	*percent_expand(const char *, ...) __attribute__((__sentinel__));
>+char	*dollar_env_expand(const char *);
> char	*tohex(const void *, size_t);
> void	 xextendf(char **s, const char *sep, const char *fmt, ...)
>     __attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ (3)));
>Index: usr.bin/ssh/readconf.c
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/readconf.c,v
>retrieving revision 1.328
>diff -u -p -r1.328 readconf.c
>--- usr.bin/ssh/readconf.c	3 Apr 2020 03:12:11 -0000	1.328
>+++ usr.bin/ssh/readconf.c	10 Apr 2020 08:25:51 -0000
>@@ -1790,7 +1790,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_env_expand(arg)) == NULL)
>+			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,7 +2339,12 @@ parse_forward(struct Forward *fwd, const
> 	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_env_expand(fwdspec);
> 
> 	/* skip leading spaces */
> 	while (isspace((u_char)*cp))
>Index: usr.bin/ssh/ssh.c
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/ssh.c,v
>retrieving revision 1.527
>diff -u -p -r1.527 ssh.c
>--- usr.bin/ssh/ssh.c	10 Apr 2020 00:52:07 -0000	1.527
>+++ usr.bin/ssh/ssh.c	10 Apr 2020 08:25:51 -0000
>@@ -244,6 +244,24 @@ default_client_percent_expand(const char
> }
> 
> /*
>+ * Expand percent then environment variables, percent first as they're less
>+ * likely to influence environment expansion.
>+ */
>+static char *
>+default_client_percent_dollar_expand(const char *str, const char *homedir,
>+    const char *remhost, const char *remuser, const char *locuser)
>+{
>+	char *str1, *str2;
>+
>+	str1 = default_client_percent_expand(str, homedir, remhost, remuser,
>+	    locuser);
>+	if ((str2 = dollar_env_expand(str1)) == NULL)
>+		fatal("invalid environment variable expansion");
>+	free(str1);
>+	return(str2);
>+}
>+
>+/*
>  * 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 +1375,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 +1392,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 +1564,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 +2193,7 @@ load_public_identity_files(struct passwd
> 			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 +2243,7 @@ load_public_identity_files(struct passwd
> 	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);
> 
>Index: usr.bin/ssh/ssh_config.5
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/ssh_config.5,v
>retrieving revision 1.324
>diff -u -p -r1.324 ssh_config.5
>--- usr.bin/ssh/ssh_config.5	10 Apr 2020 00:52:07 -0000	1.324
>+++ usr.bin/ssh/ssh_config.5	10 Apr 2020 08:25:51 -0000
>@@ -551,9 +551,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,
>+tokens described in the
> .Sx TOKENS
>+or environment variables as described in the
>+.Sx ENVIRONMENT VARIABLES
> section.
> It is recommended that any
> .Cm ControlPath
>Index: regress/usr.bin/ssh/percent.sh
>===================================================================
>RCS file: /cvs/src/regress/usr.bin/ssh/percent.sh,v
>retrieving revision 1.6
>diff -u -p -r1.6 percent.sh
>--- regress/usr.bin/ssh/percent.sh	10 Apr 2020 00:54:03 -0000	1.6
>+++ regress/usr.bin/ssh/percent.sh	10 Apr 2020 08:25:51 -0000
>@@ -51,7 +51,7 @@ trial()
> 
> for i in matchexec localcommand remotecommand controlpath identityagent \
>     forwardagent localforward remoteforward; do
>-	verbose $tid $i
>+	verbose $tid $i percent
> 	if [ "$i" = "localcommand" ]; then
> 		REMUSER=$USER
> 		trial $i '%T' NONE
>@@ -76,8 +76,19 @@ for i in matchexec localcommand remoteco
> 	    "%/$HASH/$USERID/127.0.0.1/$HOME/$HOST/$HOSTNAME/somehost/$PORT/$REMUSER/$USER"
> done
> 
>+# Subset of above since don't expand shell-style variables on anything that
>+# runs a command
>+for i in controlpath identityagent forwardagent localforward remoteforward; do
>+	verbose $tid $i dollar
>+	FOO=bar
>+	export FOO
>+	trial $i '${FOO}' $FOO
>+done
>+
>+
> # A subset of options support tilde expansion
> for i in controlpath identityagent forwardagent; do
>+	verbose $tid $i tilde
> 	trial $i '~' $HOME/
> 	trial $i '~/.ssh' $HOME/.ssh
> done
>Index: regress/usr.bin/ssh/unittests/misc/tests.c
>===================================================================
>RCS file: /cvs/src/regress/usr.bin/ssh/unittests/misc/tests.c,v
>retrieving revision 1.1
>diff -u -p -r1.1 tests.c
>--- regress/usr.bin/ssh/unittests/misc/tests.c	28 Apr 2019 22:53:26 -0000	1.1
>+++ regress/usr.bin/ssh/unittests/misc/tests.c	10 Apr 2020 08:25:51 -0000
>@@ -76,4 +76,40 @@ tests(void)
> 	ASSERT_STRING_EQ(path, "some/path");
> 	free(user); free(host); free(path);
> 	TEST_DONE();
>+
>+#define ASSERT_DOLLAR_EQ(x, y) do { \
>+	char *str = dollar_env_expand((x)); \
>+	ASSERT_STRING_EQ(str, (y)); \
>+	free(str); \
>+} while(0)
>+
>+	TEST_START("dollar_environment_expand");
>+	if (setenv("FOO", "bar", 1) != 0)
>+		abort();
>+	if (setenv("BAR", "baz", 1) != 0)
>+		abort();
>+	if (unsetenv("BAZ") != 0)
>+		abort();
>+	ASSERT_DOLLAR_EQ("${FOO}", "bar");
>+	ASSERT_DOLLAR_EQ(" ${FOO}", " bar");
>+	ASSERT_DOLLAR_EQ("${FOO} ", "bar ");
>+	ASSERT_DOLLAR_EQ(" ${FOO} ", " bar ");
>+	ASSERT_DOLLAR_EQ("${FOO}${BAR}", "barbaz");
>+	ASSERT_DOLLAR_EQ(" ${FOO} ${BAR}", " bar baz");
>+	ASSERT_DOLLAR_EQ("${FOO}${BAR} ", "barbaz ");
>+	ASSERT_DOLLAR_EQ(" ${FOO} ${BAR} ", " bar baz ");
>+	ASSERT_DOLLAR_EQ("$", "$");
>+	ASSERT_DOLLAR_EQ(" $", " $");
>+	ASSERT_DOLLAR_EQ("$ ", "$ ");
>+	/* error checking, non existent variable */
>+	ASSERT_PTR_EQ(dollar_env_expand("a${BAZ}"), NULL);
>+	ASSERT_PTR_EQ(dollar_env_expand("${BAZ}b"), NULL);
>+	ASSERT_PTR_EQ(dollar_env_expand("a${BAZ}b"), NULL);
>+	/* invalid format */
>+	ASSERT_PTR_EQ(dollar_env_expand("${"), NULL);
>+	ASSERT_PTR_EQ(dollar_env_expand("${F"), NULL);
>+	ASSERT_PTR_EQ(dollar_env_expand("${FO"), NULL);
>+	/* empty variable name */
>+	ASSERT_PTR_EQ(dollar_env_expand("${}"), NULL);
>+	TEST_DONE();
> }

Actions: View | Diff

Attachments on bug 3140: 3375