October 2018 - tor-commits - lists.torproject.org

[tor/master] Remove a double-newline and improve a comment
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit df2b46d18c4a3d6e5eb364f80111ef6c7811383c Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Oct 14 15:33:34 2018 -0400 Remove a double-newline and improve a comment --- src/core/or/connection_edge.h | 1 - src/feature/nodelist/torcert.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/or/connection_edge.h b/src/core/or/connection_edge.h index 6472de008..55b90d3ea 100644 --- a/src/core/or/connection_edge.h +++ b/src/core/or/connection_edge.h @@ -265,7 +265,6 @@ STATIC int connection_ap_process_http_connect(entry_connection_t *conn); STATIC void export_hs_client_circuit_id(edge_connection_t *edge_conn, hs_circuit_id_protocol_t protocol); - struct half_edge_t; STATIC void connection_half_edge_add(const edge_connection_t *conn, origin_circuit_t *circ); diff --git a/src/feature/nodelist/torcert.c b/src/feature/nodelist/torcert.c index 9962ed653..938d7d09f 100644 --- a/src/feature/nodelist/torcert.c +++ b/src/feature/nodelist/torcert.c @@ -51,7 +51,7 @@ tor_cert_sign_impl(const ed25519_keypair_t *signing_key, tor_cert_t *torcert = NULL; ed25519_cert_t *cert = ed25519_cert_new(); - tor_assert(cert); // Trunnel's new functions can return NULL. + tor_assert(cert); // Unlike Tor's, Trunnel's "new" functions can return NULL. cert->cert_type = cert_type; cert->exp_field = (uint32_t) CEIL_DIV(now + lifetime, 3600); cert->cert_key_type = signed_key_type;

1 0

[tor/master] Fix various GCC LTO warnings in the unit tests.
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 6925b61cfdf50a5686de02645b04b269c031f05d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 15:32:25 2018 -0400 Fix various GCC LTO warnings in the unit tests. --- src/test/test_hs_service.c | 1 + src/test/test_routerset.c | 3 +++ src/test/test_storagedir.c | 2 +- src/test/test_tortls.c | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index bceeafd14..d6404bd71 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -534,6 +534,7 @@ test_load_keys_with_client_auth(void *arg) tt_int_op(get_hs_service_map_size(), OP_EQ, 1); service = get_first_service(); + tt_assert(service); tt_assert(service->config.clients); tt_int_op(smartlist_len(service->config.clients), OP_EQ, smartlist_len(pubkey_b32_list)); diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c index 2017ef005..db7a6a54e 100644 --- a/src/test/test_routerset.c +++ b/src/test/test_routerset.c @@ -1496,6 +1496,7 @@ NS(test_main)(void *arg) int r; (void)arg; + memset(&NS(mock_node), 0, sizeof(NS(mock_node))); NS(mock_node).ri = NULL; NS(mock_node).rs = NULL; @@ -1529,6 +1530,7 @@ NS(test_main)(void *arg) strncpy(rs.nickname, nickname, sizeof(rs.nickname) - 1); rs.nickname[sizeof(rs.nickname) - 1] = '\0'; + memset(&NS(mock_node), 0, sizeof(NS(mock_node))); NS(mock_node).ri = NULL; NS(mock_node).rs = &rs; @@ -1560,6 +1562,7 @@ NS(test_main)(void *arg) strmap_set_lc(set->names, nickname, (void *)1); ri.nickname = (char *)nickname; + memset(&mock_node, 0, sizeof(mock_node)); mock_node.ri = &ri; mock_node.rs = NULL; diff --git a/src/test/test_storagedir.c b/src/test/test_storagedir.c index 68cee418a..76aae7e03 100644 --- a/src/test/test_storagedir.c +++ b/src/test/test_storagedir.c @@ -283,7 +283,7 @@ test_storagedir_save_labeled(void *arg) int r = storage_dir_save_labeled_to_file(d, labels, inp, 8192, &fname); tt_int_op(r, OP_EQ, 0); - size_t n; + size_t n = 0; saved = storage_dir_read(d, fname, 1, &n); tt_assert(memchr(saved, '\0', n)); tt_str_op((char*)saved, OP_EQ, expected); /* NUL guarantees strcmp works */ diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index f4315364a..79b52437f 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -238,6 +238,7 @@ test_tortls_x509_cert_get_id_digests(void *ignored) cert->pkey_digests_set = 1; cert->pkey_digests = *d; res = tor_x509_cert_get_id_digests(cert); + tt_assert(res); tt_int_op(res->d[0][0], OP_EQ, 42); done:

1 0

[tor/master] In tor_tls_get_my_certs(), set cert ptrs even on failure
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 7c8f20ba44a831ed9f714453fa776762d1c872c5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 15:18:52 2018 -0400 In tor_tls_get_my_certs(), set cert ptrs even on failure Nothing should ever look at them on failure, but in some cases, the unit tests don't check for failure, and then GCC-LTO freaks out. Fixes part of 27772. --- src/lib/tls/tortls.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/lib/tls/tortls.c b/src/lib/tls/tortls.c index 3ae3a1a09..56f70bc37 100644 --- a/src/lib/tls/tortls.c +++ b/src/lib/tls/tortls.c @@ -71,13 +71,19 @@ tor_tls_get_my_certs(int server, const tor_x509_cert_t **id_cert_out) { tor_tls_context_t *ctx = tor_tls_context_get(server); - if (! ctx) - return -1; + int rv = -1; + const tor_x509_cert_t *link_cert = NULL; + const tor_x509_cert_t *id_cert = NULL; + if (ctx) { + rv = 0; + link_cert = server ? ctx->my_link_cert : ctx->my_auth_cert; + id_cert = ctx->my_id_cert; + } if (link_cert_out) - *link_cert_out = server ? ctx->my_link_cert : ctx->my_auth_cert; + *link_cert_out = link_cert; if (id_cert_out) - *id_cert_out = ctx->my_id_cert; - return 0; + *id_cert_out = id_cert; + return rv; } /**

1 0

[tor/master] Merge branch 'bug27772_squashed'
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 2dccef0eb490fa7d8fabc0d93e37d6635910f174 Merge: 2c5c17510 6925b61cf Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Oct 14 15:31:52 2018 -0400 Merge branch 'bug27772_squashed' src/core/mainloop/connection.c | 11 ++++--- src/core/or/channel.c | 2 ++ src/core/or/channelpadding.c | 2 ++ src/core/or/connection_edge.c | 5 --- src/core/or/connection_edge.h | 10 ++++-- src/core/or/scheduler_kist.c | 4 +++ src/core/proto/proto_socks.c | 5 ++- src/feature/dirparse/policy_parse.c | 10 ++++-- src/feature/nodelist/torcert.c | 1 + src/lib/crypt_ops/crypto_pwbox.c | 1 + src/lib/tls/tortls.c | 16 +++++++--- src/test/test_hs_service.c | 1 + src/test/test_relaycell.c | 63 +++++++++++++++---------------------- src/test/test_routerset.c | 3 ++ src/test/test_storagedir.c | 2 +- src/test/test_tortls.c | 1 + 16 files changed, 79 insertions(+), 58 deletions(-) diff --cc src/core/or/connection_edge.h index 6dee93f1f,25e04f841..6472de008 --- a/src/core/or/connection_edge.h +++ b/src/core/or/connection_edge.h @@@ -262,10 -254,12 +262,16 @@@ STATIC void connection_ap_handshake_rew rewrite_result_t *out); STATIC int connection_ap_process_http_connect(entry_connection_t *conn); - STATIC void - export_hs_client_circuit_id(edge_connection_t *edge_conn, ++STATIC void export_hs_client_circuit_id(edge_connection_t *edge_conn, + hs_circuit_id_protocol_t protocol); + ++ + struct half_edge_t; + STATIC void connection_half_edge_add(const edge_connection_t *conn, + origin_circuit_t *circ); + STATIC struct half_edge_t *connection_half_edge_find_stream_id( + const smartlist_t *half_conns, + streamid_t stream_id); #endif /* defined(CONNECTION_EDGE_PRIVATE) */ #endif /* !defined(TOR_CONNECTION_EDGE_H) */ diff --cc src/feature/dirparse/policy_parse.c index e102a6228,000000000..f9102dd87 mode 100644,000000..100644 --- a/src/feature/dirparse/policy_parse.c +++ b/src/feature/dirparse/policy_parse.c @@@ -1,218 -1,0 +1,224 @@@ +/* Copyright (c) 2001 Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file policy_parse.c + * \brief Code to parse address policies. + **/ + +#define EXPOSE_ROUTERDESC_TOKEN_TABLE + +#include "core/or/or.h" + +#include "core/or/policies.h" +#include "feature/dirparse/parsecommon.h" +#include "feature/dirparse/policy_parse.h" +#include "feature/dirparse/routerparse.h" +#include "feature/dirparse/unparseable.h" +#include "lib/memarea/memarea.h" + +#include "core/or/addr_policy_st.h" + +static addr_policy_t *router_parse_addr_policy_private(directory_token_t *tok); + +/** Parse the addr policy in the string <b>s</b> and return it. If + * assume_action is nonnegative, then insert its action (ADDR_POLICY_ACCEPT or + * ADDR_POLICY_REJECT) for items that specify no action. + * + * Returns NULL on policy errors. + * + * Set *<b>malformed_list</b> to true if the entire policy list should be + * discarded. Otherwise, set it to false, and only this item should be ignored + * on error - the rest of the policy list can continue to be processed and + * used. + * + * The addr_policy_t returned by this function can have its address set to + * AF_UNSPEC for '*'. Use policy_expand_unspec() to turn this into a pair + * of AF_INET and AF_INET6 items. + */ +MOCK_IMPL(addr_policy_t *, +router_parse_addr_policy_item_from_string,(const char *s, int assume_action, + int *malformed_list)) +{ + directory_token_t *tok = NULL; + const char *cp, *eos; + /* Longest possible policy is + * "accept6 [ffff:ffff:..255]/128:10000-65535", + * which contains a max-length IPv6 address, plus 26 characters. + * But note that there can be an arbitrary amount of space between the + * accept and the address:mask/port element. + * We don't need to multiply TOR_ADDR_BUF_LEN by 2, as there is only one + * IPv6 address. But making the buffer shorter might cause valid long lines, + * which parsed in previous versions, to fail to parse in new versions. + * (These lines would have to have excessive amounts of whitespace.) */ + char line[TOR_ADDR_BUF_LEN*2 + 32]; + addr_policy_t *r; + memarea_t *area = NULL; + + tor_assert(malformed_list); + *malformed_list = 0; + + s = eat_whitespace(s); + /* We can only do assume_action on []-quoted IPv6, as "a" (accept) + * and ":" (port separator) are ambiguous */ + if ((*s == '*' || *s == '[' || TOR_ISDIGIT(*s)) && assume_action >= 0) { + if (tor_snprintf(line, sizeof(line), "%s %s", + assume_action == ADDR_POLICY_ACCEPT?"accept":"reject", s)<0) { + log_warn(LD_DIR, "Policy %s is too long.", escaped(s)); + return NULL; + } + cp = line; + tor_strlower(line); + } else { /* assume an already well-formed address policy line */ + cp = s; + } + + eos = cp + strlen(cp); + area = memarea_new(); + tok = get_next_token(area, &cp, eos, routerdesc_token_table); + if (tok->tp == ERR_) { + log_warn(LD_DIR, "Error reading address policy: %s", tok->error); + goto err; + } + if (tok->tp != K_ACCEPT && tok->tp != K_ACCEPT6 && + tok->tp != K_REJECT && tok->tp != K_REJECT6) { + log_warn(LD_DIR, "Expected 'accept' or 'reject'."); + goto err; + } + + /* Use the extended interpretation of accept/reject *, + * expanding it into an IPv4 wildcard and an IPv6 wildcard. + * Also permit *4 and *6 for IPv4 and IPv6 only wildcards. */ + r = router_parse_addr_policy(tok, TAPMP_EXTENDED_STAR); + if (!r) { + goto err; + } + + /* Ensure that accept6/reject6 fields are followed by IPv6 addresses. + * AF_UNSPEC addresses are only permitted on the accept/reject field type. + * Unlike descriptors, torrcs exit policy accept/reject can be followed by + * either an IPv4 or IPv6 address. */ + if ((tok->tp == K_ACCEPT6 || tok->tp == K_REJECT6) && + tor_addr_family(&r->addr) != AF_INET6) { + /* This is a non-fatal error, just ignore this one entry. */ + *malformed_list = 0; + log_warn(LD_DIR, "IPv4 address '%s' with accept6/reject6 field type in " + "exit policy. Ignoring, but continuing to parse rules. (Use " + "accept/reject with IPv4 addresses.)", + tok->n_args == 1 ? tok->args[0] : ""); + addr_policy_free(r); + r = NULL; + goto done; + } + + goto done; + err: + *malformed_list = 1; + r = NULL; + done: + token_clear(tok); + if (area) { + DUMP_AREA(area, "policy item"); + memarea_drop_all(area); + } + return r; +} + +/** Given a K_ACCEPT[6] or K_REJECT[6] token and a router, create and return + * a new exit_policy_t corresponding to the token. If TAPMP_EXTENDED_STAR + * is set in fmt_flags, K_ACCEPT6 and K_REJECT6 tokens followed by * + * expand to IPv6-only policies, otherwise they expand to IPv4 and IPv6 + * policies */ +addr_policy_t * +router_parse_addr_policy(directory_token_t *tok, unsigned fmt_flags) +{ + addr_policy_t newe; + char *arg; + + tor_assert(tok->tp == K_REJECT || tok->tp == K_REJECT6 || + tok->tp == K_ACCEPT || tok->tp == K_ACCEPT6); + + if (tok->n_args != 1) + return NULL; + arg = tok->args[0]; + + if (!strcmpstart(arg,"private")) + return router_parse_addr_policy_private(tok); + + memset(&newe, 0, sizeof(newe)); + + if (tok->tp == K_REJECT || tok->tp == K_REJECT6) + newe.policy_type = ADDR_POLICY_REJECT; + else + newe.policy_type = ADDR_POLICY_ACCEPT; + + /* accept6/reject6 * produces an IPv6 wildcard address only. + * (accept/reject * produces rules for IPv4 and IPv6 wildcard addresses.) */ + if ((fmt_flags & TAPMP_EXTENDED_STAR) + && (tok->tp == K_ACCEPT6 || tok->tp == K_REJECT6)) { + fmt_flags |= TAPMP_STAR_IPV6_ONLY; + } + + if (tor_addr_parse_mask_ports(arg, fmt_flags, &newe.addr, &newe.maskbits, + &newe.prt_min, &newe.prt_max) < 0) { + log_warn(LD_DIR,"Couldn't parse line %s. Dropping", escaped(arg)); + return NULL; + } + - return addr_policy_get_canonical_entry(&newe); ++ addr_policy_t *result = addr_policy_get_canonical_entry(&newe); ++ /* It would be a nasty error to return 'newe', and sometimes ++ * addr_policy_get_canonical_entry() can return its argument. But in this ++ * case, it won't, since newe is *not* canonical. We assert here to make ++ * sure that the compiler knows it too. ++ */ ++ tor_assert(result != &newe); ++ return result; +} + +/** Parse an exit policy line of the format "accept[6]/reject[6] private:...". + * This didn't exist until Tor 0.1.1.15, so nobody should generate it in + * router descriptors until earlier versions are obsolete. + * + * accept/reject and accept6/reject6 private all produce rules for both + * IPv4 and IPv6 addresses. + */ +static addr_policy_t * +router_parse_addr_policy_private(directory_token_t *tok) +{ + const char *arg; + uint16_t port_min, port_max; + addr_policy_t result; + + arg = tok->args[0]; + if (strcmpstart(arg, "private")) + return NULL; + + arg += strlen("private"); + arg = (char*) eat_whitespace(arg); + if (!arg || *arg != ':') + return NULL; + + if (parse_port_range(arg+1, &port_min, &port_max)<0) + return NULL; + + memset(&result, 0, sizeof(result)); + if (tok->tp == K_REJECT || tok->tp == K_REJECT6) + result.policy_type = ADDR_POLICY_REJECT; + else + result.policy_type = ADDR_POLICY_ACCEPT; + result.is_private = 1; + result.prt_min = port_min; + result.prt_max = port_max; + + if (tok->tp == K_ACCEPT6 || tok->tp == K_REJECT6) { + log_warn(LD_GENERAL, + "'%s' expands into rules which apply to all private IPv4 and " + "IPv6 addresses. (Use accept/reject private:* for IPv4 and " + "IPv6.)", tok->n_args == 1 ? tok->args[0] : ""); + } + + return addr_policy_get_canonical_entry(&result); +} - diff --cc src/test/test_relaycell.c index 0f72a575a,e2d666d52..3d3addfb9 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@@ -5,8 -5,11 +5,11 @@@ #define RELAY_PRIVATE #define CIRCUITLIST_PRIVATE + #define CONNECTION_EDGE_PRIVATE + #define CONNECTION_PRIVATE + #include "core/or/or.h" -#include "core/mainloop/main.h" +#include "core/mainloop/mainloop.h" #include "app/config/config.h" #include "core/mainloop/connection.h" #include "lib/crypt_ops/crypto_cipher.h"

1 0

[tor/master] Assert that some trunnel _new() functions return non-NULL
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 7ace8d5a61f75fb77e3619deed417edd5610a4f1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 14:43:57 2018 -0400 Assert that some trunnel _new() functions return non-NULL The trunnel functions are written under the assumption that their allocators can fail, so GCC LTO thinks they might return NULL. In point of fact, they're using tor_malloc() and friends, which can't fail, but GCC won't necessarily figure that out. Fixes part of #27772. --- src/core/proto/proto_socks.c | 3 +++ src/feature/nodelist/torcert.c | 1 + src/lib/crypt_ops/crypto_pwbox.c | 1 + 3 files changed, 5 insertions(+) diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c index ccf96f781..e2f233ad0 100644 --- a/src/core/proto/proto_socks.c +++ b/src/core/proto/proto_socks.c @@ -353,6 +353,7 @@ process_socks5_methods_request(socks_request_t *req, int have_user_pass, { socks_result_t res = SOCKS_RESULT_DONE; socks5_server_method_t *trunnel_resp = socks5_server_method_new(); + tor_assert(trunnel_resp); socks5_server_method_set_version(trunnel_resp, SOCKS_VER_5); @@ -478,6 +479,7 @@ process_socks5_userpass_auth(socks_request_t *req) socks_result_t res = SOCKS_RESULT_DONE; socks5_server_userpass_auth_t *trunnel_resp = socks5_server_userpass_auth_new(); + tor_assert(trunnel_resp); if (req->socks_version != SOCKS_VER_5) { res = SOCKS_RESULT_INVALID; @@ -869,6 +871,7 @@ socks_request_set_socks5_error(socks_request_t *req, socks5_reply_status_t reason) { socks5_server_reply_t *trunnel_resp = socks5_server_reply_new(); + tor_assert(trunnel_resp); socks5_server_reply_set_version(trunnel_resp, SOCKS_VER_5); socks5_server_reply_set_reply(trunnel_resp, reason); diff --git a/src/feature/nodelist/torcert.c b/src/feature/nodelist/torcert.c index fe67e5640..f31e8ed77 100644 --- a/src/feature/nodelist/torcert.c +++ b/src/feature/nodelist/torcert.c @@ -51,6 +51,7 @@ tor_cert_sign_impl(const ed25519_keypair_t *signing_key, tor_cert_t *torcert = NULL; ed25519_cert_t *cert = ed25519_cert_new(); + tor_assert(cert); // Trunnel's new functions can return NULL. cert->cert_type = cert_type; cert->exp_field = (uint32_t) CEIL_DIV(now + lifetime, 3600); cert->cert_key_type = signed_key_type; diff --git a/src/lib/crypt_ops/crypto_pwbox.c b/src/lib/crypt_ops/crypto_pwbox.c index 2377f216a..91536e891 100644 --- a/src/lib/crypt_ops/crypto_pwbox.c +++ b/src/lib/crypt_ops/crypto_pwbox.c @@ -61,6 +61,7 @@ crypto_pwbox(uint8_t **out, size_t *outlen_out, int rv; enc = pwbox_encoded_new(); + tor_assert(enc); pwbox_encoded_setlen_skey_header(enc, S2K_MAXLEN);

1 0

[tor/master] Use the correct function signatures in test_relaycell.c
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 370d9922a4e3cb24a5ff093cadc3098771cab957 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 15:07:02 2018 -0400 Use the correct function signatures in test_relaycell.c This is now officially an antipattern: please let's never copy a function declaration in two places again. That's what headers are for. --- src/core/or/connection_edge.c | 5 ---- src/core/or/connection_edge.h | 6 +++++ src/test/test_relaycell.c | 63 ++++++++++++++++++------------------------- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c index 35e68485b..ab08e99b1 100644 --- a/src/core/or/connection_edge.c +++ b/src/core/or/connection_edge.c @@ -155,11 +155,6 @@ static int connection_ap_process_natd(entry_connection_t *conn); static int connection_exit_connect_dir(edge_connection_t *exitconn); static int consider_plaintext_ports(entry_connection_t *conn, uint16_t port); static int connection_ap_supports_optimistic_data(const entry_connection_t *); -STATIC void connection_half_edge_add(const edge_connection_t *conn, - origin_circuit_t *circ); -STATIC half_edge_t *connection_half_edge_find_stream_id( - const smartlist_t *half_conns, - streamid_t stream_id); /** Convert a connection_t* to an edge_connection_t*; assert if the cast is * invalid. */ diff --git a/src/core/or/connection_edge.h b/src/core/or/connection_edge.h index 1348dd49f..25e04f841 100644 --- a/src/core/or/connection_edge.h +++ b/src/core/or/connection_edge.h @@ -254,6 +254,12 @@ STATIC void connection_ap_handshake_rewrite(entry_connection_t *conn, rewrite_result_t *out); STATIC int connection_ap_process_http_connect(entry_connection_t *conn); +struct half_edge_t; +STATIC void connection_half_edge_add(const edge_connection_t *conn, + origin_circuit_t *circ); +STATIC struct half_edge_t *connection_half_edge_find_stream_id( + const smartlist_t *half_conns, + streamid_t stream_id); #endif /* defined(CONNECTION_EDGE_PRIVATE) */ #endif /* !defined(TOR_CONNECTION_EDGE_H) */ diff --git a/src/test/test_relaycell.c b/src/test/test_relaycell.c index 2d020ec47..e2d666d52 100644 --- a/src/test/test_relaycell.c +++ b/src/test/test_relaycell.c @@ -5,6 +5,9 @@ #define RELAY_PRIVATE #define CIRCUITLIST_PRIVATE +#define CONNECTION_EDGE_PRIVATE +#define CONNECTION_PRIVATE + #include "core/or/or.h" #include "core/mainloop/main.h" #include "app/config/config.h" @@ -25,6 +28,9 @@ #include "core/or/socks_request_st.h" #include "core/or/half_edge_st.h" +#include "feature/client/circpathbias.h" +#include "core/or/connection_edge.h" + static int srm_ncalls; static entry_connection_t *srm_conn; static int srm_atype; @@ -34,23 +40,6 @@ static uint8_t srm_answer[512]; static int srm_ttl; static time_t srm_expires; -void connection_free_minimal(connection_t*); -int connected_cell_format_payload(uint8_t *payload_out, - const tor_addr_t *addr, - uint32_t ttl); -void pathbias_count_valid_cells(origin_circuit_t *circ, - cell_t *cell); -half_edge_t *connection_half_edge_find_stream_id( - const smartlist_t *half_conns, - streamid_t stream_id); -void connection_half_edge_add(const edge_connection_t *conn, - origin_circuit_t *circ); - -int mock_send_command(streamid_t stream_id, circuit_t *circ, - uint8_t relay_command, const char *payload, - size_t payload_len, crypt_path_t *cpath_layer, - const char *filename, int lineno); - /* Mock replacement for connection_ap_hannshake_socks_resolved() */ static void socks_resolved_mock(entry_connection_t *conn, @@ -150,7 +139,7 @@ mock_start_reading(connection_t *conn) return; } -int +static int mock_send_command(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, size_t payload_len, crypt_path_t *cpath_layer, @@ -237,7 +226,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Data cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -246,7 +235,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Sendme cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_SENDME, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -255,7 +244,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Connected cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -264,7 +253,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) /* Resolved cell not in the half-opened list */ PACK_CELL(4000, RELAY_COMMAND_RESOLVED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -274,7 +263,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) edgeconn = ENTRY_TO_EDGE_CONN(entryconn2); PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -286,7 +275,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -297,7 +286,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -309,7 +298,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_SENDME, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -320,7 +309,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_SENDME, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -331,7 +320,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -341,7 +330,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn2)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -366,7 +355,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) connection_edge_reached_eof(edgeconn); PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -376,7 +365,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn3)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_CONNECTED, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -387,7 +376,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn3)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -406,7 +395,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn3)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -426,7 +415,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_RESOLVED, "\x04\x04\x12\x00\x00\x01\x00\x00\x02\x00"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -445,7 +434,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn4)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_DATA, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -456,7 +445,7 @@ subtest_circbw_halfclosed(origin_circuit_t *circ, streamid_t init_id) ENTRY_TO_CONN(entryconn4)->outbuf_flushlen = 0; PACK_CELL(edgeconn->stream_id, RELAY_COMMAND_END, "Data1234"); if (circ->base_.purpose == CIRCUIT_PURPOSE_PATH_BIAS_TESTING) - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); else connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL, circ->cpath); @@ -877,7 +866,7 @@ test_circbw_relay(void *arg) /* Path bias: truncated */ tt_int_op(circ->base_.marked_for_close, OP_EQ, 0); PACK_CELL(0, RELAY_COMMAND_TRUNCATED, "Data1234"); - pathbias_count_valid_cells(circ, &cell); + pathbias_count_valid_cells(TO_CIRCUIT(circ), &cell); tt_int_op(circ->base_.marked_for_close, OP_EQ, 1); done:

1 0

[tor/master] Initialize some locals in socks5 parsing code.
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit dddecee291cadf391d93b569023f1f1e008880e8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 15:13:10 2018 -0400 Initialize some locals in socks5 parsing code. These confused GCC LTO, which thought they might be used uninitialized. I'm pretty sure that as long as 'res' indicates success, they will always be set to something, but let's unconfuse the compiler in any case. --- src/core/proto/proto_socks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/proto/proto_socks.c b/src/core/proto/proto_socks.c index e2f233ad0..e23da7730 100644 --- a/src/core/proto/proto_socks.c +++ b/src/core/proto/proto_socks.c @@ -744,7 +744,7 @@ handle_socks_message(const uint8_t *raw_data, size_t datalen, res = SOCKS_RESULT_MORE_EXPECTED; goto end; } else if (req->socks_version != SOCKS_VER_5) { - int have_user_pass, have_no_auth; + int have_user_pass=0, have_no_auth=0; res = parse_socks5_methods_request(raw_data, req, datalen, &have_user_pass, &have_no_auth,

1 0

[tor/master] Use assertions so GCC LTO doesn't worry about TLS channel conversion
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 965549aa07d94a9f9e510cdb7a215bf9a3ed7bb8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 15:06:36 2018 -0400 Use assertions so GCC LTO doesn't worry about TLS channel conversion Part of #27772 --- src/core/mainloop/connection.c | 11 ++++++----- src/core/or/channel.c | 2 ++ src/core/or/channelpadding.c | 2 ++ src/core/or/scheduler_kist.c | 4 ++++ 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/core/mainloop/connection.c b/src/core/mainloop/connection.c index 0c3abc844..7e2c4ce40 100644 --- a/src/core/mainloop/connection.c +++ b/src/core/mainloop/connection.c @@ -656,14 +656,15 @@ connection_free_minimal(connection_t *conn) tor_free(or_conn->nickname); if (or_conn->chan) { /* Owww, this shouldn't happen, but... */ + channel_t *base_chan = TLS_CHAN_TO_BASE(or_conn->chan); + tor_assert(base_chan); log_info(LD_CHANNEL, "Freeing orconn at %p, saw channel %p with ID " "%"PRIu64 " left un-NULLed", - or_conn, TLS_CHAN_TO_BASE(or_conn->chan), - ( - TLS_CHAN_TO_BASE(or_conn->chan)->global_identifier)); - if (!CHANNEL_FINISHED(TLS_CHAN_TO_BASE(or_conn->chan))) { - channel_close_for_error(TLS_CHAN_TO_BASE(or_conn->chan)); + or_conn, base_chan, + base_chan->global_identifier); + if (!CHANNEL_FINISHED(base_chan)) { + channel_close_for_error(base_chan); } or_conn->chan->conn = NULL; diff --git a/src/core/or/channel.c b/src/core/or/channel.c index 0c204ddfb..79c52235d 100644 --- a/src/core/or/channel.c +++ b/src/core/or/channel.c @@ -3423,6 +3423,8 @@ channel_rsa_id_group_set_badness(struct channel_list_s *lst, int force) /* it would be more efficient to do a slice, but this case is rare */ smartlist_t *or_conns = smartlist_new(); SMARTLIST_FOREACH_BEGIN(channels, channel_t *, channel) { + tor_assert(channel); // Suppresses some compiler warnings. + if (!common_ed25519_identity) common_ed25519_identity = &channel->ed25519_identity; diff --git a/src/core/or/channelpadding.c b/src/core/or/channelpadding.c index 7c3a77f62..d3d6890c0 100644 --- a/src/core/or/channelpadding.c +++ b/src/core/or/channelpadding.c @@ -296,6 +296,7 @@ channelpadding_send_disable_command(channel_t *chan) channelpadding_negotiate_t disable; cell_t cell; + tor_assert(chan); tor_assert(BASE_CHAN_TO_TLS(chan)->conn->link_proto >= MIN_LINK_PROTO_FOR_CHANNEL_PADDING); @@ -328,6 +329,7 @@ channelpadding_send_enable_command(channel_t *chan, uint16_t low_timeout, channelpadding_negotiate_t enable; cell_t cell; + tor_assert(chan); tor_assert(BASE_CHAN_TO_TLS(chan)->conn->link_proto >= MIN_LINK_PROTO_FOR_CHANNEL_PADDING); diff --git a/src/core/or/scheduler_kist.c b/src/core/or/scheduler_kist.c index 41c346ac7..f112ea635 100644 --- a/src/core/or/scheduler_kist.c +++ b/src/core/or/scheduler_kist.c @@ -116,6 +116,7 @@ static unsigned int kist_lite_mode = 1; static inline size_t channel_outbuf_length(channel_t *chan) { + tor_assert(chan); /* In theory, this can not happen because we can not scheduler a channel * without a connection that has its outbuf initialized. Just in case, bug * on this so we can understand a bit more why it happened. */ @@ -194,6 +195,8 @@ update_socket_info_impl, (socket_table_ent_t *ent)) { #ifdef HAVE_KIST_SUPPORT int64_t tcp_space, extra_space; + tor_assert(ent); + tor_assert(ent->chan); const tor_socket_t sock = TO_CONN(BASE_CHAN_TO_TLS((channel_t *) ent->chan)->conn)->s; struct tcp_info tcp; @@ -451,6 +454,7 @@ MOCK_IMPL(int, channel_should_write_to_kernel, * kernel */ MOCK_IMPL(void, channel_write_to_kernel, (channel_t *chan)) { + tor_assert(chan); log_debug(LD_SCHED, "Writing %lu bytes to kernel for chan %" PRIu64, (unsigned long)channel_outbuf_length(chan), chan->global_identifier);

1 0

[tor/master] Assert that we aren't returning a pointer to a local variable.
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 620108ea7770608de72dcbea4ca73d6fb99c1109 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 18 14:34:49 2018 -0400 Assert that we aren't returning a pointer to a local variable. GCC got confused here with LTO enabled. Fixes part of #27772. --- src/feature/nodelist/routerparse.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/feature/nodelist/routerparse.c b/src/feature/nodelist/routerparse.c index b76b2974f..ae57fbe49 100644 --- a/src/feature/nodelist/routerparse.c +++ b/src/feature/nodelist/routerparse.c @@ -4436,7 +4436,9 @@ router_parse_addr_policy(directory_token_t *tok, unsigned fmt_flags) return NULL; } - return addr_policy_get_canonical_entry(&newe); + addr_policy_t *result = addr_policy_get_canonical_entry(&newe); + tor_assert(result != &newe); // We aren't returning the one from the stack. + return result; } /** Parse an exit policy line of the format "accept[6]/reject[6] private:...".

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/402'
by nickm＠torproject.org 15 Oct '18

15 Oct '18

commit 2c5c17510acf0cbd0eeb874a2f4c62203bf9392f Merge: 67351f672 36ba50650 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Oct 14 15:20:39 2018 -0400 Merge remote-tracking branch 'tor-github/pr/402' .travis.yml | 1 + Makefile.am | 9 +++++++++ changes/ticket28024 | 4 ++++ 3 files changed, 14 insertions(+)

1 0