On Wed, Nov 25, 2015 at 6:07 AM, Ruediger Meier <sweet_f_a at gmx.de> wrote:> Hi, > > On Tuesday 24 November 2015, Radek Podgorny wrote: >> hello everyone! >> >> i'd like to sincerely ask you to include a fix for ssh-copy-id bug >> i'll be linking below. it's a trivial fix which resolves >> https://bugzilla.mindrot.org/show_bug.cgi?id=2206 and eases life of >> many. it's been field-tested by redhat devs and users so i see no >> problem in incorporating it. >> >> http://pkgs.fedoraproject.org/cgit/openssh.git/tree/openssh-6.8p1-fix >>-ss h-copy-id-on-non-sh-shell.patch> Personally I think it's hard enough to write POSIX compatible shell > scripts and I wouldn't start to add such hacks for fish and tcsh. > Next week somebody may complain that his "shell" does not > support "exec ...".Making things work for more people, when it doesn't introduce a security risk or break other tools, seems very reasonable. And there are people out there who who do use both fish and tcsh. What seems to be missing in the patch is a comment line, above the stanza, explaining why the code uses "exec". It's great to be clever and solve a problem, but it boosts your pay and makes people who follow in your role hate you a lot less if they can understand why you chose a particular solution.
On 11/25/2015 01:24 PM, Nico Kadel-Garcia wrote:> On Wed, Nov 25, 2015 at 6:07 AM, Ruediger Meier <sweet_f_a at gmx.de> wrote: >> Personally I think it's hard enough to write POSIX compatible shell >> scripts and I wouldn't start to add such hacks for fish and tcsh. >> Next week somebody may complain that his "shell" does not >> support "exec ...". > Making things work for more people, when it doesn't introduce a > security risk or break other tools, seems very reasonable. And there > are people out there who who do use both fish and tcsh. > > What seems to be missing in the patch is a comment line, above the > stanza, explaining why the code uses "exec". It's great to be clever > and solve a problem, but it boosts your pay and makes people who > follow in your role hate you a lot less if they can understand why you > chose a particular solution.Currently, it is the only solution we have and works for conventional shells as well as for fish and tcsh. Maybe there are other solutions, better or worse. I am no expert on different shells and their compatibility, so I just shared what we actually use across our systems for some time and which works for us, so other interested can benefit from it. I am not forcing anyone to use it. I completely agree that there should be some wider reasoning behind every change. But here we have just "experience" with larger subset of shells used these days. Regards, -- Jakub Jelen Associate Software Engineer Security Technologies Red Hat
Nico Kadel-Garcia <nkadel at gmail.com> writes:> On Wed, Nov 25, 2015 at 6:07 AM, Ruediger Meier <sweet_f_a at gmx.de> wrote: >> Hi, >> >> On Tuesday 24 November 2015, Radek Podgorny wrote: >>> hello everyone! >>> >>> i'd like to sincerely ask you to include a fix for ssh-copy-id bug >>> i'll be linking below. it's a trivial fix which resolves >>> https://bugzilla.mindrot.org/show_bug.cgi?id=2206 and eases life of >>> many. it's been field-tested by redhat devs and users so i see no >>> problem in incorporating it. >>> >>> http://pkgs.fedoraproject.org/cgit/openssh.git/tree/openssh-6.8p1-fix >>>-ss h-copy-id-on-non-sh-shell.patch > >> Personally I think it's hard enough to write POSIX compatible shell >> scripts and I wouldn't start to add such hacks for fish and tcsh. >> Next week somebody may complain that his "shell" does not >> support "exec ...". > > Making things work for more people, when it doesn't introduce a > security risk or break other tools, seems very reasonable. And there > are people out there who who do use both fish and tcsh. > > What seems to be missing in the patch is a comment line, above the > stanza, explaining why the code uses "exec".My reading of the presence of "exec" there was: We're assuming that the current shell may not be to our liking, so there seems to be little point keeping it in memory solely so it can at worst somehow get in the way of a clean exit. Does that really need a comment? I'm not sure I can make a succinct explanation of what's going on for anyone that doesn't already know what exec does. Feel free to make suggestions though. Cheers, Phil. -- |)| Philip Hands [+44 (0)20 8530 9560] HANDS.COM Ltd. |-| http://www.hands.com/ http://ftp.uk.debian.org/ |(| Hugo-Klemm-Strasse 34, 21075 Hamburg, GERMANY -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 818 bytes Desc: not available URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20151125/02ab5b45/attachment.bin>
On Wed, Nov 25, 2015 at 12:20 PM, Philip Hands <phil at hands.com> wrote:> Nico Kadel-Garcia <nkadel at gmail.com> writes:>> What seems to be missing in the patch is a comment line, above the >> stanza, explaining why the code uses "exec". > > My reading of the presence of "exec" there was: > > We're assuming that the current shell may not be to our liking, so > there seems to be little point keeping it in memory solely so it can > at worst somehow get in the way of a clean exit. > > Does that really need a comment? I'm not sure I can make a succinct > explanation of what's going on for anyone that doesn't already know what > exec does. Feel free to make suggestions though.That is _precisely_ why it needs a comment. It's a selection of a particular technology for a particular reason that someone may not understand as important without having to dig back to a thread or bug report like this. For example: # Use "exec sh -c" to ensure POSIX compliant scripting, especially for fish and tcsh users [ "$DRY_RUN" ] || printf '%s\n' "$NEW_IDS" | ssh "$@" " .....