DROPBEAR_CLI_AUTH_IMMEDIATE fixed, now enabled by default

cli-auth.c

@@ -41,16 +41,6 @@ void cli_authinitialise() {
 /* Send a "none" auth request to get available methods */
 void cli_auth_getmethods() {
 	TRACE(("enter cli_auth_getmethods"))
-#ifdef CLI_IMMEDIATE_AUTH
-	ses.authstate.authtypes = AUTH_TYPE_PUBKEY;
-    if (getenv(DROPBEAR_PASSWORD_ENV)) {
-		ses.authstate.authtypes |= AUTH_TYPE_PASSWORD | AUTH_TYPE_INTERACT;
-	}
-	if (cli_auth_try() == DROPBEAR_SUCCESS) {
-		TRACE(("skipped initial none auth query"))
-		return;
-	}
-#endif
 	CHECKCLEARTOWRITE();
 	buf_putbyte(ses.writepayload, SSH_MSG_USERAUTH_REQUEST);
 	buf_putstring(ses.writepayload, cli_opts.username, 
@@ -60,6 +50,18 @@ void cli_auth_getmethods() {
 	buf_putstring(ses.writepayload, "none", 4); /* 'none' method */
 
 	encrypt_packet();
+
+#ifdef DROPBEAR_CLI_IMMEDIATE_AUTH
+	ses.authstate.authtypes = AUTH_TYPE_PUBKEY;
+    if (getenv(DROPBEAR_PASSWORD_ENV)) {
+		ses.authstate.authtypes |= AUTH_TYPE_PASSWORD | AUTH_TYPE_INTERACT;
+	}
+	if (cli_auth_try() == DROPBEAR_SUCCESS) {
+		TRACE(("skipped initial none auth query"))
+		/* Note that there will be two auth responses in-flight */
+		cli_ses.ignore_next_auth_response = 1;
+	}
+#endif
 	TRACE(("leave cli_auth_getmethods"))
 }
 
@@ -150,31 +152,46 @@ void recv_msg_userauth_failure() {
 	TRACE(("<- MSG_USERAUTH_FAILURE"))
 	TRACE(("enter recv_msg_userauth_failure"))
 
+	if (ses.authstate.authdone) {
+		TRACE(("leave recv_msg_userauth_failure, already authdone."))
+		return;
+	}
+
 	if (cli_ses.state != USERAUTH_REQ_SENT) {
 		/* Perhaps we should be more fatal? */
 		dropbear_exit("Unexpected userauth failure");
 	}
 
+	/* When DROPBEAR_CLI_IMMEDIATE_AUTH is set there will be an initial response for 
+	the "none" auth request, and then a response to the immediate auth request. 
+	We need to be careful handling them. */
+	if (cli_ses.ignore_next_auth_response) {
+		TRACE(("ignore next response, state set to USERAUTH_REQ_SENT"))
+		cli_ses.state = USERAUTH_REQ_SENT;
+	} else  {
+		cli_ses.state = USERAUTH_FAIL_RCVD;
+		cli_ses.lastauthtype = AUTH_TYPE_NONE;
 #ifdef ENABLE_CLI_PUBKEY_AUTH
-	/* If it was a pubkey auth request, we should cross that key 
-	 * off the list. */
-	if (cli_ses.lastauthtype == AUTH_TYPE_PUBKEY) {
-		cli_pubkeyfail();
-	}
+		/* If it was a pubkey auth request, we should cross that key 
+		 * off the list. */
+		if (cli_ses.lastauthtype == AUTH_TYPE_PUBKEY) {
+			cli_pubkeyfail();
+		}
 #endif
 
 #ifdef ENABLE_CLI_INTERACT_AUTH
-	/* If we get a failure message for keyboard interactive without
-	 * receiving any request info packet, then we don't bother trying
-	 * keyboard interactive again */
-	if (cli_ses.lastauthtype == AUTH_TYPE_INTERACT
-			&& !cli_ses.interact_request_received) {
-		TRACE(("setting auth_interact_failed = 1"))
-		cli_ses.auth_interact_failed = 1;
-	}
+		/* If we get a failure message for keyboard interactive without
+		 * receiving any request info packet, then we don't bother trying
+		 * keyboard interactive again */
+		if (cli_ses.lastauthtype == AUTH_TYPE_INTERACT
+				&& !cli_ses.interact_request_received) {
+			TRACE(("setting auth_interact_failed = 1"))
+			cli_ses.auth_interact_failed = 1;
+		}
 #endif
+	}
 
-	cli_ses.lastauthtype = AUTH_TYPE_NONE;
+	cli_ses.ignore_next_auth_response = 0;
 
 	methods = buf_getstring(ses.payload, &methlen);
 
@@ -227,13 +244,14 @@ void recv_msg_userauth_failure() {
 	}
 
 	m_free(methods);
-
-	cli_ses.state = USERAUTH_FAIL_RCVD;
 		
 	TRACE(("leave recv_msg_userauth_failure"))
 }
 
 void recv_msg_userauth_success() {
+	/* This function can validly get called multiple times
+	if DROPBEAR_CLI_IMMEDIATE_AUTH is set */
+
 	TRACE(("received msg_userauth_success"))
 	/* Note: in delayed-zlib mode, setting authdone here 
 	 * will enable compression in the transport layer */

cli-session.c

@@ -228,6 +228,10 @@ static void cli_sessionloop() {
 			cli_ses.state = USERAUTH_REQ_SENT;
 			TRACE(("leave cli_sessionloop: sent userauth methods req"))
 			return;
+
+		case USERAUTH_REQ_SENT:
+			TRACE(("leave cli_sessionloop: waiting, req_sent"))
+			return;
 			
 		case USERAUTH_FAIL_RCVD:
 			if (cli_auth_try() == DROPBEAR_FAILURE) {

common-kex.c

@@ -792,6 +792,11 @@ static void finish_kexhashbuf(void) {
 	hash_desc->done(&hs, buf_getwriteptr(ses.hash, hash_desc->hashsize));
 	buf_setlen(ses.hash, hash_desc->hashsize);
 
+#ifdef DEBUG_KEXHASH
+	printhex("kexhashbuf", ses.kexhashbuf->data, ses.kexhashbuf->len);
+	printhex("kexhash", ses.hash->data, ses.hash->len);
+#endif
+
 	buf_burn(ses.kexhashbuf);
 	buf_free(ses.kexhashbuf);
 	ses.kexhashbuf = NULL;

dbutil.c

@@ -812,6 +812,10 @@ out:
 /* make sure that the socket closes */
 void m_close(int fd) {
 
+	if (fd == -1) {
+		return;
+	}
+
 	int val;
 	do {
 		val = close(fd);

options.h

@@ -222,13 +222,6 @@ much traffic. */
  * return the password on standard output */
 /*#define ENABLE_CLI_ASKPASS_HELPER*/
 
-/* Send a real auth request first rather than requesting a list of available methods.
- * It saves a network round trip at login but prevents immediate login to
- * accounts with no password, and might be rejected by some strict servers (none
- * encountered yet) - hence it isn't enabled by default. */
-/* #define CLI_IMMEDIATE_AUTH */
-
-
 /* Source for randomness. This must be able to provide hundreds of bytes per SSH
  * connection without blocking. In addition /dev/random is used for seeding
  * rsa/dss key generation */

session.h

@@ -233,6 +233,7 @@ typedef enum {
 
 typedef enum {
 	STATE_NOTHING,
+	USERAUTH_WAIT,
 	USERAUTH_REQ_SENT,
 	USERAUTH_FAIL_RCVD,
 	USERAUTH_SUCCESS_RCVD,
@@ -267,6 +268,7 @@ struct clientsession {
 
 	int lastauthtype; /* either AUTH_TYPE_PUBKEY or AUTH_TYPE_PASSWORD,
 						 for the last type of auth we tried */
+	int ignore_next_auth_response;
 #ifdef ENABLE_CLI_INTERACT_AUTH
 	int auth_interact_failed; /* flag whether interactive auth can still
 								 be used */

sysoptions.h

@@ -176,6 +176,12 @@
 												accept for keyb-interactive
 												auth */
 
+/* Send a real auth request immediately after sending a query for the available methods.
+ * It saves a network round trip at login.
+ * If problems are encountered it can be disabled here. */
+ #define CLI_IMMEDIATE_AUTH
+
+
 #if defined(DROPBEAR_AES256) || defined(DROPBEAR_AES128)
 #define DROPBEAR_AES
 #endif