commit 3d17fafa04baf23d124edf56560abf3b19ad4425 Author: teor <teor@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; }