[Buildroot] [PATCH] package/openssh: reset umask when init script exits

[Will Eccles]

Hi Arnout,

Interestingly, I have tested this in a few different ways and achieved
different results each time. I agree with you that the patch does not make
much sense, but on the system I wrote it on (or rather, the system I
originally tested the fix on, which was otherwise unmodified), it seemed to
fix the problem. The exact steps I followed to test this were:

1. Build the system image (of course)
2. Login as root and test that the uname value was 077 (no config files
were sourced by the shell, as none were installed on the system)
3. Apply this patch to the S50sshd file and reboot (nothing else was
changed that I'm aware of; I was logged in for all of 1 minute to make the
change and reboot)
4. Login as root again and test that the uname value was now 022 (as I
expected it to be)

I have no idea how this patch would fix the issue, and I agree that it
makes no sense, but even in a small test I had done on the host system I
had achieved results which appeared to agree with this patch. On a new
system (built minutes ago), I cannot reproduce the exact same steps I took
above, which is quite puzzling, as nothing has changed (aside from a single
device tree overlay I added, which should have no relevance here at all).

> And when you source the script, the trap doesn't even trigger at the end
of
> the script, so this patch doesn't actually reset the umask.

I wrote the script under the assumption that it was never sourced, so I
didn't even consider this. The file shouldn't ever be sourced as it doesn't
end in .sh anyway.

I am willing to accept that I had some sort of gremlin on the original
system I tested this on. I can't seem to reproduce it on a newly-generated
image while following precisely the same steps I did above. However, this
only raises more questions for me to investigate, as I have no clue what
else could have caused the behavior I saw on the original system. I admit I
didn't really think too hard about why it would work this way, I just sort
of accepted it and moved on. Will investigate further and see if I can
reproduce it again.

Yours,
Will Eccles

On Tue, Oct 19, 2021 at 4:25 PM Arnout Vandecappelle <arnout at mind.be> wrote:

>   Hi Will,
>
> On 18/10/2021 22:30, Will Eccles wrote:
> > S50sshd updates umask to 077, but does not reset it when it exits. This
> > results in the root user's umask being configured incorrectly (assuming
> > a default of 022 or otherwise).
>
>   Can you explain in which context this happens?
>
>   Normally this script is executed by /etc/init.d/rcS, which contains this
> code:
>
>       case "$i" in
>          *.sh)
>              # Source shell script for speed.
>              (
>                  trap - INT QUIT TSTP
>                  set start
>                  . $i
>              )
>              ;;
>          *)
>              # No sh extension, so fork subprocess.
>              $i start
>              ;;
>
>   Since the script doesn't end with .sh, it will fork, so the umask
> doesn't "stick".
>
>   Same when you execute the script interactively: the umask isn't
> inherited by
> the parent shell.
>
>   And when you source the script, the trap doesn't even trigger at the end
> of
> the script, so this patch doesn't actually reset the umask.
>
>
>   So I don't understand how it's possible that this patch fixes your
> problem.
>
>
>   Regards,
>   Arnout
>
>
> > This patch adds a trap to reset umask
> > when the script exits. This is convenient on systems where, for example,
> > configs such as /etc/profile may not be sourced by the root user. It may
> > also prevent issues with other init scripts which may inherit this umask
> > unintentionally, leading to improper permissions elsewhere in the
> > system.
> >
> > Signed-off-by: Will Eccles <will at eccles.dev>
> > ---
> > Backport to: 2021.02.6, 2021.08.1
> > (These are the releases on buildroot.org as of this writing, but as far
> > as I can tell, even releases as far back as 2012 have the same problem.)
> > ---
> >   package/openssh/S50sshd | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/package/openssh/S50sshd b/package/openssh/S50sshd
> > index 22da41d1ca..94cf4c14e8 100644
> > --- a/package/openssh/S50sshd
> > +++ b/package/openssh/S50sshd
> > @@ -6,6 +6,8 @@
> >   # Make sure the ssh-keygen progam exists
> >   [ -f /usr/bin/ssh-keygen ] || exit 0
> >
> > +# Reset uname at exit
> > +trap "uname $(uname)" EXIT
> >   umask 077
> >
> >   start() {
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildroot.org/pipermail/buildroot/attachments/20211019/cf9fde97/attachment-0001.html>