Allow empty username/password in SOCKS5 username/password auth message

3 files changed, 51 insertions, 4 deletions

diff --git a/changes/bug29175_035 b/changes/bug29175_035
new file mode 100644
index 0000000000..134c1d9529
--- /dev/null
+++ b/changes/bug29175_035
@@ -0,0 +1,4 @@
+  o Major bugfixes (networking):
+    - Gracefully handle empty username/password fields in SOCKS5
+      username/password auth messsage and allow SOCKS5 handshake to
+      continue. Fixes bug 29175; bugfix on 0.3.5.1-alpha.
diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c
index a8f4a06c26..8b78ed44c2 100644
--- a/src/core/proto/proto_socks.c
+++ b/src/core/proto/proto_socks.c
@@ -450,18 +450,22 @@ parse_socks5_userpass_auth(const uint8_t *raw_data, socks_request_t *req,
     tor_free(req->username);
     req->username = tor_memdup_nulterm(username, usernamelen);
     req->usernamelen = usernamelen;
-
-    req->got_auth = 1;
   }
 
   if (passwordlen && password) {
     tor_free(req->password);
     req->password = tor_memdup_nulterm(password, passwordlen);
     req->passwordlen = passwordlen;
-
-    req->got_auth = 1;
   }
 
+  /**
+   * Yes, we allow username and/or password to be empty. Yes, that does
+   * violate RFC 1929. However, some client software can send a username/
+   * password message with these fields being empty and we want to allow them
+   * to be used with Tor.
+   */
+  req->got_auth = 1;
+
   end:
   socks5_client_userpass_auth_free(trunnel_req);
   return res;
diff --git a/src/test/test_socks.c b/src/test/test_socks.c
index 2832d42fc1..783f4726ee 100644
--- a/src/test/test_socks.c
+++ b/src/test/test_socks.c
@@ -479,6 +479,44 @@ test_socks_5_authenticate(void *ptr)
   ;
 }
 
+/** Perform SOCKS 5 authentication with empty username/password fields.
+ * Technically this violates RfC 1929, but some client software will send
+ * this kind of message to Tor.
+ * */
+static void
+test_socks_5_authenticate_empty_user_pass(void *ptr)
+{
+  SOCKS_TEST_INIT();
+
+  /* SOCKS 5 Negotiate username/password authentication */
+  ADD_DATA(buf, "\x05\x01\x02");
+
+  tt_assert(!fetch_from_buf_socks(buf, socks,
+                                   get_options()->TestSocks,
+                                   get_options()->SafeSocks));
+  tt_int_op(2,OP_EQ, socks->replylen);
+  tt_int_op(5,OP_EQ, socks->reply[0]);
+  tt_int_op(SOCKS_USER_PASS,OP_EQ, socks->reply[1]);
+  tt_int_op(5,OP_EQ, socks->socks_version);
+
+  tt_int_op(0,OP_EQ, buf_datalen(buf));
+
+  /* SOCKS 5 Send username/password auth message with empty user/pass fields */
+  ADD_DATA(buf, "\x01\x00\x00");
+  tt_assert(!fetch_from_buf_socks(buf, socks,
+                                   get_options()->TestSocks,
+                                   get_options()->SafeSocks));
+  tt_int_op(5,OP_EQ, socks->socks_version);
+  tt_int_op(2,OP_EQ, socks->replylen);
+  tt_int_op(1,OP_EQ, socks->reply[0]);
+  tt_int_op(0,OP_EQ, socks->reply[1]);
+
+  tt_int_op(0,OP_EQ, socks->usernamelen);
+  tt_int_op(0,OP_EQ, socks->passwordlen);
+
+ done:
+  ;
+}
 /** Perform SOCKS 5 authentication and send data all in one go */
 static void
 test_socks_5_authenticate_with_data(void *ptr)
@@ -1035,6 +1073,7 @@ struct testcase_t socks_tests[] = {
   SOCKSENT(5_auth_unsupported_version),
   SOCKSENT(5_auth_before_negotiation),
   SOCKSENT(5_authenticate),
+  SOCKSENT(5_authenticate_empty_user_pass),
   SOCKSENT(5_authenticate_with_data),
   SOCKSENT(5_malformed_commands),
   SOCKSENT(5_bad_arguments),