tor-commits May 2017

tor-commits@lists.torproject.org

16 participants
1135 discussions

[tor/master] Break connection_dir_client_reached_eof() into smaller functions
by nickm＠torproject.org 03 May '17

03 May '17

commit db86b9194dfc6759cd731f8eedc37b7534afbf0d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 2 13:06:25 2017 -0400 Break connection_dir_client_reached_eof() into smaller functions This was a >630-line function, which doesn't make anybody happy. It was also mostly composed of a bunch of if-statements that handled different directory responses differently depending on the original purpose of the directory connection. The logical refactoring here is to move the body of each switch statement into a separate handler function, and to invoke those functions from a separate switch statement. This commit leaves whitespace mostly untouched, for ease of review. I'll reindent in the next commit. --- changes/refactor_reached_eof | 5 + src/or/directory.c | 319 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 286 insertions(+), 38 deletions(-) diff --git a/changes/refactor_reached_eof b/changes/refactor_reached_eof new file mode 100644 index 0000000..33ab931 --- /dev/null +++ b/changes/refactor_reached_eof @@ -0,0 +1,5 @@ + o Code simplification and refactoring: + + - Break up the 630-line function connection_dir_client_reached_eof() into + a dozen smaller functions. This change should help maintainability and + readability of the client directory code. diff --git a/src/or/directory.c b/src/or/directory.c index 2b9f18a..0635be1 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2073,6 +2073,39 @@ load_downloaded_routers(const char *body, smartlist_t *which, return added; } +/** A structure to hold arguments passed into each directory response + * handler */ +typedef struct response_handler_args_t { + int status_code; + const char *reason; + const char *body; + size_t body_len; + const char *headers; +} response_handler_args_t; + +static int handle_response_fetch_consensus(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_fetch_certificate(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_fetch_status_vote(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_fetch_detached_signatures(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_fetch_desc(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_fetch_microdesc(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_upload_dir(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_upload_vote(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_upload_signatures(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_fetch_renddesc_v2(dir_connection_t *, + const response_handler_args_t *); +static int handle_response_upload_renddesc_v2(dir_connection_t *, + const response_handler_args_t *); + /** We are a client, and we've finished reading the server's * response. Parse it and act appropriately. * @@ -2098,8 +2131,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) int allow_partial = (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO || conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC); - time_t now = time(NULL); - int src_code; size_t received_bytes; received_bytes = connection_get_inbuf_len(TO_CONN(conn)); @@ -2193,6 +2224,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) "'%s:%d'. I'll try again soon.", status_code, escaped(reason), conn->base_.address, conn->base_.port); + time_t now = approx_time(); if ((rs = router_get_mutable_consensus_status_by_id(id_digest))) rs->last_dir_503_at = now; if ((ds = router_get_fallback_dirserver_by_digest(id_digest))) @@ -2260,7 +2292,77 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_CONSENSUS) { + int rv; + response_handler_args_t args; + memset(&args, 0, sizeof(args)); + args.status_code = status_code; + args.reason = reason; + args.body = body; + args.body_len = body_len; + args.headers = headers; + + switch (conn->base_.purpose) { + case DIR_PURPOSE_FETCH_CONSENSUS: + rv = handle_response_fetch_consensus(conn, &args); + break; + case DIR_PURPOSE_FETCH_CERTIFICATE: + rv = handle_response_fetch_certificate(conn, &args); + break; + case DIR_PURPOSE_FETCH_STATUS_VOTE: + rv = handle_response_fetch_status_vote(conn, &args); + break; + case DIR_PURPOSE_FETCH_DETACHED_SIGNATURES: + rv = handle_response_fetch_detached_signatures(conn, &args); + break; + case DIR_PURPOSE_FETCH_SERVERDESC: + case DIR_PURPOSE_FETCH_EXTRAINFO: + rv = handle_response_fetch_desc(conn, &args); + break; + case DIR_PURPOSE_FETCH_MICRODESC: + rv = handle_response_fetch_microdesc(conn, &args); + break; + case DIR_PURPOSE_FETCH_RENDDESC_V2: + rv = handle_response_fetch_renddesc_v2(conn, &args); + break; + case DIR_PURPOSE_UPLOAD_DIR: + rv = handle_response_upload_dir(conn, &args); + break; + case DIR_PURPOSE_UPLOAD_SIGNATURES: + rv = handle_response_upload_signatures(conn, &args); + break; + case DIR_PURPOSE_UPLOAD_VOTE: + rv = handle_response_upload_vote(conn, &args); + break; + case DIR_PURPOSE_UPLOAD_RENDDESC_V2: + rv = handle_response_upload_renddesc_v2(conn, &args); + break; + default: + tor_assert_nonfatal_unreached(); + rv = -1; + break; + } + tor_free(body); + tor_free(headers); + tor_free(reason); + return rv; +} + +/** + * Handler function: processes a response to a request for a networkstatus + * consensus document by checking the consensus, storing it, and marking + * router requests as reachable. + **/ +static int +handle_response_fetch_consensus(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_CONSENSUS); + const int status_code = args->status_code; + const char *body = args->body; + const size_t body_len = args->body_len; + const char *reason = args->reason; + const time_t now = approx_time(); + int r; const char *flavname = conn->requested_resource; if (status_code != 200) { @@ -2270,7 +2372,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) "'%s:%d' while fetching consensus directory.", status_code, escaped(reason), conn->base_.address, conn->base_.port); - tor_free(body); tor_free(headers); tor_free(reason); networkstatus_consensus_download_failed(status_code, flavname); return -1; } @@ -2282,7 +2383,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) "Unable to load %s consensus directory downloaded from " "server '%s:%d'. I'll try again soon.", flavname, conn->base_.address, conn->base_.port); - tor_free(body); tor_free(headers); tor_free(reason); networkstatus_consensus_download_failed(0, flavname); return -1; } @@ -2300,17 +2400,31 @@ connection_dir_client_reached_eof(dir_connection_t *conn) networkstatus_get_latest_consensus_by_flavor(FLAV_NS)); } log_info(LD_DIR, "Successfully loaded consensus."); - } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_CERTIFICATE) { - if (status_code != 200) { + return 0; +} + +/** + * Handler function: processes a response to a request for one or more + * authority certificates + **/ +static int +handle_response_fetch_certificate(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_CERTIFICATE); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *body = args->body; + const size_t body_len = args->body_len; + + if (status_code != 200) { log_warn(LD_DIR, "Received http status code %d (%s) from server " "'%s:%d' while fetching \"/tor/keys/%s\".", status_code, escaped(reason), conn->base_.address, conn->base_.port, conn->requested_resource); connection_dir_download_cert_failed(conn, status_code); - tor_free(body); tor_free(headers); tor_free(reason); return -1; } log_info(LD_DIR,"Received authority certificates (body size %d) from " @@ -2321,7 +2435,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) * Tell trusted_dirs_load_certs_from_string() whether it was by fp * or fp-sk pair. */ - src_code = -1; + int src_code = -1; if (!strcmpstart(conn->requested_resource, "fp/")) { src_code = TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST; } else if (!strcmpstart(conn->requested_resource, "fp-sk/")) { @@ -2336,6 +2450,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) * ones got flushed to disk so it's safe to call this on them */ connection_dir_download_cert_failed(conn, status_code); } else { + time_t now = approx_time(); directory_info_has_arrived(now, 0, 0); log_info(LD_DIR, "Successfully loaded certificates from fetch."); } @@ -2346,8 +2461,24 @@ connection_dir_client_reached_eof(dir_connection_t *conn) conn->requested_resource); connection_dir_download_cert_failed(conn, status_code); } - } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_STATUS_VOTE) { + return 0; +} + + +/** + * Handler function: processes a response to a request for an authority's + * current networkstatus vote. + **/ +static int +handle_response_fetch_status_vote(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_STATUS_VOTE); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *body = args->body; + const size_t body_len = args->body_len; + const char *msg; int st; log_info(LD_DIR,"Got votes (body size %d) from server %s:%d", @@ -2358,7 +2489,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) "'%s:%d' while fetching \"/tor/status-vote/next/%s.z\".", status_code, escaped(reason), conn->base_.address, conn->base_.port, conn->requested_resource); - tor_free(body); tor_free(headers); tor_free(reason); return -1; } dirvote_add_vote(body, &msg, &st); @@ -2367,8 +2497,24 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } else { log_info(LD_DIR, "Added vote(s) successfully [msg: %s]", msg); } - } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_DETACHED_SIGNATURES) { + + return 0; +} + +/** + * Handler function: processes a response to a request for the signatures + * that an authority knows about on a given consensus. + **/ +static int +handle_response_fetch_detached_signatures(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_DETACHED_SIGNATURES); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *body = args->body; + const size_t body_len = args->body_len; + const char *msg = NULL; log_info(LD_DIR,"Got detached signatures (body size %d) from server %s:%d", (int)body_len, conn->base_.address, conn->base_.port); @@ -2378,17 +2524,31 @@ connection_dir_client_reached_eof(dir_connection_t *conn) "\"/tor/status-vote/next/consensus-signatures.z\".", status_code, escaped(reason), conn->base_.address, conn->base_.port); - tor_free(body); tor_free(headers); tor_free(reason); return -1; } if (dirvote_add_signatures(body, conn->base_.address, &msg)<0) { log_warn(LD_DIR, "Problem adding detached signatures from %s:%d: %s", conn->base_.address, conn->base_.port, msg?msg:"???"); } - } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || - conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO) { + return 0; +} + +/** + * Handler function: processes a response to a request for a group of server + * descriptors or an extrainfo documents. + **/ +static int +handle_response_fetch_desc(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || + conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *body = args->body; + const size_t body_len = args->body_len; + int was_ei = conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO; smartlist_t *which = NULL; int n_asked_for = 0; @@ -2425,7 +2585,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); smartlist_free(which); } - tor_free(body); tor_free(headers); tor_free(reason); return dir_okay ? 0 : -1; } /* Learn the routers, assuming we requested by fingerprint or "all" @@ -2449,8 +2608,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn) // descriptor_digests, conn->router_purpose); if (load_downloaded_routers(body, which, descriptor_digests, conn->router_purpose, - conn->base_.address)) + conn->base_.address)) { + time_t now = approx_time(); directory_info_has_arrived(now, 0, 0); + } } } if (which) { /* mark remaining ones as failed */ @@ -2468,8 +2629,24 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } if (directory_conn_is_self_reachability_test(conn)) router_dirport_found_reachable(); - } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC) { + + return 0; +} + +/** + * Handler function: processes a response to a request for a group of + * microdescriptors + **/ +static int +handle_response_fetch_microdesc(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *body = args->body; + const size_t body_len = args->body_len; + smartlist_t *which = NULL; log_info(LD_DIR,"Received answer to microdescriptor request (status %d, " "body size %d) from server '%s:%d'", @@ -2490,10 +2667,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn) dir_microdesc_download_failed(which, status_code); SMARTLIST_FOREACH(which, char *, cp, tor_free(cp)); smartlist_free(which); - tor_free(body); tor_free(headers); tor_free(reason); return 0; } else { smartlist_t *mds; + time_t now = approx_time(); mds = microdescs_add_to_cache(get_microdesc_cache(), body, body+body_len, SAVED_NOWHERE, 0, now, which); @@ -2510,9 +2687,23 @@ connection_dir_client_reached_eof(dir_connection_t *conn) smartlist_free(which); smartlist_free(mds); } - } - if (conn->base_.purpose == DIR_PURPOSE_UPLOAD_DIR) { + return 0; +} + +/** + * Handler function: processes a response to a POST request to upload our + * router descriptor. + **/ +static int +handle_response_upload_dir(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_UPLOAD_DIR); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *headers = args->headers; + switch (status_code) { case 200: { dir_server_t *ds = @@ -2563,9 +2754,22 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } /* return 0 in all cases, since we don't want to mark any * dirservers down just because they don't like us. */ - } - if (conn->base_.purpose == DIR_PURPOSE_UPLOAD_VOTE) { + return 0; +} + +/** + * Handler function: processes a response to POST request to upload our + * own networkstatus vote. + **/ +static int +handle_response_upload_vote(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_UPLOAD_VOTE); + const int status_code = args->status_code; + const char *reason = args->reason; + switch (status_code) { case 200: { log_notice(LD_DIR,"Uploaded a vote to dirserver %s:%d", @@ -2587,9 +2791,21 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } /* return 0 in all cases, since we don't want to mark any * dirservers down just because they don't like us. */ - } + return 0; +} + +/** + * Handler function: processes a response to POST request to upload our + * view of the signatures on the current consensus. + **/ +static int +handle_response_upload_signatures(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_UPLOAD_SIGNATURES); + const int status_code = args->status_code; + const char *reason = args->reason; - if (conn->base_.purpose == DIR_PURPOSE_UPLOAD_SIGNATURES) { switch (status_code) { case 200: { log_notice(LD_DIR,"Uploaded signature(s) to dirserver %s:%d", @@ -2611,10 +2827,25 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } /* return 0 in all cases, since we don't want to mark any * dirservers down just because they don't like us. */ - } - if (conn->base_.purpose == DIR_PURPOSE_FETCH_RENDDESC_V2) { - #define SEND_HS_DESC_FAILED_EVENT(reason) ( \ + return 0; +} + +/** + * Handler function: processes a response to a request for a v2 hidden service + * descriptor. + **/ +static int +handle_response_fetch_renddesc_v2(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_FETCH_RENDDESC_V2); + const int status_code = args->status_code; + const char *reason = args->reason; + const char *body = args->body; + const size_t body_len = args->body_len; + + #define SEND_HS_DESC_FAILED_EVENT(reason) ( \ control_event_hs_descriptor_failed(conn->rend_data, \ conn->identity_digest, \ reason) ) @@ -2689,10 +2920,23 @@ connection_dir_client_reached_eof(dir_connection_t *conn) SEND_HS_DESC_FAILED_CONTENT(); break; } - } - if (conn->base_.purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2) { - #define SEND_HS_DESC_UPLOAD_FAILED_EVENT(reason) ( \ + return 0; +} + +/** + * Handler function: processes a response to a POST request to upload a v2 + * hidden service descriptor. + **/ +static int +handle_response_upload_renddesc_v2(dir_connection_t *conn, + const response_handler_args_t *args) +{ + tor_assert(conn->base_.purpose == DIR_PURPOSE_UPLOAD_RENDDESC_V2); + const int status_code = args->status_code; + const char *reason = args->reason; + + #define SEND_HS_DESC_UPLOAD_FAILED_EVENT(reason) ( \ control_event_hs_descriptor_upload_failed( \ conn->identity_digest, \ rend_data_get_address(conn->rend_data), \ @@ -2726,8 +2970,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) SEND_HS_DESC_UPLOAD_FAILED_EVENT("UNEXPECTED"); break; } - } - tor_free(body); tor_free(headers); tor_free(reason); + return 0; }

1 0

[tor/master] Merge branch 'refactor_dir_client_handler'
by nickm＠torproject.org 03 May '17

03 May '17

commit e0b1fd4d3da05701c0970dabd93010d6c7550d43 Merge: 7bc9f93 e0c937f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 2 20:42:00 2017 -0400 Merge branch 'refactor_dir_client_handler' changes/refactor_reached_eof | 5 + src/or/directory.c | 1092 ++++++++++++++++++++++++++---------------- 2 files changed, 674 insertions(+), 423 deletions(-)

1 0

[chutney/master] Give chutney a CHUTNEY_DEBUG option
by teor＠torproject.org 03 May '17

03 May '17

commit 876933cb3173b0cc14848189b7dac41c7905560f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 2 19:24:56 2017 -0400 Give chutney a CHUTNEY_DEBUG option test-network.sh gets a --debug flag, and they both get documented in the README. debug_flag is no longer recommended. --- README | 1 + lib/chutney/Traffic.py | 4 ++-- scripts/chutney_tests/verify.py | 4 +--- tools/test-network.sh | 5 ++++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/README b/README index d184947..8030842 100644 --- a/README +++ b/README @@ -57,6 +57,7 @@ Warning Options: --only-warnings CHUTNEY_WARNINGS_ONLY=true Expert Options: + --debug CHUTNEY_DEBUG=true --coverage USE_COVERAGE_BINARY=true --net-dir CHUTNEY_DATA_DIR --dry-run NETWORK_DRY_RUN=true diff --git a/lib/chutney/Traffic.py b/lib/chutney/Traffic.py index b5f75fd..a22d16b 100644 --- a/lib/chutney/Traffic.py +++ b/lib/chutney/Traffic.py @@ -28,11 +28,11 @@ import select import struct import errno import time +import os # Set debug_flag=True in order to debug this program or to get hints # about what's going wrong in your system. -debug_flag = False - +debug_flag = os.environ.get("CHUTNEY_DEBUG", "") != "" def debug(s): "Print a debug message on stdout if debug_flag is True." diff --git a/scripts/chutney_tests/verify.py b/scripts/chutney_tests/verify.py index b887ee7..98beb74 100644 --- a/scripts/chutney_tests/verify.py +++ b/scripts/chutney_tests/verify.py @@ -19,9 +19,7 @@ def run_test(network): time.sleep(5) print("Transmission: %s" % ("Success" if status else "Failure")) if not status: - # TODO: allow the debug flag to be passed as an argument to - # src/test/test-network.sh and chutney - print("Set 'debug_flag = True' in Traffic.py to diagnose.") + print("Set CHUTNEY_DEBUG to diagnose.") return status diff --git a/tools/test-network.sh b/tools/test-network.sh index 9da20b1..7ea0fd4 100755 --- a/tools/test-network.sh +++ b/tools/test-network.sh @@ -35,7 +35,10 @@ do # the name or path of a tor-gencert binary export CHUTNEY_TOR_GENCERT="$2" shift - ;; + ;; + --debug) + export CHUTEY_DEBUG="yes" + ;; --flavor|--flavour|--network-flavor|--network-flavour) export NETWORK_FLAVOUR="$2" shift

1 0

[tor/master] Remove the unused field or_connection_t.is_connection_with_client
by nickm＠torproject.org 02 May '17

02 May '17

commit 934003587305dcfb2cbc7f01e7248e3bd378317b Author: teor <teor2345(a)gmail.com> Date: Wed Mar 1 16:22:37 2017 +1100 Remove the unused field or_connection_t.is_connection_with_client To discover if a connection is from a tor client, use: channel_is_client(TLS_CHAN_TO_BASE(or_connection_t.chan)) Part of 21406. --- src/or/or.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 0db9f23..a1a0bd4 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1543,10 +1543,6 @@ typedef struct or_connection_t { * NETINFO cell listed the address we're connected to as recognized. */ unsigned int is_canonical:1; - /** True iff we have decided that the other end of this connection - * is a client. Connections with this flag set should never be used - * to satisfy an EXTEND request. */ - unsigned int is_connection_with_client:1; /** True iff this is an outgoing connection. */ unsigned int is_outgoing:1; unsigned int proxy_type:2; /**< One of PROXY_NONE...PROXY_SOCKS5 */

1 0

[tor/master] Accurately identify client connections by their lack of peer authentication
by nickm＠torproject.org 02 May '17

02 May '17

commit f9af7e8bd01d4b7a70776a96c649a89cf1717dd1 Author: teor <teor2345(a)gmail.com> Date: Wed Mar 1 16:23:02 2017 +1100 Accurately identify client connections by their lack of peer authentication This means that we bail out earlier if asked to extend to a client. Follow-up to 21407. Fixes bug 21406; bugfix on 0.2.4.23. --- changes/bug21406 | 5 +++++ src/or/channel.h | 4 ++-- src/or/channeltls.c | 4 ++++ src/or/command.c | 10 ++++++++-- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/changes/bug21406 b/changes/bug21406 new file mode 100644 index 0000000..170e631 --- /dev/null +++ b/changes/bug21406 @@ -0,0 +1,5 @@ + o Minor bugfixes (code correctness): + - Accurately identify client connections using their lack of peer + authentication. This means that we bail out earlier if asked to extend + to a client. Follow-up to 21407. + Fixes bug 21406; bugfix on 0.2.4.23. diff --git a/src/or/channel.h b/src/or/channel.h index 26aa93b..748a099 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -214,8 +214,8 @@ struct channel_s { unsigned int is_bad_for_new_circs:1; /** True iff we have decided that the other end of this connection - * is a client. Channels with this flag set should never be used - * to satisfy an EXTEND request. */ + * is a client or bridge relay. Connections with this flag set should never + * be used to satisfy an EXTEND request. */ unsigned int is_client:1; /** Set if the channel was initiated remotely (came from a listener) */ diff --git a/src/or/channeltls.c b/src/or/channeltls.c index dbed95f..e734913 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1654,6 +1654,10 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) tor_assert(tor_mem_is_zero( (const char*)(chan->conn->handshake_state-> authenticated_ed25519_peer_id.pubkey), 32)); + /* If the client never authenticated, it's a tor client or bridge + * relay, and we must not use it for EXTEND requests (nor could we, as + * there are no authenticated peer IDs) */ + channel_mark_client(TLS_CHAN_TO_BASE(chan)); channel_set_circid_type(TLS_CHAN_TO_BASE(chan), NULL, chan->conn->link_proto < MIN_LINK_PROTO_FOR_WIDE_CIRC_IDS); diff --git a/src/or/command.c b/src/or/command.c index 5866c38..4d3830f 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -344,8 +344,14 @@ command_process_create_cell(cell_t *cell, channel_t *chan) int len; created_cell_t created_cell; - /* Make sure we never try to use the OR connection on which we - * received this cell to satisfy an EXTEND request, */ + /* If the client used CREATE_FAST, it's probably a tor client or bridge + * relay, and we must not use it for EXTEND requests (in most cases, we + * won't have an authenticated peer ID for the extend). + * Public relays on 0.2.9 and later will use CREATE_FAST if they have no + * ntor onion key for this relay, but that should be a rare occurrence. + * Clients on 0.3.1 and later avoid using CREATE_FAST as much as they can, + * even during bootstrap, so the CREATE_FAST check is most accurate for + * earlier tor client versions. */ channel_mark_client(chan); memset(&created_cell, 0, sizeof(created_cell));

1 0

[tor/master] Merge branch 'teor_connection-with-client-v2_squashed'
by nickm＠torproject.org 02 May '17

02 May '17

commit 7bc9f93abbfcabba1acde441b5fac98d4676c08e Merge: a91f948 f9af7e8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue May 2 19:18:04 2017 -0400 Merge branch 'teor_connection-with-client-v2_squashed' changes/bug21406 | 5 +++++ src/or/channel.h | 4 ++-- src/or/channeltls.c | 4 ++++ src/or/command.c | 10 ++++++++-- src/or/or.h | 4 ---- 5 files changed, 19 insertions(+), 8 deletions(-)

1 0

[tor/master] Delete useless checks in confparse.c
by nickm＠torproject.org 02 May '17

02 May '17

commit 5494087ed76475d30d80b47205cf8a0c5fb8ea69 Author: Taylor Yu <catalyst(a)torproject.org> Date: Mon May 1 14:00:06 2017 -0400 Delete useless checks in confparse.c config_parse_interval() and config_parse_msec_interval() were checking whether the variable "ok" (a pointer to an int) was null, rather than derefencing it. Both functions are static, and all existing callers pass a valid pointer to those static functions. The callers do check the variables (also confusingly named "ok") whose addresses they pass as the "ok" arguments, so even if the pointer check were corrected to be a dereference, it would be redundant. Fixes #22103. --- src/or/confparse.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/or/confparse.c b/src/or/confparse.c index 75ec92e..abae7e3 100644 --- a/src/or/confparse.c +++ b/src/or/confparse.c @@ -1190,8 +1190,6 @@ config_parse_msec_interval(const char *s, int *ok) { uint64_t r; r = config_parse_units(s, time_msec_units, ok); - if (!ok) - return -1; if (r > INT_MAX) { log_warn(LD_CONFIG, "Msec interval '%s' is too long", s); *ok = 0; @@ -1209,8 +1207,6 @@ config_parse_interval(const char *s, int *ok) { uint64_t r; r = config_parse_units(s, time_units, ok); - if (!ok) - return -1; if (r > INT_MAX) { log_warn(LD_CONFIG, "Interval '%s' is too long", s); *ok = 0;

1 0

[tor/master] Add tests for #22103
by nickm＠torproject.org 02 May '17

02 May '17

commit a91f948ccac5e681de9e8e63efb267c7ad9e9f5f Author: Taylor Yu <catalyst(a)torproject.org> Date: Tue May 2 15:49:36 2017 -0400 Add tests for #22103 Also factor out the error message comparisions from test_options_validate_impl() into a separate function so it can check for error messages in different phases of config parsing. --- src/test/test_options.c | 119 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 82 insertions(+), 37 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 1ba6a05..29f6665 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -105,27 +105,13 @@ clear_log_messages(void) "EDE6D711294FADF8E7951F4DE6CA56B58 194.109.206.212:80 7EA6 EAD6 FD83" \ " 083C 538F 4403 8BBF A077 587D D755\n" -static void -test_options_validate_impl(const char *configuration, - const char *expect_errmsg, - int expect_log_severity, - const char *expect_log) +static int +test_options_checkmsgs(const char *configuration, + const char *expect_errmsg, + int expect_log_severity, + const char *expect_log, + char *msg) { - or_options_t *opt=NULL; - or_options_t *dflt; - config_line_t *cl=NULL; - char *msg=NULL; - int r; - - setup_options(opt, dflt); - - r = config_get_lines(configuration, &cl, 1); - tt_int_op(r, OP_EQ, 0); - - r = config_assign(&options_format, opt, cl, 0, &msg); - tt_int_op(r, OP_EQ, 0); - - r = options_validate(NULL, opt, dflt, 0, &msg); if (expect_errmsg && !msg) { TT_DIE(("Expected error message <%s> from <%s>, but got none.", expect_errmsg, configuration)); @@ -136,8 +122,6 @@ test_options_validate_impl(const char *configuration, TT_DIE(("Expected no error message from <%s> but got <%s>.", configuration, msg)); } - tt_int_op((r == 0), OP_EQ, (msg == NULL)); - if (expect_log) { int found = 0; if (messages) { @@ -157,6 +141,56 @@ test_options_validate_impl(const char *configuration, configuration, msg)); } } + return 0; + + done: + return -1; +} + +/* Which phases of config parsing/validation to check for messages/logs */ +enum { PH_GETLINES, PH_ASSIGN, PH_VALIDATE }; + +static void +test_options_validate_impl(const char *configuration, + const char *expect_errmsg, + int expect_log_severity, + const char *expect_log, + int phase) +{ + or_options_t *opt=NULL; + or_options_t *dflt; + config_line_t *cl=NULL; + char *msg=NULL; + int r; + + setup_options(opt, dflt); + + r = config_get_lines(configuration, &cl, 1); + if (phase == PH_GETLINES) { + if (test_options_checkmsgs(configuration, expect_errmsg, + expect_log_severity, + expect_log, msg)) + goto done; + } + tt_int_op((r == 0), OP_EQ, (msg == NULL)); + + r = config_assign(&options_format, opt, cl, 0, &msg); + if (phase == PH_ASSIGN) { + if (test_options_checkmsgs(configuration, expect_errmsg, + expect_log_severity, + expect_log, msg)) + goto done; + } + tt_int_op((r == 0), OP_EQ, (msg == NULL)); + + r = options_validate(NULL, opt, dflt, 0, &msg); + if (phase == PH_VALIDATE) { + if (test_options_checkmsgs(configuration, expect_errmsg, + expect_log_severity, + expect_log, msg)) + goto done; + } + tt_int_op((r == 0), OP_EQ, (msg == NULL)); done: escaped(NULL); @@ -168,14 +202,14 @@ test_options_validate_impl(const char *configuration, clear_log_messages(); } -#define WANT_ERR(config, msg) \ - test_options_validate_impl((config), (msg), 0, NULL) -#define WANT_LOG(config, severity, msg) \ - test_options_validate_impl((config), NULL, (severity), (msg)) -#define WANT_ERR_LOG(config, msg, severity, logmsg) \ - test_options_validate_impl((config), (msg), (severity), (logmsg)) -#define OK(config) \ - test_options_validate_impl((config), NULL, 0, NULL) +#define WANT_ERR(config, msg, ph) \ + test_options_validate_impl((config), (msg), 0, NULL, (ph)) +#define WANT_LOG(config, severity, msg, ph) \ + test_options_validate_impl((config), NULL, (severity), (msg), (ph)) +#define WANT_ERR_LOG(config, msg, severity, logmsg, ph) \ + test_options_validate_impl((config), (msg), (severity), (logmsg), (ph)) +#define OK(config, ph) \ + test_options_validate_impl((config), NULL, 0, NULL, (ph)) static void test_options_validate(void *arg) @@ -184,21 +218,32 @@ test_options_validate(void *arg) setup_log_callback(); sandbox_disable_getaddrinfo_cache(); - WANT_ERR("ExtORPort 500000", "Invalid ExtORPort"); + WANT_ERR("ExtORPort 500000", "Invalid ExtORPort", PH_VALIDATE); WANT_ERR_LOG("ServerTransportOptions trebuchet", "ServerTransportOptions did not parse", - LOG_WARN, "Too few arguments"); - OK("ServerTransportOptions trebuchet sling=snappy"); - OK("ServerTransportOptions trebuchet sling="); + LOG_WARN, "Too few arguments", PH_VALIDATE); + OK("ServerTransportOptions trebuchet sling=snappy", PH_VALIDATE); + OK("ServerTransportOptions trebuchet sling=", PH_VALIDATE); WANT_ERR_LOG("ServerTransportOptions trebuchet slingsnappy", "ServerTransportOptions did not parse", - LOG_WARN, "\"slingsnappy\" is not a k=v"); + LOG_WARN, "\"slingsnappy\" is not a k=v", PH_VALIDATE); WANT_ERR("DirPort 8080\nDirCache 0", - "DirPort configured but DirCache disabled."); + "DirPort configured but DirCache disabled.", PH_VALIDATE); WANT_ERR("BridgeRelay 1\nDirCache 0", - "We're a bridge but DirCache is disabled."); + "We're a bridge but DirCache is disabled.", PH_VALIDATE); + + WANT_ERR_LOG("HeartbeatPeriod 21 snarks", + "Interval 'HeartbeatPeriod 21 snarks' is malformed or" + " out of bounds.", LOG_WARN, "Unknown unit 'snarks'.", + PH_ASSIGN); + WANT_ERR_LOG("LogTimeGranularity 21 snarks", + "Msec interval 'LogTimeGranularity 21 snarks' is malformed or" + " out of bounds.", LOG_WARN, "Unknown unit 'snarks'.", + PH_ASSIGN); + OK("HeartbeatPeriod 1 hour", PH_VALIDATE); + OK("LogTimeGranularity 100 milliseconds", PH_VALIDATE); close_temp_logs(); clear_log_messages();

1 0

[torspec/master] fix trivial typo
by arma＠torproject.org 02 May '17

02 May '17

commit f9b2bdb3f58991446af0d1d99b25e4990d138af9 Author: Roger Dingledine <arma(a)torproject.org> Date: Tue May 2 19:01:11 2017 -0400 fix trivial typo --- proposals/274-rotate-onion-keys-less.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/274-rotate-onion-keys-less.txt b/proposals/274-rotate-onion-keys-less.txt index 4099759..7c17873 100644 --- a/proposals/274-rotate-onion-keys-less.txt +++ b/proposals/274-rotate-onion-keys-less.txt @@ -41,7 +41,7 @@ Implemented-In: 0.3.1.1-alpha The number of microdescriptors to actually download should decrease by a similar number. - This amount to a significant reduction: currently, by + This amounts to a significant reduction: currently, by back-of-the-envelope estimates, an always-on client that downloads all the directory info in a month downloads about 449MB of compressed consensuses and something around 97 MB of compressed

1 0

[atlas/master] Remove the data-original-title attribute
by irl＠torproject.org 02 May '17

02 May '17

commit 7b0f624a3f2c69231570dd964400fade8510ec30 Author: cypherpunks <cypherpunks(a)torproject.org> Date: Sun Apr 23 19:38:05 2017 +0000 Remove the data-original-title attribute The Bootstrap Tooltip plugin overwrites the data-original-title attribute when the title attribute is specified. All of the elements that specify the data-original-title attribute also specify the title attribute which makes the former serve no purpose. Also the title attribute has been standardized while the data-original-title attribute is not. Closes #22047. --- templates/details/bridge.html | 24 +++++++++++----------- templates/details/router.html | 46 +++++++++++++++++++++---------------------- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/templates/details/bridge.html b/templates/details/bridge.html index dae5a6b..fda73f6 100644 --- a/templates/details/bridge.html +++ b/templates/details/bridge.html @@ -14,14 +14,14 @@ <div class="span6"> <h3>Configuration</h3> <dl> - <dt>Nickname</dt> + <dt>Nickname</dt> <dd><%= _.escape(relay.get('nickname')) %></dd> - <dt>OR Addresses</dt> + <dt>OR Addresses</dt> <dd><%= relay.get('or_addresses') %></dd> <dt> - Advertised Bandwidth + Advertised Bandwidth </dt> <dd><%= relay.get('bandwidth_hr') ? relay.get('bandwidth_hr') : 'not defined' %></dd> </dl> @@ -36,20 +36,20 @@ <% } %> <dl> - <dt>Hashed Fingerprint</dt> + <dt>Hashed Fingerprint</dt> <dd><pre><%= relay.get('fingerprint') %></pre></dd> <% if (relay.get('running') === true) { %> - <dt>Uptime</dt> + <dt>Uptime</dt> <dd><%= relay.get('uptime_hrfull') %></dd> <% } else { %> - <dt>Downtime</dt> + <dt>Downtime</dt> <dd><%= relay.get('downtime') %></dd> - <dt>Last Seen</dt> + <dt>Last Seen</dt> <dd><%= relay.get('last_seen') %></dd> <% } %> - <dt>Flags</dt> + <dt>Flags</dt> <dd><% _.each(relay.get('flags'), function(flag) { %> "> <img class="inline" alt="" src="img/flags/<%= flag[1] %>.png"/> @@ -58,7 +58,7 @@ <% }); %> </dd> - <dt>Properties</dt> + <dt>Properties</dt> <dd> <% var propertyCount = 0; %> <% if (relay.get('recommended_version') === false) { @@ -72,13 +72,13 @@ <%=propertyCount==0?'none':''%> </dd> - <dt>Last Restarted</dt> + <dt>Last Restarted</dt> <dd><%= relay.get('last_restarted') %></dd> - <dt>Platform</dt> + <dt>Platform</dt> <dd><%= _.escape(relay.get('platform')) %></dd> - <dt>Transport protocols</dt> + <dt>Transport protocols</dt> <dd><%= relay.get('transports') ? relay.get('transports') : 'none' %></dd> </dl> diff --git a/templates/details/router.html b/templates/details/router.html index 5ffec67..430091b 100644 --- a/templates/details/router.html +++ b/templates/details/router.html @@ -14,23 +14,23 @@ <div class="span6"> <h3>Configuration</h3> <dl> - <dt>Nickname</dt> + <dt>Nickname</dt> <dd><%= _.escape(relay.get('nickname')) %></dd> - <dt>OR Addresses</dt> + <dt>OR Addresses</dt> <dd><%= relay.get('or_addresses') %></dd> - <dt>Contact</dt> + <dt>Contact</dt> <dd><%= _.escape( relay.get('contact')!='undefined' ? relay.get('contact') : 'none' ) %></dd> - <dt>Dir Address</dt> + <dt>Dir Address</dt> <dd><%= relay.get('dir_address') ? relay.get('dir_address') : 'none' %></dd> <dt> - Advertised Bandwidth + Advertised Bandwidth </dt> - <dd> Bandwidth burst: <%= relay.get('bandwidthb') %> Observed bandwidth: <%= relay.get('obandwidth') %>" data-original-title="Bandwidth values"><%= relay.get('bandwidth_hr') %></dd> - <dt>IPv4 Exit Policy Summary</dt> + <dd> Bandwidth burst: <%= relay.get('bandwidthb') %> Observed bandwidth: <%= relay.get('obandwidth') %>"><%= relay.get('bandwidth_hr') %></dd> + <dt>IPv4 Exit Policy Summary</dt> <dd> <% if ('accept' in relay.get('exit_policy_summary')) { %> <pre class="pre-scrollable">accept<% _.each(relay.get('exit_policy_summary')['accept'], @@ -39,7 +39,7 @@ <pre class="pre-scrollable">reject<% _.each(relay.get('exit_policy_summary')['reject'], function(ports) { %> <%= $.trim(ports) %><% });} %></pre> </dd> - <dt>IPv6 Exit Policy Summary</dt> + <dt>IPv6 Exit Policy Summary</dt> <dd> <% if (relay.get('exit_policy_v6_summary') && ('accept' in relay.get('exit_policy_v6_summary'))) { %> <pre class="pre-scrollable">accept<% _.each(relay.get('exit_policy_v6_summary')['accept'], @@ -51,7 +51,7 @@ <pre class="pre-scrollable">reject 1-65535</pre> <% } %> </dd> - <dt>Exit Policy</dt> + <dt>Exit Policy</dt> <dd> <pre class="pre-scrollable"><% _.each(relay.get('exit_policy'), function(line) { %> <%= $.trim(line) %><% }); %></pre> @@ -68,20 +68,20 @@ <% } %> <dl> - <dt>Fingerprint</dt> + <dt>Fingerprint</dt> <dd><pre><%= relay.get('fingerprint') %></pre></dd> <% if (relay.get('running') === true) { %> - <dt>Uptime</dt> + <dt>Uptime</dt> <dd><%= relay.get('uptime_hrfull') %></dd> <% } else { %> - <dt>Downtime</dt> + <dt>Downtime</dt> <dd><%= relay.get('downtime') %></dd> - <dt>Last Seen</dt> + <dt>Last Seen</dt> <dd><%= relay.get('last_seen') %></dd> <% } %> - <dt>Flags</dt> + <dt>Flags</dt> <dd><% _.each(relay.get('flags'), function(flag) { %> "> <img class="inline" alt="" src="img/flags/<%= flag[1] %>.png"/> @@ -90,7 +90,7 @@ <% }); %> </dd> - <dt>Properties</dt> + <dt>Properties</dt> <dd> <% var propertyCount = 0; %> <% if (relay.get('recommended_version') === false) { @@ -104,35 +104,35 @@ <%=propertyCount==0?'none':''%> </dd> - <dt>Country</dt> + <dt>Country</dt> <dd><% if (relay.get('country')) { %><img class="inline country" title="<%= relay.get('countryname') %>" alt="" src="img/cc/<%= relay.get('country') %>.png"/> <%= relay.get('countryname') %><% } else { %>Unknown<% } %></dd> - <dt>AS Number</dt> + <dt>AS Number</dt> <dd><%= relay.get('as_no') ? _.escape(relay.get('as_no')) : "unknown" %></dd> - <dt>AS Name</dt> + <dt>AS Name</dt> <dd><%= relay.get('as_name') ? _.escape(relay.get('as_name')) : "unknown" %></dd> - <dt>Last Restarted</dt> + <dt>Last Restarted</dt> <dd><%= relay.get('last_restarted') %></dd> - <dt>Effective Family Members</dt> + <dt>Effective Family Members</dt> <dd> <pre><% if (relay.get('effective_family') !== null) { %><% _.each(relay.get('effective_family'), function(member) { %><% var valid = member.match(RegExp("^\$[A-F0-9]{40}$")); %> <% if (valid) { %><a href="#details/<%= member.replace('$', '') %>" title="Effective family member"><% } %><%= member %><% if (valid) { %></a><% }}); %> <% } else { %>none<% } %></pre> </dd> - <dt>Alleged Family Members</dt> + <dt>Alleged Family Members</dt> <dd> <pre><% if (relay.get('alleged_family') !== null) { %><% _.each(relay.get('alleged_family'), function(member) { %><% var valid = member.match(RegExp("^\$[A-F0-9]{40}$")); %> <% if (valid) { %><a href="#details/<%= member.replace('$', '') %>" title="Alleged family member" class="alleged"><% } %><%= member %><% if (valid) { %></a><% }}); %> <% } else { %>none<% } %></pre> </dd> - <dt>Platform</dt> + <dt>Platform</dt> <dd><%= _.escape(relay.get('platform')) %></dd> - <dt><span class="tip" title="Weight assigned to this relay by the directory authorities that clients use in their path selection algorithm. The unit is arbitrary; currently it's kilobytes per second, but that might change in the future." data-original-title="Consensus Weight">Consensus Weight</dt> + <dt><span class="tip" title="Weight assigned to this relay by the directory authorities that clients use in their path selection algorithm. The unit is arbitrary; currently it's kilobytes per second, but that might change in the future.">Consensus Weight</dt> <dd><%= relay.get('consensus_weight') %></dd> </dl>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits May 2017