[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Alexandru Ardelean]

When dropbear changed the way how to configure things, by providing a
`localoptions.h` file, it also defaulted (somehow) to disable the display
of /etc/motd by default.

This can be configured by adding a `#define DO_MOTD 1` in the
`localoptions.h`, which requires it's own `BR2_PACKAGE_DROPBEAR_MOTD`
Kconfig option.

By default, support for displaying `/etc/motd` on login is disabled.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.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 62f77bad9d..936379d10c 100644
--- a/package/dropbear/Config.in
+++ b/package/dropbear/Config.in
@@ -56,6 +56,12 @@ 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_MOTD
+	bool "display motd on login"
+	help
+	  Add support for displaying the contents of /etc/motd
+	  when a user logs into a SSH session.
+
 config BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO
 	bool "enable legacy crypto"
 	help
diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
index e10c851606..d16b578b2d 100644
--- a/package/dropbear/dropbear.mk
+++ b/package/dropbear/dropbear.mk
@@ -71,6 +71,10 @@ define DROPBEAR_ENABLE_REVERSE_DNS
 	echo '#define DO_HOST_LOOKUP 1'                 >> $(@D)/localoptions.h
 endef
 
+define DROPBEAR_ENABLE_MOTD
+	echo '#define DO_MOTD 1'                        >> $(@D)/localoptions.h
+endef
+
 define DROPBEAR_BUILD_FEATURED
 	echo '#define DROPBEAR_SMALL_CODE 0'            >> $(@D)/localoptions.h
 	echo '#define DROPBEAR_TWOFISH128 1'            >> $(@D)/localoptions.h
@@ -124,6 +128,10 @@ ifneq ($(BR2_PACKAGE_DROPBEAR_LASTLOG),y)
 DROPBEAR_CONF_OPTS += --disable-lastlog
 endif
 
+ifeq ($(BR2_PACKAGE_DROPBEAR_MOTD),y)
+DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_ENABLE_MOTD
+endif
+
 define DROPBEAR_INSTALL_TARGET_CMDS
 	$(INSTALL) -m 755 $(@D)/dropbearmulti $(TARGET_DIR)/usr/sbin/dropbear
 	for f in $(DROPBEAR_TARGET_BINS); do \
-- 
2.17.1

[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Thomas Petazzoni]

Hello Alexandru,

On Thu, 23 May 2019 15:11:15 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> When dropbear changed the way how to configure things, by providing a
> `localoptions.h` file, it also defaulted (somehow) to disable the display
> of /etc/motd by default.
> 
> This can be configured by adding a `#define DO_MOTD 1` in the
> `localoptions.h`, which requires it's own `BR2_PACKAGE_DROPBEAR_MOTD`
> Kconfig option.
> 
> By default, support for displaying `/etc/motd` on login is disabled.
> 
> Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>

Thanks for your patch.

> +config BR2_PACKAGE_DROPBEAR_MOTD
> +	bool "display motd on login"
> +	help
> +	  Add support for displaying the contents of /etc/motd
> +	  when a user logs into a SSH session.

In fact, I am a bit worried by all those additional dropbear options to
tweak "minor" details of the dropbear configuration.

Perhaps we should bite the bullet and add an option like this:

config BR2_PACKAGE_DROPBEAR_CONFIG_FILE
	string "path to dropbear config file"
	help
	  Path to a file whose contents will be appended to Dropbear
	  localoptions.h. It can be used to tweak the Dropbear
	  configuration.

And then, in the .mk:

DROPBEAR_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_DROPBEAR_CONFIG_FILE))
ifneq ($(DROPBEAR_CONFIG_FILE),)
define DROPBEAR_APPEND_CONFIG_FILE
	cat $(DROPBEAR_CONFIG_FILE) >> $(@D)/localoptions.h
endef
DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_APPEND_CONFIG_FILE
endif

This way, we have something that is much more flexible, as it allows
you to define whatever Dropbear option you like.

Note: this is just a suggestion; I don't know what other
maintainers/developers will think about it.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Alexandru Ardelean]

On Thu, May 23, 2019 at 3:28 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Alexandru,
>
> On Thu, 23 May 2019 15:11:15 +0300
> Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
>
> > When dropbear changed the way how to configure things, by providing a
> > `localoptions.h` file, it also defaulted (somehow) to disable the display
> > of /etc/motd by default.
> >
> > This can be configured by adding a `#define DO_MOTD 1` in the
> > `localoptions.h`, which requires it's own `BR2_PACKAGE_DROPBEAR_MOTD`
> > Kconfig option.
> >
> > By default, support for displaying `/etc/motd` on login is disabled.
> >
> > Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
>
> Thanks for your patch.
>
> > +config BR2_PACKAGE_DROPBEAR_MOTD
> > +     bool "display motd on login"
> > +     help
> > +       Add support for displaying the contents of /etc/motd
> > +       when a user logs into a SSH session.
>
> In fact, I am a bit worried by all those additional dropbear options to
> tweak "minor" details of the dropbear configuration.
>
> Perhaps we should bite the bullet and add an option like this:
>
> config BR2_PACKAGE_DROPBEAR_CONFIG_FILE
>         string "path to dropbear config file"
>         help
>           Path to a file whose contents will be appended to Dropbear
>           localoptions.h. It can be used to tweak the Dropbear
>           configuration.
>
> And then, in the .mk:
>
> DROPBEAR_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_DROPBEAR_CONFIG_FILE))
> ifneq ($(DROPBEAR_CONFIG_FILE),)
> define DROPBEAR_APPEND_CONFIG_FILE
>         cat $(DROPBEAR_CONFIG_FILE) >> $(@D)/localoptions.h
> endef
> DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_APPEND_CONFIG_FILE
> endif
>

I'd be fine with this approach as well.

> This way, we have something that is much more flexible, as it allows
> you to define whatever Dropbear option you like.
>
> Note: this is just a suggestion; I don't know what other
> maintainers/developers will think about it.
>
> Best regards,
>

Thanks
Alex

> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

Hi,

 > This way, we have something that is much more flexible, as it allows
 > you to define whatever Dropbear option you like.

 > Note: this is just a suggestion; I don't know what other
 > maintainers/developers will think about it.

Agreed, if we want such finegrained configuration then that would be the
way to go.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:
 > Hi,

 >> This way, we have something that is much more flexible, as it allows
 >> you to define whatever Dropbear option you like.

 >> Note: this is just a suggestion; I don't know what other
 >> maintainers/developers will think about it.

 > Agreed, if we want such finegrained configuration then that would be the
 > way to go.

Alexandru, will you send such a patch? In the mean time I have marked
this patch as changes requested in patchwork.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package: dropbear: add support for displaying /etc/motd

[Alexandru Ardelean]

On Fri, May 24, 2019 at 10:50 AM Peter Korsgaard <peter@korsgaard.com> wrote:
>
> >>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:
>
> >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:
>  > Hi,
>
>  >> This way, we have something that is much more flexible, as it allows
>  >> you to define whatever Dropbear option you like.
>
>  >> Note: this is just a suggestion; I don't know what other
>  >> maintainers/developers will think about it.
>
>  > Agreed, if we want such finegrained configuration then that would be the
>  > way to go.
>
> Alexandru, will you send such a patch? In the mean time I have marked
> this patch as changes requested in patchwork.

Yes.

Sorry for the late reply.
I left the thread unread (I do it as personal task-tracker), and I did
not notice this mail.

Thanks
Alex

>
> --
> Bye, Peter Korsgaard

[Buildroot] [PATCH] dropbear: add build opt to provide file with localoptions.h

[Alexandru Ardelean]

From: Alexandru Ardelean <ardeleanalex@gmail.com>

Based on discussion here:
  https://patchwork.ozlabs.org/patch/1104071/
Thomas suggested to implement an option to specify a file that gets
appended to the `localoptions.h` of dropbear, as well as providing the code
for this

This patch implements the changes.
The patch was tested successfully with the DO_MOTD option, which the
initial patch tried to address.

Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
---
Changelog v1 -> v2:
* re-implemented with suggestions from Thomas
* tested patch with `#define DO_MOTD 1` option

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

diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in
index 62f77bad9d..bc8d80ecf4 100644
--- a/package/dropbear/Config.in
+++ b/package/dropbear/Config.in
@@ -66,4 +66,11 @@ config BR2_PACKAGE_DROPBEAR_LEGACY_CRYPTO
 	    DSA public keys
 	    Diffie-Hellman Group1 key exchange
 
+config BR2_PACKAGE_DROPBEAR_CONFIG_FILE
+	string "path to dropbear config file"
+	help
+	  Path to a file whose contents will be appended to Dropbear
+	  localoptions.h. It can be used to tweak the Dropbear
+	  configuration.
+
 endif
diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
index d93faa48ea..86c7769c51 100644
--- a/package/dropbear/dropbear.mk
+++ b/package/dropbear/dropbear.mk
@@ -124,6 +124,14 @@ ifneq ($(BR2_PACKAGE_DROPBEAR_LASTLOG),y)
 DROPBEAR_CONF_OPTS += --disable-lastlog
 endif
 
+DROPBEAR_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_DROPBEAR_CONFIG_FILE))
+ifneq ($(DROPBEAR_CONFIG_FILE),)
+define DROPBEAR_APPEND_CONFIG_FILE
+	cat $(DROPBEAR_CONFIG_FILE) >> $(@D)/localoptions.h
+endef
+DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_APPEND_CONFIG_FILE
+endif
+
 define DROPBEAR_INSTALL_TARGET_CMDS
 	$(INSTALL) -m 755 $(@D)/dropbearmulti $(TARGET_DIR)/usr/sbin/dropbear
 	for f in $(DROPBEAR_TARGET_BINS); do \
-- 
2.20.1

[Buildroot] [PATCH] dropbear: add build opt to provide file with localoptions.h

[Thomas Petazzoni]

Hello Alexandru,

Thanks for this new iteration!

On Wed, 29 May 2019 13:27:06 +0300
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:

> From: Alexandru Ardelean <ardeleanalex@gmail.com>
> 
> Based on discussion here:
>   https://patchwork.ozlabs.org/patch/1104071/
> Thomas suggested to implement an option to specify a file that gets
> appended to the `localoptions.h` of dropbear, as well as providing the code
> for this
> 
> This patch implements the changes.
> The patch was tested successfully with the DO_MOTD option, which the
> initial patch tried to address.
> 
> Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
> ---
> Changelog v1 -> v2:
> * re-implemented with suggestions from Thomas
> * tested patch with `#define DO_MOTD 1` option

I've applied to next, after renaming the option. To me
BR2_PACKAGE_DROPBEAR_CONFIG_FILE sounded like the configuration file
you would have in /etc/dropbear/ for example. I also reworked teh
commit log.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com