From 965c2064da9ae731873110a1f6da66e2f6fa020e Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Fri, 5 Apr 2019 17:27:15 -0500 Subject: [PATCH 1/9] Correct file name in doxygen comment --- src/feature/control/control_fmt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/feature/control/control_fmt.c b/src/feature/control/control_fmt.c index b2ab4f10bb..dac3f87795 100644 --- a/src/feature/control/control_fmt.c +++ b/src/feature/control/control_fmt.c @@ -3,7 +3,7 @@ /* See LICENSE for licensing information */ /** - * \file control.c + * \file control_fmt.c * \brief Formatting functions for controller data. */ From 8e7316bae43e666a9fa3b99448561af90cbc21b8 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Mon, 8 Apr 2019 11:34:12 -0500 Subject: [PATCH 2/9] Split reply formatting out of control_fmt.c Split the core reply formatting code out of control_fmt.c into control_proto.c. The remaining code in control_format.c deals with specific subsystems and will eventually move to join those subsystems. --- scripts/maint/practracker/exceptions.txt | 2 +- src/core/include.am | 2 + src/feature/control/control.c | 2 +- src/feature/control/control_auth.c | 2 +- src/feature/control/control_cmd.c | 2 +- src/feature/control/control_events.c | 1 + src/feature/control/control_fmt.c | 142 +------------------ src/feature/control/control_fmt.h | 9 -- src/feature/control/control_getinfo.c | 1 + src/feature/control/control_proto.c | 165 +++++++++++++++++++++++ src/feature/control/control_proto.h | 24 ++++ src/test/test_util.c | 2 +- 12 files changed, 199 insertions(+), 155 deletions(-) create mode 100644 src/feature/control/control_proto.c create mode 100644 src/feature/control/control_proto.h diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index d90ed1f4bd..ea774e2e83 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -152,7 +152,7 @@ problem function-size /src/feature/control/control_cmd.c:handle_control_add_onio problem function-size /src/feature/control/control_cmd.c:add_onion_helper_keyarg() 125 problem function-size /src/feature/control/control_cmd.c:handle_control_command() 104 problem function-size /src/feature/control/control_events.c:control_event_stream_status() 119 -problem include-count /src/feature/control/control_getinfo.c 53 +problem include-count /src/feature/control/control_getinfo.c 54 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_misc() 109 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_dir() 304 problem function-size /src/feature/control/control_getinfo.c:getinfo_helper_events() 236 diff --git a/src/core/include.am b/src/core/include.am index 4ec42182a6..2533a4990f 100644 --- a/src/core/include.am +++ b/src/core/include.am @@ -79,6 +79,7 @@ LIBTOR_APP_A_SOURCES = \ src/feature/control/control_events.c \ src/feature/control/control_fmt.c \ src/feature/control/control_getinfo.c \ + src/feature/control/control_proto.c \ src/feature/control/fmt_serverstatus.c \ src/feature/control/getinfo_geoip.c \ src/feature/dirauth/keypin.c \ @@ -307,6 +308,7 @@ noinst_HEADERS += \ src/feature/control/control_events.h \ src/feature/control/control_fmt.h \ src/feature/control/control_getinfo.h \ + src/feature/control/control_proto.h \ src/feature/control/fmt_serverstatus.h \ src/feature/control/getinfo_geoip.h \ src/feature/dirauth/authmode.h \ diff --git a/src/feature/control/control.c b/src/feature/control/control.c index 23ef83ef95..1f85bd3fc0 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -47,7 +47,7 @@ #include "feature/control/control_auth.h" #include "feature/control/control_cmd.h" #include "feature/control/control_events.h" -#include "feature/control/control_fmt.h" +#include "feature/control/control_proto.h" #include "feature/rend/rendcommon.h" #include "feature/rend/rendservice.h" #include "lib/evloop/procmon.h" diff --git a/src/feature/control/control_auth.c b/src/feature/control/control_auth.c index a86442c21f..bdabdc9896 100644 --- a/src/feature/control/control_auth.c +++ b/src/feature/control/control_auth.c @@ -15,7 +15,7 @@ #include "feature/control/control_auth.h" #include "feature/control/control_cmd_args_st.h" #include "feature/control/control_connection_st.h" -#include "feature/control/control_fmt.h" +#include "feature/control/control_proto.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_util.h" #include "lib/encoding/confline.h" diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index 9afa734d86..d69177f4cd 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -27,8 +27,8 @@ #include "feature/control/control_auth.h" #include "feature/control/control_cmd.h" #include "feature/control/control_events.h" -#include "feature/control/control_fmt.h" #include "feature/control/control_getinfo.h" +#include "feature/control/control_proto.h" #include "feature/hs/hs_control.h" #include "feature/nodelist/nodelist.h" #include "feature/nodelist/routerinfo.h" diff --git a/src/feature/control/control_events.c b/src/feature/control/control_events.c index 129776f49f..e596a8aee2 100644 --- a/src/feature/control/control_events.c +++ b/src/feature/control/control_events.c @@ -24,6 +24,7 @@ #include "feature/control/control.h" #include "feature/control/control_events.h" #include "feature/control/control_fmt.h" +#include "feature/control/control_proto.h" #include "feature/dircommon/directory.h" #include "feature/nodelist/networkstatus.h" #include "feature/nodelist/nodelist.h" diff --git a/src/feature/control/control_fmt.c b/src/feature/control/control_fmt.c index dac3f87795..e0e77eb2d0 100644 --- a/src/feature/control/control_fmt.c +++ b/src/feature/control/control_fmt.c @@ -14,6 +14,7 @@ #include "core/or/circuitlist.h" #include "core/or/connection_edge.h" #include "feature/control/control_fmt.h" +#include "feature/control/control_proto.h" #include "feature/nodelist/nodelist.h" #include "core/or/cpath_build_state_st.h" @@ -23,39 +24,6 @@ #include "core/or/socks_request_st.h" #include "feature/control/control_connection_st.h" -/** Append a NUL-terminated string s to the end of - * conn-\>outbuf. - */ -void -connection_write_str_to_buf(const char *s, control_connection_t *conn) -{ - size_t len = strlen(s); - connection_buf_add(s, len, TO_CONN(conn)); -} - -/** Acts like sprintf, but writes its formatted string to the end of - * conn-\>outbuf. */ -void -connection_printf_to_buf(control_connection_t *conn, const char *format, ...) -{ - va_list ap; - char *buf = NULL; - int len; - - va_start(ap,format); - len = tor_vasprintf(&buf, format, ap); - va_end(ap); - - if (len < 0) { - log_err(LD_BUG, "Unable to format string for controller."); - tor_assert(0); - } - - connection_buf_add(buf, (size_t)len, TO_CONN(conn)); - - tor_free(buf); -} - /** Given an AP connection conn and a len-character buffer * buf, determine the address:port combination requested on * conn, and write it to buf. Return 0 on success, -1 on @@ -197,114 +165,6 @@ circuit_describe_status_for_controller(origin_circuit_t *circ) return rv; } -/** Given a len-character string in data, made of lines - * terminated by CRLF, allocate a new string in *out, and copy the - * contents of data into *out, adding a period before any period - * that appears at the start of a line, and adding a period-CRLF line at - * the end. Replace all LF characters sequences with CRLF. Return the number - * of bytes in *out. - */ -size_t -write_escaped_data(const char *data, size_t len, char **out) -{ - tor_assert(len < SIZE_MAX - 9); - size_t sz_out = len+8+1; - char *outp; - const char *start = data, *end; - size_t i; - int start_of_line; - for (i=0; i < len; ++i) { - if (data[i] == '\n') { - sz_out += 2; /* Maybe add a CR; maybe add a dot. */ - if (sz_out >= SIZE_T_CEILING) { - log_warn(LD_BUG, "Input to write_escaped_data was too long"); - *out = tor_strdup(".\r\n"); - return 3; - } - } - } - *out = outp = tor_malloc(sz_out); - end = data+len; - start_of_line = 1; - while (data < end) { - if (*data == '\n') { - if (data > start && data[-1] != '\r') - *outp++ = '\r'; - start_of_line = 1; - } else if (*data == '.') { - if (start_of_line) { - start_of_line = 0; - *outp++ = '.'; - } - } else { - start_of_line = 0; - } - *outp++ = *data++; - } - if (outp < *out+2 || fast_memcmp(outp-2, "\r\n", 2)) { - *outp++ = '\r'; - *outp++ = '\n'; - } - *outp++ = '.'; - *outp++ = '\r'; - *outp++ = '\n'; - *outp = '\0'; /* NUL-terminate just in case. */ - tor_assert(outp >= *out); - tor_assert((size_t)(outp - *out) <= sz_out); - return outp - *out; -} - -/** Given a len-character string in data, made of lines - * terminated by CRLF, allocate a new string in *out, and copy - * the contents of data into *out, removing any period - * that appears at the start of a line, and replacing all CRLF sequences - * with LF. Return the number of - * bytes in *out. */ -size_t -read_escaped_data(const char *data, size_t len, char **out) -{ - char *outp; - const char *next; - const char *end; - - *out = outp = tor_malloc(len+1); - - end = data+len; - - while (data < end) { - /* we're at the start of a line. */ - if (*data == '.') - ++data; - next = memchr(data, '\n', end-data); - if (next) { - size_t n_to_copy = next-data; - /* Don't copy a CR that precedes this LF. */ - if (n_to_copy && *(next-1) == '\r') - --n_to_copy; - memcpy(outp, data, n_to_copy); - outp += n_to_copy; - data = next+1; /* This will point at the start of the next line, - * or the end of the string, or a period. */ - } else { - memcpy(outp, data, end-data); - outp += (end-data); - *outp = '\0'; - return outp - *out; - } - *outp++ = '\n'; - } - - *outp = '\0'; - return outp - *out; -} - -/** Send a "DONE" message down the control connection conn. */ -void -send_control_done(control_connection_t *conn) -{ - connection_write_str_to_buf("250 OK\r\n", conn); -} - /** Return a longname the node whose identity is id_digest. If * node_get_by_id() returns NULL, base 16 encoding of id_digest is * returned instead. diff --git a/src/feature/control/control_fmt.h b/src/feature/control/control_fmt.h index 8bbbaa95d0..6446e37079 100644 --- a/src/feature/control/control_fmt.h +++ b/src/feature/control/control_fmt.h @@ -12,21 +12,12 @@ #ifndef TOR_CONTROL_FMT_H #define TOR_CONTROL_FMT_H -void connection_write_str_to_buf(const char *s, control_connection_t *conn); -void connection_printf_to_buf(control_connection_t *conn, - const char *format, ...) - CHECK_PRINTF(2,3); - int write_stream_target_to_buf(entry_connection_t *conn, char *buf, size_t len); void orconn_target_get_name(char *buf, size_t len, or_connection_t *conn); char *circuit_describe_status_for_controller(origin_circuit_t *circ); -size_t write_escaped_data(const char *data, size_t len, char **out); -size_t read_escaped_data(const char *data, size_t len, char **out); -void send_control_done(control_connection_t *conn); - MOCK_DECL(const char *, node_describe_longname_by_id,(const char *id_digest)); #endif /* !defined(TOR_CONTROL_FMT_H) */ diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c index 5c6a0d4aa2..6f5e69dc0b 100644 --- a/src/feature/control/control_getinfo.c +++ b/src/feature/control/control_getinfo.c @@ -28,6 +28,7 @@ #include "feature/control/control_events.h" #include "feature/control/control_fmt.h" #include "feature/control/control_getinfo.h" +#include "feature/control/control_proto.h" #include "feature/control/fmt_serverstatus.h" #include "feature/control/getinfo_geoip.h" #include "feature/dircache/dirserv.h" diff --git a/src/feature/control/control_proto.c b/src/feature/control/control_proto.c new file mode 100644 index 0000000000..daf5b1189e --- /dev/null +++ b/src/feature/control/control_proto.c @@ -0,0 +1,165 @@ +/* Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file control_proto.c + * \brief Formatting functions for controller data. + */ + +#include "core/or/or.h" + +#include "core/mainloop/connection.h" +#include "core/or/circuitbuild.h" +#include "core/or/circuitlist.h" +#include "core/or/connection_edge.h" +#include "feature/control/control_proto.h" +#include "feature/nodelist/nodelist.h" + +#include "core/or/cpath_build_state_st.h" +#include "core/or/entry_connection_st.h" +#include "core/or/or_connection_st.h" +#include "core/or/origin_circuit_st.h" +#include "core/or/socks_request_st.h" +#include "feature/control/control_connection_st.h" + +/** Append a NUL-terminated string s to the end of + * conn-\>outbuf. + */ +void +connection_write_str_to_buf(const char *s, control_connection_t *conn) +{ + size_t len = strlen(s); + connection_buf_add(s, len, TO_CONN(conn)); +} + +/** Acts like sprintf, but writes its formatted string to the end of + * conn-\>outbuf. */ +void +connection_printf_to_buf(control_connection_t *conn, const char *format, ...) +{ + va_list ap; + char *buf = NULL; + int len; + + va_start(ap,format); + len = tor_vasprintf(&buf, format, ap); + va_end(ap); + + if (len < 0) { + log_err(LD_BUG, "Unable to format string for controller."); + tor_assert(0); + } + + connection_buf_add(buf, (size_t)len, TO_CONN(conn)); + + tor_free(buf); +} + +/** Given a len-character string in data, made of lines + * terminated by CRLF, allocate a new string in *out, and copy the + * contents of data into *out, adding a period before any period + * that appears at the start of a line, and adding a period-CRLF line at + * the end. Replace all LF characters sequences with CRLF. Return the number + * of bytes in *out. + */ +size_t +write_escaped_data(const char *data, size_t len, char **out) +{ + tor_assert(len < SIZE_MAX - 9); + size_t sz_out = len+8+1; + char *outp; + const char *start = data, *end; + size_t i; + int start_of_line; + for (i=0; i < len; ++i) { + if (data[i] == '\n') { + sz_out += 2; /* Maybe add a CR; maybe add a dot. */ + if (sz_out >= SIZE_T_CEILING) { + log_warn(LD_BUG, "Input to write_escaped_data was too long"); + *out = tor_strdup(".\r\n"); + return 3; + } + } + } + *out = outp = tor_malloc(sz_out); + end = data+len; + start_of_line = 1; + while (data < end) { + if (*data == '\n') { + if (data > start && data[-1] != '\r') + *outp++ = '\r'; + start_of_line = 1; + } else if (*data == '.') { + if (start_of_line) { + start_of_line = 0; + *outp++ = '.'; + } + } else { + start_of_line = 0; + } + *outp++ = *data++; + } + if (outp < *out+2 || fast_memcmp(outp-2, "\r\n", 2)) { + *outp++ = '\r'; + *outp++ = '\n'; + } + *outp++ = '.'; + *outp++ = '\r'; + *outp++ = '\n'; + *outp = '\0'; /* NUL-terminate just in case. */ + tor_assert(outp >= *out); + tor_assert((size_t)(outp - *out) <= sz_out); + return outp - *out; +} + +/** Given a len-character string in data, made of lines + * terminated by CRLF, allocate a new string in *out, and copy + * the contents of data into *out, removing any period + * that appears at the start of a line, and replacing all CRLF sequences + * with LF. Return the number of + * bytes in *out. */ +size_t +read_escaped_data(const char *data, size_t len, char **out) +{ + char *outp; + const char *next; + const char *end; + + *out = outp = tor_malloc(len+1); + + end = data+len; + + while (data < end) { + /* we're at the start of a line. */ + if (*data == '.') + ++data; + next = memchr(data, '\n', end-data); + if (next) { + size_t n_to_copy = next-data; + /* Don't copy a CR that precedes this LF. */ + if (n_to_copy && *(next-1) == '\r') + --n_to_copy; + memcpy(outp, data, n_to_copy); + outp += n_to_copy; + data = next+1; /* This will point at the start of the next line, + * or the end of the string, or a period. */ + } else { + memcpy(outp, data, end-data); + outp += (end-data); + *outp = '\0'; + return outp - *out; + } + *outp++ = '\n'; + } + + *outp = '\0'; + return outp - *out; +} + +/** Send a "DONE" message down the control connection conn. */ +void +send_control_done(control_connection_t *conn) +{ + connection_write_str_to_buf("250 OK\r\n", conn); +} diff --git a/src/feature/control/control_proto.h b/src/feature/control/control_proto.h new file mode 100644 index 0000000000..5720b22601 --- /dev/null +++ b/src/feature/control/control_proto.h @@ -0,0 +1,24 @@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file control_proto.h + * \brief Header file for control_proto.c. + **/ + +#ifndef TOR_CONTROL_PROTO_H +#define TOR_CONTROL_PROTO_H + +void connection_write_str_to_buf(const char *s, control_connection_t *conn); +void connection_printf_to_buf(control_connection_t *conn, + const char *format, ...) + CHECK_PRINTF(2,3); + +size_t write_escaped_data(const char *data, size_t len, char **out); +size_t read_escaped_data(const char *data, size_t len, char **out); +void send_control_done(control_connection_t *conn); + +#endif /* !defined(TOR_CONTROL_PROTO_H) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 61e41e9a9b..88eda847a2 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -15,7 +15,7 @@ #include "lib/buf/buffers.h" #include "app/config/config.h" #include "feature/control/control.h" -#include "feature/control/control_fmt.h" +#include "feature/control/control_proto.h" #include "feature/client/transports.h" #include "lib/crypt_ops/crypto_format.h" #include "lib/crypt_ops/crypto_rand.h" From 482437754a7a30b651f096d4432a42aa61bd3b17 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Mon, 8 Apr 2019 13:39:04 -0500 Subject: [PATCH 3/9] Add clarifying comments to control_proto.c Refer to control-spec.txt grammar productions in comments in control_proto.c for clarity. --- src/feature/control/control_proto.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/feature/control/control_proto.c b/src/feature/control/control_proto.c index daf5b1189e..4756a02632 100644 --- a/src/feature/control/control_proto.c +++ b/src/feature/control/control_proto.c @@ -62,6 +62,8 @@ connection_printf_to_buf(control_connection_t *conn, const char *format, ...) * that appears at the start of a line, and adding a period-CRLF line at * the end. Replace all LF characters sequences with CRLF. Return the number * of bytes in *out. + * + * This corresponds to CmdData in control-spec.txt. */ size_t write_escaped_data(const char *data, size_t len, char **out) @@ -118,7 +120,10 @@ write_escaped_data(const char *data, size_t len, char **out) * the contents of data into *out, removing any period * that appears at the start of a line, and replacing all CRLF sequences * with LF. Return the number of - * bytes in *out. */ + * bytes in *out. + * + * This corresponds to CmdData in control-spec.txt. + */ size_t read_escaped_data(const char *data, size_t len, char **out) { From 61976a4b1ca5dc9c7f9494ee3bf17a96c1182cf0 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Tue, 9 Apr 2019 12:22:31 -0500 Subject: [PATCH 4/9] Factor out control reply output Create a set of abstractions for controller commands and events to output replies to the control channel. The control protocol has a relatively consistent SMTP-like structure, so it's helpful when code that implements control commands and events doesn't explicitly format everything on its own. --- src/feature/control/control_proto.c | 106 ++++++++++++++++++++++++++++ src/feature/control/control_proto.h | 24 +++++++ 2 files changed, 130 insertions(+) diff --git a/src/feature/control/control_proto.c b/src/feature/control/control_proto.c index 4756a02632..7bd237de94 100644 --- a/src/feature/control/control_proto.c +++ b/src/feature/control/control_proto.c @@ -168,3 +168,109 @@ send_control_done(control_connection_t *conn) { connection_write_str_to_buf("250 OK\r\n", conn); } + +/** Write a reply to the control channel. + * + * @param conn control connection + * @param code numeric result code + * @param c separator character, usually ' ', '-', or '+' + * @param s string + */ +void +control_write_reply(control_connection_t *conn, int code, int c, const char *s) +{ + connection_printf_to_buf(conn, "%03d%c%s\r\n", code, c, s); +} + +/** Write a formatted reply to the control channel. + * + * @param conn control connection + * @param code numeric result code + * @param c separator character, usually ' ', '-', or '+' + * @param fmt format string + * @param ap va_list from caller + */ +void +control_vprintf_reply(control_connection_t *conn, int code, int c, + const char *fmt, va_list ap) +{ + char *buf = NULL; + int len; + + len = tor_vasprintf(&buf, fmt, ap); + if (len < 0) { + log_err(LD_BUG, "Unable to format string for controller."); + tor_assert(0); + } + control_write_reply(conn, code, c, buf); + tor_free(buf); +} + +/** Write an EndReplyLine */ +void +control_write_endreply(control_connection_t *conn, int code, const char *s) +{ + control_write_reply(conn, code, ' ', s); +} + +/** Write a formatted EndReplyLine */ +void +control_printf_endreply(control_connection_t *conn, int code, + const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + control_vprintf_reply(conn, code, ' ', fmt, ap); + va_end(ap); +} + +/** Write a MidReplyLine */ +void +control_write_midreply(control_connection_t *conn, int code, const char *s) +{ + control_write_reply(conn, code, '-', s); +} + +/** Write a formatted MidReplyLine */ +void +control_printf_midreply(control_connection_t *conn, int code, const char *fmt, + ...) +{ + va_list ap; + + va_start(ap, fmt); + control_vprintf_reply(conn, code, '-', fmt, ap); + va_end(ap); +} + +/** Write a DataReplyLine */ +void +control_write_datareply(control_connection_t *conn, int code, const char *s) +{ + control_write_reply(conn, code, '+', s); +} + +/** Write a formatted DataReplyLine */ +void +control_printf_datareply(control_connection_t *conn, int code, const char *fmt, + ...) +{ + va_list ap; + + va_start(ap, fmt); + control_vprintf_reply(conn, code, '+', fmt, ap); + va_end(ap); +} + +/** Write a CmdData */ +void +control_write_data(control_connection_t *conn, const char *data) +{ + char *esc = NULL; + size_t esc_len; + + esc_len = write_escaped_data(data, strlen(data), &esc); + connection_buf_add(esc, esc_len, TO_CONN(conn)); + tor_free(esc); +} diff --git a/src/feature/control/control_proto.h b/src/feature/control/control_proto.h index 5720b22601..101b808d88 100644 --- a/src/feature/control/control_proto.h +++ b/src/feature/control/control_proto.h @@ -21,4 +21,28 @@ size_t write_escaped_data(const char *data, size_t len, char **out); size_t read_escaped_data(const char *data, size_t len, char **out); void send_control_done(control_connection_t *conn); +void control_write_reply(control_connection_t *conn, int code, int c, + const char *s); +void control_vprintf_reply(control_connection_t *conn, int code, int c, + const char *fmt, va_list ap) + CHECK_PRINTF(4, 0); +void control_write_endreply(control_connection_t *conn, int code, + const char *s); +void control_printf_endreply(control_connection_t *conn, int code, + const char *fmt, ...) + CHECK_PRINTF(3, 4); +void control_write_midreply(control_connection_t *conn, int code, + const char *s); +void control_printf_midreply(control_connection_t *conn, int code, + const char *fmt, + ...) + CHECK_PRINTF(3, 4); +void control_write_datareply(control_connection_t *conn, int code, + const char *s); +void control_printf_datareply(control_connection_t *conn, int code, + const char *fmt, + ...) + CHECK_PRINTF(3, 4); +void control_write_data(control_connection_t *conn, const char *data); + #endif /* !defined(TOR_CONTROL_PROTO_H) */ From 769eb07a7a3bd93091ca1677564d3de62e3c5c2c Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 10 Apr 2019 15:11:36 -0500 Subject: [PATCH 5/9] Manually fix some control replies Manually fix up some reply-generating code that the Coccinelle scripts won't match. Some more complicated ones remain -- these are mostly ones that accumulate data to send, and then call connection_buf_add() or connection_write_str_to_buf() directly. --- src/feature/control/control_cmd.c | 53 +++++++++++++-------------- src/feature/control/control_getinfo.c | 12 ++---- 2 files changed, 28 insertions(+), 37 deletions(-) diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index d69177f4cd..5aa828ca53 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -355,7 +355,6 @@ handle_control_loadconf(control_connection_t *conn, { setopt_err_t retval; char *errstring = NULL; - const char *msg = NULL; retval = options_init_from_string(NULL, args->cmddata, CMD_RUN_TOR, NULL, &errstring); @@ -365,31 +364,29 @@ handle_control_loadconf(control_connection_t *conn, "Controller gave us config file that didn't validate: %s", errstring); +#define SEND_ERRMSG(code, msg) \ + control_printf_endreply(conn, code, msg "%s%s", \ + errstring ? ": " : "", \ + errstring ? errstring : "") switch (retval) { case SETOPT_ERR_PARSE: - msg = "552 Invalid config file"; + SEND_ERRMSG(552, "Invalid config file"); break; case SETOPT_ERR_TRANSITION: - msg = "553 Transition not allowed"; + SEND_ERRMSG(553, "Transition not allowed"); break; case SETOPT_ERR_SETTING: - msg = "553 Unable to set option"; + SEND_ERRMSG(553, "Unable to set option"); break; case SETOPT_ERR_MISC: default: - msg = "550 Unable to load config"; + SEND_ERRMSG(550, "Unable to load config"); break; case SETOPT_OK: + send_control_done(conn); break; } - if (msg) { - if (errstring) - connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring); - else - connection_printf_to_buf(conn, "%s\r\n", msg); - } else { - send_control_done(conn); - } +#undef SEND_ERRMSG tor_free(errstring); return 0; } @@ -600,30 +597,32 @@ control_setconf_helper(control_connection_t *conn, opt_err = options_trial_assign(lines, flags, &errstring); { - const char *msg; +#define SEND_ERRMSG(code, msg) \ + control_printf_endreply(conn, code, msg ": %s", errstring); + switch (opt_err) { case SETOPT_ERR_MISC: - msg = "552 Unrecognized option"; + SEND_ERRMSG(552, "Unrecognized option"); break; case SETOPT_ERR_PARSE: - msg = "513 Unacceptable option value"; + SEND_ERRMSG(513, "Unacceptable option value"); break; case SETOPT_ERR_TRANSITION: - msg = "553 Transition not allowed"; + SEND_ERRMSG(553, "Transition not allowed"); break; case SETOPT_ERR_SETTING: default: - msg = "553 Unable to set option"; + SEND_ERRMSG(553, "Unable to set option"); break; case SETOPT_OK: config_free_lines(lines); send_control_done(conn); return 0; } +#undef SEND_ERRMSG log_warn(LD_CONTROL, "Controller gave us config lines that didn't validate: %s", errstring); - connection_printf_to_buf(conn, "%s: %s\r\n", msg, errstring); config_free_lines(lines); tor_free(errstring); return 0; @@ -1309,15 +1308,13 @@ handle_control_protocolinfo(control_connection_t *conn, smartlist_free(mlist); } - connection_printf_to_buf(conn, - "250-PROTOCOLINFO 1\r\n" - "250-AUTH METHODS=%s%s%s\r\n" - "250-VERSION Tor=%s\r\n" - "250 OK\r\n", - methods, - cookies?" COOKIEFILE=":"", - cookies?esc_cfile:"", - escaped(VERSION)); + control_write_midreply(conn, 250, "PROTOCOLINFO 1"); + control_printf_midreply(conn, 250, "AUTH METHODS=%s%s%s", methods, + cookies?" COOKIEFILE=":"", + cookies?esc_cfile:""); + control_printf_midreply(conn, 250, "VERSION Tor=%s", escaped(VERSION)); + send_control_done(conn); + tor_free(methods); tor_free(cfile); tor_free(abs_cfile); diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c index 6f5e69dc0b..c10229bfbb 100644 --- a/src/feature/control/control_getinfo.c +++ b/src/feature/control/control_getinfo.c @@ -1639,16 +1639,10 @@ handle_control_getinfo(control_connection_t *conn, char *k = smartlist_get(answers, i); char *v = smartlist_get(answers, i+1); if (!strchr(v, '\n') && !strchr(v, '\r')) { - connection_printf_to_buf(conn, "250-%s=", k); - connection_write_str_to_buf(v, conn); - connection_write_str_to_buf("\r\n", conn); + control_printf_midreply(conn, 250, "%s=%s", k, v); } else { - char *esc = NULL; - size_t esc_len; - esc_len = write_escaped_data(v, strlen(v), &esc); - connection_printf_to_buf(conn, "250+%s=\r\n", k); - connection_buf_add(esc, esc_len, TO_CONN(conn)); - tor_free(esc); + control_printf_datareply(conn, 250, "%s=", k); + control_write_data(conn, v); } } connection_write_str_to_buf("250 OK\r\n", conn); From 58ec88e8060508a253bbe8ba5c19b020a2629ea0 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 10 Apr 2019 12:27:50 -0500 Subject: [PATCH 6/9] Coccinelle scripts for control.c refactor --- scripts/coccinelle/ctrl-reply-cleanup.cocci | 43 ++++++++++ scripts/coccinelle/ctrl-reply.cocci | 87 +++++++++++++++++++++ scripts/coccinelle/tor-coccinelle.h | 3 + 3 files changed, 133 insertions(+) create mode 100644 scripts/coccinelle/ctrl-reply-cleanup.cocci create mode 100644 scripts/coccinelle/ctrl-reply.cocci create mode 100644 scripts/coccinelle/tor-coccinelle.h diff --git a/scripts/coccinelle/ctrl-reply-cleanup.cocci b/scripts/coccinelle/ctrl-reply-cleanup.cocci new file mode 100644 index 0000000000..f085cd4684 --- /dev/null +++ b/scripts/coccinelle/ctrl-reply-cleanup.cocci @@ -0,0 +1,43 @@ +// Script to clean up after ctrl-reply.cocci -- run as a separate step +// because cleanup_write2 (even when disabled) somehow prevents the +// match rule in ctrl-reply.cocci from matching. + +// If it doesn't have to be a printf, turn it into a write + +@ cleanup_write @ +expression E; +constant code, s; +@@ +-control_printf_endreply(E, code, s) ++control_write_endreply(E, code, s) + +// Use send_control_done() instead of explicitly writing it out +@ cleanup_send_done @ +type T; +identifier f != send_control_done; +expression E; +@@ + T f(...) { +<... +-control_write_endreply(E, 250, "OK") ++send_control_done(E) + ...> + } + +// Clean up more printfs that could be writes +// +// For some reason, including this rule, even disabled, causes the +// match rule in ctrl-reply.cocci to fail to match some code that has +// %s in its format strings + +@ cleanup_write2 @ +expression E1, E2; +constant code; +@@ +( +-control_printf_endreply(E1, code, "%s", E2) ++control_write_endreply(E1, code, E2) +| +-control_printf_midreply(E1, code, "%s", E2) ++control_write_midreply(E1, code, E2) +) diff --git a/scripts/coccinelle/ctrl-reply.cocci b/scripts/coccinelle/ctrl-reply.cocci new file mode 100644 index 0000000000..d6e9aeedd7 --- /dev/null +++ b/scripts/coccinelle/ctrl-reply.cocci @@ -0,0 +1,87 @@ +// Script to edit control_*.c for refactored control reply output functions + +@ initialize:python @ +@@ +import re +from coccilib.report import * + +# reply strings "NNN-foo", "NNN+foo", "NNN foo", etc. +r = re.compile(r'^"(\d+)([ +-])(.*)\\r\\n"$') + +# Generate name of function to call based on which separator character +# comes between the numeric code and the text +def idname(sep, base): + if sep == '+': + return base + "datareply" + elif sep == '-': + return base + "midreply" + else: + return base + "endreply" + +# Generate the actual replacements used by the rules +def gen(s, base, p): + pos = p[0] + print_report(pos, "%s %s" % (base, s)) + m = r.match(s) + if m is None: + # String not correct format, so fail match + cocci.include_match(False) + print_report(pos, "BAD STRING %s" % s) + return + + code, sep, s1 = m.groups() + + if r'\r\n' in s1: + # Extra CRLF in string, so fail match + cocci.include_match(False) + print_report(pos, "extra CRLF in string %s" % s) + return + + coccinelle.code = code + # Need a string that is a single C token, because Coccinelle only allows + # "identifiers" to be output from Python scripts? + coccinelle.body = '"%s"' % s1 + coccinelle.id = idname(sep, base) + return + +@ match @ +identifier f; +position p; +expression E; +constant s; +@@ +( + connection_printf_to_buf@f@p(E, s, ...) +| + connection_write_str_to_buf@f@p(s, E) +) + +@ script:python sc1 @ +s << match.s; +p << match.p; +f << match.f; +id; +body; +code; +@@ +if f == 'connection_printf_to_buf': + gen(s, 'control_printf_', p) +elif f == 'connection_write_str_to_buf': + gen(s, 'control_write_', p) +else: + raise(ValueError("%s: %s" % (f, s))) + +@ replace @ +constant match.s; +expression match.E; +identifier match.f; +identifier sc1.body, sc1.id, sc1.code; +@@ +( +-connection_write_str_to_buf@f(s, E) ++id(E, code, body) +| +-connection_printf_to_buf@f(E, s ++id(E, code, body + , ...) +) diff --git a/scripts/coccinelle/tor-coccinelle.h b/scripts/coccinelle/tor-coccinelle.h new file mode 100644 index 0000000000..8f625dcee4 --- /dev/null +++ b/scripts/coccinelle/tor-coccinelle.h @@ -0,0 +1,3 @@ +#define MOCK_IMPL(a, b, c) a b c +#define CHECK_PRINTF(a, b) +#define STATIC static From 983452e2215d27033ed91012a53c1b9ad3dd4a34 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 10 Apr 2019 17:04:09 -0500 Subject: [PATCH 7/9] Run Coccinelle for control.c refactor --- src/feature/control/control.c | 8 +- src/feature/control/control_auth.c | 38 +++--- src/feature/control/control_cmd.c | 166 +++++++++++++------------- src/feature/control/control_getinfo.c | 13 +- src/feature/control/control_proto.c | 2 +- 5 files changed, 106 insertions(+), 121 deletions(-) diff --git a/src/feature/control/control.c b/src/feature/control/control.c index 1f85bd3fc0..436bf423cf 100644 --- a/src/feature/control/control.c +++ b/src/feature/control/control.c @@ -401,7 +401,7 @@ connection_control_process_inbuf(control_connection_t *conn) return 0; else if (r == -1) { if (data_len + conn->incoming_cmd_cur_len > MAX_COMMAND_LINE_LENGTH) { - connection_write_str_to_buf("500 Line too long.\r\n", conn); + control_write_endreply(conn, 500, "Line too long."); connection_stop_reading(TO_CONN(conn)); connection_mark_and_flush(TO_CONN(conn)); } @@ -455,20 +455,20 @@ connection_control_process_inbuf(control_connection_t *conn) /* Otherwise, Quit is always valid. */ if (!strcasecmp(conn->current_cmd, "QUIT")) { - connection_write_str_to_buf("250 closing connection\r\n", conn); + control_write_endreply(conn, 250, "closing connection"); connection_mark_and_flush(TO_CONN(conn)); return 0; } if (conn->base_.state == CONTROL_CONN_STATE_NEEDAUTH && !is_valid_initial_command(conn, conn->current_cmd)) { - connection_write_str_to_buf("514 Authentication required.\r\n", conn); + control_write_endreply(conn, 514, "Authentication required."); connection_mark_for_close(TO_CONN(conn)); return 0; } if (data_len >= UINT32_MAX) { - connection_write_str_to_buf("500 A 4GB command? Nice try.\r\n", conn); + control_write_endreply(conn, 500, "A 4GB command? Nice try."); connection_mark_for_close(TO_CONN(conn)); return 0; } diff --git a/src/feature/control/control_auth.c b/src/feature/control/control_auth.c index bdabdc9896..51c8da1b99 100644 --- a/src/feature/control/control_auth.c +++ b/src/feature/control/control_auth.c @@ -141,27 +141,26 @@ handle_control_authchallenge(control_connection_t *conn, char server_nonce_encoded[(2*SAFECOOKIE_SERVER_NONCE_LEN) + 1]; if (strcasecmp(smartlist_get(args->args, 0), "SAFECOOKIE")) { - connection_write_str_to_buf("513 AUTHCHALLENGE only supports SAFECOOKIE " - "authentication\r\n", conn); + control_write_endreply(conn, 513, + "AUTHCHALLENGE only supports SAFECOOKIE " "authentication"); goto fail; } if (!authentication_cookie_is_set) { - connection_write_str_to_buf("515 Cookie authentication is disabled\r\n", - conn); + control_write_endreply(conn, 515, "Cookie authentication is disabled"); goto fail; } if (args->kwargs == NULL || args->kwargs->next != NULL) { /* connection_write_str_to_buf("512 AUTHCHALLENGE requires exactly " "2 arguments.\r\n", conn); */ - connection_printf_to_buf(conn, - "512 AUTHCHALLENGE dislikes argument list %s\r\n", + control_printf_endreply(conn, 512, + "AUTHCHALLENGE dislikes argument list %s", escaped(args->raw_body)); goto fail; } if (strcmp(args->kwargs->key, "")) { - connection_write_str_to_buf("512 AUTHCHALLENGE does not accept keyword " - "arguments.\r\n", conn); + control_write_endreply(conn, 512, + "AUTHCHALLENGE does not accept keyword " "arguments."); goto fail; } @@ -177,8 +176,7 @@ handle_control_authchallenge(control_connection_t *conn, client_nonce = tor_malloc(client_nonce_len); if (base16_decode(client_nonce, client_nonce_len, hex_nonce, strlen(hex_nonce)) != (int)client_nonce_len) { - connection_write_str_to_buf("513 Invalid base16 client nonce\r\n", - conn); + control_write_endreply(conn, 513, "Invalid base16 client nonce"); tor_free(client_nonce); goto fail; } @@ -223,9 +221,8 @@ handle_control_authchallenge(control_connection_t *conn, base16_encode(server_nonce_encoded, sizeof(server_nonce_encoded), server_nonce, sizeof(server_nonce)); - connection_printf_to_buf(conn, - "250 AUTHCHALLENGE SERVERHASH=%s " - "SERVERNONCE=%s\r\n", + control_printf_endreply(conn, 250, + "AUTHCHALLENGE SERVERHASH=%s " "SERVERNONCE=%s", server_hash_encoded, server_nonce_encoded); @@ -263,13 +260,12 @@ handle_control_authenticate(control_connection_t *conn, password = tor_strdup(""); password_len = 0; } else if (args->kwargs->next) { - connection_write_str_to_buf( - "512 Too many arguments to AUTHENTICATE.\r\n", conn); + control_write_endreply(conn, 512, "Too many arguments to AUTHENTICATE."); connection_mark_for_close(TO_CONN(conn)); return 0; } else if (strcmp(args->kwargs->key, "")) { - connection_write_str_to_buf( - "512 AUTHENTICATE does not accept keyword arguments.\r\n", conn); + control_write_endreply(conn, 512, + "AUTHENTICATE does not accept keyword arguments."); connection_mark_for_close(TO_CONN(conn)); return 0; } else if (strchr(args->raw_body, '\"')) { @@ -282,10 +278,8 @@ handle_control_authenticate(control_connection_t *conn, password = tor_malloc(password_len+1); if (base16_decode(password, password_len+1, hex_passwd, strlen(hex_passwd)) != (int) password_len) { - connection_write_str_to_buf( - "551 Invalid hexadecimal encoding. Maybe you tried a plain text " - "password? If so, the standard requires that you put it in " - "double quotes.\r\n", conn); + control_write_endreply(conn, 551, + "Invalid hexadecimal encoding. Maybe you tried a plain text " "password? If so, the standard requires that you put it in " "double quotes."); connection_mark_for_close(TO_CONN(conn)); tor_free(password); return 0; @@ -418,7 +412,7 @@ handle_control_authenticate(control_connection_t *conn, err: tor_free(password); - connection_printf_to_buf(conn, "515 Authentication failed: %s\r\n", errstr); + control_printf_endreply(conn, 515, "Authentication failed: %s", errstr); connection_mark_for_close(TO_CONN(conn)); if (sl) { /* clean up */ SMARTLIST_FOREACH(sl, char *, str, tor_free(str)); diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index 5aa828ca53..ae4d6584cc 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -319,11 +319,11 @@ handle_control_getconf(control_connection_t *conn, if ((len = smartlist_len(unrecognized))) { for (i=0; i < len-1; ++i) - connection_printf_to_buf(conn, - "552-Unrecognized configuration key \"%s\"\r\n", + control_printf_midreply(conn, 552, + "Unrecognized configuration key \"%s\"", (char*)smartlist_get(unrecognized, i)); - connection_printf_to_buf(conn, - "552 Unrecognized configuration key \"%s\"\r\n", + control_printf_endreply(conn, 552, + "Unrecognized configuration key \"%s\"", (char*)smartlist_get(unrecognized, len-1)); } else if ((len = smartlist_len(answers))) { char *tmp = smartlist_get(answers, len-1); @@ -332,7 +332,7 @@ handle_control_getconf(control_connection_t *conn, msg = smartlist_join_strings(answers, "", 0, &msg_len); connection_buf_add(msg, msg_len, TO_CONN(conn)); } else { - connection_write_str_to_buf("250 OK\r\n", conn); + send_control_done(conn); } SMARTLIST_FOREACH(answers, char *, cp, tor_free(cp)); @@ -424,7 +424,7 @@ handle_control_setevents(control_connection_t *conn, } if (event_code == -1) { - connection_printf_to_buf(conn, "552 Unrecognized event \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unrecognized event \"%s\"", ev); return 0; } @@ -455,8 +455,8 @@ handle_control_saveconf(control_connection_t *conn, bool force = config_lines_contain_flag(args->kwargs, "FORCE"); const or_options_t *options = get_options(); if ((!force && options->IncludeUsed) || options_save_current() < 0) { - connection_write_str_to_buf( - "551 Unable to write configuration to disk.\r\n", conn); + control_write_endreply(conn, 551, + "Unable to write configuration to disk."); } else { send_control_done(conn); } @@ -489,7 +489,7 @@ handle_control_signal(control_connection_t *conn, } if (sig < 0) - connection_printf_to_buf(conn, "552 Unrecognized signal code \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unrecognized signal code \"%s\"", s); if (sig < 0) return 0; @@ -776,7 +776,7 @@ handle_control_extendcircuit(control_connection_t *conn, if (purpose_line) { intended_purpose = circuit_purpose_from_string(purpose_line->value); if (intended_purpose == CIRCUIT_PURPOSE_UNKNOWN) { - connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unknown purpose \"%s\"", purpose_line->value); goto done; } @@ -787,22 +787,22 @@ handle_control_extendcircuit(control_connection_t *conn, // "EXTENDCIRCUIT 0" with no path. circ = circuit_launch(intended_purpose, CIRCLAUNCH_NEED_CAPACITY); if (!circ) { - connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn); + control_write_endreply(conn, 551, "Couldn't start circuit"); } else { - connection_printf_to_buf(conn, "250 EXTENDED %lu\r\n", - (unsigned long)circ->global_identifier); + control_printf_endreply(conn, 250, "EXTENDED %lu", + (unsigned long)circ->global_identifier); } goto done; } } if (!zero_circ && !(circ = get_circ(circ_id))) { - connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id); + control_printf_endreply(conn, 552, "Unknown circuit \"%s\"", circ_id); goto done; } if (!path_str) { - connection_printf_to_buf(conn, "512 syntax error: path required.\r\n"); + control_write_endreply(conn, 512, "syntax error: path required."); goto done; } @@ -813,11 +813,11 @@ handle_control_extendcircuit(control_connection_t *conn, SMARTLIST_FOREACH_BEGIN(router_nicknames, const char *, n) { const node_t *node = node_get_by_nickname(n, 0); if (!node) { - connection_printf_to_buf(conn, "552 No such router \"%s\"\r\n", n); + control_printf_endreply(conn, 552, "No such router \"%s\"", n); goto done; } if (!node_has_preferred_descriptor(node, first_node)) { - connection_printf_to_buf(conn, "552 No descriptor for \"%s\"\r\n", n); + control_printf_endreply(conn, 552, "No descriptor for \"%s\"", n); goto done; } smartlist_add(nodes, (void*)node); @@ -825,7 +825,7 @@ handle_control_extendcircuit(control_connection_t *conn, } SMARTLIST_FOREACH_END(n); if (!smartlist_len(nodes)) { - connection_write_str_to_buf("512 No router names provided\r\n", conn); + control_write_endreply(conn, 512, "No router names provided"); goto done; } @@ -863,7 +863,7 @@ handle_control_extendcircuit(control_connection_t *conn, int err_reason = 0; if ((err_reason = circuit_handle_first_hop(circ)) < 0) { circuit_mark_for_close(TO_CIRCUIT(circ), -err_reason); - connection_write_str_to_buf("551 Couldn't start circuit\r\n", conn); + control_write_endreply(conn, 551, "Couldn't start circuit"); goto done; } } else { @@ -875,13 +875,13 @@ handle_control_extendcircuit(control_connection_t *conn, log_info(LD_CONTROL, "send_next_onion_skin failed; circuit marked for closing."); circuit_mark_for_close(TO_CIRCUIT(circ), -err_reason); - connection_write_str_to_buf("551 Couldn't send onion skin\r\n", conn); + control_write_endreply(conn, 551, "Couldn't send onion skin"); goto done; } } } - connection_printf_to_buf(conn, "250 EXTENDED %lu\r\n", + control_printf_endreply(conn, 250, "EXTENDED %lu", (unsigned long)circ->global_identifier); if (zero_circ) /* send a 'launched' event, for completeness */ circuit_event_status(circ, CIRC_EVENT_LAUNCHED, 0); @@ -909,26 +909,26 @@ handle_control_setcircuitpurpose(control_connection_t *conn, const char *circ_id = smartlist_get(args->args,0); if (!(circ = get_circ(circ_id))) { - connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id); + control_printf_endreply(conn, 552, "Unknown circuit \"%s\"", circ_id); goto done; } { const config_line_t *purp = config_line_find_case(args->kwargs, "PURPOSE"); if (!purp) { - connection_write_str_to_buf("552 No purpose given\r\n", conn); + control_write_endreply(conn, 552, "No purpose given"); goto done; } new_purpose = circuit_purpose_from_string(purp->value); if (new_purpose == CIRCUIT_PURPOSE_UNKNOWN) { - connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unknown purpose \"%s\"", purp->value); goto done; } } circuit_change_purpose(TO_CIRCUIT(circ), new_purpose); - connection_write_str_to_buf("250 OK\r\n", conn); + send_control_done(conn); done: return 0; @@ -959,17 +959,17 @@ handle_control_attachstream(control_connection_t *conn, const config_line_t *hoparg = config_line_find_case(args->kwargs, "HOP"); if (!(ap_conn = get_stream(stream_id))) { - connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n", stream_id); + control_printf_endreply(conn, 552, "Unknown stream \"%s\"", stream_id); return 0; } else if (!zero_circ && !(circ = get_circ(circ_id))) { - connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id); + control_printf_endreply(conn, 552, "Unknown circuit \"%s\"", circ_id); return 0; } else if (circ) { if (hoparg) { hop = (int) tor_parse_ulong(hoparg->value, 10, 0, INT_MAX, &hop_line_ok, NULL); if (!hop_line_ok) { /* broken hop line */ - connection_printf_to_buf(conn, "552 Bad value hop=%s\r\n", + control_printf_endreply(conn, 552, "Bad value hop=%s", hoparg->value); return 0; } @@ -979,9 +979,8 @@ handle_control_attachstream(control_connection_t *conn, if (ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONTROLLER_WAIT && ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONNECT_WAIT && ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_RESOLVE_WAIT) { - connection_write_str_to_buf( - "555 Connection is not managed by controller.\r\n", - conn); + control_write_endreply(conn, 555, + "Connection is not managed by controller."); return 0; } @@ -1000,15 +999,14 @@ handle_control_attachstream(control_connection_t *conn, } if (circ && (circ->base_.state != CIRCUIT_STATE_OPEN)) { - connection_write_str_to_buf( - "551 Can't attach stream to non-open origin circuit\r\n", - conn); + control_write_endreply(conn, 551, + "Can't attach stream to non-open origin circuit"); return 0; } /* Is this a single hop circuit? */ if (circ && (circuit_get_cpath_len(circ)<2 || hop==1)) { - connection_write_str_to_buf( - "551 Can't attach stream to this one-hop circuit.\r\n", conn); + control_write_endreply(conn, 551, + "Can't attach stream to this one-hop circuit."); return 0; } @@ -1016,13 +1014,12 @@ handle_control_attachstream(control_connection_t *conn, /* find this hop in the circuit, and set cpath */ cpath = circuit_get_cpath_hop(circ, hop); if (!cpath) { - connection_printf_to_buf(conn, - "551 Circuit doesn't have %d hops.\r\n", hop); + control_printf_endreply(conn, 551, "Circuit doesn't have %d hops.", hop); return 0; } } if (connection_ap_handshake_rewrite_and_attach(ap_conn, circ, cpath) < 0) { - connection_write_str_to_buf("551 Unable to attach stream\r\n", conn); + control_write_endreply(conn, 551, "Unable to attach stream"); return 0; } send_control_done(conn); @@ -1054,7 +1051,7 @@ handle_control_postdescriptor(control_connection_t *conn, line = config_line_find_case(args->kwargs, "purpose"); if (line) { purpose = router_purpose_from_string(line->value); - connection_printf_to_buf(conn, "552 Unknown purpose \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unknown purpose \"%s\"", line->value); goto done; } @@ -1065,7 +1062,7 @@ handle_control_postdescriptor(control_connection_t *conn, else if (!strcasecmp(line->value, "yes")) cache = 1; else { - connection_printf_to_buf(conn, "552 Unknown cache request \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unknown cache request \"%s\"", line->value); goto done; } @@ -1074,11 +1071,11 @@ handle_control_postdescriptor(control_connection_t *conn, switch (router_load_single_router(args->cmddata, purpose, cache, &msg)) { case -1: if (!msg) msg = "Could not parse descriptor"; - connection_printf_to_buf(conn, "554 %s\r\n", msg); + control_write_endreply(conn, 554, msg); break; case 0: if (!msg) msg = "Descriptor not added"; - connection_printf_to_buf(conn, "251 %s\r\n",msg); + control_write_endreply(conn, 251, msg); break; case 1: send_control_done(conn); @@ -1107,7 +1104,7 @@ handle_control_redirectstream(control_connection_t *conn, if (!(ap_conn = get_stream(smartlist_get(args, 0))) || !ap_conn->socks_request) { - connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unknown stream \"%s\"", (char*)smartlist_get(args, 0)); } else { int ok = 1; @@ -1116,7 +1113,7 @@ handle_control_redirectstream(control_connection_t *conn, 10, 1, 65535, &ok, NULL); } if (!ok) { - connection_printf_to_buf(conn, "512 Cannot parse port \"%s\"\r\n", + control_printf_endreply(conn, 512, "Cannot parse port \"%s\"", (char*)smartlist_get(args, 2)); } else { new_addr = tor_strdup(smartlist_get(args, 1)); @@ -1155,13 +1152,13 @@ handle_control_closestream(control_connection_t *conn, tor_assert(smartlist_len(args) >= 2); if (!(ap_conn = get_stream(smartlist_get(args, 0)))) - connection_printf_to_buf(conn, "552 Unknown stream \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unknown stream \"%s\"", (char*)smartlist_get(args, 0)); else { reason = (uint8_t) tor_parse_ulong(smartlist_get(args,1), 10, 0, 255, &ok, NULL); if (!ok) { - connection_printf_to_buf(conn, "552 Unrecognized reason \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unrecognized reason \"%s\"", (char*)smartlist_get(args, 1)); ap_conn = NULL; } @@ -1192,7 +1189,7 @@ handle_control_closecircuit(control_connection_t *conn, origin_circuit_t *circ = NULL; if (!(circ=get_circ(circ_id))) { - connection_printf_to_buf(conn, "552 Unknown circuit \"%s\"\r\n", circ_id); + control_printf_endreply(conn, 552, "Unknown circuit \"%s\"", circ_id); return 0; } @@ -1277,7 +1274,7 @@ handle_control_protocolinfo(control_connection_t *conn, } }); if (bad_arg) { - connection_printf_to_buf(conn, "513 No such version %s\r\n", + control_printf_endreply(conn, 513, "No such version %s", escaped(bad_arg)); /* Don't tolerate bad arguments when not authenticated. */ if (!STATE_IS_OPEN(TO_CONN(conn)->state)) @@ -1342,7 +1339,7 @@ handle_control_usefeature(control_connection_t *conn, else if (!strcasecmp(arg, "EXTENDED_EVENTS")) ; else { - connection_printf_to_buf(conn, "552 Unrecognized feature \"%s\"\r\n", + control_printf_endreply(conn, 552, "Unrecognized feature \"%s\"", arg); bad = 1; break; @@ -1426,7 +1423,7 @@ handle_control_hsfetch(control_connection_t *conn, version = HS_VERSION_THREE; hs_parse_address(hsaddress, &v3_pk, NULL, NULL); } else { - connection_printf_to_buf(conn, "513 Invalid argument \"%s\"\r\n", + control_printf_endreply(conn, 513, "Invalid argument \"%s\"", arg1); goto done; } @@ -1437,7 +1434,7 @@ handle_control_hsfetch(control_connection_t *conn, const node_t *node = node_get_by_hex_id(server, 0); if (!node) { - connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n", + control_printf_endreply(conn, 552, "Server \"%s\" not found", server); goto done; } @@ -1456,7 +1453,7 @@ handle_control_hsfetch(control_connection_t *conn, rend_query = rend_data_client_create(hsaddress, desc_id, NULL, REND_NO_AUTH); if (rend_query == NULL) { - connection_printf_to_buf(conn, "551 Error creating the HS query\r\n"); + control_write_endreply(conn, 551, "Error creating the HS query"); goto done; } } @@ -1464,7 +1461,7 @@ handle_control_hsfetch(control_connection_t *conn, /* Using a descriptor ID, we force the user to provide at least one * hsdir server using the SERVER= option. */ if (desc_id && (!hsdirs || !smartlist_len(hsdirs))) { - connection_printf_to_buf(conn, "512 SERVER option is required\r\n"); + control_write_endreply(conn, 512, "SERVER option is required"); goto done; } @@ -1516,7 +1513,7 @@ handle_control_hspost(control_connection_t *conn, const node_t *node = node_get_by_hex_id(server, 0); if (!node || !node->rs) { - connection_printf_to_buf(conn, "552 Server \"%s\" not found\r\n", + control_printf_endreply(conn, 552, "Server \"%s\" not found", server); goto done; } @@ -1527,7 +1524,7 @@ handle_control_hspost(control_connection_t *conn, } else if (!strcasecmpstart(line->key, "HSADDRESS")) { const char *address = line->value; if (!hs_address_is_valid(address)) { - connection_printf_to_buf(conn, "512 Malformed onion address\r\n"); + control_write_endreply(conn, 512, "Malformed onion address"); goto done; } onion_address = address; @@ -1539,7 +1536,7 @@ handle_control_hspost(control_connection_t *conn, /* Handle the v3 case. */ if (onion_address) { if (hs_control_hspost_command(encoded_desc, onion_address, hs_dirs) < 0) { - connection_printf_to_buf(conn, "554 Invalid descriptor\r\n"); + control_write_endreply(conn, 554, "Invalid descriptor"); } else { send_control_done(conn); } @@ -1579,7 +1576,7 @@ handle_control_hspost(control_connection_t *conn, rend_service_descriptor_free(parsed); } else { - connection_printf_to_buf(conn, "554 Invalid descriptor\r\n"); + control_write_endreply(conn, 554, "Invalid descriptor"); } tor_free(intro_content); @@ -1685,7 +1682,7 @@ handle_control_add_onion(control_connection_t *conn, rend_service_port_config_t *cfg = rend_service_parse_port_config(arg->value, ",", NULL); if (!cfg) { - connection_printf_to_buf(conn, "512 Invalid VIRTPORT/TARGET\r\n"); + control_write_endreply(conn, 512, "Invalid VIRTPORT/TARGET"); goto out; } smartlist_add(port_cfgs, cfg); @@ -1694,7 +1691,7 @@ handle_control_add_onion(control_connection_t *conn, int ok = 0; max_streams = (int)tor_parse_long(arg->value, 10, 0, 65535, &ok, NULL); if (!ok) { - connection_printf_to_buf(conn, "512 Invalid MaxStreams\r\n"); + control_write_endreply(conn, 512, "Invalid MaxStreams"); goto out; } } else if (!strcasecmp(arg->key, "Flags")) { @@ -1722,7 +1719,7 @@ handle_control_add_onion(control_connection_t *conn, smartlist_split_string(flags, arg->value, ",", SPLIT_IGNORE_BLANK, 0); if (smartlist_len(flags) < 1) { - connection_printf_to_buf(conn, "512 Invalid 'Flags' argument\r\n"); + control_write_endreply(conn, 512, "Invalid 'Flags' argument"); bad = 1; } SMARTLIST_FOREACH_BEGIN(flags, const char *, flag) @@ -1738,8 +1735,7 @@ handle_control_add_onion(control_connection_t *conn, } else if (!strcasecmp(flag, non_anonymous_flag)) { non_anonymous = 1; } else { - connection_printf_to_buf(conn, - "512 Invalid 'Flags' argument: %s\r\n", + control_printf_endreply(conn, 512, "Invalid 'Flags' argument: %s", escaped(flag)); bad = 1; break; @@ -1773,8 +1769,7 @@ handle_control_add_onion(control_connection_t *conn, } } SMARTLIST_FOREACH_END(ac); if (bad) { - connection_printf_to_buf(conn, - "512 Duplicate name in ClientAuth\r\n"); + control_write_endreply(conn, 512, "Duplicate name in ClientAuth"); rend_authorized_client_free(client); goto out; } @@ -1792,19 +1787,19 @@ handle_control_add_onion(control_connection_t *conn, } } if (smartlist_len(port_cfgs) == 0) { - connection_printf_to_buf(conn, "512 Missing 'Port' argument\r\n"); + control_write_endreply(conn, 512, "Missing 'Port' argument"); goto out; } else if (auth_type == REND_NO_AUTH && auth_clients != NULL) { - connection_printf_to_buf(conn, "512 No auth type specified\r\n"); + control_write_endreply(conn, 512, "No auth type specified"); goto out; } else if (auth_type != REND_NO_AUTH && auth_clients == NULL) { - connection_printf_to_buf(conn, "512 No auth clients specified\r\n"); + control_write_endreply(conn, 512, "No auth clients specified"); goto out; } else if ((auth_type == REND_BASIC_AUTH && smartlist_len(auth_clients) > 512) || (auth_type == REND_STEALTH_AUTH && smartlist_len(auth_clients) > 16)) { - connection_printf_to_buf(conn, "512 Too many auth clients\r\n"); + control_write_endreply(conn, 512, "Too many auth clients"); goto out; } else if (non_anonymous != rend_service_non_anonymous_mode_enabled( get_options())) { @@ -1815,8 +1810,8 @@ handle_control_add_onion(control_connection_t *conn, * 512 Tor is in non-anonymous hidden service mode * (I've deliberately written them out in full here to aid searchability.) */ - connection_printf_to_buf(conn, "512 Tor is in %sanonymous hidden service " - "mode\r\n", + control_printf_endreply(conn, 512, + "Tor is in %sanonymous hidden service " "mode", non_anonymous ? "" : "non-"); goto out; } @@ -1843,7 +1838,7 @@ handle_control_add_onion(control_connection_t *conn, /* Hidden service version 3 don't have client authentication support so if * ClientAuth was given, send back an error. */ if (hs_version == HS_VERSION_THREE && auth_clients) { - connection_printf_to_buf(conn, "513 ClientAuth not supported\r\n"); + control_write_endreply(conn, 513, "ClientAuth not supported"); goto out; } @@ -1873,10 +1868,10 @@ handle_control_add_onion(control_connection_t *conn, } tor_assert(service_id); - connection_printf_to_buf(conn, "250-ServiceID=%s\r\n", service_id); + control_printf_midreply(conn, 250, "ServiceID=%s", service_id); if (key_new_alg) { tor_assert(key_new_blob); - connection_printf_to_buf(conn, "250-PrivateKey=%s:%s\r\n", + control_printf_midreply(conn, 250, "PrivateKey=%s:%s", key_new_alg, key_new_blob); } if (auth_created_clients) { @@ -1891,24 +1886,24 @@ handle_control_add_onion(control_connection_t *conn, }); } - connection_printf_to_buf(conn, "250 OK\r\n"); + send_control_done(conn); break; } case RSAE_BADPRIVKEY: - connection_printf_to_buf(conn, "551 Failed to generate onion address\r\n"); + control_write_endreply(conn, 551, "Failed to generate onion address"); break; case RSAE_ADDREXISTS: - connection_printf_to_buf(conn, "550 Onion address collision\r\n"); + control_write_endreply(conn, 550, "Onion address collision"); break; case RSAE_BADVIRTPORT: - connection_printf_to_buf(conn, "512 Invalid VIRTPORT/TARGET\r\n"); + control_write_endreply(conn, 512, "Invalid VIRTPORT/TARGET"); break; case RSAE_BADAUTH: - connection_printf_to_buf(conn, "512 Invalid client authorization\r\n"); + control_write_endreply(conn, 512, "Invalid client authorization"); break; case RSAE_INTERNAL: /* FALLSTHROUGH */ default: - connection_printf_to_buf(conn, "551 Failed to add Onion Service\r\n"); + control_write_endreply(conn, 551, "Failed to add Onion Service"); } if (key_new_blob) { memwipe(key_new_blob, 0, strlen(key_new_blob)); @@ -2150,7 +2145,7 @@ handle_control_del_onion(control_connection_t *conn, } else if (hs_address_is_valid(service_id)) { hs_version = HS_VERSION_THREE; } else { - connection_printf_to_buf(conn, "512 Malformed Onion Service id\r\n"); + control_write_endreply(conn, 512, "Malformed Onion Service id"); goto out; } @@ -2174,7 +2169,7 @@ handle_control_del_onion(control_connection_t *conn, } } if (onion_services == NULL) { - connection_printf_to_buf(conn, "552 Unknown Onion Service id\r\n"); + control_write_endreply(conn, 552, "Unknown Onion Service id"); } else { int ret = -1; switch (hs_version) { @@ -2226,7 +2221,7 @@ handle_control_obsolete(control_connection_t *conn, (void)args; char *command = tor_strdup(conn->current_cmd); tor_strupper(command); - connection_printf_to_buf(conn, "511 %s is obsolete.\r\n", command); + control_printf_endreply(conn, 511, "%s is obsolete.", command); tor_free(command); return 0; } @@ -2359,8 +2354,7 @@ handle_single_control_command(const control_cmd_def_t *def, cmd_data_len, args, &err); if (!parsed_args) { - connection_printf_to_buf(conn, - "512 Bad arguments to %s: %s\r\n", + control_printf_endreply(conn, 512, "Bad arguments to %s: %s", conn->current_cmd, err?err:""); tor_free(err); } else { @@ -2401,7 +2395,7 @@ handle_control_command(control_connection_t *conn, } } - connection_printf_to_buf(conn, "510 Unrecognized command \"%s\"\r\n", + control_printf_endreply(conn, 510, "Unrecognized command \"%s\"", conn->current_cmd); return 0; diff --git a/src/feature/control/control_getinfo.c b/src/feature/control/control_getinfo.c index c10229bfbb..3e31bb9e8f 100644 --- a/src/feature/control/control_getinfo.c +++ b/src/feature/control/control_getinfo.c @@ -1608,7 +1608,7 @@ handle_control_getinfo(control_connection_t *conn, if (handle_getinfo_helper(conn, q, &ans, &errmsg) < 0) { if (!errmsg) errmsg = "Internal error"; - connection_printf_to_buf(conn, "551 %s\r\n", errmsg); + control_write_endreply(conn, 551, errmsg); goto done; } if (!ans) { @@ -1625,13 +1625,10 @@ handle_control_getinfo(control_connection_t *conn, if (smartlist_len(unrecognized)) { /* control-spec section 2.3, mid-reply '-' or end of reply ' ' */ for (i=0; i < smartlist_len(unrecognized)-1; ++i) - connection_printf_to_buf(conn, - "552-%s\r\n", - (char *)smartlist_get(unrecognized, i)); - - connection_printf_to_buf(conn, - "552 %s\r\n", + control_write_midreply(conn, 552, (char *)smartlist_get(unrecognized, i)); + + control_write_endreply(conn, 552, (char *)smartlist_get(unrecognized, i)); goto done; } @@ -1645,7 +1642,7 @@ handle_control_getinfo(control_connection_t *conn, control_write_data(conn, v); } } - connection_write_str_to_buf("250 OK\r\n", conn); + send_control_done(conn); done: SMARTLIST_FOREACH(answers, char *, cp, tor_free(cp)); diff --git a/src/feature/control/control_proto.c b/src/feature/control/control_proto.c index 7bd237de94..1dd62da2be 100644 --- a/src/feature/control/control_proto.c +++ b/src/feature/control/control_proto.c @@ -166,7 +166,7 @@ read_escaped_data(const char *data, size_t len, char **out) void send_control_done(control_connection_t *conn) { - connection_write_str_to_buf("250 OK\r\n", conn); + control_write_endreply(conn, 250, "OK"); } /** Write a reply to the control channel. From 68caca58a88633c5da1860eec9e17155beaf9417 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 10 Apr 2019 17:22:36 -0500 Subject: [PATCH 8/9] Clean up formatting after Coccinelle Clean up some minor formatting quirks after the Coccinelle run. --- src/feature/control/control_auth.c | 20 ++++++----- src/feature/control/control_cmd.c | 58 ++++++++++++++---------------- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/feature/control/control_auth.c b/src/feature/control/control_auth.c index 51c8da1b99..49d4d415c6 100644 --- a/src/feature/control/control_auth.c +++ b/src/feature/control/control_auth.c @@ -142,7 +142,8 @@ handle_control_authchallenge(control_connection_t *conn, if (strcasecmp(smartlist_get(args->args, 0), "SAFECOOKIE")) { control_write_endreply(conn, 513, - "AUTHCHALLENGE only supports SAFECOOKIE " "authentication"); + "AUTHCHALLENGE only supports SAFECOOKIE " + "authentication"); goto fail; } if (!authentication_cookie_is_set) { @@ -154,13 +155,14 @@ handle_control_authchallenge(control_connection_t *conn, "2 arguments.\r\n", conn); */ control_printf_endreply(conn, 512, - "AUTHCHALLENGE dislikes argument list %s", - escaped(args->raw_body)); + "AUTHCHALLENGE dislikes argument list %s", + escaped(args->raw_body)); goto fail; } if (strcmp(args->kwargs->key, "")) { control_write_endreply(conn, 512, - "AUTHCHALLENGE does not accept keyword " "arguments."); + "AUTHCHALLENGE does not accept keyword " + "arguments."); goto fail; } @@ -222,9 +224,9 @@ handle_control_authchallenge(control_connection_t *conn, server_nonce, sizeof(server_nonce)); control_printf_endreply(conn, 250, - "AUTHCHALLENGE SERVERHASH=%s " "SERVERNONCE=%s", - server_hash_encoded, - server_nonce_encoded); + "AUTHCHALLENGE SERVERHASH=%s SERVERNONCE=%s", + server_hash_encoded, + server_nonce_encoded); tor_free(client_nonce); return 0; @@ -279,7 +281,9 @@ handle_control_authenticate(control_connection_t *conn, if (base16_decode(password, password_len+1, hex_passwd, strlen(hex_passwd)) != (int) password_len) { control_write_endreply(conn, 551, - "Invalid hexadecimal encoding. Maybe you tried a plain text " "password? If so, the standard requires that you put it in " "double quotes."); + "Invalid hexadecimal encoding. Maybe you tried a plain text " + "password? If so, the standard requires that you put it in " + "double quotes."); connection_mark_for_close(TO_CONN(conn)); tor_free(password); return 0; diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index ae4d6584cc..5555a2c5c4 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -320,11 +320,11 @@ handle_control_getconf(control_connection_t *conn, if ((len = smartlist_len(unrecognized))) { for (i=0; i < len-1; ++i) control_printf_midreply(conn, 552, - "Unrecognized configuration key \"%s\"", - (char*)smartlist_get(unrecognized, i)); + "Unrecognized configuration key \"%s\"", + (char*)smartlist_get(unrecognized, i)); control_printf_endreply(conn, 552, - "Unrecognized configuration key \"%s\"", - (char*)smartlist_get(unrecognized, len-1)); + "Unrecognized configuration key \"%s\"", + (char*)smartlist_get(unrecognized, len-1)); } else if ((len = smartlist_len(answers))) { char *tmp = smartlist_get(answers, len-1); tor_assert(strlen(tmp)>4); @@ -424,8 +424,7 @@ handle_control_setevents(control_connection_t *conn, } if (event_code == -1) { - control_printf_endreply(conn, 552, "Unrecognized event \"%s\"", - ev); + control_printf_endreply(conn, 552, "Unrecognized event \"%s\"", ev); return 0; } } @@ -489,8 +488,7 @@ handle_control_signal(control_connection_t *conn, } if (sig < 0) - control_printf_endreply(conn, 552, "Unrecognized signal code \"%s\"", - s); + control_printf_endreply(conn, 552, "Unrecognized signal code \"%s\"", s); if (sig < 0) return 0; @@ -777,7 +775,7 @@ handle_control_extendcircuit(control_connection_t *conn, intended_purpose = circuit_purpose_from_string(purpose_line->value); if (intended_purpose == CIRCUIT_PURPOSE_UNKNOWN) { control_printf_endreply(conn, 552, "Unknown purpose \"%s\"", - purpose_line->value); + purpose_line->value); goto done; } } @@ -882,7 +880,7 @@ handle_control_extendcircuit(control_connection_t *conn, } control_printf_endreply(conn, 250, "EXTENDED %lu", - (unsigned long)circ->global_identifier); + (unsigned long)circ->global_identifier); if (zero_circ) /* send a 'launched' event, for completeness */ circuit_event_status(circ, CIRC_EVENT_LAUNCHED, 0); done: @@ -922,7 +920,7 @@ handle_control_setcircuitpurpose(control_connection_t *conn, new_purpose = circuit_purpose_from_string(purp->value); if (new_purpose == CIRCUIT_PURPOSE_UNKNOWN) { control_printf_endreply(conn, 552, "Unknown purpose \"%s\"", - purp->value); + purp->value); goto done; } } @@ -970,7 +968,7 @@ handle_control_attachstream(control_connection_t *conn, &hop_line_ok, NULL); if (!hop_line_ok) { /* broken hop line */ control_printf_endreply(conn, 552, "Bad value hop=%s", - hoparg->value); + hoparg->value); return 0; } } @@ -1052,7 +1050,7 @@ handle_control_postdescriptor(control_connection_t *conn, if (line) { purpose = router_purpose_from_string(line->value); control_printf_endreply(conn, 552, "Unknown purpose \"%s\"", - line->value); + line->value); goto done; } line = config_line_find_case(args->kwargs, "cache"); @@ -1063,7 +1061,7 @@ handle_control_postdescriptor(control_connection_t *conn, cache = 1; else { control_printf_endreply(conn, 552, "Unknown cache request \"%s\"", - line->value); + line->value); goto done; } } @@ -1105,7 +1103,7 @@ handle_control_redirectstream(control_connection_t *conn, if (!(ap_conn = get_stream(smartlist_get(args, 0))) || !ap_conn->socks_request) { control_printf_endreply(conn, 552, "Unknown stream \"%s\"", - (char*)smartlist_get(args, 0)); + (char*)smartlist_get(args, 0)); } else { int ok = 1; if (smartlist_len(args) > 2) { /* they included a port too */ @@ -1114,7 +1112,7 @@ handle_control_redirectstream(control_connection_t *conn, } if (!ok) { control_printf_endreply(conn, 512, "Cannot parse port \"%s\"", - (char*)smartlist_get(args, 2)); + (char*)smartlist_get(args, 2)); } else { new_addr = tor_strdup(smartlist_get(args, 1)); } @@ -1153,13 +1151,13 @@ handle_control_closestream(control_connection_t *conn, if (!(ap_conn = get_stream(smartlist_get(args, 0)))) control_printf_endreply(conn, 552, "Unknown stream \"%s\"", - (char*)smartlist_get(args, 0)); + (char*)smartlist_get(args, 0)); else { reason = (uint8_t) tor_parse_ulong(smartlist_get(args,1), 10, 0, 255, &ok, NULL); if (!ok) { control_printf_endreply(conn, 552, "Unrecognized reason \"%s\"", - (char*)smartlist_get(args, 1)); + (char*)smartlist_get(args, 1)); ap_conn = NULL; } } @@ -1275,7 +1273,7 @@ handle_control_protocolinfo(control_connection_t *conn, }); if (bad_arg) { control_printf_endreply(conn, 513, "No such version %s", - escaped(bad_arg)); + escaped(bad_arg)); /* Don't tolerate bad arguments when not authenticated. */ if (!STATE_IS_OPEN(TO_CONN(conn)->state)) connection_mark_for_close(TO_CONN(conn)); @@ -1340,7 +1338,7 @@ handle_control_usefeature(control_connection_t *conn, ; else { control_printf_endreply(conn, 552, "Unrecognized feature \"%s\"", - arg); + arg); bad = 1; break; } @@ -1423,8 +1421,7 @@ handle_control_hsfetch(control_connection_t *conn, version = HS_VERSION_THREE; hs_parse_address(hsaddress, &v3_pk, NULL, NULL); } else { - control_printf_endreply(conn, 513, "Invalid argument \"%s\"", - arg1); + control_printf_endreply(conn, 513, "Invalid argument \"%s\"", arg1); goto done; } @@ -1434,8 +1431,7 @@ handle_control_hsfetch(control_connection_t *conn, const node_t *node = node_get_by_hex_id(server, 0); if (!node) { - control_printf_endreply(conn, 552, "Server \"%s\" not found", - server); + control_printf_endreply(conn, 552, "Server \"%s\" not found", server); goto done; } if (!hsdirs) { @@ -1514,7 +1510,7 @@ handle_control_hspost(control_connection_t *conn, if (!node || !node->rs) { control_printf_endreply(conn, 552, "Server \"%s\" not found", - server); + server); goto done; } /* Valid server, add it to our local list. */ @@ -1736,7 +1732,7 @@ handle_control_add_onion(control_connection_t *conn, non_anonymous = 1; } else { control_printf_endreply(conn, 512, "Invalid 'Flags' argument: %s", - escaped(flag)); + escaped(flag)); bad = 1; break; } @@ -1811,8 +1807,8 @@ handle_control_add_onion(control_connection_t *conn, * (I've deliberately written them out in full here to aid searchability.) */ control_printf_endreply(conn, 512, - "Tor is in %sanonymous hidden service " "mode", - non_anonymous ? "" : "non-"); + "Tor is in %sanonymous hidden service " "mode", + non_anonymous ? "" : "non-"); goto out; } @@ -1872,7 +1868,7 @@ handle_control_add_onion(control_connection_t *conn, if (key_new_alg) { tor_assert(key_new_blob); control_printf_midreply(conn, 250, "PrivateKey=%s:%s", - key_new_alg, key_new_blob); + key_new_alg, key_new_blob); } if (auth_created_clients) { SMARTLIST_FOREACH(auth_created_clients, rend_authorized_client_t *, ac, { @@ -2355,7 +2351,7 @@ handle_single_control_command(const control_cmd_def_t *def, &err); if (!parsed_args) { control_printf_endreply(conn, 512, "Bad arguments to %s: %s", - conn->current_cmd, err?err:""); + conn->current_cmd, err?err:""); tor_free(err); } else { if (BUG(err)) @@ -2396,7 +2392,7 @@ handle_control_command(control_connection_t *conn, } control_printf_endreply(conn, 510, "Unrecognized command \"%s\"", - conn->current_cmd); + conn->current_cmd); return 0; } From 09003679962e5d0e0c15cb278b83e3ef79affdda Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Fri, 26 Apr 2019 13:25:12 -0500 Subject: [PATCH 9/9] Changes file for ticket30007 --- changes/ticket30007 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket30007 diff --git a/changes/ticket30007 b/changes/ticket30007 new file mode 100644 index 0000000000..e87f6b956f --- /dev/null +++ b/changes/ticket30007 @@ -0,0 +1,3 @@ + o Code simplification and refactoring: + - Abstract out the low-level formatting of replies on the control + port. Implements ticket 30007.