Feedback regarding the ssh(1) Match directive

[Damien Miller]

On Thu, 17 Oct 2013, Iain Morgan wrote:

> Hi,
> 
> I noticed the recent commit adding Match support to ssh(1). I look
> forward to giving it a try, but I have some initial feedback based on
> ssh_config.5 and an examiniation of match_cfg_line().
> 
> First, the "command" keyword could be a little deceptive. Although the
> man page makes the use of this keyword quite clear, my initial
> assumption was that the intent was to match against the remote command
> that is being requested. That would seem to be a more natural
> interpretation of this keyword. Instead it is an arbitrary local test.
> Perhaps "localtest" would be a better choice for the keyword.

Maybe rename it to "exec"?

Index: readconf.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/readconf.c,v
retrieving revision 1.209
diff -u -p -r1.209 readconf.c
--- readconf.c	16 Oct 2013 22:49:38 -0000	1.209
+++ readconf.c	20 Oct 2013 06:12:56 -0000
@@ -498,7 +498,7 @@ match_cfg_line(Options *options, char **
 				debug("%.200s line %d: matched "
 				    "'LocalUser %.100s' ",
 				    filename, linenum, pw->pw_name);
-		} else if (strcasecmp(attrib, "command") == 0) {
+		} else if (strcasecmp(attrib, "exec") == 0) {
 			if (gethostname(thishost, sizeof(thishost)) == -1)
 				fatal("gethostname: %s", strerror(errno));
 			strlcpy(shorthost, thishost, sizeof(shorthost));
@@ -517,11 +517,11 @@ match_cfg_line(Options *options, char **
 			    (char *)NULL);
 			r = execute_in_shell(cmd);
 			if (r == -1) {
-				fatal("%.200s line %d: match command '%.100s' "
+				fatal("%.200s line %d: match exec '%.100s' "
 				    "error", filename, linenum, cmd);
 			} else if (r == 0) {
 				debug("%.200s line %d: matched "
-				    "'Command \"%.100s\"' ",
+				    "'exec \"%.100s\"' ",
 				    filename, linenum, cmd);
 			} else
 				result = 0;
Index: ssh_config.5
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssh_config.5,v
retrieving revision 1.175
diff -u -p -r1.175 ssh_config.5
--- ssh_config.5	20 Oct 2013 04:39:28 -0000	1.175
+++ ssh_config.5	20 Oct 2013 06:12:56 -0000
@@ -136,7 +136,7 @@ keyword) to be used only when the condit
 keyword are satisfied.
 Match conditions are specified using one or more keyword/criteria pairs.
 The available keywords are:
-.Cm command ,
+.Cm exec ,
 .Cm host ,
 .Cm originalhost ,
 .Cm user ,
@@ -144,8 +144,8 @@ and
 .Cm localuser .
 .Pp
 The criteria for the
-.Cm command
-keyword is a path to a command that is executed.
+.Cm exec
+keyword is command that is executed under the user's shell..
 If the command returns a zero exit status then the condition is considered true.
 Commands containing whitespace characters must be quoted.
 The following character sequences in the command will be expanded prior to
@@ -158,7 +158,7 @@ will be substituted by the local host na
 will be substituted by the target host name,
 .Ql %n
 will be substituted by the original target host name
-specified on the command line,
+specified on the command-line,
 .Ql %p
 the destination port,
 .Ql %r

> There are cases where it would be useful to match on the requested command,
> to select a different public key or possibly use a different port. For
> example, I use one key for CVS operations against a local server, where
> the key is restricted to "cvs server," and a different key for shell
> logins to that same server. Currently, I manage this by using different
> hostnames, but I was hoping that the Match directive would provide a
> cleaner approach.

I guess that's reasonable. Our wildcard matching code is a little
inexpressive to do anything sophisticated though.

> Also, the man page does not document any of the percent macros supported
> by teh command keyword.

I've added these.

-d