commit 7daf15217240acefaf2ef802b6d89e04f4e51cae Author: Nick Mathewson <nickm@torproject.org> Date: Thu Nov 10 12:24:07 2016 -0500 Enforce Ed25519 identities (client-side) This patch makes two absolutely critical changes: - If an ed25519 identity is not as expected when creating a channel, we call that channel unsuccessful and close it. - When a client creating a channel or an extend cell for a circuit, we only include the ed25519 identity if we believe that the node on the other side supports ed25519 link authentication (from #15055). Otherwise we will insist on nodes without the right link protocol authenticating themselves. - When deciding to extend to another relay, we only upgrade the extend to extend by ed25519 ID when we know the ed25519 ID _and_ we know that the other side can authenticate. This patch also tells directory servers, when probing nodes, to try to check their ed25519 identities too (if they can authenticate by ed25519 identity). Also, handle the case where we connect by RSA Id, and learn the ED25519 ID for the node in doing so. --- src/or/circuitbuild.c | 17 +++++++++++-- src/or/connection_or.c | 67 +++++++++++++++++++++++++++++++++++++++++--------- src/or/dirserv.c | 13 +++++++--- src/or/nodelist.c | 25 +++++++++++++++++++ src/or/nodelist.h | 1 + 5 files changed, 106 insertions(+), 17 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index bdbbacd..bac68bb 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1180,7 +1180,9 @@ circuit_extend(cell_t *cell, circuit_t *circ) if (ed25519_public_key_is_zero(&ec.ed_pubkey)) { const node_t *node = node_get_by_id((const char*)ec.node_id); const ed25519_public_key_t *node_ed_id = NULL; - if (node && (node_ed_id = node_get_ed25519_id(node))) { + if (node && + node_supports_ed25519_link_authentication(node) && + (node_ed_id = node_get_ed25519_id(node))) { memcpy(ec.ed_pubkey.pubkey, node_ed_id->pubkey, ED25519_PUBKEY_LEN); } } @@ -2450,7 +2452,18 @@ extend_info_from_node(const node_t *node, int for_direct_connect) return NULL; } - const ed25519_public_key_t *ed_pubkey = node_get_ed25519_id(node); + const ed25519_public_key_t *ed_pubkey = NULL; + + /* Don't send the ed25519 pubkey unless the target node actually supports + * authenticating with it. */ + if (node_supports_ed25519_link_authentication(node)) { + log_info(LD_CIRC, "Including Ed25519 ID for %s", node_describe(node)); + ed_pubkey = node_get_ed25519_id(node); + } else if (node_get_ed25519_id(node)) { + log_info(LD_CIRC, "Not including the ed25519 ID for %s, since it won't " + " be able to authenticate it.", + node_describe(node)); + } if (valid_addr && node->ri) return extend_info_new(node->ri->nickname, diff --git a/src/or/connection_or.c b/src/or/connection_or.c index a22c4ad..0caf8a9 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1547,8 +1547,24 @@ connection_or_client_learned_peer_id(or_connection_t *conn, const ed25519_public_key_t *ed_peer_id) { const or_options_t *options = get_options(); - - if (tor_digest_is_zero(conn->identity_digest)) { + channel_tls_t *chan_tls = conn->chan; + channel_t *chan = channel_tls_to_base(chan_tls); + tor_assert(chan); + + const int expected_rsa_key = + ! tor_digest_is_zero(conn->identity_digest); + const int expected_ed_key = + ! ed25519_public_key_is_zero(&chan->ed25519_identity); + + log_info(LD_HANDSHAKE, "learned peer id for %p (%s): %s, %s", + conn, + safe_str_client(conn->base_.address), + hex_str((const char*)rsa_peer_id, DIGEST_LEN), + ed25519_fmt(ed_peer_id)); + + if (! expected_rsa_key && ! expected_ed_key) { + log_info(LD_HANDSHAKE, "(we had no ID in mind when we made this " + "connection."); connection_or_set_identity_digest(conn, (const char*)rsa_peer_id, ed_peer_id); tor_free(conn->nickname); @@ -1565,13 +1581,35 @@ connection_or_client_learned_peer_id(or_connection_t *conn, (const char*)rsa_peer_id, ed_peer_id); } - if (tor_memneq(rsa_peer_id, conn->identity_digest, DIGEST_LEN)) { + const int rsa_mismatch = expected_rsa_key && + tor_memneq(rsa_peer_id, conn->identity_digest, DIGEST_LEN); + /* It only counts as an ed25519 mismatch if we wanted an ed25519 identity + * and didn't get it. It's okay if we get one that we didn't ask for. */ + const int ed25519_mismatch = + expected_ed_key && + (ed_peer_id == NULL || + ! ed25519_pubkey_eq(&chan->ed25519_identity, ed_peer_id)); + + if (rsa_mismatch || ed25519_mismatch) { /* I was aiming for a particular digest. I didn't get it! */ - char seen[HEX_DIGEST_LEN+1]; - char expected[HEX_DIGEST_LEN+1]; - base16_encode(seen, sizeof(seen), (const char*)rsa_peer_id, DIGEST_LEN); - base16_encode(expected, sizeof(expected), conn->identity_digest, + char seen_rsa[HEX_DIGEST_LEN+1]; + char expected_rsa[HEX_DIGEST_LEN+1]; + char seen_ed[ED25519_BASE64_LEN+1]; + char expected_ed[ED25519_BASE64_LEN+1]; + base16_encode(seen_rsa, sizeof(seen_rsa), + (const char*)rsa_peer_id, DIGEST_LEN); + base16_encode(expected_rsa, sizeof(expected_rsa), conn->identity_digest, DIGEST_LEN); + if (ed_peer_id) { + ed25519_public_to_base64(seen_ed, ed_peer_id); + } else { + strlcpy(seen_ed, "no ed25519 key", sizeof(seen_ed)); + } + if (! ed25519_public_key_is_zero(&chan->ed25519_identity)) { + ed25519_public_to_base64(expected_ed, &chan->ed25519_identity); + } else { + strlcpy(expected_ed, "no ed25519 key", sizeof(expected_ed)); + } const int using_hardcoded_fingerprints = !networkstatus_get_reasonably_live_consensus(time(NULL), usable_consensus_flavor()); @@ -1606,9 +1644,11 @@ connection_or_client_learned_peer_id(or_connection_t *conn, } log_fn(severity, LD_HANDSHAKE, - "Tried connecting to router at %s:%d, but identity key was not " - "as expected: wanted %s but got %s.%s", - conn->base_.address, conn->base_.port, expected, seen, extra_log); + "Tried connecting to router at %s:%d, but RSA identity key was not " + "as expected: wanted %s + %s but got %s + %s.%s", + conn->base_.address, conn->base_.port, + expected_rsa, expected_ed, seen_rsa, seen_ed, extra_log); + entry_guard_register_connect_status(conn->identity_digest, 0, 1, time(NULL)); control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, @@ -1621,7 +1661,12 @@ connection_or_client_learned_peer_id(or_connection_t *conn, return -1; } - /* XXXX 15056 -- use the Ed25519 key */ + if (!expected_ed_key && ed_peer_id) { + log_info(LD_HANDSHAKE, "(we had no Ed25519 ID in mind when we made this " + "connection."); + connection_or_set_identity_digest(conn, + (const char*)rsa_peer_id, ed_peer_id); + } if (authdir_mode_tests_reachability(options)) { dirserv_orconn_tls_done(&conn->base_.addr, conn->base_.port, diff --git a/src/or/dirserv.c b/src/or/dirserv.c index b141a5d..d060b29 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3259,20 +3259,26 @@ dirserv_single_reachability_test(time_t now, routerinfo_t *router) channel_t *chan = NULL; node_t *node = NULL; tor_addr_t router_addr; + const ed25519_public_key_t *ed_id_key; (void) now; tor_assert(router); node = node_get_mutable_by_id(router->cache_info.identity_digest); tor_assert(node); + if (node_supports_ed25519_link_authentication(node)) { + ed_id_key = &router->cache_info.signing_key_cert->signing_key; + } else { + ed_id_key = NULL; + } + /* IPv4. */ log_debug(LD_OR,"Testing reachability of %s at %s:%u.", router->nickname, fmt_addr32(router->addr), router->or_port); tor_addr_from_ipv4h(&router_addr, router->addr); chan = channel_tls_connect(&router_addr, router->or_port, router->cache_info.identity_digest, - NULL // XXXX Ed25519 ID. - ); + ed_id_key); if (chan) command_setup_channel(chan); /* Possible IPv6. */ @@ -3285,8 +3291,7 @@ dirserv_single_reachability_test(time_t now, routerinfo_t *router) router->ipv6_orport); chan = channel_tls_connect(&router->ipv6_addr, router->ipv6_orport, router->cache_info.identity_digest, - NULL // XXXX Ed25519 ID. - ); + ed_id_key); if (chan) command_setup_channel(chan); } } diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 9486224..1f993e4 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -49,6 +49,7 @@ #include "networkstatus.h" #include "nodelist.h" #include "policies.h" +#include "protover.h" #include "rendservice.h" #include "router.h" #include "routerlist.h" @@ -670,6 +671,30 @@ node_get_ed25519_id(const node_t *node) return NULL; } + +/** Return true iff <b>node</b> supports authenticating itself + * by ed25519 ID during the link handshake in a way that we can understand + * when we probe it. */ +int +node_supports_ed25519_link_authentication(const node_t *node) +{ + /* XXXX Oh hm. What if some day in the future there are link handshake + * versions that aren't 3 but which are ed25519 */ + if (! node_get_ed25519_id(node)) + return 0; + if (node->ri) { + const char *protos = node->ri->protocol_list; + if (protos == NULL) + return 0; + return protocol_list_supports_protocol(protos, PRT_LINKAUTH, 3); + } + if (node->rs) { + return node->rs->supports_ed25519_link_handshake; + } + tor_assert_nonfatal_unreached_once(); + return 0; +} + /** Return the RSA ID key's SHA1 digest for the provided node. */ const uint8_t * node_get_rsa_id_digest(const node_t *node) diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 2cdcdce..57c3b43 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -56,6 +56,7 @@ long node_get_declared_uptime(const node_t *node); time_t node_get_published_on(const node_t *node); const smartlist_t *node_get_declared_family(const node_t *node); const ed25519_public_key_t *node_get_ed25519_id(const node_t *node); +int node_supports_ed25519_link_authentication(const node_t *node); const uint8_t *node_get_rsa_id_digest(const node_t *node); int node_has_ipv6_addr(const node_t *node);