diff options
Diffstat (limited to 'src')
39 files changed, 600 insertions, 227 deletions
diff --git a/src/app/config/config.c b/src/app/config/config.c index bdfa547fd7..451593a5fa 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -1197,7 +1197,11 @@ init_protocol_warning_severity_level(void) static void cleanup_protocol_warning_severity_level(void) { - atomic_counter_destroy(&protocol_warning_severity_level); + /* Destroying a locked mutex is undefined behaviour. This mutex may be + * locked, because multiple threads can access it. But we need to destroy + * it, otherwise re-initialisation will trigger undefined behaviour. + * See #31735 for details. */ + atomic_counter_destroy(&protocol_warning_severity_level); } /** Add the default directory authorities directly into the trusted dir list, @@ -1455,7 +1459,7 @@ options_act_reversible(const or_options_t *old_options, char **msg) "on this OS/with this build."); goto rollback; } -#else /* !(!defined(HAVE_SYS_UN_H)) */ +#else /* defined(HAVE_SYS_UN_H) */ if (options->ControlSocketsGroupWritable && !options->ControlSocket) { *msg = tor_strdup("Setting ControlSocketGroupWritable without setting" "a ControlSocket makes no sense."); @@ -5101,7 +5105,7 @@ find_torrc_filename(config_line_t *cmd_arg, } else { fname = dflt ? tor_strdup(dflt) : NULL; } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ fname = dflt ? tor_strdup(dflt) : NULL; #endif /* !defined(_WIN32) */ } @@ -8425,7 +8429,7 @@ init_cookie_authentication(const char *fname, const char *header, log_warn(LD_FS,"Unable to make %s group-readable.", escaped(fname)); } } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ (void) group_readable; #endif /* !defined(_WIN32) */ diff --git a/src/core/or/channeltls.c b/src/core/or/channeltls.c index 2a6edc951c..f727799387 100644 --- a/src/core/or/channeltls.c +++ b/src/core/or/channeltls.c @@ -1027,6 +1027,16 @@ channel_tls_time_process_cell(cell_t *cell, channel_tls_t *chan, int *time, } #endif /* defined(KEEP_TIMING_STATS) */ +#ifdef KEEP_TIMING_STATS +#define PROCESS_CELL(tp, cl, cn) STMT_BEGIN { \ + ++num ## tp; \ + channel_tls_time_process_cell(cl, cn, & tp ## time , \ + channel_tls_process_ ## tp ## _cell); \ + } STMT_END +#else /* !(defined(KEEP_TIMING_STATS)) */ +#define PROCESS_CELL(tp, cl, cn) channel_tls_process_ ## tp ## _cell(cl, cn) +#endif /* defined(KEEP_TIMING_STATS) */ + /** * Handle an incoming cell on a channel_tls_t. * @@ -1046,16 +1056,6 @@ channel_tls_handle_cell(cell_t *cell, or_connection_t *conn) channel_tls_t *chan; int handshaking; -#ifdef KEEP_TIMING_STATS -#define PROCESS_CELL(tp, cl, cn) STMT_BEGIN { \ - ++num ## tp; \ - channel_tls_time_process_cell(cl, cn, & tp ## time , \ - channel_tls_process_ ## tp ## _cell); \ - } STMT_END -#else /* !(defined(KEEP_TIMING_STATS)) */ -#define PROCESS_CELL(tp, cl, cn) channel_tls_process_ ## tp ## _cell(cl, cn) -#endif /* defined(KEEP_TIMING_STATS) */ - tor_assert(cell); tor_assert(conn); @@ -1073,7 +1073,8 @@ channel_tls_handle_cell(cell_t *cell, or_connection_t *conn) return; /* Reject all but VERSIONS and NETINFO when handshaking. */ - /* (VERSIONS should actually be impossible; it's variable-length.) */ + /* (VERSIONS actually indicates a protocol warning: it's variable-length, + * so if it reaches this function, we're on a v1 connection.) */ if (handshaking && cell->command != CELL_VERSIONS && cell->command != CELL_NETINFO) { log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, @@ -1106,7 +1107,15 @@ channel_tls_handle_cell(cell_t *cell, or_connection_t *conn) /* do nothing */ break; case CELL_VERSIONS: - tor_fragile_assert(); + /* A VERSIONS cell should always be a variable-length cell, and + * so should never reach this function (which handles constant-sized + * cells). But if the connection is using the (obsolete) v1 link + * protocol, all cells will be treated as constant-sized, and so + * it's possible we'll reach this code. + */ + log_fn(LOG_PROTOCOL_WARN, LD_CHANNEL, + "Received unexpected VERSIONS cell on a channel using link " + "protocol %d; ignoring.", conn->link_proto); break; case CELL_NETINFO: ++stats_n_netinfo_cells_processed; @@ -1320,6 +1329,8 @@ channel_tls_handle_var_cell(var_cell_t *var_cell, or_connection_t *conn) } } +#undef PROCESS_CELL + /** * Update channel marks after connection_or.c has changed an address. * diff --git a/src/feature/dirauth/process_descs.c b/src/feature/dirauth/process_descs.c index e1a02179b0..1c026372b0 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)); diff --git a/src/feature/dirparse/microdesc_parse.c b/src/feature/dirparse/microdesc_parse.c index e02dfcf11a..4bb4db7821 100644 --- a/src/feature/dirparse/microdesc_parse.c +++ b/src/feature/dirparse/microdesc_parse.c @@ -98,6 +98,184 @@ policy_is_reject_star_or_null(struct short_policy_t *policy) return !policy || short_policy_is_reject_star(policy); } +/** + * Return a human-readable description of a given saved_location_t. + * Never returns NULL. + **/ +static const char * +saved_location_to_string(saved_location_t where) +{ + const char *location; + switch (where) { + case SAVED_NOWHERE: + location = "download or generated string"; + break; + case SAVED_IN_CACHE: + location = "cache"; + break; + case SAVED_IN_JOURNAL: + location = "journal"; + break; + default: + location = "unknown location"; + break; + } + return location; +} + +/** + * Given a microdescriptor stored in <b>where</b> which starts at <b>s</b>, + * which ends at <b>start_of_next_microdescriptor</b>, and which is located + * within a larger document beginning at <b>start</b>: Fill in the body, + * bodylen, bodylen, saved_location, off, and digest fields of <b>md</b> as + * appropriate. + * + * The body field will be an alias within <b>s</b> if <b>saved_location</b> + * is SAVED_IN_CACHE, and will be copied into body and nul-terminated + * otherwise. + **/ +static int +microdesc_extract_body(microdesc_t *md, + const char *start, + const char *s, const char *start_of_next_microdesc, + saved_location_t where) +{ + const bool copy_body = (where != SAVED_IN_CACHE); + + const char *cp = tor_memstr(s, start_of_next_microdesc-s, "onion-key"); + + const bool 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; + + crypto_digest256(md->digest, md->body, md->bodylen, DIGEST_SHA256); + + return no_onion_key ? -1 : 0; +} + +/** + * Parse a microdescriptor which begins at <b>s</b> and ends at + * <b>start_of_next_microdesc. Store its fields into <b>md</b>. Use + * <b>where</b> for generating log information. If <b>allow_annotations</b> + * is true, then one or more annotations may precede the microdescriptor body + * proper. Use <b>area</b> for memory management, clearing it when done. + * + * 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_new(); + int rv = -1; + 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 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 err; + } + } + + 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 err; + } + 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 err; + } + 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 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 err; + } + md->ed25519_identity_pkey = tor_memdup(&k, sizeof(k)); + } + } 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); + } + } + + if ((tok = find_opt_by_keyword(tokens, K_FAMILY))) { + md->family = nodefamily_parse(tok->args[0], + NULL, + NF_WARN_MALFORMED); + } + + 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; + err: + + 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 * <b>s</b> and ending at <b>eos</b>. If allow_annotations is set, read any * annotations we recognize and ignore ones we don't. @@ -115,16 +293,11 @@ microdescs_parse_from_string(const char *s, const char *eos, saved_location_t where, smartlist_t *invalid_digests_out) { - smartlist_t *tokens; smartlist_t *result; microdesc_t *md = NULL; memarea_t *area; const char *start = s; const char *start_of_next_microdesc; - int flags = allow_annotations ? TS_ANNOTATIONS_OK : 0; - const int copy_body = (where != SAVED_IN_CACHE); - - directory_token_t *tok; if (!eos) eos = s + strlen(s); @@ -132,156 +305,47 @@ microdescs_parse_from_string(const char *s, const char *eos, s = eat_whitespace_eos(s, eos); area = memarea_new(); result = smartlist_new(); - tokens = smartlist_new(); while (s < eos) { - int okay = 0; + bool okay = false; 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]; { - 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 bool body_not_found = + microdesc_extract_body(md, start, s, + start_of_next_microdesc, + where) < 0; - 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); - if (no_onion_key) { + 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)) { - const char *location; - switch (where) { - case SAVED_NOWHERE: - location = "download or generated string"; - break; - case SAVED_IN_CACHE: - location = "cache"; - break; - case SAVED_IN_JOURNAL: - location = "journal"; - break; - default: - location = "unknown location"; - break; - } - log_warn(LD_DIR, "Unparseable microdescriptor found in %s", location); - 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."); - 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)); - } - - 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_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); - } - } - - if ((tok = find_opt_by_keyword(tokens, K_FAMILY))) { - md->family = nodefamily_parse(tok->args[0], - NULL, - NF_WARN_MALFORMED); - } - - 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; + if (microdesc_parse_fields(md, area, s, start_of_next_microdesc, + allow_annotations, where) == 0) { + smartlist_add(result, md); + md = NULL; // prevent free + okay = true; } - smartlist_add(result, md); - okay = 1; - - md = NULL; next: if (! okay && invalid_digests_out) { smartlist_add(invalid_digests_out, - tor_memdup(md->digest, DIGEST256_LEN)); + tor_memdup(md_digest, DIGEST256_LEN)); } 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; } diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index a8796c0029..924ab3115e 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -1477,10 +1477,8 @@ decrypt_descriptor_cookie(const hs_descriptor_t *desc, */ MOCK_IMPL(STATIC size_t, decrypt_desc_layer,(const hs_descriptor_t *desc, - const uint8_t *encrypted_blob, - size_t encrypted_blob_size, const uint8_t *descriptor_cookie, - int is_superencrypted_layer, + bool is_superencrypted_layer, char **decrypted_out)) { uint8_t *decrypted = NULL; @@ -1490,6 +1488,12 @@ decrypt_desc_layer,(const hs_descriptor_t *desc, uint8_t mac_key[DIGEST256_LEN], our_mac[DIGEST256_LEN]; const uint8_t *salt, *encrypted, *desc_mac; size_t encrypted_len, result_len = 0; + const uint8_t *encrypted_blob = (is_superencrypted_layer) + ? desc->plaintext_data.superencrypted_blob + : desc->superencrypted_data.encrypted_blob; + size_t encrypted_blob_size = (is_superencrypted_layer) + ? desc->plaintext_data.superencrypted_blob_size + : desc->superencrypted_data.encrypted_blob_size; tor_assert(decrypted_out); tor_assert(desc); @@ -1603,9 +1607,8 @@ desc_decrypt_superencrypted(const hs_descriptor_t *desc, char **decrypted_out) tor_assert(decrypted_out); superencrypted_len = decrypt_desc_layer(desc, - desc->plaintext_data.superencrypted_blob, - desc->plaintext_data.superencrypted_blob_size, - NULL, 1, &superencrypted_plaintext); + NULL, + true, &superencrypted_plaintext); if (!superencrypted_len) { log_warn(LD_REND, "Decrypting superencrypted desc failed."); @@ -1654,9 +1657,9 @@ desc_decrypt_encrypted(const hs_descriptor_t *desc, } encrypted_len = decrypt_desc_layer(desc, - desc->superencrypted_data.encrypted_blob, - desc->superencrypted_data.encrypted_blob_size, - descriptor_cookie, 0, &encrypted_plaintext); + descriptor_cookie, + false, &encrypted_plaintext); + if (!encrypted_len) { goto err; } diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h index dbe0cb1c94..0a843f4f3c 100644 --- a/src/feature/hs/hs_descriptor.h +++ b/src/feature/hs/hs_descriptor.h @@ -276,6 +276,7 @@ void hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client); hs_desc_authorized_client_free_, (client)) hs_desc_authorized_client_t *hs_desc_build_fake_authorized_client(void); + void hs_desc_build_authorized_client(const uint8_t *subcredential, const curve25519_public_key_t * client_auth_pk, @@ -308,10 +309,8 @@ STATIC int desc_sig_is_valid(const char *b64_sig, const char *encoded_desc, size_t encoded_len); MOCK_DECL(STATIC size_t, decrypt_desc_layer,(const hs_descriptor_t *desc, - const uint8_t *encrypted_blob, - size_t encrypted_blob_size, const uint8_t *descriptor_cookie, - int is_superencrypted_layer, + bool is_superencrypted_layer, char **decrypted_out)); #endif /* defined(HS_DESCRIPTOR_PRIVATE) */ diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 51ced6289d..ab0762e17e 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -284,19 +284,17 @@ construct_ntor_key_map(void) { di_digest256_map_t *m = NULL; - if (!fast_mem_is_zero((const char*) - curve25519_onion_key.pubkey.public_key, - CURVE25519_PUBKEY_LEN)) { - dimap_add_entry(&m, - curve25519_onion_key.pubkey.public_key, + const uint8_t *cur_pk = curve25519_onion_key.pubkey.public_key; + const uint8_t *last_pk = last_curve25519_onion_key.pubkey.public_key; + + if (!fast_mem_is_zero((const char *)cur_pk, CURVE25519_PUBKEY_LEN)) { + dimap_add_entry(&m, cur_pk, tor_memdup(&curve25519_onion_key, sizeof(curve25519_keypair_t))); } - if (!fast_mem_is_zero((const char*) - last_curve25519_onion_key.pubkey.public_key, - CURVE25519_PUBKEY_LEN)) { - dimap_add_entry(&m, - last_curve25519_onion_key.pubkey.public_key, + if (!fast_mem_is_zero((const char*)last_pk, CURVE25519_PUBKEY_LEN) && + tor_memneq(cur_pk, last_pk, CURVE25519_PUBKEY_LEN)) { + dimap_add_entry(&m, last_pk, tor_memdup(&last_curve25519_onion_key, sizeof(curve25519_keypair_t))); } @@ -3465,6 +3463,10 @@ router_free_all(void) crypto_pk_free(server_identitykey); crypto_pk_free(client_identitykey); + /* Destroying a locked mutex is undefined behaviour. This mutex may be + * locked, because multiple threads can access it. But we need to destroy + * it, otherwise re-initialisation will trigger undefined behaviour. + * See #31735 for details. */ tor_mutex_free(key_lock); routerinfo_free(desc_routerinfo); extrainfo_free(desc_extrainfo); diff --git a/src/lib/crypt_ops/compat_openssl.h b/src/lib/crypt_ops/compat_openssl.h index 9c10386c34..61ca51315f 100644 --- a/src/lib/crypt_ops/compat_openssl.h +++ b/src/lib/crypt_ops/compat_openssl.h @@ -45,7 +45,7 @@ ((st) == SSL3_ST_SW_SRVR_HELLO_B)) #define OSSL_HANDSHAKE_STATE int #define CONST_IF_OPENSSL_1_1_API -#else /* !(!defined(OPENSSL_1_1_API)) */ +#else /* defined(OPENSSL_1_1_API) */ #define STATE_IS_SW_SERVER_HELLO(st) \ ((st) == TLS_ST_SW_SRVR_HELLO) #define CONST_IF_OPENSSL_1_1_API const diff --git a/src/lib/crypt_ops/crypto_openssl_mgt.c b/src/lib/crypt_ops/crypto_openssl_mgt.c index 9ec59e7c81..6d8364ebf8 100644 --- a/src/lib/crypt_ops/crypto_openssl_mgt.c +++ b/src/lib/crypt_ops/crypto_openssl_mgt.c @@ -176,6 +176,10 @@ crypto_openssl_free_all(void) tor_free(crypto_openssl_version_str); tor_free(crypto_openssl_header_version_str); + /* Destroying a locked mutex is undefined behaviour. This mutex may be + * locked, because multiple threads can access it. But we need to destroy + * it, otherwise re-initialisation will trigger undefined behaviour. + * See #31735 for details. */ #ifndef NEW_THREAD_API if (n_openssl_mutexes_) { int n = n_openssl_mutexes_; diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c index c2011285c0..8bc7e6965c 100644 --- a/src/lib/err/backtrace.c +++ b/src/lib/err/backtrace.c @@ -57,7 +57,8 @@ #include "lib/err/torerr.h" #if defined(HAVE_EXECINFO_H) && defined(HAVE_BACKTRACE) && \ - defined(HAVE_BACKTRACE_SYMBOLS_FD) && defined(HAVE_SIGACTION) + defined(HAVE_BACKTRACE_SYMBOLS_FD) && defined(HAVE_SIGACTION) && \ + defined(HAVE_PTHREAD_H) #define USE_BACKTRACE #endif @@ -190,13 +191,15 @@ dump_stack_symbols_to_error_fds(void) backtrace_symbols_fd(cb_buf, (int)depth, fds[i]); } +/* The signals that we want our backtrace handler to trap */ +static int trap_signals[] = { SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS, + SIGIO, -1 }; + /** Install signal handlers as needed so that when we crash, we produce a * useful stack trace. Return 0 on success, -errno on failure. */ static int install_bt_handler(void) { - int trap_signals[] = { SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS, - SIGIO, -1 }; int i, rv=0; struct sigaction sa; @@ -232,6 +235,23 @@ install_bt_handler(void) static void remove_bt_handler(void) { + int i; + + struct sigaction sa; + + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = SIG_DFL; + sigfillset(&sa.sa_mask); + + for (i = 0; trap_signals[i] >= 0; ++i) { + /* remove_bt_handler() is called on shutdown, from low-level code. + * It's not a fatal error, so we just ignore it. */ + (void)sigaction(trap_signals[i], &sa, NULL); + } + + /* cb_buf_mutex is statically initialised, so we can not destroy it. + * If we destroy it, and then re-initialise tor, all our backtraces will + * fail. */ } #endif /* defined(USE_BACKTRACE) */ diff --git a/src/lib/fs/conffile.c b/src/lib/fs/conffile.c index 7bb2f23931..0d5d56b335 100644 --- a/src/lib/fs/conffile.c +++ b/src/lib/fs/conffile.c @@ -153,16 +153,18 @@ config_process_include(const char *path, int recursion_level, int extended, int rv = -1; SMARTLIST_FOREACH_BEGIN(config_files, const char *, config_file) { config_line_t *included_config = NULL; + config_line_t *included_config_last = NULL; if (config_get_included_config(config_file, recursion_level, extended, - &included_config, list_last, + &included_config, &included_config_last, opened_lst) < 0) { goto done; } *next = included_config; - if (*list_last) - next = &(*list_last)->next; - + if (included_config_last) { + next = &included_config_last->next; + *list_last = included_config_last; + } } SMARTLIST_FOREACH_END(config_file); *list = ret_list; rv = 0; diff --git a/src/lib/fs/dir.c b/src/lib/fs/dir.c index 3c31e00d99..291f1bbf04 100644 --- a/src/lib/fs/dir.c +++ b/src/lib/fs/dir.c @@ -262,7 +262,7 @@ check_private_dir,(const char *dirname, cpd_check_t check, } } close(fd); -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ /* Win32 case: we can't open() a directory. */ (void)effective_user; diff --git a/src/lib/lock/compat_mutex.c b/src/lib/lock/compat_mutex.c index 4ad5929715..670bd0174c 100644 --- a/src/lib/lock/compat_mutex.c +++ b/src/lib/lock/compat_mutex.c @@ -29,7 +29,15 @@ tor_mutex_new_nonrecursive(void) tor_mutex_init_nonrecursive(m); return m; } -/** Release all storage and system resources held by <b>m</b>. */ +/** Release all storage and system resources held by <b>m</b>. + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void tor_mutex_free_(tor_mutex_t *m) { diff --git a/src/lib/lock/compat_mutex_pthreads.c b/src/lib/lock/compat_mutex_pthreads.c index ee5f520cd0..f82ad9f0e8 100644 --- a/src/lib/lock/compat_mutex_pthreads.c +++ b/src/lib/lock/compat_mutex_pthreads.c @@ -88,12 +88,26 @@ tor_mutex_release(tor_mutex_t *m) } /** Clean up the mutex <b>m</b> so that it no longer uses any system * resources. Does not free <b>m</b>. This function must only be called on - * mutexes from tor_mutex_init(). */ + * mutexes from tor_mutex_init(). + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void tor_mutex_uninit(tor_mutex_t *m) { int err; raw_assert(m); + /* If the mutex is already locked, wait until after it is unlocked to destroy + * it. Locking and releasing the mutex makes undefined behaviour less likely, + * but does not prevent it. Another thread can lock the mutex between release + * and destroy. */ + tor_mutex_acquire(m); + tor_mutex_release(m); err = pthread_mutex_destroy(&m->mutex); if (PREDICT_UNLIKELY(err)) { // LCOV_EXCL_START diff --git a/src/lib/log/log.c b/src/lib/log/log.c index be6f459554..d2002f6eae 100644 --- a/src/lib/log/log.c +++ b/src/lib/log/log.c @@ -55,10 +55,6 @@ #include <android/log.h> #endif // HAVE_ANDROID_LOG_H. -/** Given a severity, yields an index into log_severity_list_t.masks to use - * for that severity. */ -#define SEVERITY_MASK_IDX(sev) ((sev) - LOG_ERR) - /** @{ */ /** The string we stick at the end of a log message when it is too long, * and its length. */ @@ -687,8 +683,9 @@ tor_log_update_sigsafe_err_fds(void) n_fds = 1; for (lf = logfiles; lf; lf = lf->next) { - /* Don't try callback to the control port, or syslogs: We can't - * do them from a signal handler. Don't try stdout: we always do stderr. + /* Don't try callback to the control port, syslogs, android logs, or any + * other non-file descriptor log: We can't call arbitrary functions from a + * signal handler. */ if (lf->is_temporary || logfile_is_external(lf) || lf->seems_dead || lf->fd < 0) @@ -720,7 +717,10 @@ tor_log_update_sigsafe_err_fds(void) if (!found_real_stderr && int_array_contains(log_fds, n_fds, STDOUT_FILENO)) { - /* Don't use a virtual stderr when we're also logging to stdout. */ + /* Don't use a virtual stderr when we're also logging to stdout. + * If we reached max_fds logs, we'll now have (max_fds - 1) logs. + * That's ok, max_fds is large enough that most tor instances don't exceed + * it. */ raw_assert(n_fds >= 2); /* Don't tor_assert inside log fns */ --n_fds; log_fds[0] = log_fds[n_fds]; @@ -832,7 +832,10 @@ logs_free_all(void) } /* We _could_ destroy the log mutex here, but that would screw up any logs - * that happened between here and the end of execution. */ + * that happened between here and the end of execution. + * If tor is re-initialized, log_mutex_initialized will still be 1. So we + * won't trigger any undefined behaviour by trying to re-initialize the + * log mutex. */ } /** Close signal-safe log files. diff --git a/src/lib/log/log.h b/src/lib/log/log.h index 4291418eb6..da4bcbe608 100644 --- a/src/lib/log/log.h +++ b/src/lib/log/log.h @@ -297,4 +297,10 @@ MOCK_DECL(STATIC void, logv, (int severity, log_domain_mask_t domain, va_list ap) CHECK_PRINTF(5,0)); #endif +#if defined(LOG_PRIVATE) || defined(TOR_UNIT_TESTS) +/** Given a severity, yields an index into log_severity_list_t.masks to use + * for that severity. */ +#define SEVERITY_MASK_IDX(sev) ((sev) - LOG_ERR) +#endif + #endif /* !defined(TOR_TORLOG_H) */ diff --git a/src/lib/log/util_bug.h b/src/lib/log/util_bug.h index 546ae1e3ef..d7f01618e8 100644 --- a/src/lib/log/util_bug.h +++ b/src/lib/log/util_bug.h @@ -96,7 +96,7 @@ (void)(a); \ (void)(fmt); \ STMT_END -#else /* !(defined(TOR_UNIT_TESTS) && ... */ +#else /* !(defined(TOR_UNIT_TESTS) && defined(DISABLE_ASSERTS_IN_UNIT_TES... */ /** Like assert(3), but send assertion failures to the log as well as to * stderr. */ #define tor_assert(expr) tor_assertf(expr, NULL) diff --git a/src/lib/math/fp.c b/src/lib/math/fp.c index 616e4f15c0..49a2a6a2ca 100644 --- a/src/lib/math/fp.c +++ b/src/lib/math/fp.c @@ -75,7 +75,7 @@ clamp_double_to_int64(double number) */ #define PROBLEMATIC_FLOAT_CONVERSION_WARNING DISABLE_GCC_WARNING(float-conversion) -#endif /* defined(MINGW_ANY) && GCC_VERSION >= 409 */ +#endif /* (defined(MINGW_ANY)||defined(__FreeBSD__)) && GCC_VERSION >= 409 */ /* With clang 4.0 we apparently run into "double promotion" warnings here, diff --git a/src/lib/memarea/memarea.c b/src/lib/memarea/memarea.c index 84c73b0b95..0a88210906 100644 --- a/src/lib/memarea/memarea.c +++ b/src/lib/memarea/memarea.c @@ -315,7 +315,7 @@ memarea_assert_ok(memarea_t *area) } } -#else /* !(!defined(DISABLE_MEMORY_SENTINELS)) */ +#else /* defined(DISABLE_MEMORY_SENTINELS) */ struct memarea_t { smartlist_t *pieces; diff --git a/src/lib/process/daemon.c b/src/lib/process/daemon.c index 3b90bef671..ae34b5bcb8 100644 --- a/src/lib/process/daemon.c +++ b/src/lib/process/daemon.c @@ -165,7 +165,7 @@ finish_daemon(const char *desired_cwd) return 0; } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ /* defined(_WIN32) */ int start_daemon(void) diff --git a/src/lib/process/process.c b/src/lib/process/process.c index 631c7169f1..2194a603ff 100644 --- a/src/lib/process/process.c +++ b/src/lib/process/process.c @@ -513,7 +513,7 @@ process_get_unix_process(const process_t *process) tor_assert(process->unix_process); return process->unix_process; } -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ /** Get the internal handle for Windows backend. */ process_win32_t * process_get_win32_process(const process_t *process) diff --git a/src/lib/process/restrict.c b/src/lib/process/restrict.c index 534b39d101..93d06de9a2 100644 --- a/src/lib/process/restrict.c +++ b/src/lib/process/restrict.c @@ -214,7 +214,7 @@ set_max_file_descriptors(rlim_t limit, int *max_out) return -1; } limit = MAX_CONNECTIONS; -#else /* !(!defined(HAVE_GETRLIMIT)) */ +#else /* defined(HAVE_GETRLIMIT) */ struct rlimit rlim; if (getrlimit(RLIMIT_NOFILE, &rlim) != 0) { diff --git a/src/lib/process/setuid.c b/src/lib/process/setuid.c index 6e8258f279..e132787943 100644 --- a/src/lib/process/setuid.c +++ b/src/lib/process/setuid.c @@ -376,7 +376,7 @@ switch_id(const char *user, const unsigned flags) #endif /* defined(__linux__) && defined(HAVE_SYS_PRCTL_H) && ... */ return 0; -#else /* !(!defined(_WIN32)) */ +#else /* defined(_WIN32) */ (void)user; (void)flags; diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index b652397f5a..faaf463f29 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -294,6 +294,7 @@ sb_rt_sigaction(scmp_filter_ctx ctx, sandbox_cfg_t *filter) unsigned i; int rc; int param[] = { SIGINT, SIGTERM, SIGPIPE, SIGUSR1, SIGUSR2, SIGHUP, SIGCHLD, + SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS, SIGIO, #ifdef SIGXFSZ SIGXFSZ #endif diff --git a/src/lib/thread/compat_threads.c b/src/lib/thread/compat_threads.c index 1c4a5c4e3f..5c8ffa55c6 100644 --- a/src/lib/thread/compat_threads.c +++ b/src/lib/thread/compat_threads.c @@ -67,7 +67,15 @@ atomic_counter_init(atomic_counter_t *counter) memset(counter, 0, sizeof(*counter)); tor_mutex_init_nonrecursive(&counter->mutex); } -/** Clean up all resources held by an atomic counter. */ +/** Clean up all resources held by an atomic counter. + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void atomic_counter_destroy(atomic_counter_t *counter) { diff --git a/src/lib/thread/threads.h b/src/lib/thread/threads.h index ecf60641b5..de3da6a585 100644 --- a/src/lib/thread/threads.h +++ b/src/lib/thread/threads.h @@ -131,7 +131,17 @@ atomic_counter_init(atomic_counter_t *counter) { atomic_init(&counter->val, 0); } -/** Clean up all resources held by an atomic counter. */ +/** Clean up all resources held by an atomic counter. + * + * This usage note applies to the compat_threads implementation of + * atomic_counter_destroy(): + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ static inline void atomic_counter_destroy(atomic_counter_t *counter) { diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index 86f0ac42cc..58a7b20dec 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -657,7 +657,7 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime, if (r < 0) goto error; } -#else /* !(defined(SSL_CTX_set1_groups_list) || ...) */ +#else /* !(defined(SSL_CTX_set1_groups_list) || defined(HAVE_SSL_CTX_SET1... */ if (! is_client) { int nid; EC_KEY *ec_key; @@ -673,7 +673,7 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime, SSL_CTX_set_tmp_ecdh(result->ctx, ec_key); EC_KEY_free(ec_key); } -#endif /* defined(SSL_CTX_set1_groups_list) || ...) */ +#endif /* defined(SSL_CTX_set1_groups_list) || defined(HAVE_SSL_CTX_SET1_... */ SSL_CTX_set_verify(result->ctx, SSL_VERIFY_PEER, always_accept_verify_cb); /* let us realloc bufs that we're writing from */ diff --git a/src/test/conf_examples/include_bug_31408/expected b/src/test/conf_examples/include_bug_31408/expected new file mode 100644 index 0000000000..2e822f1a78 --- /dev/null +++ b/src/test/conf_examples/include_bug_31408/expected @@ -0,0 +1,2 @@ +Nickname test31408 +ORPort 31408 diff --git a/src/test/conf_examples/include_bug_31408/included/01_nickname.inc b/src/test/conf_examples/include_bug_31408/included/01_nickname.inc new file mode 100644 index 0000000000..508dd89a35 --- /dev/null +++ b/src/test/conf_examples/include_bug_31408/included/01_nickname.inc @@ -0,0 +1 @@ +Nickname test31408 diff --git a/src/test/conf_examples/include_bug_31408/included/02_no_configs.inc b/src/test/conf_examples/include_bug_31408/included/02_no_configs.inc new file mode 100644 index 0000000000..140e927f19 --- /dev/null +++ b/src/test/conf_examples/include_bug_31408/included/02_no_configs.inc @@ -0,0 +1,3 @@ +# Bug 31048 is triggered when the last file in a config directory: +# * contains no configuration options, +# * but is non-empty: that is, it contains comments or whitespace. diff --git a/src/test/conf_examples/include_bug_31408/torrc b/src/test/conf_examples/include_bug_31408/torrc new file mode 100644 index 0000000000..a42685e93c --- /dev/null +++ b/src/test/conf_examples/include_bug_31408/torrc @@ -0,0 +1,2 @@ +%include "included" +ORPort 31408 diff --git a/src/test/fuzz/fuzz_hsdescv3.c b/src/test/fuzz/fuzz_hsdescv3.c index 2cbd655898..9d4a6dbb55 100644 --- a/src/test/fuzz/fuzz_hsdescv3.c +++ b/src/test/fuzz/fuzz_hsdescv3.c @@ -35,16 +35,21 @@ mock_rsa_ed25519_crosscert_check(const uint8_t *crosscert, static size_t mock_decrypt_desc_layer(const hs_descriptor_t *desc, - const uint8_t *encrypted_blob, - size_t encrypted_blob_size, const uint8_t *descriptor_cookie, - int is_superencrypted_layer, + bool is_superencrypted_layer, char **decrypted_out) { (void)is_superencrypted_layer; (void)desc; (void)descriptor_cookie; const size_t overhead = HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN; + const uint8_t *encrypted_blob = (is_superencrypted_layer) + ? desc->plaintext_data.superencrypted_blob + : desc->superencrypted_data.encrypted_blob; + size_t encrypted_blob_size = (is_superencrypted_layer) + ? desc->plaintext_data.superencrypted_blob_size + : desc->superencrypted_data.encrypted_blob_size; + if (encrypted_blob_size < overhead) return 0; *decrypted_out = tor_memdup_nulterm( diff --git a/src/test/fuzz/fuzzing_common.c b/src/test/fuzz/fuzzing_common.c index 862acb2b35..e269c36b42 100644 --- a/src/test/fuzz/fuzzing_common.c +++ b/src/test/fuzz/fuzzing_common.c @@ -167,7 +167,7 @@ main(int argc, char **argv) memset(&s, 0, sizeof(s)); set_log_severity_config(loglevel, LOG_ERR, &s); /* ALWAYS log bug warnings. */ - s.masks[LOG_WARN-LOG_ERR] |= LD_BUG; + s.masks[SEVERITY_MASK_IDX(LOG_WARN)] |= LD_BUG; add_stream_log(&s, "", fileno(stdout)); } diff --git a/src/test/test_address.c b/src/test/test_address.c index ef6daa06b4..32fb4aa232 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,42 +1246,95 @@ test_address_tor_node_in_same_network_family(void *ignored) helper_free_mock_node(node_b); } -#define CHECK_RI_ADDR(addr_str, rv) STMT_BEGIN \ +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. */ +#define TEST_ROUTER_VALID_ADDRESS_HELPER(ipv4_addr_str, ipv6_addr_str, rv) \ + STMT_BEGIN \ ri = tor_malloc_zero(sizeof(routerinfo_t)); \ tor_addr_t addr; \ - tor_addr_parse(&addr, (addr_str)); \ + tor_addr_parse(&addr, (ipv4_addr_str)); \ ri->addr = tor_addr_to_ipv4h(&addr); \ - tor_addr_make_null(&ri->ipv6_addr, AF_INET6); \ - tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, (rv)); \ + tor_addr_parse(&ri->ipv6_addr, (ipv6_addr_str)); \ + tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, (rv)); \ tor_free(ri); \ STMT_END -/* XXX: Here, we use a non-internal IPv4 as dirserv_router_has_valid_address() - * will check internal/null IPv4 first. */ -#define CHECK_RI_ADDR6(addr_str, rv) STMT_BEGIN \ - ri = tor_malloc_zero(sizeof(routerinfo_t)); \ - ri->addr = 16777217; /* 1.0.0.1 */ \ - tor_addr_parse(&ri->ipv6_addr, (addr_str)); \ - tt_int_op(dirserv_router_has_valid_address(ri), OP_EQ, (rv)); \ - tor_free(ri); \ - STMT_END +/* Like TEST_ROUTER_VALID_ADDRESS_HELPER(), but always passes a null + * IPv6 address. */ +#define CHECK_RI_ADDR(ipv4_addr_str, rv) \ + TEST_ROUTER_VALID_ADDRESS_HELPER(ipv4_addr_str, "::", rv) + +/* Like TEST_ROUTER_VALID_ADDRESS_HELPER(), but always passes a non-internal + * IPv4 address, so that the IPv6 check is reached. */ +#define CHECK_RI_ADDR6(ipv6_addr_str, rv) \ + 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", 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); + + /* 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, private_rv); + 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); + 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), @@ -1313,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 }; diff --git a/src/test/test_config.c b/src/test/test_config.c index 78f9ae9c3f..1c6c913078 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -5326,6 +5326,73 @@ test_config_include_folder_order(void *data) } static void +test_config_include_blank_file_last(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *torrcd = NULL; + char *path = NULL; + char *dir = tor_strdup(get_fname("test_include_blank_file_last")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&torrcd, "%s"PATH_SEPARATOR"%s", dir, "torrc.d"); + +#ifdef _WIN32 + tt_int_op(mkdir(torrcd), OP_EQ, 0); +#else + tt_int_op(mkdir(torrcd, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "aa_1st"); + tt_int_op(write_str_to_file(path, "Test 1\n", 0), OP_EQ, 0); + tor_free(path); + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "bb_2nd"); + tt_int_op(write_str_to_file(path, "Test 2\n", 0), OP_EQ, 0); + tor_free(path); + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "cc_comment"); + tt_int_op(write_str_to_file(path, "# comment only\n", 0), OP_EQ, 0); + tor_free(path); + + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), + "%%include %s\n" + "Test 3\n", + torrcd); + + int include_used; + tt_int_op(config_get_lines_include(torrc_contents, &result, 0, &include_used, + NULL), OP_EQ, 0); + tt_ptr_op(result, OP_NE, NULL); + tt_int_op(include_used, OP_EQ, 1); + + int len = 0; + config_line_t *next; + for (next = result; next != NULL; next = next->next) { + char expected[10]; + tor_snprintf(expected, sizeof(expected), "%d", len + 1); + tt_str_op(next->key, OP_EQ, "Test"); + tt_str_op(next->value, OP_EQ, expected); + len++; + } + tt_int_op(len, OP_EQ, 3); + + done: + config_free_lines(result); + tor_free(torrcd); + tor_free(path); + tor_free(dir); +} + +static void test_config_include_path_syntax(void *data) { (void)data; @@ -6045,6 +6112,7 @@ struct testcase_t config_tests[] = { CONFIG_TEST(include_recursion_before_after, 0), CONFIG_TEST(include_recursion_after_only, 0), CONFIG_TEST(include_folder_order, 0), + CONFIG_TEST(include_blank_file_last, 0), CONFIG_TEST(include_path_syntax, 0), CONFIG_TEST(include_not_processed, 0), CONFIG_TEST(include_has_include, 0), diff --git a/src/test/test_logging.c b/src/test/test_logging.c index bb7018fe1c..203ce64e32 100644 --- a/src/test/test_logging.c +++ b/src/test/test_logging.c @@ -35,7 +35,7 @@ test_get_sigsafe_err_fds(void *arg) set_log_severity_config(LOG_WARN, LOG_ERR, &include_bug); set_log_severity_config(LOG_WARN, LOG_ERR, &no_bug); - no_bug.masks[0] &= ~(LD_BUG|LD_GENERAL); + no_bug.masks[SEVERITY_MASK_IDX(LOG_ERR)] &= ~(LD_BUG|LD_GENERAL); set_log_severity_config(LOG_INFO, LOG_NOTICE, &no_bug2); /* Add some logs; make sure the output is as expected. */ diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index ad211c7ea8..804e6c546a 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -21,6 +21,7 @@ #include "feature/nodelist/routerstatus_st.h" #include "test/test.h" +#include "test/log_test_helpers.h" #ifdef HAVE_SYS_STAT_H #include <sys/stat.h> @@ -770,6 +771,80 @@ test_md_parse(void *arg) tor_free(mem_op_hex_tmp); } +static void +test_md_parse_id_ed25519(void *arg) +{ + (void)arg; + + /* A correct MD with an ed25519 ID ... and an unspecified ID type, + * which is permitted. */ + const char GOOD_MD[] = + "onion-key\n" + "-----BEGIN RSA PUBLIC KEY-----\n" + "MIGJAoGBAM7uUtq5F6h63QNYIvC+4NcWaD0DjtnrOORZMkdpJhinXUOwce3cD5Dj\n" + "sgdN1wJpWpTQMXJ2DssfSgmOVXETP7qJuZyRprxalQhaEATMDNJA/66Ml1jSO9mZ\n" + "+8Xb7m/4q778lNtkSbsvMaYD2Dq6k2QQ3kMhr9z8oUtX0XA23+pfAgMBAAE=\n" + "-----END RSA PUBLIC KEY-----\n" + "id ed25519 VGhpcyBpc24ndCBhY3R1YWxseSBhIHB1YmxpYyBrZXk\n" + "id wumpus dodecahedron\n"; + + smartlist_t *mds = NULL; + const microdesc_t *md; + + mds = microdescs_parse_from_string(GOOD_MD, + NULL, 1, SAVED_NOWHERE, NULL); + tt_assert(mds); + tt_int_op(smartlist_len(mds), OP_EQ, 1); + md = smartlist_get(mds, 0); + tt_mem_op(md->ed25519_identity_pkey, OP_EQ, + "This isn't actually a public key", ED25519_PUBKEY_LEN); + SMARTLIST_FOREACH(mds, microdesc_t *, m, microdesc_free(m)); + smartlist_free(mds); + + /* As above, but ed25519 ID key appears twice. */ + const char DUPLICATE_KEY[] = + "onion-key\n" + "-----BEGIN RSA PUBLIC KEY-----\n" + "MIGJAoGBAM7uUtq5F6h63QNYIvC+4NcWaD0DjtnrOORZMkdpJhinXUOwce3cD5Dj\n" + "sgdN1wJpWpTQMXJ2DssfSgmOVXETP7qJuZyRprxalQhaEATMDNJA/66Ml1jSO9mZ\n" + "+8Xb7m/4q778lNtkSbsvMaYD2Dq6k2QQ3kMhr9z8oUtX0XA23+pfAgMBAAE=\n" + "-----END RSA PUBLIC KEY-----\n" + "id ed25519 VGhpcyBpc24ndCBhY3R1YWxseSBhIHB1YmxpYyBrZXk\n" + "id ed25519 VGhpcyBpc24ndCBhY3R1YWxseSBhIHB1YmxpYyBrZXk\n"; + + setup_capture_of_logs(LOG_WARN); + mds = microdescs_parse_from_string(DUPLICATE_KEY, + NULL, 1, SAVED_NOWHERE, NULL); + tt_assert(mds); + tt_int_op(smartlist_len(mds), OP_EQ, 0); // no entries. + expect_single_log_msg_containing("Extra ed25519 key"); + mock_clean_saved_logs(); + smartlist_free(mds); + + /* As above, but ed25519 ID key is invalid. */ + const char BOGUS_KEY[] = + "onion-key\n" + "-----BEGIN RSA PUBLIC KEY-----\n" + "MIGJAoGBAM7uUtq5F6h63QNYIvC+4NcWaD0DjtnrOORZMkdpJhinXUOwce3cD5Dj\n" + "sgdN1wJpWpTQMXJ2DssfSgmOVXETP7qJuZyRprxalQhaEATMDNJA/66Ml1jSO9mZ\n" + "+8Xb7m/4q778lNtkSbsvMaYD2Dq6k2QQ3kMhr9z8oUtX0XA23+pfAgMBAAE=\n" + "-----END RSA PUBLIC KEY-----\n" + "id ed25519 VGhpcyBpc24ndCBhY3R1YWxseSBhIHB1YmxpYyZZZZZZZZZZZ\n"; + + mds = microdescs_parse_from_string(BOGUS_KEY, + NULL, 1, SAVED_NOWHERE, NULL); + tt_assert(mds); + tt_int_op(smartlist_len(mds), OP_EQ, 0); // no entries. + expect_single_log_msg_containing("Bogus ed25519 key"); + + done: + if (mds) { + SMARTLIST_FOREACH(mds, microdesc_t *, m, microdesc_free(m)); + smartlist_free(mds); + } + teardown_capture_of_logs(); +} + static int mock_rgsbd_called = 0; static routerstatus_t *mock_rgsbd_val_a = NULL; static routerstatus_t *mock_rgsbd_val_b = NULL; @@ -903,6 +978,7 @@ struct testcase_t microdesc_tests[] = { { "broken_cache", test_md_cache_broken, TT_FORK, NULL, NULL }, { "generate", test_md_generate, 0, NULL, NULL }, { "parse", test_md_parse, 0, NULL, NULL }, + { "parse_id_ed25519", test_md_parse_id_ed25519, 0, NULL, NULL }, { "reject_cache", test_md_reject_cache, TT_FORK, NULL, NULL }, { "corrupt_desc", test_md_corrupt_desc, TT_FORK, NULL, NULL }, END_OF_TESTCASES diff --git a/src/test/test_options.c b/src/test/test_options.c index 0747a2e062..b3654ede7d 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -54,9 +54,9 @@ setup_log_callback(void) { log_severity_list_t lst; memset(&lst, 0, sizeof(lst)); - lst.masks[LOG_ERR - LOG_ERR] = ~0; - lst.masks[LOG_WARN - LOG_ERR] = ~0; - lst.masks[LOG_NOTICE - LOG_ERR] = ~0; + lst.masks[SEVERITY_MASK_IDX(LOG_ERR)] = ~0; + lst.masks[SEVERITY_MASK_IDX(LOG_WARN)] = ~0; + lst.masks[SEVERITY_MASK_IDX(LOG_NOTICE)] = ~0; add_callback_log(&lst, log_cback); mark_logs_temp(); } diff --git a/src/test/testing_common.c b/src/test/testing_common.c index ad22898ce5..9e7d83dcdc 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -295,7 +295,7 @@ main(int c, const char **v) memset(&s, 0, sizeof(s)); set_log_severity_config(loglevel, LOG_ERR, &s); /* ALWAYS log bug warnings. */ - s.masks[LOG_WARN-LOG_ERR] |= LD_BUG; + s.masks[SEVERITY_MASK_IDX(LOG_WARN)] |= LD_BUG; add_stream_log(&s, "", fileno(stdout)); } { @@ -303,7 +303,7 @@ main(int c, const char **v) log_severity_list_t s; memset(&s, 0, sizeof(s)); set_log_severity_config(LOG_ERR, LOG_ERR, &s); - s.masks[LOG_WARN-LOG_ERR] |= LD_BUG; + s.masks[SEVERITY_MASK_IDX(LOG_WARN)] |= LD_BUG; add_callback_log(&s, log_callback_failure); } flush_log_messages_from_startup(); |