diff mbox

[RFC,2/8] package/dropbear: Add separate configuration options for client and server

Message ID 1389862338.918265.958087681541.2.gpush@pablo
State Superseded
Headers show

Commit Message

Jeremy Kerr Jan. 16, 2014, 8:52 a.m. UTC
Currently, the dropbear package installs both client and server
components. For example, this means that when we only want the
client binaries, we also get the server run from init.

Even though it's a multi-call binary (the client and server
exist in the same executable), we'd like to selectively install the
links and init scripts. This change introduces separate configuration
options (both 'default y') for the client and server bits.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
 package/dropbear/Config.in   |   12 ++++++++++++
 package/dropbear/dropbear.mk |    9 ++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN March 3, 2014, 11:46 p.m. UTC | #1
Jérémy, All,

On 2014-01-16 16:52 +0800, Jeremy Kerr spake thusly:
[--SNIP--]
> index 68c3b71a..4a7327b7 100644
> --- a/package/dropbear/Config.in
> +++ b/package/dropbear/Config.in
> @@ -8,6 +8,18 @@ config BR2_PACKAGE_DROPBEAR
>  
>  if BR2_PACKAGE_DROPBEAR
>  
> +config BR2_PACKAGE_DROPBEAR_SERVER
> +	bool "dropbear ssh server"
> +	default y
> +	help
> +	  Enable the dropbear ssh server, run from init
> +
> +config BR2_PACKAGE_DROPBEAR_CLIENT
> +	bool "dropbear ssh client"
> +	default y
> +	help
> +	  Enable the dropbear ssh client, scp and utilities

What if none is selected? What about:

    config BR2_PACKAGE_DROPBEAR_HAS_CLIENT
        def_bool y
        depends on !BR2_PACKAGE_DROPBEAR_CLIENT
        select BR2_PACKAGE_DROPBEAR_SERVER

    config BR2_PACKAGE_DROPBEAR_SERVER
        bool "dropbear ssh server"

    config BR2_PACKAGE_DROPBEAR_CLIENT
        bool "dropbear ssh client"

(with your help entries, of course).

> diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
> index 3ef6c8da..a1cc5691 100644
> --- a/package/dropbear/dropbear.mk
> +++ b/package/dropbear/dropbear.mk
> @@ -7,7 +7,7 @@
>  DROPBEAR_VERSION = 2013.62
>  DROPBEAR_SITE = http://matt.ucc.asn.au/dropbear/releases
>  DROPBEAR_SOURCE = dropbear-$(DROPBEAR_VERSION).tar.bz2
> -DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
> +DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert
>  DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
>  		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
		                   ^^^^^^^^
Should we not disable dbclient if it is not selected?

Regards,
Yann E. MORIN.
Jeremy Kerr March 3, 2014, 11:54 p.m. UTC | #2
Hi Yann,

> On 2014-01-16 16:52 +0800, Jeremy Kerr spake thusly:
> [--SNIP--]
>> index 68c3b71a..4a7327b7 100644
>> --- a/package/dropbear/Config.in
>> +++ b/package/dropbear/Config.in
>> @@ -8,6 +8,18 @@ config BR2_PACKAGE_DROPBEAR
>>  
>>  if BR2_PACKAGE_DROPBEAR
>>  
>> +config BR2_PACKAGE_DROPBEAR_SERVER
>> +	bool "dropbear ssh server"
>> +	default y
>> +	help
>> +	  Enable the dropbear ssh server, run from init
>> +
>> +config BR2_PACKAGE_DROPBEAR_CLIENT
>> +	bool "dropbear ssh client"
>> +	default y
>> +	help
>> +	  Enable the dropbear ssh client, scp and utilities
> 
> What if none is selected?

Then neither the client nor server is installed; is that an issue?

We could just have a comment (when !_CLIENT && !_SERVER) to notify the
user that it's not a sensible configuration...

>> diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
>> index 3ef6c8da..a1cc5691 100644
>> --- a/package/dropbear/dropbear.mk
>> +++ b/package/dropbear/dropbear.mk
>> @@ -7,7 +7,7 @@
>>  DROPBEAR_VERSION = 2013.62
>>  DROPBEAR_SITE = http://matt.ucc.asn.au/dropbear/releases
>>  DROPBEAR_SOURCE = dropbear-$(DROPBEAR_VERSION).tar.bz2
>> -DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
>> +DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert
>>  DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
>>  		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
> 		                   ^^^^^^^^
> Should we not disable dbclient if it is not selected?

We should. I'll sort this out in my next series.

Thanks for the review!


Jeremy
Gustavo Zacarias March 4, 2014, 12:14 a.m. UTC | #3
On 01/16/2014 05:52 AM, Jeremy Kerr wrote:

> Currently, the dropbear package installs both client and server
> components. For example, this means that when we only want the
> client binaries, we also get the server run from init.
> 
> Even though it's a multi-call binary (the client and server
> exist in the same executable), we'd like to selectively install the
> links and init scripts. This change introduces separate configuration
> options (both 'default y') for the client and server bits.

Normally we don't make initscript installation an option and since the
dropbear package options are essentially a no-op what's the point?
(if you don't want sshd to startup on boot you can use a post build
script to just remove the initscript from the target, that's the
recommended way).
Regards.
Jeremy Kerr March 4, 2014, 1:07 a.m. UTC | #4
Hi Gustavo,

> Normally we don't make initscript installation an option

I think dropbear is a bit exceptional here, as both server and client
are in the one package.

> and since the
> dropbear package options are essentially a no-op what's the point?

There are some scenarios where there's no point starting a SSH server
(in my case, it's because there's no persistent storage, so no
persistent host keys). However, I'd still like a ssh/scp client to be
available.

> (if you don't want sshd to startup on boot you can use a post build
> script to just remove the initscript from the target, that's the
> recommended way).

But isn't something that will go upstream and be available in a
defconfig, right?

Regards,


Jeremy
Gustavo Zacarias March 4, 2014, 1:44 a.m. UTC | #5
On 03/03/2014 10:07 PM, Jeremy Kerr wrote:

> Hi Gustavo,

Hi Jeremy.

>> Normally we don't make initscript installation an option
> 
> I think dropbear is a bit exceptional here, as both server and client
> are in the one package.
> 
>> and since the
>> dropbear package options are essentially a no-op what's the point?
> 
> There are some scenarios where there's no point starting a SSH server
> (in my case, it's because there's no persistent storage, so no
> persistent host keys). However, I'd still like a ssh/scp client to be
> available.

Well you'll want at least one of the options of dropbear, there's no
point in excluding both.
My common sense tells me you'll always want the client anyway, it's no
security threat or space-consuming menace, so why not add just a SERVER
option maybe?
BR2_PACKAGE_DROPBEAR -> builds and install the client portion.
BR2_PACKAGE_DROPBEAR_SERVER -> installs the server bits (default=y for
compatibility reasons).
Let's see others chiming in though...

>> (if you don't want sshd to startup on boot you can use a post build
>> script to just remove the initscript from the target, that's the
>> recommended way).
> 
> But isn't something that will go upstream and be available in a
> defconfig, right?

That's still open to debate, until now all defconfigs were supposed to
be minimal, but there's a growing consensus that featured or demo
defconfigs are desired and useful.
It's just a matter of doing it right and documenting it at this point i
think.
Regards.
Jeremy Kerr March 4, 2014, 1:51 a.m. UTC | #6
Hi Gustavo,

>> There are some scenarios where there's no point starting a SSH server
>> (in my case, it's because there's no persistent storage, so no
>> persistent host keys). However, I'd still like a ssh/scp client to be
>> available.
> 
> Well you'll want at least one of the options of dropbear, there's no
> point in excluding both.
> My common sense tells me you'll always want the client anyway, it's no
> security threat or space-consuming menace, so why not add just a SERVER
> option maybe?
> BR2_PACKAGE_DROPBEAR -> builds and install the client portion.
> BR2_PACKAGE_DROPBEAR_SERVER -> installs the server bits (default=y for
> compatibility reasons).

That sounds pretty good to me too. I'd be happy to implement whatever
the general consensus is :)

>> But isn't something that will go upstream and be available in a
>> defconfig, right?
> 
> That's still open to debate, until now all defconfigs were supposed to
> be minimal, but there's a growing consensus that featured or demo
> defconfigs are desired and useful.

OK, understood.

If the defconfig isn't upstream, I'd need to provide both a config and a
postinst script, and whoever is replicating the build needs to point the
config to the postinst script correctly. Having the dropbear server
available as a simple config option allows me to either use a defconfig,
or provide a single defconfig file, to allow others to build a
petitboot-compatible builtroot image, without having to mess around too
much.

Cheers,


Jeremy
Yann E. MORIN March 4, 2014, 6:36 p.m. UTC | #7
Jeremy, All,

On 2014-03-04 09:51 +0800, Jeremy Kerr spake thusly:
> >> There are some scenarios where there's no point starting a SSH server
> >> (in my case, it's because there's no persistent storage, so no
> >> persistent host keys). However, I'd still like a ssh/scp client to be
> >> available.
> > 
> > Well you'll want at least one of the options of dropbear, there's no
> > point in excluding both.
> > My common sense tells me you'll always want the client anyway, it's no
> > security threat or space-consuming menace, so why not add just a SERVER
> > option maybe?
> > BR2_PACKAGE_DROPBEAR -> builds and install the client portion.
> > BR2_PACKAGE_DROPBEAR_SERVER -> installs the server bits (default=y for
> > compatibility reasons).
> 
> That sounds pretty good to me too. I'd be happy to implement whatever
> the general consensus is :)

OK for me. But make sure the server is "default y", so old .config files
still work.

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in
index 68c3b71a..4a7327b7 100644
--- a/package/dropbear/Config.in
+++ b/package/dropbear/Config.in
@@ -8,6 +8,18 @@  config BR2_PACKAGE_DROPBEAR
 
 if BR2_PACKAGE_DROPBEAR
 
+config BR2_PACKAGE_DROPBEAR_SERVER
+	bool "dropbear ssh server"
+	default y
+	help
+	  Enable the dropbear ssh server, run from init
+
+config BR2_PACKAGE_DROPBEAR_CLIENT
+	bool "dropbear ssh client"
+	default y
+	help
+	  Enable the dropbear ssh client, scp and utilities
+
 config BR2_PACKAGE_DROPBEAR_DISABLE_REVERSEDNS
 	bool "disable reverse DNS lookups"
 	help
diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
index 3ef6c8da..a1cc5691 100644
--- a/package/dropbear/dropbear.mk
+++ b/package/dropbear/dropbear.mk
@@ -7,7 +7,7 @@ 
 DROPBEAR_VERSION = 2013.62
 DROPBEAR_SITE = http://matt.ucc.asn.au/dropbear/releases
 DROPBEAR_SOURCE = dropbear-$(DROPBEAR_VERSION).tar.bz2
-DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
+DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert
 DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
 		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
 
@@ -43,6 +43,7 @@  define DROPBEAR_DISABLE_STANDALONE
 	$(SED) 's:\(#define NON_INETD_MODE\):/*\1 */:' $(@D)/options.h
 endef
 
+ifeq ($(BR2_PACKAGE_DROPBEAR_SERVER),y)
 define DROPBEAR_INSTALL_INIT_SYSTEMD
 	$(INSTALL) -D -m 644 package/dropbear/dropbear.service \
 		$(TARGET_DIR)/etc/systemd/system/dropbear.service
@@ -60,6 +61,12 @@  else
 DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_DISABLE_STANDALONE
 endif
 
+endif
+
+ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
+DROPBEAR_TARGET_BINS += dbclient scp ssh
+endif
+
 ifeq ($(BR2_PACKAGE_DROPBEAR_DISABLE_REVERSEDNS),)
 DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_ENABLE_REVERSE_DNS
 endif