[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Romain Naour]

When bash is selected, /bin/bash is not added to /etc/shells
(see man shells). So, login tools like dropbear reject the ssh
connexions for users using bash as shell in /etc/passwd.

buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected

Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
Signed-off-by: Romain Naour <romain.naour@smile.fr>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
The same issue can happend with other shells.

v2: add /bin/bash to /etc/shells only if it's missing (Yann)
---
 package/bash/bash.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/bash/bash.mk b/package/bash/bash.mk
index 089d062..03f8f28 100644
--- a/package/bash/bash.mk
+++ b/package/bash/bash.mk
@@ -40,10 +40,14 @@ endif
 endif
 
 # Make /bin/sh -> bash (no other shell, better than busybox shells)
+# Add /bin/bash to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
 define BASH_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
 		DESTDIR=$(TARGET_DIR) exec_prefix=/ install
 	rm -f $(TARGET_DIR)/bin/bashbug
+	grep -qsE '^/bin/bash' $(TARGET_DIR)/etc/shells \
+		|| echo "/bin/bash" >> $(TARGET_DIR)/etc/shells
 endef
 
 $(eval $(autotools-package))
-- 
2.7.4

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Yann E. MORIN]

Romain, All,

On 2018-01-13 17:05 +0100, Romain Naour spake thusly:
> When bash is selected, /bin/bash is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using bash as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
> The same issue can happend with other shells.
> 
> v2: add /bin/bash to /etc/shells only if it's missing (Yann)
> ---
>  package/bash/bash.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/bash/bash.mk b/package/bash/bash.mk
> index 089d062..03f8f28 100644
> --- a/package/bash/bash.mk
> +++ b/package/bash/bash.mk
> @@ -40,10 +40,14 @@ endif
>  endif
>  
>  # Make /bin/sh -> bash (no other shell, better than busybox shells)
> +# Add /bin/bash to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
>  define BASH_INSTALL_TARGET_CMDS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
>  		DESTDIR=$(TARGET_DIR) exec_prefix=/ install
>  	rm -f $(TARGET_DIR)/bin/bashbug
> +	grep -qsE '^/bin/bash' $(TARGET_DIR)/etc/shells \
> +		|| echo "/bin/bash" >> $(TARGET_DIR)/etc/shells
>  endef
>  
>  $(eval $(autotools-package))
> -- 
> 2.7.4
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Thomas Petazzoni]

Hello,

On Sat, 13 Jan 2018 17:05:27 +0100, Romain Naour wrote:
> When bash is selected, /bin/bash is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using bash as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> The same issue can happend with other shells.

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Arnout Vandecappelle]

On 13-01-18 17:12, Yann E. MORIN wrote:
> Romain, All,
> 
> On 2018-01-13 17:05 +0100, Romain Naour spake thusly:
>> When bash is selected, /bin/bash is not added to /etc/shells
>> (see man shells). So, login tools like dropbear reject the ssh
>> connexions for users using bash as shell in /etc/passwd.
>>
>> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
>>
>> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

 Really? The guy who wrote the script that checks that no two packages can touch
the same file acks this change?

[snip]
>> +	grep -qsE '^/bin/bash' $(TARGET_DIR)/etc/shells \
>> +		|| echo "/bin/bash" >> $(TARGET_DIR)/etc/shells

 So this is a nice example of a package breaking top-level parallel build.

 I guess the solution is to collect the shells in a make variable and create
/etc/shells in a finalize hook. Or alternatively, do this in a finalize hook
instead of target-install hook.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Romain Naour]

Hi Arnout,

Le 18/01/2018 ? 00:53, Arnout Vandecappelle a ?crit?:
> 
> 
> On 13-01-18 17:12, Yann E. MORIN wrote:
>> Romain, All,
>>
>> On 2018-01-13 17:05 +0100, Romain Naour spake thusly:
>>> When bash is selected, /bin/bash is not added to /etc/shells
>>> (see man shells). So, login tools like dropbear reject the ssh
>>> connexions for users using bash as shell in /etc/passwd.
>>>
>>> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
>>>
>>> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
>>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>
>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
>  Really? The guy who wrote the script that checks that no two packages can touch
> the same file acks this change?

Which script ?

> 
> [snip]
>>> +	grep -qsE '^/bin/bash' $(TARGET_DIR)/etc/shells \
>>> +		|| echo "/bin/bash" >> $(TARGET_DIR)/etc/shells
> 
>  So this is a nice example of a package breaking top-level parallel build.

ok, I did not take into account the top-level parallel build...

> 
>  I guess the solution is to collect the shells in a make variable and create
> /etc/shells in a finalize hook. Or alternatively, do this in a finalize hook
> instead of target-install hook.

What do you think about something in the pkg-generic infra ?

A package like bash register the shell path using

BASH_REGISTER_SHELL = /bin/bash

And the pkg-generic infra will add a hook in TARGET_FINALIZE_HOOKS.

Yann suggested this while discussing about this series.
We hesitated to modify the infra just for less that 10 packages, so I've keep
the /etc/shells handling in each packages.

About the finalize hook, the manual say:
"They are seldom used, and your package probably do not need them."
It discourages users to use them.

Best regards,
Romain

> 
>  Regards,
>  Arnout
> 

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Arnout Vandecappelle]

On 18-01-18 10:46, Romain Naour wrote:
> Hi Arnout,
> 
> Le 18/01/2018 ? 00:53, Arnout Vandecappelle a ?crit?:
>>
>>
>> On 13-01-18 17:12, Yann E. MORIN wrote:
>>> Romain, All,
>>>
>>> On 2018-01-13 17:05 +0100, Romain Naour spake thusly:
>>>> When bash is selected, /bin/bash is not added to /etc/shells
>>>> (see man shells). So, login tools like dropbear reject the ssh
>>>> connexions for users using bash as shell in /etc/passwd.
>>>>
>>>> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
>>>>
>>>> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
>>>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>>>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>
>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>
>>  Really? The guy who wrote the script that checks that no two packages can touch
>> the same file acks this change?
> 
> Which script ?

 support/scripts/check-uniq-files was created by Yann, who Acked your patch.

[snip]
>>  I guess the solution is to collect the shells in a make variable and create
>> /etc/shells in a finalize hook. Or alternatively, do this in a finalize hook
>> instead of target-install hook.
> 
> What do you think about something in the pkg-generic infra ?

 That's what I meant with "collect the shells in a make variable".


> A package like bash register the shell path using
> 
> BASH_REGISTER_SHELL = /bin/bash
> 
> And the pkg-generic infra will add a hook in TARGET_FINALIZE_HOOKS.
> 
> Yann suggested this while discussing about this series.
> We hesitated to modify the infra just for less that 10 packages, so I've keep
> the /etc/shells handling in each packages.

 Indeed I also prefer to avoid adding infra when it is not needed. We have too
much infra already IMO.


> About the finalize hook, the manual say:
> "They are seldom used, and your package probably do not need them."
> It discourages users to use them.

 Well, changing the infra is discouraged even more, so much that it's not even
mentioned as a possibility in the manual :-).

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Romain Naour]

Hi Arnout,

Le 18/01/2018 ? 13:25, Arnout Vandecappelle a ?crit?:
> 
> 
> On 18-01-18 10:46, Romain Naour wrote:
>> Hi Arnout,
>>
>> Le 18/01/2018 ? 00:53, Arnout Vandecappelle a ?crit?:
>>>
>>>
>>> On 13-01-18 17:12, Yann E. MORIN wrote:
>>>> Romain, All,
>>>>
>>>> On 2018-01-13 17:05 +0100, Romain Naour spake thusly:
>>>>> When bash is selected, /bin/bash is not added to /etc/shells
>>>>> (see man shells). So, login tools like dropbear reject the ssh
>>>>> connexions for users using bash as shell in /etc/passwd.
>>>>>
>>>>> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
>>>>>
>>>>> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
>>>>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>>>>> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>>
>>>> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>>
>>>  Really? The guy who wrote the script that checks that no two packages can touch
>>> the same file acks this change?
>>
>> Which script ?
> 
>  support/scripts/check-uniq-files was created by Yann, who Acked your patch.

Thanks, I missed this new feature.

> 
> [snip]
>>>  I guess the solution is to collect the shells in a make variable and create
>>> /etc/shells in a finalize hook. Or alternatively, do this in a finalize hook
>>> instead of target-install hook.
>>
>> What do you think about something in the pkg-generic infra ?
> 
>  That's what I meant with "collect the shells in a make variable".
> 
> 
>> A package like bash register the shell path using
>>
>> BASH_REGISTER_SHELL = /bin/bash
>>
>> And the pkg-generic infra will add a hook in TARGET_FINALIZE_HOOKS.
>>
>> Yann suggested this while discussing about this series.
>> We hesitated to modify the infra just for less that 10 packages, so I've keep
>> the /etc/shells handling in each packages.
> 
>  Indeed I also prefer to avoid adding infra when it is not needed. We have too
> much infra already IMO.

Ok, I'll changes to a TARGET_FINALIZE_HOOKS, it doesn't matter when the
/etc/shells file is generated.

> 
>> About the finalize hook, the manual say:
>> "They are seldom used, and your package probably do not need them."
>> It discourages users to use them.
> 
>  Well, changing the infra is discouraged even more, so much that it's not even
> mentioned as a possibility in the manual :-).

:)

Best regards,
Romain

> 
>  Regards,
>  Arnout
> 

[Buildroot] [PATCH v2] package/bash: add /bin/bash to /etc/shells

[Yann E. MORIN]

Arnout, All,

On 2018-01-18 00:53 +0100, Arnout Vandecappelle spake thusly:
> On 13-01-18 17:12, Yann E. MORIN wrote:
> > Romain, All,
> > 
> > On 2018-01-13 17:05 +0100, Romain Naour spake thusly:
> >> When bash is selected, /bin/bash is not added to /etc/shells
> >> (see man shells). So, login tools like dropbear reject the ssh
> >> connexions for users using bash as shell in /etc/passwd.
> >>
> >> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> >>
> >> Reported-by: Jeremy Rosen <jeremy.rosen@smile.fr>
> >> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> >> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
>  Really? The guy who wrote the script that checks that no two packages can touch
> the same file acks this change?

Yes, nobody's perfect, and I never claimed I was.

Oh, I forgot:  ;-)

> [snip]
> >> +	grep -qsE '^/bin/bash' $(TARGET_DIR)/etc/shells \
> >> +		|| echo "/bin/bash" >> $(TARGET_DIR)/etc/shells
> 
>  So this is a nice example of a package breaking top-level parallel build.
> 
>  I guess the solution is to collect the shells in a make variable and create
> /etc/shells in a finalize hook. Or alternatively, do this in a finalize hook
> instead of target-install hook.

Yeah, target-finalize hooks is way better, because that does not add any
infra; it just uses existign infra.

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'