September 2019 - tor-commits - lists.torproject.org

[tor/master] test: dirserv_router_has_valid_address() with zero-family addresses
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 46fea1dfeeae1b575d427a9c3c6b4a8d1ab7f2b9 Author: teor <teor(a)torproject.org> Date: Thu Sep 19 15:24:01 2019 +1000 test: dirserv_router_has_valid_address() with zero-family addresses Sometimes tor doesn't initialise an address, so its family is zero. Failing test for 31793. Future commits will fix the code. --- src/test/test_address.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/test/test_address.c b/src/test/test_address.c index ef6daa06b..c186cda73 100644 --- a/src/test/test_address.c +++ b/src/test/test_address.c @@ -1271,10 +1271,44 @@ test_address_dirserv_router_addr_private(void *ignored) (void)ignored; /* A stub routerinfo structure, with only its address fields set. */ routerinfo_t *ri = NULL; + CHECK_RI_ADDR("1.0.0.1", 0); CHECK_RI_ADDR("10.0.0.1", -1); + CHECK_RI_ADDR6("2600::1", 0); CHECK_RI_ADDR6("fe80::1", -1); + + /* Null addresses */ + /* IPv4 null fails, regardless of IPv6 */ + CHECK_RI_ADDR("0.0.0.0", -1); + + { + ri = tor_malloc_zero(sizeof(routerinfo_t)); + ri->addr = 0; /* 0.0.0.0 */ + tor_addr_parse(&ri->ipv6_addr, "::"); + tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, -1); + tor_free(ri); + } + + /* IPv6 null succeeds, because IPv4 is not null */ + CHECK_RI_ADDR6("::", 0); + + /* Byte-zeroed null addresses */ + /* IPv4 null fails, regardless of IPv6 */ + { + ri = tor_malloc_zero(sizeof(routerinfo_t)); + tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, -1); + tor_free(ri); + } + + /* IPv6 null succeeds, because IPv4 is not internal */ + { + ri = tor_malloc_zero(sizeof(routerinfo_t)); + ri->addr = 16777217; /* 1.0.0.1 */ + tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, 0); + tor_free(ri); + } + done: tor_free(ri); }

1 0

[tor/master] test: test dirserv_router_has_valid_address() with DirAllowPrivateAddresses
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 69a1f9c8a7a9c8221029ce83f19d0e8d3e93f917 Author: teor <teor(a)torproject.org> Date: Thu Sep 19 16:17:17 2019 +1000 test: test dirserv_router_has_valid_address() with DirAllowPrivateAddresses Part of 31793. --- src/test/test_address.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/src/test/test_address.c b/src/test/test_address.c index ab95113dc..32fb4aa23 100644 --- a/src/test/test_address.c +++ b/src/test/test_address.c @@ -24,6 +24,7 @@ #endif /* defined(HAVE_IFCONF_TO_SMARTLIST) */ #include "core/or/or.h" +#include "app/config/config.h" #include "feature/dirauth/process_descs.h" #include "feature/nodelist/routerinfo_st.h" #include "feature/nodelist/node_st.h" @@ -1245,6 +1246,14 @@ test_address_tor_node_in_same_network_family(void *ignored) helper_free_mock_node(node_b); } +static or_options_t mock_options; + +static const or_options_t * +mock_get_options(void) +{ + return &mock_options; +} + /* Test dirserv_router_has_valid_address() on a stub routerinfo, with only its * address fields set. Use IPv4 ipv4_addr_str and IPv6 ipv6_addr_str. * Fail if it does not return rv. */ @@ -1270,22 +1279,32 @@ test_address_tor_node_in_same_network_family(void *ignored) TEST_ROUTER_VALID_ADDRESS_HELPER("1.0.0.1", ipv6_addr_str, rv) static void -test_address_dirserv_router_addr_private(void *ignored) +test_address_dirserv_router_addr_private(void *opt_dir_allow_private) { - (void)ignored; /* A stub routerinfo structure, with only its address fields set. */ routerinfo_t *ri = NULL; + /* The expected return value for private addresses. + * Modified if DirAllowPrivateAddresses is 1. */ + int private_rv = -1; + + memset(&mock_options, 0, sizeof(or_options_t)); + MOCK(get_options, mock_get_options); + + if (opt_dir_allow_private) { + mock_options.DirAllowPrivateAddresses = 1; + private_rv = 0; + } CHECK_RI_ADDR("1.0.0.1", 0); - CHECK_RI_ADDR("10.0.0.1", -1); + CHECK_RI_ADDR("10.0.0.1", private_rv); CHECK_RI_ADDR6("2600::1", 0); - CHECK_RI_ADDR6("fe80::1", -1); + CHECK_RI_ADDR6("fe80::1", private_rv); /* Null addresses */ /* IPv4 null fails, regardless of IPv6 */ - CHECK_RI_ADDR("0.0.0.0", -1); - TEST_ROUTER_VALID_ADDRESS_HELPER("0.0.0.0", "::", -1); + CHECK_RI_ADDR("0.0.0.0", private_rv); + TEST_ROUTER_VALID_ADDRESS_HELPER("0.0.0.0", "::", private_rv); /* IPv6 null succeeds, because IPv4 is not null */ CHECK_RI_ADDR6("::", 0); @@ -1294,7 +1313,7 @@ test_address_dirserv_router_addr_private(void *ignored) /* IPv4 null fails, regardless of IPv6 */ { ri = tor_malloc_zero(sizeof(routerinfo_t)); - tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, -1); + tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, private_rv); tor_free(ri); } @@ -1308,10 +1327,14 @@ test_address_dirserv_router_addr_private(void *ignored) done: tor_free(ri); + UNMOCK(get_options); } #define ADDRESS_TEST(name, flags) \ { #name, test_address_ ## name, flags, NULL, NULL } +#define ADDRESS_TEST_STR_ARG(name, flags, str_arg) \ + { #name "/" str_arg, test_address_ ## name, flags, &passthrough_setup, \ + (void *)(str_arg) } struct testcase_t address_tests[] = { ADDRESS_TEST(udp_socket_trick_whitebox, TT_FORK), @@ -1344,5 +1367,6 @@ struct testcase_t address_tests[] = { ADDRESS_TEST(tor_addr_in_same_network_family, 0), ADDRESS_TEST(tor_node_in_same_network_family, 0), ADDRESS_TEST(dirserv_router_addr_private, 0), + ADDRESS_TEST_STR_ARG(dirserv_router_addr_private, 0, "allow_private"), END_OF_TESTCASES };

1 0

[tor/master] Merge branch 'tor-github/pr/1345'
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 219962964812f75cff3ab8cede254e71ea48dd7b Merge: e77ad795c 69a1f9c8a Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Sep 25 14:14:30 2019 +0300 Merge branch 'tor-github/pr/1345' src/feature/dirauth/process_descs.c | 10 +++-- src/test/test_address.c | 89 ++++++++++++++++++++++++++++++------- 2 files changed, 78 insertions(+), 21 deletions(-)

1 0

[tor/master] dirauth: reorder the checks in dirserv_router_has_valid_address()
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 03c2b1be8d1e749b4f468cfe522da9e8375a83ad Author: teor <teor(a)torproject.org> Date: Thu Sep 19 15:27:39 2019 +1000 dirauth: reorder the checks in dirserv_router_has_valid_address() To avoid a bug warning. Fixes 31793. Bug not in any release, no changes file required. --- src/feature/dirauth/process_descs.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/feature/dirauth/process_descs.c b/src/feature/dirauth/process_descs.c index e1a02179b..1c026372b 100644 --- a/src/feature/dirauth/process_descs.c +++ b/src/feature/dirauth/process_descs.c @@ -432,20 +432,22 @@ STATIC int dirserv_router_has_valid_address(routerinfo_t *ri) { tor_addr_t addr; + if (get_options()->DirAllowPrivateAddresses) return 0; /* whatever it is, we're fine with it */ - tor_addr_from_ipv4h(&addr, ri->addr); - if (tor_addr_is_internal(&addr, 0) || tor_addr_is_null(&addr)) { + tor_addr_from_ipv4h(&addr, ri->addr); + if (tor_addr_is_null(&addr) || tor_addr_is_internal(&addr, 0)) { log_info(LD_DIRSERV, "Router %s published internal IPv4 address. Refusing.", router_describe(ri)); return -1; /* it's a private IP, we should reject it */ } + /* We only check internal v6 on non-null addresses because we do not require * IPv6 and null IPv6 is normal. */ - if (tor_addr_is_internal(&ri->ipv6_addr, 0) && - !tor_addr_is_null(&ri->ipv6_addr)) { + if (!tor_addr_is_null(&ri->ipv6_addr) && + tor_addr_is_internal(&ri->ipv6_addr, 0)) { log_info(LD_DIRSERV, "Router %s published internal IPv6 address. Refusing.", router_describe(ri));

1 0

[tor/master] Add a coccinelle script to look for {inc, dec}rements in log_debug
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 387cfccee47394adeba8cbf49c130cc9b332b025 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Sep 11 18:53:16 2019 -0400 Add a coccinelle script to look for {inc,dec}rements in log_debug We want to forbid this pattern since, unlike the other log_*() macros, log_debug() conditionally evaluates its arguments only if debug-level logging is enabled. Thus, a call to log_debug("%d", x++); will only increment x if debugging logs are enabled, which is probably not what the programmer intended. One bug caused by this pattern was #30628. This script detects log_debug( ) calls with any of E++, E--, ++E, or --E in their arguments, where E is an arbitrary expression. Closes ticket 30743. --- changes/ticket30743 | 7 +++++++ scripts/coccinelle/debugmm.cocci | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/changes/ticket30743 b/changes/ticket30743 new file mode 100644 index 000000000..4f029717d --- /dev/null +++ b/changes/ticket30743 @@ -0,0 +1,7 @@ + o Minor features (maintenance scripts): + - Add a coccinelle script to detect bugs caused by incrementing or + decrementing a variable inside a call to log_debug(). Since + log_debug() is a macro whose arguments are conditionally evaluated, it + is usually an error to do this. One such bug was 30628, in which SENDME + cells were miscounted by a decrement operator inside a log_debug() + call. Closes ticket 30743. diff --git a/scripts/coccinelle/debugmm.cocci b/scripts/coccinelle/debugmm.cocci new file mode 100644 index 000000000..dbd308df3 --- /dev/null +++ b/scripts/coccinelle/debugmm.cocci @@ -0,0 +1,29 @@ +// Look for use of expressions with side-effects inside of debug logs. +// +// This script detects expressions like ++E, --E, E++, and E-- inside of +// calls to log_debug(). +// +// The log_debug() macro exits early if debug logging is not enabled, +// potentially causing problems if its arguments have side-effects. + +@@ +expression E; +@@ +*log_debug(... , <+... --E ...+>, ... ); + + +@@ +expression E; +@@ +*log_debug(... , <+... ++E ...+>, ... ); + +@@ +expression E; +@@ +*log_debug(... , <+... E-- ...+>, ... ); + + +@@ +expression E; +@@ +*log_debug(... , <+... E++ ...+>, ... );

1 0

[tor/master] Merge branch 'tor-github/pr/1322'
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit e77ad795c68f28e8431dc5b098fba05fcec49c19 Merge: 99f75373d 387cfccee Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Sep 25 14:13:44 2019 +0300 Merge branch 'tor-github/pr/1322' changes/ticket30743 | 7 +++++++ scripts/coccinelle/debugmm.cocci | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)

1 0

[tor/master] Reindent microdesc_extract_body().
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit b1552e8814559fc8bf773765975fa1fded83c391 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 9 15:22:16 2019 -0400 Reindent microdesc_extract_body(). This commit is whitespace-only; I suggest reviewing with -b. --- src/feature/dirparse/microdesc_parse.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/feature/dirparse/microdesc_parse.c b/src/feature/dirparse/microdesc_parse.c index 621e3c10d..29bebbed3 100644 --- a/src/feature/dirparse/microdesc_parse.c +++ b/src/feature/dirparse/microdesc_parse.c @@ -142,24 +142,24 @@ microdesc_extract_body(microdesc_t *md, { const int copy_body = (where != SAVED_IN_CACHE); - const char *cp = tor_memstr(s, start_of_next_microdesc-s, - "onion-key"); - const int no_onion_key = (cp == NULL); - if (no_onion_key) { - cp = s; /* So that we have *some* junk to put in the body */ - } + const char *cp = tor_memstr(s, start_of_next_microdesc-s, "onion-key"); + + const int no_onion_key = (cp == NULL); + if (no_onion_key) { + cp = s; /* So that we have *some* junk to put in the body */ + } - md->bodylen = start_of_next_microdesc - cp; - md->saved_location = where; - if (copy_body) - md->body = tor_memdup_nulterm(cp, md->bodylen); - else - md->body = (char*)cp; - md->off = cp - start; + md->bodylen = start_of_next_microdesc - cp; + md->saved_location = where; + if (copy_body) + md->body = tor_memdup_nulterm(cp, md->bodylen); + else + md->body = (char*)cp; + md->off = cp - start; - crypto_digest256(md->digest, md->body, md->bodylen, DIGEST_SHA256); + crypto_digest256(md->digest, md->body, md->bodylen, DIGEST_SHA256); - return no_onion_key ? -1 : 0; + return no_onion_key ? -1 : 0; } /** Parse as many microdescriptors as are found from the string starting at

1 0

[tor/master] Reindent microdesc_parse_fields().
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 2094796c24e1f4e5868a4d4e2e29fd45d4e0a799 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 9 15:44:56 2019 -0400 Reindent microdesc_parse_fields(). I suggest reviewing this with -b: it should be whitespace-only. --- src/feature/dirparse/microdesc_parse.c | 150 ++++++++++++++++----------------- 1 file changed, 75 insertions(+), 75 deletions(-) diff --git a/src/feature/dirparse/microdesc_parse.c b/src/feature/dirparse/microdesc_parse.c index 5af4c60cb..e421cae54 100644 --- a/src/feature/dirparse/microdesc_parse.c +++ b/src/feature/dirparse/microdesc_parse.c @@ -183,97 +183,97 @@ microdesc_parse_fields(microdesc_t *md, int flags = allow_annotations ? TS_ANNOTATIONS_OK : 0; directory_token_t *tok; - if (tokenize_string(area, s, start_of_next_microdesc, tokens, - microdesc_token_table, flags)) { - log_warn(LD_DIR, "Unparseable microdescriptor found in %s", - saved_location_to_string(where)); - goto next; - } + if (tokenize_string(area, s, start_of_next_microdesc, tokens, + microdesc_token_table, flags)) { + log_warn(LD_DIR, "Unparseable microdescriptor found in %s", + saved_location_to_string(where)); + goto next; + } - if ((tok = find_opt_by_keyword(tokens, A_LAST_LISTED))) { - if (parse_iso_time(tok->args[0], &md->last_listed)) { - log_warn(LD_DIR, "Bad last-listed time in microdescriptor"); - goto next; - } + if ((tok = find_opt_by_keyword(tokens, A_LAST_LISTED))) { + if (parse_iso_time(tok->args[0], &md->last_listed)) { + log_warn(LD_DIR, "Bad last-listed time in microdescriptor"); + goto next; } + } - tok = find_by_keyword(tokens, K_ONION_KEY); - if (!crypto_pk_public_exponent_ok(tok->key)) { - log_warn(LD_DIR, - "Relay's onion key had invalid exponent."); + tok = find_by_keyword(tokens, K_ONION_KEY); + if (!crypto_pk_public_exponent_ok(tok->key)) { + log_warn(LD_DIR, + "Relay's onion key had invalid exponent."); + goto next; + } + md->onion_pkey = tor_memdup(tok->object_body, tok->object_size); + md->onion_pkey_len = tok->object_size; + crypto_pk_free(tok->key); + + if ((tok = find_opt_by_keyword(tokens, K_ONION_KEY_NTOR))) { + curve25519_public_key_t k; + tor_assert(tok->n_args >= 1); + if (curve25519_public_from_base64(&k, tok->args[0]) < 0) { + log_warn(LD_DIR, "Bogus ntor-onion-key in microdesc"); goto next; } - md->onion_pkey = tor_memdup(tok->object_body, tok->object_size); - md->onion_pkey_len = tok->object_size; - crypto_pk_free(tok->key); - - if ((tok = find_opt_by_keyword(tokens, K_ONION_KEY_NTOR))) { - curve25519_public_key_t k; - tor_assert(tok->n_args >= 1); - if (curve25519_public_from_base64(&k, tok->args[0]) < 0) { - log_warn(LD_DIR, "Bogus ntor-onion-key in microdesc"); - goto next; - } - md->onion_curve25519_pkey = - tor_memdup(&k, sizeof(curve25519_public_key_t)); - } + md->onion_curve25519_pkey = + tor_memdup(&k, sizeof(curve25519_public_key_t)); + } - smartlist_t *id_lines = find_all_by_keyword(tokens, K_ID); - if (id_lines) { - SMARTLIST_FOREACH_BEGIN(id_lines, directory_token_t *, t) { - tor_assert(t->n_args >= 2); - if (!strcmp(t->args[0], "ed25519")) { - if (md->ed25519_identity_pkey) { - log_warn(LD_DIR, "Extra ed25519 key in microdesc"); - smartlist_free(id_lines); - goto next; - } - ed25519_public_key_t k; - if (ed25519_public_from_base64(&k, t->args[1])<0) { - log_warn(LD_DIR, "Bogus ed25519 key in microdesc"); - smartlist_free(id_lines); - goto next; - } - md->ed25519_identity_pkey = tor_memdup(&k, sizeof(k)); + smartlist_t *id_lines = find_all_by_keyword(tokens, K_ID); + if (id_lines) { + SMARTLIST_FOREACH_BEGIN(id_lines, directory_token_t *, t) { + tor_assert(t->n_args >= 2); + if (!strcmp(t->args[0], "ed25519")) { + if (md->ed25519_identity_pkey) { + log_warn(LD_DIR, "Extra ed25519 key in microdesc"); + smartlist_free(id_lines); + goto next; } - } SMARTLIST_FOREACH_END(t); - smartlist_free(id_lines); - } - - { - smartlist_t *a_lines = find_all_by_keyword(tokens, K_A); - if (a_lines) { - find_single_ipv6_orport(a_lines, &md->ipv6_addr, &md->ipv6_orport); - smartlist_free(a_lines); + ed25519_public_key_t k; + if (ed25519_public_from_base64(&k, t->args[1])<0) { + log_warn(LD_DIR, "Bogus ed25519 key in microdesc"); + smartlist_free(id_lines); + goto next; + } + md->ed25519_identity_pkey = tor_memdup(&k, sizeof(k)); } - } + } SMARTLIST_FOREACH_END(t); + smartlist_free(id_lines); + } - if ((tok = find_opt_by_keyword(tokens, K_FAMILY))) { - md->family = nodefamily_parse(tok->args[0], - NULL, - NF_WARN_MALFORMED); + { + smartlist_t *a_lines = find_all_by_keyword(tokens, K_A); + if (a_lines) { + find_single_ipv6_orport(a_lines, &md->ipv6_addr, &md->ipv6_orport); + smartlist_free(a_lines); } + } - if ((tok = find_opt_by_keyword(tokens, K_P))) { - md->exit_policy = parse_short_policy(tok->args[0]); - } - if ((tok = find_opt_by_keyword(tokens, K_P6))) { - md->ipv6_exit_policy = parse_short_policy(tok->args[0]); - } + if ((tok = find_opt_by_keyword(tokens, K_FAMILY))) { + md->family = nodefamily_parse(tok->args[0], + NULL, + NF_WARN_MALFORMED); + } - if (policy_is_reject_star_or_null(md->exit_policy) && - policy_is_reject_star_or_null(md->ipv6_exit_policy)) { - md->policy_is_reject_star = 1; - } + if ((tok = find_opt_by_keyword(tokens, K_P))) { + md->exit_policy = parse_short_policy(tok->args[0]); + } + if ((tok = find_opt_by_keyword(tokens, K_P6))) { + md->ipv6_exit_policy = parse_short_policy(tok->args[0]); + } + + if (policy_is_reject_star_or_null(md->exit_policy) && + policy_is_reject_star_or_null(md->ipv6_exit_policy)) { + md->policy_is_reject_star = 1; + } - rv = 0; + rv = 0; next: // todo: rename this label. - SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); - memarea_clear(area); - smartlist_free(tokens); + SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); + memarea_clear(area); + smartlist_free(tokens); - return rv; + return rv; } /** Parse as many microdescriptors as are found from the string starting at

1 0

[tor/master] microdesc_parse: Rename "next" label to "err".
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit 194884977cf00804ddac19e6211cd47aec9099ca Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 9 15:47:01 2019 -0400 microdesc_parse: Rename "next" label to "err". --- src/feature/dirparse/microdesc_parse.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/feature/dirparse/microdesc_parse.c b/src/feature/dirparse/microdesc_parse.c index e421cae54..96a345426 100644 --- a/src/feature/dirparse/microdesc_parse.c +++ b/src/feature/dirparse/microdesc_parse.c @@ -187,13 +187,13 @@ microdesc_parse_fields(microdesc_t *md, microdesc_token_table, flags)) { log_warn(LD_DIR, "Unparseable microdescriptor found in %s", saved_location_to_string(where)); - goto next; + goto err; } if ((tok = find_opt_by_keyword(tokens, A_LAST_LISTED))) { if (parse_iso_time(tok->args[0], &md->last_listed)) { log_warn(LD_DIR, "Bad last-listed time in microdescriptor"); - goto next; + goto err; } } @@ -201,7 +201,7 @@ microdesc_parse_fields(microdesc_t *md, if (!crypto_pk_public_exponent_ok(tok->key)) { log_warn(LD_DIR, "Relay's onion key had invalid exponent."); - goto next; + goto err; } md->onion_pkey = tor_memdup(tok->object_body, tok->object_size); md->onion_pkey_len = tok->object_size; @@ -212,7 +212,7 @@ microdesc_parse_fields(microdesc_t *md, tor_assert(tok->n_args >= 1); if (curve25519_public_from_base64(&k, tok->args[0]) < 0) { log_warn(LD_DIR, "Bogus ntor-onion-key in microdesc"); - goto next; + goto err; } md->onion_curve25519_pkey = tor_memdup(&k, sizeof(curve25519_public_key_t)); @@ -226,13 +226,13 @@ microdesc_parse_fields(microdesc_t *md, if (md->ed25519_identity_pkey) { log_warn(LD_DIR, "Extra ed25519 key in microdesc"); smartlist_free(id_lines); - goto next; + goto err; } ed25519_public_key_t k; if (ed25519_public_from_base64(&k, t->args[1])<0) { log_warn(LD_DIR, "Bogus ed25519 key in microdesc"); smartlist_free(id_lines); - goto next; + goto err; } md->ed25519_identity_pkey = tor_memdup(&k, sizeof(k)); } @@ -267,7 +267,7 @@ microdesc_parse_fields(microdesc_t *md, } rv = 0; - next: // todo: rename this label. + err: SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); memarea_clear(area);

1 0

[tor/master] Extract field-parsing code for microdesc_parse.c
by asn＠torproject.org 25 Sep '19

25 Sep '19

commit ec368409fa7732a08e85c1bfb254469f02458266 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 9 15:41:24 2019 -0400 Extract field-parsing code for microdesc_parse.c The code here parses the fields from the microdescriptor, including possible annotations, and stores them into a microdesc_t object. This commit is almost pure code movement; I recommend using --color-moved to review it. --- src/feature/dirparse/microdesc_parse.c | 143 +++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 60 deletions(-) diff --git a/src/feature/dirparse/microdesc_parse.c b/src/feature/dirparse/microdesc_parse.c index 29bebbed3..5af4c60cb 100644 --- a/src/feature/dirparse/microdesc_parse.c +++ b/src/feature/dirparse/microdesc_parse.c @@ -162,62 +162,27 @@ microdesc_extract_body(microdesc_t *md, return no_onion_key ? -1 : 0; } -/** Parse as many microdescriptors as are found from the string starting at - * s and ending at eos. If allow_annotations is set, read any - * annotations we recognize and ignore ones we don't. - * - * If saved_location isn't SAVED_IN_CACHE, make a local copy of each - * descriptor in the body field of each microdesc_t. +/** + * Parse a microdescriptor which begins at s and ends at + * start_of_next_microdesc. Store its fields into md. Use + * where for generating log information. If allow_annotations + * is true, then one or more annotations may precede the microdescriptor body + * proper. Use area for memory management, clearing it when done. * - * Return all newly parsed microdescriptors in a newly allocated - * smartlist_t. If invalid_disgests_out is provided, add a SHA256 - * microdesc digest to it for every microdesc that we found to be badly - * formed. (This may cause duplicates) */ -smartlist_t * -microdescs_parse_from_string(const char *s, const char *eos, - int allow_annotations, - saved_location_t where, - smartlist_t *invalid_digests_out) + * On success, return 0; otherwise return -1. + **/ +static int +microdesc_parse_fields(microdesc_t *md, + memarea_t *area, + const char *s, const char *start_of_next_microdesc, + int allow_annotations, + saved_location_t where) { - smartlist_t *tokens; - smartlist_t *result; - microdesc_t *md = NULL; - memarea_t *area; - const char *start = s; - const char *start_of_next_microdesc; + smartlist_t *tokens = smartlist_new(); + int rv = -1; int flags = allow_annotations ? TS_ANNOTATIONS_OK : 0; - directory_token_t *tok; - if (!eos) - eos = s + strlen(s); - - s = eat_whitespace_eos(s, eos); - area = memarea_new(); - result = smartlist_new(); - tokens = smartlist_new(); - - while (s < eos) { - int okay = 0; - - start_of_next_microdesc = find_start_of_next_microdesc(s, eos); - if (!start_of_next_microdesc) - start_of_next_microdesc = eos; - - md = tor_malloc_zero(sizeof(microdesc_t)); - uint8_t md_digest[DIGEST256_LEN]; - { - int body_not_found = microdesc_extract_body(md, start, s, - start_of_next_microdesc, - where) < 0; - - memcpy(md_digest, md->digest, DIGEST256_LEN); - if (body_not_found) { - log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Malformed or truncated descriptor"); - goto next; - } - } - if (tokenize_string(area, s, start_of_next_microdesc, tokens, microdesc_token_table, flags)) { log_warn(LD_DIR, "Unparseable microdescriptor found in %s", @@ -301,10 +266,74 @@ microdescs_parse_from_string(const char *s, const char *eos, md->policy_is_reject_star = 1; } - smartlist_add(result, md); - okay = 1; + rv = 0; + next: // todo: rename this label. + + SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); + memarea_clear(area); + smartlist_free(tokens); + + return rv; +} + +/** Parse as many microdescriptors as are found from the string starting at + * s and ending at eos. If allow_annotations is set, read any + * annotations we recognize and ignore ones we don't. + * + * If saved_location isn't SAVED_IN_CACHE, make a local copy of each + * descriptor in the body field of each microdesc_t. + * + * Return all newly parsed microdescriptors in a newly allocated + * smartlist_t. If invalid_disgests_out is provided, add a SHA256 + * microdesc digest to it for every microdesc that we found to be badly + * formed. (This may cause duplicates) */ +smartlist_t * +microdescs_parse_from_string(const char *s, const char *eos, + int allow_annotations, + saved_location_t where, + smartlist_t *invalid_digests_out) +{ + smartlist_t *result; + microdesc_t *md = NULL; + memarea_t *area; + const char *start = s; + const char *start_of_next_microdesc; + + if (!eos) + eos = s + strlen(s); + + s = eat_whitespace_eos(s, eos); + area = memarea_new(); + result = smartlist_new(); + + while (s < eos) { + int okay = 0; + + start_of_next_microdesc = find_start_of_next_microdesc(s, eos); + if (!start_of_next_microdesc) + start_of_next_microdesc = eos; + + md = tor_malloc_zero(sizeof(microdesc_t)); + uint8_t md_digest[DIGEST256_LEN]; + { + int body_not_found = microdesc_extract_body(md, start, s, + start_of_next_microdesc, + where) < 0; + + memcpy(md_digest, md->digest, DIGEST256_LEN); + if (body_not_found) { + log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Malformed or truncated descriptor"); + goto next; + } + } + + if (microdesc_parse_fields(md, area, s, start_of_next_microdesc, + allow_annotations, where) == 0) { + smartlist_add(result, md); + md = NULL; // prevent free + okay = 1; + } - md = NULL; next: if (! okay && invalid_digests_out) { smartlist_add(invalid_digests_out, @@ -312,16 +341,10 @@ microdescs_parse_from_string(const char *s, const char *eos, } microdesc_free(md); md = NULL; - - SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); - memarea_clear(area); - smartlist_clear(tokens); s = start_of_next_microdesc; } - SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); memarea_drop_all(area); - smartlist_free(tokens); return result; }

1 0