[bug#32545] [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[bug#32545] [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[Leo Famulari]

Dropbear users, please test!

* gnu/packages/patches/dropbear-CVE-2018-15599.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/ssh.scm (dropbear)[source]: Use it.
---
 gnu/local.mk                                  |   1 +
 .../patches/dropbear-CVE-2018-15599.patch     | 240 ++++++++++++++++++
 gnu/packages/ssh.scm                          |   1 +
 3 files changed, 242 insertions(+)
 create mode 100644 gnu/packages/patches/dropbear-CVE-2018-15599.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index d0e84c597..6a6486354 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -651,6 +651,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/dovecot-trees-support-dovecot-2.3.patch	\
   %D%/packages/patches/doxygen-gcc-ice.patch			\
   %D%/packages/patches/doxygen-test.patch			\
+  %D%/packages/patches/dropbear-CVE-2018-15599.patch		\
   %D%/packages/patches/dvd+rw-tools-add-include.patch 		\
   %D%/packages/patches/elfutils-tests-ptrace.patch		\
   %D%/packages/patches/elogind-glibc-2.27.patch			\
diff --git a/gnu/packages/patches/dropbear-CVE-2018-15599.patch b/gnu/packages/patches/dropbear-CVE-2018-15599.patch
new file mode 100644
index 000000000..a474552cd
--- /dev/null
+++ b/gnu/packages/patches/dropbear-CVE-2018-15599.patch
@@ -0,0 +1,240 @@
+Fix CVE-2018-15599:
+
+http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/2018q3/002108.html
+https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-15599
+
+Patch copied from upstream source repository:
+
+https://github.com/mkj/dropbear/commit/52adbb34c32d3e2e1bcdb941e20a6f81138b8248
+
+From 52adbb34c32d3e2e1bcdb941e20a6f81138b8248 Mon Sep 17 00:00:00 2001
+From: Matt Johnston <matt@ucc.asn.au>
+Date: Thu, 23 Aug 2018 23:43:12 +0800
+Subject: [PATCH] Wait to fail invalid usernames
+
+---
+ auth.h           |  6 +++---
+ svr-auth.c       | 19 +++++--------------
+ svr-authpam.c    | 26 ++++++++++++++++++++++----
+ svr-authpasswd.c | 27 ++++++++++++++-------------
+ svr-authpubkey.c | 11 ++++++++++-
+ 5 files changed, 54 insertions(+), 35 deletions(-)
+
+diff --git a/auth.h b/auth.h
+index da498f5b..98f54683 100644
+--- a/auth.h
++++ b/auth.h
+@@ -37,9 +37,9 @@ void recv_msg_userauth_request(void);
+ void send_msg_userauth_failure(int partial, int incrfail);
+ void send_msg_userauth_success(void);
+ void send_msg_userauth_banner(const buffer *msg);
+-void svr_auth_password(void);
+-void svr_auth_pubkey(void);
+-void svr_auth_pam(void);
++void svr_auth_password(int valid_user);
++void svr_auth_pubkey(int valid_user);
++void svr_auth_pam(int valid_user);
+ 
+ #if DROPBEAR_SVR_PUBKEY_OPTIONS_BUILT
+ int svr_pubkey_allows_agentfwd(void);
+diff --git a/svr-auth.c b/svr-auth.c
+index c19c0901..edde86bc 100644
+--- a/svr-auth.c
++++ b/svr-auth.c
+@@ -149,10 +149,8 @@ void recv_msg_userauth_request() {
+ 		if (methodlen == AUTH_METHOD_PASSWORD_LEN &&
+ 				strncmp(methodname, AUTH_METHOD_PASSWORD,
+ 					AUTH_METHOD_PASSWORD_LEN) == 0) {
+-			if (valid_user) {
+-				svr_auth_password();
+-				goto out;
+-			}
++			svr_auth_password(valid_user);
++			goto out;
+ 		}
+ 	}
+ #endif
+@@ -164,10 +162,8 @@ void recv_msg_userauth_request() {
+ 		if (methodlen == AUTH_METHOD_PASSWORD_LEN &&
+ 				strncmp(methodname, AUTH_METHOD_PASSWORD,
+ 					AUTH_METHOD_PASSWORD_LEN) == 0) {
+-			if (valid_user) {
+-				svr_auth_pam();
+-				goto out;
+-			}
++			svr_auth_pam(valid_user);
++			goto out;
+ 		}
+ 	}
+ #endif
+@@ -177,12 +173,7 @@ void recv_msg_userauth_request() {
+ 	if (methodlen == AUTH_METHOD_PUBKEY_LEN &&
+ 			strncmp(methodname, AUTH_METHOD_PUBKEY,
+ 				AUTH_METHOD_PUBKEY_LEN) == 0) {
+-		if (valid_user) {
+-			svr_auth_pubkey();
+-		} else {
+-			/* pubkey has no failure delay */
+-			send_msg_userauth_failure(0, 0);
+-		}
++		svr_auth_pubkey(valid_user);
+ 		goto out;
+ 	}
+ #endif
+diff --git a/svr-authpam.c b/svr-authpam.c
+index 05e4f3e5..d201bc96 100644
+--- a/svr-authpam.c
++++ b/svr-authpam.c
+@@ -178,13 +178,14 @@ pamConvFunc(int num_msg,
+  * Keyboard interactive would be a lot nicer, but since PAM is synchronous, it
+  * gets very messy trying to send the interactive challenges, and read the
+  * interactive responses, over the network. */
+-void svr_auth_pam() {
++void svr_auth_pam(int valid_user) {
+ 
+ 	struct UserDataS userData = {NULL, NULL};
+ 	struct pam_conv pamConv = {
+ 		pamConvFunc,
+ 		&userData /* submitted to pamvConvFunc as appdata_ptr */ 
+ 	};
++	const char* printable_user = NULL;
+ 
+ 	pam_handle_t* pamHandlep = NULL;
+ 
+@@ -204,12 +205,23 @@ void svr_auth_pam() {
+ 
+ 	password = buf_getstring(ses.payload, &passwordlen);
+ 
++	/* We run the PAM conversation regardless of whether the username is valid
++	in case the conversation function has an inherent delay.
++	Use ses.authstate.username rather than ses.authstate.pw_name.
++	After PAM succeeds we then check the valid_user flag too */
++
+ 	/* used to pass data to the PAM conversation function - don't bother with
+ 	 * strdup() etc since these are touched only by our own conversation
+ 	 * function (above) which takes care of it */
+-	userData.user = ses.authstate.pw_name;
++	userData.user = ses.authstate.username;
+ 	userData.passwd = password;
+ 
++	if (ses.authstate.pw_name) {
++		printable_user = ses.authstate.pw_name;
++	} else {
++		printable_user = "<invalid username>";
++	}
++
+ 	/* Init pam */
+ 	if ((rc = pam_start("sshd", NULL, &pamConv, &pamHandlep)) != PAM_SUCCESS) {
+ 		dropbear_log(LOG_WARNING, "pam_start() failed, rc=%d, %s", 
+@@ -242,7 +254,7 @@ void svr_auth_pam() {
+ 				rc, pam_strerror(pamHandlep, rc));
+ 		dropbear_log(LOG_WARNING,
+ 				"Bad PAM password attempt for '%s' from %s",
+-				ses.authstate.pw_name,
++				printable_user,
+ 				svr_ses.addrstring);
+ 		send_msg_userauth_failure(0, 1);
+ 		goto cleanup;
+@@ -253,12 +265,18 @@ void svr_auth_pam() {
+ 				rc, pam_strerror(pamHandlep, rc));
+ 		dropbear_log(LOG_WARNING,
+ 				"Bad PAM password attempt for '%s' from %s",
+-				ses.authstate.pw_name,
++				printable_user,
+ 				svr_ses.addrstring);
+ 		send_msg_userauth_failure(0, 1);
+ 		goto cleanup;
+ 	}
+ 
++	if (!valid_user) {
++		/* PAM auth succeeded but the username isn't allowed in for another reason
++		(checkusername() failed) */
++		send_msg_userauth_failure(0, 1);
++	}
++
+ 	/* successful authentication */
+ 	dropbear_log(LOG_NOTICE, "PAM password auth succeeded for '%s' from %s",
+ 			ses.authstate.pw_name,
+diff --git a/svr-authpasswd.c b/svr-authpasswd.c
+index bdee2aa1..69c7d8af 100644
+--- a/svr-authpasswd.c
++++ b/svr-authpasswd.c
+@@ -48,22 +48,14 @@ static int constant_time_strcmp(const char* a, const char* b) {
+ 
+ /* Process a password auth request, sending success or failure messages as
+  * appropriate */
+-void svr_auth_password() {
++void svr_auth_password(int valid_user) {
+ 	
+ 	char * passwdcrypt = NULL; /* the crypt from /etc/passwd or /etc/shadow */
+ 	char * testcrypt = NULL; /* crypt generated from the user's password sent */
+-	char * password;
++	char * password = NULL;
+ 	unsigned int passwordlen;
+-
+ 	unsigned int changepw;
+ 
+-	passwdcrypt = ses.authstate.pw_passwd;
+-
+-#ifdef DEBUG_HACKCRYPT
+-	/* debugging crypt for non-root testing with shadows */
+-	passwdcrypt = DEBUG_HACKCRYPT;
+-#endif
+-
+ 	/* check if client wants to change password */
+ 	changepw = buf_getbool(ses.payload);
+ 	if (changepw) {
+@@ -73,12 +65,21 @@ void svr_auth_password() {
+ 	}
+ 
+ 	password = buf_getstring(ses.payload, &passwordlen);
+-
+-	/* the first bytes of passwdcrypt are the salt */
+-	testcrypt = crypt(password, passwdcrypt);
++	if (valid_user) {
++		/* the first bytes of passwdcrypt are the salt */
++		passwdcrypt = ses.authstate.pw_passwd;
++		testcrypt = crypt(password, passwdcrypt);
++	}
+ 	m_burn(password, passwordlen);
+ 	m_free(password);
+ 
++	/* After we have got the payload contents we can exit if the username
++	is invalid. Invalid users have already been logged. */
++	if (!valid_user) {
++		send_msg_userauth_failure(0, 1);
++		return;
++	}
++
+ 	if (testcrypt == NULL) {
+ 		/* crypt() with an invalid salt like "!!" */
+ 		dropbear_log(LOG_WARNING, "User account '%s' is locked",
+diff --git a/svr-authpubkey.c b/svr-authpubkey.c
+index aa6087c9..ff481c87 100644
+--- a/svr-authpubkey.c
++++ b/svr-authpubkey.c
+@@ -79,7 +79,7 @@ static int checkfileperm(char * filename);
+ 
+ /* process a pubkey auth request, sending success or failure message as
+  * appropriate */
+-void svr_auth_pubkey() {
++void svr_auth_pubkey(int valid_user) {
+ 
+ 	unsigned char testkey; /* whether we're just checking if a key is usable */
+ 	char* algo = NULL; /* pubkey algo */
+@@ -102,6 +102,15 @@ void svr_auth_pubkey() {
+ 	keybloblen = buf_getint(ses.payload);
+ 	keyblob = buf_getptr(ses.payload, keybloblen);
+ 
++	if (!valid_user) {
++		/* Return failure once we have read the contents of the packet
++		required to validate a public key. 
++		Avoids blind user enumeration though it isn't possible to prevent
++		testing for user existence if the public key is known */
++		send_msg_userauth_failure(0, 0);
++		goto out;
++	}
++
+ 	/* check if the key is valid */
+ 	if (checkpubkey(algo, algolen, keyblob, keybloblen) == DROPBEAR_FAILURE) {
+ 		send_msg_userauth_failure(0, 0);
diff --git a/gnu/packages/ssh.scm b/gnu/packages/ssh.scm
index a58ebff48..03c4e3cc0 100644
--- a/gnu/packages/ssh.scm
+++ b/gnu/packages/ssh.scm
@@ -440,6 +440,7 @@ TCP, not the SSH protocol.")
               (uri (string-append
                     "https://matt.ucc.asn.au/" name "/releases/"
                     name "-" version ".tar.bz2"))
+              (patches (search-patches "dropbear-CVE-2018-15599.patch"))
               (sha256
                (base32
                 "0rgavbzw7jrs5wslxm0dnwx2m409yzxd9hazd92r7kx8xikr3yzj"))))
-- 
2.18.0

[bug#32545] [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[Ludovic Courtès]

Hi!

Leo Famulari <leo@famulari.name> skribis:

> Dropbear users, please test!
>
> * gnu/packages/patches/dropbear-CVE-2018-15599.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/ssh.scm (dropbear)[source]: Use it.

I haven’t tested it but the patch LGTM, FWIW.  You can also run “make
check-system TESTS=dropbear” if you haven’t already, to make sure the
basics work.

Thanks,
Ludo’.

[bug#32545] [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[Clément Lassieur]

Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> Leo Famulari <leo@famulari.name> skribis:
>
>> Dropbear users, please test!
>>
>> * gnu/packages/patches/dropbear-CVE-2018-15599.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add it.
>> * gnu/packages/ssh.scm (dropbear)[source]: Use it.
>
> I haven’t tested it but the patch LGTM, FWIW.  You can also run “make
> check-system TESTS=dropbear” if you haven’t already, to make sure the
> basics work.

Leo said on IRC that this produces 0 tests, and I can reproduce this:

    $ ~/.guix$ make check-system TESTS="dropbear"
    Compiling Scheme modules...
    Running 0 system tests...
    TOTAL: 0

bug#32545: [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

On Tue, Aug 28, 2018 at 02:22:59PM +0200, Clément Lassieur wrote:
> Ludovic Courtès <ludo@gnu.org> writes:
> > I haven’t tested it but the patch LGTM, FWIW.  You can also run “make
> > check-system TESTS=dropbear” if you haven’t already, to make sure the
> > basics work.

Thanks! Pushed as 8a5a1eff422c5e3bca785f3967d444d0eafcf9c3

> Leo said on IRC that this produces 0 tests, and I can reproduce this:
> 
>     $ ~/.guix$ make check-system TESTS="dropbear"
>     Compiling Scheme modules...
>     Running 0 system tests...
>     TOTAL: 0

Yes, I'm starting a new bug for this :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#32545] [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[Ludovic Courtès]

Hello,

Clément Lassieur <clement@lassieur.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi!
>>
>> Leo Famulari <leo@famulari.name> skribis:
>>
>>> Dropbear users, please test!
>>>
>>> * gnu/packages/patches/dropbear-CVE-2018-15599.patch: New file.
>>> * gnu/local.mk (dist_patch_DATA): Add it.
>>> * gnu/packages/ssh.scm (dropbear)[source]: Use it.
>>
>> I haven’t tested it but the patch LGTM, FWIW.  You can also run “make
>> check-system TESTS=dropbear” if you haven’t already, to make sure the
>> basics work.
>
> Leo said on IRC that this produces 0 tests, and I can reproduce this:
>
>     $ ~/.guix$ make check-system TESTS="dropbear"
>     Compiling Scheme modules...
>     Running 0 system tests...
>     TOTAL: 0

“rm gnu/tests/ssh.go && make” will fix it.

The reason is that 6772ed1e07d6b8ce557199d91aaa1442c77186c7 changed the
ABI of <openssh-configuration>.  Thus, gnu/tests/ssh.go is stale, and if
you try to load it manually, you get the “ABI mismatch” error that
invites you to recompile.

The command above uses (guix discovery) to find system tests exported by
modules under (gnu tests …).  Since it fails to load (gnu tests ssh), it
just silently skips it and concludes that there’s no “dropbear” test.

Commit d258c791441b46705f4360cf141343363d1751f2 has a warning displayed
in this case.

Thanks,
Ludo’.

[bug#32545] [PATCH] gnu: dropbear: Fix CVE-2018-15599.

[Clément Lassieur]

Hello Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Hi!
>>>
>>> Leo Famulari <leo@famulari.name> skribis:
>>>
>>>> Dropbear users, please test!
>>>>
>>>> * gnu/packages/patches/dropbear-CVE-2018-15599.patch: New file.
>>>> * gnu/local.mk (dist_patch_DATA): Add it.
>>>> * gnu/packages/ssh.scm (dropbear)[source]: Use it.
>>>
>>> I haven’t tested it but the patch LGTM, FWIW.  You can also run “make
>>> check-system TESTS=dropbear” if you haven’t already, to make sure the
>>> basics work.
>>
>> Leo said on IRC that this produces 0 tests, and I can reproduce this:
>>
>>     $ ~/.guix$ make check-system TESTS="dropbear"
>>     Compiling Scheme modules...
>>     Running 0 system tests...
>>     TOTAL: 0
>
> “rm gnu/tests/ssh.go && make” will fix it.
>
> The reason is that 6772ed1e07d6b8ce557199d91aaa1442c77186c7 changed the
> ABI of <openssh-configuration>.  Thus, gnu/tests/ssh.go is stale, and if
> you try to load it manually, you get the “ABI mismatch” error that
> invites you to recompile.
>
> The command above uses (guix discovery) to find system tests exported by
> modules under (gnu tests …).  Since it fails to load (gnu tests ssh), it
> just silently skips it and concludes that there’s no “dropbear” test.
>
> Commit d258c791441b46705f4360cf141343363d1751f2 has a warning displayed
> in this case.
>
> Thanks,
> Ludo’.

Understood, thank you for the explanation!