From d483d3144aa20b316578a37cf6a8b74343ca7dba Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 14 Nov 2007 20:01:12 +0000 Subject: [PATCH] r16669@catbus: nickm | 2007-11-14 14:50:03 -0500 When we complete an OR handshake, set up all the internal fields and mark the connection as open. svn:r12495 --- doc/TODO | 7 ++-- src/common/tortls.c | 12 +++---- src/common/tortls.h | 1 + src/or/command.c | 73 +++++++++++++++++++++++++-------------- src/or/connection_or.c | 78 ++++++++++++++++++++++++++++++++---------- src/or/or.h | 4 ++- 6 files changed, 120 insertions(+), 55 deletions(-) diff --git a/doc/TODO b/doc/TODO index aefeead536..19b9f82a29 100644 --- a/doc/TODO +++ b/doc/TODO @@ -50,11 +50,10 @@ Things we'd like to do in 0.2.0.x: o Generate CERT cells o Keep copies of X509 certs around, not necessarily associated with connection. - . LINK_AUTH cells + o LINK_AUTH cells o Code to generate o Remember certificate digests from TLS o Code to parse and check - - Unit tests - Revised handshake: TLS - Server checks for new cipher types, and if it finds them, sends only one cert and does not ask for client certs. @@ -65,9 +64,9 @@ Things we'd like to do in 0.2.0.x: o If in 'handshaking' state (since v2+ conn is in use), accept VERSIONS and NETINFO and CERT and LINK_AUTH. o After we send NETINFO, send CERT and LINK_AUTH if needed. - - Once we get a good LINK_AUTH, the connection is OPEN. + o Once we get a good LINK_AUTH, the connection is OPEN. - Ban most cell types on a non-OPEN connection. - - Close connections on handshake failure. + o Close connections on handshake failure. o Make code work right wrt TLS context rotation. - NETINFO fallout - Don't extend a circuit over a noncanonical connection with diff --git a/src/common/tortls.c b/src/common/tortls.c index 5b902c9e9a..87e2f3aea6 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -933,6 +933,7 @@ tor_tls_verify_certs_v2(int severity, tor_tls_t *tls, const char *id_cert_str, size_t id_cert_len, crypto_pk_env_t **cert_key_out, char *conn_cert_digest_out, + crypto_pk_env_t **id_key_out, char *id_digest_out) { X509 *cert = NULL, *id_cert = NULL; @@ -942,6 +943,7 @@ tor_tls_verify_certs_v2(int severity, tor_tls_t *tls, tor_assert(cert_key_out); tor_assert(conn_cert_digest_out); + tor_assert(id_key_out); tor_assert(id_digest_out); *cert_key_out = NULL; @@ -996,13 +998,9 @@ tor_tls_verify_certs_v2(int severity, tor_tls_t *tls, goto done; } - { - crypto_pk_env_t *i = _crypto_new_pk_env_evp_pkey(id_pkey); - if (!i) - goto done; - crypto_pk_get_digest(i, id_digest_out); - crypto_free_pk_env(i); - } + if (!(*id_key_out = _crypto_new_pk_env_evp_pkey(id_pkey))) + goto done; + crypto_pk_get_digest(*id_key_out, id_digest_out); if (!(cert_pkey = X509_get_pubkey(cert))) goto done; if (!(*cert_key_out = _crypto_new_pk_env_evp_pkey(cert_pkey))) diff --git a/src/common/tortls.h b/src/common/tortls.h index d30a154fa2..63380f54eb 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -67,6 +67,7 @@ int tor_tls_verify_certs_v2(int severity, tor_tls_t *tls, const char *id_cert_str, size_t id_cert_len, crypto_pk_env_t **cert_key_out, char *conn_cert_digest_out, + crypto_pk_env_t **id_key_out, char *id_digest_out); int tor_tls_check_lifetime(tor_tls_t *tls, int tolerance); int tor_tls_read(tor_tls_t *tls, char *cp, size_t len); diff --git a/src/or/command.c b/src/or/command.c index 8e8aa4abdc..9dc8408a87 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -475,18 +475,20 @@ command_process_versions_cell(var_cell_t *cell, or_connection_t *conn) } if (!highest_supported_version) { log_fn(LOG_PROTOCOL_WARN, LD_OR, - "Couldn't find a version in common; defaulting to v1."); - /*XXXX020 just break the connection! */ - conn->link_proto = 1; + "Couldn't find a version in common between my version list and the " + "list in the VERSIONS cell; closing connection."); + connection_mark_for_close(TO_CONN(conn)); return; } conn->link_proto = highest_supported_version; conn->handshake_state->received_versions = 1; if (highest_supported_version >= 2) { - /*XXXX020 check return values. */ - connection_or_send_netinfo(conn); - connection_or_send_cert(conn); + if (connection_or_send_netinfo(conn) < 0 || + connection_or_send_cert(conn) < 0) { + connection_mark_for_close(TO_CONN(conn)); + return; + } if (conn->handshake_state->started_here) connection_or_send_link_auth(conn); } else { @@ -536,8 +538,8 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn) cp = cell->payload + 6 + my_addr_len; if (cp >= end) { log_fn(LOG_PROTOCOL_WARN, LD_OR, - "Address too long in netinfo cell; dropping."); - /*XXXX020 reject and break OR conn! */ + "Addresses too long in netinfo cell; closing connection."); + connection_mark_for_close(TO_CONN(conn)); return; } else if (my_addr_type == RESOLVED_TYPE_IPV4 && my_addr_len == 4) { conn->handshake_state->my_apparent_addr = ntohl(get_uint32(my_addr_ptr)); @@ -549,8 +551,12 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn) * "canonical." */ uint8_t other_addr_type = (uint8_t) *cp++; uint8_t other_addr_len = (uint8_t) *cp++; - if (cp + other_addr_len >= end) - break; /*XXXX020 protocol warn. */ + if (cp + other_addr_len >= end) { + log_fn(LOG_PROTOCOL_WARN, LD_OR, + "Address too long in netinfo cell; closing connection."); + connection_mark_for_close(TO_CONN(conn)); + return; + } if (other_addr_type == RESOLVED_TYPE_IPV4 && other_addr_len == 4) { uint32_t addr = ntohl(get_uint32(cp)); if (addr == conn->real_addr) { @@ -568,12 +574,12 @@ command_process_netinfo_cell(cell_t *cell, or_connection_t *conn) /*XXXX020 move to connection_or.c */ /** DOCDOC Called when we're done authenticating; act on stuff we * learned in netinfo. */ -void +int connection_or_act_on_netinfo(or_connection_t *conn) { long delta; if (!conn->handshake_state) - return; + return -1; tor_assert(conn->handshake_state->authenticated != 0); @@ -601,6 +607,8 @@ connection_or_act_on_netinfo(or_connection_t *conn) if (conn->handshake_state->apparently_canonical) { conn->is_canonical = 1; } + + return 0; } /*DOCDOC*/ @@ -611,16 +619,25 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn) uint16_t conn_cert_len = 0, id_cert_len = 0; const char *conn_cert = NULL, *id_cert = NULL; const char *cp, *end; - int authenticated = 0; + int done = 0; - /*XXXX020 log messages*/ - if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING) - goto err; + if (conn->_base.state != OR_CONN_STATE_OR_HANDSHAKING) { + log_fn(LOG_PROTOCOL_WARN, LD_OR, "Got CERT cell when not handshaking. " + "Ignoring."); + return; + } tor_assert(conn->handshake_state); if (!conn->handshake_state->received_versions || - !conn->handshake_state->received_netinfo || - conn->handshake_state->received_certs) + !conn->handshake_state->received_netinfo) { + log_fn(LOG_PROTOCOL_WARN, LD_OR, "Got CERT cell before VERSIONS and " + "NETINFO. Closing the connection."); goto err; + } + if (conn->handshake_state->received_certs) { + log_fn(LOG_PROTOCOL_WARN, LD_OR, "Got duplicate CERT cell. " + "Closing the connection."); + goto err; + } cp = cell->payload; end = cell->payload + cell->payload_len; @@ -651,6 +668,7 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn) /* Now we have 0, 1, or 2 certs. */ if (n_certs == 0) { /* The other side is unauthenticated. */ + done = 1; } else { int r; r = tor_tls_verify_certs_v2(LOG_PROTOCOL_WARN, conn->tls, @@ -660,23 +678,27 @@ command_process_cert_cell(var_cell_t *cell, or_connection_t *conn) (conn->handshake_state->started_here ? conn->handshake_state->server_cert_digest : conn->handshake_state->client_cert_digest), + &conn->handshake_state->identity_key, conn->handshake_state->cert_id_digest); if (r < 0) goto err; - if (r == 1) - authenticated = 1; + if (r == 1) { + done = 1; + conn->handshake_state->authenticated = 1; + } } conn->handshake_state->received_certs = 1; - if (authenticated) { - /* XXXX020 make the connection open. */ + if (done) { + if (connection_or_finish_or_handshake(conn) < 0) + goto err; } if (! conn->handshake_state->signing_key) goto err; return; err: - /*XXXX020 close the connection */; + connection_mark_for_close(TO_CONN(conn)); } #define LINK_AUTH_STRING "Tor initiator certificate verification" @@ -746,11 +768,12 @@ command_process_link_auth_cell(cell_t *cell, or_connection_t *conn) goto err; } - /* Okay, we're authenticated. */ s->authenticated = 1; - /* XXXX020 act on being authenticated: Open the connection. */ + if (connection_or_finish_or_handshake(conn)<0) + goto err; + tor_free(checked); return; err: tor_free(checked); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 4071c9cf8d..c3f2774b49 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -614,6 +614,8 @@ connection_or_nonopen_was_started_here(or_connection_t *conn) tor_assert(conn->_base.type == CONN_TYPE_OR); if (!conn->tls) return 1; /* it's still in proxy states or something */ + if (conn->handshake_state) + return conn->handshake_state->started_here; return !tor_tls_is_server(conn->tls); } @@ -651,8 +653,15 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, started_here ? conn->_base.address : safe_str(conn->_base.address); const char *conn_type = started_here ? "outgoing" : "incoming"; int has_cert = 0, has_identity = 0; + int v1 = (conn->link_proto == 1); check_no_tls_errors(); + if (v1) { + has_cert = tor_tls_peer_has_cert(conn->tls); + } else { + tor_assert(conn->handshake_state); + has_cert = !tor_digest_is_zero(conn->handshake_state->cert_id_digest); + } has_cert = tor_tls_peer_has_cert(conn->tls); if (started_here && !has_cert) { log_info(LD_PROTOCOL,"Tried connecting to router at %s:%d, but it didn't " @@ -665,28 +674,34 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, } check_no_tls_errors(); - if (has_cert) { - int v = tor_tls_verify_v1(started_here?severity:LOG_INFO, - conn->tls, &identity_rcvd); - if (started_here && v<0) { - log_fn(severity,LD_OR,"Tried connecting to router at %s:%d: It" - " has a cert but it's invalid. Closing.", - safe_address, conn->_base.port); - return -1; - } else if (v<0) { - log_info(LD_PROTOCOL,"Incoming connection gave us an invalid cert " - "chain; ignoring."); - } else { - log_debug(LD_OR,"The certificate seems to be valid on %s connection " - "with %s:%d", conn_type, safe_address, conn->_base.port); + if (v1) { + if (has_cert) { + int v = tor_tls_verify_v1(started_here?severity:LOG_INFO, + conn->tls, &identity_rcvd); + if (started_here && v<0) { + log_fn(severity,LD_OR,"Tried connecting to router at %s:%d: It" + " has a cert but it's invalid. Closing.", + safe_address, conn->_base.port); + return -1; + } else if (v<0) { + log_info(LD_PROTOCOL,"Incoming connection gave us an invalid cert " + "chain; ignoring."); + } else { + log_debug(LD_OR,"The certificate seems to be valid on %s connection " + "with %s:%d", conn_type, safe_address, conn->_base.port); + } + check_no_tls_errors(); + } + } else { + if (conn->handshake_state->authenticated && + conn->handshake_state->identity_key) { + identity_rcvd = crypto_pk_dup_key(conn->handshake_state->identity_key); } - check_no_tls_errors(); } if (identity_rcvd) { has_identity=1; crypto_pk_get_digest(identity_rcvd, digest_rcvd_out); - if (crypto_pk_cmp_keys(get_identity_key(), identity_rcvd)<0) { conn->circ_id_type = CIRC_ID_TYPE_LOWER; } else { @@ -744,6 +759,29 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, return 0; } +/** DOCDOC */ +int +connection_or_finish_or_handshake(or_connection_t *conn) +{ + char id_digest[DIGEST_LEN]; + tor_assert(conn); + tor_assert(conn->handshake_state); + tor_assert(conn->link_proto >= 2); + tor_assert(conn->handshake_state->received_versions != 0); + tor_assert(conn->handshake_state->received_netinfo != 0); + tor_assert(conn->handshake_state->received_certs != 0); + + if (connection_or_check_valid_tls_handshake(conn, + conn->handshake_state->started_here, + id_digest) < 0) + return -1; + connection_or_init_conn_from_address(conn, conn->_base.addr, + conn->_base.port, id_digest, 0); + if (connection_or_act_on_netinfo(conn)<0) + return -1; + return connection_or_set_state_open(conn); +} + /** The tls handshake is finished. * * Make sure we are happy with the person we just handshaked with. @@ -815,6 +853,8 @@ or_handshake_state_free(or_handshake_state_t *state) tor_assert(state); if (state->signing_key) crypto_free_pk_env(state->signing_key); + if (state->identity_key) + crypto_free_pk_env(state->identity_key); memset(state, 0xBE, sizeof(or_handshake_state_t)); tor_free(state); } @@ -836,6 +876,10 @@ connection_or_set_state_open(or_connection_t *conn) } router_set_status(conn->identity_digest, 1); } + if (conn->handshake_state) { + or_handshake_state_free(conn->handshake_state); + conn->handshake_state = NULL; + } connection_watch_events(TO_CONN(conn), EV_READ); circuit_n_conn_done(conn, 1); /* send the pending creates, if any. */ @@ -1120,8 +1164,6 @@ connection_or_send_link_auth(or_connection_t *conn) connection_or_write_cell_to_buf(&cell, conn); - /* XXXX020 at this point, as a client, we can consider ourself - * authenticated. */ return 0; } diff --git a/src/or/or.h b/src/or/or.h index 2dd1e3595a..263e7c420a 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -894,6 +894,7 @@ typedef struct or_handshake_state_t { /* from certs */ char cert_id_digest[DIGEST_LEN]; crypto_pk_env_t *signing_key; + crypto_pk_env_t *identity_key; } or_handshake_state_t; /** Subtype of connection_t for an "OR connection" -- that is, one that speaks @@ -2561,7 +2562,7 @@ int connection_ap_handshake_attach_circuit(edge_connection_t *conn); void command_process_cell(cell_t *cell, or_connection_t *conn); void command_process_var_cell(var_cell_t *cell, or_connection_t *conn); -void connection_or_act_on_netinfo(or_connection_t *conn); +int connection_or_act_on_netinfo(or_connection_t *conn); extern uint64_t stats_n_padding_cells_processed; extern uint64_t stats_n_create_cells_processed; @@ -2781,6 +2782,7 @@ int connection_or_process_inbuf(or_connection_t *conn); int connection_or_flushed_some(or_connection_t *conn); int connection_or_finished_flushing(or_connection_t *conn); int connection_or_finished_connecting(or_connection_t *conn); +int connection_or_finish_or_handshake(or_connection_t *conn); or_connection_t *connection_or_connect(uint32_t addr, uint16_t port, const char *id_digest);