[tor-commits] [tor/master] Client & HS ignore UseNTorHandshake, all non-HS handshakes use ntor
nickm at torproject.org
nickm at torproject.org
Mon Aug 29 19:11:31 UTC 2016
commit 10aa913accaf81d72dba6f1bcd9dcc128d9d8703
Author: teor (Tim Wilson-Brown) <teor2345 at 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))
More information about the tor-commits
mailing list