diff mbox

[1/1] dropbear: add extra build customization options

Message ID 1410450212-30718-1-git-send-email-bos@je-eigen-domein.nl
State Changes Requested
Headers show

Commit Message

Floris Bos Sept. 11, 2014, 3:43 p.m. UTC
Adds:

- Option to build client (defaults y, for compatibility)
- Option to disable password authentication,
  to only allow public key authentication instead
- Option to disable TCP forwarding.
  Defaults to y, as most legitimate users are not using it,
  and the feature is very popular with spammers that scan
  for devices with weak passwords and use them to relay spam.

Signed-off-by: Floris Bos <bos@je-eigen-domein.nl>
---
 package/dropbear/Config.in   | 21 +++++++++++++++++++++
 package/dropbear/dropbear.mk | 31 ++++++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 3 deletions(-)

Comments

Thomas Petazzoni Feb. 3, 2015, 2:04 p.m. UTC | #1
Dear Floris Bos,

On Thu, 11 Sep 2014 17:43:31 +0200, Floris Bos wrote:

> - Option to disable password authentication,
>   to only allow public key authentication instead

This can be done at runtime using the -s option, and presumably
disabling it at build time doesn't give much space savings, so we'd
rather not have a Config.in option for this.

> - Option to disable TCP forwarding.
>   Defaults to y, as most legitimate users are not using it,
>   and the feature is very popular with spammers that scan
>   for devices with weak passwords and use them to relay spam.

This can be done at runtime using the -j and -k options, so same logic
as for the password authentication disabling.

We'd however be open to merge the option to install or not the clients,
but we do have some comments/questions below.

> +ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
>  DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
>  DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
>  		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
> -
> -DROPBEAR_LICENSE = MIT, BSD-2c-like, BSD-2c
> -DROPBEAR_LICENSE_FILES = LICENSE
> +else
> +DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp
> +DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
> +		PROGRAMS="dropbear dropbearkey dropbearconvert scp"
> +endif

Why is scp part of the server-only installation?

Also, can you make this a bit smarter to avoid duplication. For example:

DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp

ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
DROPBEAR_TARGET_BINS += ssh dbclient
endif

And then use:

	PROGRAMS="dropbear $(DROPBEAR_TARGET_BINS)"

When doing the $(MAKE) call.

We'll mark your patch as 'Changes Requested' in patchwork, so can you
resend an updated version that takes into account those comments?

Thanks!

Thomas
Floris Bos Feb. 3, 2015, 5:53 p.m. UTC | #2
Hi,

On 02/03/2015 03:04 PM, Thomas Petazzoni wrote:
> On Thu, 11 Sep 2014 17:43:31 +0200, Floris Bos wrote:
>
>> - Option to disable password authentication,
>>    to only allow public key authentication instead
> This can be done at runtime using the -s option, and presumably
> disabling it at build time doesn't give much space savings, so we'd
> rather not have a Config.in option for this.
>
>> - Option to disable TCP forwarding.
>>    Defaults to y, as most legitimate users are not using it,
>>    and the feature is very popular with spammers that scan
>>    for devices with weak passwords and use them to relay spam.
> This can be done at runtime using the -j and -k options, so same logic
> as for the password authentication disabling.

Fair enough


>
> We'd however be open to merge the option to install or not the clients,
> but we do have some comments/questions below.
>
>> +ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
>>   DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
>>   DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
>>   		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
>> -
>> -DROPBEAR_LICENSE = MIT, BSD-2c-like, BSD-2c
>> -DROPBEAR_LICENSE_FILES = LICENSE
>> +else
>> +DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp
>> +DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
>> +		PROGRAMS="dropbear dropbearkey dropbearconvert scp"
>> +endif
> Why is scp part of the server-only installation?

Because scp is both a client and server program, similar to other 
programs that can tunnel data over SSH like rsync.
When using scp on the client, it simply calls the ssh client program to 
connect to the SSH server and executes scp server-side there with a flag 
to tell it to play server and read further instructions/data from stdin, 
send data to stdout.

Do could make building scp a seperate Config.in option.


Yours sincerely,

Floris Bos
Thomas Petazzoni Feb. 3, 2015, 7:30 p.m. UTC | #3
Dear Floris Bos,

On Tue, 03 Feb 2015 18:53:42 +0100, Floris Bos wrote:

> > We'd however be open to merge the option to install or not the clients,
> > but we do have some comments/questions below.
> >
> >> +ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
> >>   DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
> >>   DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
> >>   		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
> >> -
> >> -DROPBEAR_LICENSE = MIT, BSD-2c-like, BSD-2c
> >> -DROPBEAR_LICENSE_FILES = LICENSE
> >> +else
> >> +DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp
> >> +DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
> >> +		PROGRAMS="dropbear dropbearkey dropbearconvert scp"
> >> +endif
> > Why is scp part of the server-only installation?
> 
> Because scp is both a client and server program, similar to other 
> programs that can tunnel data over SSH like rsync.
> When using scp on the client, it simply calls the ssh client program to 
> connect to the SSH server and executes scp server-side there with a flag 
> to tell it to play server and read further instructions/data from stdin, 
> send data to stdout.

Ah, ok. Can you indicate that in the commit log, or maybe better, as a
comment in the code above the place where we defined what gets built
for the server-only case vs. server+client case?

> Do could make building scp a seperate Config.in option.

I don't think that's needed.

So, can you cook an updated patch?

Thanks a lot!

Thomas
Floris Bos Feb. 4, 2015, 3:35 p.m. UTC | #4
On 02/03/2015 03:04 PM, Thomas Petazzoni wrote:
> Also, can you make this a bit smarter to avoid duplication. For example:
>
> DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp
>
> ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
> DROPBEAR_TARGET_BINS += ssh dbclient
> endif
>
> And then use:
>
> 	PROGRAMS="dropbear $(DROPBEAR_TARGET_BINS)"
>
> When doing the $(MAKE) call.

Just remembered what was up with that.

Dropbear's SSH client is called dbclient.
The extra "ssh" convenience symlink is something non-standard the 
buildroot package made up.
One cannot pass a $(DROPBEAR_TARGET_BINS) containing "ssh" to the 
$(MAKE) call, as upstream dropbear has no clue how to make "ssh"


Yours sincerely,

Floris Bos
Thomas Petazzoni Feb. 4, 2015, 3:40 p.m. UTC | #5
Dear Floris Bos,

On Wed, 04 Feb 2015 16:35:26 +0100, Floris Bos wrote:

> Dropbear's SSH client is called dbclient.
> The extra "ssh" convenience symlink is something non-standard the 
> buildroot package made up.
> One cannot pass a $(DROPBEAR_TARGET_BINS) containing "ssh" to the 
> $(MAKE) call, as upstream dropbear has no clue how to make "ssh"

Ok, but then you can still have two separate variables for the
TARGET_BINS and the PROGRAMS, but not have duplication between the
server+client, and server only cases.

Or even:

DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp

ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
DROPBEAR_TARGET_BINS += dbclient ssh
endif

...
	# Here some comment to explain why we filter out ssh
	$(MAKE) MULTI=1 SCPPROGRESS=1 \
		PROGRAMS=$(filter-out ssh,$(DROPBEAR_TARGET_BINS))
...

Best regards,

Thomas
diff mbox

Patch

diff --git a/package/dropbear/Config.in b/package/dropbear/Config.in
index 68c3b71..dad2ab3 100644
--- a/package/dropbear/Config.in
+++ b/package/dropbear/Config.in
@@ -8,6 +8,12 @@  config BR2_PACKAGE_DROPBEAR
 
 if BR2_PACKAGE_DROPBEAR
 
+config BR2_PACKAGE_DROPBEAR_CLIENT
+	bool "client programs"
+	default y
+	help
+	  Provides dbclient, ssh
+
 config BR2_PACKAGE_DROPBEAR_DISABLE_REVERSEDNS
 	bool "disable reverse DNS lookups"
 	help
@@ -15,6 +21,21 @@  config BR2_PACKAGE_DROPBEAR_DISABLE_REVERSEDNS
 	  on systems without working DNS, as connections otherwise
 	  stall until DNS times out.
 
+config BR2_PACKAGE_DROPBEAR_DISABLE_PASSWORD_AUTH
+	bool "disable password authentication"
+	help
+	  Disable password authentication. Typically used when security
+	  requirements demand that only public key authentication is allowed.
+
+config BR2_PACKAGE_DROPBEAR_DISABLE_TCP_FORWARDING
+	bool "disable TCP forwarding"
+	default y
+	help
+	  Disable TCP forwarding. SSH allows tunneling TCP connections,
+	  if you do not need that, it is better to disable it.
+	  Spammers are known to scan for accounts with weak passwords
+	  and abuse this functionality as easy cross-platform way to relay spam.
+
 config BR2_PACKAGE_DROPBEAR_SMALL
 	bool "optimize for size"
 	default y
diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk
index e8fcdf6..4d1492c 100644
--- a/package/dropbear/dropbear.mk
+++ b/package/dropbear/dropbear.mk
@@ -7,12 +7,18 @@ 
 DROPBEAR_VERSION = 2014.65
 DROPBEAR_SITE = http://matt.ucc.asn.au/dropbear/releases
 DROPBEAR_SOURCE = dropbear-$(DROPBEAR_VERSION).tar.bz2
+DROPBEAR_LICENSE = MIT, BSD-2c-like, BSD-2c
+DROPBEAR_LICENSE_FILES = LICENSE
+
+ifeq ($(BR2_PACKAGE_DROPBEAR_CLIENT),y)
 DROPBEAR_TARGET_BINS = dbclient dropbearkey dropbearconvert scp ssh
 DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
 		PROGRAMS="dropbear dbclient dropbearkey dropbearconvert scp"
-
-DROPBEAR_LICENSE = MIT, BSD-2c-like, BSD-2c
-DROPBEAR_LICENSE_FILES = LICENSE
+else
+DROPBEAR_TARGET_BINS = dropbearkey dropbearconvert scp
+DROPBEAR_MAKE =	$(MAKE) MULTI=1 SCPPROGRESS=1 \
+		PROGRAMS="dropbear dropbearkey dropbearconvert scp"
+endif
 
 ifeq ($(BR2_PREFER_STATIC_LIB),y)
 DROPBEAR_MAKE += STATIC=1
@@ -28,6 +34,17 @@  define DROPBEAR_ENABLE_REVERSE_DNS
 	$(SED) 's:.*\(#define DO_HOST_LOOKUP\).*:\1:' $(@D)/options.h
 endef
 
+define DROPBEAR_DISABLE_PASSWORD_AUTH
+	$(SED) 's:\(#define ENABLE_SVR_PASSWORD_AUTH\).*:/*\1 */:' $(@D)/options.h
+endef
+
+define DROPBEAR_DISABLE_TCP_FORWARDING
+	$(SED) 's:\(#define ENABLE_CLI_LOCALTCPFWD\).*:/*\1 */:' $(@D)/options.h
+	$(SED) 's:\(#define ENABLE_CLI_REMOTETCPFWD\).*:/*\1 */:' $(@D)/options.h
+	$(SED) 's:\(#define ENABLE_SVR_LOCALTCPFWD\).*:/*\1 */:' $(@D)/options.h
+	$(SED) 's:\(#define ENABLE_SVR_REMOTETCPFWD\).*:/*\1 */:' $(@D)/options.h
+endef
+
 define DROPBEAR_BUILD_SMALL
 	$(SED) 's:.*\(#define DROPBEAR_SMALL_CODE\).*:\1:' $(@D)/options.h
 	$(SED) 's:.*\(#define NO_FAST_EXPTMOD\).*:\1:' $(@D)/options.h
@@ -64,6 +81,14 @@  ifeq ($(BR2_PACKAGE_DROPBEAR_DISABLE_REVERSEDNS),)
 DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_ENABLE_REVERSE_DNS
 endif
 
+ifeq ($(BR2_PACKAGE_DROPBEAR_DISABLE_TCP_FORWARDING),y)
+DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_DISABLE_TCP_FORWARDING
+endif
+
+ifeq ($(BR2_PACKAGE_DROPBEAR_DISABLE_PASSWORD_AUTH),y)
+DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_DISABLE_PASSWORD_AUTH
+endif
+
 ifeq ($(BR2_PACKAGE_DROPBEAR_SMALL),y)
 DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_BUILD_SMALL
 DROPBEAR_CONF_OPT += --disable-zlib