| Message ID | 20180309224149.20225-1-mmayer@broadcom.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] package: dropbear: make PATH configurable | expand |
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
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@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
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.
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@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.
> 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, 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.
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
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(+)