diff options
Diffstat (limited to 'src')
31 files changed, 962 insertions, 801 deletions
diff --git a/src/core/or/circuitpadding.c b/src/core/or/circuitpadding.c index ba6bfe1f53..2895d49e18 100644 --- a/src/core/or/circuitpadding.c +++ b/src/core/or/circuitpadding.c @@ -224,25 +224,20 @@ circpad_machine_current_state(const circpad_machine_state_t *mi) } /** - * Calculate the lower bound of a histogram bin. The upper bound - * is obtained by calling this function with bin+1, and subtracting 1. + * Get the lower bound of a histogram bin. * - * The 0th bin has a special value -- it only represents start_usec. - * This is so we can specify a probability on 0-delay values. + * You can obtain the upper bound using histogram_get_bin_upper_bound(). * - * After bin 0, bins are exponentially spaced, so that each subsequent - * bin is twice as large as the previous. This is done so that higher - * time resolution is given to lower time values. - * - * The infinity bin is a the last bin in the array (histogram_len-1). - * It has a usec value of CIRCPAD_DELAY_INFINITE (UINT32_MAX). + * This function can also be called with 'bin' set to a value equal or greater + * than histogram_len in which case the infinity bin is chosen and + * CIRCPAD_DELAY_INFINITE is returned. */ STATIC circpad_delay_t circpad_histogram_bin_to_usec(const circpad_machine_state_t *mi, circpad_hist_index_t bin) { const circpad_state_t *state = circpad_machine_current_state(mi); - circpad_delay_t start_usec; + circpad_delay_t rtt_add_usec = 0; /* Our state should have been checked to be non-null by the caller * (circpad_machine_remove_token()) */ @@ -250,27 +245,29 @@ circpad_histogram_bin_to_usec(const circpad_machine_state_t *mi, return CIRCPAD_DELAY_INFINITE; } - if (state->use_rtt_estimate) - start_usec = mi->rtt_estimate_usec+state->start_usec; - else - start_usec = state->start_usec; - - if (bin >= CIRCPAD_INFINITY_BIN(state)) + /* The infinity bin has an upper bound of infinity, so make sure we return + * that if they ask for it. */ + if (bin > CIRCPAD_INFINITY_BIN(mi)) { return CIRCPAD_DELAY_INFINITE; + } - if (bin == 0) - return start_usec; + /* If we are using an RTT estimate, consider it as well. */ + if (state->use_rtt_estimate) { + rtt_add_usec = mi->rtt_estimate_usec; + } - if (bin == 1) - return start_usec+1; + return state->histogram_edges[bin] + rtt_add_usec; +} - /* The bin widths double every index, so that we can have more resolution - * for lower time values in the histogram. */ - const circpad_time_t bin_width_exponent = - 1 << (CIRCPAD_INFINITY_BIN(state) - bin); - return (circpad_delay_t)MIN(start_usec + - state->range_usec/bin_width_exponent, - CIRCPAD_DELAY_INFINITE); +/** + * Like circpad_histogram_bin_to_usec() but return the upper bound of bin. + * (The upper bound is included in the bin.) + */ +STATIC circpad_delay_t +histogram_get_bin_upper_bound(const circpad_machine_state_t *mi, + circpad_hist_index_t bin) +{ + return circpad_histogram_bin_to_usec(mi, bin+1) - 1; } /** Return the midpoint of the histogram bin <b>bin_index</b>. */ @@ -279,8 +276,7 @@ circpad_get_histogram_bin_midpoint(const circpad_machine_state_t *mi, int bin_index) { circpad_delay_t left_bound = circpad_histogram_bin_to_usec(mi, bin_index); - circpad_delay_t right_bound = - circpad_histogram_bin_to_usec(mi, bin_index+1)-1; + circpad_delay_t right_bound = histogram_get_bin_upper_bound(mi, bin_index); return left_bound + (right_bound - left_bound)/2; } @@ -289,19 +285,17 @@ circpad_get_histogram_bin_midpoint(const circpad_machine_state_t *mi, * Return the bin that contains the usec argument. * "Contains" is defined as us in [lower, upper). * - * This function will never return the infinity bin (histogram_len-1), - * in order to simplify the rest of the code. - * - * This means that technically the last bin (histogram_len-2) - * has range [start_usec+range_usec, CIRCPAD_DELAY_INFINITE]. + * This function will never return the infinity bin (histogram_len-1), in order + * to simplify the rest of the code, so if a usec is provided that falls above + * the highest non-infinity bin, that bin index will be returned. */ STATIC circpad_hist_index_t circpad_histogram_usec_to_bin(const circpad_machine_state_t *mi, circpad_delay_t usec) { const circpad_state_t *state = circpad_machine_current_state(mi); - circpad_delay_t start_usec; - int32_t bin; /* Larger than return type to properly clamp overflow */ + circpad_delay_t rtt_add_usec = 0; + circpad_hist_index_t bin; /* Our state should have been checked to be non-null by the caller * (circpad_machine_remove_token()) */ @@ -309,34 +303,25 @@ circpad_histogram_usec_to_bin(const circpad_machine_state_t *mi, return 0; } - if (state->use_rtt_estimate) - start_usec = mi->rtt_estimate_usec+state->start_usec; - else - start_usec = state->start_usec; - - /* The first bin (#0) has zero width and starts (and ends) at start_usec. */ - if (usec <= start_usec) - return 0; - - if (usec == start_usec+1) - return 1; + /* If we are using an RTT estimate, consider it as well. */ + if (state->use_rtt_estimate) { + rtt_add_usec = mi->rtt_estimate_usec; + } - const circpad_time_t histogram_range_usec = state->range_usec; - /* We need to find the bin corresponding to our position in the range. - * Since bins are exponentially spaced in powers of two, we need to - * take the log2 of our position in histogram_range_usec. However, - * since tor_log2() returns the floor(log2(u64)), we have to adjust - * it to behave like ceil(log2(u64)). This is verified in our tests - * to properly invert the operation done in - * circpad_histogram_bin_to_usec(). */ - bin = CIRCPAD_INFINITY_BIN(state) - - tor_log2(2*histogram_range_usec/(usec-start_usec+1)); + /* Walk through the bins and check the upper bound of each bin, if 'usec' is + * less-or-equal to that, return that bin. If rtt_estimate is enabled then + * add that to the upper bound of each bin. + * + * We don't want to return the infinity bin here, so don't go there. */ + for (bin = 0 ; bin < CIRCPAD_INFINITY_BIN(state) ; bin++) { + if (usec <= histogram_get_bin_upper_bound(mi, bin) + rtt_add_usec) { + return bin; + } + } - /* Clamp the return value to account for timevals before the start - * of bin 0, or after the last bin. Don't return the infinity bin - * index. */ - bin = MIN(MAX(bin, 1), CIRCPAD_INFINITY_BIN(state)-1); - return bin; + /* We don't want to return the infinity bin here, so if we still didn't find + * the right bin, return the highest non-infinity bin */ + return CIRCPAD_INFINITY_BIN(state)-1; } /** @@ -398,20 +383,22 @@ circpad_choose_state_length(circpad_machine_state_t *mi) /** * Sample a value from our iat_dist, and clamp it safely * to circpad_delay_t. + * + * Before returning, add <b>delay_shift</b> (can be zero) to the sampled value. */ static circpad_delay_t circpad_distribution_sample_iat_delay(const circpad_state_t *state, - circpad_delay_t start_usec) + circpad_delay_t delay_shift) { double val = circpad_distribution_sample(state->iat_dist); /* These comparisons are safe, because the output is in the range * [0, 2**32), and double has a precision of 53 bits. */ val = MAX(0, val); - val = MIN(val, state->range_usec); + val = MIN(val, state->dist_max_sample_usec); - /* This addition is exact: val is at most 2**32-1, start_usec + /* This addition is exact: val is at most 2**32-1, min_delay * is at most 2**32-1, and doubles have a precision of 53 bits. */ - val += start_usec; + val += delay_shift; /* Clamp the distribution at infinite delay val */ return (circpad_delay_t)MIN(tor_llround(val), CIRCPAD_DELAY_INFINITE); @@ -431,7 +418,6 @@ circpad_machine_sample_delay(circpad_machine_state_t *mi) const circpad_hist_token_t *histogram = NULL; circpad_hist_index_t curr_bin = 0; circpad_delay_t bin_start, bin_end; - circpad_delay_t start_usec; /* These three must all be larger than circpad_hist_token_t, because * we sum several circpad_hist_token_t values across the histogram */ uint64_t curr_weight = 0; @@ -440,14 +426,12 @@ circpad_machine_sample_delay(circpad_machine_state_t *mi) tor_assert(state); - if (state->use_rtt_estimate) - start_usec = mi->rtt_estimate_usec+state->start_usec; - else - start_usec = state->start_usec; - if (state->iat_dist.type != CIRCPAD_DIST_NONE) { /* Sample from a fixed IAT distribution and return */ - return circpad_distribution_sample_iat_delay(state, start_usec); + circpad_delay_t iat_delay_shift = state->use_rtt_estimate ? + mi->rtt_estimate_usec + state->dist_added_shift_usec : + state->dist_added_shift_usec; + return circpad_distribution_sample_iat_delay(state, iat_delay_shift); } else if (state->token_removal != CIRCPAD_TOKEN_REMOVAL_NONE) { /* We have a mutable histogram. Do basic sanity check and apply: */ if (BUG(!mi->histogram) || @@ -512,16 +496,13 @@ circpad_machine_sample_delay(circpad_machine_state_t *mi) * function below samples from [bin_start, bin_end) */ bin_end = circpad_histogram_bin_to_usec(mi, curr_bin+1); - /* Truncate the high bin in case it's the infinity bin: - * Don't actually schedule an "infinite"-1 delay */ - bin_end = MIN(bin_end, start_usec+state->range_usec); - - // Sample uniformly between histogram[i] to histogram[i+1]-1, - // but no need to sample if they are the same timeval (aka bin 0 or bin 1). - if (bin_end <= bin_start+1) + /* Bin edges are monotonically increasing so this is a bug. Handle it. */ + if (BUG(bin_start > bin_end)) { return bin_start; - else - return (circpad_delay_t)crypto_rand_uint64_range(bin_start, bin_end); + } + + /* Sample randomly from within the bin width */ + return (circpad_delay_t)crypto_rand_uint64_range(bin_start, bin_end); } /** @@ -627,7 +608,7 @@ circpad_machine_first_higher_index(const circpad_machine_state_t *mi, /* Don't remove from the infinity bin */ for (; bin < CIRCPAD_INFINITY_BIN(mi); bin++) { if (mi->histogram[bin] && - circpad_histogram_bin_to_usec(mi, bin+1) > target_bin_usec) { + histogram_get_bin_upper_bound(mi, bin) >= target_bin_usec) { return bin; } } @@ -2083,16 +2064,92 @@ circpad_setup_machine_on_circ(circuit_t *on_circ, on_circ->padding_machine[machine->machine_index] = machine; } -/* These padding machines are only used for tests pending #28634. */ #ifdef TOR_UNIT_TESTS +/** Validate a single state of a padding machine */ +static bool +padding_machine_state_is_valid(const circpad_state_t *state) +{ + int b; + uint32_t tokens_count = 0; + circpad_delay_t prev_bin_edge = 0; + + /* We only validate histograms */ + if (!state->histogram_len) { + return true; + } + + /* We need at least two bins in a histogram */ + if (state->histogram_len < 2) { + log_warn(LD_GENERAL, "You can't have a histogram with less than 2 bins"); + return false; + } + + /* For each machine state, if it's a histogram, make sure all the + * histogram edges are well defined (i.e. are strictly monotonic). */ + for (b = 0 ; b < state->histogram_len ; b++) { + /* Check that histogram edges are strictly increasing. Ignore the first + * edge since it can be zero. */ + if (prev_bin_edge >= state->histogram_edges[b] && b > 0) { + log_warn(LD_GENERAL, "Histogram edges are not increasing [%u/%u]", + prev_bin_edge, state->histogram_edges[b]); + return false; + } + + prev_bin_edge = state->histogram_edges[b]; + + /* Also count the number of tokens as we go through the histogram states */ + tokens_count += state->histogram[b]; + } + /* Verify that the total number of tokens is correct */ + if (tokens_count != state->histogram_total_tokens) { + log_warn(LD_GENERAL, "Histogram token count is wrong [%u/%u]", + tokens_count, state->histogram_total_tokens); + return false; + } + + return true; +} + +/** Basic validation of padding machine */ +static bool +padding_machine_is_valid(const circpad_machine_spec_t *machine) +{ + int i; + + /* Validate the histograms of the padding machine */ + for (i = 0 ; i < machine->num_states ; i++) { + if (!padding_machine_state_is_valid(&machine->states[i])) { + return false; + } + } + + return true; +} + +/* Validate and register <b>machine</b> into <b>machine_list</b>. If + * <b>machine_list</b> is NULL, then just validate. */ +STATIC void +register_padding_machine(circpad_machine_spec_t *machine, + smartlist_t *machine_list) +{ + if (!padding_machine_is_valid(machine)) { + log_warn(LD_GENERAL, "Machine #%u is invalid. Ignoring.", + machine->machine_num); + return; + } + + if (machine_list) { + smartlist_add(machine_list, machine); + } +} + +/* These padding machines are only used for tests pending #28634. */ static void circpad_circ_client_machine_init(void) { circpad_machine_spec_t *circ_client_machine = tor_malloc_zero(sizeof(circpad_machine_spec_t)); - // XXX: Better conditions for merge.. Or disable this machine in - // merge? circ_client_machine->conditions.min_hops = 2; circ_client_machine->conditions.state_mask = CIRCPAD_CIRC_BUILDING|CIRCPAD_CIRC_OPENED|CIRCPAD_CIRC_HAS_RELAY_EARLY; @@ -2124,19 +2181,20 @@ circpad_circ_client_machine_init(void) circ_client_machine->states[CIRCPAD_STATE_BURST].token_removal = CIRCPAD_TOKEN_REMOVAL_CLOSEST; - // FIXME: Tune this histogram circ_client_machine->states[CIRCPAD_STATE_BURST].histogram_len = 2; - circ_client_machine->states[CIRCPAD_STATE_BURST].start_usec = 500; - circ_client_machine->states[CIRCPAD_STATE_BURST].range_usec = 1000000; + circ_client_machine->states[CIRCPAD_STATE_BURST].histogram_edges[0]= 500; + circ_client_machine->states[CIRCPAD_STATE_BURST].histogram_edges[1]= 1000000; + /* We have 5 tokens in the histogram, which means that all circuits will look * like they have 7 hops (since we start this machine after the second hop, * and tokens are decremented for any valid hops, and fake extends are * used after that -- 2+5==7). */ circ_client_machine->states[CIRCPAD_STATE_BURST].histogram[0] = 5; + circ_client_machine->states[CIRCPAD_STATE_BURST].histogram_total_tokens = 5; circ_client_machine->machine_num = smartlist_len(origin_padding_machines); - smartlist_add(origin_padding_machines, circ_client_machine); + register_padding_machine(circ_client_machine, origin_padding_machines); } static void @@ -2183,8 +2241,9 @@ circpad_circ_responder_machine_init(void) circ_responder_machine->states[CIRCPAD_STATE_BURST].use_rtt_estimate = 1; /* The histogram is 2 bins: an empty one, and infinity */ circ_responder_machine->states[CIRCPAD_STATE_BURST].histogram_len = 2; - circ_responder_machine->states[CIRCPAD_STATE_BURST].start_usec = 5000; - circ_responder_machine->states[CIRCPAD_STATE_BURST].range_usec = 1000000; + circ_responder_machine->states[CIRCPAD_STATE_BURST].histogram_edges[0]= 500; + circ_responder_machine->states[CIRCPAD_STATE_BURST].histogram_edges[1] = + 1000000; /* During burst state we wait forever for padding to arrive. We are waiting for a padding cell from the client to come in, so that we @@ -2215,8 +2274,14 @@ circpad_circ_responder_machine_init(void) before you send a padding response */ circ_responder_machine->states[CIRCPAD_STATE_GAP].use_rtt_estimate = 1; circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_len = 6; - circ_responder_machine->states[CIRCPAD_STATE_GAP].start_usec = 5000; - circ_responder_machine->states[CIRCPAD_STATE_GAP].range_usec = 1000000; + /* Specify histogram bins */ + circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_edges[0]= 500; + circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_edges[1]= 1000; + circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_edges[2]= 2000; + circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_edges[3]= 4000; + circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_edges[4]= 8000; + circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_edges[5]= 16000; + /* Specify histogram tokens */ circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram[0] = 0; circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram[1] = 1; circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram[2] = 2; @@ -2224,11 +2289,12 @@ circpad_circ_responder_machine_init(void) circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram[4] = 1; /* Total number of tokens */ circ_responder_machine->states[CIRCPAD_STATE_GAP].histogram_total_tokens = 6; + circ_responder_machine->states[CIRCPAD_STATE_GAP].token_removal = CIRCPAD_TOKEN_REMOVAL_CLOSEST_USEC; circ_responder_machine->machine_num = smartlist_len(relay_padding_machines); - smartlist_add(relay_padding_machines, circ_responder_machine); + register_padding_machine(circ_responder_machine, relay_padding_machines); } #endif diff --git a/src/core/or/circuitpadding.h b/src/core/or/circuitpadding.h index 92fd4fc2d5..54039fa59a 100644 --- a/src/core/or/circuitpadding.h +++ b/src/core/or/circuitpadding.h @@ -228,13 +228,18 @@ typedef uint16_t circpad_statenum_t; #define CIRCPAD_STATENUM_MAX (UINT16_MAX) /** A histogram is used to sample padding delays given a machine state. This - * constant defines the maximum histogram width (i.e. the max number of bins) + * constant defines the maximum histogram width (i.e. the max number of bins). * - * Each histogram bin is twice as large as the previous. Two exceptions: The - * first bin has zero width (which means that minimum delay is applied to the - * next padding cell), and the last bin (infinity bin) has infinite width - * (which means that the next padding cell will be delayed infinitely). */ -#define CIRCPAD_MAX_HISTOGRAM_LEN (sizeof(circpad_delay_t)*8 + 1) + * The current limit is arbitrary and could be raised if there is a need, + * however too many bins will be hard to serialize in the future. + * + * Memory concerns are not so great here since the corresponding histogram and + * histogram_edges arrays are global and not per-circuit. + * + * If we ever upgrade this to a value that can't be represented by 8-bits we + * also need to upgrade circpad_hist_index_t. + */ +#define CIRCPAD_MAX_HISTOGRAM_LEN (100) /** * A state of a padding state machine. The information here are immutable and @@ -248,38 +253,60 @@ typedef uint16_t circpad_statenum_t; * or the consensus. */ typedef struct circpad_state_t { - /** If a histogram is used for this state, this specifies the number of bins - * of this histogram. Histograms must have at least 2 bins. + /** + * If a histogram is used for this state, this specifies the number of bins + * of this histogram. Histograms must have at least 2 bins. + * + * In particular, the following histogram: + * + * Tokens + * + + * 10 | +----+ + * 9 | | | +---------+ + * 8 | | | | | + * 7 | | | +-----+ | + * 6 +----+ Bin+-----+ | +---------------+ + * 5 | | #1 | | | | | + * | Bin| | Bin | Bin | Bin #4 | Bin #5 | + * | #0 | | #2 | #3 | | (infinity bin)| + * | | | | | | | + * | | | | | | | + * 0 +----+----+-----+-----+---------+---------------+ + * 0 100 200 350 500 1000 ∞ microseconds * - * If a delay probability distribution is used for this state, this is set - * to 0. */ + * would be specified the following way: + * histogram_len = 6; + * histogram[] = { 6, 10, 6, 7, 9, 6 } + * histogram_edges[] = { 0, 100, 200, 350, 500, 1000 } + * + * The final bin is called the "infinity bin" and if it's chosen we don't + * schedule any padding. The infinity bin is strange because its lower edge + * is the max value of possible non-infinite delay allowed by this histogram, + * and its upper edge is CIRCPAD_DELAY_INFINITE. You can tell if the infinity + * bin is chosen by inspecting its bin index or inspecting its upper edge. + * + * If a delay probability distribution is used for this state, this is set + * to 0. */ circpad_hist_index_t histogram_len; /** The histogram itself: an array of uint16s of tokens, whose - * widths are exponentially spaced, in microseconds */ + * widths are exponentially spaced, in microseconds. + * + * This array must have histogram_len elements that are strictly + * monotonically increasing. */ circpad_hist_token_t histogram[CIRCPAD_MAX_HISTOGRAM_LEN]; + /* The histogram bin edges in usec. + * + * Each element of this array specifies the left edge of the corresponding + * bin. The rightmost edge is always infinity and is not specified in this + * array. + * + * This array must have histogram_len elements. */ + circpad_delay_t histogram_edges[CIRCPAD_MAX_HISTOGRAM_LEN+1]; /** Total number of tokens in this histogram. This is a constant and is *not* * decremented every time we spend a token. It's used for initializing and * refilling the histogram. */ uint32_t histogram_total_tokens; - /** Minimum padding delay of this state in microseconds. - * - * If histograms are used, this is the left (and right) bound of the first - * bin (since it has zero width). - * - * If a delay probability distribution is used, this represents the minimum - * delay we can sample from the distribution. - */ - circpad_delay_t start_usec; - - /** If histograms are used, this is the width of the whole histogram in - * microseconds, and it's used to calculate individual bin width. - * - * If a delay probability distribution is used, this is used as the max - * delay we can sample from the distribution. - */ - circpad_delay_t range_usec; - /** * Represents a delay probability distribution (aka IAT distribution). It's a * parametrized way of encoding inter-packet delay information in @@ -292,6 +319,16 @@ typedef struct circpad_state_t { * results of sampling from this distribution (range_sec is used as a max). */ circpad_distribution_t iat_dist; + /* If a delay probability distribution is used, this is used as the max + * value we can sample from the distribution. However, RTT measurements and + * dist_added_shift gets applied on top of this value to derive the final + * padding delay. */ + circpad_delay_t dist_max_sample_usec; + /* If a delay probability distribution is used and this is set, we will add + * this value on top of the value sampled from the IAT distribution to + * derive the final padding delay (We also add the RTT measurement if it's + * enabled.). */ + circpad_delay_t dist_added_shift_usec; /** * The length dist is a parameterized way of encoding how long this @@ -686,9 +723,17 @@ circpad_send_command_to_hop,(struct origin_circuit_t *circ, uint8_t hopnum, uint8_t relay_command, const uint8_t *payload, ssize_t payload_len)); +STATIC circpad_delay_t +histogram_get_bin_upper_bound(const circpad_machine_state_t *mi, + circpad_hist_index_t bin); + #ifdef TOR_UNIT_TESTS extern smartlist_t *origin_padding_machines; extern smartlist_t *relay_padding_machines; + +STATIC void +register_padding_machine(circpad_machine_spec_t *machine, + smartlist_t *machine_list); #endif #endif diff --git a/src/feature/dirauth/shared_random.c b/src/feature/dirauth/shared_random.c index 34b2283250..137c49800f 100644 --- a/src/feature/dirauth/shared_random.c +++ b/src/feature/dirauth/shared_random.c @@ -120,8 +120,8 @@ static const char sr_flag_ns_str[] = "shared-rand-participate"; static int32_t num_srv_agreements_from_vote; /* Return a heap allocated copy of the SRV <b>orig</b>. */ -STATIC sr_srv_t * -srv_dup(const sr_srv_t *orig) +sr_srv_t * +sr_srv_dup(const sr_srv_t *orig) { sr_srv_t *duplicate = NULL; @@ -1253,8 +1253,8 @@ sr_act_post_consensus(const networkstatus_t *consensus) * decided by the majority. */ sr_state_unset_fresh_srv(); /* Set the SR values from the given consensus. */ - sr_state_set_previous_srv(srv_dup(consensus->sr_info.previous_srv)); - sr_state_set_current_srv(srv_dup(consensus->sr_info.current_srv)); + sr_state_set_previous_srv(sr_srv_dup(consensus->sr_info.previous_srv)); + sr_state_set_current_srv(sr_srv_dup(consensus->sr_info.current_srv)); } /* Prepare our state so that it's ready for the next voting period. */ diff --git a/src/feature/dirauth/shared_random.h b/src/feature/dirauth/shared_random.h index 25d95ebbc7..0b45ad1ed7 100644 --- a/src/feature/dirauth/shared_random.h +++ b/src/feature/dirauth/shared_random.h @@ -154,6 +154,7 @@ const char *sr_commit_get_rsa_fpr(const sr_commit_t *commit) void sr_compute_srv(void); sr_commit_t *sr_generate_our_commit(time_t timestamp, const authority_cert_t *my_rsa_cert); +sr_srv_t *sr_srv_dup(const sr_srv_t *orig); #ifdef SHARED_RANDOM_PRIVATE @@ -172,7 +173,6 @@ STATIC sr_srv_t *get_majority_srv_from_votes(const smartlist_t *votes, int current); STATIC void save_commit_to_state(sr_commit_t *commit); -STATIC sr_srv_t *srv_dup(const sr_srv_t *orig); STATIC int commitments_are_the_same(const sr_commit_t *commit_one, const sr_commit_t *commit_two); STATIC int commit_is_authoritative(const sr_commit_t *commit, diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c index 92f0b3e737..a7b7480edd 100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@ -102,6 +102,8 @@ static const config_format_t state_format = { &state_extra_var, }; +static void state_query_del_(sr_state_object_t obj_type, void *data); + /* Return a string representation of a protocol phase. */ STATIC const char * get_phase_str(sr_phase_t phase) @@ -834,6 +836,9 @@ state_query_get_commit(const char *rsa_fpr) static void * state_query_get_(sr_state_object_t obj_type, const void *data) { + if (BUG(!sr_state)) + return NULL; + void *obj = NULL; switch (obj_type) { @@ -862,23 +867,44 @@ state_query_get_(sr_state_object_t obj_type, const void *data) } /* Helper function: This handles the PUT state action using an - * <b>obj_type</b> and <b>data</b> needed for the action. */ + * <b>obj_type</b> and <b>data</b> needed for the action. + * PUT frees the previous data before replacing it, if needed. */ static void state_query_put_(sr_state_object_t obj_type, void *data) { + if (BUG(!sr_state)) + return; + switch (obj_type) { case SR_STATE_OBJ_COMMIT: { sr_commit_t *commit = data; tor_assert(commit); + /* commit_add_to_state() frees the old commit, if there is one */ commit_add_to_state(commit, sr_state); break; } case SR_STATE_OBJ_CURSRV: - sr_state->current_srv = (sr_srv_t *) data; + /* Check if the new pointer is the same as the old one: if it is, it's + * probably a bug. The caller may have confused current and previous, + * or they may have forgotten to sr_srv_dup(). + * Putting NULL multiple times is allowed. */ + if (!BUG(data && sr_state->current_srv == (sr_srv_t *) data)) { + /* We own the old SRV, so we need to free it. */ + state_query_del_(SR_STATE_OBJ_CURSRV, NULL); + sr_state->current_srv = (sr_srv_t *) data; + } break; case SR_STATE_OBJ_PREVSRV: - sr_state->previous_srv = (sr_srv_t *) data; + /* Check if the new pointer is the same as the old one: if it is, it's + * probably a bug. The caller may have confused current and previous, + * or they may have forgotten to sr_srv_dup(). + * Putting NULL multiple times is allowed. */ + if (!BUG(data && sr_state->previous_srv == (sr_srv_t *) data)) { + /* We own the old SRV, so we need to free it. */ + state_query_del_(SR_STATE_OBJ_PREVSRV, NULL); + sr_state->previous_srv = (sr_srv_t *) data; + } break; case SR_STATE_OBJ_VALID_AFTER: sr_state->valid_after = *((time_t *) data); @@ -898,6 +924,9 @@ state_query_put_(sr_state_object_t obj_type, void *data) static void state_query_del_all_(sr_state_object_t obj_type) { + if (BUG(!sr_state)) + return; + switch (obj_type) { case SR_STATE_OBJ_COMMIT: { @@ -926,6 +955,9 @@ state_query_del_(sr_state_object_t obj_type, void *data) { (void) data; + if (BUG(!sr_state)) + return; + switch (obj_type) { case SR_STATE_OBJ_PREVSRV: tor_free(sr_state->previous_srv); @@ -986,7 +1018,7 @@ state_query(sr_state_action_t action, sr_state_object_t obj_type, /* Delete the current SRV value from the state freeing it and the value is set * to NULL meaning empty. */ -static void +STATIC void state_del_current_srv(void) { state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_CURSRV, NULL, NULL); @@ -994,22 +1026,22 @@ state_del_current_srv(void) /* Delete the previous SRV value from the state freeing it and the value is * set to NULL meaning empty. */ -static void +STATIC void state_del_previous_srv(void) { state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_PREVSRV, NULL, NULL); } -/* Rotate SRV value by freeing the previous value, assigning the current - * value to the previous one and nullifying the current one. */ +/* Rotate SRV value by setting the previous SRV to the current SRV, and + * clearing the current SRV. */ STATIC void state_rotate_srv(void) { /* First delete previous SRV from the state. Object will be freed. */ state_del_previous_srv(); - /* Set previous SRV with the current one. */ - sr_state_set_previous_srv(sr_state_get_current_srv()); - /* Nullify the current srv. */ + /* Set previous SRV to a copy of the current one. */ + sr_state_set_previous_srv(sr_srv_dup(sr_state_get_current_srv())); + /* Free and NULL the current srv. */ sr_state_set_current_srv(NULL); } @@ -1030,7 +1062,9 @@ sr_state_get_phase(void) return *(sr_phase_t *) ptr; } -/* Return the previous SRV value from our state. Value CAN be NULL. */ +/* Return the previous SRV value from our state. Value CAN be NULL. + * The state object owns the SRV, so the calling code should not free the SRV. + * Use sr_srv_dup() if you want to keep a copy of the SRV. */ const sr_srv_t * sr_state_get_previous_srv(void) { @@ -1049,7 +1083,9 @@ sr_state_set_previous_srv(const sr_srv_t *srv) NULL); } -/* Return the current SRV value from our state. Value CAN be NULL. */ +/* Return the current SRV value from our state. Value CAN be NULL. + * The state object owns the SRV, so the calling code should not free the SRV. + * Use sr_srv_dup() if you want to keep a copy of the SRV. */ const sr_srv_t * sr_state_get_current_srv(void) { diff --git a/src/feature/dirauth/shared_random_state.h b/src/feature/dirauth/shared_random_state.h index 35626be3f6..08f999f9d4 100644 --- a/src/feature/dirauth/shared_random_state.h +++ b/src/feature/dirauth/shared_random_state.h @@ -140,6 +140,8 @@ STATIC int is_phase_transition(sr_phase_t next_phase); STATIC void set_sr_phase(sr_phase_t phase); STATIC sr_state_t *get_sr_state(void); +STATIC void state_del_previous_srv(void); +STATIC void state_del_current_srv(void); #endif /* defined(TOR_UNIT_TESTS) */ diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 597982b34e..1dae9c79c1 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -756,7 +756,14 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, idx < trn_cell_introduce_encrypted_get_nspec(enc_cell); idx++) { link_specifier_t *lspec = trn_cell_introduce_encrypted_get_nspecs(enc_cell, idx); - smartlist_add(data->link_specifiers, hs_link_specifier_dup(lspec)); + if (BUG(!lspec)) { + goto done; + } + link_specifier_t *lspec_dup = link_specifier_dup(lspec); + if (BUG(!lspec_dup)) { + goto done; + } + smartlist_add(data->link_specifiers, lspec_dup); } /* Success. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index e3873d2f18..253c24d643 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -565,81 +565,6 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) return; } -/* Add all possible link specifiers in node to lspecs: - * - legacy ID is mandatory thus MUST be present in node; - * - include ed25519 link specifier if present in the node, and the node - * supports ed25519 link authentication, even if its link versions are not - * compatible with us; - * - include IPv4 link specifier, if the primary address is not IPv4, log a - * BUG() warning, and return an empty smartlist; - * - include IPv6 link specifier if present in the node. */ -static void -get_lspecs_from_node(const node_t *node, smartlist_t *lspecs) -{ - link_specifier_t *ls; - tor_addr_port_t ap; - - tor_assert(node); - tor_assert(lspecs); - - /* Get the relay's IPv4 address. */ - node_get_prim_orport(node, &ap); - - /* We expect the node's primary address to be a valid IPv4 address. - * This conforms to the protocol, which requires either an IPv4 or IPv6 - * address (or both). */ - if (BUG(!tor_addr_is_v4(&ap.addr)) || - BUG(!tor_addr_port_is_valid_ap(&ap, 0))) { - return; - } - - ls = link_specifier_new(); - link_specifier_set_ls_type(ls, LS_IPV4); - link_specifier_set_un_ipv4_addr(ls, tor_addr_to_ipv4h(&ap.addr)); - link_specifier_set_un_ipv4_port(ls, ap.port); - /* Four bytes IPv4 and two bytes port. */ - link_specifier_set_ls_len(ls, sizeof(ap.addr.addr.in_addr) + - sizeof(ap.port)); - smartlist_add(lspecs, ls); - - /* Legacy ID is mandatory and will always be present in node. */ - ls = link_specifier_new(); - link_specifier_set_ls_type(ls, LS_LEGACY_ID); - memcpy(link_specifier_getarray_un_legacy_id(ls), node->identity, - link_specifier_getlen_un_legacy_id(ls)); - link_specifier_set_ls_len(ls, link_specifier_getlen_un_legacy_id(ls)); - smartlist_add(lspecs, ls); - - /* ed25519 ID is only included if the node has it, and the node declares a - protocol version that supports ed25519 link authentication, even if that - link version is not compatible with us. (We are sending the ed25519 key - to another tor, which may support different link versions.) */ - if (!ed25519_public_key_is_zero(&node->ed25519_id) && - node_supports_ed25519_link_authentication(node, 0)) { - ls = link_specifier_new(); - link_specifier_set_ls_type(ls, LS_ED25519_ID); - memcpy(link_specifier_getarray_un_ed25519_id(ls), &node->ed25519_id, - link_specifier_getlen_un_ed25519_id(ls)); - link_specifier_set_ls_len(ls, link_specifier_getlen_un_ed25519_id(ls)); - smartlist_add(lspecs, ls); - } - - /* Check for IPv6. If so, include it as well. */ - if (node_has_ipv6_orport(node)) { - ls = link_specifier_new(); - node_get_pref_ipv6_orport(node, &ap); - link_specifier_set_ls_type(ls, LS_IPV6); - size_t addr_len = link_specifier_getlen_un_ipv6_addr(ls); - const uint8_t *in6_addr = tor_addr_to_in6_addr8(&ap.addr); - uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(ls); - memcpy(ipv6_array, in6_addr, addr_len); - link_specifier_set_un_ipv6_port(ls, ap.port); - /* Sixteen bytes IPv6 and two bytes port. */ - link_specifier_set_ls_len(ls, addr_len + sizeof(ap.port)); - smartlist_add(lspecs, ls); - } -} - /* Using the given descriptor intro point ip, the node of the * rendezvous point rp_node and the service's subcredential, populate the * already allocated intro1_data object with the needed key material and link @@ -662,10 +587,9 @@ setup_introduce1_data(const hs_desc_intro_point_t *ip, tor_assert(subcredential); tor_assert(intro1_data); - /* Build the link specifiers from the extend information of the rendezvous - * circuit that we've picked previously. */ - rp_lspecs = smartlist_new(); - get_lspecs_from_node(rp_node, rp_lspecs); + /* Build the link specifiers from the node at the end of the rendezvous + * circuit that we opened for this introduction. */ + rp_lspecs = node_get_link_specifier_smartlist(rp_node, 0); if (smartlist_len(rp_lspecs) == 0) { /* We can't rendezvous without link specifiers. */ smartlist_free(rp_lspecs); @@ -1044,9 +968,7 @@ hs_circ_handle_introduce2(const hs_service_t *service, ret = 0; done: - SMARTLIST_FOREACH(data.link_specifiers, link_specifier_t *, lspec, - link_specifier_free(lspec)); - smartlist_free(data.link_specifiers); + link_specifier_smartlist_free(data.link_specifiers); memwipe(&data, 0, sizeof(data)); return ret; } diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 075f1d5c41..c34271efca 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -546,13 +546,15 @@ find_desc_intro_point_by_legacy_id(const char *legacy_id, SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, hs_desc_intro_point_t *, ip) { SMARTLIST_FOREACH_BEGIN(ip->link_specifiers, - const hs_desc_link_specifier_t *, lspec) { + const link_specifier_t *, lspec) { /* Not all tor node have an ed25519 identity key so we still rely on the * legacy identity digest. */ - if (lspec->type != LS_LEGACY_ID) { + if (link_specifier_get_ls_type(lspec) != LS_LEGACY_ID) { continue; } - if (fast_memneq(legacy_id, lspec->u.legacy_id, DIGEST_LEN)) { + if (fast_memneq(legacy_id, + link_specifier_getconstarray_un_legacy_id(lspec), + DIGEST_LEN)) { break; } /* Found it. */ @@ -771,24 +773,13 @@ STATIC extend_info_t * desc_intro_point_to_extend_info(const hs_desc_intro_point_t *ip) { extend_info_t *ei; - smartlist_t *lspecs = smartlist_new(); tor_assert(ip); - /* We first encode the descriptor link specifiers into the binary - * representation which is a trunnel object. */ - SMARTLIST_FOREACH_BEGIN(ip->link_specifiers, - const hs_desc_link_specifier_t *, desc_lspec) { - link_specifier_t *lspec = hs_desc_lspec_to_trunnel(desc_lspec); - smartlist_add(lspecs, lspec); - } SMARTLIST_FOREACH_END(desc_lspec); - /* Explicitly put the direct connection option to 0 because this is client * side and there is no such thing as a non anonymous client. */ - ei = hs_get_extend_info_from_lspecs(lspecs, &ip->onion_key, 0); + ei = hs_get_extend_info_from_lspecs(ip->link_specifiers, &ip->onion_key, 0); - SMARTLIST_FOREACH(lspecs, link_specifier_t *, ls, link_specifier_free(ls)); - smartlist_free(lspecs); return ei; } diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index 14655c53a5..b2227432d2 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1010,24 +1010,6 @@ hs_build_address(const ed25519_public_key_t *key, uint8_t version, tor_assert(hs_address_is_valid(addr_out)); } -/* Return a newly allocated copy of lspec. */ -link_specifier_t * -hs_link_specifier_dup(const link_specifier_t *lspec) -{ - link_specifier_t *result = link_specifier_new(); - memcpy(result, lspec, sizeof(*result)); - /* The unrecognized field is a dynamic array so make sure to copy its - * content and not the pointer. */ - link_specifier_setlen_un_unrecognized( - result, link_specifier_getlen_un_unrecognized(lspec)); - if (link_specifier_getlen_un_unrecognized(result)) { - memcpy(link_specifier_getarray_un_unrecognized(result), - link_specifier_getconstarray_un_unrecognized(lspec), - link_specifier_getlen_un_unrecognized(result)); - } - return result; -} - /* From a given ed25519 public key pk and an optional secret, compute a * blinded public key and put it in blinded_pk_out. This is only useful to * the client side because the client only has access to the identity public @@ -1698,6 +1680,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, tor_assert(lspecs); + if (smartlist_len(lspecs) == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, "Empty link specifier list."); + /* Return NULL. */ + goto done; + } + SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) { switch (link_specifier_get_ls_type(ls)) { case LS_IPV4: @@ -1731,6 +1719,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, /* Legacy ID is mandatory, and we require IPv4. */ if (!have_v4 || !have_legacy_id) { + bool both = !have_v4 && !have_legacy_id; + log_fn(LOG_PROTOCOL_WARN, LD_REND, "Missing %s%s%s link specifier%s.", + !have_v4 ? "IPv4" : "", + both ? " and " : "", + !have_legacy_id ? "legacy ID" : "", + both ? "s" : ""); goto done; } @@ -1749,6 +1743,10 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, * release. */ } else { /* If we can't reach IPv4, return NULL. */ + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Received an IPv4 link specifier, " + "but the address is not reachable: %s:%u", + fmt_addr(&addr_v4), port_v4); goto done; } @@ -1756,7 +1754,7 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, validate: /* We'll validate now that the address we've picked isn't a private one. If - * it is, are we allowing to extend to private address? */ + * it is, are we allowed to extend to private addresses? */ if (!extend_info_addr_is_allowed(&addr_v4)) { log_fn(LOG_PROTOCOL_WARN, LD_REND, "Requested address is private and we are not allowed to extend to " @@ -1828,3 +1826,42 @@ hs_inc_rdv_stream_counter(origin_circuit_t *circ) tor_assert_nonfatal_unreached(); } } + +/* Return a newly allocated link specifier object that is a copy of dst. */ +link_specifier_t * +link_specifier_dup(const link_specifier_t *src) +{ + link_specifier_t *dup = NULL; + uint8_t *buf = NULL; + + if (BUG(!src)) { + goto err; + } + + ssize_t encoded_len_alloc = link_specifier_encoded_len(src); + if (BUG(encoded_len_alloc < 0)) { + goto err; + } + + buf = tor_malloc_zero(encoded_len_alloc); + ssize_t encoded_len_data = link_specifier_encode(buf, + encoded_len_alloc, + src); + if (BUG(encoded_len_data < 0)) { + goto err; + } + + ssize_t parsed_len = link_specifier_parse(&dup, buf, encoded_len_alloc); + if (BUG(parsed_len < 0)) { + goto err; + } + + goto done; + + err: + dup = NULL; + + done: + tor_free(buf); + return dup; +} diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h index a44505930a..abf39fa431 100644 --- a/src/feature/hs/hs_common.h +++ b/src/feature/hs/hs_common.h @@ -217,8 +217,6 @@ uint64_t hs_get_time_period_num(time_t now); uint64_t hs_get_next_time_period_num(time_t now); time_t hs_get_start_time_of_next_time_period(time_t now); -link_specifier_t *hs_link_specifier_dup(const link_specifier_t *lspec); - MOCK_DECL(int, hs_in_period_between_tp_and_srv, (const networkstatus_t *consensus, time_t now)); @@ -262,6 +260,8 @@ extend_info_t *hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, const struct curve25519_public_key_t *onion_key, int direct_conn); +link_specifier_t *link_specifier_dup(const link_specifier_t *src); + #ifdef HS_COMMON_PRIVATE STATIC void get_disaster_srv(uint64_t time_period_num, uint8_t *srv_out); diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index b09d50e010..8f7bdf86ef 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -324,12 +324,11 @@ encode_link_specifiers(const smartlist_t *specs) link_specifier_list_set_n_spec(lslist, smartlist_len(specs)); - SMARTLIST_FOREACH_BEGIN(specs, const hs_desc_link_specifier_t *, + SMARTLIST_FOREACH_BEGIN(specs, const link_specifier_t *, spec) { - link_specifier_t *ls = hs_desc_lspec_to_trunnel(spec); - if (ls) { - link_specifier_list_add_spec(lslist, ls); - } + link_specifier_t *ls = link_specifier_dup(spec); + tor_assert(ls); + link_specifier_list_add_spec(lslist, ls); } SMARTLIST_FOREACH_END(spec); { @@ -1190,52 +1189,22 @@ decode_link_specifiers(const char *encoded) results = smartlist_new(); for (i = 0; i < link_specifier_list_getlen_spec(specs); i++) { - hs_desc_link_specifier_t *hs_spec; link_specifier_t *ls = link_specifier_list_get_spec(specs, i); - tor_assert(ls); - - hs_spec = tor_malloc_zero(sizeof(*hs_spec)); - hs_spec->type = link_specifier_get_ls_type(ls); - switch (hs_spec->type) { - case LS_IPV4: - tor_addr_from_ipv4h(&hs_spec->u.ap.addr, - link_specifier_get_un_ipv4_addr(ls)); - hs_spec->u.ap.port = link_specifier_get_un_ipv4_port(ls); - break; - case LS_IPV6: - tor_addr_from_ipv6_bytes(&hs_spec->u.ap.addr, (const char *) - link_specifier_getarray_un_ipv6_addr(ls)); - hs_spec->u.ap.port = link_specifier_get_un_ipv6_port(ls); - break; - case LS_LEGACY_ID: - /* Both are known at compile time so let's make sure they are the same - * else we can copy memory out of bound. */ - tor_assert(link_specifier_getlen_un_legacy_id(ls) == - sizeof(hs_spec->u.legacy_id)); - memcpy(hs_spec->u.legacy_id, link_specifier_getarray_un_legacy_id(ls), - sizeof(hs_spec->u.legacy_id)); - break; - case LS_ED25519_ID: - /* Both are known at compile time so let's make sure they are the same - * else we can copy memory out of bound. */ - tor_assert(link_specifier_getlen_un_ed25519_id(ls) == - sizeof(hs_spec->u.ed25519_id)); - memcpy(hs_spec->u.ed25519_id, - link_specifier_getconstarray_un_ed25519_id(ls), - sizeof(hs_spec->u.ed25519_id)); - break; - default: - tor_free(hs_spec); + if (BUG(!ls)) { goto err; } - - smartlist_add(results, hs_spec); + link_specifier_t *ls_dup = link_specifier_dup(ls); + if (BUG(!ls_dup)) { + goto err; + } + smartlist_add(results, ls_dup); } goto done; err: if (results) { - SMARTLIST_FOREACH(results, hs_desc_link_specifier_t *, s, tor_free(s)); + SMARTLIST_FOREACH(results, link_specifier_t *, s, + link_specifier_free(s)); smartlist_free(results); results = NULL; } @@ -2878,8 +2847,8 @@ hs_desc_intro_point_free_(hs_desc_intro_point_t *ip) return; } if (ip->link_specifiers) { - SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *, - ls, hs_desc_link_specifier_free(ls)); + SMARTLIST_FOREACH(ip->link_specifiers, link_specifier_t *, + ls, link_specifier_free(ls)); smartlist_free(ip->link_specifiers); } tor_cert_free(ip->auth_key_cert); @@ -2972,69 +2941,6 @@ hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client) tor_free(client); } -/* Free the given descriptor link specifier. */ -void -hs_desc_link_specifier_free_(hs_desc_link_specifier_t *ls) -{ - if (ls == NULL) { - return; - } - tor_free(ls); -} - -/* Return a newly allocated descriptor link specifier using the given extend - * info and requested type. Return NULL on error. */ -hs_desc_link_specifier_t * -hs_desc_link_specifier_new(const extend_info_t *info, uint8_t type) -{ - hs_desc_link_specifier_t *ls = NULL; - - tor_assert(info); - - ls = tor_malloc_zero(sizeof(*ls)); - ls->type = type; - switch (ls->type) { - case LS_IPV4: - if (info->addr.family != AF_INET) { - goto err; - } - tor_addr_copy(&ls->u.ap.addr, &info->addr); - ls->u.ap.port = info->port; - break; - case LS_IPV6: - if (info->addr.family != AF_INET6) { - goto err; - } - tor_addr_copy(&ls->u.ap.addr, &info->addr); - ls->u.ap.port = info->port; - break; - case LS_LEGACY_ID: - /* Bug out if the identity digest is not set */ - if (BUG(tor_mem_is_zero(info->identity_digest, - sizeof(info->identity_digest)))) { - goto err; - } - memcpy(ls->u.legacy_id, info->identity_digest, sizeof(ls->u.legacy_id)); - break; - case LS_ED25519_ID: - /* ed25519 keys are optional for intro points */ - if (ed25519_public_key_is_zero(&info->ed_identity)) { - goto err; - } - memcpy(ls->u.ed25519_id, info->ed_identity.pubkey, - sizeof(ls->u.ed25519_id)); - break; - default: - /* Unknown type is code flow error. */ - tor_assert(0); - } - - return ls; - err: - tor_free(ls); - return NULL; -} - /* From the given descriptor, remove and free every introduction point. */ void hs_descriptor_clear_intro_points(hs_descriptor_t *desc) @@ -3050,59 +2956,3 @@ hs_descriptor_clear_intro_points(hs_descriptor_t *desc) smartlist_clear(ips); } } - -/* From a descriptor link specifier object spec, returned a newly allocated - * link specifier object that is the encoded representation of spec. Return - * NULL on error. */ -link_specifier_t * -hs_desc_lspec_to_trunnel(const hs_desc_link_specifier_t *spec) -{ - tor_assert(spec); - - link_specifier_t *ls = link_specifier_new(); - link_specifier_set_ls_type(ls, spec->type); - - switch (spec->type) { - case LS_IPV4: - link_specifier_set_un_ipv4_addr(ls, - tor_addr_to_ipv4h(&spec->u.ap.addr)); - link_specifier_set_un_ipv4_port(ls, spec->u.ap.port); - /* Four bytes IPv4 and two bytes port. */ - link_specifier_set_ls_len(ls, sizeof(spec->u.ap.addr.addr.in_addr) + - sizeof(spec->u.ap.port)); - break; - case LS_IPV6: - { - size_t addr_len = link_specifier_getlen_un_ipv6_addr(ls); - const uint8_t *in6_addr = tor_addr_to_in6_addr8(&spec->u.ap.addr); - uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(ls); - memcpy(ipv6_array, in6_addr, addr_len); - link_specifier_set_un_ipv6_port(ls, spec->u.ap.port); - /* Sixteen bytes IPv6 and two bytes port. */ - link_specifier_set_ls_len(ls, addr_len + sizeof(spec->u.ap.port)); - break; - } - case LS_LEGACY_ID: - { - size_t legacy_id_len = link_specifier_getlen_un_legacy_id(ls); - uint8_t *legacy_id_array = link_specifier_getarray_un_legacy_id(ls); - memcpy(legacy_id_array, spec->u.legacy_id, legacy_id_len); - link_specifier_set_ls_len(ls, legacy_id_len); - break; - } - case LS_ED25519_ID: - { - size_t ed25519_id_len = link_specifier_getlen_un_ed25519_id(ls); - uint8_t *ed25519_id_array = link_specifier_getarray_un_ed25519_id(ls); - memcpy(ed25519_id_array, spec->u.ed25519_id, ed25519_id_len); - link_specifier_set_ls_len(ls, ed25519_id_len); - break; - } - default: - tor_assert_nonfatal_unreached(); - link_specifier_free(ls); - ls = NULL; - } - - return ls; -} diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h index 04a8e16d63..dbe0cb1c94 100644 --- a/src/feature/hs/hs_descriptor.h +++ b/src/feature/hs/hs_descriptor.h @@ -69,28 +69,10 @@ typedef enum { HS_DESC_AUTH_ED25519 = 1 } hs_desc_auth_type_t; -/* Link specifier object that contains information on how to extend to the - * relay that is the address, port and handshake type. */ -typedef struct hs_desc_link_specifier_t { - /* Indicate the type of link specifier. See trunnel ed25519_cert - * specification. */ - uint8_t type; - - /* It must be one of these types, can't be more than one. */ - union { - /* IP address and port of the relay use to extend. */ - tor_addr_port_t ap; - /* Legacy identity. A 20-byte SHA1 identity fingerprint. */ - uint8_t legacy_id[DIGEST_LEN]; - /* ed25519 identity. A 32-byte key. */ - uint8_t ed25519_id[ED25519_PUBKEY_LEN]; - } u; -} hs_desc_link_specifier_t; - /* Introduction point information located in a descriptor. */ typedef struct hs_desc_intro_point_t { /* Link specifier(s) which details how to extend to the relay. This list - * contains hs_desc_link_specifier_t object. It MUST have at least one. */ + * contains link_specifier_t objects. It MUST have at least one. */ smartlist_t *link_specifiers; /* Onion key of the introduction point used to extend to it for the ntor @@ -261,12 +243,6 @@ void hs_desc_encrypted_data_free_(hs_desc_encrypted_data_t *desc); #define hs_desc_encrypted_data_free(desc) \ FREE_AND_NULL(hs_desc_encrypted_data_t, hs_desc_encrypted_data_free_, (desc)) -void hs_desc_link_specifier_free_(hs_desc_link_specifier_t *ls); -#define hs_desc_link_specifier_free(ls) \ - FREE_AND_NULL(hs_desc_link_specifier_t, hs_desc_link_specifier_free_, (ls)) - -hs_desc_link_specifier_t *hs_desc_link_specifier_new( - const extend_info_t *info, uint8_t type); void hs_descriptor_clear_intro_points(hs_descriptor_t *desc); MOCK_DECL(int, @@ -299,9 +275,6 @@ void hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client); FREE_AND_NULL(hs_desc_authorized_client_t, \ hs_desc_authorized_client_free_, (client)) -link_specifier_t *hs_desc_lspec_to_trunnel( - const hs_desc_link_specifier_t *spec); - 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 * diff --git a/src/feature/hs/hs_intropoint.c b/src/feature/hs/hs_intropoint.c index b28a5c2b80..c9cd3a0419 100644 --- a/src/feature/hs/hs_intropoint.c +++ b/src/feature/hs/hs_intropoint.c @@ -601,8 +601,8 @@ hs_intropoint_clear(hs_intropoint_t *ip) return; } tor_cert_free(ip->auth_key_cert); - SMARTLIST_FOREACH(ip->link_specifiers, hs_desc_link_specifier_t *, ls, - hs_desc_link_specifier_free(ls)); + SMARTLIST_FOREACH(ip->link_specifiers, link_specifier_t *, ls, + link_specifier_free(ls)); smartlist_free(ip->link_specifiers); memset(ip, 0, sizeof(hs_intropoint_t)); } diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 8d286f2bad..e3d0043460 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -280,9 +280,10 @@ describe_intro_point(const hs_service_intro_point_t *ip) const char *legacy_id = NULL; SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers, - const hs_desc_link_specifier_t *, lspec) { - if (lspec->type == LS_LEGACY_ID) { - legacy_id = (const char *) lspec->u.legacy_id; + const link_specifier_t *, lspec) { + if (link_specifier_get_ls_type(lspec) == LS_LEGACY_ID) { + legacy_id = (const char *) + link_specifier_getconstarray_un_legacy_id(lspec); break; } } SMARTLIST_FOREACH_END(lspec); @@ -426,23 +427,16 @@ service_intro_point_free_void(void *obj) } /* Return a newly allocated service intro point and fully initialized from the - * given extend_info_t ei if non NULL. - * If is_legacy is true, we also generate the legacy key. - * If supports_ed25519_link_handshake_any is true, we add the relay's ed25519 - * key to the link specifiers. + * given node_t node, if non NULL. * - * If ei is NULL, returns a hs_service_intro_point_t with an empty link + * If node is NULL, returns a hs_service_intro_point_t with an empty link * specifier list and no onion key. (This is used for testing.) * On any other error, NULL is returned. * - * ei must be an extend_info_t containing an IPv4 address. (We will add supoort - * for IPv6 in a later release.) When calling extend_info_from_node(), pass - * 0 in for_direct_connection to make sure ei always has an IPv4 address. */ + * node must be an node_t with an IPv4 address. */ STATIC hs_service_intro_point_t * -service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, - unsigned int supports_ed25519_link_handshake_any) +service_intro_point_new(const node_t *node) { - hs_desc_link_specifier_t *ls; hs_service_intro_point_t *ip; ip = tor_malloc_zero(sizeof(*ip)); @@ -472,12 +466,17 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, ip->replay_cache = replaycache_new(0, 0); /* Initialize the base object. We don't need the certificate object. */ - ip->base.link_specifiers = smartlist_new(); + ip->base.link_specifiers = node_get_link_specifier_smartlist(node, 0); + + if (node == NULL) { + goto done; + } /* Generate the encryption key for this intro point. */ curve25519_keypair_generate(&ip->enc_key_kp, 0); - /* Figure out if this chosen node supports v3 or is legacy only. */ - if (is_legacy) { + /* Figure out if this chosen node supports v3 or is legacy only. + * NULL nodes are used in the unit tests. */ + if (!node_supports_ed25519_hs_intro(node)) { ip->base.is_only_legacy = 1; /* Legacy mode that is doesn't support v3+ with ed25519 auth key. */ ip->legacy_key = crypto_pk_new(); @@ -490,40 +489,9 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, } } - if (ei == NULL) { - goto done; - } - - /* We'll try to add all link specifiers. Legacy is mandatory. - * IPv4 or IPv6 is required, and we always send IPv4. */ - ls = hs_desc_link_specifier_new(ei, LS_IPV4); - /* It is impossible to have an extend info object without a v4. */ - if (BUG(!ls)) { - goto err; - } - smartlist_add(ip->base.link_specifiers, ls); - - ls = hs_desc_link_specifier_new(ei, LS_LEGACY_ID); - /* It is impossible to have an extend info object without an identity - * digest. */ - if (BUG(!ls)) { - goto err; - } - smartlist_add(ip->base.link_specifiers, ls); - - /* ed25519 identity key is optional for intro points. If the node supports - * ed25519 link authentication, we include it. */ - if (supports_ed25519_link_handshake_any) { - ls = hs_desc_link_specifier_new(ei, LS_ED25519_ID); - if (ls) { - smartlist_add(ip->base.link_specifiers, ls); - } - } - - /* IPv6 is not supported in this release. */ - - /* Finally, copy onion key from the extend_info_t object. */ - memcpy(&ip->onion_key, &ei->curve25519_onion_key, sizeof(ip->onion_key)); + /* Finally, copy onion key from the node. */ + memcpy(&ip->onion_key, node_get_curve25519_onion_key(node), + sizeof(ip->onion_key)); done: return ip; @@ -656,16 +624,16 @@ get_objects_from_ident(const hs_ident_circuit_t *ident, * encountered in the link specifier list. Return NULL if it can't be found. * * The caller does NOT have ownership of the object, the intro point does. */ -static hs_desc_link_specifier_t * +static link_specifier_t * get_link_spec_by_type(const hs_service_intro_point_t *ip, uint8_t type) { - hs_desc_link_specifier_t *lnk_spec = NULL; + link_specifier_t *lnk_spec = NULL; tor_assert(ip); SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers, - hs_desc_link_specifier_t *, ls) { - if (ls->type == type) { + link_specifier_t *, ls) { + if (link_specifier_get_ls_type(ls) == type) { lnk_spec = ls; goto end; } @@ -681,7 +649,7 @@ get_link_spec_by_type(const hs_service_intro_point_t *ip, uint8_t type) STATIC const node_t * get_node_from_intro_point(const hs_service_intro_point_t *ip) { - const hs_desc_link_specifier_t *ls; + const link_specifier_t *ls; tor_assert(ip); @@ -690,7 +658,8 @@ get_node_from_intro_point(const hs_service_intro_point_t *ip) return NULL; } /* XXX In the future, we want to only use the ed25519 ID (#22173). */ - return node_get_by_id((const char *) ls->u.legacy_id); + return node_get_by_id( + (const char *) link_specifier_getconstarray_un_legacy_id(ls)); } /* Given a service intro point, return the extend_info_t for it. This can @@ -1557,7 +1526,7 @@ remember_failing_intro_point(const hs_service_intro_point_t *ip, hs_service_descriptor_t *desc, time_t now) { time_t *time_of_failure, *prev_ptr; - const hs_desc_link_specifier_t *legacy_ls; + const link_specifier_t *legacy_ls; tor_assert(ip); tor_assert(desc); @@ -1566,22 +1535,13 @@ remember_failing_intro_point(const hs_service_intro_point_t *ip, *time_of_failure = now; legacy_ls = get_link_spec_by_type(ip, LS_LEGACY_ID); tor_assert(legacy_ls); - prev_ptr = digestmap_set(desc->intro_points.failed_id, - (const char *) legacy_ls->u.legacy_id, - time_of_failure); + prev_ptr = digestmap_set( + desc->intro_points.failed_id, + (const char *) link_specifier_getconstarray_un_legacy_id(legacy_ls), + time_of_failure); tor_free(prev_ptr); } -/* Copy the descriptor link specifier object from src to dst. */ -static void -link_specifier_copy(hs_desc_link_specifier_t *dst, - const hs_desc_link_specifier_t *src) -{ - tor_assert(dst); - tor_assert(src); - memcpy(dst, src, sizeof(hs_desc_link_specifier_t)); -} - /* Using a given descriptor signing keypair signing_kp, a service intro point * object ip and the time now, setup the content of an already allocated * descriptor intro desc_ip. @@ -1616,9 +1576,14 @@ setup_desc_intro_point(const ed25519_keypair_t *signing_kp, /* Copy link specifier(s). */ SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers, - const hs_desc_link_specifier_t *, ls) { - hs_desc_link_specifier_t *copy = tor_malloc_zero(sizeof(*copy)); - link_specifier_copy(copy, ls); + const link_specifier_t *, ls) { + if (BUG(!ls)) { + goto done; + } + link_specifier_t *copy = link_specifier_dup(ls); + if (BUG(!copy)) { + goto done; + } smartlist_add(desc_ip->link_specifiers, copy); } SMARTLIST_FOREACH_END(ls); @@ -2107,7 +2072,6 @@ static hs_service_intro_point_t * pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes) { const node_t *node; - extend_info_t *info = NULL; hs_service_intro_point_t *ip = NULL; /* Normal 3-hop introduction point flags. */ router_crn_flags_t flags = CRN_NEED_UPTIME | CRN_NEED_DESC; @@ -2128,43 +2092,17 @@ pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes) * we don't want to use that node anymore. */ smartlist_add(exclude_nodes, (void *) node); - /* We do this to ease our life but also this call makes appropriate checks - * of the node object such as validating ntor support for instance. - * - * We must provide an extend_info for clients to connect over a 3-hop path, - * so we don't pass direct_conn here. */ - info = extend_info_from_node(node, 0); - if (BUG(info == NULL)) { - goto err; - } - - /* Let's do a basic sanity check here so that we don't end up advertising the - * ed25519 identity key of relays that don't actually support the link - * protocol */ - if (!node_supports_ed25519_link_authentication(node, 0)) { - tor_assert_nonfatal(ed25519_public_key_is_zero(&info->ed_identity)); - } else { - /* Make sure we *do* have an ed key if we support the link authentication. - * Sending an empty key would result in a failure to extend. */ - tor_assert_nonfatal(!ed25519_public_key_is_zero(&info->ed_identity)); - } + /* Create our objects and populate them with the node information. */ + ip = service_intro_point_new(node); - /* Create our objects and populate them with the node information. - * We don't care if the intro's link auth is compatible with us, because - * we are sending the ed25519 key to a remote client via the descriptor. */ - ip = service_intro_point_new(info, !node_supports_ed25519_hs_intro(node), - node_supports_ed25519_link_authentication(node, - 0)); if (ip == NULL) { goto err; } - log_info(LD_REND, "Picked intro point: %s", extend_info_describe(info)); - extend_info_free(info); + log_info(LD_REND, "Picked intro point: %s", node_describe(node)); return ip; err: service_intro_point_free(ip); - extend_info_free(info); return NULL; } diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index ec53f2f23b..8d7f773219 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -369,10 +369,7 @@ STATIC hs_service_t *find_service(hs_service_ht *map, STATIC void remove_service(hs_service_ht *map, hs_service_t *service); STATIC int register_service(hs_service_ht *map, hs_service_t *service); /* Service introduction point functions. */ -STATIC hs_service_intro_point_t *service_intro_point_new( - const extend_info_t *ei, - unsigned int is_legacy, - unsigned int supports_ed25519_link_handshake_any); +STATIC hs_service_intro_point_t *service_intro_point_new(const node_t *node); STATIC void service_intro_point_free_(hs_service_intro_point_t *ip); #define service_intro_point_free(ip) \ FREE_AND_NULL(hs_service_intro_point_t, \ diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 9a27701803..32ee2677bb 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -1189,6 +1189,102 @@ node_get_rsa_id_digest(const node_t *node) return (const uint8_t*)node->identity; } +/* Returns a new smartlist with all possible link specifiers from node: + * - legacy ID is mandatory thus MUST be present in node; + * - include ed25519 link specifier if present in the node, and the node + * supports ed25519 link authentication, and: + * - if direct_conn is true, its link versions are compatible with us, + * - if direct_conn is false, regardless of its link versions; + * - include IPv4 link specifier, if the primary address is not IPv4, log a + * BUG() warning, and return an empty smartlist; + * - include IPv6 link specifier if present in the node. + * + * If node is NULL, returns an empty smartlist. + * + * The smartlist must be freed using link_specifier_smartlist_free(). */ +smartlist_t * +node_get_link_specifier_smartlist(const node_t *node, bool direct_conn) +{ + link_specifier_t *ls; + tor_addr_port_t ap; + smartlist_t *lspecs = smartlist_new(); + + if (!node) + return lspecs; + + /* Get the relay's IPv4 address. */ + node_get_prim_orport(node, &ap); + + /* We expect the node's primary address to be a valid IPv4 address. + * This conforms to the protocol, which requires either an IPv4 or IPv6 + * address (or both). */ + if (BUG(!tor_addr_is_v4(&ap.addr)) || + BUG(!tor_addr_port_is_valid_ap(&ap, 0))) { + return lspecs; + } + + ls = link_specifier_new(); + link_specifier_set_ls_type(ls, LS_IPV4); + link_specifier_set_un_ipv4_addr(ls, tor_addr_to_ipv4h(&ap.addr)); + link_specifier_set_un_ipv4_port(ls, ap.port); + /* Four bytes IPv4 and two bytes port. */ + link_specifier_set_ls_len(ls, sizeof(ap.addr.addr.in_addr) + + sizeof(ap.port)); + smartlist_add(lspecs, ls); + + /* Legacy ID is mandatory and will always be present in node. */ + ls = link_specifier_new(); + link_specifier_set_ls_type(ls, LS_LEGACY_ID); + memcpy(link_specifier_getarray_un_legacy_id(ls), node->identity, + link_specifier_getlen_un_legacy_id(ls)); + link_specifier_set_ls_len(ls, link_specifier_getlen_un_legacy_id(ls)); + smartlist_add(lspecs, ls); + + /* ed25519 ID is only included if the node has it, and the node declares a + protocol version that supports ed25519 link authentication. + If direct_conn is true, we also require that the node's link version is + compatible with us. (Otherwise, we will be sending the ed25519 key + to another tor, which may support different link versions.) */ + if (!ed25519_public_key_is_zero(&node->ed25519_id) && + node_supports_ed25519_link_authentication(node, direct_conn)) { + ls = link_specifier_new(); + link_specifier_set_ls_type(ls, LS_ED25519_ID); + memcpy(link_specifier_getarray_un_ed25519_id(ls), &node->ed25519_id, + link_specifier_getlen_un_ed25519_id(ls)); + link_specifier_set_ls_len(ls, link_specifier_getlen_un_ed25519_id(ls)); + smartlist_add(lspecs, ls); + } + + /* Check for IPv6. If so, include it as well. */ + if (node_has_ipv6_orport(node)) { + ls = link_specifier_new(); + node_get_pref_ipv6_orport(node, &ap); + link_specifier_set_ls_type(ls, LS_IPV6); + size_t addr_len = link_specifier_getlen_un_ipv6_addr(ls); + const uint8_t *in6_addr = tor_addr_to_in6_addr8(&ap.addr); + uint8_t *ipv6_array = link_specifier_getarray_un_ipv6_addr(ls); + memcpy(ipv6_array, in6_addr, addr_len); + link_specifier_set_un_ipv6_port(ls, ap.port); + /* Sixteen bytes IPv6 and two bytes port. */ + link_specifier_set_ls_len(ls, addr_len + sizeof(ap.port)); + smartlist_add(lspecs, ls); + } + + return lspecs; +} + +/* Free a link specifier list. */ +void +link_specifier_smartlist_free_(smartlist_t *ls_list) +{ + if (!ls_list) + return; + + SMARTLIST_FOREACH(ls_list, link_specifier_t *, lspec, + link_specifier_free(lspec)); + smartlist_free(ls_list); +} + /** Return the nickname of <b>node</b>, or NULL if we can't find one. */ const char * node_get_nickname(const node_t *node) diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index 3420959618..a3d65347a8 100644 --- a/src/feature/nodelist/nodelist.h +++ b/src/feature/nodelist/nodelist.h @@ -77,6 +77,11 @@ int node_supports_v3_hsdir(const node_t *node); int node_supports_ed25519_hs_intro(const node_t *node); int node_supports_v3_rendezvous_point(const node_t *node); const uint8_t *node_get_rsa_id_digest(const node_t *node); +smartlist_t *node_get_link_specifier_smartlist(const node_t *node, + bool direct_conn); +void link_specifier_smartlist_free_(smartlist_t *ls_list); +#define link_specifier_smartlist_free(ls_list) \ + FREE_AND_NULL(smartlist_t, link_specifier_smartlist_free_, (ls_list)) int node_has_ipv6_addr(const node_t *node); int node_has_ipv6_orport(const node_t *node); diff --git a/src/lib/net/address.c b/src/lib/net/address.c index e1c9e1310f..214d8aa3eb 100644 --- a/src/lib/net/address.c +++ b/src/lib/net/address.c @@ -238,9 +238,18 @@ tor_addr_make_null(tor_addr_t *a, sa_family_t family) a->family = family; } -/** Return true iff <b>ip</b> is an IP reserved to localhost or local networks - * in RFC1918 or RFC4193 or RFC4291. (fec0::/10, deprecated by RFC3879, is - * also treated as internal for now.) +/** Return true iff <b>ip</b> is an IP reserved to localhost or local networks. + * + * If <b>ip</b> is in RFC1918 or RFC4193 or RFC4291, we will return true. + * (fec0::/10, deprecated by RFC3879, is also treated as internal for now + * and will return true.) + * + * If <b>ip</b> is 0.0.0.0 or 100.64.0.0/10 (RFC6598), we will act as: + * - Internal if <b>for_listening</b> is 0, as these addresses are not + * routable on the internet and we won't be publicly accessible to clients. + * - External if <b>for_listening</b> is 1, as clients could connect to us + * from the internet (in the case of 0.0.0.0) or a service provider's + * internal network (in the case of RFC6598). */ int tor_addr_is_internal_(const tor_addr_t *addr, int for_listening, @@ -288,11 +297,13 @@ tor_addr_is_internal_(const tor_addr_t *addr, int for_listening, return 0; } else if (v_family == AF_INET) { - if (for_listening && !iph4) /* special case for binding to 0.0.0.0 */ + /* special case for binding to 0.0.0.0 or 100.64/10 (RFC6598) */ + if (for_listening && (!iph4 || ((iph4 & 0xffc00000) == 0x64400000))) return 0; if (((iph4 & 0xff000000) == 0x0a000000) || /* 10/8 */ ((iph4 & 0xff000000) == 0x00000000) || /* 0/8 */ ((iph4 & 0xff000000) == 0x7f000000) || /* 127/8 */ + ((iph4 & 0xffc00000) == 0x64400000) || /* 100.64/10 */ ((iph4 & 0xffff0000) == 0xa9fe0000) || /* 169.254/16 */ ((iph4 & 0xfff00000) == 0xac100000) || /* 172.16/12 */ ((iph4 & 0xffff0000) == 0xc0a80000)) /* 192.168/16 */ diff --git a/src/lib/time/compat_time.c b/src/lib/time/compat_time.c index 33e077a587..c6625c7806 100644 --- a/src/lib/time/compat_time.c +++ b/src/lib/time/compat_time.c @@ -522,7 +522,9 @@ monotime_init_internal(void) GetTickCount64_fn = (GetTickCount64_fn_t) GetProcAddress(h, "GetTickCount64"); } - // FreeLibrary(h) ? + // We can't call FreeLibrary(h) here, because freeing the handle may + // unload the library, and cause future calls to GetTickCount64_fn() + // to fail. See 29642 for details. } void diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index f2ae8398df..c57bdc730b 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -21,26 +21,35 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, /* For a usable intro point we need at least two link specifiers: One legacy * keyid and one ipv4 */ { - hs_desc_link_specifier_t *ls_legacy = tor_malloc_zero(sizeof(*ls_legacy)); - hs_desc_link_specifier_t *ls_v4 = tor_malloc_zero(sizeof(*ls_v4)); - ls_legacy->type = LS_LEGACY_ID; - memcpy(ls_legacy->u.legacy_id, "0299F268FCA9D55CD157976D39AE92B4B455B3A8", - DIGEST_LEN); - ls_v4->u.ap.port = 9001; - int family = tor_addr_parse(&ls_v4->u.ap.addr, addr); + tor_addr_t a; + tor_addr_make_unspec(&a); + link_specifier_t *ls_legacy = link_specifier_new(); + link_specifier_t *ls_ip = link_specifier_new(); + link_specifier_set_ls_type(ls_legacy, LS_LEGACY_ID); + memset(link_specifier_getarray_un_legacy_id(ls_legacy), 'C', + link_specifier_getlen_un_legacy_id(ls_legacy)); + int family = tor_addr_parse(&a, addr); switch (family) { case AF_INET: - ls_v4->type = LS_IPV4; + link_specifier_set_ls_type(ls_ip, LS_IPV4); + link_specifier_set_un_ipv4_addr(ls_ip, tor_addr_to_ipv4h(&a)); + link_specifier_set_un_ipv4_port(ls_ip, 9001); break; case AF_INET6: - ls_v4->type = LS_IPV6; + link_specifier_set_ls_type(ls_ip, LS_IPV6); + memcpy(link_specifier_getarray_un_ipv6_addr(ls_ip), + tor_addr_to_in6_addr8(&a), + link_specifier_getlen_un_ipv6_addr(ls_ip)); + link_specifier_set_un_ipv6_port(ls_ip, 9001); break; default: - /* Stop the test, not suppose to have an error. */ - tt_int_op(family, OP_EQ, AF_INET); + /* Stop the test, not supposed to have an error. + * Compare with -1 to show the actual family. + */ + tt_int_op(family, OP_EQ, -1); } smartlist_add(ip->link_specifiers, ls_legacy); - smartlist_add(ip->link_specifiers, ls_v4); + smartlist_add(ip->link_specifiers, ls_ip); } ret = ed25519_keypair_generate(&auth_kp, 0); @@ -153,8 +162,11 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, /* Add four intro points. */ smartlist_add(desc->encrypted_data.intro_points, hs_helper_build_intro_point(signing_kp, now, "1.2.3.4", 0)); +/* IPv6-only introduction points are not supported yet, see #23588 */ +#if 0 smartlist_add(desc->encrypted_data.intro_points, hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0)); +#endif smartlist_add(desc->encrypted_data.intro_points, hs_helper_build_intro_point(signing_kp, now, "3.2.1.4", 1)); smartlist_add(desc->encrypted_data.intro_points, @@ -202,7 +214,6 @@ void hs_helper_desc_equal(const hs_descriptor_t *desc1, const hs_descriptor_t *desc2) { - char *addr1 = NULL, *addr2 = NULL; /* Plaintext data section. */ tt_int_op(desc1->plaintext_data.version, OP_EQ, desc2->plaintext_data.version); @@ -291,35 +302,57 @@ hs_helper_desc_equal(const hs_descriptor_t *desc1, tt_int_op(smartlist_len(ip1->link_specifiers), ==, smartlist_len(ip2->link_specifiers)); for (int j = 0; j < smartlist_len(ip1->link_specifiers); j++) { - hs_desc_link_specifier_t *ls1 = smartlist_get(ip1->link_specifiers, j), - *ls2 = smartlist_get(ip2->link_specifiers, j); - tt_int_op(ls1->type, ==, ls2->type); - switch (ls1->type) { + link_specifier_t *ls1 = smartlist_get(ip1->link_specifiers, j), + *ls2 = smartlist_get(ip2->link_specifiers, j); + tt_int_op(link_specifier_get_ls_type(ls1), ==, + link_specifier_get_ls_type(ls2)); + switch (link_specifier_get_ls_type(ls1)) { case LS_IPV4: + { + uint32_t addr1 = link_specifier_get_un_ipv4_addr(ls1); + uint32_t addr2 = link_specifier_get_un_ipv4_addr(ls2); + tt_int_op(addr1, OP_EQ, addr2); + uint16_t port1 = link_specifier_get_un_ipv4_port(ls1); + uint16_t port2 = link_specifier_get_un_ipv4_port(ls2); + tt_int_op(port1, ==, port2); + } + break; case LS_IPV6: { - addr1 = tor_addr_to_str_dup(&ls1->u.ap.addr); - addr2 = tor_addr_to_str_dup(&ls2->u.ap.addr); - tt_str_op(addr1, OP_EQ, addr2); - tor_free(addr1); - tor_free(addr2); - tt_int_op(ls1->u.ap.port, ==, ls2->u.ap.port); + const uint8_t *addr1 = + link_specifier_getconstarray_un_ipv6_addr(ls1); + const uint8_t *addr2 = + link_specifier_getconstarray_un_ipv6_addr(ls2); + tt_int_op(link_specifier_getlen_un_ipv6_addr(ls1), OP_EQ, + link_specifier_getlen_un_ipv6_addr(ls2)); + tt_mem_op(addr1, OP_EQ, addr2, + link_specifier_getlen_un_ipv6_addr(ls1)); + uint16_t port1 = link_specifier_get_un_ipv6_port(ls1); + uint16_t port2 = link_specifier_get_un_ipv6_port(ls2); + tt_int_op(port1, ==, port2); } break; case LS_LEGACY_ID: - tt_mem_op(ls1->u.legacy_id, OP_EQ, ls2->u.legacy_id, - sizeof(ls1->u.legacy_id)); + { + const uint8_t *id1 = + link_specifier_getconstarray_un_legacy_id(ls1); + const uint8_t *id2 = + link_specifier_getconstarray_un_legacy_id(ls2); + tt_int_op(link_specifier_getlen_un_legacy_id(ls1), OP_EQ, + link_specifier_getlen_un_legacy_id(ls2)); + tt_mem_op(id1, OP_EQ, id2, + link_specifier_getlen_un_legacy_id(ls1)); + } break; default: /* Unknown type, caught it and print its value. */ - tt_int_op(ls1->type, OP_EQ, -1); + tt_int_op(link_specifier_get_ls_type(ls1), OP_EQ, -1); } } } } done: - tor_free(addr1); - tor_free(addr2); + ; } diff --git a/src/test/test-network.sh b/src/test/test-network.sh index 372c8cbac3..4d56e83806 100755 --- a/src/test/test-network.sh +++ b/src/test/test-network.sh @@ -1,4 +1,4 @@ -#!/bin/sh +#!/usr/bin/env bash # This script calls the equivalent script in chutney/tools @@ -18,6 +18,10 @@ ECHO="${ECHO:-echo}" # Output is prefixed with the name of the script myname=$(basename "$0") +# Save the arguments before we destroy them +# This might not preserve arguments with spaces in them +ORIGINAL_ARGS=( "$@" ) + # We need to find CHUTNEY_PATH, so that we can call the version of this script # in chutney/tools with the same arguments. We also need to respect --quiet. until [ -z "$1" ] @@ -95,7 +99,7 @@ if [ -d "$CHUTNEY_PATH" ] && [ -x "$TEST_NETWORK" ]; then # this may fail if some arguments have spaces in them # if so, set CHUTNEY_PATH before calling test-network.sh, and spaces # will be handled correctly - exec "$TEST_NETWORK" "$@" + exec "$TEST_NETWORK" "${ORIGINAL_ARGS[@]}" # $ORIGINAL_ARGS else $ECHO "$myname: Could not find tools/test-network.sh in CHUTNEY_PATH." $ECHO "$myname: Please update your chutney using 'git pull'." diff --git a/src/test/test_addr.c b/src/test/test_addr.c index 1aa7a3dcd6..fb8df5f0fb 100644 --- a/src/test/test_addr.c +++ b/src/test/test_addr.c @@ -1243,6 +1243,23 @@ test_addr_make_null(void *data) tor_free(zeros); } +#define TEST_ADDR_INTERNAL(a, for_listening, rv) STMT_BEGIN \ + tor_addr_t t; \ + tt_int_op(tor_inet_pton(AF_INET, a, &t.addr.in_addr), OP_EQ, 1); \ + t.family = AF_INET; \ + tt_int_op(tor_addr_is_internal(&t, for_listening), OP_EQ, rv); \ + STMT_END; + +static void +test_addr_rfc6598(void *arg) +{ + (void)arg; + TEST_ADDR_INTERNAL("100.64.0.1", 0, 1); + TEST_ADDR_INTERNAL("100.64.0.1", 1, 0); + done: + ; +} + #define ADDR_LEGACY(name) \ { #name, test_addr_ ## name , 0, NULL, NULL } @@ -1257,5 +1274,6 @@ struct testcase_t addr_tests[] = { { "sockaddr_to_str", test_addr_sockaddr_to_str, 0, NULL, NULL }, { "is_loopback", test_addr_is_loopback, 0, NULL, NULL }, { "make_null", test_addr_make_null, 0, NULL, NULL }, + { "rfc6598", test_addr_rfc6598, 0, NULL, NULL }, END_OF_TESTCASES }; diff --git a/src/test/test_circuitpadding.c b/src/test/test_circuitpadding.c index eee1edc50c..1976e54bb9 100644 --- a/src/test/test_circuitpadding.c +++ b/src/test/test_circuitpadding.c @@ -333,7 +333,7 @@ test_circuitpadding_rtt(void *arg) OP_EQ, relay_side->padding_info[0]->rtt_estimate_usec+ circpad_machine_current_state( - relay_side->padding_info[0])->start_usec); + relay_side->padding_info[0])->histogram_edges[0]); circpad_cell_event_nonpadding_received((circuit_t*)relay_side); circpad_cell_event_nonpadding_received((circuit_t*)relay_side); @@ -349,7 +349,7 @@ test_circuitpadding_rtt(void *arg) OP_EQ, relay_side->padding_info[0]->rtt_estimate_usec+ circpad_machine_current_state( - relay_side->padding_info[0])->start_usec); + relay_side->padding_info[0])->histogram_edges[0]); /* Test 2: Termination of RTT measurement (from the previous test) */ tt_int_op(relay_side->padding_info[0]->stop_rtt_update, OP_EQ, 1); @@ -367,7 +367,7 @@ test_circuitpadding_rtt(void *arg) OP_EQ, relay_side->padding_info[0]->rtt_estimate_usec+ circpad_machine_current_state( - relay_side->padding_info[0])->start_usec); + relay_side->padding_info[0])->histogram_edges[0]); /* Test 3: Make sure client side machine properly ignores RTT */ circpad_cell_event_nonpadding_received((circuit_t*)client_side); @@ -383,7 +383,7 @@ test_circuitpadding_rtt(void *arg) tt_int_op(circpad_histogram_bin_to_usec(client_side->padding_info[0], 0), OP_EQ, circpad_machine_current_state( - client_side->padding_info[0])->start_usec); + client_side->padding_info[0])->histogram_edges[0]); done: free_fake_orcirc(relay_side); circuitmux_detach_all_circuits(dummy_channel.cmux, NULL); @@ -414,19 +414,23 @@ helper_create_basic_machine(void) circ_client_machine.states[CIRCPAD_STATE_BURST]. next_state[CIRCPAD_EVENT_NONPADDING_SENT] = CIRCPAD_STATE_CANCEL; - // FIXME: Is this what we want? circ_client_machine.states[CIRCPAD_STATE_BURST].token_removal = CIRCPAD_TOKEN_REMOVAL_HIGHER; - // FIXME: Tune this histogram circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_len = 5; - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 500; - circ_client_machine.states[CIRCPAD_STATE_BURST].range_usec = 1000000; + + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[0] = 500; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[1] = 2500; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[2] = 5000; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[3] = 10000; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[4] = 20000; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram[0] = 1; circ_client_machine.states[CIRCPAD_STATE_BURST].histogram[1] = 0; circ_client_machine.states[CIRCPAD_STATE_BURST].histogram[2] = 2; circ_client_machine.states[CIRCPAD_STATE_BURST].histogram[3] = 2; circ_client_machine.states[CIRCPAD_STATE_BURST].histogram[4] = 2; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_total_tokens = 7; circ_client_machine.states[CIRCPAD_STATE_BURST].use_rtt_estimate = 1; @@ -458,15 +462,25 @@ helper_create_machine_with_big_histogram(circpad_removal_t removal_strategy) burst_state->token_removal = CIRCPAD_TOKEN_REMOVAL_HIGHER; burst_state->histogram_len = BIG_HISTOGRAM_LEN; - burst_state->start_usec = 0; - burst_state->range_usec = 1000; int n_tokens = 0; - for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { + int i; + for (i = 0; i < BIG_HISTOGRAM_LEN ; i++) { burst_state->histogram[i] = tokens_per_bin; n_tokens += tokens_per_bin; } + burst_state->histogram_edges[0] = 0; + burst_state->histogram_edges[1] = 1; + burst_state->histogram_edges[2] = 7; + burst_state->histogram_edges[3] = 15; + burst_state->histogram_edges[4] = 31; + burst_state->histogram_edges[5] = 62; + burst_state->histogram_edges[6] = 125; + burst_state->histogram_edges[7] = 250; + burst_state->histogram_edges[8] = 500; + burst_state->histogram_edges[9] = 1000; + burst_state->histogram_total_tokens = n_tokens; burst_state->length_dist.type = CIRCPAD_DIST_UNIFORM; burst_state->length_dist.param1 = n_tokens; @@ -527,12 +541,20 @@ test_circuitpadding_token_removal_higher(void *arg) /* Test left boundaries of each histogram bin: */ const circpad_delay_t bin_left_bounds[] = - {0, 1, 7, 15, 31, 62, 125, 250, 500, CIRCPAD_DELAY_INFINITE}; - for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { + {0, 1, 7, 15, 31, 62, 125, 250, 500, 1000, CIRCPAD_DELAY_INFINITE}; + for (int i = 0; i <= BIG_HISTOGRAM_LEN ; i++) { tt_uint_op(bin_left_bounds[i], OP_EQ, circpad_histogram_bin_to_usec(mi, i)); } + /* Test right boundaries of each histogram bin: */ + const circpad_delay_t bin_right_bounds[] = + {0, 6, 14, 30, 61, 124, 249, 499, 999, CIRCPAD_DELAY_INFINITE-1}; + for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { + tt_uint_op(bin_right_bounds[i], OP_EQ, + histogram_get_bin_upper_bound(mi, i)); + } + /* Check that all bins have two tokens right now */ for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { tt_int_op(mi->histogram[i], OP_EQ, 2); @@ -576,8 +598,8 @@ test_circuitpadding_token_removal_higher(void *arg) /* Test below the lowest bin, for coverage */ tt_int_op(client_side->padding_info[0]->current_state, OP_EQ, CIRCPAD_STATE_BURST); - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 100; - mi->padding_scheduled_at_usec = current_time - 1; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[0] = 100; + mi->padding_scheduled_at_usec = current_time; circpad_machine_remove_token(mi); tt_int_op(mi->histogram[0], OP_EQ, 1); @@ -624,8 +646,8 @@ test_circuitpadding_token_removal_lower(void *arg) /* Test left boundaries of each histogram bin: */ const circpad_delay_t bin_left_bounds[] = - {0, 1, 7, 15, 31, 62, 125, 250, 500, CIRCPAD_DELAY_INFINITE}; - for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { + {0, 1, 7, 15, 31, 62, 125, 250, 500, 1000, CIRCPAD_DELAY_INFINITE}; + for (int i = 0; i <= BIG_HISTOGRAM_LEN ; i++) { tt_uint_op(bin_left_bounds[i], OP_EQ, circpad_histogram_bin_to_usec(mi, i)); } @@ -673,7 +695,8 @@ test_circuitpadding_token_removal_lower(void *arg) /* Test above the highest bin, for coverage */ tt_int_op(client_side->padding_info[0]->current_state, OP_EQ, CIRCPAD_STATE_BURST); - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 100; + circ_client_machine.states[CIRCPAD_STATE_BURST]. + histogram_edges[BIG_HISTOGRAM_LEN-2] = 100; mi->padding_scheduled_at_usec = current_time - 29202; circpad_machine_remove_token(mi); tt_int_op(mi->histogram[BIG_HISTOGRAM_LEN-2], OP_EQ, 1); @@ -721,8 +744,8 @@ test_circuitpadding_closest_token_removal(void *arg) /* Test left boundaries of each histogram bin: */ const circpad_delay_t bin_left_bounds[] = - {0, 1, 7, 15, 31, 62, 125, 250, 500, CIRCPAD_DELAY_INFINITE}; - for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { + {0, 1, 7, 15, 31, 62, 125, 250, 500, 1000, CIRCPAD_DELAY_INFINITE}; + for (int i = 0; i <= BIG_HISTOGRAM_LEN ; i++) { tt_uint_op(bin_left_bounds[i], OP_EQ, circpad_histogram_bin_to_usec(mi, i)); } @@ -769,7 +792,9 @@ test_circuitpadding_closest_token_removal(void *arg) /* Test below the lowest bin, for coverage */ tt_int_op(client_side->padding_info[0]->current_state, OP_EQ, CIRCPAD_STATE_BURST); - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 100; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[0] = 100; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[1] = 101; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[2] = 120; mi->padding_scheduled_at_usec = current_time - 102; mi->histogram[0] = 0; circpad_machine_remove_token(mi); @@ -778,7 +803,6 @@ test_circuitpadding_closest_token_removal(void *arg) /* Test above the highest bin, for coverage */ tt_int_op(client_side->padding_info[0]->current_state, OP_EQ, CIRCPAD_STATE_BURST); - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 100; mi->padding_scheduled_at_usec = current_time - 29202; circpad_machine_remove_token(mi); tt_int_op(mi->histogram[BIG_HISTOGRAM_LEN-2], OP_EQ, 1); @@ -826,8 +850,8 @@ test_circuitpadding_closest_token_removal_usec(void *arg) /* Test left boundaries of each histogram bin: */ const circpad_delay_t bin_left_bounds[] = - {0, 1, 7, 15, 31, 62, 125, 250, 500, CIRCPAD_DELAY_INFINITE}; - for (int i = 0; i < BIG_HISTOGRAM_LEN ; i++) { + {0, 1, 7, 15, 31, 62, 125, 250, 500, 1000, CIRCPAD_DELAY_INFINITE}; + for (int i = 0; i <= BIG_HISTOGRAM_LEN ; i++) { tt_uint_op(bin_left_bounds[i], OP_EQ, circpad_histogram_bin_to_usec(mi, i)); } @@ -877,7 +901,9 @@ test_circuitpadding_closest_token_removal_usec(void *arg) /* Test below the lowest bin, for coverage */ tt_int_op(client_side->padding_info[0]->current_state, OP_EQ, CIRCPAD_STATE_BURST); - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 100; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[0] = 100; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[1] = 101; + circ_client_machine.states[CIRCPAD_STATE_BURST].histogram_edges[2] = 120; mi->padding_scheduled_at_usec = current_time - 102; mi->histogram[0] = 0; circpad_machine_remove_token(mi); @@ -886,7 +912,8 @@ test_circuitpadding_closest_token_removal_usec(void *arg) /* Test above the highest bin, for coverage */ tt_int_op(client_side->padding_info[0]->current_state, OP_EQ, CIRCPAD_STATE_BURST); - circ_client_machine.states[CIRCPAD_STATE_BURST].start_usec = 100; + circ_client_machine.states[CIRCPAD_STATE_BURST]. + histogram_edges[BIG_HISTOGRAM_LEN-2] = 100; mi->padding_scheduled_at_usec = current_time - 29202; circpad_machine_remove_token(mi); tt_int_op(mi->histogram[BIG_HISTOGRAM_LEN-2], OP_EQ, 1); @@ -1043,7 +1070,7 @@ test_circuitpadding_tokens(void *arg) // Test 1: converting usec->bin->usec->bin // Bin 0+1 have different semantics. - for (circpad_delay_t i = 0; i <= state->start_usec+1; i++) { + for (circpad_delay_t i = 0; i <= state->histogram_edges[0]; i++) { int bin = circpad_histogram_usec_to_bin(client_side->padding_info[0], i); circpad_delay_t usec = @@ -1053,8 +1080,9 @@ test_circuitpadding_tokens(void *arg) tt_int_op(bin, OP_EQ, bin2); tt_int_op(i, OP_LE, usec); } - for (circpad_delay_t i = state->start_usec+1; - i <= state->start_usec + state->range_usec; i++) { + for (circpad_delay_t i = state->histogram_edges[0]+1; + i <= state->histogram_edges[0] + + state->histogram_edges[state->histogram_len-2]; i++) { int bin = circpad_histogram_usec_to_bin(client_side->padding_info[0], i); circpad_delay_t usec = @@ -1106,8 +1134,7 @@ test_circuitpadding_tokens(void *arg) /* 2.c. Bin 0 */ { tt_int_op(mi->histogram[0], OP_EQ, 1); - circpad_machine_remove_higher_token(mi, - state->start_usec/2); + circpad_machine_remove_higher_token(mi, state->histogram_edges[0]/2); tt_int_op(mi->histogram[0], OP_EQ, 0); } @@ -1126,8 +1153,7 @@ test_circuitpadding_tokens(void *arg) /* 3.a. Bin 0 */ { tt_int_op(mi->histogram[0], OP_EQ, 1); - circpad_machine_remove_higher_token(mi, - state->start_usec/2); + circpad_machine_remove_higher_token(mi, state->histogram_edges[0]/2); tt_int_op(mi->histogram[0], OP_EQ, 0); } @@ -1615,15 +1641,20 @@ helper_create_conditional_machine(void) ret->states[CIRCPAD_STATE_BURST]. next_state[CIRCPAD_EVENT_LENGTH_COUNT] = CIRCPAD_STATE_END; + /* Use EXACT removal strategy, otherwise setup_tokens() does not work */ ret->states[CIRCPAD_STATE_BURST].token_removal = - CIRCPAD_TOKEN_REMOVAL_NONE; + CIRCPAD_TOKEN_REMOVAL_EXACT; ret->states[CIRCPAD_STATE_BURST].histogram_len = 3; - ret->states[CIRCPAD_STATE_BURST].start_usec = 0; - ret->states[CIRCPAD_STATE_BURST].range_usec = 1000000; + + ret->states[CIRCPAD_STATE_BURST].histogram_edges[0] = 0; + ret->states[CIRCPAD_STATE_BURST].histogram_edges[1] = 1; + ret->states[CIRCPAD_STATE_BURST].histogram_edges[2] = 1000000; + ret->states[CIRCPAD_STATE_BURST].histogram[0] = 6; ret->states[CIRCPAD_STATE_BURST].histogram[1] = 0; - ret->states[CIRCPAD_STATE_BURST].histogram[1] = 0; + ret->states[CIRCPAD_STATE_BURST].histogram[2] = 0; + ret->states[CIRCPAD_STATE_BURST].histogram_total_tokens = 6; ret->states[CIRCPAD_STATE_BURST].use_rtt_estimate = 0; ret->states[CIRCPAD_STATE_BURST].length_includes_nonpadding = 1; @@ -1654,8 +1685,7 @@ helper_create_conditional_machines(void) add->conditions.state_mask = CIRCPAD_CIRC_BUILDING| CIRCPAD_CIRC_NO_STREAMS|CIRCPAD_CIRC_HAS_RELAY_EARLY; add->conditions.purpose_mask = CIRCPAD_PURPOSE_ALL; - - smartlist_add(origin_padding_machines, add); + register_padding_machine(add, origin_padding_machines); add = helper_create_conditional_machine(); add->machine_num = 3; @@ -1674,15 +1704,15 @@ helper_create_conditional_machines(void) add->conditions.state_mask = CIRCPAD_CIRC_OPENED| CIRCPAD_CIRC_STREAMS|CIRCPAD_CIRC_HAS_NO_RELAY_EARLY; add->conditions.purpose_mask = CIRCPAD_PURPOSE_ALL; - smartlist_add(origin_padding_machines, add); + register_padding_machine(add, origin_padding_machines); add = helper_create_conditional_machine(); add->machine_num = 2; - smartlist_add(relay_padding_machines, add); + register_padding_machine(add, relay_padding_machines); add = helper_create_conditional_machine(); add->machine_num = 3; - smartlist_add(relay_padding_machines, add); + register_padding_machine(add, relay_padding_machines); } void @@ -2069,48 +2099,48 @@ helper_circpad_circ_distribution_machine_setup(int min, int max) zero_st->iat_dist.type = CIRCPAD_DIST_UNIFORM; zero_st->iat_dist.param1 = min; zero_st->iat_dist.param2 = max; - zero_st->start_usec = min; - zero_st->range_usec = max; + zero_st->dist_added_shift_usec = min; + zero_st->dist_max_sample_usec = max; circpad_state_t *first_st = &circ_client_machine.states[1]; first_st->next_state[CIRCPAD_EVENT_NONPADDING_RECV] = 2; first_st->iat_dist.type = CIRCPAD_DIST_LOGISTIC; first_st->iat_dist.param1 = min; first_st->iat_dist.param2 = max; - first_st->start_usec = min; - first_st->range_usec = max; + first_st->dist_added_shift_usec = min; + first_st->dist_max_sample_usec = max; circpad_state_t *second_st = &circ_client_machine.states[2]; second_st->next_state[CIRCPAD_EVENT_NONPADDING_RECV] = 3; second_st->iat_dist.type = CIRCPAD_DIST_LOG_LOGISTIC; second_st->iat_dist.param1 = min; second_st->iat_dist.param2 = max; - second_st->start_usec = min; - second_st->range_usec = max; + second_st->dist_added_shift_usec = min; + second_st->dist_max_sample_usec = max; circpad_state_t *third_st = &circ_client_machine.states[3]; third_st->next_state[CIRCPAD_EVENT_NONPADDING_RECV] = 4; third_st->iat_dist.type = CIRCPAD_DIST_GEOMETRIC; third_st->iat_dist.param1 = min; third_st->iat_dist.param2 = max; - third_st->start_usec = min; - third_st->range_usec = max; + third_st->dist_added_shift_usec = min; + third_st->dist_max_sample_usec = max; circpad_state_t *fourth_st = &circ_client_machine.states[4]; fourth_st->next_state[CIRCPAD_EVENT_NONPADDING_RECV] = 5; fourth_st->iat_dist.type = CIRCPAD_DIST_WEIBULL; fourth_st->iat_dist.param1 = min; fourth_st->iat_dist.param2 = max; - fourth_st->start_usec = min; - fourth_st->range_usec = max; + fourth_st->dist_added_shift_usec = min; + fourth_st->dist_max_sample_usec = max; circpad_state_t *fifth_st = &circ_client_machine.states[5]; fifth_st->next_state[CIRCPAD_EVENT_NONPADDING_RECV] = 6; fifth_st->iat_dist.type = CIRCPAD_DIST_PARETO; fifth_st->iat_dist.param1 = min; fifth_st->iat_dist.param2 = max; - fifth_st->start_usec = min; - fifth_st->range_usec = max; + fifth_st->dist_added_shift_usec = min; + fifth_st->dist_max_sample_usec = max; } /** Simple test that the padding delays sampled from a uniform distribution diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c index 0c93f593ce..6e00e8807e 100644 --- a/src/test/test_hs_cell.c +++ b/src/test/test_hs_cell.c @@ -39,7 +39,7 @@ test_gen_establish_intro_cell(void *arg) attempt to parse it. */ { /* We only need the auth key pair here. */ - hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0); + hs_service_intro_point_t *ip = service_intro_point_new(NULL); /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ ret = hs_cell_build_establish_intro(circ_nonce, ip, buf); @@ -107,7 +107,7 @@ test_gen_establish_intro_cell_bad(void *arg) ed25519_sign_prefixed() function and make it fail. */ cell = trn_cell_establish_intro_new(); tt_assert(cell); - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, NULL); service_intro_point_free(ip); expect_log_msg_containing("Unable to make signature for " diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 2f2bb45581..8362b6cbda 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -403,6 +403,9 @@ test_client_pick_intro(void *arg) /* 2) Mark all intro points except _the chosen one_ as failed. Then query the * desc and get a random intro: check that we got _the chosen one_. */ { + /* Tell hs_get_extend_info_from_lspecs() to skip the private address check. + */ + get_options_mutable()->ExtendAllowPrivateAddresses = 1; /* Pick the chosen intro point and get its ei */ hs_desc_intro_point_t *chosen_intro_point = smartlist_get(desc->encrypted_data.intro_points, 0); @@ -476,6 +479,7 @@ test_client_pick_intro(void *arg) SMARTLIST_FOREACH_BEGIN(desc->encrypted_data.intro_points, hs_desc_intro_point_t *, ip) { extend_info_t *intro_ei = desc_intro_point_to_extend_info(ip); + tt_assert(intro_ei); if (intro_ei) { const char *ptr; char ip_addr[TOR_ADDR_BUF_LEN]; diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index de584ed47a..09c6c3e700 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -179,115 +179,6 @@ test_descriptor_padding(void *arg) } static void -test_link_specifier(void *arg) -{ - ssize_t ret; - hs_desc_link_specifier_t spec; - smartlist_t *link_specifiers = smartlist_new(); - char buf[256]; - char *b64 = NULL; - link_specifier_t *ls = NULL; - - (void) arg; - - /* Always this port. */ - spec.u.ap.port = 42; - smartlist_add(link_specifiers, &spec); - - /* Test IPv4 for starter. */ - { - uint32_t ipv4; - - spec.type = LS_IPV4; - ret = tor_addr_parse(&spec.u.ap.addr, "1.2.3.4"); - tt_int_op(ret, OP_EQ, AF_INET); - b64 = encode_link_specifiers(link_specifiers); - tt_assert(b64); - - /* Decode it and validate the format. */ - ret = base64_decode(buf, sizeof(buf), b64, strlen(b64)); - tt_int_op(ret, OP_GT, 0); - /* First byte is the number of link specifier. */ - tt_int_op(get_uint8(buf), OP_EQ, 1); - ret = link_specifier_parse(&ls, (uint8_t *) buf + 1, ret - 1); - tt_int_op(ret, OP_EQ, 8); - /* Should be 2 bytes for port and 4 bytes for IPv4. */ - tt_int_op(link_specifier_get_ls_len(ls), OP_EQ, 6); - ipv4 = link_specifier_get_un_ipv4_addr(ls); - tt_int_op(tor_addr_to_ipv4h(&spec.u.ap.addr), OP_EQ, ipv4); - tt_int_op(link_specifier_get_un_ipv4_port(ls), OP_EQ, spec.u.ap.port); - - link_specifier_free(ls); - ls = NULL; - tor_free(b64); - } - - /* Test IPv6. */ - { - uint8_t ipv6[16]; - - spec.type = LS_IPV6; - ret = tor_addr_parse(&spec.u.ap.addr, "[1:2:3:4::]"); - tt_int_op(ret, OP_EQ, AF_INET6); - b64 = encode_link_specifiers(link_specifiers); - tt_assert(b64); - - /* Decode it and validate the format. */ - ret = base64_decode(buf, sizeof(buf), b64, strlen(b64)); - tt_int_op(ret, OP_GT, 0); - /* First byte is the number of link specifier. */ - tt_int_op(get_uint8(buf), OP_EQ, 1); - ret = link_specifier_parse(&ls, (uint8_t *) buf + 1, ret - 1); - tt_int_op(ret, OP_EQ, 20); - /* Should be 2 bytes for port and 16 bytes for IPv6. */ - tt_int_op(link_specifier_get_ls_len(ls), OP_EQ, 18); - for (unsigned int i = 0; i < sizeof(ipv6); i++) { - ipv6[i] = link_specifier_get_un_ipv6_addr(ls, i); - } - tt_mem_op(tor_addr_to_in6_addr8(&spec.u.ap.addr), OP_EQ, ipv6, - sizeof(ipv6)); - tt_int_op(link_specifier_get_un_ipv6_port(ls), OP_EQ, spec.u.ap.port); - - link_specifier_free(ls); - ls = NULL; - tor_free(b64); - } - - /* Test legacy. */ - { - uint8_t *id; - - spec.type = LS_LEGACY_ID; - memset(spec.u.legacy_id, 'Y', sizeof(spec.u.legacy_id)); - b64 = encode_link_specifiers(link_specifiers); - tt_assert(b64); - - /* Decode it and validate the format. */ - ret = base64_decode(buf, sizeof(buf), b64, strlen(b64)); - tt_int_op(ret, OP_GT, 0); - /* First byte is the number of link specifier. */ - tt_int_op(get_uint8(buf), OP_EQ, 1); - ret = link_specifier_parse(&ls, (uint8_t *) buf + 1, ret - 1); - /* 20 bytes digest + 1 byte type + 1 byte len. */ - tt_int_op(ret, OP_EQ, 22); - tt_int_op(link_specifier_getlen_un_legacy_id(ls), OP_EQ, DIGEST_LEN); - /* Digest length is 20 bytes. */ - tt_int_op(link_specifier_get_ls_len(ls), OP_EQ, DIGEST_LEN); - id = link_specifier_getarray_un_legacy_id(ls); - tt_mem_op(spec.u.legacy_id, OP_EQ, id, DIGEST_LEN); - - link_specifier_free(ls); - ls = NULL; - tor_free(b64); - } - - done: - link_specifier_free(ls); - tor_free(b64); - smartlist_free(link_specifiers); -} - -static void test_encode_descriptor(void *arg) { int ret; @@ -932,8 +823,6 @@ struct testcase_t hs_descriptor[] = { /* Encoding tests. */ { "cert_encoding", test_cert_encoding, TT_FORK, NULL, NULL }, - { "link_specifier", test_link_specifier, TT_FORK, - NULL, NULL }, { "encode_descriptor", test_encode_descriptor, TT_FORK, NULL, NULL }, { "descriptor_padding", test_descriptor_padding, TT_FORK, diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 660f21ffd8..b7163c5c13 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -50,7 +50,7 @@ new_establish_intro_cell(const char *circ_nonce, /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, buf); tt_i64_op(cell_len, OP_GT, 0); @@ -76,7 +76,7 @@ new_establish_intro_encoded_cell(const char *circ_nonce, uint8_t *cell_out) /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, cell_out); tt_i64_op(cell_len, OP_GT, 0); diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 43bf894383..57132e6197 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -328,17 +328,18 @@ helper_create_service_with_clients(int num_clients) static hs_service_intro_point_t * helper_create_service_ip(void) { - hs_desc_link_specifier_t *ls; - hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0); + link_specifier_t *ls; + hs_service_intro_point_t *ip = service_intro_point_new(NULL); tor_assert(ip); /* Add a first unused link specifier. */ - ls = tor_malloc_zero(sizeof(*ls)); - ls->type = LS_IPV4; + ls = link_specifier_new(); + link_specifier_set_ls_type(ls, LS_IPV4); smartlist_add(ip->base.link_specifiers, ls); /* Add a second link specifier used by a test. */ - ls = tor_malloc_zero(sizeof(*ls)); - ls->type = LS_LEGACY_ID; - memset(ls->u.legacy_id, 'A', sizeof(ls->u.legacy_id)); + ls = link_specifier_new(); + link_specifier_set_ls_type(ls, LS_LEGACY_ID); + memset(link_specifier_getarray_un_legacy_id(ls), 'A', + link_specifier_getlen_un_legacy_id(ls)); smartlist_add(ip->base.link_specifiers, ls); return ip; @@ -811,10 +812,11 @@ test_helper_functions(void *arg) const node_t *node = get_node_from_intro_point(ip); tt_ptr_op(node, OP_EQ, &mock_node); SMARTLIST_FOREACH_BEGIN(ip->base.link_specifiers, - hs_desc_link_specifier_t *, ls) { - if (ls->type == LS_LEGACY_ID) { + link_specifier_t *, ls) { + if (link_specifier_get_ls_type(ls) == LS_LEGACY_ID) { /* Change legacy id in link specifier which is not the mock node. */ - memset(ls->u.legacy_id, 'B', sizeof(ls->u.legacy_id)); + memset(link_specifier_getarray_un_legacy_id(ls), 'B', + link_specifier_getlen_un_legacy_id(ls)); } } SMARTLIST_FOREACH_END(ls); node = get_node_from_intro_point(ip); diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 68e5498a78..8431f12b04 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -58,7 +58,8 @@ trusteddirserver_get_by_v3_auth_digest_m(const char *digest) } /* Setup a minimal dirauth environment by initializing the SR state and - * making sure the options are set to be an authority directory. */ + * making sure the options are set to be an authority directory. + * You must only call this function once per process. */ static void init_authority_state(void) { @@ -595,7 +596,9 @@ test_encoding(void *arg) /** Setup some SRVs in our SR state. If <b>also_current</b> is set, then set * both current and previous SRVs. - * Helper of test_vote() and test_sr_compute_srv(). */ + * Helper of test_vote() and test_sr_compute_srv(). + * You must call sr_state_free_all() to free the state at the end of each test + * function (on pass or fail). */ static void test_sr_setup_srv(int also_current) { @@ -605,6 +608,8 @@ test_sr_setup_srv(int also_current) "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ", sizeof(srv->value)); + /* sr_state_set_previous_srv() does not free() the old previous srv. */ + state_del_previous_srv(); sr_state_set_previous_srv(srv); if (also_current) { @@ -614,6 +619,8 @@ test_sr_setup_srv(int also_current) "NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN", sizeof(srv->value)); + /* sr_state_set_previous_srv() does not free() the old current srv. */ + state_del_current_srv(); sr_state_set_current_srv(srv); } } @@ -1069,12 +1076,13 @@ test_sr_get_majority_srv_from_votes(void *arg) smartlist_free(votes); } +/* Test utils that don't depend on authority state */ static void -test_utils(void *arg) +test_utils_general(void *arg) { (void) arg; - /* Testing srv_dup(). */ + /* Testing sr_srv_dup(). */ { sr_srv_t *srv = NULL, *dup_srv = NULL; const char *srv_value = @@ -1082,7 +1090,7 @@ test_utils(void *arg) srv = tor_malloc_zero(sizeof(*srv)); srv->num_reveals = 42; memcpy(srv->value, srv_value, sizeof(srv->value)); - dup_srv = srv_dup(srv); + dup_srv = sr_srv_dup(srv); tt_assert(dup_srv); tt_u64_op(dup_srv->num_reveals, OP_EQ, srv->num_reveals); tt_mem_op(dup_srv->value, OP_EQ, srv->value, sizeof(srv->value)); @@ -1133,9 +1141,19 @@ test_utils(void *arg) tt_str_op(get_phase_str(SR_PHASE_COMMIT), OP_EQ, "commit"); } + done: + return; +} + +/* Test utils that depend on authority state */ +static void +test_utils_auth(void *arg) +{ + (void)arg; + init_authority_state(); + /* Testing phase transition */ { - init_authority_state(); set_sr_phase(SR_PHASE_COMMIT); tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 1); tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 0); @@ -1146,8 +1164,193 @@ test_utils(void *arg) tt_int_op(is_phase_transition(42), OP_EQ, 1); } + /* Testing get, set, delete, clean SRVs */ + + { + /* Just set the previous SRV */ + test_sr_setup_srv(0); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + } + + { + /* Delete the SRVs one at a time */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And in the opposite order */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And both at once */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And do the gets and sets multiple times */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + } + + { + /* Now set the SRVs to NULL instead */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_current_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + sr_state_set_previous_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And in the opposite order */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_previous_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_current_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And both at once */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And do the gets and sets multiple times */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_previous_srv(NULL); + sr_state_set_previous_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_current_srv(NULL); + sr_state_set_previous_srv(NULL); + sr_state_set_current_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + } + + { + /* Now copy the values across */ + test_sr_setup_srv(1); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Check that the content is different */ + tt_mem_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Set the current to the previous: the protocol goes the other way */ + sr_state_set_current_srv(sr_srv_dup(sr_state_get_previous_srv())); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Check that the content is the same */ + tt_mem_op(sr_state_get_previous_srv(), OP_EQ, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + } + + { + /* Now copy a value onto itself */ + test_sr_setup_srv(1); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Take a copy of the old value */ + sr_srv_t old_current_srv; + memcpy(&old_current_srv, sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Check that the content is different */ + tt_mem_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Set the current to the current: the protocol never replaces an SRV with + * the same value */ + sr_state_set_current_srv(sr_srv_dup(sr_state_get_current_srv())); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Check that the content is different between current and previous */ + tt_mem_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Check that the content is the same as the old content */ + tt_mem_op(&old_current_srv, OP_EQ, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + } + + /* I don't think we can say "expect a BUG()" in our tests. */ +#if 0 + { + /* Now copy a value onto itself without sr_srv_dup(). + * This should fail with a BUG() warning. */ + test_sr_setup_srv(1); + sr_state_set_current_srv(sr_state_get_current_srv()); + sr_state_set_previous_srv(sr_state_get_previous_srv()); + } +#endif + done: - return; + sr_state_free_all(); } static void @@ -1155,6 +1358,7 @@ test_state_transition(void *arg) { sr_state_t *state = NULL; time_t now = time(NULL); + sr_srv_t *cur = NULL; (void) arg; @@ -1193,44 +1397,47 @@ test_state_transition(void *arg) /* Test SRV rotation in our state. */ { - const sr_srv_t *cur, *prev; test_sr_setup_srv(1); - cur = sr_state_get_current_srv(); + tt_assert(sr_state_get_current_srv()); + /* Take a copy of the data, because the state owns the pointer */ + cur = sr_srv_dup(sr_state_get_current_srv()); tt_assert(cur); - /* After, current srv should be the previous and then set to NULL. */ + /* After, the previous SRV should be the same as the old current SRV, and + * the current SRV should be set to NULL */ state_rotate_srv(); - prev = sr_state_get_previous_srv(); - tt_assert(prev == cur); + tt_mem_op(sr_state_get_previous_srv(), OP_EQ, cur, sizeof(sr_srv_t)); tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); sr_state_clean_srvs(); + tor_free(cur); } /* New protocol run. */ { - const sr_srv_t *cur; /* Setup some new SRVs so we can confirm that a new protocol run * actually makes them rotate and compute new ones. */ test_sr_setup_srv(1); - cur = sr_state_get_current_srv(); - tt_assert(cur); + tt_assert(sr_state_get_current_srv()); + /* Take a copy of the data, because the state owns the pointer */ + cur = sr_srv_dup(sr_state_get_current_srv()); set_sr_phase(SR_PHASE_REVEAL); MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m); new_protocol_run(now); UNMOCK(get_my_v3_authority_cert); /* Rotation happened. */ - tt_assert(sr_state_get_previous_srv() == cur); + tt_mem_op(sr_state_get_previous_srv(), OP_EQ, cur, sizeof(sr_srv_t)); /* We are going into COMMIT phase so we had to rotate our SRVs. Usually * our current SRV would be NULL but a new protocol run should make us * compute a new SRV. */ tt_assert(sr_state_get_current_srv()); /* Also, make sure we did change the current. */ - tt_assert(sr_state_get_current_srv() != cur); + tt_mem_op(sr_state_get_current_srv(), OP_NE, cur, sizeof(sr_srv_t)); /* We should have our commitment alone. */ tt_int_op(digestmap_size(state->commits), OP_EQ, 1); tt_int_op(state->n_reveal_rounds, OP_EQ, 0); tt_int_op(state->n_commit_rounds, OP_EQ, 0); /* 46 here since we were at 45 just before. */ tt_u64_op(state->n_protocol_runs, OP_EQ, 46); + tor_free(cur); } /* Cleanup of SRVs. */ @@ -1241,6 +1448,7 @@ test_state_transition(void *arg) } done: + tor_free(cur); sr_state_free_all(); } @@ -1436,7 +1644,8 @@ struct testcase_t sr_tests[] = { { "sr_compute_srv", test_sr_compute_srv, TT_FORK, NULL, NULL }, { "sr_get_majority_srv_from_votes", test_sr_get_majority_srv_from_votes, TT_FORK, NULL, NULL }, - { "utils", test_utils, TT_FORK, NULL, NULL }, + { "utils_general", test_utils_general, TT_FORK, NULL, NULL }, + { "utils_auth", test_utils_auth, TT_FORK, NULL, NULL }, { "state_transition", test_state_transition, TT_FORK, NULL, NULL }, { "state_update", test_state_update, TT_FORK, NULL, NULL }, diff --git a/src/trunnel/ed25519_cert.trunnel b/src/trunnel/ed25519_cert.trunnel index 8d6483d558..e424ce5464 100644 --- a/src/trunnel/ed25519_cert.trunnel +++ b/src/trunnel/ed25519_cert.trunnel @@ -28,12 +28,6 @@ const LS_IPV6 = 0x01; const LS_LEGACY_ID = 0x02; const LS_ED25519_ID = 0x03; -// XXX hs_link_specifier_dup() violates the opaqueness of link_specifier_t by -// taking its sizeof(). If we ever want to turn on TRUNNEL_OPAQUE, or -// if we ever make link_specifier contain other types, we will -// need to refactor that function to do the copy by encoding and decoding the -// object. - // amended from tor.trunnel struct link_specifier { u8 ls_type; |