All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/7] Add /etc/shells handling
@ 2018-01-14 21:03 Romain Naour
  2018-01-14 21:03 ` [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells Romain Naour
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

Started with commit [1], all package providing a shell interpreter
should  be listed in /etc/shells file to avoid issues with dropbear
or other tools.

Although a macro could be added to pkg-generic.mk to avoid
duplicating the /etc/shells handling, there is less than ten
shell packages in Buildroot. So add /etc/shells handling as
a POST_INSTALL_TARGET_HOOKS or inside INSTALL_TARGET_CMDS.

[1] 4d279697afbf8fb295274784103be2b837113d5e

Romain Naour (7):
  package/skeleton-init-common: add /bin/sh to /etc/shells
  package/busybox: add /bin/ash to /etc/shells
  package/dash: add /etc/dash to /etc/shells
  package/screen: add /usr/bin/screen to /etc/shells
  package/tmux: add /usr/bin/tmux to /etc/shells
  package/zsh: add /bin/zsh to /etc/shells
  package/mksh: add /bin/mksh to /etc/shells

 package/busybox/busybox.mk                           | 10 ++++++++++
 package/dash/dash.mk                                 |  4 ++++
 package/mksh/mksh.mk                                 |  9 +++++++++
 package/screen/screen.mk                             | 12 +++++++++++-
 package/skeleton-init-common/skeleton-init-common.mk |  7 +++++++
 package/tmux/tmux.mk                                 |  9 +++++++++
 package/zsh/zsh.mk                                   | 10 +++++++++-
 system/skeleton/etc/shells                           |  1 +
 8 files changed, 60 insertions(+), 2 deletions(-)
 create mode 100644 system/skeleton/etc/shells

-- 
2.7.4

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

* [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:51   ` Yann E. MORIN
  2018-01-15 14:41   ` Peter Korsgaard
  2018-01-14 21:03 ` [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash " Romain Naour
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

Add the default shell /bin/sh to /etc/shells only if BR2_SYSTEM_BIN_SH_NONE
is not selected.

Add an empty /etc/shells in the rootfs skeleton.

Signed-off-by: Romain Naour <romain.naour@smile.fr>
---
 package/skeleton-init-common/skeleton-init-common.mk | 7 +++++++
 system/skeleton/etc/shells                           | 1 +
 2 files changed, 8 insertions(+)
 create mode 100644 system/skeleton/etc/shells

diff --git a/package/skeleton-init-common/skeleton-init-common.mk b/package/skeleton-init-common/skeleton-init-common.mk
index 8228a81..707f1aa 100644
--- a/package/skeleton-init-common/skeleton-init-common.mk
+++ b/package/skeleton-init-common/skeleton-init-common.mk
@@ -79,6 +79,12 @@ define SKELETON_INIT_COMMON_SET_BIN_SH
 	rm -f $(TARGET_DIR)/bin/sh
 endef
 else
+# Add /bin/sh to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
+define SKELETON_INIT_COMMON_ADD_SH_TO_SHELLS
+	grep -qsE '^/bin/sh' $(TARGET_DIR)/etc/shells \
+		|| echo "/bin/sh" >> $(TARGET_DIR)/etc/shells
+endef
 ifneq ($(SKELETON_INIT_COMMON_BIN_SH),)
 define SKELETON_INIT_COMMON_SET_BIN_SH
 	ln -sf $(SKELETON_INIT_COMMON_BIN_SH) $(TARGET_DIR)/bin/sh
@@ -86,5 +92,6 @@ endef
 endif
 endif
 SKELETON_INIT_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_INIT_COMMON_SET_BIN_SH
+SKELETON_INIT_COMMON_POST_INSTALL_TARGET_HOOKS += SKELETON_INIT_COMMON_ADD_SH_TO_SHELLS
 
 $(eval $(generic-package))
diff --git a/system/skeleton/etc/shells b/system/skeleton/etc/shells
new file mode 100644
index 0000000..b5fd584
--- /dev/null
+++ b/system/skeleton/etc/shells
@@ -0,0 +1 @@
+# /etc/shells: valid login shells
-- 
2.7.4

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

* [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
  2018-01-14 21:03 ` [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:43   ` Yann E. MORIN
  2018-01-14 21:03 ` [Buildroot] [PATCH 3/7] package/dash: add /etc/dash " Romain Naour
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

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

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

ash is disabled for MMUless systems, so don't add /bin/ash in
/etc/shells in this case.

Signed-off-by: Romain Naour <romain.naour@smile.fr>
---
 package/busybox/busybox.mk | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index d0bbd3f..37d064a 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -258,6 +258,15 @@ define BUSYBOX_INSTALL_TELNET_SCRIPT
 	fi
 endef
 
+# Add /bin/ash to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
+ifeq ($(BR2_USE_MMU),y)
+define BUSYBOX_INSTALL_ADD_ASH_TO_SHELLS
+	grep -qsE '^/bin/ash' $(TARGET_DIR)/etc/shells \
+		|| echo "/bin/ash" >> $(TARGET_DIR)/etc/shells
+endef
+endif
+
 # Enable "noclobber" in install.sh, to prevent BusyBox from overwriting any
 # full-blown versions of apps installed by other packages with sym/hard links.
 define BUSYBOX_NOCLOBBER_INSTALL
@@ -291,6 +300,7 @@ define BUSYBOX_INSTALL_TARGET_CMDS
 	$(BUSYBOX_INSTALL_INITTAB)
 	$(BUSYBOX_INSTALL_UDHCPC_SCRIPT)
 	$(BUSYBOX_INSTALL_MDEV_CONF)
+	$(BUSYBOX_INSTALL_ADD_ASH_TO_SHELLS)
 endef
 
 define BUSYBOX_INSTALL_INIT_SYSV
-- 
2.7.4

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

* [Buildroot] [PATCH 3/7] package/dash: add /etc/dash to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
  2018-01-14 21:03 ` [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells Romain Naour
  2018-01-14 21:03 ` [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash " Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:26   ` Yann E. MORIN
  2018-01-14 21:03 ` [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen " Romain Naour
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Romain Naour <romain.naour@smile.fr>
---
 package/dash/dash.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/dash/dash.mk b/package/dash/dash.mk
index c8b4164..391c06e 100644
--- a/package/dash/dash.mk
+++ b/package/dash/dash.mk
@@ -9,8 +9,12 @@ DASH_SITE = http://gondor.apana.org.au/~herbert/dash/files
 DASH_LICENSE = BSD-3-Clause, GPL-2.0+ (mksignames.c)
 DASH_LICENSE_FILES = COPYING
 
+# Add /bin/dash to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
 define DASH_INSTALL_TARGET_CMDS
 	$(INSTALL) -m 0755 $(@D)/src/dash $(TARGET_DIR)/bin/dash
+	grep -qsE '^/bin/dash' $(TARGET_DIR)/etc/shells \
+		|| echo "/bin/dash" >> $(TARGET_DIR)/etc/shells
 endef
 
 $(eval $(autotools-package))
-- 
2.7.4

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

* [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
                   ` (2 preceding siblings ...)
  2018-01-14 21:03 ` [Buildroot] [PATCH 3/7] package/dash: add /etc/dash " Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:29   ` Yann E. MORIN
  2018-01-14 21:03 ` [Buildroot] [PATCH 5/7] package/tmux: add /usr/bin/tmux " Romain Naour
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Romain Naour <romain.naour@smile.fr>
---
 package/screen/screen.mk | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/package/screen/screen.mk b/package/screen/screen.mk
index 8d67c04..5f2eb1e 100644
--- a/package/screen/screen.mk
+++ b/package/screen/screen.mk
@@ -16,8 +16,18 @@ SCREEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) SCREEN=screen install_bin
 
 define SCREEN_INSTALL_SCREENRC
 	$(INSTALL) -m 0755 -D $(@D)/etc/screenrc $(TARGET_DIR)/etc/screenrc
+	grep -qsE '^/usr/bin/screen' $(TARGET_DIR)/etc/shells \
+		|| echo "/usr/bin/screen" >> $(TARGET_DIR)/etc/shells
 endef
 
-SCREEN_POST_INSTALL_TARGET_HOOKS += SCREEN_INSTALL_SCREENRC
+# Add /usr/bin/screen to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
+define SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS
+	grep -qsE '^/usr/bin/screen' $(TARGET_DIR)/etc/shells \
+		|| echo "/usr/bin/screen" >> $(TARGET_DIR)/etc/shells
+endef
+
+SCREEN_POST_INSTALL_TARGET_HOOKS += SCREEN_INSTALL_SCREENRC \
+	SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS
 
 $(eval $(autotools-package))
-- 
2.7.4

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

* [Buildroot] [PATCH 5/7] package/tmux: add /usr/bin/tmux to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
                   ` (3 preceding siblings ...)
  2018-01-14 21:03 ` [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen " Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:30   ` Yann E. MORIN
  2018-01-14 21:03 ` [Buildroot] [PATCH 6/7] package/zsh: add /bin/zsh " Romain Naour
  2018-01-14 21:03 ` [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh " Romain Naour
  6 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Romain Naour <romain.naour@smile.fr>
Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/tmux/tmux.mk | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/package/tmux/tmux.mk b/package/tmux/tmux.mk
index 6e03d01..27727fe 100644
--- a/package/tmux/tmux.mk
+++ b/package/tmux/tmux.mk
@@ -10,4 +10,13 @@ TMUX_LICENSE = ISC
 TMUX_LICENSE_FILES = README
 TMUX_DEPENDENCIES = libevent ncurses host-pkgconf
 
+# Add /usr/bin/tmux to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
+define TMUX_INSTALL_ADD_TMUX_TO_SHELLS
+	grep -qsE '^/usr/bin/tmux' $(TARGET_DIR)/etc/shells \
+		|| echo "/usr/bin/tmux" >> $(TARGET_DIR)/etc/shells
+endef
+
+TMUX_POST_INSTALL_TARGET_HOOKS += TMUX_INSTALL_ADD_TMUX_TO_SHELLS
+
 $(eval $(autotools-package))
-- 
2.7.4

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

* [Buildroot] [PATCH 6/7] package/zsh: add /bin/zsh to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
                   ` (4 preceding siblings ...)
  2018-01-14 21:03 ` [Buildroot] [PATCH 5/7] package/tmux: add /usr/bin/tmux " Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:34   ` Yann E. MORIN
  2018-01-14 21:03 ` [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh " Romain Naour
  6 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Romain Naour <romain.naour@smile.fr>
---
 package/zsh/zsh.mk | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/package/zsh/zsh.mk b/package/zsh/zsh.mk
index bfd0a1a..c2b0270 100644
--- a/package/zsh/zsh.mk
+++ b/package/zsh/zsh.mk
@@ -34,10 +34,18 @@ else
 ZSH_CONF_OPTS += --disable-pcre
 endif
 
+# Add /bin/zsh to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
+define ZSH_INSTALL_ADD_ZSH_TO_SHELLS
+	grep -qsE '^/bin/zsh' $(TARGET_DIR)/etc/shells \
+		|| echo "/bin/zsh" >> $(TARGET_DIR)/etc/shells
+endef
+
 # Remove versioned zsh-x.y.z binary taking up space
 define ZSH_TARGET_INSTALL_FIXUPS
 	rm -f $(TARGET_DIR)/bin/zsh-$(ZSH_VERSION)
 endef
-ZSH_POST_INSTALL_TARGET_HOOKS += ZSH_TARGET_INSTALL_FIXUPS
+ZSH_POST_INSTALL_TARGET_HOOKS += ZSH_TARGET_INSTALL_FIXUPS \
+	ZSH_INSTALL_ADD_ZSH_TO_SHELLS
 
 $(eval $(autotools-package))
-- 
2.7.4

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

* [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh to /etc/shells
  2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
                   ` (5 preceding siblings ...)
  2018-01-14 21:03 ` [Buildroot] [PATCH 6/7] package/zsh: add /bin/zsh " Romain Naour
@ 2018-01-14 21:03 ` Romain Naour
  2018-01-14 22:37   ` Yann E. MORIN
  6 siblings, 1 reply; 20+ messages in thread
From: Romain Naour @ 2018-01-14 21:03 UTC (permalink / raw)
  To: buildroot

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

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

Signed-off-by: Romain Naour <romain.naour@smile.fr>
---
 package/mksh/mksh.mk | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/package/mksh/mksh.mk b/package/mksh/mksh.mk
index b5706db..52d4b00 100644
--- a/package/mksh/mksh.mk
+++ b/package/mksh/mksh.mk
@@ -21,4 +21,13 @@ define MKSH_INSTALL_TARGET_CMDS
 	$(INSTALL) -m 0755 -D $(@D)/mksh $(TARGET_DIR)/bin/mksh
 endef
 
+# Add /bin/mksh to /etc/shells otherwise some login tools like dropbear
+# can reject the user connexion. See man shells.
+define MKSH_INSTALL_ADD_MKSH_TO_SHELLS
+	grep -qsE '^/bin/mksh' $(TARGET_DIR)/etc/shells \
+		|| echo "/bin/mksh" >> $(TARGET_DIR)/etc/shells
+endef
+
+MKSH_POST_INSTALL_TARGET_HOOKS += MKSH_INSTALL_ADD_MKSH_TO_SHELLS
+
 $(eval $(generic-package))
-- 
2.7.4

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

* [Buildroot] [PATCH 3/7] package/dash: add /etc/dash to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 3/7] package/dash: add /etc/dash " Romain Naour
@ 2018-01-14 22:26   ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:26 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> When dash is selected, /bin/dash is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using dash as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
>  package/dash/dash.mk | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/package/dash/dash.mk b/package/dash/dash.mk
> index c8b4164..391c06e 100644
> --- a/package/dash/dash.mk
> +++ b/package/dash/dash.mk
> @@ -9,8 +9,12 @@ DASH_SITE = http://gondor.apana.org.au/~herbert/dash/files
>  DASH_LICENSE = BSD-3-Clause, GPL-2.0+ (mksignames.c)
>  DASH_LICENSE_FILES = COPYING
>  
> +# Add /bin/dash to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
>  define DASH_INSTALL_TARGET_CMDS
>  	$(INSTALL) -m 0755 $(@D)/src/dash $(TARGET_DIR)/bin/dash
> +	grep -qsE '^/bin/dash' $(TARGET_DIR)/etc/shells \
> +		|| echo "/bin/dash" >> $(TARGET_DIR)/etc/shells

Very unlikely, but what if there already is a shell named /bin/dashfoo ?
You should make sure to also match the end of the line (double-dollar
because of make...):

    grep -qsE '^/bin/dash$$'

Note: comment valid for all the patches in the series, of course...

And now that I re-read your bash patch, I missed it during the review.
Or I did suggest you do, but you missed it, and then I missed it in the
second review...
    http://lists.busybox.net/pipermail/buildroot/2018-January/211338.html

Regards,
Yann E. MORIN.

>  endef
>  
>  $(eval $(autotools-package))
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen " Romain Naour
@ 2018-01-14 22:29   ` Yann E. MORIN
  2018-01-14 22:36     ` Yann E. MORIN
  0 siblings, 1 reply; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:29 UTC (permalink / raw)
  To: buildroot

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> When screen is selected, /usr/bin/screen is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using screen as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
>  package/screen/screen.mk | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/package/screen/screen.mk b/package/screen/screen.mk
> index 8d67c04..5f2eb1e 100644
> --- a/package/screen/screen.mk
> +++ b/package/screen/screen.mk
> @@ -16,8 +16,18 @@ SCREEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) SCREEN=screen install_bin
>  
>  define SCREEN_INSTALL_SCREENRC
>  	$(INSTALL) -m 0755 -D $(@D)/etc/screenrc $(TARGET_DIR)/etc/screenrc
> +	grep -qsE '^/usr/bin/screen' $(TARGET_DIR)/etc/shells \
> +		|| echo "/usr/bin/screen" >> $(TARGET_DIR)/etc/shells
>  endef
>  
> -SCREEN_POST_INSTALL_TARGET_HOOKS += SCREEN_INSTALL_SCREENRC
> +# Add /usr/bin/screen to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
> +define SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS
> +	grep -qsE '^/usr/bin/screen' $(TARGET_DIR)/etc/shells \
> +		|| echo "/usr/bin/screen" >> $(TARGET_DIR)/etc/shells
> +endef

Uh? Why are you doing it twice?

I prefer indeed that we have a macro for each thing.

> +SCREEN_POST_INSTALL_TARGET_HOOKS += SCREEN_INSTALL_SCREENRC \
> +	SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS

For multi=line assignments, we usually try to have a single assignment
per line and the first line is just a continuation line. Besides, not
need for a append-assign (+=), here, it is a sinmple assignment (=):

    SCREEN_POST_INSTALL_TARGET_HOOKS = \
        SCREEN_INSTALL_SCREENRC \
        SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS

Regards,
Yann E. MORIN.

>  $(eval $(autotools-package))
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 5/7] package/tmux: add /usr/bin/tmux to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 5/7] package/tmux: add /usr/bin/tmux " Romain Naour
@ 2018-01-14 22:30   ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:30 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> When tmux is selected, /usr/bin/tmux is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using tmux as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> Cc: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  package/tmux/tmux.mk | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/package/tmux/tmux.mk b/package/tmux/tmux.mk
> index 6e03d01..27727fe 100644
> --- a/package/tmux/tmux.mk
> +++ b/package/tmux/tmux.mk
> @@ -10,4 +10,13 @@ TMUX_LICENSE = ISC
>  TMUX_LICENSE_FILES = README
>  TMUX_DEPENDENCIES = libevent ncurses host-pkgconf
>  
> +# Add /usr/bin/tmux to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
> +define TMUX_INSTALL_ADD_TMUX_TO_SHELLS
> +	grep -qsE '^/usr/bin/tmux' $(TARGET_DIR)/etc/shells \
> +		|| echo "/usr/bin/tmux" >> $(TARGET_DIR)/etc/shells
> +endef
> +
> +TMUX_POST_INSTALL_TARGET_HOOKS += TMUX_INSTALL_ADD_TMUX_TO_SHELLS

No need for an append-assign (+=) here, just use a plain asignment (=).

Regards,
Yann E. MORIN.

>  $(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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 6/7] package/zsh: add /bin/zsh to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 6/7] package/zsh: add /bin/zsh " Romain Naour
@ 2018-01-14 22:34   ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:34 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> When zsh is selected, /bin/zsh is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using zsh as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
>  package/zsh/zsh.mk | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/package/zsh/zsh.mk b/package/zsh/zsh.mk
> index bfd0a1a..c2b0270 100644
> --- a/package/zsh/zsh.mk
> +++ b/package/zsh/zsh.mk
> @@ -34,10 +34,18 @@ else
>  ZSH_CONF_OPTS += --disable-pcre
>  endif
>  
> +# Add /bin/zsh to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
> +define ZSH_INSTALL_ADD_ZSH_TO_SHELLS
> +	grep -qsE '^/bin/zsh' $(TARGET_DIR)/etc/shells \
> +		|| echo "/bin/zsh" >> $(TARGET_DIR)/etc/shells
> +endef
> +
>  # Remove versioned zsh-x.y.z binary taking up space
>  define ZSH_TARGET_INSTALL_FIXUPS
>  	rm -f $(TARGET_DIR)/bin/zsh-$(ZSH_VERSION)
>  endef
> -ZSH_POST_INSTALL_TARGET_HOOKS += ZSH_TARGET_INSTALL_FIXUPS
> +ZSH_POST_INSTALL_TARGET_HOOKS += ZSH_TARGET_INSTALL_FIXUPS \
> +	ZSH_INSTALL_ADD_ZSH_TO_SHELLS

The usual dance here... ;-)

    ZSH_POST_INSTALL_TARGET_HOOKS = \
        ZSH_TARGET_INSTALL_FIXUPS \
        ZSH_INSTALL_ADD_ZSH_TO_SHELLS

Although I wonder if we don;t in fact prefer to assign right after the
definition if the hooks...

    define ZSH_INSTALL_ADD_ZSH_TO_SHELLS
        grep -qsE '^/bin/zsh' $(TARGET_DIR)/etc/shells \
        || echo "/bin/zsh" >> $(TARGET_DIR)/etc/shells
    endef
    ZSH_POST_INSTALL_TARGET_HOOKS = ZSH_INSTALL_ADD_ZSH_TO_SHELLS

    # Remove versioned zsh-x.y.z binary taking up space
    define ZSH_TARGET_INSTALL_FIXUPS
        rm -f $(TARGET_DIR)/bin/zsh-$(ZSH_VERSION)
    endef
    ZSH_POST_INSTALL_TARGET_HOOKS += ZSH_TARGET_INSTALL_FIXUPS

Note that we register before the existign hook, but use a simple
assignment, so that we don;t need to change the xisting append-assign to
a simple assign...

Regards,
Yann E. MORIN.

>  $(eval $(autotools-package))
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen to /etc/shells
  2018-01-14 22:29   ` Yann E. MORIN
@ 2018-01-14 22:36     ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:36 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2018-01-14 23:29 +0100, Yann E. MORIN spake thusly:
> On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> > When screen is selected, /usr/bin/screen is not added to /etc/shells
> > (see man shells). So, login tools like dropbear reject the ssh
> > connexions for users using screen as shell in /etc/passwd.
> > 
> > buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> > 
> > Signed-off-by: Romain Naour <romain.naour@smile.fr>
> > ---
> >  package/screen/screen.mk | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/screen/screen.mk b/package/screen/screen.mk
> > index 8d67c04..5f2eb1e 100644
> > --- a/package/screen/screen.mk
> > +++ b/package/screen/screen.mk
> > @@ -16,8 +16,18 @@ SCREEN_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) SCREEN=screen install_bin
> >  
> >  define SCREEN_INSTALL_SCREENRC
> >  	$(INSTALL) -m 0755 -D $(@D)/etc/screenrc $(TARGET_DIR)/etc/screenrc
> > +	grep -qsE '^/usr/bin/screen' $(TARGET_DIR)/etc/shells \
> > +		|| echo "/usr/bin/screen" >> $(TARGET_DIR)/etc/shells
> >  endef
> >  
> > -SCREEN_POST_INSTALL_TARGET_HOOKS += SCREEN_INSTALL_SCREENRC
> > +# Add /usr/bin/screen to /etc/shells otherwise some login tools like dropbear
> > +# can reject the user connexion. See man shells.
> > +define SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS
> > +	grep -qsE '^/usr/bin/screen' $(TARGET_DIR)/etc/shells \
> > +		|| echo "/usr/bin/screen" >> $(TARGET_DIR)/etc/shells
> > +endef
> 
> Uh? Why are you doing it twice?
> 
> I prefer indeed that we have a macro for each thing.
> 
> > +SCREEN_POST_INSTALL_TARGET_HOOKS += SCREEN_INSTALL_SCREENRC \
> > +	SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS
> 
> For multi=line assignments, we usually try to have a single assignment
> per line and the first line is just a continuation line. Besides, not
> need for a append-assign (+=), here, it is a sinmple assignment (=):
> 
>     SCREEN_POST_INSTALL_TARGET_HOOKS = \
>         SCREEN_INSTALL_SCREENRC \
>         SCREEN_INSTALL_ADD_SCREEN_TO_SHELLS


In fact, jsut register the hooks right after they are defined; do not
try to commonalise the asignment.

Since the existign one isalready an append-assinn (+=), just register
your new hook before the existing one, but assign yours with a plain
assignement (=).

Regards,
Yann E. MORIN.

> Regards,
> Yann E. MORIN.
> 
> >  $(eval $(autotools-package))
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
> 
> -- 
> .-----------------.--------------------.------------------.--------------------.
> |  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 mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh " Romain Naour
@ 2018-01-14 22:37   ` Yann E. MORIN
  2018-01-15 10:35     ` Thomas Petazzoni
  0 siblings, 1 reply; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:37 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> When mksh is selected, /bin/mksh is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using mksh as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
>  package/mksh/mksh.mk | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/package/mksh/mksh.mk b/package/mksh/mksh.mk
> index b5706db..52d4b00 100644
> --- a/package/mksh/mksh.mk
> +++ b/package/mksh/mksh.mk
> @@ -21,4 +21,13 @@ define MKSH_INSTALL_TARGET_CMDS
>  	$(INSTALL) -m 0755 -D $(@D)/mksh $(TARGET_DIR)/bin/mksh
>  endef
>  
> +# Add /bin/mksh to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
> +define MKSH_INSTALL_ADD_MKSH_TO_SHELLS
> +	grep -qsE '^/bin/mksh' $(TARGET_DIR)/etc/shells \
> +		|| echo "/bin/mksh" >> $(TARGET_DIR)/etc/shells
> +endef
> +
> +MKSH_POST_INSTALL_TARGET_HOOKS += MKSH_INSTALL_ADD_MKSH_TO_SHELLS

Use a simple assignement here.

Regards,
Yann E. MORIN.

>  $(eval $(generic-package))
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash " Romain Naour
@ 2018-01-14 22:43   ` Yann E. MORIN
  2018-01-17 15:17     ` Romain Naour
  0 siblings, 1 reply; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:43 UTC (permalink / raw)
  To: buildroot

Romain, All,

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> When ash (busybox) is selected, /bin/ash is not added to /etc/shells
> (see man shells). So, login tools like dropbear reject the ssh
> connexions for users using ash as shell in /etc/passwd.
> 
> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> 
> ash is disabled for MMUless systems, so don't add /bin/ash in
> /etc/shells in this case.
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
>  package/busybox/busybox.mk | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index d0bbd3f..37d064a 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -258,6 +258,15 @@ define BUSYBOX_INSTALL_TELNET_SCRIPT
>  	fi
>  endef
>  
> +# Add /bin/ash to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
> +ifeq ($(BR2_USE_MMU),y)
> +define BUSYBOX_INSTALL_ADD_ASH_TO_SHELLS
> +	grep -qsE '^/bin/ash' $(TARGET_DIR)/etc/shells \
> +		|| echo "/bin/ash" >> $(TARGET_DIR)/etc/shells

What if the user uses a custom config file where ash is not enabled?
What about the other shells busybox may install?

    define BUSYBOX_INSTALL_ADD_TO_SHELLS
        if grep -qsE '^CONFIG_ASH=y$$' $(@D)/.config; then
            grep -qsE '^/bin/ash' $(TARGET_DIR)/etc/shells \
            || echo "/bin/ash" >> $(TARGET_DIR)/etc/shells; \
        fi
        if grep -qsE '^CONFIG_HUSH=y$$' $(@D)/.config; then
            grep -qsE '^/bin/hush' $(TARGET_DIR)/etc/shells \
            || echo "/bin/hush" >> $(TARGET_DIR)/etc/shells; \
        fi
        if grep -qsE '^CONFIG_MSH=y$$' $(@D)/.config; then
            grep -qsE '^/bin/msh' $(TARGET_DIR)/etc/shells \
            || echo "/bin/msh" >> $(TARGET_DIR)/etc/shells; \
        fi
    endef

(msh is just an alias for hush, but it can be a shell nonethless...)

Regards,
Yann E. MORIN.

> +endef
> +endif
> +
>  # Enable "noclobber" in install.sh, to prevent BusyBox from overwriting any
>  # full-blown versions of apps installed by other packages with sym/hard links.
>  define BUSYBOX_NOCLOBBER_INSTALL
> @@ -291,6 +300,7 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>  	$(BUSYBOX_INSTALL_INITTAB)
>  	$(BUSYBOX_INSTALL_UDHCPC_SCRIPT)
>  	$(BUSYBOX_INSTALL_MDEV_CONF)
> +	$(BUSYBOX_INSTALL_ADD_ASH_TO_SHELLS)
>  endef
>  
>  define BUSYBOX_INSTALL_INIT_SYSV
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells Romain Naour
@ 2018-01-14 22:51   ` Yann E. MORIN
  2018-01-15 14:41   ` Peter Korsgaard
  1 sibling, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-14 22:51 UTC (permalink / raw)
  To: buildroot

On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> Add the default shell /bin/sh to /etc/shells only if BR2_SYSTEM_BIN_SH_NONE
> is not selected.
> 
> Add an empty /etc/shells in the rootfs skeleton.
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> ---
>  package/skeleton-init-common/skeleton-init-common.mk | 7 +++++++
>  system/skeleton/etc/shells                           | 1 +
>  2 files changed, 8 insertions(+)
>  create mode 100644 system/skeleton/etc/shells
> 
> diff --git a/package/skeleton-init-common/skeleton-init-common.mk b/package/skeleton-init-common/skeleton-init-common.mk
> index 8228a81..707f1aa 100644
> --- a/package/skeleton-init-common/skeleton-init-common.mk
> +++ b/package/skeleton-init-common/skeleton-init-common.mk
> @@ -79,6 +79,12 @@ define SKELETON_INIT_COMMON_SET_BIN_SH
>  	rm -f $(TARGET_DIR)/bin/sh
>  endef
>  else
> +# Add /bin/sh to /etc/shells otherwise some login tools like dropbear
> +# can reject the user connexion. See man shells.
> +define SKELETON_INIT_COMMON_ADD_SH_TO_SHELLS
> +	grep -qsE '^/bin/sh' $(TARGET_DIR)/etc/shells \
> +		|| echo "/bin/sh" >> $(TARGET_DIR)/etc/shells
> +endef
>  ifneq ($(SKELETON_INIT_COMMON_BIN_SH),)
>  define SKELETON_INIT_COMMON_SET_BIN_SH
>  	ln -sf $(SKELETON_INIT_COMMON_BIN_SH) $(TARGET_DIR)/bin/sh
> @@ -86,5 +92,6 @@ endef
>  endif
>  endif
>  SKELETON_INIT_COMMON_TARGET_FINALIZE_HOOKS += SKELETON_INIT_COMMON_SET_BIN_SH
> +SKELETON_INIT_COMMON_POST_INSTALL_TARGET_HOOKS += SKELETON_INIT_COMMON_ADD_SH_TO_SHELLS

Rgister your new hook right after it's defined.

The existing one is registered after the if-block, becasue it is the
same hook-name for the two cases of the condition, b ut your new hook is
only defined in the else case, so register it there.

Regards,
Yann E. MORIN.

>  $(eval $(generic-package))
> diff --git a/system/skeleton/etc/shells b/system/skeleton/etc/shells
> new file mode 100644
> index 0000000..b5fd584
> --- /dev/null
> +++ b/system/skeleton/etc/shells
> @@ -0,0 +1 @@
> +# /etc/shells: valid login shells
> -- 
> 2.7.4
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh to /etc/shells
  2018-01-14 22:37   ` Yann E. MORIN
@ 2018-01-15 10:35     ` Thomas Petazzoni
  0 siblings, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2018-01-15 10:35 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 14 Jan 2018 23:37:36 +0100, Yann E. MORIN wrote:

> On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
> > When mksh is selected, /bin/mksh is not added to /etc/shells
> > (see man shells). So, login tools like dropbear reject the ssh
> > connexions for users using mksh as shell in /etc/passwd.
> > 
> > buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
> > 
> > Signed-off-by: Romain Naour <romain.naour@smile.fr>
> > ---
> >  package/mksh/mksh.mk | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/package/mksh/mksh.mk b/package/mksh/mksh.mk
> > index b5706db..52d4b00 100644
> > --- a/package/mksh/mksh.mk
> > +++ b/package/mksh/mksh.mk
> > @@ -21,4 +21,13 @@ define MKSH_INSTALL_TARGET_CMDS
> >  	$(INSTALL) -m 0755 -D $(@D)/mksh $(TARGET_DIR)/bin/mksh
> >  endef
> >  
> > +# Add /bin/mksh to /etc/shells otherwise some login tools like dropbear
> > +# can reject the user connexion. See man shells.
> > +define MKSH_INSTALL_ADD_MKSH_TO_SHELLS
> > +	grep -qsE '^/bin/mksh' $(TARGET_DIR)/etc/shells \
> > +		|| echo "/bin/mksh" >> $(TARGET_DIR)/etc/shells
> > +endef
> > +
> > +MKSH_POST_INSTALL_TARGET_HOOKS += MKSH_INSTALL_ADD_MKSH_TO_SHELLS  
> 
> Use a simple assignement here.

Well, actually, I'm quite happy with using += for hooks, even if
there's only one registered.

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

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

* [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells
  2018-01-14 21:03 ` [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells Romain Naour
  2018-01-14 22:51   ` Yann E. MORIN
@ 2018-01-15 14:41   ` Peter Korsgaard
  2018-01-15 17:06     ` Yann E. MORIN
  1 sibling, 1 reply; 20+ messages in thread
From: Peter Korsgaard @ 2018-01-15 14:41 UTC (permalink / raw)
  To: buildroot

>>>>> "Romain" == Romain Naour <romain.naour@smile.fr> writes:

 > Add the default shell /bin/sh to /etc/shells only if BR2_SYSTEM_BIN_SH_NONE
 > is not selected.

 > Add an empty /etc/shells in the rootfs skeleton.

Is there any specific reason to have an empty /etc/shells?

It is a bit noisy to have this extra file in the BIN_SH_NONE case, and
it afaik changes behaviour (E.G. getusershell() will return default
values if there is no /etc/shells, but will now presumably return the
empty string).

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells
  2018-01-15 14:41   ` Peter Korsgaard
@ 2018-01-15 17:06     ` Yann E. MORIN
  0 siblings, 0 replies; 20+ messages in thread
From: Yann E. MORIN @ 2018-01-15 17:06 UTC (permalink / raw)
  To: buildroot

Peter, All,

On 2018-01-15 15:41 +0100, Peter Korsgaard spake thusly:
> >>>>> "Romain" == Romain Naour <romain.naour@smile.fr> writes:
>  > Add the default shell /bin/sh to /etc/shells only if BR2_SYSTEM_BIN_SH_NONE
>  > is not selected.
> 
>  > Add an empty /etc/shells in the rootfs skeleton.
> 
> Is there any specific reason to have an empty /etc/shells?

In fact, I don't see the point of an empty one, indeed...

> It is a bit noisy to have this extra file in the BIN_SH_NONE case, and
> it afaik changes behaviour (E.G. getusershell() will return default
> values if there is no /etc/shells, but will now presumably return the
> empty string).

Woah.. I did not know about getusershell(), but its API is ugly... :-/

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash to /etc/shells
  2018-01-14 22:43   ` Yann E. MORIN
@ 2018-01-17 15:17     ` Romain Naour
  0 siblings, 0 replies; 20+ messages in thread
From: Romain Naour @ 2018-01-17 15:17 UTC (permalink / raw)
  To: buildroot

Hi Yann,

Le 14/01/2018 ? 23:43, Yann E. MORIN a ?crit?:
> Romain, All,
> 
> On 2018-01-14 22:03 +0100, Romain Naour spake thusly:
>> When ash (busybox) is selected, /bin/ash is not added to /etc/shells
>> (see man shells). So, login tools like dropbear reject the ssh
>> connexions for users using ash as shell in /etc/passwd.
>>
>> buildroot authpriv.warn dropbear[853]: User 'kubu' has invalid shell, rejected
>>
>> ash is disabled for MMUless systems, so don't add /bin/ash in
>> /etc/shells in this case.
>>
>> Signed-off-by: Romain Naour <romain.naour@smile.fr>
>> ---
>>  package/busybox/busybox.mk | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
>> index d0bbd3f..37d064a 100644
>> --- a/package/busybox/busybox.mk
>> +++ b/package/busybox/busybox.mk
>> @@ -258,6 +258,15 @@ define BUSYBOX_INSTALL_TELNET_SCRIPT
>>  	fi
>>  endef
>>  
>> +# Add /bin/ash to /etc/shells otherwise some login tools like dropbear
>> +# can reject the user connexion. See man shells.
>> +ifeq ($(BR2_USE_MMU),y)
>> +define BUSYBOX_INSTALL_ADD_ASH_TO_SHELLS
>> +	grep -qsE '^/bin/ash' $(TARGET_DIR)/etc/shells \
>> +		|| echo "/bin/ash" >> $(TARGET_DIR)/etc/shells
> 
> What if the user uses a custom config file where ash is not enabled?
> What about the other shells busybox may install?
> 
>     define BUSYBOX_INSTALL_ADD_TO_SHELLS
>         if grep -qsE '^CONFIG_ASH=y$$' $(@D)/.config; then
>             grep -qsE '^/bin/ash' $(TARGET_DIR)/etc/shells \
>             || echo "/bin/ash" >> $(TARGET_DIR)/etc/shells; \
>         fi
>         if grep -qsE '^CONFIG_HUSH=y$$' $(@D)/.config; then
>             grep -qsE '^/bin/hush' $(TARGET_DIR)/etc/shells \
>             || echo "/bin/hush" >> $(TARGET_DIR)/etc/shells; \
>         fi
>         if grep -qsE '^CONFIG_MSH=y$$' $(@D)/.config; then
>             grep -qsE '^/bin/msh' $(TARGET_DIR)/etc/shells \
>             || echo "/bin/msh" >> $(TARGET_DIR)/etc/shells; \
>         fi
>     endef
> 
> (msh is just an alias for hush, but it can be a shell nonethless...)

I didn't know about msh and hush...

Btw msh has been removed from Busybox 1.28.0
https://git.busybox.net/busybox/commit/?id=2e989ef232e35750df573898077dd356003705b2

Best regards,
Romain

> 
> Regards,
> Yann E. MORIN.
> 
>> +endef
>> +endif
>> +
>>  # Enable "noclobber" in install.sh, to prevent BusyBox from overwriting any
>>  # full-blown versions of apps installed by other packages with sym/hard links.
>>  define BUSYBOX_NOCLOBBER_INSTALL
>> @@ -291,6 +300,7 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>>  	$(BUSYBOX_INSTALL_INITTAB)
>>  	$(BUSYBOX_INSTALL_UDHCPC_SCRIPT)
>>  	$(BUSYBOX_INSTALL_MDEV_CONF)
>> +	$(BUSYBOX_INSTALL_ADD_ASH_TO_SHELLS)
>>  endef
>>  
>>  define BUSYBOX_INSTALL_INIT_SYSV
>> -- 
>> 2.7.4
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 

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

end of thread, other threads:[~2018-01-17 15:17 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-14 21:03 [Buildroot] [PATCH 0/7] Add /etc/shells handling Romain Naour
2018-01-14 21:03 ` [Buildroot] [PATCH 1/7] package/skeleton-init-common: add /bin/sh to /etc/shells Romain Naour
2018-01-14 22:51   ` Yann E. MORIN
2018-01-15 14:41   ` Peter Korsgaard
2018-01-15 17:06     ` Yann E. MORIN
2018-01-14 21:03 ` [Buildroot] [PATCH 2/7] package/busybox: add /bin/ash " Romain Naour
2018-01-14 22:43   ` Yann E. MORIN
2018-01-17 15:17     ` Romain Naour
2018-01-14 21:03 ` [Buildroot] [PATCH 3/7] package/dash: add /etc/dash " Romain Naour
2018-01-14 22:26   ` Yann E. MORIN
2018-01-14 21:03 ` [Buildroot] [PATCH 4/7] package/screen: add /usr/bin/screen " Romain Naour
2018-01-14 22:29   ` Yann E. MORIN
2018-01-14 22:36     ` Yann E. MORIN
2018-01-14 21:03 ` [Buildroot] [PATCH 5/7] package/tmux: add /usr/bin/tmux " Romain Naour
2018-01-14 22:30   ` Yann E. MORIN
2018-01-14 21:03 ` [Buildroot] [PATCH 6/7] package/zsh: add /bin/zsh " Romain Naour
2018-01-14 22:34   ` Yann E. MORIN
2018-01-14 21:03 ` [Buildroot] [PATCH 7/7] package/mksh: add /bin/mksh " Romain Naour
2018-01-14 22:37   ` Yann E. MORIN
2018-01-15 10:35     ` Thomas Petazzoni

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.