Browse Source

Merge remote-tracking branch 'tor-github/pr/1358'

Nick Mathewson 4 years ago
parent
commit
5fd137c482
3 changed files with 59 additions and 25 deletions
  1. 3 0
      changes/ticket31840
  2. 1 1
      scripts/maint/practracker/exceptions.txt
  3. 55 24
      src/feature/control/control.c

+ 3 - 0
changes/ticket31840

@@ -0,0 +1,3 @@
+  o Code simplification and refactoring:
+    - Refactor connection_control_process_inbuf() to reduce the size of a
+      practracker exception. Closes ticket 31840.

+ 1 - 1
scripts/maint/practracker/exceptions.txt

@@ -186,7 +186,7 @@ problem file-size /src/feature/client/entrynodes.h 639
 problem function-size /src/feature/client/transports.c:handle_proxy_line() 108
 problem function-size /src/feature/client/transports.c:parse_method_line_helper() 110
 problem function-size /src/feature/client/transports.c:create_managed_proxy_environment() 109
-problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 136
+problem function-size /src/feature/control/control.c:connection_control_process_inbuf() 113
 problem function-size /src/feature/control/control_auth.c:handle_control_authenticate() 186
 problem function-size /src/feature/control/control_cmd.c:handle_control_extendcircuit() 150
 problem function-size /src/feature/control/control_cmd.c:handle_control_add_onion() 256

+ 55 - 24
src/feature/control/control.c

@@ -339,6 +339,60 @@ static const char CONTROLPORT_IS_NOT_AN_HTTP_PROXY_MSG[] =
   "</body>\n"
   "</html>\n";
 
+/** Return an error on a control connection that tried to use the v0 protocol.
+ */
+static void
+control_send_v0_reject(control_connection_t *conn)
+{
+  size_t body_len;
+  char buf[128];
+  set_uint16(buf+2, htons(0x0000)); /* type == error */
+  set_uint16(buf+4, htons(0x0001)); /* code == internal error */
+  strlcpy(buf+6, "The v0 control protocol is not supported by Tor 0.1.2.17 "
+          "and later; upgrade your controller.",
+          sizeof(buf)-6);
+  body_len = 2+strlen(buf+6)+2; /* code, msg, nul. */
+  set_uint16(buf+0, htons(body_len));
+  connection_buf_add(buf, 4+body_len, TO_CONN(conn));
+
+  connection_mark_and_flush(TO_CONN(conn));
+}
+
+/** Return an error on a control connection that tried to use HTTP.
+ */
+static void
+control_send_http_reject(control_connection_t *conn)
+{
+  connection_write_str_to_buf(CONTROLPORT_IS_NOT_AN_HTTP_PROXY_MSG, conn);
+  log_notice(LD_CONTROL, "Received HTTP request on ControlPort");
+  connection_mark_and_flush(TO_CONN(conn));
+}
+
+/** Check if a control connection has tried to use a known invalid protocol.
+ * If it has, then:
+ *  - send a reject response,
+ *  - log a notice-level message, and
+ *  - return false. */
+static bool
+control_protocol_is_valid(control_connection_t *conn)
+{
+  /* Detect v0 commands and send a "no more v0" message. */
+  if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH &&
+      peek_connection_has_control0_command(TO_CONN(conn))) {
+    control_send_v0_reject(conn);
+    return 0;
+  }
+
+  /* If the user has the HTTP proxy port and the control port confused. */
+  if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH &&
+      peek_connection_has_http_command(TO_CONN(conn))) {
+    control_send_http_reject(conn);
+    return 0;
+  }
+
+  return 1;
+}
+
 /** Called when data has arrived on a v1 control connection: Try to fetch
  * commands from conn->inbuf, and execute them.
  */
@@ -359,30 +413,7 @@ connection_control_process_inbuf(control_connection_t *conn)
     conn->incoming_cmd_cur_len = 0;
   }
 
-  if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH &&
-      peek_connection_has_control0_command(TO_CONN(conn))) {
-    /* Detect v0 commands and send a "no more v0" message. */
-    size_t body_len;
-    char buf[128];
-    set_uint16(buf+2, htons(0x0000)); /* type == error */
-    set_uint16(buf+4, htons(0x0001)); /* code == internal error */
-    strlcpy(buf+6, "The v0 control protocol is not supported by Tor 0.1.2.17 "
-            "and later; upgrade your controller.",
-            sizeof(buf)-6);
-    body_len = 2+strlen(buf+6)+2; /* code, msg, nul. */
-    set_uint16(buf+0, htons(body_len));
-    connection_buf_add(buf, 4+body_len, TO_CONN(conn));
-
-    connection_mark_and_flush(TO_CONN(conn));
-    return 0;
-  }
-
-  /* If the user has the HTTP proxy port and the control port confused. */
-  if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH &&
-      peek_connection_has_http_command(TO_CONN(conn))) {
-    connection_write_str_to_buf(CONTROLPORT_IS_NOT_AN_HTTP_PROXY_MSG, conn);
-    log_notice(LD_CONTROL, "Received HTTP request on ControlPort");
-    connection_mark_and_flush(TO_CONN(conn));
+  if (!control_protocol_is_valid(conn)) {
     return 0;
   }