commit 579a80d4ae54ec03fd9b02c4a125b2943770c85d Author: teor (Tim Wilson-Brown) <teor2345@gmail.com> Date: Thu Jul 7 12:58:47 2016 +1000 Clients avoid choosing nodes that can't do ntor If we know a node's version, and it can't do ntor, consider it not running. If we have a node's descriptor, and it doesn't have a valid ntor key, consider it not running. Refactor these checks so they're consistent between authorities and clients. --- changes/reject-tap | 3 +++ src/or/circuitbuild.c | 7 ++++--- src/or/dirserv.c | 4 +--- src/or/networkstatus.c | 6 ++++-- src/or/nodelist.c | 25 +++++++++++++++++++++++-- src/or/routerlist.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/or/routerlist.h | 3 +++ 7 files changed, 82 insertions(+), 10 deletions(-) diff --git a/changes/reject-tap b/changes/reject-tap index 77ca63b..7580018 100644 --- a/changes/reject-tap +++ b/changes/reject-tap @@ -7,4 +7,7 @@ instead, they check specifically for 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 + mirrors, or as introduction or rendezvous points. Fixes bug 19163; bugfix on 0.2.4.18-rc. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 13cc166..d6c5c85 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -820,8 +820,8 @@ 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</b> to do so, we should use an EXTEND2 or an - * EXTEND cell to do so, and set *<b>cell_type_out</b> and + * 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. */ static void circuit_pick_extend_handshake(uint8_t *cell_type_out, @@ -837,7 +837,8 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out, if (node_prev && *handshake_type_out != ONION_HANDSHAKE_TYPE_TAP && (node_has_curve25519_onion_key(node_prev) || - (node_prev->rs && node_prev->rs->version_supports_extend2_cells))) { + (node_prev->rs && + routerstatus_version_supports_ntor(node_prev->rs, 0)))) { *cell_type_out = RELAY_COMMAND_EXTEND2; *create_cell_type_out = CELL_CREATE2; } else { diff --git a/src/or/dirserv.c b/src/or/dirserv.c index ef3a305..ff50ca4 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -260,9 +260,7 @@ dirserv_router_get_status(const routerinfo_t *router, const char **msg, * But just in case a relay doesn't provide or lies about its version, or * doesn't include an ntor key in its descriptor, check that it exists, * and is non-zero (clients check that it's non-zero before using it). */ - if (router->onion_curve25519_pkey == NULL || - tor_mem_is_zero((const char*)router->onion_curve25519_pkey->public_key, - CURVE25519_PUBKEY_LEN)) { + if (!routerinfo_has_curve25519_onion_key(router)) { log_fn(severity, LD_DIR, "Descriptor from router %s is missing an ntor curve25519 onion " "key.", router_describe(router)); diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 61753e5..1223788 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -2272,8 +2272,10 @@ client_would_use_router(const routerstatus_t *rs, time_t now, /* We'd drop it immediately for being too old. */ return 0; } - if (rs->version_known && !rs->version_supports_extend2_cells) { - /* We'd ignore it because it doesn't support ntor. */ + if (!routerstatus_version_supports_ntor(rs, 1)) { + /* We'd ignore it because it doesn't support ntor. + * If we don't know the version, download the descriptor so we can + * check if it supports ntor. */ return 0; } return 1; diff --git a/src/or/nodelist.c b/src/or/nodelist.c index a49bf03..a888ebe 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -1171,14 +1171,35 @@ node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out) } } +/** Return true iff <b>md</b> has a curve25519 onion key. + * Use node_has_curve25519_onion_key() instead of calling this directly. */ +static int +microdesc_has_curve25519_onion_key(const microdesc_t *md) +{ + if (!md) { + return 0; + } + + if (!md->onion_curve25519_pkey) { + return 0; + } + + if (tor_mem_is_zero((const char*)md->onion_curve25519_pkey->public_key, + CURVE25519_PUBKEY_LEN)) { + return 0; + } + + return 1; +} + /** Return true iff <b>node</b> has a curve25519 onion key. */ int node_has_curve25519_onion_key(const node_t *node) { if (node->ri) - return node->ri->onion_curve25519_pkey != NULL; + return routerinfo_has_curve25519_onion_key(node->ri); else if (node->md) - return node->md->onion_curve25519_pkey != NULL; + return microdesc_has_curve25519_onion_key(node->md); else return 0; } diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 6ea9d8b..0801503 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2260,6 +2260,11 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, continue; if (node_is_unreliable(node, need_uptime, need_capacity, need_guard)) continue; + /* Don't choose nodes if we are certain they can't do ntor */ + if (node->rs && !routerstatus_version_supports_ntor(node->rs, 1)) + continue; + if ((node->ri || node->md) && !node_has_curve25519_onion_key(node)) + continue; /* Choose a node with an OR address that matches the firewall rules */ if (check_reach && !fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, @@ -5488,6 +5493,45 @@ routerinfo_incompatible_with_extrainfo(const crypto_pk_t *identity_pkey, return r; } +/* Does ri have a valid ntor onion key? + * Valid ntor onion keys exist and have at least one non-zero byte. */ +int +routerinfo_has_curve25519_onion_key(const routerinfo_t *ri) +{ + if (!ri) { + return 0; + } + + if (!ri->onion_curve25519_pkey) { + return 0; + } + + if (tor_mem_is_zero((const char*)ri->onion_curve25519_pkey->public_key, + CURVE25519_PUBKEY_LEN)) { + return 0; + } + + return 1; +} + +/* Is rs running a tor version known to support ntor? + * If allow_unknown_versions is true, return true if the version is unknown. + * Otherwise, return false if the version is unknown. */ +int +routerstatus_version_supports_ntor(const routerstatus_t *rs, + int allow_unknown_versions) +{ + if (!rs) { + return allow_unknown_versions; + } + + if (!rs->version_known) { + return allow_unknown_versions; + } + + return rs->version_supports_extend2_cells; +} + /** Assert that the internal representation of <b>rl</b> is * self-consistent. */ void diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 01f7644..4041a38 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -205,6 +205,9 @@ int routerinfo_incompatible_with_extrainfo(const crypto_pk_t *ri, extrainfo_t *ei, signed_descriptor_t *sd, const char **msg); +int routerinfo_has_curve25519_onion_key(const routerinfo_t *ri); +int routerstatus_version_supports_ntor(const routerstatus_t *rs, + int allow_unknown_versions); void routerlist_assert_ok(const routerlist_t *rl); const char *esc_router_info(const routerinfo_t *router);