August 2017 - tor-commits - lists.torproject.org

[tor/master] Don't use "0" as a "base" argument to tor_parse_*().
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit e37c1df9cd79bb8c540aded6ff35ea4cde8ddc60 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 9 09:55:12 2017 -0400 Don't use "0" as a "base" argument to tor_parse_*(). Telling these functions to autodetect the numeric base has lead to trouble in the past. Fixes bug 22469. Bugfix on 0.2.2.various. --- changes/bug22802 | 10 ++++++++++ src/common/procmon.c | 2 +- src/or/circuitstats.c | 4 ++-- src/or/dirserv.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/changes/bug22802 b/changes/bug22802 new file mode 100644 index 000000000..7255164fd --- /dev/null +++ b/changes/bug22802 @@ -0,0 +1,10 @@ + o Minor bugfixes (format strictness): + - Restrict several data formats to decimal. Previously, the + BuildTimeHistogram entries in the state file, the "bw=" entries in the + bandwidth authority file, and process IDs passed to the + __OwningControllerProcess option could all be specified in hex or octal + as well as in decimal. This was not an intentional feature. + Fixes bug 22802; bugfixes on 0.2.2.1-alpha, 0.2.2.2-alpha, and + 0.2.2.28-beta. + + diff --git a/src/common/procmon.c b/src/common/procmon.c index d49e7f18f..5ef16c726 100644 --- a/src/common/procmon.c +++ b/src/common/procmon.c @@ -71,7 +71,7 @@ parse_process_specifier(const char *process_spec, /* If we're lucky, long will turn out to be large enough to hold a * PID everywhere that Tor runs. */ - pid_l = tor_parse_long(process_spec, 0, 1, LONG_MAX, &pid_ok, &pspec_next); + pid_l = tor_parse_long(process_spec, 10, 1, LONG_MAX, &pid_ok, &pspec_next); /* Reserve room in the ‘process specifier’ for additional * (platform-specific) identifying information beyond the PID, to diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c index 51d580a1a..963892c9d 100644 --- a/src/or/circuitstats.c +++ b/src/or/circuitstats.c @@ -939,7 +939,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, uint32_t count, k; build_time_t ms; int ok; - ms = (build_time_t)tor_parse_ulong(ms_str, 0, 0, + ms = (build_time_t)tor_parse_ulong(ms_str, 10, 0, CBT_BUILD_TIME_MAX, &ok, NULL); if (!ok) { log_warn(LD_GENERAL, "Unable to parse circuit build times: " @@ -949,7 +949,7 @@ circuit_build_times_parse_state(circuit_build_times_t *cbt, smartlist_free(args); break; } - count = (uint32_t)tor_parse_ulong(count_str, 0, 0, + count = (uint32_t)tor_parse_ulong(count_str, 10, 0, UINT32_MAX, &ok, NULL); if (!ok) { log_warn(LD_GENERAL, "Unable to parse circuit build times: " diff --git a/src/or/dirserv.c b/src/or/dirserv.c index e5654e3b9..01f70858e 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2700,7 +2700,7 @@ measured_bw_line_parse(measured_bw_line_t *out, const char *orig_line) } cp+=strlen("bw="); - out->bw_kb = tor_parse_long(cp, 0, 0, LONG_MAX, &parse_ok, &endptr); + out->bw_kb = tor_parse_long(cp, 10, 0, LONG_MAX, &parse_ok, &endptr); if (!parse_ok || (*endptr && !TOR_ISSPACE(*endptr))) { log_warn(LD_DIRSERV, "Invalid bandwidth in bandwidth file line: %s", escaped(orig_line));

1 0

[tor/master] Don't fall back to _atoi64
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit b88d00fea35630d96bf91bda362922af43730a04 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 9 09:58:16 2017 -0400 Don't fall back to _atoi64 We only did this on windows when building with MSVC 6 and earlier, which is now considered a screamingly bad idea. --- src/common/util.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 0858d17fe..9cd9ebdc4 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -1233,15 +1233,7 @@ tor_parse_uint64(const char *s, int base, uint64_t min, #ifdef HAVE_STRTOULL r = (uint64_t)strtoull(s, &endptr, base); #elif defined(_WIN32) -#if defined(_MSC_VER) && _MSC_VER < 1300 - tor_assert(base <= 10); - r = (uint64_t)_atoi64(s); - endptr = (char*)s; - while (TOR_ISSPACE(*endptr)) endptr++; - while (TOR_ISDIGIT(*endptr)) endptr++; -#else r = (uint64_t)_strtoui64(s, &endptr, base); -#endif #elif SIZEOF_LONG == 8 r = (uint64_t)strtoul(s, &endptr, base); #else

1 0

[tor/master] Merge branch 'bug22802_squashed'
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit 4b4b3afb569a40db99c1614bc83d835b0a23fe27 Merge: 5b8956df3 b88d00fea Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 28 10:23:05 2017 -0400 Merge branch 'bug22802_squashed' changes/bug22802 | 10 ++++++++++ src/common/procmon.c | 2 +- src/common/util.c | 8 -------- src/or/circuitstats.c | 4 ++-- src/or/dirserv.c | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-)

1 0

[tor/master] In test_establish_intro_wrong_purpose, use tt_i64_op on ssize_t
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit 5b8956df3b3bc4593c0372703cfb07d75b4fc15e Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 28 10:11:49 2017 -0400 In test_establish_intro_wrong_purpose, use tt_i64_op on ssize_t Since ssize_t is signed and might be 64 bits, we should use tt_i64_op to make sure it's positive. Otherwise, if it is negative, and we use tt_u64_op, we'll be treating it as a uint64_t, and we won't detect negative values. This fixes CID 1416338 and 1416339. Bug not in any released Tor. --- src/test/test_hs_intropoint.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 2c8f78008..2cce8a370 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -48,10 +48,10 @@ new_establish_intro_cell(const char *circ_nonce, ip = service_intro_point_new(NULL, 0); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, buf); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); cell_len = trn_cell_establish_intro_parse(&cell, buf, sizeof(buf)); - tt_int_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); tt_assert(cell); *cell_out = cell; @@ -71,7 +71,7 @@ new_establish_intro_encoded_cell(const char *circ_nonce, uint8_t *cell_out) ip = service_intro_point_new(NULL, 0); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, cell_out); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); done: service_intro_point_free(ip); @@ -184,7 +184,7 @@ test_establish_intro_wrong_purpose(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we attempt to parse it. */ cell_len = new_establish_intro_encoded_cell(circ_nonce, cell_body); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); /* Receive the cell. Should fail. */ setup_full_capture_of_logs(LOG_INFO); @@ -250,7 +250,7 @@ test_establish_intro_wrong_keytype2(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we * attempt to parse it. */ cell_len = new_establish_intro_encoded_cell(circ_nonce, cell_body); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); /* Mutate the auth key type! :) */ cell_body[0] = 42; @@ -286,7 +286,7 @@ test_establish_intro_wrong_mac(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we * attempt to parse it. */ cell_len = new_establish_intro_cell(circ_nonce, &cell); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); tt_assert(cell); /* Mangle one byte of the MAC. */ @@ -307,7 +307,7 @@ test_establish_intro_wrong_mac(void *arg) /* Encode payload so we can sign it. */ cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body), cell); - tt_int_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); retval = ed25519_sign_prefixed(&sig, cell_body, cell_len - @@ -321,7 +321,7 @@ test_establish_intro_wrong_mac(void *arg) /* Re-encode with the new signature. */ cell_len = trn_cell_establish_intro_encode(cell_body, sizeof(cell_body), cell); - tt_int_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); } /* Receive the cell. Should fail because our MAC is wrong. */ @@ -358,7 +358,7 @@ test_establish_intro_wrong_auth_key_len(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we * attempt to parse it. */ cell_len = new_establish_intro_cell(circ_nonce, &cell); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); tt_assert(cell); /* Mangle the auth key length. */ @@ -403,7 +403,7 @@ test_establish_intro_wrong_sig_len(void *arg) /* Create outgoing ESTABLISH_INTRO cell and extract its payload so that we * attempt to parse it. */ cell_len = new_establish_intro_cell(circ_nonce, &cell); - tt_u64_op(cell_len, OP_GT, 0); + tt_i64_op(cell_len, OP_GT, 0); tt_assert(cell); /* Mangle the signature length. */

1 0

[tor/master] Fix unlikely memory leak introduced in 418f3d6298beb27e050
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit 8de4a80125ad674c2ed3b34920ac221693061794 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 28 10:08:52 2017 -0400 Fix unlikely memory leak introduced in 418f3d6298beb27e050 This is CID 1416880; bug not in any released Tor. --- src/or/connection_or.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index c776afe94..fc304e6f1 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1549,6 +1549,7 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, if (identity_rcvd) { if (crypto_pk_get_digest(identity_rcvd, digest_rcvd_out) < 0) { + crypto_pk_free(identity_rcvd); return -1; } } else {

1 0

[tor/master] Silence some leftover warnings.
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit ad56a342b400ef57bd2a4087c5d53d48098418bc Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Aug 28 14:54:36 2017 +0300 Silence some leftover warnings. --- src/or/hs_service.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index a207fe331..5ff118222 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2392,10 +2392,10 @@ service_desc_hsdirs_changed(const hs_service_t *service, /* Check whether the set of HSDirs changed */ if (!smartlist_strings_eq(b64_responsible_dirs, desc->previous_hsdirs)) { - log_warn(LD_GENERAL, "Received new dirinfo and set of hsdirs changed!"); + log_info(LD_GENERAL, "Received new dirinfo and set of hsdirs changed!"); retval = 1; } else { - log_warn(LD_GENERAL, "No change in hsdir set!"); + log_debug(LD_GENERAL, "No change in hsdir set!"); } done:

1 0

[tor/master] Fix compilation warning on old clangs.
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit b1cb16867e6520bb943d68a9ce804099135e52a7 Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Aug 28 15:00:09 2017 +0300 Fix compilation warning on old clangs. --- src/or/directory.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/or/directory.h b/src/or/directory.h index fc71bf800..42bcb55eb 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -188,7 +188,6 @@ STATIC char *accept_encoding_header(void); STATIC int allowed_anonymous_connection_compression_method(compress_method_t); STATIC void warn_disallowed_anonymous_compression_method(compress_method_t); -typedef struct response_handler_args_t response_handler_args_t; STATIC int handle_response_fetch_hsdesc_v3(dir_connection_t *conn, const response_handler_args_t *args);

1 0

[tor/master] Merge remote-tracking branch 'asn/bug23335'
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit b27c029266b3d72011b58cea2795a65613a422f6 Merge: e8c584176 b1cb16867 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 28 10:05:21 2017 -0400 Merge remote-tracking branch 'asn/bug23335' src/or/directory.h | 1 - src/or/hs_service.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-)

1 0

[tor/master] hs: Note the connection attempt if descriptor is unusable
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit e8c584176b92799126f9280a51f45e40db3cc119 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Aug 25 13:39:40 2017 -0400 hs: Note the connection attempt if descriptor is unusable This way, we can clear off the directory requests from our cache and thus allow the next client to query those HSDir again at the next SOCKS connection. Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 9c2aa34e9..99be058eb 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -1103,7 +1103,9 @@ hs_client_desc_has_arrived(const hs_ident_dir_conn_t *ident) "Closing streams."); connection_mark_unattached_ap(entry_conn, END_STREAM_REASON_RESOLVEFAILED); - /* XXX: Note the connection attempt. */ + /* We are unable to use the descriptor so remove the directory request + * from the cache so the next connection can try again. */ + note_connection_attempt_succeeded(edge_conn->hs_ident); goto end; }

1 0

[tor/master] hs: Implement note_connection_attempt_succeeded()
by nickm＠torproject.org 28 Aug '17

28 Aug '17

commit 11443bb74c4c97014046b0a4ec404a4659881e5a Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Aug 25 11:30:31 2017 -0400 hs: Implement note_connection_attempt_succeeded() v3 client now cleans up the HSDir request cache when a connection to a service was successful. Closes #23308 Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/hs_client.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/or/hs_client.c b/src/or/hs_client.c index c0e24ac85..9c2aa34e9 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -57,15 +57,49 @@ flag_all_conn_wait_desc(const ed25519_public_key_t *service_identity_pk) smartlist_free(conns); } +/* Remove tracked HSDir requests from our history for this hidden service + * identity public key. */ +static void +purge_hid_serv_request(const ed25519_public_key_t *identity_pk) +{ + char base64_blinded_pk[ED25519_BASE64_LEN + 1]; + ed25519_public_key_t blinded_pk; + + tor_assert(identity_pk); + + /* Get blinded pubkey of hidden service. It is possible that we just moved + * to a new time period meaning that we won't be able to purge the request + * from the previous time period. That is fine because they will expire at + * some point and we don't care about those anymore. */ + hs_build_blinded_pubkey(identity_pk, NULL, 0, + hs_get_time_period_num(approx_time()), &blinded_pk); + if (BUG(ed25519_public_to_base64(base64_blinded_pk, &blinded_pk) < 0)) { + return; + } + /* Purge last hidden service request from cache for this blinded key. */ + hs_purge_hid_serv_from_last_hid_serv_requests(base64_blinded_pk); +} + /* A v3 HS circuit successfully connected to the hidden service. Update the * stream state at <b>hs_conn_ident</b> appropriately. */ static void note_connection_attempt_succeeded(const hs_ident_edge_conn_t *hs_conn_ident) { - (void) hs_conn_ident; - - /* TODO: When implementing client side */ - return; + tor_assert(hs_conn_ident); + + /* Remove from the hid serv cache all requests for that service so we can + * query the HSDir again later on for various reasons. */ + purge_hid_serv_request(&hs_conn_ident->identity_pk); + + /* The v2 subsystem cleans up the intro point time out flag at this stage. + * We don't try to do it here because we still need to keep intact the intro + * point state for future connections. Even though we are able to connect to + * the service, doesn't mean we should reset the timed out intro points. + * + * It is not possible to have successfully connected to an intro point + * present in our cache that was on error or timed out. Every entry in that + * cache have a 2 minutes lifetime so ultimately the intro point(s) state + * will be reset and thus possible to be retried. */ } /* Given the pubkey of a hidden service in <b>onion_identity_pk</b>, fetch its

1 0