From f35ef825f948ddf50bf5b3022a707dc91dc58f16 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Tue, 23 Aug 2005 09:50:51 +0000 Subject: [PATCH] Our logic to decide if the OR we connected to was the right guy was brittle and maybe open to a mitm for unverified routers. Now we be sure to check the digest, and if the nickname he claims is not a verified one then we don't care what nickname he claims. svn:r4823 --- src/or/connection_or.c | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 713c7b8c6c..f68aed0de4 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -29,8 +29,8 @@ static int connection_or_process_cells_from_inbuf(connection_t *conn); static void cell_pack(char *dest, const cell_t *src) { - *(uint16_t*)dest = htons(src->circ_id); - *(uint8_t*)(dest+2) = src->command; + *(uint16_t*)dest = htons(src->circ_id); + *(uint8_t*)(dest+2) = src->command; memcpy(dest+3, src->payload, CELL_PAYLOAD_SIZE); } @@ -232,7 +232,9 @@ connection_or_init_conn_from_router(connection_t *conn, routerinfo_t *router) conn->address = tor_strdup(router->address); } -/** DOCDOC */ +/** If we don't necessarily know the router we're connecting to, but we + * have an addr/port/id_digest, then fill in as much as we can. Start + * by checking to see if this describes a router we know. */ static void connection_or_init_conn_from_address(connection_t *conn, uint32_t addr, uint16_t port, @@ -269,7 +271,8 @@ connection_or_init_conn_from_address(connection_t *conn, tor_inet_ntoa(&in,conn->address,INET_NTOA_BUF_LEN); } -/** DOCDOC */ +/** "update an OR connection nickname on the fly" + * Actually, nobody calls this. Should we remove it? */ void connection_or_update_nickname(connection_t *conn) { @@ -308,7 +311,7 @@ connection_or_update_nickname(connection_t *conn) * If id_digest is me, do nothing. If we're already connected to it, * return that connection. If the connect() is in progress, set the * new conn's state to 'connecting' and return it. If connect() succeeds, - * call * connection_tls_start_handshake() on it. + * call connection_tls_start_handshake() on it. * * This function is called from router_retry_connections(), for * ORs connecting to ORs, and circuit_establish_circuit(), for @@ -333,7 +336,6 @@ connection_or_connect(uint32_t addr, uint16_t port, const char *id_digest) /* this function should never be called if we're already connected to * id_digest, but check first to be sure */ - /*XXX this is getting called, at least by dirservers. */ conn = connection_get_by_identity_digest(id_digest, CONN_TYPE_OR); if (conn) { tor_assert(conn->nickname); @@ -455,7 +457,7 @@ connection_or_nonopen_was_started_here(connection_t *conn) * * Make sure he sent a correctly formed certificate. If it has a * recognized (approved) nickname, make sure his identity key matches - * to it. If I initiated the connection, make sure it's the right guy. + * it. If I initiated the connection, make sure it's the right guy. * * If we return 0, write a hash of the identity key into digest_rcvd, * which must have DIGEST_LEN space in it. (If we return -1 this @@ -520,23 +522,15 @@ connection_or_check_valid_handshake(connection_t *conn, char *digest_rcvd) if (connection_or_nonopen_was_started_here(conn)) { int as_advertised = 1; - if (conn->nickname[0] == '$') { - /* I was aiming for a particular digest. Did I get it? */ - char d[HEX_DIGEST_LEN+1]; - base16_encode(d, HEX_DIGEST_LEN+1, digest_rcvd, DIGEST_LEN); - if (strcasecmp(d,conn->nickname+1)) { - log_fn(severity, - "Identity key not as expected for router at %s:%d: wanted %s but got %s", - conn->address, conn->port, conn->nickname+1, d); - helper_node_set_status(conn->identity_digest, 0); - control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED); - as_advertised = 0; - } - } else if (strcasecmp(conn->nickname, nickname)) { - /* I was aiming for a nickname. Did I get it? */ + if (memcmp(digest_rcvd, conn->identity_digest, DIGEST_LEN)) { + /* 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), digest_rcvd, DIGEST_LEN); + base16_encode(expected, sizeof(expected), conn->identity_digest, DIGEST_LEN); log_fn(severity, - "Other side (%s:%d) is '%s', but we tried to connect to '%s'", - conn->address, conn->port, nickname, conn->nickname); + "Identity key not as expected for router at %s:%d: wanted %s but got %s", + conn->address, conn->port, expected, seen); helper_node_set_status(conn->identity_digest, 0); control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED); as_advertised = 0;