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.