The short version: There's a "#define USE_PIPES" in the middle of session.c; it would be better if it were in (e.g.) defines.h or some other .h file. (If in fact it needs to be defined at all; I'm not convinced that it does.) Here's the (much) longer version: I recently installed the latest OpenSSH on some of our servers (RHEL5, which provides the 4.3 release) and soon afterward we received a complaint from one of our users, reporting that his .bashrc file was no longer being sourced when he ran remote commands via a non-interactive single-command connection. He was right. The problem stemmed from the fact that bash contains the following in run_startup_files(), which gets called to determine which dotfiles (if any) to source ... if (interactive_shell == 0 && no_rc == 0 && login_shell == 0 && act_like_sh == 0 && command_execution_string) { #ifdef SSH_SOURCE_BASHRC run_by_ssh = (find_variable ("SSH_CLIENT") != (SHELL_VAR *)0) || (find_variable ("SSH2_CLIENT") != (SHELL_VAR *)0); #else run_by_ssh = 0; #endif /* If we were run by sshd or we think we were run by rshd, execute ~/.bashrc if we are a top-level shell. */ if ((run_by_ssh || isnetconn (fileno (stdin))) && shell_level < 2) ... and RedHat's bash is built with SSH_SOURCE_BASHRC not defined, so everything depends on isnetconn(); in fact, that seems to be bash's default, since one finds this in a (very old) entry in their CHANGES file: This document details the changes between this version, bash-2.05a-release, and the previous version, bash-2.05a-rc1. [ ... ] z. Bash no longer attempts to discover if it's being run by sshd in order to run the startup files. If the SSH_SOURCE_BASHRC is uncommented in config-top.h it will attempt to do so as previously, but that's commented out in the distributed version. But it turns out that bash's isnetconn() function was returning 0, which in turn was caused by getpeername() failing (errno == ENOTSOCK) when it was being used to get information about stdin. And sure enough, the child process spawned by Redhat's sshd showed the following for standard input (reported by "lsof") ... 0u unix 0xffff81033fec2b80 10237328 socket ... whereas the same child process spawned by the sshd which I'd just built had this ... 0r FIFO 0,6 3093574 pipe So what was responsible for this difference? It turns out that there's some code in session.c (specifically, in the do_exec_no_pty() function) conditionalized with ... #ifdef USE_PIPES [ ... ] #else [ ... ] #endif ... so this might lead one to believe that there would be something in one of the .h files that would govern which of those chunks of code would get used, particularly in light of the fact that defines.h contains ... /* * Define this to use pipes instead of socketpairs for communicating with the * client program. Socketpairs do not seem to work on all systems. * * configure.ac sets this for a few OS's which are known to have problems * but you may need to set it yourself */ /* #define USE_PIPES 1 */ ... and the configure script creates a config.h with ... /* Use PIPES instead of a socketpair() */ /* #undef USE_PIPES */ ... both of which would lead one to infer that USE_PIPES is _not_ defined by default. But back in session.c, more then four hundred lines from the beginning of the file, we find this ... #define USE_PIPES ... just before the start of the do_exec_no_pty() function! This wasn't always there; it appeared for the first time in the 5.1p1 release (it's not in 5.0p1). So first, I'm curious as to why it was felt necessary to make USE_PIPES the default; and secondly, even if there is a good reason for it to be the default, shouldn't the "#define" be in a .h file, so that it will be obvious what the setting is for people who might want to change it? Burying it down in the middle of session.c just seems like an unwise decision ... --------------- Mark Bartelt Center for Advanced Computing Research California Institute of Technology Pasadena, California 91125 626 395 2522 626 584 5917 fax 626 628 3994 e-fax mark at cacr.caltech.edu http://www.cacr.caltech.edu/~mark
I don't mean to flame you, but this bash idiosyncracy has been talked to death time and time again on this list. Please consult the archive. On Thu, Jun 30, 2011 at 16:34:55 -0500, Mark Bartelt wrote:> The short version: There's a "#define USE_PIPES" > in the middle of session.c; it would be better if > it were in (e.g.) defines.h or some other .h file. > (If in fact it needs to be defined at all; I'm not > convinced that it does.) Here's the (much) longer > version: > > I recently installed the latest OpenSSH on some of > our servers (RHEL5, which provides the 4.3 release) > and soon afterward we received a complaint from one > of our users, reporting that his .bashrc file was no > longer being sourced when he ran remote commands via > a non-interactive single-command connection. He was > right. The problem stemmed from the fact that bash > contains the following in run_startup_files(), which > gets called to determine which dotfiles (if any) to > source ... > > if (interactive_shell == 0 && no_rc == 0 && login_shell == 0 && > act_like_sh == 0 && command_execution_string) > { > #ifdef SSH_SOURCE_BASHRC > run_by_ssh = (find_variable ("SSH_CLIENT") != (SHELL_VAR *)0) || > (find_variable ("SSH2_CLIENT") != (SHELL_VAR *)0); > #else > run_by_ssh = 0; > #endif > /* If we were run by sshd or we think we were run by rshd, execute > ~/.bashrc if we are a top-level shell. */ > if ((run_by_ssh || isnetconn (fileno (stdin))) && shell_level < 2) > > ... and RedHat's bash is built with SSH_SOURCE_BASHRC > not defined, so everything depends on isnetconn(); in > fact, that seems to be bash's default, since one finds > this in a (very old) entry in their CHANGES file: > > This document details the changes between this version, bash-2.05a-release, > and the previous version, bash-2.05a-rc1. > [ ... ] > z. Bash no longer attempts to discover if it's being run by sshd in order to > run the startup files. If the SSH_SOURCE_BASHRC is uncommented in > config-top.h it will attempt to do so as previously, but that's commented > out in the distributed version. > > But it turns out that bash's isnetconn() function was > returning 0, which in turn was caused by getpeername() > failing (errno == ENOTSOCK) when it was being used to > get information about stdin. > > And sure enough, the child process spawned by Redhat's > sshd showed the following for standard input (reported > by "lsof") ... > > 0u unix 0xffff81033fec2b80 10237328 socket > > ... whereas the same child process spawned by the sshd > which I'd just built had this ... > > 0r FIFO 0,6 3093574 pipe > > So what was responsible for this difference? It turns > out that there's some code in session.c (specifically, > in the do_exec_no_pty() function) conditionalized with ... > > #ifdef USE_PIPES > [ ... ] > #else > [ ... ] > #endif > > ... so this might lead one to believe that there would be > something in one of the .h files that would govern which > of those chunks of code would get used, particularly in > light of the fact that defines.h contains ... > > /* > * Define this to use pipes instead of socketpairs for communicating with the > * client program. Socketpairs do not seem to work on all systems. > * > * configure.ac sets this for a few OS's which are known to have problems > * but you may need to set it yourself > */ > /* #define USE_PIPES 1 */ > > ... and the configure script creates a config.h with ... > > /* Use PIPES instead of a socketpair() */ > /* #undef USE_PIPES */ > > ... both of which would lead one to infer that USE_PIPES > is _not_ defined by default. But back in session.c, more > then four hundred lines from the beginning of the file, we > find this ... > > #define USE_PIPES > > ... just before the start of the do_exec_no_pty() function! > > This wasn't always there; it appeared for the first time in > the 5.1p1 release (it's not in 5.0p1). > > So first, I'm curious as to why it was felt necessary to make > USE_PIPES the default; and secondly, even if there is a good > reason for it to be the default, shouldn't the "#define" be > in a .h file, so that it will be obvious what the setting is > for people who might want to change it? > > Burying it down in the middle of session.c just seems like an > unwise decision ... > > --------------- > > Mark Bartelt > Center for Advanced Computing Research > California Institute of Technology > Pasadena, California 91125 > > 626 395 2522 > 626 584 5917 fax > 626 628 3994 e-fax > > mark at cacr.caltech.edu > > http://www.cacr.caltech.edu/~mark > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev-- Iain Morgan
Actually, it doesn't surprise me at all that this has already been discussed before. And I did try searching the archives before sending my message; more on that below. But first, as for the topic itself ... The "bash" manpage is annoyingly vague about what the behaviour should be. On the one hand, the "FILES" section talks about the ".bashrc" file as the "individual per-interactive-shell startup file", which sort of implies that it's not used for connections which _don't_ cause an interactive shell to be run. But on the other hand, elsewhere it says ... Bash attempts to determine when it is being run by the remote shell daemon, usually rshd. If bash determines it is being run by rshd, it reads and executes commands from ~/.bashrc, if that file exists and is readable. That wording ("usually rshd") is obviously quite dated, but one could certainly argue that sshd _is_ the "remote shell daemon" these days. So it just seems better (to me, at least) if sshd would ensure that bash's behaviour were consistent with what's in the manpage. One could fault the distributors of "bash", for not making the "#define SSH_SOURCE_BASHRC" a default. But even so, I really don't understand why having sshd's default be _not_ to define USE_PIPES wouldn't be sensible, letting people define it when their environment requires it. And regardless of whether the consensus is that it's better to have it defined or not to have it defined, it just seems unwise to have that "#define USE_PIPES" buried down in the middle of a .c file, rather than at the very top, or (better) in a .h file; wouldn't that be more conformant with good C programming style? Finally, regarding the archives ... I actually did try doing a search. But when I asked it to search for "USE_PIPES", after it displayed the results my "USE_PIPES" in the search box had been replaced by "pipes", and the browser window title bar displayed "'pipes' in openssh-unix-dev", so for some reason it looked like it was chopping off part of the search term I gave it (even when I enclosed it in quotes), so it wasn't clear what fraction of the search results it was displaying would be relevant. If I entered "USE_PIPES" and ".bashrc" in the search box, it gave me four messages from April 2009, but those seemed to be dealing with whether socketpairs and pipes could be mixed in channels.c, so didn't necessarily seem relevant. (And the thread topic was "ssh localhost yes | true (follow up)", which also didn't seem like it was coconnected to the .bashrc issue.) Maybe better search criteria would have turned up the relevant old threads. But just offhand, do you happen to know why their search engine seems so insistent on transforming "USE_PIPES" to just plain "pipes"? --------------- Mark Bartelt Center for Advanced Computing Research California Institute of Technology Pasadena, California 91125 626 395 2522 626 584 5917 fax 626 628 3994 e-fax mark at cacr.caltech.edu http://www.cacr.caltech.edu/~mark
>> bash was doing the wrong thing varying its behaviour based on the >> type of fd it was talking to, so please turn your attention there.Fair enough; I'm basically agnostic on the issue of whether bash {is|isn't} doing the wrong thing. But regardless, I still feel that the change in sshd's behaviour shouldn't have been sprung on people without a big "hey, heads-up, bash users!" in one of the README files. It first appeared in the 5.1p1 release (it wasn't there in 5.0p1), and although the change was documented in that ChangeLog file, it's more than 800 lines down, with no suggestion about what the potential side-effects of the change might be. But if I've overlooked something in a location that people are more likely to pay attention to, feel free to point me in its direction. Just seems that when a change gets made which affects backward compatibility and interoperability, it would be best if folks were alerted about the potential impact of moving from an older release to the newer one. But my main gripe is the fact that the "#define USE_PIPES" was buried more than 400 lines down inside a .c source file, while the relevant header file (or at least the file which one might naively assume is relevant, defines.h) still (as of the 5.8p2 release) contains ... /* * Define this to use pipes instead of socketpairs for communicating with the * client program. Socketpairs do not seem to work on all systems. * * configure.ac sets this for a few OS's which are known to have problems * but you may need to set it yourself */ /* #define USE_PIPES 1 */ ... which suggests that, if that statement were left as it is (i.e. commented out), USE_PIPES would be undefined when sshd was built.
>> We didn't mention it because we didn't notice it, and because >> OpenSSH had already used pipes on several platform for years >> without anyone complaining (to me at least).OK; I'll accept "we didn't notice it" as an adequate defence. ;-) I like Iain's suggestion about adding it to the FAQ (or perhaps to a README.bash file or some such). The first we learned about this was shortly after we'd deployed 5.8p2 (after having used a pre 5.1p1 release for a long, _long_ time) and one of our users (a principal investigator, no less!) sent us a message which had "URGENT:" in the subject line, reporting that something he'd done for a long time had suddenly stopped working. And maybe the stuff in defines.h (which might lead one to believe that USE_PIPES isn't defined unless it's uncommented there) could be removed or changed. Anyway, case closed, as far as I'm concerned. Thank to everybody for your replies.