commit 10aa913accaf81d72dba6f1bcd9dcc128d9d8703 Author: teor (Tim Wilson-Brown) teor2345@gmail.com Date: Fri Jul 8 14:46:00 2016 +1000
Client & HS ignore UseNTorHandshake, all non-HS handshakes use ntor
Rely on onion_populate_cpath to check that we're only using TAP for the rare hidden service cases.
Check and log if handshakes only support TAP when they should support ntor. --- changes/reject-tap | 18 ++++---- doc/tor.1.txt | 10 ----- src/or/circuitbuild.c | 121 +++++++++++++++++++++++++++++++++++++------------- src/or/circuitbuild.h | 4 ++ src/or/config.c | 2 +- src/or/nodelist.c | 3 ++ src/or/or.h | 3 -- src/or/rendclient.c | 7 ++- 8 files changed, 114 insertions(+), 54 deletions(-)
diff --git a/changes/reject-tap b/changes/reject-tap index 7580018..8e616de 100644 --- a/changes/reject-tap +++ b/changes/reject-tap @@ -1,13 +1,15 @@ o Major bug fixes (circuit building): - - Tor authorities, relays, and clients no longer support - circuit-building using TAP. (The hidden service protocol - still uses TAP.) - - Relays make sure their own descriptor has an ntor key. - - Authorites no longer trust the version a relay claims (if any), - instead, they check specifically for an ntor key. + - Tor authorities, relays, and clients only use ntor, except for + rare cases in the hidden service protocol. + - Authorities, relays and clients specifically check that each + descriptor has an ntor key. - Clients avoid downloading a descriptor if the relay version is too old to support ntor. - - Client code ignores nodes without ntor keys: they will not be - selected during circuit-building, or as guards, or as directory + - Client code never chooses nodes without ntor keys: they will not + be selected during circuit-building, or as guards, or as directory mirrors, or as introduction or rendezvous points. + - Circuit-building code assumes that all hops can use ntor, + except for rare hidden service protocol cases. + - Hidden service client to intro point and service to rendezvous point + connections use the TAP key supplied by the protocol. Fixes bug 19163; bugfix on 0.2.4.18-rc. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index b5d6e87..fc021f6 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1468,16 +1468,6 @@ The following options are useful only for clients (that is, if "auto" (recommended) then it is on for all clients that do not set FetchUselessDescriptors. (Default: auto)
-[[UseNTorHandshake]] **UseNTorHandshake** **0**|**1**|**auto**:: - The "ntor" circuit-creation handshake is faster and (we think) more - secure than the original ("TAP") circuit handshake, but starting to use - it too early might make your client stand out. If this option is 0, your - Tor client won't use the ntor handshake. If it's 1, your Tor client - will use the ntor handshake to extend circuits through servers that - support it. If this option is "auto", then your client - will use the ntor handshake once enough directory authorities recommend - it. (Default: 1) - [[PathBiasCircThreshold]] **PathBiasCircThreshold** __NUM__ +
[[PathBiasNoticeRate]] **PathBiasNoticeRate** __NUM__ + diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index c309a8d..70d1a10 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -58,7 +58,6 @@ static crypt_path_t *onion_next_hop_in_cpath(crypt_path_t *cpath); static int onion_extend_cpath(origin_circuit_t *circ); static int count_acceptable_nodes(smartlist_t *routers); static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); -static int circuits_can_use_ntor(void);
/** This function tries to get a channel to the specified endpoint, * and then calls command_setup_channel() to give it the right @@ -780,10 +779,13 @@ should_use_create_fast_for_circuit(origin_circuit_t *circ) tor_assert(circ->cpath); tor_assert(circ->cpath->extend_info);
- if (!circ->cpath->extend_info->onion_key) - return 1; /* our hand is forced: only a create_fast will work. */ + if (!circuit_has_usable_onion_key(circ)) { + /* We don't have ntor, and we don't have or can't use TAP, + * so our hand is forced: only a create_fast will work. */ + return 1; + } if (public_server_mode(options)) { - /* We're a server, and we know an onion key. We can choose. + /* We're a server, and we have a usable onion key. We can choose. * Prefer to blend our circuit into the other circuits we are * creating on behalf of others. */ return 0; @@ -808,28 +810,20 @@ circuit_timeout_want_to_count_circ(origin_circuit_t *circ) && circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN; }
-/** Return true if the ntor handshake is enabled in the configuration, or if - * it's been set to "auto" in the configuration and it's enabled in the - * consensus. */ -static int -circuits_can_use_ntor(void) -{ - const or_options_t *options = get_options(); - if (options->UseNTorHandshake != -1) - return options->UseNTorHandshake; - return networkstatus_get_param(NULL, "UseNTorHandshake", 0, 0, 1); -} - /** Decide whether to use a TAP or ntor handshake for connecting to <b>ei</b> * directly, and set *<b>cell_type_out</b> and *<b>handshake_type_out</b> - * accordingly. */ + * accordingly. + * Note that TAP handshakes are only used for direct connections: + * - from Tor2web to intro points not in the client's consensus, and + * - from Single Onions to rend points not in the service's consensus. + * This is checked in onion_populate_cpath. */ static void circuit_pick_create_handshake(uint8_t *cell_type_out, uint16_t *handshake_type_out, const extend_info_t *ei) { - /* XXXX029 Remove support for deciding to use TAP. */ - if (extend_info_supports_ntor(ei) && circuits_can_use_ntor()) { + /* XXXX030 Remove support for deciding to use TAP. */ + if (extend_info_supports_ntor(ei)) { *cell_type_out = CELL_CREATE2; *handshake_type_out = ONION_HANDSHAKE_TYPE_NTOR; return; @@ -841,9 +835,13 @@ circuit_pick_create_handshake(uint8_t *cell_type_out,
/** Decide whether to use a TAP or ntor handshake for connecting to <b>ei</b> * directly, and set *<b>handshake_type_out</b> accordingly. Decide whether, - * in extending through <b>node_prev</b> to do so, we should use an EXTEND2 or - * an EXTEND cell to do so, and set *<b>cell_type_out</b> and - * *<b>create_cell_type_out</b> accordingly. */ + * in extending through <b>node</b> to do so, we should use an EXTEND2 or an + * EXTEND cell to do so, and set *<b>cell_type_out</b> and + * *<b>create_cell_type_out</b> accordingly. + * Note that TAP handshakes are only used for extend handshakes: + * - from clients to intro points, and + * - from hidden services to rend points. + * This is checked in onion_populate_cpath. */ static void circuit_pick_extend_handshake(uint8_t *cell_type_out, uint8_t *create_cell_type_out, @@ -854,18 +852,27 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out, uint8_t t; circuit_pick_create_handshake(&t, handshake_type_out, ei);
- /* XXXX029 Remove support for deciding to use TAP. */ + /* XXXX030 Remove support for deciding to use TAP. */ + + /* It is an error to extend if there is no previous node. */ + tor_assert_nonfatal(node_prev); + /* It is an error for a node with a known version to be so old it does not + * support ntor. */ + tor_assert_nonfatal(routerstatus_version_supports_ntor(node_prev->rs, 1)); + + /* Assume relays without tor versions or routerstatuses support ntor. + * The authorities enforce ntor support, and assuming and failing is better + * than allowing a malicious node to perform a protocol downgrade to TAP. */ if (node_prev && *handshake_type_out != ONION_HANDSHAKE_TYPE_TAP && (node_has_curve25519_onion_key(node_prev) || - (node_prev->rs && - routerstatus_version_supports_ntor(node_prev->rs, 0)))) { - *cell_type_out = RELAY_COMMAND_EXTEND2; - *create_cell_type_out = CELL_CREATE2; - } else { - *cell_type_out = RELAY_COMMAND_EXTEND; - *create_cell_type_out = CELL_CREATE; - } + (routerstatus_version_supports_ntor(node_prev->rs, 1)))) { + *cell_type_out = RELAY_COMMAND_EXTEND2; + *create_cell_type_out = CELL_CREATE2; + } else { + *cell_type_out = RELAY_COMMAND_EXTEND; + *create_cell_type_out = CELL_CREATE; + } }
/** This is the backbone function for building circuits. @@ -2468,6 +2475,15 @@ extend_info_addr_is_allowed(const tor_addr_t *addr) return 0; }
+/* Does ei have a valid TAP key? */ +int +extend_info_supports_tap(const extend_info_t* ei) +{ + tor_assert(ei); + /* Valid TAP keys are not NULL */ + return ei->onion_key != NULL; +} + /* Does ei have a valid ntor key? */ int extend_info_supports_ntor(const extend_info_t* ei) @@ -2478,3 +2494,46 @@ extend_info_supports_ntor(const extend_info_t* ei) (const char*)ei->curve25519_onion_key.public_key, CURVE25519_PUBKEY_LEN); } + +/* Is circuit purpose allowed to use the deprecated TAP encryption protocol? + * The hidden service protocol still uses TAP for some connections, because + * ntor onion keys aren't included in HS descriptors or INTRODUCE cells. */ +static int +circuit_purpose_can_use_tap_impl(uint8_t purpose) +{ + return (purpose == CIRCUIT_PURPOSE_S_CONNECT_REND || + purpose == CIRCUIT_PURPOSE_C_INTRODUCING); +} + +/* Is circ allowed to use the deprecated TAP encryption protocol? + * The hidden service protocol still uses TAP for some connections, because + * ntor onion keys aren't included in HS descriptors or INTRODUCE cells. */ +int +circuit_can_use_tap(const origin_circuit_t *circ) +{ + tor_assert(circ); + tor_assert(circ->cpath); + tor_assert(circ->cpath->extend_info); + return (circuit_purpose_can_use_tap_impl(circ->base_.purpose) && + extend_info_supports_tap(circ->cpath->extend_info)); +} + +/* Does circ have an onion key which it's allowed to use? */ +int +circuit_has_usable_onion_key(const origin_circuit_t *circ) +{ + tor_assert(circ); + tor_assert(circ->cpath); + tor_assert(circ->cpath->extend_info); + return (extend_info_supports_ntor(circ->cpath->extend_info) || + circuit_can_use_tap(circ)); +} + +/* Does ei have an onion key which it would prefer to use? + * Currently, we prefer ntor keys*/ +int +extend_info_has_preferred_onion_key(const extend_info_t* ei) +{ + tor_assert(ei); + return extend_info_supports_ntor(ei); +} diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 73630b4..1244601 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -54,7 +54,11 @@ extend_info_t *extend_info_from_node(const node_t *r, int for_direct_connect); extend_info_t *extend_info_dup(extend_info_t *info); void extend_info_free(extend_info_t *info); int extend_info_addr_is_allowed(const tor_addr_t *addr); +int extend_info_supports_tap(const extend_info_t* ei); int extend_info_supports_ntor(const extend_info_t* ei); +int circuit_can_use_tap(const origin_circuit_t *circ); +int circuit_has_usable_onion_key(const origin_circuit_t *circ); +int extend_info_has_preferred_onion_key(const extend_info_t* ei); const node_t *build_state_get_exit_node(cpath_build_state_t *state); const char *build_state_get_exit_nickname(cpath_build_state_t *state);
diff --git a/src/or/config.c b/src/or/config.c index 55be06c..7478e60 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -438,7 +438,7 @@ static config_var_t option_vars_[] = { V(UseEntryGuardsAsDirGuards, BOOL, "1"), V(UseGuardFraction, AUTOBOOL, "auto"), V(UseMicrodescriptors, AUTOBOOL, "auto"), - V(UseNTorHandshake, AUTOBOOL, "1"), + OBSOLETE("UseNTorHandshake"), V(User, STRING, NULL), V(UserspaceIOCPBuffers, BOOL, "0"), V(AuthDirSharedRandomness, BOOL, "1"), diff --git a/src/or/nodelist.c b/src/or/nodelist.c index a888ebe..391b682 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -1196,6 +1196,9 @@ microdesc_has_curve25519_onion_key(const microdesc_t *md) int node_has_curve25519_onion_key(const node_t *node) { + if (!node) + return 0; + if (node->ri) return routerinfo_has_curve25519_onion_key(node->ri); else if (node->md) diff --git a/src/or/or.h b/src/or/or.h index af40cf7..e0a30e7 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4456,9 +4456,6 @@ typedef struct {
char *TLSECGroup; /**< One of "P256", "P224", or nil for auto */
- /** Autobool: should we use the ntor handshake if we can? */ - int UseNTorHandshake; - /** Fraction: */ double PathsNeededToBuildCircuits;
diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 64d3673..96f4edd 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -1351,8 +1351,13 @@ rend_client_get_random_intro_impl(const rend_cache_entry_t *entry,
i = crypto_rand_int(smartlist_len(usable_nodes)); intro = smartlist_get(usable_nodes, i); + if (BUG(!intro->extend_info)) { + /* This should never happen, but it isn't fatal, just try another */ + smartlist_del(usable_nodes, i); + goto again; + } /* Do we need to look up the router or is the extend info complete? */ - if (!intro->extend_info->onion_key) { + if (!extend_info_supports_tap(intro->extend_info)) { const node_t *node; extend_info_t *new_extend_info; if (tor_digest_is_zero(intro->extend_info->identity_digest))