Bug 2471 - "match exec" prepends "exec" to its command
Summary: "match exec" prepends "exec" to its command
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 7.1p1
Hardware: All All
: P5 normal
Assignee: Darren Tucker
URL:
Keywords:
Depends on:
Blocks: V_7_2
  Show dependency treegraph
 
Reported: 2015-09-23 23:04 AEST by Richard E. Silverman
Modified: 2016-08-02 10:42 AEST (History)
2 users (show)

See Also:


Attachments
remove "exec" from match exec handling (1.30 KB, patch)
2015-10-23 13:22 AEDT, Darren Tucker
djm: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard E. Silverman 2015-09-23 23:04:33 AEST
The "match exec" feature in ssh_config does this:

  static int
  execute_in_shell(const char *cmd)
  {
  ...
        /*                                                                                                                          
         * Use "exec" to avoid "sh -c" processes on some platforms                                                                  
         * (e.g. Solaris)                                                                                                           
         */
        xasprintf(&command_string, "exec %s", cmd);

This is a problem, because it assumes the user's shell supports a particular command syntax, beyond just requiring that it support the "$SHELL -c <command>" convention. For example, if your shell is fish and you have e.g.:

  match exec "getent passwd %u | grep -q NOT_THERE"

This doesn't work, because:

  $ fish -c "getent passwd res | grep -q NOT_THERE" && echo match
  $ fish -c "exec getent passwd res | grep -q NOT_THERE" && echo match
  res:x:11500:11500::/home/res:/bin/bash
  match

... and of course, the shell might not have an "exec" command at all. I think you should just remove this optimization. At least, it should be documented if you leave it in; ssh_config(1) only says:

  The exec keyword executes the specified command under the user’s shell.

Thanks,

Richard E. Silverman
Comment 1 Darren Tucker 2015-10-22 14:54:47 AEDT
Well it assumes the user's shell supports a particular command syntax that's specified by SuSv2:
http://pubs.opengroup.org/onlinepubs/007908799/xcu/chap2.html#tag_001_014_006

which fish does seem to actually have:
http://fishshell.com/docs/current/commands.html#exec

Is the fish behaviour version specific?
Comment 2 Richard E. Silverman 2015-10-22 15:11:00 AEDT
> Well it assumes the user's shell supports a particular command syntax that's specified by SuSv2:
> http://pubs.opengroup.org/onlinepubs/007908799/xcu/chap2.html#tag_001_014_006
> 
> which fish does seem to actually have:
> http://fishshell.com/docs/current/commands.html#exec

I did show in my initial report with an example, actually, that fish does *have* an "exec" command --
it just doesn't have quite the required behavior. Though I think it's questionable to require the
shell to support any specific command set, SuSv2 notwithstanding.

> Is the fish behaviour version specific?

I'm afraid I don't know; it happens with the RHEL6 fish 2.1.2, in any event.

-- 
  Richard
Comment 3 Darren Tucker 2015-10-22 15:36:33 AEDT
(In reply to Richard E. Silverman from comment #2)
> I think it's questionable to require the
> shell to support any specific command set, SuSv2 notwithstanding.

Hm.  that's a fair point, and there's no particular requirement that a user's shell conforms to SuSv2 either.
Comment 4 Darren Tucker 2015-10-23 13:22:39 AEDT
Created attachment 2734 [details]
remove "exec" from match exec handling
Comment 5 Darren Tucker 2015-10-23 13:26:48 AEDT
Damien points out that the ProxyCommand codes does the same thing (although that's likely to be less problematic because | is unidirectional and the ProxyCommand needs to be bidirectional).
Comment 6 Darren Tucker 2015-10-23 13:46:57 AEDT
The commit that added it to ProxyCommand was
https://anongit.mindrot.org/openssh.git/commit/sshconnect.c?id=8c4e18a6ec22a09b9082ff74b668685c30a028e7 , referencing bug #223.

Looking at the bug we can probably get rid of the exec now too: ssh -W is neater than netcat, and in the case where it still matters exec can be prepended to the ProxyCommand in the config.
Comment 7 Darren Tucker 2015-10-26 10:14:39 AEDT
Patch has been applied and will be in 7.2.  Thanks.
Comment 8 Damien Miller 2016-08-02 10:42:59 AEST
Close all resolved bugs after 7.3p1 release