diff options
| -rw-r--r-- | changes/bug7139 | 9 | ||||
| -rw-r--r-- | changes/dirserv-BUGGY-a | 7 | ||||
| -rw-r--r-- | src/common/tortls.c | 8 | ||||
| -rw-r--r-- | src/or/channel.c | 37 | ||||
| -rw-r--r-- | src/or/channel.h | 10 | ||||
| -rw-r--r-- | src/or/channeltls.c | 94 | ||||
| -rw-r--r-- | src/or/circuitbuild.c | 2 | ||||
| -rw-r--r-- | src/or/connection_edge.c | 7 | ||||
| -rw-r--r-- | src/or/directory.c | 2 | ||||
| -rw-r--r-- | src/or/reasons.c | 20 |
10 files changed, 142 insertions, 54 deletions
diff --git a/changes/bug7139 b/changes/bug7139 new file mode 100644 index 0000000000..dfb7d32838 --- /dev/null +++ b/changes/bug7139 @@ -0,0 +1,9 @@ + o Major bugfixes (security): + + - Disable TLS session tickets. OpenSSL's implementation were giving + our TLS session keys the lifetime of our TLS context objects, when + perfect forward secrecy would want us to discard anything that + could decrypt a link connection as soon as the link connection was + closed. Fixes bug 7139; bugfix on all versions of Tor linked + against OpenSSL 1.0.0 or later. Found by "nextgens". + diff --git a/changes/dirserv-BUGGY-a b/changes/dirserv-BUGGY-a new file mode 100644 index 0000000000..35b492a2d7 --- /dev/null +++ b/changes/dirserv-BUGGY-a @@ -0,0 +1,7 @@ + o Minor bugfixes: + + - Don't serve or accept v2 hidden service descriptors over a + relay's DirPort. It's never correct to do so, and disabling it + might make it more annoying to exploit any bugs that turn up in the + descriptor-parsing code. Fixes bug 7149. + diff --git a/src/common/tortls.c b/src/common/tortls.c index 75889e9ad4..2ff18355d1 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1190,6 +1190,14 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime, #ifdef SSL_OP_NO_TLSv1_1 SSL_CTX_set_options(result->ctx, SSL_OP_NO_TLSv1_1); #endif + /* Disable TLS tickets if they're supported. We never want to use them; + * using them can make our perfect forward secrecy a little worse, *and* + * create an opportunity to fingerprint us (since it's unusual to use them + * with TLS sessions turned off). + */ +#ifdef SSL_OP_NO_TICKET + SSL_CTX_set_options(result->ctx, SSL_OP_NO_TICKET); +#endif if ( #ifdef DISABLE_SSL3_HANDSHAKE diff --git a/src/or/channel.c b/src/or/channel.c index 49ce129585..89ec4de0b8 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -3243,6 +3243,7 @@ channel_dump_statistics(channel_t *chan, int severity) /* Handle remote address and descriptions */ have_remote_addr = channel_get_addr_if_possible(chan, &remote_addr); if (have_remote_addr) { + char *actual = tor_strdup(channel_get_actual_remote_descr(chan)); remote_addr_str = tor_dup_addr(&remote_addr); log(severity, LD_GENERAL, " * Channel " U64_FORMAT " says its remote address" @@ -3251,16 +3252,19 @@ channel_dump_statistics(channel_t *chan, int severity) U64_PRINTF_ARG(chan->global_identifier), remote_addr_str, channel_get_canonical_remote_descr(chan), - channel_get_actual_remote_descr(chan)); + actual); tor_free(remote_addr_str); + tor_free(actual); } else { + char *actual = tor_strdup(channel_get_actual_remote_descr(chan)); log(severity, LD_GENERAL, " * Channel " U64_FORMAT " does not know its remote " "address, but gives a canonical description of \"%s\" and an " "actual description of \"%s\"", U64_PRINTF_ARG(chan->global_identifier), channel_get_canonical_remote_descr(chan), - channel_get_actual_remote_descr(chan)); + actual); + tor_free(actual); } /* Handle marks */ @@ -3475,8 +3479,10 @@ channel_listener_dump_transport_statistics(channel_listener_t *chan_l, * This function return a test provided by the lower layer of the remote * endpoint for this channel; it should specify the actual address connected * to/from. + * + * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} + * may invalidate the return value from this function. */ - const char * channel_get_actual_remote_descr(channel_t *chan) { @@ -3484,7 +3490,20 @@ channel_get_actual_remote_descr(channel_t *chan) tor_assert(chan->get_remote_descr); /* Param 1 indicates the actual description */ - return chan->get_remote_descr(chan, 1); + return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL); +} + +/** + * Return the text address of the remote endpoint. + * + * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} + * may invalidate the return value from this function. + */ +const char * +channel_get_actual_remote_address(channel_t *chan) +{ + /* Param 1 indicates the actual description */ + return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY); } /** @@ -3493,8 +3512,10 @@ channel_get_actual_remote_descr(channel_t *chan) * This function return a test provided by the lower layer of the remote * endpoint for this channel; it should use the known canonical address for * this OR's identity digest if possible. + * + * Subsequent calls to channel_get_{actual,canonical}_remote_{address,descr} + * may invalidate the return value from this function. */ - const char * channel_get_canonical_remote_descr(channel_t *chan) { @@ -3506,12 +3527,12 @@ channel_get_canonical_remote_descr(channel_t *chan) } /** - * Get remote address if possible + * Get remote address if possible. * * Write the remote address out to a tor_addr_t if the underlying transport - * supports this operation. + * supports this operation, and return 1. Return 0 if the underlying transport + * doesn't let us do this. */ - int channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out) { diff --git a/src/or/channel.h b/src/or/channel.h index 33b7c8f88b..d90335c194 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -79,10 +79,13 @@ struct channel_s { * available. */ int (*get_remote_addr)(channel_t *, tor_addr_t *); +#define GRD_FLAG_ORIGINAL 1 +#define GRD_FLAG_ADDR_ONLY 2 /* - * Get a text description of the remote endpoint; canonicalized if the - * arg is 0, or the one we originally connected to/received from if it's - * 1. + * Get a text description of the remote endpoint; canonicalized if the flag + * GRD_FLAG_ORIGINAL is not set, or the one we originally connected + * to/received from if it is. If GRD_FLAG_ADDR_ONLY is set, we return only + * the original address. */ const char * (*get_remote_descr)(channel_t *, int); /* Check if the lower layer has queued writes */ @@ -424,6 +427,7 @@ const char * channel_describe_transport(channel_t *chan); void channel_dump_statistics(channel_t *chan, int severity); void channel_dump_transport_statistics(channel_t *chan, int severity); const char * channel_get_actual_remote_descr(channel_t *chan); +const char * channel_get_actual_remote_address(channel_t *chan); int channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out); const char * channel_get_canonical_remote_descr(channel_t *chan); int channel_has_queued_writes(channel_t *chan); diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 4470e92b79..4e3c20ab71 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -46,6 +46,9 @@ uint64_t stats_n_authorize_cells_processed = 0; /** Active listener, if any */ channel_listener_t *channel_tls_listener = NULL; +/* Utility function declarations */ +static void channel_tls_common_init(channel_tls_t *tlschan); + /* channel_tls_t method declarations */ static void channel_tls_close_method(channel_t *chan); @@ -53,7 +56,7 @@ static const char * channel_tls_describe_transport_method(channel_t *chan); static int channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out); static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int req); +channel_tls_get_remote_descr_method(channel_t *chan, int flags); static int channel_tls_has_queued_writes_method(channel_t *chan); static int channel_tls_is_canonical_method(channel_t *chan, int req); static int @@ -92,19 +95,18 @@ static int enter_v3_handshake_with_cell(var_cell_t *cell, channel_tls_t *tlschan); /** - * Start a new TLS channel - * - * Launch a new OR connection to <b>addr</b>:<b>port</b> and expect to - * handshake with an OR with identity digest <b>id_digest</b>, and wrap - * it in a channel_tls_t. + * Do parts of channel_tls_t initialization common to channel_tls_connect() + * and channel_tls_handle_incoming(). */ -channel_t * -channel_tls_connect(const tor_addr_t *addr, uint16_t port, - const char *id_digest) +static void +channel_tls_common_init(channel_tls_t *tlschan) { - channel_tls_t *tlschan = tor_malloc_zero(sizeof(*tlschan)); - channel_t *chan = &(tlschan->base_); + channel_t *chan; + + tor_assert(tlschan); + + chan = &(tlschan->base_); channel_init(chan); chan->magic = TLS_CHAN_MAGIC; chan->state = CHANNEL_STATE_OPENING; @@ -120,6 +122,29 @@ channel_tls_connect(const tor_addr_t *addr, uint16_t port, chan->write_packed_cell = channel_tls_write_packed_cell_method; chan->write_var_cell = channel_tls_write_var_cell_method; + chan->cmux = circuitmux_alloc(); + if (cell_ewma_enabled()) { + circuitmux_set_policy(chan->cmux, &ewma_policy); + } +} + +/** + * Start a new TLS channel + * + * Launch a new OR connection to <b>addr</b>:<b>port</b> and expect to + * handshake with an OR with identity digest <b>id_digest</b>, and wrap + * it in a channel_tls_t. + */ + +channel_t * +channel_tls_connect(const tor_addr_t *addr, uint16_t port, + const char *id_digest) +{ + channel_tls_t *tlschan = tor_malloc_zero(sizeof(*tlschan)); + channel_t *chan = &(tlschan->base_); + + channel_tls_common_init(tlschan); + log_debug(LD_CHANNEL, "In channel_tls_connect() for channel %p " "(global id " U64_FORMAT ")", @@ -129,11 +154,6 @@ channel_tls_connect(const tor_addr_t *addr, uint16_t port, if (is_local_addr(addr)) channel_mark_local(chan); channel_mark_outgoing(chan); - chan->cmux = circuitmux_alloc(); - if (cell_ewma_enabled()) { - circuitmux_set_policy(chan->cmux, &ewma_policy); - } - /* Set up or_connection stuff */ tlschan->conn = connection_or_connect(addr, port, id_digest, tlschan); /* connection_or_connect() will fill in tlschan->conn */ @@ -255,19 +275,7 @@ channel_tls_handle_incoming(or_connection_t *orconn) tor_assert(orconn); tor_assert(!(orconn->chan)); - channel_init(chan); - chan->magic = TLS_CHAN_MAGIC; - chan->state = CHANNEL_STATE_OPENING; - chan->close = channel_tls_close_method; - chan->describe_transport = channel_tls_describe_transport_method; - chan->get_remote_descr = channel_tls_get_remote_descr_method; - chan->has_queued_writes = channel_tls_has_queued_writes_method; - chan->is_canonical = channel_tls_is_canonical_method; - chan->matches_extend_info = channel_tls_matches_extend_info_method; - chan->matches_target = channel_tls_matches_target_method; - chan->write_cell = channel_tls_write_cell_method; - chan->write_packed_cell = channel_tls_write_packed_cell_method; - chan->write_var_cell = channel_tls_write_var_cell_method; + channel_tls_common_init(tlschan); /* Link the channel and orconn to each other */ tlschan->conn = orconn; @@ -276,11 +284,6 @@ channel_tls_handle_incoming(or_connection_t *orconn) if (is_local_addr(&(TO_CONN(orconn)->addr))) channel_mark_local(chan); channel_mark_incoming(chan); - chan->cmux = circuitmux_alloc(); - if (cell_ewma_enabled()) { - circuitmux_set_policy(chan->cmux, &ewma_policy); - } - /* If we got one, we should register it */ if (chan) channel_register(chan); @@ -412,7 +415,7 @@ channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out) */ static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int req) +channel_tls_get_remote_descr_method(channel_t *chan, int flags) { #define MAX_DESCR_LEN 32 @@ -427,21 +430,34 @@ channel_tls_get_remote_descr_method(channel_t *chan, int req) conn = TO_CONN(tlschan->conn); - switch (req) { + switch (flags) { case 0: - /* Canonical address */ + /* Canonical address with port*/ tor_snprintf(buf, MAX_DESCR_LEN + 1, "%s:%u", conn->address, conn->port); answer = buf; break; - case 1: - /* Actual address */ + case GRD_FLAG_ORIGINAL: + /* Actual address with port */ addr_str = tor_dup_addr(&(tlschan->conn->real_addr)); tor_snprintf(buf, MAX_DESCR_LEN + 1, "%s:%u", addr_str, conn->port); tor_free(addr_str); answer = buf; break; + case GRD_FLAG_ADDR_ONLY: + /* Canonical address, no port */ + strlcpy(buf, conn->address, sizeof(buf)); + answer = buf; + break; + case GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY: + /* Actual address, no port */ + addr_str = tor_dup_addr(&(tlschan->conn->real_addr)); + strlcpy(buf, addr_str, sizeof(buf)); + tor_free(addr_str); + answer = buf; + break; + default: /* Something's broken in channel.c */ tor_assert(1); diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 9287084cbb..b16dab2cca 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3050,7 +3050,7 @@ circuit_truncated(origin_circuit_t *circ, crypt_path_t *layer, int reason) * just give up. */ circuit_mark_for_close(TO_CIRCUIT(circ), - END_CIRC_REASON_FLAG_REMOTE|END_CIRC_REASON_CHANNEL_CLOSED|reason); + END_CIRC_REASON_FLAG_REMOTE|reason); return 0; #if 0 diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index e7bc09a197..4d528a810e 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -3105,7 +3105,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) * we might already have corrected base_.addr[ess] for the relay's * canonical IP address. */ if (or_circ && or_circ->p_chan) - address = tor_strdup(channel_get_actual_remote_descr(or_circ->p_chan)); + address = tor_strdup(channel_get_actual_remote_address(or_circ->p_chan)); else address = tor_strdup("127.0.0.1"); port = 1; /* XXXX This value is never actually used anywhere, and there @@ -3180,7 +3180,12 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) n_stream->on_circuit = circ; if (rh.command == RELAY_COMMAND_BEGIN_DIR) { + tor_addr_t tmp_addr; tor_assert(or_circ); + if (or_circ->p_chan && + channel_get_addr_if_possible(or_circ->p_chan, &tmp_addr)) { + tor_addr_copy(&n_stream->base_.addr, &tmp_addr); + } return connection_exit_connect_dir(n_stream); } diff --git a/src/or/directory.c b/src/or/directory.c index 2f70d1100d..26f9acc0e2 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3168,6 +3168,7 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, } if (options->HidServDirectoryV2 && + connection_dir_is_encrypted(conn) && !strcmpstart(url,"/tor/rendezvous2/")) { /* Handle v2 rendezvous descriptor fetch request. */ const char *descp; @@ -3354,6 +3355,7 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers, /* Handle v2 rendezvous service publish request. */ if (options->HidServDirectoryV2 && + connection_dir_is_encrypted(conn) && !strcmpstart(url,"/tor/rendezvous2/publish")) { switch (rend_cache_store_v2_desc_as_dir(body)) { case -2: diff --git a/src/or/reasons.c b/src/or/reasons.c index a04cd869a2..874a86774b 100644 --- a/src/or/reasons.c +++ b/src/or/reasons.c @@ -300,8 +300,13 @@ errno_to_orconn_end_reason(int e) const char * circuit_end_reason_to_control_string(int reason) { - if (reason >= 0 && reason & END_CIRC_REASON_FLAG_REMOTE) + int is_remote = 0; + + if (reason >= 0 && reason & END_CIRC_REASON_FLAG_REMOTE) { reason &= ~END_CIRC_REASON_FLAG_REMOTE; + is_remote = 1; + } + switch (reason) { case END_CIRC_AT_ORIGIN: /* This shouldn't get passed here; it's a catch-all reason. */ @@ -338,7 +343,18 @@ circuit_end_reason_to_control_string(int reason) case END_CIRC_REASON_MEASUREMENT_EXPIRED: return "MEASUREMENT_EXPIRED"; default: - log_warn(LD_BUG, "Unrecognized reason code %d", (int)reason); + if (is_remote) { + /* + * If it's remote, it's not a bug *here*, so don't use LD_BUG, but + * do note that the someone we're talking to is speaking the Tor + * protocol with a weird accent. + */ + log_warn(LD_PROTOCOL, + "Remote server sent bogus reason code %d", reason); + } else { + log_warn(LD_BUG, + "Unrecognized reason code %d", reason); + } return NULL; } } |