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