[Buildroot] [PATCH v2] package: dropbear: make PATH configurable

[Buildroot] [PATCH v2] package: dropbear: make PATH configurable

[Markus Mayer]

Make the default PATH dropbear is using configurable.

If not specified, it will continue to default to dropbear's
DEFAULT_PATH.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---

Changes since v1:
- Made regex safer
  + allow multiple spaces after #define
  + make sure there are spaces or tabs after DEFAULT_PATH

 package/dropbear/Config.in   | 6 ++++++
 package/dropbear/dropbear.mk | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in
index 6700778161ba..0cc68d737d2c 100644
--- a/package/dropbear/Config.in
+++ b/package/dropbear/Config.in
@@ -55,4 +55,10 @@ config BR2_PACKAGE_DROPBEAR_LASTLOG
 	  Enable logging of dropbear access to lastlog. Notice that
 	  Buildroot does not generate lastlog by default.
 
+config BR2_PACKAGE_DROPBEAR_PATH
+	string "dropbear default path"
+	help
+	  Use the path specified here as dropbear's default path.
+	  If not specified, dropbear will use "/usr/bin:/bin".
+
 endif
diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
index 01a1a07b7696..9da340670a70 100644
--- a/package/dropbear/dropbear.mk
+++ b/package/dropbear/dropbear.mk
@@ -51,6 +51,14 @@ define DROPBEAR_DISABLE_STANDALONE
 	$(SED) 's:\(#define NON_INETD_MODE\):/*\1 */:' $(@D)/options.h
 endef
 
+ifneq ($(BR2_PACKAGE_DROPBEAR_PATH),"")
+define DROPBEAR_CUSTOM_PATH
+	$(SED) 's|^\(#define \+DEFAULT_PATH\)[	 ]\+.*|\1 $(BR2_PACKAGE_DROPBEAR_PATH)|' $(@D)/options.h
+endef
+
+DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_CUSTOM_PATH
+endif
+
 define DROPBEAR_INSTALL_INIT_SYSTEMD
 	$(INSTALL) -D -m 644 package/dropbear/dropbear.service \
 		$(TARGET_DIR)/usr/lib/systemd/system/dropbear.service
-- 
2.7.4

[Buildroot] [PATCH] package: dropbear: make PATH configurable

[Carlos Santos]

Markus Mayer wrote:

> Make the default PATH dropbear is using configurable.
> 
> If not specified, it will continue to default to dropbear's
> DEFAULT_PATH.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  package/dropbear/Config.in   | 6 ++++++
>  package/dropbear/dropbear.mk | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in
> index 6700778161..0cc68d737d 100644
> --- a/package/dropbear/Config.in
> +++ b/package/dropbear/Config.in
> @@ -55,4 +55,10 @@ config BR2_PACKAGE_DROPBEAR_LASTLOG
>  	  Enable logging of dropbear access to lastlog. Notice that
>  	  Buildroot does not generate lastlog by default.
>  
> +config BR2_PACKAGE_DROPBEAR_PATH
> +	string "dropbear default path"
> +	help
> +	  Use the path specified here as dropbear's default path.
> +	  If not specified, dropbear will use "/usr/bin:/bin".
> +
>  endif

Looks nice but it would be good to add a similar configuration for
openssl, whose default user PATH is "/usr/bin:/bin:/usr/sbin:/sbin".
In fact I believe we should make both ssh servers use the same user
PATH.

> diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
> index 01a1a07b76..9da340670a 100644
> --- a/package/dropbear/dropbear.mk
> +++ b/package/dropbear/dropbear.mk
> @@ -51,6 +51,14 @@ define DROPBEAR_DISABLE_STANDALONE
>  	$(SED) 's:\(#define NON_INETD_MODE\):/*\1 */:' $(@D)/options.h
>  endef
>  
> +ifneq ($(BR2_PACKAGE_DROPBEAR_PATH),"")
> +define DROPBEAR_CUSTOM_PATH
> +	$(SED) 's|^\(#define \+DEFAULT_PATH\)[	 ]\+.*|\1 $(BR2_PACKAGE_DROPBEAR_PATH)|' $(@D)/options.h
> +endef
> +
> +DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_CUSTOM_PATH
> +endif
> +
>  define DROPBEAR_INSTALL_INIT_SYSTEMD
>  	$(INSTALL) -D -m 644 package/dropbear/dropbear.service \
>  		$(TARGET_DIR)/usr/lib/systemd/system/dropbear.service

This must be updated to put customised options in localoptions.h (refer
to commit 2e035a9aecc37b87a277fd53f84743a18a6f03a7).

--
Carlos Santos
DATACOM P&D

[Buildroot] [PATCH v2] package: dropbear: make PATH configurable

[Yann E. MORIN]

Markus, All,

On 2018-03-09 14:41 -0800, Markus Mayer spake thusly:
> Make the default PATH dropbear is using configurable.

We've discussed this with Thomas, and as Carlos noticed, we woudl need a
similar solution for openssh.

But pushing the thing even further, we think a generic solution is even
better:

  - in the "System configuration" sub-menu, add a new option that
    basically is "Default PATH" and defaults to /bin:/sbin:/usr/bin:/usr/sbin

  - use that to set PATH in /etc/profile (from the skeleton-init-common
    package)

  - use that for dropbear

  - use that for openssh

Care to have a look?

Regards,
Yann E. MORIN.

> If not specified, it will continue to default to dropbear's
> DEFAULT_PATH.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
> 
> Changes since v1:
> - Made regex safer
>   + allow multiple spaces after #define
>   + make sure there are spaces or tabs after DEFAULT_PATH
> 
>  package/dropbear/Config.in   | 6 ++++++
>  package/dropbear/dropbear.mk | 8 ++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in
> index 6700778161ba..0cc68d737d2c 100644
> --- a/package/dropbear/Config.in
> +++ b/package/dropbear/Config.in
> @@ -55,4 +55,10 @@ config BR2_PACKAGE_DROPBEAR_LASTLOG
>  	  Enable logging of dropbear access to lastlog. Notice that
>  	  Buildroot does not generate lastlog by default.
>  
> +config BR2_PACKAGE_DROPBEAR_PATH
> +	string "dropbear default path"
> +	help
> +	  Use the path specified here as dropbear's default path.
> +	  If not specified, dropbear will use "/usr/bin:/bin".
> +
>  endif
> diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
> index 01a1a07b7696..9da340670a70 100644
> --- a/package/dropbear/dropbear.mk
> +++ b/package/dropbear/dropbear.mk
> @@ -51,6 +51,14 @@ define DROPBEAR_DISABLE_STANDALONE
>  	$(SED) 's:\(#define NON_INETD_MODE\):/*\1 */:' $(@D)/options.h
>  endef
>  
> +ifneq ($(BR2_PACKAGE_DROPBEAR_PATH),"")
> +define DROPBEAR_CUSTOM_PATH
> +	$(SED) 's|^\(#define \+DEFAULT_PATH\)[	 ]\+.*|\1 $(BR2_PACKAGE_DROPBEAR_PATH)|' $(@D)/options.h
> +endef
> +
> +DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_CUSTOM_PATH
> +endif
> +
>  define DROPBEAR_INSTALL_INIT_SYSTEMD
>  	$(INSTALL) -D -m 644 package/dropbear/dropbear.service \
>  		$(TARGET_DIR)/usr/lib/systemd/system/dropbear.service
> -- 
> 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] [PATCH v2] package: dropbear: make PATH configurable

[Yann E. MORIN]

Markus, All,

On 2018-12-16 09:04 -0800, Markus Mayer spake thusly:
> On Sun, 16 Dec 2018 at 07:26, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2018-03-09 14:41 -0800, Markus Mayer spake thusly:
> > > Make the default PATH dropbear is using configurable.
> > But pushing the thing even further, we think a generic solution is even
> > better:
> >   - in the "System configuration" sub-menu, add a new option that
> >     basically is "Default PATH" and defaults to /bin:/sbin:/usr/bin:/usr/sbin
> 
> Would it make sense to use the order /bin:/usr/bin:/sbin:/usr/sbin?
> Conceptually, that would make it pick up non-admin and non-system
> tools first. In reality, it likely wouldn't make a difference, because
> there won't be programs of the same name in a "bin" path and in an
> "sbin" path. But if there are, for whatever reason, it's nicer if it
> picks the one that will do "less damage" by default, unless explicitly
> told otherwise.
> Anyway, it's just a thought.

Currently, system/skeleton/etc/profile contains /bin:/sbin:/usr/bin:/usr/sbin
so I'd suggest we keep that as the default for this new option.

Whether that value makes sense or not is a different topic. I had a look
at POSIX, and they define a default value, nor suggest one. On my
machine, /etc/environment exports a PATH with:
    /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games
while /etc/login.defs sets PATH to:
    /usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games

So, I don;t really care, but if it is changed, it should be in its own
patch.

> > Care to have a look?
> I like it. :-)

Cool thanks! Don't forget to Cc me on the resulting series, so I'm sure
not to miss it. (which does not mean you'll get a fast review, mind
you! :-/)

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

[Buildroot] [PATCH v2] package: dropbear: make PATH configurable

[Yann E. MORIN]

Markus, All,

On 2018-12-17 10:51 -0800, Markus Mayer spake thusly:
> On Sun, 16 Dec 2018 at 10:47, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > > Would it make sense to use the order /bin:/usr/bin:/sbin:/usr/sbin?
> > > Conceptually, that would make it pick up non-admin and non-system
> > > tools first. In reality, it likely wouldn't make a difference, because
> > > there won't be programs of the same name in a "bin" path and in an
> > > "sbin" path. But if there are, for whatever reason, it's nicer if it
> > > picks the one that will do "less damage" by default, unless explicitly
> > > told otherwise.
> > > Anyway, it's just a thought.
> >
> > Currently, system/skeleton/etc/profile contains /bin:/sbin:/usr/bin:/usr/sbin
> > so I'd suggest we keep that as the default for this new option.
> 
> Okay, I'll leave it as is.
> 
> > Cool thanks! Don't forget to Cc me on the resulting series, so I'm sure
> > not to miss it. (which does not mean you'll get a fast review, mind
> > you! :-/)
> 
> Looks like the main challenge will be to change etc/profile. Right
> now, it is being copied (rsynched) as is. That won't be possible
> anymore. It'll have to replace a place holder with the actual PATH as
> set in the new config option.
> 
> $ head system/skeleton/etc/profile
> export PATH=/bin:/sbin:/usr/bin:/usr/sbin
> 
> if [ "$PS1" ]; then
>     if [ "`id -u`" -eq 0 ]; then
>         export PS1='# '
>     else
>         export PS1='$ '
>     fi
> fi
> 
> Do you know of any examples in the Buildroot code where the "build"
> process has to modify a script and insert the value of a Buildroot
> config option? If this is already being done elsewhere, I'd like to
> use the same approach here if possible.

Use $(SED) to sed the file after it has been installed. See for example
pppd:
    https://git.buildroot.org/buildroot/tree/package/pppd/pppd.mk#n74

So, in SKELETON_INIT_COMMON_INSTALL_TARGET_CMDS, add a line something
like:
    $(SED) 's at PATH=.*$@PATH=$(BR2_SYSTEM_PATH)@' \
        $(TARGET_DIR)/etc/profile

And note in the commit log that BR2_SYSTEM_PATH is a kconfig string, so
it is already quoted, so we end up with a properly-quoted assignment.

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

[Buildroot] [PATCH v2] package: dropbear: make PATH configurable

[Carlos Santos]

> From: "Yann Morin" <yann.morin.1998@free.fr>
> To: "Markus Mayer" <mmayer@broadcom.com>
> Cc: "buildroot" <buildroot@buildroot.org>
> Sent: Domingo, 16 de dezembro de 2018 13:26:03
> Subject: Re: [Buildroot] [PATCH v2] package: dropbear: make PATH configurable

> Markus, All,
> 
> On 2018-03-09 14:41 -0800, Markus Mayer spake thusly:
>> Make the default PATH dropbear is using configurable.
> 
> We've discussed this with Thomas, and as Carlos noticed, we woudl need a
> similar solution for openssh.
> 
> But pushing the thing even further, we think a generic solution is even
> better:
> 
>  - in the "System configuration" sub-menu, add a new option that
>    basically is "Default PATH" and defaults to /bin:/sbin:/usr/bin:/usr/sbin

Notice that /bin and /sbin are redundant with their /usr counterparts for
BR2_ROOTFS_MERGED_USR=y.

>  - use that to set PATH in /etc/profile (from the skeleton-init-common
>    package)

In one of my current projects there is a /etc/profile.d/path.sh file in a
rootfs overlay containing

   #!/bin/sh
   PATH="$(test -L /bin || echo /bin:)$(test -L /sbin || echo /sbin:)/usr/bin:/usr/sbin"

-- 
Carlos Santos (Casantos) - DATACOM, P&D

[Buildroot] [PATCH v2] package: dropbear: make PATH configurable

[Yann E. MORIN]

Carlos, All,

On 2018-12-17 21:44 -0200, Carlos Santos spake thusly:
> Notice that /bin and /sbin are redundant with their /usr counterparts for
> BR2_ROOTFS_MERGED_USR=y.
[--SNIP--]
> In one of my current projects there is a /etc/profile.d/path.sh file in a
> rootfs overlay containing
>    #!/bin/sh
>    PATH="$(test -L /bin || echo /bin:)$(test -L /sbin || echo /sbin:)/usr/bin:/usr/sbin"

On the other hand, and as I replied to Markus in his new iteration, is
the overhead to search in PATH so huge it needs this micro-optimisation?

If the commands are in /usr/bin, does it matter they are primarily found
in /bin instead? They are the same commands to begin with, so...

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