All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/openssh: reset umask when init script exits
@ 2021-10-18 20:30 Will Eccles
  2021-10-19 20:25 ` Arnout Vandecappelle
  2021-10-21 17:46 ` Yann E. MORIN
  0 siblings, 2 replies; 7+ messages in thread
From: Will Eccles @ 2021-10-18 20:30 UTC (permalink / raw)
  To: buildroot; +Cc: Will Eccles

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). 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@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() {
-- 
2.33.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/openssh: reset umask when init script exits
  2021-10-18 20:30 [Buildroot] [PATCH] package/openssh: reset umask when init script exits Will Eccles
@ 2021-10-19 20:25 ` Arnout Vandecappelle
  2021-10-19 21:42   ` Will Eccles
  2021-10-21 17:46 ` Yann E. MORIN
  1 sibling, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2021-10-19 20:25 UTC (permalink / raw)
  To: Will Eccles, buildroot

  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@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() {
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/openssh: reset umask when init script exits
  2021-10-19 20:25 ` Arnout Vandecappelle
@ 2021-10-19 21:42   ` Will Eccles
  2021-10-20 12:08     ` Arnout Vandecappelle
  0 siblings, 1 reply; 7+ messages in thread
From: Will Eccles @ 2021-10-19 21:42 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: buildroot


[-- Attachment #1.1: Type: text/plain, Size: 4452 bytes --]

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@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@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() {
> >
>

[-- Attachment #1.2: Type: text/html, Size: 5448 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/openssh: reset umask when init script exits
  2021-10-19 21:42   ` Will Eccles
@ 2021-10-20 12:08     ` Arnout Vandecappelle
  2021-10-20 13:57       ` Will Eccles
  0 siblings, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2021-10-20 12:08 UTC (permalink / raw)
  To: Will Eccles; +Cc: buildroot



On 19/10/2021 23:42, Will Eccles wrote:
> 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)
> 

  Hm, this reminds me: I believe that busybox ash does something smart to 
execute shell scripts. IIRC, it fork()s but doesn't exec(), instead directly 
jumping to the interpreter.

  That doesn't actually explain the behaviour you observed, because it should 
still not propagate the umask to the parent process... And it definitely doesn't 
explain how that behaviour could suddenly disappear...

  Regards,
  Arnout


> 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@mind.be 
> <mailto:arnout@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@eccles.dev <mailto:will@eccles.dev>>
>      > ---
>      > Backport to: 2021.02.6, 2021.08.1
>      > (These are the releases on buildroot.org <http://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() {
>      >
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/openssh: reset umask when init script exits
  2021-10-20 12:08     ` Arnout Vandecappelle
@ 2021-10-20 13:57       ` Will Eccles
  0 siblings, 0 replies; 7+ messages in thread
From: Will Eccles @ 2021-10-20 13:57 UTC (permalink / raw)
  To: Arnout Vandecappelle; +Cc: buildroot


[-- Attachment #1.1: Type: text/plain, Size: 6460 bytes --]

> IIRC, it fork()s but doesn't exec(), instead directly
> jumping to the interpreter.

Huh, that's interesting. Never looked much into busybox utilities before,
save for a few specific ones.

> That doesn't actually explain the behaviour you observed, because it
> should still not propagate the umask to the parent process... And
> it definitely doesn't explain how that behaviour could suddenly
> disappear...

I agree, and I am still looking into this. No results yet. I am ready to
accept that the system I found this on was somehow broken and this was a
fluke. It's just strange to me that I saw success with this before at all,
now that I think about it.

Yours,
Will


On Wed, Oct 20, 2021 at 8:08 AM Arnout Vandecappelle <arnout@mind.be> wrote:

>
>
> On 19/10/2021 23:42, Will Eccles wrote:
> > 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)
> >
>
>   Hm, this reminds me: I believe that busybox ash does something smart to
> execute shell scripts. IIRC, it fork()s but doesn't exec(), instead
> directly
> jumping to the interpreter.
>
>   That doesn't actually explain the behaviour you observed, because it
> should
> still not propagate the umask to the parent process... And it definitely
> doesn't
> explain how that behaviour could suddenly disappear...
>
>   Regards,
>   Arnout
>
>
> > 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@mind.be
> > <mailto:arnout@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@eccles.dev <mailto:
> will@eccles.dev>>
> >      > ---
> >      > Backport to: 2021.02.6, 2021.08.1
> >      > (These are the releases on buildroot.org <http://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() {
> >      >
> >
>

[-- Attachment #1.2: Type: text/html, Size: 8573 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/openssh: reset umask when init script exits
  2021-10-18 20:30 [Buildroot] [PATCH] package/openssh: reset umask when init script exits Will Eccles
  2021-10-19 20:25 ` Arnout Vandecappelle
@ 2021-10-21 17:46 ` Yann E. MORIN
  2021-10-21 19:21   ` Will Eccles
  1 sibling, 1 reply; 7+ messages in thread
From: Yann E. MORIN @ 2021-10-21 17:46 UTC (permalink / raw)
  To: Will Eccles; +Cc: buildroot

Will, All,

On 2021-10-18 16:30 -0400, Will Eccles spake thusly:
> 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). 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@eccles.dev>

Besides what Arnout said (and which I agree with), I am not so sure this
patch is even technically correct... See below...

> ---
> 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

This does not even do what you said it does. This is 'uname', not
'umask'...

Furthermore, the above code would fail anyway:

    $ uname $(uname); echo $?
    uname: extra operand ‘Linux’
    Try 'uname --help' for more information.
    1

So, if this very patch makes it work for you, then your issue is not
about umask being set below...

Regards,
Yann E. MORIN.

>  umask 077
>  
>  start() {
> -- 
> 2.33.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/openssh: reset umask when init script exits
  2021-10-21 17:46 ` Yann E. MORIN
@ 2021-10-21 19:21   ` Will Eccles
  0 siblings, 0 replies; 7+ messages in thread
From: Will Eccles @ 2021-10-21 19:21 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot


[-- Attachment #1.1: Type: text/plain, Size: 3093 bytes --]

Hi Yann,

Apologies, I have no idea how that typo even made it into the patch. That
is supposed to say "umask $(umask)". Not only did the typo make it in
there, but I haven't even noticed it until now (and the patch I applied to
my own says umask, so apparently I typo'd when formalizing it). In any
case, it's hardly worth correcting now, since the patch isn't useful.

On Thu, Oct 21, 2021 at 1:46 PM Yann E. MORIN <yann.morin.1998@free.fr>
wrote:

> Will, All,
>
> On 2021-10-18 16:30 -0400, Will Eccles spake thusly:
> > 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). 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@eccles.dev>
>
> Besides what Arnout said (and which I agree with), I am not so sure this
> patch is even technically correct... See below...
>
> > ---
> > 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
>
> This does not even do what you said it does. This is 'uname', not
> 'umask'...
>
> Furthermore, the above code would fail anyway:
>
>     $ uname $(uname); echo $?
>     uname: extra operand ‘Linux’
>     Try 'uname --help' for more information.
>     1
>
> So, if this very patch makes it work for you, then your issue is not
> about umask being set below...
>
> Regards,
> Yann E. MORIN.
>
> >  umask 077
> >
> >  start() {
> > --
> > 2.33.1
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
>      |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is
> no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
>  conspiracy.  |
>
> '------------------------------^-------^------------------^--------------------'
>

[-- Attachment #1.2: Type: text/html, Size: 4159 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-21 19:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-18 20:30 [Buildroot] [PATCH] package/openssh: reset umask when init script exits Will Eccles
2021-10-19 20:25 ` Arnout Vandecappelle
2021-10-19 21:42   ` Will Eccles
2021-10-20 12:08     ` Arnout Vandecappelle
2021-10-20 13:57       ` Will Eccles
2021-10-21 17:46 ` Yann E. MORIN
2021-10-21 19:21   ` Will Eccles

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.