[tor-commits] [tor/master] control/control: Refactor some error handling code

[nickm at torproject.org]

commit 3d17fafa04baf23d124edf56560abf3b19ad4425
Author: teor <teor at torproject.org>
Date:   Tue Aug 6 00:18:41 2019 +1000

    control/control: Refactor some error handling code
    
    Split some protocol error handling out of
    connection_control_process_inbuf().
    
    This refactor reduces the size of a practracker exception.
    
    Closes 31840.
---
 changes/ticket31840                      |  3 ++
 scripts/maint/practracker/exceptions.txt |  2 +-
 src/feature/control/control.c            | 79 ++++++++++++++++++++++----------
 3 files changed, 59 insertions(+), 25 deletions(-)

diff --git a/changes/ticket31840 b/changes/ticket31840
new file mode 100644
index 000000000..c75c5629f
--- /dev/null
+++ b/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.
diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt
index 2e676d904..88b34b21f 100644
--- a/scripts/maint/practracker/exceptions.txt
+++ b/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
diff --git a/src/feature/control/control.c b/src/feature/control/control.c
index 436bf423c..d6581808c 100644
--- a/src/feature/control/control.c
+++ b/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;
   }