diff options
-rw-r--r-- | changes/bug22446 | 4 | ||||
-rw-r--r-- | changes/ticket22281 | 3 | ||||
-rw-r--r-- | src/or/config.c | 32 | ||||
-rw-r--r-- | src/or/hs_circuitmap.c | 24 | ||||
-rw-r--r-- | src/or/hs_client.c | 17 |
5 files changed, 69 insertions, 11 deletions
diff --git a/changes/bug22446 b/changes/bug22446 new file mode 100644 index 0000000000..eab65aac00 --- /dev/null +++ b/changes/bug22446 @@ -0,0 +1,4 @@ + o Minor features (code style, backport from 0.3.1.3-alpha): + - Add "Falls through" comments to our codebase, in order to silence + GCC 7's -Wimplicit-fallthrough warnings. Patch from Andreas + Stieger. Closes ticket 22446. diff --git a/changes/ticket22281 b/changes/ticket22281 new file mode 100644 index 0000000000..95787580ff --- /dev/null +++ b/changes/ticket22281 @@ -0,0 +1,3 @@ + o Minor features (bug detection): + - Log a warning message, with stack trace, for any attempt to call + get_options() during option validation. Closes ticket 22281. diff --git a/src/or/config.c b/src/or/config.c index c0072fe4ef..aebca907f2 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -766,6 +766,9 @@ static int have_parsed_cmdline = 0; static char *global_dirfrontpagecontents = NULL; /** List of port_cfg_t for all configured ports. */ static smartlist_t *configured_ports = NULL; +/** True iff we're currently validating options, and any calls to + * get_options() are likely to be bugs. */ +static int in_option_validation = 0; /** Return the contents of our frontpage string, or NULL if not configured. */ MOCK_IMPL(const char*, @@ -779,6 +782,7 @@ MOCK_IMPL(or_options_t *, get_options_mutable, (void)) { tor_assert(global_options); + tor_assert_nonfatal(! in_option_validation); return global_options; } @@ -2308,24 +2312,35 @@ options_trial_assign(config_line_t *list, unsigned flags, char **msg) return r; } + setopt_err_t rv; + + in_option_validation = 1; + if (options_validate(get_options_mutable(), trial_options, global_default_options, 1, msg) < 0) { or_options_free(trial_options); - return SETOPT_ERR_PARSE; /*XXX make this a separate return value. */ + rv = SETOPT_ERR_PARSE; /*XXX make this a separate return value. */ + goto done; } if (options_transition_allowed(get_options(), trial_options, msg) < 0) { or_options_free(trial_options); - return SETOPT_ERR_TRANSITION; + rv = SETOPT_ERR_TRANSITION; + goto done; } + in_option_validation = 0; if (set_options(trial_options, msg)<0) { or_options_free(trial_options); - return SETOPT_ERR_SETTING; + rv = SETOPT_ERR_SETTING; + goto done; } /* we liked it. put it in place. */ - return SETOPT_OK; + rv = SETOPT_OK; + done: + in_option_validation = 0; + return rv; } /** Print a usage message for tor. */ @@ -2828,8 +2843,11 @@ static int options_validate_cb(void *old_options, void *options, void *default_options, int from_setconf, char **msg) { - return options_validate(old_options, options, default_options, + in_option_validation = 1; + int rv = options_validate(old_options, options, default_options, from_setconf, msg); + in_option_validation = 0; + return rv; } #define REJECT(arg) \ @@ -5216,6 +5234,7 @@ options_init_from_string(const char *cf_defaults, const char *cf, } newoptions->IncludeUsed = cf_has_include; + in_option_validation = 1; /* Validate newoptions */ if (options_validate(oldoptions, newoptions, newdefaultoptions, @@ -5228,17 +5247,20 @@ options_init_from_string(const char *cf_defaults, const char *cf, err = SETOPT_ERR_TRANSITION; goto err; } + in_option_validation = 0; if (set_options(newoptions, msg)) { err = SETOPT_ERR_SETTING; goto err; /* frees and replaces old options */ } + or_options_free(global_default_options); global_default_options = newdefaultoptions; return SETOPT_OK; err: + in_option_validation = 0; or_options_free(newoptions); or_options_free(newdefaultoptions); if (*msg) { diff --git a/src/or/hs_circuitmap.c b/src/or/hs_circuitmap.c index 09704d796c..63d5c1ba2e 100644 --- a/src/or/hs_circuitmap.c +++ b/src/or/hs_circuitmap.c @@ -407,9 +407,20 @@ hs_circuitmap_get_rend_circ_service_side(const uint8_t *cookie) } /* Public function: Return client-side rendezvous circuit with rendezvous - * <b>cookie</b>. It will first lookup for the CIRCUIT_PURPOSE_C_REND_READY - * purpose and then try for CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED and then - * finally tries for CIRCUIT_PURPOSE_C_ESTABLISH_REND. + * <b>cookie</b>. It will look for circuits with the following purposes: + + * a) CIRCUIT_PURPOSE_C_REND_READY: Established rend circuit (received + * RENDEZVOUS_ESTABLISHED). Waiting for RENDEZVOUS2 from service, and for + * INTRODUCE_ACK from intro point. + * + * b) CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: Established rend circuit and + * introduce circuit acked. Waiting for RENDEZVOUS2 from service. + * + * c) CIRCUIT_PURPOSE_C_REND_JOINED: Established rend circuit and received + * RENDEZVOUS2 from service. + * + * d) CIRCUIT_PURPOSE_C_ESTABLISH_REND: Rend circuit open but not yet + * established. * * Return NULL if no such circuit is found in the circuitmap. */ origin_circuit_t * @@ -433,6 +444,13 @@ hs_circuitmap_get_rend_circ_client_side(const uint8_t *cookie) circ = hs_circuitmap_get_origin_circuit(HS_TOKEN_REND_CLIENT_SIDE, REND_TOKEN_LEN, cookie, + CIRCUIT_PURPOSE_C_REND_JOINED); + if (circ) { + return circ; + } + + circ = hs_circuitmap_get_origin_circuit(HS_TOKEN_REND_CLIENT_SIDE, + REND_TOKEN_LEN, cookie, CIRCUIT_PURPOSE_C_ESTABLISH_REND); return circ; } diff --git a/src/or/hs_client.c b/src/or/hs_client.c index f85ebc8473..cde0231d9d 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -746,6 +746,14 @@ handle_introduce_ack_success(origin_circuit_t *intro_circ) } assert_circ_anonymity_ok(rend_circ, get_options()); + + /* It is possible to get a RENDEZVOUS2 cell before the INTRODUCE_ACK which + * means that the circuit will be joined and already transmitting data. In + * that case, simply skip the purpose change and close the intro circuit + * like it should be. */ + if (TO_CIRCUIT(rend_circ)->purpose == CIRCUIT_PURPOSE_C_REND_JOINED) { + goto end; + } circuit_change_purpose(TO_CIRCUIT(rend_circ), CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED); /* Set timestamp_dirty, because circuit_expire_building expects it to @@ -943,7 +951,10 @@ hs_client_decode_descriptor(const char *desc_str, ret = hs_desc_decode_descriptor(desc_str, subcredential, desc); memwipe(subcredential, 0, sizeof(subcredential)); if (ret < 0) { - log_warn(LD_GENERAL, "Could not parse received descriptor as client"); + log_warn(LD_GENERAL, "Could not parse received descriptor as client."); + if (get_options()->SafeLogging_ == SAFELOG_SCRUB_NONE) { + log_warn(LD_GENERAL, "%s", escaped(desc_str)); + } goto err; } @@ -1006,8 +1017,8 @@ hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk) cached_desc = hs_cache_lookup_as_client(identity_pk); if (cached_desc && hs_client_any_intro_points_usable(identity_pk, cached_desc)) { - log_warn(LD_GENERAL, "We would fetch a v3 hidden service descriptor " - "but we already have a useable descriprot."); + log_info(LD_GENERAL, "We would fetch a v3 hidden service descriptor " + "but we already have a usable descriptor."); return 0; } } |