commit 550af7be0ae49f98d6ee7724934de115c80de4a9 Author: Nick Mathewson nickm@torproject.org Date: Tue Jul 16 13:54:10 2013 -0400
Fix a variety of issues in 4773
memwipe some stack-allocated stuff Add DOCDOC comments for state machines Use memdup_nulterm as appropriate Check for NULs in useraddr Add a macro so that <= AUTH_MAX has a meaning. --- src/or/ext_orport.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/or/ext_orport.c b/src/or/ext_orport.c index 72dbaa5..7dfe624 100644 --- a/src/or/ext_orport.c +++ b/src/or/ext_orport.c @@ -165,6 +165,7 @@ init_ext_or_cookie_authentication(int is_enabled) log_info(LD_GENERAL, "Generated Extended ORPort cookie file in '%s'.", fname);
+ memwipe(cookie_file_string, 0, sizeof(cookie_file_string)); tor_free(fname); return 0; } @@ -223,6 +224,9 @@ connection_ext_or_auth_handle_client_nonce(connection_t *conn) EXT_OR_PORT_AUTH_NONCE_LEN, conn) < 0) return -1;
+ /* DOCDOC comment this function more, with comments about what the + * protocol is. */ + /* Get our nonce */ if (crypto_rand(server_nonce, EXT_OR_PORT_AUTH_NONCE_LEN) < 0) return -1; @@ -271,6 +275,9 @@ connection_ext_or_auth_handle_client_nonce(connection_t *conn) with the hash sent by the client. */ TO_OR_CONN(conn)->ext_or_auth_correct_client_hash = correct_client_hash;
+ memwipe(hmac_s_msg, 0, hmac_s_msg_len); + memwipe(hmac_c_msg, 0, hmac_c_msg_len); + tor_free(hmac_s_msg); tor_free(hmac_c_msg); } @@ -290,6 +297,10 @@ connection_ext_or_auth_handle_client_nonce(connection_t *conn) log_debug(LD_GENERAL, "server_hash: '%s'\nserver_nonce: '%s'\nclient_nonce: '%s'", server_hash_encoded, server_nonce_encoded, client_nonce_encoded); + + memwipe(server_hash_encoded, 0, sizeof(server_hash_encoded)); + memwipe(server_nonce_encoded, 0, sizeof(server_nonce_encoded)); + memwipe(client_nonce_encoded, 0, sizeof(client_nonce_encoded)); }
{ /* write reply: (server_hash, server_nonce) */ @@ -297,6 +308,7 @@ connection_ext_or_auth_handle_client_nonce(connection_t *conn) memcpy(reply + EXT_OR_PORT_AUTH_HASH_LEN, server_nonce, EXT_OR_PORT_AUTH_NONCE_LEN); connection_write_to_buf(reply, sizeof(reply), conn); + memwipe(reply, 0, sizeof(reply)); }
log_debug(LD_GENERAL, "Got client nonce, and sent our own nonce and hash."); @@ -362,6 +374,8 @@ connection_ext_or_auth_process_inbuf(or_connection_t *or_conn) { connection_t *conn = TO_CONN(or_conn);
+ /* DOCDOC Document the state machine here! */ + switch (conn->state) { /* Functionify */ case EXT_OR_CONN_STATE_AUTH_WAIT_AUTH_TYPE: return connection_ext_or_auth_neg_auth_type(conn); @@ -406,9 +420,12 @@ connection_ext_or_handle_cmd_useraddr(connection_t *conn, char *addr_str; char *address_part=NULL; int res; - addr_str = tor_malloc(len + 1); - memcpy(addr_str, payload, len); - addr_str[len] = 0; + if (memchr(payload, '\0', len)) { + log_fn(LOG_PROTOCOL_WARN, LD_NET, "Unexpected NUL in ExtORPort UserAddr"); + return -1; + } + + addr_str = tor_memdup_nulterm(payload, len);
res = tor_addr_port_split(LOG_INFO, addr_str, &address_part, &port); tor_free(addr_str); @@ -470,6 +487,9 @@ connection_ext_or_handle_cmd_transport(or_connection_t *conn, return 0; }
+#define EXT_OR_CONN_STATE_IS_AUTHENTICATING(st) \ + ((st) <= EXT_OR_CONN_STATE_AUTH_MAX) + /** Process Extended ORPort messages from <b>or_conn</b>. */ int connection_ext_or_process_inbuf(or_connection_t *or_conn) @@ -478,9 +498,11 @@ connection_ext_or_process_inbuf(or_connection_t *or_conn) ext_or_cmd_t *command; int r;
+ /* DOCDOC Document the state machine and transitions in this function */ + /* If we are still in the authentication stage, process traffic as authentication data: */ - while (conn->state <= EXT_OR_CONN_STATE_AUTH_MAX) { + while (EXT_OR_CONN_STATE_IS_AUTHENTICATING(conn->state)) { log_debug(LD_GENERAL, "Got Extended ORPort authentication data (%u).", (unsigned int) connection_get_inbuf_len(conn)); r = connection_ext_or_auth_process_inbuf(or_conn);