commit ddd33d39c7e115045d296dd88fd4d310b932a4e1 Author: Nick Mathewson nickm@torproject.org Date: Tue Apr 9 10:09:38 2019 -0400

Port the authenticate and authchallenge commands to the new parser

These two presented their own challenge, because of their use of QString, and their distinguished handling of quoted versus non-quoted values. --- src/feature/control/control_auth.c | 143 ++++++++++++++++-------------- src/feature/control/control_auth.h | 13 ++- src/feature/control/control_cmd.c | 9 +- src/feature/control/control_cmd.h | 7 ++ src/feature/control/control_cmd_args_st.h | 4 + 5 files changed, 102 insertions(+), 74 deletions(-)

diff --git a/src/feature/control/control_auth.c b/src/feature/control/control_auth.c index 820429048..a86442c21 100644 --- a/src/feature/control/control_auth.c +++ b/src/feature/control/control_auth.c @@ -11,12 +11,15 @@ #include "app/config/config.h" #include "core/mainloop/connection.h" #include "feature/control/control.h" +#include "feature/control/control_cmd.h" #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 "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_util.h" #include "lib/encoding/confline.h" +#include "lib/encoding/kvline.h" #include "lib/encoding/qstring.h"

#include "lib/crypt_ops/crypto_s2k.h" @@ -117,12 +120,19 @@ decode_hashed_passwords(config_line_t *passwords) return NULL; }

+const control_cmd_syntax_t authchallenge_syntax = { + .min_args = 1, + .max_args = 1, + .accept_keywords=true, + .kvline_flags=KV_OMIT_KEYS|KV_QUOTED_QSTRING, + .store_raw_body=true +}; + /** Called when we get an AUTHCHALLENGE command. */ int -handle_control_authchallenge(control_connection_t *conn, uint32_t len, - const char *body) +handle_control_authchallenge(control_connection_t *conn, + const control_cmd_args_t *args) { - const char *cp = body; char *client_nonce; size_t client_nonce_len; char server_hash[DIGEST256_LEN]; @@ -130,63 +140,50 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len, char server_nonce[SAFECOOKIE_SERVER_NONCE_LEN]; char server_nonce_encoded[(2*SAFECOOKIE_SERVER_NONCE_LEN) + 1];

- cp += strspn(cp, " \t\n\r"); - if (!strcasecmpstart(cp, "SAFECOOKIE")) { - cp += strlen("SAFECOOKIE"); - } else { + if (strcasecmp(smartlist_get(args->args, 0), "SAFECOOKIE")) { connection_write_str_to_buf("513 AUTHCHALLENGE only supports SAFECOOKIE " "authentication\r\n", conn); - connection_mark_for_close(TO_CONN(conn)); - return -1; + goto fail; } - if (!authentication_cookie_is_set) { connection_write_str_to_buf("515 Cookie authentication is disabled\r\n", conn); - connection_mark_for_close(TO_CONN(conn)); - return -1; + 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", + 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); + goto fail; }

- cp += strspn(cp, " \t\n\r"); - if (*cp == '"') { - const char *newcp = - decode_qstring(cp, len - (cp - body), - &client_nonce, &client_nonce_len); - if (newcp == NULL) { - connection_write_str_to_buf("513 Invalid quoted client nonce\r\n", - conn); - connection_mark_for_close(TO_CONN(conn)); - return -1; - } - cp = newcp; + bool contains_quote = strchr(args->raw_body, '"'); + if (contains_quote) { + /* The nonce was quoted */ + client_nonce = tor_strdup(args->kwargs->value); + client_nonce_len = strlen(client_nonce); } else { - size_t client_nonce_encoded_len = strspn(cp, "0123456789ABCDEFabcdef"); - - client_nonce_len = client_nonce_encoded_len / 2; - client_nonce = tor_malloc_zero(client_nonce_len); - - if (base16_decode(client_nonce, client_nonce_len, - cp, client_nonce_encoded_len) - != (int) client_nonce_len) { + /* The nonce was should be in hex. */ + const char *hex_nonce = args->kwargs->value; + client_nonce_len = strlen(hex_nonce) / 2; + 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); - connection_mark_for_close(TO_CONN(conn)); tor_free(client_nonce); - return -1; + goto fail; } - - cp += client_nonce_encoded_len; }

- cp += strspn(cp, " \t\n\r"); - if (*cp != '\0' || - cp != body + len) { - connection_write_str_to_buf("513 Junk at end of AUTHCHALLENGE command\r\n", - conn); - connection_mark_for_close(TO_CONN(conn)); - tor_free(client_nonce); - return -1; - } crypto_rand(server_nonce, SAFECOOKIE_SERVER_NONCE_LEN);

/* Now compute and send the server-to-controller response, and the @@ -234,38 +231,56 @@ handle_control_authchallenge(control_connection_t *conn, uint32_t len,

tor_free(client_nonce); return 0; + fail: + connection_mark_for_close(TO_CONN(conn)); + return -1; }

+const control_cmd_syntax_t authenticate_syntax = { + .max_args = 0, + .accept_keywords=true, + .kvline_flags=KV_OMIT_KEYS|KV_QUOTED_QSTRING, + .store_raw_body=true +}; + /** Called when we get an AUTHENTICATE message. Check whether the * authentication is valid, and if so, update the connection's state to * OPEN. Reply with DONE or ERROR. */ int -handle_control_authenticate(control_connection_t *conn, uint32_t len, - const char *body) +handle_control_authenticate(control_connection_t *conn, + const control_cmd_args_t *args) { - int used_quoted_string = 0; + bool used_quoted_string = false; const or_options_t *options = get_options(); const char *errstr = "Unknown error"; char *password; size_t password_len; - const char *cp; - int i; int bad_cookie=0, bad_password=0; smartlist_t *sl = NULL;

- if (!len) { + if (args->kwargs == NULL) { password = tor_strdup(""); password_len = 0; - } else if (TOR_ISXDIGIT(body[0])) { - cp = body; - while (TOR_ISXDIGIT(*cp)) - ++cp; - i = (int)(cp - body); - tor_assert(i>0); - password_len = i/2; - password = tor_malloc(password_len + 1); - if (base16_decode(password, password_len+1, body, i) + } else if (args->kwargs->next) { + connection_write_str_to_buf( + "512 Too many arguments to AUTHENTICATE.\r\n", conn); + 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); + connection_mark_for_close(TO_CONN(conn)); + return 0; + } else if (strchr(args->raw_body, '"')) { + used_quoted_string = true; + password = tor_strdup(args->kwargs->value); + password_len = strlen(password); + } else { + const char *hex_passwd = args->kwargs->value; + password_len = strlen(hex_passwd) / 2; + 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 " @@ -275,14 +290,6 @@ handle_control_authenticate(control_connection_t *conn, uint32_t len, tor_free(password); return 0; } - } else { - if (!decode_qstring(body, len, &password, &password_len)) { - connection_write_str_to_buf("551 Invalid quoted string. You need " - "to put the password in double quotes.\r\n", conn); - connection_mark_for_close(TO_CONN(conn)); - return 0; - } - used_quoted_string = 1; }

if (conn->safecookie_client_hash != NULL) { diff --git a/src/feature/control/control_auth.h b/src/feature/control/control_auth.h index f436482e4..246e18ccb 100644 --- a/src/feature/control/control_auth.h +++ b/src/feature/control/control_auth.h @@ -12,16 +12,21 @@ #ifndef TOR_CONTROL_AUTH_H #define TOR_CONTROL_AUTH_H

+struct control_cmd_args_t; +struct control_cmd_syntax_t; + int init_control_cookie_authentication(int enabled); char *get_controller_cookie_file_name(void); struct config_line_t; smartlist_t *decode_hashed_passwords(struct config_line_t *passwords);

-int handle_control_authchallenge(control_connection_t *conn, uint32_t len, - const char *body); +int handle_control_authchallenge(control_connection_t *conn, + const struct control_cmd_args_t *args); int handle_control_authenticate(control_connection_t *conn, - uint32_t cmd_data_len, - const char *args); + const struct control_cmd_args_t *args); void control_auth_free_all(void);

+extern const struct control_cmd_syntax_t authchallenge_syntax; +extern const struct control_cmd_syntax_t authenticate_syntax; + #endif /* !defined(TOR_CONTROL_AUTH_H) */ diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index 0b4f3555f..a29ab72ff 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -116,6 +116,11 @@ control_cmd_parse_args(const char *command,

result->command = command;

+ if (syntax->store_raw_body) { + tor_assert(body[body_len] == 0); + result->raw_body = body; + } + const char *eol = memchr(body, '\n', body_len); if (syntax->want_object) { if (! eol || (eol+1) == body+body_len) { @@ -2309,7 +2314,7 @@ static const control_cmd_def_t CONTROL_COMMANDS[] = ONE_LINE_PARSED(getconf, 0), MULTLINE_PARSED(loadconf, 0), ONE_LINE_PARSED(setevents, 0), - ONE_LINE(authenticate, legacy, CMD_FL_WIPE), + ONE_LINE_PARSED(authenticate, CMD_FL_WIPE), ONE_LINE_PARSED(saveconf, 0), ONE_LINE_PARSED(signal, 0), ONE_LINE_PARSED(takeownership, 0), @@ -2327,7 +2332,7 @@ static const control_cmd_def_t CONTROL_COMMANDS[] = ONE_LINE_PARSED(usefeature, 0), ONE_LINE_PARSED(resolve, 0), ONE_LINE_PARSED(protocolinfo, 0), - ONE_LINE(authchallenge, legacy, CMD_FL_WIPE), + ONE_LINE_PARSED(authchallenge, CMD_FL_WIPE), ONE_LINE_PARSED(dropguards, 0), ONE_LINE_PARSED(hsfetch, 0), MULTLINE_PARSED(hspost, 0), diff --git a/src/feature/control/control_cmd.h b/src/feature/control/control_cmd.h index 6f35c74de..b825d6da5 100644 --- a/src/feature/control/control_cmd.h +++ b/src/feature/control/control_cmd.h @@ -64,6 +64,13 @@ typedef struct control_cmd_syntax_t { * True iff this command wants to be followed by a multiline object. **/ bool want_object; + /** + * True iff this command needs access to the raw body of the input. + * + * This should not be needed for pure commands; it is purely a legacy + * option. + **/ + bool store_raw_body; } control_cmd_syntax_t;

#ifdef CONTROL_CMD_PRIVATE diff --git a/src/feature/control/control_cmd_args_st.h b/src/feature/control/control_cmd_args_st.h index 06e2a183b..fb4fe64a0 100644 --- a/src/feature/control/control_cmd_args_st.h +++ b/src/feature/control/control_cmd_args_st.h @@ -43,6 +43,10 @@ struct control_cmd_args_t { * A multiline object passed with this command. **/ char *object; + /** + * If set, a nul-terminated string containing the raw unparsed arguments. + **/ + const char *raw_body; };

#endif /* !defined(TOR_CONTROL_CMD_ST_H) */