December 2016 - tor-commits - lists.torproject.org

[tor/master] When attempting to find a channel by ID, consider Ed ID.
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit ef5158b2d2f67a1d70e6194b0fde291f853d485e Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Sep 18 20:06:48 2016 -0400 When attempting to find a channel by ID, consider Ed ID. Right now, there's only a mechanism to look for a channel where the RSA ID matches *and* the ED ID matches. We can add a separate map later if we want. --- src/or/channel.c | 67 ++++++++++++++++++++++++++++++++++++++++---------- src/or/channel.h | 21 +++++++++++----- src/or/channeltls.c | 1 - src/or/circuitbuild.c | 21 +++++++++------- src/or/connection_or.c | 4 +-- 5 files changed, 83 insertions(+), 31 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index af58107..d484e71 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -733,27 +733,62 @@ channel_find_by_global_id(uint64_t global_identifier) return rv; } +/** Return true iff chan matches rsa_id_digest and ed_id. + * as its identity keys. If either is NULL, do not check for a match. */ +static int +channel_remote_identity_matches(const channel_t *chan, + const char *rsa_id_digest, + const ed25519_public_key_t *ed_id) +{ + if (BUG(!chan)) + return 0; + if (rsa_id_digest) { + if (tor_memneq(rsa_id_digest, chan->identity_digest, DIGEST_LEN)) + return 0; + } + if (ed_id) { + if (tor_memneq(ed_id->pubkey, chan->ed25519_identity.pubkey, + ED25519_PUBKEY_LEN)) + return 0; + } + return 1; +} + /** - * Find channel by digest of the remote endpoint + * Find channel by RSA/Ed25519 identity of of the remote endpoint * - * This function looks up a channel by the digest of its remote endpoint in - * the channel digest map. It's possible that more than one channel to a - * given endpoint exists. Use channel_next_with_digest() to walk the list. + * This function looks up a channel by the digest of its remote endpoint's RSA + * identity key. If ed_id is provided and nonzero, only a channel + * matching the ed_id will be returned. + * + * It's possible that more than one channel to a given endpoint exists. Use + * channel_next_with_rsa_identity() to walk the list of channels; make sure + * to test for Ed25519 identity match too (as appropriate) */ - channel_t * -channel_find_by_remote_digest(const char *identity_digest) +channel_find_by_remote_identity(const char *rsa_id_digest, + const ed25519_public_key_t *ed_id) { channel_t *rv = NULL; channel_idmap_entry_t *ent, search; - tor_assert(identity_digest); + tor_assert(rsa_id_digest); /* For now, we require that every channel have + * an RSA identity, and that every lookup + * contain an RSA identity */ + if (ed_id && ed25519_public_key_is_zero(ed_id)) { + /* Treat zero as meaning "We don't care about the presence or absence of + * an Ed key", not "There must be no Ed key". */ + ed_id = NULL; + } - memcpy(search.digest, identity_digest, DIGEST_LEN); + memcpy(search.digest, rsa_id_digest, DIGEST_LEN); ent = HT_FIND(channel_idmap, &channel_identity_map, &search); if (ent) { rv = TOR_LIST_FIRST(&ent->channel_list); } + while (rv && ! channel_remote_identity_matches(rv, rsa_id_digest, ed_id)) { + rv = channel_next_with_rsa_identity(rv); + } return rv; } @@ -766,7 +801,7 @@ channel_find_by_remote_digest(const char *identity_digest) */ channel_t * -channel_next_with_digest(channel_t *chan) +channel_next_with_rsa_identity(channel_t *chan) { tor_assert(chan); @@ -3296,7 +3331,8 @@ channel_is_better(time_t now, channel_t *a, channel_t *b, */ channel_t * -channel_get_for_extend(const char *digest, +channel_get_for_extend(const char *rsa_id_digest, + const ed25519_public_key_t *ed_id, const tor_addr_t *target_addr, const char **msg_out, int *launch_out) @@ -3309,14 +3345,14 @@ channel_get_for_extend(const char *digest, tor_assert(msg_out); tor_assert(launch_out); - chan = channel_find_by_remote_digest(digest); + chan = channel_find_by_remote_identity(rsa_id_digest, ed_id); /* Walk the list, unrefing the old one and refing the new at each * iteration. */ - for (; chan; chan = channel_next_with_digest(chan)) { + for (; chan; chan = channel_next_with_rsa_identity(chan)) { tor_assert(tor_memeq(chan->identity_digest, - digest, DIGEST_LEN)); + rsa_id_digest, DIGEST_LEN)); if (CHANNEL_CONDEMNED(chan)) continue; @@ -3327,6 +3363,11 @@ channel_get_for_extend(const char *digest, continue; } + /* The Ed25519 key has to match too */ + if (!channel_remote_identity_matches(chan, rsa_id_digest, ed_id)) { + continue; + } + /* Never return a non-open connection. */ if (!CHANNEL_IS_OPEN(chan)) { /* If the address matches, don't launch a new connection for this diff --git a/src/or/channel.h b/src/or/channel.h index 7e7b2ec..2747e52 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -153,10 +153,16 @@ struct channel_s { int (*write_var_cell)(channel_t *, var_cell_t *); /** - * Hash of the public RSA key for the other side's identity key, or - * zeroes if the other side hasn't shown us a valid identity key. + * Hash of the public RSA key for the other side's RSA identity key, or + * zeroes if the other side hasn't shown us a valid RSA identity key. */ char identity_digest[DIGEST_LEN]; + /** + * The Ed25519 public identity key for the other side, or zeros if the other + * size hasn't shown us a valid Ed25519 identity key + */ + ed25519_public_key_t ed25519_identity; + /** Nickname of the OR on the other side, or NULL if none. */ char *nickname; @@ -489,10 +495,11 @@ int channel_send_destroy(circid_t circ_id, channel_t *chan, */ channel_t * channel_connect(const tor_addr_t *addr, uint16_t port, - const char *id_digest, + const char *rsa_id_digest, const ed25519_public_key_t *ed_id); -channel_t * channel_get_for_extend(const char *digest, +channel_t * channel_get_for_extend(const char *rsa_id_digest, + const ed25519_public_key_t *ed_id, const tor_addr_t *target_addr, const char **msg_out, int *launch_out); @@ -506,11 +513,13 @@ int channel_is_better(time_t now, */ channel_t * channel_find_by_global_id(uint64_t global_identifier); -channel_t * channel_find_by_remote_digest(const char *identity_digest); +channel_t * channel_find_by_remote_identity(const char *rsa_id_digest, + const ed25519_public_key_t *ed_id); /** For things returned by channel_find_by_remote_digest(), walk the list. + * The RSA key will match for all returned elements; the Ed25519 key might not. */ -channel_t * channel_next_with_digest(channel_t *chan); +channel_t * channel_next_with_rsa_identity(channel_t *chan); /* * Helper macros to lookup state of given channel. diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 9fb309d..8384576 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -174,7 +174,6 @@ channel_tls_connect(const tor_addr_t *addr, uint16_t port, const char *id_digest, const ed25519_public_key_t *ed_id) { - (void) ed_id; // XXXX not fully used yet channel_tls_t *tlschan = tor_malloc_zero(sizeof(*tlschan)); channel_t *chan = &(tlschan->base_); diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index e578a94..9a3af40 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -63,8 +63,9 @@ #include "transports.h" static channel_t * channel_connect_for_circuit(const tor_addr_t *addr, - uint16_t port, - const char *id_digest); + uint16_t port, + const char *id_digest, + const ed25519_public_key_t *ed_id); static int circuit_deliver_create_cell(circuit_t *circ, const create_cell_t *create_cell, int relayed); @@ -80,13 +81,12 @@ static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); */ static channel_t * channel_connect_for_circuit(const tor_addr_t *addr, uint16_t port, - const char *id_digest) + const char *id_digest, + const ed25519_public_key_t *ed_id) { channel_t *chan; - chan = channel_connect(addr, port, id_digest, - NULL // XXXX Ed25519 id. - ); + chan = channel_connect(addr, port, id_digest, ed_id); if (chan) command_setup_channel(chan); return chan; @@ -556,6 +556,7 @@ circuit_handle_first_hop(origin_circuit_t *circ) firsthop->extend_info->port)); n_chan = channel_get_for_extend(firsthop->extend_info->identity_digest, + &firsthop->extend_info->ed_identity, &firsthop->extend_info->addr, &msg, &should_launch); @@ -573,7 +574,8 @@ circuit_handle_first_hop(origin_circuit_t *circ) n_chan = channel_connect_for_circuit( &firsthop->extend_info->addr, firsthop->extend_info->port, - firsthop->extend_info->identity_digest); + firsthop->extend_info->identity_digest, + &firsthop->extend_info->ed_identity); if (!n_chan) { /* connect failed, forget the whole thing */ log_info(LD_CIRC,"connect to firsthop failed. Closing."); return -END_CIRC_REASON_CONNECTFAILED; @@ -1185,7 +1187,7 @@ circuit_extend(cell_t *cell, circuit_t *circ) } n_chan = channel_get_for_extend((const char*)ec.node_id, - /*&ec.ed25519_id 15056 */ + &ec.ed_pubkey, &ec.orport_ipv4.addr, &msg, &should_launch); @@ -1212,7 +1214,8 @@ circuit_extend(cell_t *cell, circuit_t *circ) /* we should try to open a connection */ n_chan = channel_connect_for_circuit(&ec.orport_ipv4.addr, ec.orport_ipv4.port, - (const char*)ec.node_id); + (const char*)ec.node_id, + &ec.ed_pubkey); if (!n_chan) { log_info(LD_CIRC,"Launching n_chan failed. Closing circuit."); circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index eb67f06..e83dca2 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -149,7 +149,7 @@ connection_or_set_identity_digest(or_connection_t *conn, const char *rsa_digest, const ed25519_public_key_t *ed_id) { - (void) ed_id; // DOCDOC // XXXX not implemented yet. + (void) ed_id; // DOCDOC // XXXX not implemented yet. 15056 or_connection_t *tmp; tor_assert(conn); tor_assert(rsa_digest); @@ -1182,7 +1182,7 @@ connection_or_connect, (const tor_addr_t *_addr, uint16_t port, const ed25519_public_key_t *ed_id, channel_tls_t *chan)) { - (void) ed_id; // XXXX not fully used yet. + (void) ed_id; // XXXX not fully used yet. 15056 or_connection_t *conn; const or_options_t *options = get_options(); int socket_error = 0;

1 0

[tor/master] Unit tests for channel identity map code
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 921ac5c548789af348b2a0cadfc738b7fbfbf84a Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 11 13:09:25 2016 -0500 Unit tests for channel identity map code --- src/test/test_channel.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/src/test/test_channel.c b/src/test/test_channel.c index e87f99e..f158b1a 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -1768,6 +1768,111 @@ test_channel_write(void *arg) return; } +static void +test_channel_id_map(void *arg) +{ + (void)arg; + const int N_CHAN = 6; + char rsa_id[N_CHAN][DIGEST_LEN]; + ed25519_public_key_t *ed_id[N_CHAN]; + channel_t *chan[N_CHAN]; + int i; + ed25519_public_key_t ed_zero; + memset(&ed_zero, 0, sizeof(ed_zero)); + + tt_assert(sizeof(rsa_id[0]) == DIGEST_LEN); // Do I remember C? + + for (i = 0; i < N_CHAN; ++i) { + crypto_rand(rsa_id[i], DIGEST_LEN); + ed_id[i] = tor_malloc_zero(sizeof(*ed_id[i])); + crypto_rand((char*)ed_id[i]->pubkey, sizeof(ed_id[i]->pubkey)); + } + + /* For channel 3, have no Ed identity. */ + tor_free(ed_id[3]); + + /* Channel 2 and 4 have same ROSA identity */ + memcpy(rsa_id[4], rsa_id[2], DIGEST_LEN); + + /* Channel 2 and 4 and 5 have same RSA identity */ + memcpy(rsa_id[4], rsa_id[2], DIGEST_LEN); + memcpy(rsa_id[5], rsa_id[2], DIGEST_LEN); + + /* Channels 2 and 5 have same Ed25519 identity */ + memcpy(ed_id[5], ed_id[2], sizeof(*ed_id[2])); + + for (i = 0; i < N_CHAN; ++i) { + chan[i] = new_fake_channel(); + channel_register(chan[i]); + channel_set_identity_digest(chan[i], rsa_id[i], ed_id[i]); + } + + /* Lookup by RSA id only */ + tt_ptr_op(chan[0], OP_EQ, + channel_find_by_remote_identity(rsa_id[0], NULL)); + tt_ptr_op(chan[1], OP_EQ, + channel_find_by_remote_identity(rsa_id[1], NULL)); + tt_ptr_op(chan[3], OP_EQ, + channel_find_by_remote_identity(rsa_id[3], NULL)); + channel_t *ch; + ch = channel_find_by_remote_identity(rsa_id[2], NULL); + tt_assert(ch == chan[2] || ch == chan[4] || ch == chan[5]); + ch = channel_next_with_rsa_identity(ch); + tt_assert(ch == chan[2] || ch == chan[4] || ch == chan[5]); + ch = channel_next_with_rsa_identity(ch); + tt_assert(ch == chan[2] || ch == chan[4] || ch == chan[5]); + ch = channel_next_with_rsa_identity(ch); + tt_assert(ch == NULL); + + /* As above, but with zero Ed25519 ID (meaning "any ID") */ + tt_ptr_op(chan[0], OP_EQ, + channel_find_by_remote_identity(rsa_id[0], &ed_zero)); + tt_ptr_op(chan[1], OP_EQ, + channel_find_by_remote_identity(rsa_id[1], &ed_zero)); + tt_ptr_op(chan[3], OP_EQ, + channel_find_by_remote_identity(rsa_id[3], &ed_zero)); + ch = channel_find_by_remote_identity(rsa_id[2], &ed_zero); + tt_assert(ch == chan[2] || ch == chan[4] || ch == chan[5]); + ch = channel_next_with_rsa_identity(ch); + tt_assert(ch == chan[2] || ch == chan[4] || ch == chan[5]); + ch = channel_next_with_rsa_identity(ch); + tt_assert(ch == chan[2] || ch == chan[4] || ch == chan[5]); + ch = channel_next_with_rsa_identity(ch); + tt_assert(ch == NULL); + + /* Lookup nonexistent RSA identity */ + tt_ptr_op(NULL, OP_EQ, + channel_find_by_remote_identity("!!!!!!!!!!!!!!!!!!!!", NULL)); + + /* Look up by full identity pair */ + tt_ptr_op(chan[0], OP_EQ, + channel_find_by_remote_identity(rsa_id[0], ed_id[0])); + tt_ptr_op(chan[1], OP_EQ, + channel_find_by_remote_identity(rsa_id[1], ed_id[1])); + tt_ptr_op(chan[3], OP_EQ, + channel_find_by_remote_identity(rsa_id[3], ed_id[3] /*NULL*/)); + tt_ptr_op(chan[4], OP_EQ, + channel_find_by_remote_identity(rsa_id[4], ed_id[4])); + ch = channel_find_by_remote_identity(rsa_id[2], ed_id[2]); + tt_assert(ch == chan[2] || ch == chan[5]); + + /* Look up RSA identity with wrong ed25519 identity */ + tt_ptr_op(NULL, OP_EQ, + channel_find_by_remote_identity(rsa_id[4], ed_id[0])); + tt_ptr_op(NULL, OP_EQ, + channel_find_by_remote_identity(rsa_id[2], ed_id[1])); + tt_ptr_op(NULL, OP_EQ, + channel_find_by_remote_identity(rsa_id[3], ed_id[1])); + + done: + for (i = 0; i < N_CHAN; ++i) { + channel_clear_identity_digest(chan[i]); + channel_unregister(chan[i]); + free_fake_channel(chan[i]); + tor_free(ed_id[i]); + } +} + struct testcase_t channel_tests[] = { { "dumpstats", test_channel_dumpstats, TT_FORK, NULL, NULL }, { "flush", test_channel_flush, TT_FORK, NULL, NULL }, @@ -1780,6 +1885,7 @@ struct testcase_t channel_tests[] = { { "queue_incoming", test_channel_queue_incoming, TT_FORK, NULL, NULL }, { "queue_size", test_channel_queue_size, TT_FORK, NULL, NULL }, { "write", test_channel_write, TT_FORK, NULL, NULL }, + { "id_map", test_channel_id_map, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Use connection_or_clear_identity in connection_or_clear_identity_map.
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 9bf9e34a01a5b0b8d28ff2486c73e243561b4e3e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 7 12:57:51 2016 -0500 Use connection_or_clear_identity in connection_or_clear_identity_map. --- src/or/connection_or.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 088900c..046865d 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -101,8 +101,7 @@ connection_or_clear_identity_map(void) SMARTLIST_FOREACH(conns, connection_t *, conn, { if (conn->type == CONN_TYPE_OR) { - or_connection_t *or_conn = TO_OR_CONN(conn); - memset(or_conn->identity_digest, 0, DIGEST_LEN); + connection_or_clear_identity(TO_OR_CONN(conn)); } }); }

1 0

[tor/master] Changes file for feature 15056 (prop220, extend cell side)
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit a7cae7f8f7075da1856c619d141b54c56bfc5f31 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 11 15:39:29 2016 -0500 Changes file for feature 15056 (prop220, extend cell side) --- changes/feature15056 | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/changes/feature15056 b/changes/feature15056 new file mode 100644 index 0000000..46226f8 --- /dev/null +++ b/changes/feature15056 @@ -0,0 +1,28 @@ + o Major features (ed25519 identity keys): + - Relays now understand requests to extend to other relays + by their Ed25519 identity keys. When an Ed25519 identity key + is included in an EXTEND2 cell, the relay will only extend + the circuit if the other relay can prove ownership of that identity. + Implements part of ticket 15056; part of proposal 220. + - Clients now support including Ed25519 identity keys in the EXTEND2 + cells they generate. By default, this is controlled by a consensus + parameter, currently disabled. You can turn this feature on for + testing by setting ExtendByEd25519ID in your configuration. This might + make your traffic appear different than the traffic generated by other + users, however. + Implements part of ticket 15056; part of proposal 220. + + o Code simplification and refactoring: + - The code to generate and parse EXTEND and EXTEND2 cells has + been replaced with code automatically generated by the "trunnel" + utility. + - Remove data structures that were used to index or_connection objects by + their RSA identity digests. These structures are fully redundant with + the similar structures used in the channel abstraction. + + o Minor features (directory authority): + - Add a new authority-only AuthDirTestEd25519LinkKeys option (on by + default) to control whether authorities should try to probe relays by + their Ed25519 link keys. This option will go away in a few + releases--unless we encounter major trouble in our ed25519 link + protocol rollout, in which case it will serve as a safety option.

1 0

[tor/master] Teach channel_rsa_id_group_set_badness_() about Ed25519
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 3b1e04fe4505ccbd7d93061dccb5673523519c64 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 10 16:38:04 2016 -0500 Teach channel_rsa_id_group_set_badness_() about Ed25519 (Only run the connection_or_group_set_badness_() function on groups of channels that have the same RSA and Ed25519 identities.) There's a possible opportunity here where we might want to set a channel to "bad" if it has no ed25519 identity and some other channel has some. Also there's an opportunity to add a warning if we ever have an Ed mismatch on open connections with the same RSA ID. --- src/or/channel.c | 41 ++++++++++++++++++++++++++++++++++------- src/or/connection_or.c | 6 +++--- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index 0a96f23..7984558 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -4551,18 +4551,45 @@ channel_set_circid_type,(channel_t *chan, static void channel_rsa_id_group_set_badness(struct channel_list_s *lst, int force) { + /*XXXX This function should really be about channels. 15056 */ channel_t *chan; - smartlist_t *or_conns = smartlist_new(); + /* First, get a minimal list of the ed25519 identites */ + smartlist_t *ed_identities = smartlist_new(); TOR_LIST_FOREACH(chan, lst, next_with_same_id) { - channel_tls_t *chantls = BASE_CHAN_TO_TLS(chan); - or_connection_t *orconn = chantls->conn; - if (orconn) - smartlist_add(or_conns, orconn); + uint8_t *id_copy = + tor_memdup(&chan->ed25519_identity.pubkey, DIGEST256_LEN); + smartlist_add(ed_identities, id_copy); } - /*XXXX This function should really be about channels. 15056 */ - connection_or_group_set_badness_(or_conns, force); + smartlist_sort_digests256(ed_identities); + smartlist_uniq_digests256(ed_identities); + + /* Now, for each Ed identity, build a smartlist and find the best entry on + * it. */ + smartlist_t *or_conns = smartlist_new(); + SMARTLIST_FOREACH_BEGIN(ed_identities, const uint8_t *, ed_id) { + TOR_LIST_FOREACH(chan, lst, next_with_same_id) { + channel_tls_t *chantls = BASE_CHAN_TO_TLS(chan); + if (tor_memneq(ed_id, &chan->ed25519_identity.pubkey, DIGEST256_LEN)) + continue; + or_connection_t *orconn = chantls->conn; + if (orconn) { + tor_assert(orconn->chan == chantls); + smartlist_add(or_conns, orconn); + } + } + + connection_or_group_set_badness_(or_conns, force); + smartlist_clear(or_conns); + } SMARTLIST_FOREACH_END(ed_id); + + /* XXXX 15056 we may want to do something special with connections that have + * no set Ed25519 identity! */ + smartlist_free(or_conns); + + SMARTLIST_FOREACH(ed_identities, uint8_t *, ed_id, tor_free(ed_id)); + smartlist_free(ed_identities); } /** Go through all the channels (or if digest is non-NULL, just diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 2889bb7..953e9df 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -966,9 +966,9 @@ connection_or_mark_bad_for_new_circs(or_connection_t *or_conn) void connection_or_group_set_badness_(smartlist_t *group, int force) { - /* XXXX this should be entirely about channels, not OR connections. 15056*/ - /* XXXX Look at Ed25519 ids too! 15056 */ - + /* XXXX this function should be entirely about channels, not OR + * XXXX connections. */ + or_connection_t *best = NULL; int n_old = 0, n_inprogress = 0, n_canonical = 0, n_other = 0; time_t now = time(NULL);

1 0

[tor/master] Canonicity update for ed25519.
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit cd741cc595d705daef3aa9158bc7822fff935846 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 11 12:38:11 2016 -0500 Canonicity update for ed25519. If a node can prove its Ed25519 identity, don't consider connections to it canonical unless they match both identities. Includes link handshake changes needed to avoid crashing with bug warnings, since the tests now reach more parts of the code. Closes ticket 20355 --- src/or/connection_or.c | 41 ++++++++++++++++++++++++++++++++++++++++- src/test/test_link_handshake.c | 4 ++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 953e9df..088900c 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -75,6 +75,9 @@ static void connection_or_mark_bad_for_new_circs(or_connection_t *or_conn); static void connection_or_change_state(or_connection_t *conn, uint8_t state); +static void connection_or_check_canonicity(or_connection_t *conn, + int started_here); + /**************************************************************/ /** Global map between Extended ORPort identifiers and OR @@ -869,13 +872,38 @@ connection_or_init_conn_from_address(or_connection_t *conn, ed25519_fmt(ed_id), started_here); - const node_t *r = node_get_by_id(id_digest); connection_or_set_identity_digest(conn, id_digest, ed_id); connection_or_update_token_buckets_helper(conn, 1, get_options()); conn->base_.port = port; tor_addr_copy(&conn->base_.addr, addr); tor_addr_copy(&conn->real_addr, addr); + + connection_or_check_canonicity(conn, started_here); +} + +/** Check whether the identity of conn matches a known node. If it + * does, check whether the address of conn matches the expected address, and + * update the connection's is_canonical flag, nickname, and address fields as + * appropriate. */ +static void +connection_or_check_canonicity(or_connection_t *conn, int started_here) +{ + const char *id_digest = conn->identity_digest; + const ed25519_public_key_t *ed_id = NULL; + const tor_addr_t *addr = &conn->real_addr; + if (conn->chan) + ed_id = & TLS_CHAN_TO_BASE(conn->chan)->ed25519_identity; + + const node_t *r = node_get_by_id(id_digest); + if (r && + node_supports_ed25519_link_authentication(r) && + ! node_ed25519_id_matches(r, ed_id)) { + /* If this node is capable of proving an ed25519 ID, + * we can't call this a canonical connection unless both IDs match. */ + r = NULL; + } + if (r) { tor_addr_port_t node_ap; node_get_pref_orport(r, &node_ap); @@ -897,10 +925,12 @@ connection_or_init_conn_from_address(or_connection_t *conn, tor_addr_copy(&conn->base_.addr, &node_ap.addr); conn->base_.port = node_ap.port; } + tor_free(conn->nickname); conn->nickname = tor_strdup(node_get_nickname(r)); tor_free(conn->base_.address); conn->base_.address = tor_addr_to_str_dup(&node_ap.addr); } else { + tor_free(conn->nickname); conn->nickname = tor_malloc(HEX_DIGEST_LEN+2); conn->nickname[0] = '$'; base16_encode(conn->nickname+1, HEX_DIGEST_LEN+1, @@ -1589,6 +1619,7 @@ connection_or_client_learned_peer_id(or_connection_t *conn, const or_options_t *options = get_options(); channel_tls_t *chan_tls = conn->chan; channel_t *chan = channel_tls_to_base(chan_tls); + int changed_identity = 0; tor_assert(chan); const int expected_rsa_key = @@ -1619,6 +1650,7 @@ connection_or_client_learned_peer_id(or_connection_t *conn, * we do -- remember it for future attempts. */ learned_router_identity(&conn->base_.addr, conn->base_.port, (const char*)rsa_peer_id, ed_peer_id); + changed_identity = 1; } const int rsa_mismatch = expected_rsa_key && @@ -1706,6 +1738,13 @@ connection_or_client_learned_peer_id(or_connection_t *conn, "connection."); connection_or_set_identity_digest(conn, (const char*)rsa_peer_id, ed_peer_id); + changed_identity = 1; + } + + if (changed_identity) { + /* If we learned an identity for this connection, then we might have + * just discovered it to be canonical. */ + connection_or_check_canonicity(conn, conn->handshake_state->started_here); } if (authdir_mode_tests_reachability(options)) { diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index 9931702..421f3aa 100644 --- a/src/test/test_link_handshake.c +++ b/src/test/test_link_handshake.c @@ -117,6 +117,9 @@ test_link_handshake_certs_ok(void *arg) crypto_pk_t *key1 = NULL, *key2 = NULL; const int with_ed = !strcmp((const char *)arg, "Ed25519"); + tor_addr_from_ipv4h(&c1->base_.addr, 0x7f000001); + tor_addr_from_ipv4h(&c2->base_.addr, 0x7f000001); + scheduler_init(); MOCK(tor_tls_cert_matches_key, mock_tls_cert_matches_key); @@ -354,6 +357,7 @@ recv_certs_setup(const struct testcase_t *test) d->chan = tor_malloc_zero(sizeof(*d->chan)); d->c->chan = d->chan; d->c->base_.address = tor_strdup("HaveAnAddress"); + tor_addr_from_ipv4h(&d->c->base_.addr, 0x801f0127); d->c->base_.state = OR_CONN_STATE_OR_HANDSHAKING_V3; d->chan->conn = d->c; tt_int_op(connection_init_or_handshake_state(d->c, 1), ==, 0);

1 0

[tor/master] Add an ed25519_copy; use it in a couple of places dgoulet suggested.
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 937aef48eeae4b81528de3e04b39047d0f826d9b Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Dec 5 12:55:52 2016 -0500 Add an ed25519_copy; use it in a couple of places dgoulet suggested. --- src/common/crypto_ed25519.c | 12 ++++++++++++ src/common/crypto_ed25519.h | 3 +++ src/or/circuitbuild.c | 5 ++--- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/common/crypto_ed25519.c b/src/common/crypto_ed25519.c index 809ad12..b7c8311 100644 --- a/src/common/crypto_ed25519.c +++ b/src/common/crypto_ed25519.c @@ -628,6 +628,18 @@ ed25519_pubkey_eq(const ed25519_public_key_t *key1, return tor_memeq(key1->pubkey, key2->pubkey, ED25519_PUBKEY_LEN); } +/** + * Set dest to contain the same key as src. + */ +void +ed25519_pubkey_copy(ed25519_public_key_t *dest, + const ed25519_public_key_t *src) +{ + tor_assert(dest); + tor_assert(src); + memcpy(dest, src, sizeof(ed25519_public_key_t)); +} + /** Check whether the given Ed25519 implementation seems to be working. * If so, return 0; otherwise return -1. */ static int diff --git a/src/common/crypto_ed25519.h b/src/common/crypto_ed25519.h index 5d63d90..929b2b5 100644 --- a/src/common/crypto_ed25519.h +++ b/src/common/crypto_ed25519.h @@ -121,6 +121,9 @@ void ed25519_keypair_free(ed25519_keypair_t *kp); int ed25519_pubkey_eq(const ed25519_public_key_t *key1, const ed25519_public_key_t *key2); +void ed25519_pubkey_copy(ed25519_public_key_t *dest, + const ed25519_public_key_t *src); + void ed25519_set_impl_params(int use_donna); void ed25519_init(void); diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index bac68bb..96bd472 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1045,8 +1045,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) memcpy(ec.node_id, hop->extend_info->identity_digest, DIGEST_LEN); /* Set the ED25519 identity too -- it will only get included * in the extend2 cell if we're configured to use it, though. */ - memcpy(&ec.ed_pubkey, &hop->extend_info->ed_identity, - sizeof(ed25519_public_key_t)); + ed25519_pubkey_copy(&ec.ed_pubkey, &hop->extend_info->ed_identity); len = onion_skin_create(ec.create_cell.handshake_type, hop->extend_info, @@ -1183,7 +1182,7 @@ circuit_extend(cell_t *cell, circuit_t *circ) 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); + ed25519_pubkey_copy(&ec.ed_pubkey, node_ed_id); } }

1 0

[tor/master] Tiny cleanup of chan handling when setting connection ID digests
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 5eef00eb049606bf5b5610af8e3ce9514bd95882 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 7 13:03:21 2016 -0500 Tiny cleanup of chan handling when setting connection ID digests --- src/or/connection_or.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 046865d..f0292bc 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -153,8 +153,8 @@ connection_or_set_identity_digest(or_connection_t *conn, /* If the identity was set previously, remove the old mapping. */ if (rsa_id_was_set) { connection_or_clear_identity(conn); - if (conn->chan) - channel_clear_identity_digest(TLS_CHAN_TO_BASE(conn->chan)); + if (chan) + channel_clear_identity_digest(chan); } memcpy(conn->identity_digest, rsa_digest, DIGEST_LEN); @@ -165,9 +165,8 @@ connection_or_set_identity_digest(or_connection_t *conn, return; /* Deal with channels */ - if (conn->chan) - channel_set_identity_digest(TLS_CHAN_TO_BASE(conn->chan), - rsa_digest, ed_id); + if (chan) + channel_set_identity_digest(chan, rsa_digest, ed_id); } /** Remove the Extended ORPort identifier of conn from the

1 0

[tor/master] Add some unit testing for ed25519 IDs in extend2 cells.
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 835b04819ab75224b876c9b9ad9925f7f7b24229 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 11 14:28:15 2016 -0500 Add some unit testing for ed25519 IDs in extend2 cells. --- src/test/test_cell_formats.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c index f839a5b..bbcff38 100644 --- a/src/test/test_cell_formats.c +++ b/src/test/test_cell_formats.c @@ -11,6 +11,7 @@ #include "channel.h" #include "connection_edge.h" #include "connection_or.h" +#include "config.h" #include "onion.h" #include "onion_tap.h" #include "onion_fast.h" @@ -698,6 +699,7 @@ test_cfmt_extend_cells(void *arg) tt_int_op(61681, OP_EQ, ec.orport_ipv4.port); tt_str_op("2002::f0:c51e", OP_EQ, fmt_addr(&ec.orport_ipv6.addr)); tt_int_op(4370, OP_EQ, ec.orport_ipv6.port); + tt_assert(ed25519_public_key_is_zero(&ec.ed_pubkey)); tt_mem_op(ec.node_id,OP_EQ, "anthropomorphization", 20); tt_int_op(cc->cell_type, OP_EQ, CELL_CREATE2); tt_int_op(cc->handshake_type, OP_EQ, 0x105); @@ -717,6 +719,37 @@ test_cfmt_extend_cells(void *arg) tt_mem_op(p2+1+8+22+4,OP_EQ, b, 99+20); tt_int_op(0, OP_EQ, create_cell_format_relayed(&cell, cc)); + /* Now let's add an ed25519 key to that extend2 cell. */ + memcpy(ec.ed_pubkey.pubkey, + "brownshoesdontmakeit/brownshoesd", 32); + + /* As before, since we aren't extending by ed25519. */ + get_options_mutable()->ExtendByEd25519ID = 0; + tt_int_op(0, OP_EQ, extend_cell_format(&p2_cmd, &p2_len, p2, &ec)); + tt_int_op(p2_len, OP_EQ, 89+99-34-20); + test_memeq_hex(p2, + "02000612F40001F0F1" + "0214616e7468726f706f6d6f727068697a6174696f6e" + "01050063"); + + /* Now try with the ed25519 ID. */ + get_options_mutable()->ExtendByEd25519ID = 1; + tt_int_op(0, OP_EQ, extend_cell_format(&p2_cmd, &p2_len, p2, &ec)); + tt_int_op(p2_len, OP_EQ, 89+99-34-20 + 34); + test_memeq_hex(p2, + "03000612F40001F0F1" + "0214616e7468726f706f6d6f727068697a6174696f6e" + // ed digest follows: + "0320" "62726f776e73686f6573646f6e746d616b656" + "9742f62726f776e73686f657364" + "01050063"); + /* Can we parse that? Did the key come through right? */ + memset(&ec, 0, sizeof(ec)); + tt_int_op(0, OP_EQ, extend_cell_parse(&ec, RELAY_COMMAND_EXTEND2, + p2, p2_len)); + tt_mem_op("brownshoesdontmakeit/brownshoesd", OP_EQ, + ec.ed_pubkey.pubkey, 32); + /* == Now try parsing some junk */ /* Try a too-long handshake */ @@ -1257,7 +1290,7 @@ struct testcase_t cell_format_tests[] = { TEST(connected_cells, 0), TEST(create_cells, 0), TEST(created_cells, 0), - TEST(extend_cells, 0), + TEST(extend_cells, TT_FORK), TEST(extended_cells, 0), TEST(resolved_cells, 0), TEST(is_destroy, 0),

1 0

[tor/master] helper to test a node for matching an ed25519 ID.
by nickm＠torproject.org 08 Dec '16

08 Dec '16

commit 424ae9e18bc902e6c96a0e49b609f40868174af8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Nov 11 12:37:58 2016 -0500 helper to test a node for matching an ed25519 ID. --- src/or/nodelist.c | 12 ++++++++++++ src/or/nodelist.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/src/or/nodelist.c b/src/or/nodelist.c index 1f993e4..d8d2dbb 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -671,6 +671,18 @@ node_get_ed25519_id(const node_t *node) return NULL; } +/** Return true iff this node's Ed25519 identity matches id. + * (An absent Ed25519 identity matches NULL or zero.) */ +int +node_ed25519_id_matches(const node_t *node, const ed25519_public_key_t *id) +{ + const ed25519_public_key_t *node_id = node_get_ed25519_id(node); + if (node_id == NULL || ed25519_public_key_is_zero(node_id)) { + return id == NULL || ed25519_public_key_is_zero(id); + } else { + return id && ed25519_pubkey_eq(node_id, id); + } +} /** Return true iff node supports authenticating itself * by ed25519 ID during the link handshake in a way that we can understand diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 57c3b43..31fdc64 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -56,6 +56,8 @@ 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_ed25519_id_matches(const node_t *node, + const ed25519_public_key_t *id); int node_supports_ed25519_link_authentication(const node_t *node); const uint8_t *node_get_rsa_id_digest(const node_t *node);

1 0