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

[tor/release-0.2.8] Work around a behavior change in openssl's BUF_MEM code
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 9b18b215bb34c4df1b94dec218c68a532c341e26 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 3 10:46:27 2016 -0400 Work around a behavior change in openssl's BUF_MEM code In our code to write public keys to a string, for some unfathomable reason since 253f0f160e1185c, we would allocate a memory BIO, then set the NOCLOSE flag on it, extract its memory buffer, and free it. Then a little while later we'd free the memory buffer with BUF_MEM_free(). As of openssl 1.1 this doesn't work any more, since there is now a BIO_BUF_MEM structure that wraps the BUF_MEM structure. This BIO_BUF_MEM doesn't get freed in our code. So, we had a memory leak! Is this an openssl bug? Maybe. But our code was already pretty silly. Why mess around with the NOCLOSE flag here when we can just keep the BIO object around until we don't need the buffer any more? Fixes bug 20553; bugfix on 0.0.2pre8 --- changes/bug20553 | 3 +++ src/common/crypto.c | 5 ++--- src/tools/tor-gencert.c | 5 ++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/changes/bug20553 b/changes/bug20553 new file mode 100644 index 0000000..12a2780 --- /dev/null +++ b/changes/bug20553 @@ -0,0 +1,3 @@ + o Minor bugfixes (memory leak): + - Work around a memory leak in OpenSSL 1.1 when encoding public keys. + Fixes bug 20553; bugfix on 0.0.2pre8. diff --git a/src/common/crypto.c b/src/common/crypto.c index 2b96324..c5d07df 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -755,14 +755,13 @@ crypto_pk_write_key_to_string_impl(crypto_pk_t *env, char **dest, } BIO_get_mem_ptr(b, &buf); - (void)BIO_set_close(b, BIO_NOCLOSE); /* so BIO_free doesn't free buf */ - BIO_free(b); *dest = tor_malloc(buf->length+1); memcpy(*dest, buf->data, buf->length); (*dest)[buf->length] = 0; /* nul terminate it */ *len = buf->length; - BUF_MEM_free(buf); + + BIO_free(b); return 0; } diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c index 5f2cd3a..ed6c066 100644 --- a/src/tools/tor-gencert.c +++ b/src/tools/tor-gencert.c @@ -429,12 +429,11 @@ key_to_string(EVP_PKEY *key) } BIO_get_mem_ptr(b, &buf); - (void) BIO_set_close(b, BIO_NOCLOSE); - BIO_free(b); result = tor_malloc(buf->length + 1); memcpy(result, buf->data, buf->length); result[buf->length] = 0; - BUF_MEM_free(buf); + + BIO_free(b); RSA_free(rsa); return result;

1 0

[tor/release-0.2.9] Work around a behavior change in openssl's BUF_MEM code
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 9b18b215bb34c4df1b94dec218c68a532c341e26 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 3 10:46:27 2016 -0400 Work around a behavior change in openssl's BUF_MEM code In our code to write public keys to a string, for some unfathomable reason since 253f0f160e1185c, we would allocate a memory BIO, then set the NOCLOSE flag on it, extract its memory buffer, and free it. Then a little while later we'd free the memory buffer with BUF_MEM_free(). As of openssl 1.1 this doesn't work any more, since there is now a BIO_BUF_MEM structure that wraps the BUF_MEM structure. This BIO_BUF_MEM doesn't get freed in our code. So, we had a memory leak! Is this an openssl bug? Maybe. But our code was already pretty silly. Why mess around with the NOCLOSE flag here when we can just keep the BIO object around until we don't need the buffer any more? Fixes bug 20553; bugfix on 0.0.2pre8 --- changes/bug20553 | 3 +++ src/common/crypto.c | 5 ++--- src/tools/tor-gencert.c | 5 ++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/changes/bug20553 b/changes/bug20553 new file mode 100644 index 0000000..12a2780 --- /dev/null +++ b/changes/bug20553 @@ -0,0 +1,3 @@ + o Minor bugfixes (memory leak): + - Work around a memory leak in OpenSSL 1.1 when encoding public keys. + Fixes bug 20553; bugfix on 0.0.2pre8. diff --git a/src/common/crypto.c b/src/common/crypto.c index 2b96324..c5d07df 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -755,14 +755,13 @@ crypto_pk_write_key_to_string_impl(crypto_pk_t *env, char **dest, } BIO_get_mem_ptr(b, &buf); - (void)BIO_set_close(b, BIO_NOCLOSE); /* so BIO_free doesn't free buf */ - BIO_free(b); *dest = tor_malloc(buf->length+1); memcpy(*dest, buf->data, buf->length); (*dest)[buf->length] = 0; /* nul terminate it */ *len = buf->length; - BUF_MEM_free(buf); + + BIO_free(b); return 0; } diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c index 5f2cd3a..ed6c066 100644 --- a/src/tools/tor-gencert.c +++ b/src/tools/tor-gencert.c @@ -429,12 +429,11 @@ key_to_string(EVP_PKEY *key) } BIO_get_mem_ptr(b, &buf); - (void) BIO_set_close(b, BIO_NOCLOSE); - BIO_free(b); result = tor_malloc(buf->length + 1); memcpy(result, buf->data, buf->length); result[buf->length] = 0; - BUF_MEM_free(buf); + + BIO_free(b); RSA_free(rsa); return result;

1 0

[tor/master] Do assert when calling relay_send_command_from_edge() on OR-side circs
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 23b878b875d7feab195be0e856da4aa3f5ac2e15 Author: Ivan Markin <twim(a)riseup.net> Date: Mon Oct 17 16:06:36 2016 +0000 Do assert when calling relay_send_command_from_edge() on OR-side circs --- src/or/relay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/relay.c b/src/or/relay.c index 823e074..aa29cc8 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -617,7 +617,7 @@ relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, cell.circ_id = TO_OR_CIRCUIT(circ)->p_circ_id; cell_direction = CELL_DIRECTION_IN; } else { - return -1; + tor_assert(0); } memset(&rh, 0, sizeof(rh));

1 0

[tor/master] Do not mark circs for close again after relay_send_command_from_edge()
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 2d049469602f5e6efbe8f1ded0cbcd3aa957f3eb Author: Ivan Markin <twim(a)riseup.net> Date: Sun Oct 16 20:08:26 2016 +0000 Do not mark circs for close again after relay_send_command_from_edge() --- src/or/rendmid.c | 8 ++------ src/or/rendservice.c | 6 ++---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/or/rendmid.c b/src/or/rendmid.c index ca0ad7b..96993b6 100644 --- a/src/or/rendmid.c +++ b/src/or/rendmid.c @@ -106,7 +106,7 @@ rend_mid_establish_intro(or_circuit_t *circ, const uint8_t *request, RELAY_COMMAND_INTRO_ESTABLISHED, "", 0, NULL)<0) { log_info(LD_GENERAL, "Couldn't send INTRO_ESTABLISHED cell."); - goto err; + return -1; } /* Now, set up this circuit. */ @@ -208,7 +208,6 @@ rend_mid_introduce(or_circuit_t *circ, const uint8_t *request, RELAY_COMMAND_INTRODUCE_ACK, NULL,0,NULL)) { log_warn(LD_GENERAL, "Unable to send INTRODUCE_ACK cell to Tor client."); - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); return -1; } @@ -220,8 +219,6 @@ rend_mid_introduce(or_circuit_t *circ, const uint8_t *request, RELAY_COMMAND_INTRODUCE_ACK, nak_body, 1, NULL)) { log_warn(LD_GENERAL, "Unable to send NAK to Tor client."); - /* Is this right? */ - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); } return -1; } @@ -269,8 +266,7 @@ rend_mid_establish_rendezvous(or_circuit_t *circ, const uint8_t *request, RELAY_COMMAND_RENDEZVOUS_ESTABLISHED, "", 0, NULL)<0) { log_warn(LD_PROTOCOL, "Couldn't send RENDEZVOUS_ESTABLISHED cell."); - reason = END_CIRC_REASON_INTERNAL; - goto err; + return -1; } circuit_change_purpose(TO_CIRCUIT(circ), CIRCUIT_PURPOSE_REND_POINT_WAITING); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ab7ec3f..3aeade8 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3060,8 +3060,7 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) log_info(LD_GENERAL, "Couldn't send introduction request for service %s on circuit %u", serviceid, (unsigned)circuit->base_.n_circ_id); - reason = END_CIRC_REASON_INTERNAL; - goto err; + goto done; } /* We've attempted to use this circuit */ @@ -3223,8 +3222,7 @@ rend_service_rendezvous_has_opened(origin_circuit_t *circuit) buf, REND_COOKIE_LEN+DH_KEY_LEN+DIGEST_LEN, circuit->cpath->prev)<0) { log_warn(LD_GENERAL, "Couldn't send RENDEZVOUS1 cell."); - reason = END_CIRC_REASON_INTERNAL; - goto err; + goto done; } crypto_dh_free(hop->rend_dh_handshake_state);

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/bug20376_030_01'
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 40487b0141096ddb40bec38f7d1c6a4367d8c279 Merge: df2615d 70b9e79 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 3 14:35:03 2016 -0400 Merge remote-tracking branch 'dgoulet/bug20376_030_01' src/or/relay.c | 7 +++---- src/or/rendmid.c | 19 ++++++++++--------- src/or/rendservice.c | 6 ++---- 3 files changed, 15 insertions(+), 17 deletions(-)

1 0

[tor/master] Slightly refactor and fix couple callsites
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 70b9e79700f85ac07b931a2c836b4d63d0ba70c1 Author: David Goulet <dgoulet(a)torproject.org> Date: Thu Oct 27 10:34:02 2016 -0400 Slightly refactor and fix couple callsites Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/relay.c | 7 +++---- src/or/rendmid.c | 13 +++++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/or/relay.c b/src/or/relay.c index aa29cc8..f5e9a6b 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -610,14 +610,13 @@ relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, memset(&cell, 0, sizeof(cell_t)); cell.command = CELL_RELAY; - if (cpath_layer) { + if (CIRCUIT_IS_ORIGIN(circ)) { + tor_assert(cpath_layer); cell.circ_id = circ->n_circ_id; cell_direction = CELL_DIRECTION_OUT; - } else if (! CIRCUIT_IS_ORIGIN(circ)) { + } else { cell.circ_id = TO_OR_CIRCUIT(circ)->p_circ_id; cell_direction = CELL_DIRECTION_IN; - } else { - tor_assert(0); } memset(&rh, 0, sizeof(rh)); diff --git a/src/or/rendmid.c b/src/or/rendmid.c index 96993b6..f39c92a 100644 --- a/src/or/rendmid.c +++ b/src/or/rendmid.c @@ -106,7 +106,7 @@ rend_mid_establish_intro(or_circuit_t *circ, const uint8_t *request, RELAY_COMMAND_INTRO_ESTABLISHED, "", 0, NULL)<0) { log_info(LD_GENERAL, "Couldn't send INTRO_ESTABLISHED cell."); - return -1; + goto err_no_close; } /* Now, set up this circuit. */ @@ -122,8 +122,9 @@ rend_mid_establish_intro(or_circuit_t *circ, const uint8_t *request, log_warn(LD_PROTOCOL, "Rejecting truncated ESTABLISH_INTRO cell."); reason = END_CIRC_REASON_TORPROTOCOL; err: - if (pk) crypto_pk_free(pk); circuit_mark_for_close(TO_CIRCUIT(circ), reason); + err_no_close: + if (pk) crypto_pk_free(pk); return -1; } @@ -201,13 +202,15 @@ rend_mid_introduce(or_circuit_t *circ, const uint8_t *request, (char*)request, request_len, NULL)) { log_warn(LD_GENERAL, "Unable to send INTRODUCE2 cell to Tor client."); - goto err; + /* Stop right now, the circuit has been closed. */ + return -1; } /* And send an ack down the client's circuit. Empty body means succeeded. */ if (relay_send_command_from_edge(0,TO_CIRCUIT(circ), RELAY_COMMAND_INTRODUCE_ACK, NULL,0,NULL)) { log_warn(LD_GENERAL, "Unable to send INTRODUCE_ACK cell to Tor client."); + /* Stop right now, the circuit has been closed. */ return -1; } @@ -266,6 +269,7 @@ rend_mid_establish_rendezvous(or_circuit_t *circ, const uint8_t *request, RELAY_COMMAND_RENDEZVOUS_ESTABLISHED, "", 0, NULL)<0) { log_warn(LD_PROTOCOL, "Couldn't send RENDEZVOUS_ESTABLISHED cell."); + /* Stop right now, the circuit has been closed. */ return -1; } @@ -342,7 +346,8 @@ rend_mid_rendezvous(or_circuit_t *circ, const uint8_t *request, log_warn(LD_GENERAL, "Unable to send RENDEZVOUS2 cell to client on circuit %u.", (unsigned)rend_circ->p_circ_id); - goto err; + /* Stop right now, the circuit has been closed. */ + return -1; } /* Join the circuits. */

1 0

[tor/master] Add assertion that cpath_layer==NULL on OR circ
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit f0b86e30d0d53e997872a9ece42242e8e14d45c8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 3 14:36:10 2016 -0400 Add assertion that cpath_layer==NULL on OR circ --- src/or/relay.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/or/relay.c b/src/or/relay.c index f5e9a6b..99750af 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -615,6 +615,7 @@ relay_send_command_from_edge_(streamid_t stream_id, circuit_t *circ, cell.circ_id = circ->n_circ_id; cell_direction = CELL_DIRECTION_OUT; } else { + tor_assert(! cpath_layer); cell.circ_id = TO_OR_CIRCUIT(circ)->p_circ_id; cell_direction = CELL_DIRECTION_IN; }

1 0

[tor/master] Merge branch 'bug20527_030_01_squashed'
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 16e75587f6c6e1dd0d316e8360c966f4841dc32c Merge: 409984c 2f52fae Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 3 14:23:47 2016 -0400 Merge branch 'bug20527_030_01_squashed' src/or/rendservice.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)

1 0

[tor/master] hs: Escape service directories before printing them
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit 2f52faee033bb2c0a203b73f7ad86f98dc0fc296 Author: Ivan Markin <twim(a)riseup.net> Date: Tue Nov 1 18:55:03 2016 -0100 hs: Escape service directories before printing them Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- src/or/rendservice.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ab7ec3f..1584691 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -286,8 +286,8 @@ rend_add_service(rend_service_t *service) } } smartlist_add(rend_service_list, service); - log_debug(LD_REND,"Configuring service with directory \"%s\"", - service->directory); + log_debug(LD_REND,"Configuring service with directory %s", + rend_service_escaped_dir(service)); for (i = 0; i < smartlist_len(service->ports); ++i) { p = smartlist_get(service->ports, i); if (!(p->is_unix_addr)) { @@ -504,7 +504,8 @@ rend_config_services(const or_options_t *options, int validate_only) } log_info(LD_CONFIG, "HiddenServiceAllowUnknownPorts=%d for %s", - (int)service->allow_unknown_ports, service->directory); + (int)service->allow_unknown_ports, + rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) { service->dir_group_readable = (int)tor_parse_long(line->value, @@ -518,7 +519,7 @@ rend_config_services(const or_options_t *options, int validate_only) } log_info(LD_CONFIG, "HiddenServiceDirGroupReadable=%d for %s", - service->dir_group_readable, service->directory); + service->dir_group_readable, rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { service->max_streams_per_circuit = (int)tor_parse_long(line->value, 10, 0, 65535, &ok, NULL); @@ -531,7 +532,7 @@ rend_config_services(const or_options_t *options, int validate_only) } log_info(LD_CONFIG, "HiddenServiceMaxStreams=%d for %s", - service->max_streams_per_circuit, service->directory); + service->max_streams_per_circuit, rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { service->max_streams_close_circuit = (int)tor_parse_long(line->value, 10, 0, 1, &ok, NULL); @@ -545,7 +546,8 @@ rend_config_services(const or_options_t *options, int validate_only) } log_info(LD_CONFIG, "HiddenServiceMaxStreamsCloseCircuit=%d for %s", - (int)service->max_streams_close_circuit, service->directory); + (int)service->max_streams_close_circuit, + rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { service->n_intro_points_wanted = (unsigned int) tor_parse_long(line->value, 10, @@ -561,7 +563,7 @@ rend_config_services(const or_options_t *options, int validate_only) return -1; } log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", - service->n_intro_points_wanted, service->directory); + service->n_intro_points_wanted, rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) { /* Parse auth type and comma-separated list of client names and add a * rend_authorized_client_t for each client to the service's list @@ -1187,8 +1189,8 @@ rend_service_load_all_keys(const smartlist_t *service_list) SMARTLIST_FOREACH_BEGIN(s_list, rend_service_t *, s) { if (s->private_key) continue; - log_info(LD_REND, "Loading hidden-service keys from \"%s\"", - s->directory); + log_info(LD_REND, "Loading hidden-service keys from %s", + rend_service_escaped_dir(s)); if (rend_service_load_keys(s) < 0) return -1; @@ -1266,7 +1268,8 @@ rend_service_load_keys(rend_service_t *s) if (s->dir_group_readable) { /* Only new dirs created get new opts, also enforce group read. */ if (chmod(s->directory, 0750)) { - log_warn(LD_FS,"Unable to make %s group-readable.", s->directory); + log_warn(LD_FS,"Unable to make %s group-readable.", + rend_service_escaped_dir(s)); } } #endif @@ -4009,8 +4012,8 @@ rend_service_dump_stats(int severity) for (i=0; i < smartlist_len(rend_service_list); ++i) { service = smartlist_get(rend_service_list, i); - tor_log(severity, LD_GENERAL, "Service configured in \"%s\":", - service->directory); + tor_log(severity, LD_GENERAL, "Service configured in %s:", + rend_service_escaped_dir(service)); for (j=0; j < smartlist_len(service->intro_nodes); ++j) { intro = smartlist_get(service->intro_nodes, j); safe_name = safe_str_client(intro->extend_info->nickname);

1 0

[tor/master] fix wide lines
by nickm＠torproject.org 03 Nov '16

03 Nov '16

commit df2615d43da4578b7fb68eb753cc1c0d943642af Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Nov 3 14:29:18 2016 -0400 fix wide lines --- src/or/rendservice.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 1584691..305beb1 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -519,7 +519,8 @@ rend_config_services(const or_options_t *options, int validate_only) } log_info(LD_CONFIG, "HiddenServiceDirGroupReadable=%d for %s", - service->dir_group_readable, rend_service_escaped_dir(service)); + service->dir_group_readable, + rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { service->max_streams_per_circuit = (int)tor_parse_long(line->value, 10, 0, 65535, &ok, NULL); @@ -532,7 +533,8 @@ rend_config_services(const or_options_t *options, int validate_only) } log_info(LD_CONFIG, "HiddenServiceMaxStreams=%d for %s", - service->max_streams_per_circuit, rend_service_escaped_dir(service)); + service->max_streams_per_circuit, + rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { service->max_streams_close_circuit = (int)tor_parse_long(line->value, 10, 0, 1, &ok, NULL); @@ -563,7 +565,8 @@ rend_config_services(const or_options_t *options, int validate_only) return -1; } log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", - service->n_intro_points_wanted, rend_service_escaped_dir(service)); + service->n_intro_points_wanted, + rend_service_escaped_dir(service)); } else if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) { /* Parse auth type and comma-separated list of client names and add a * rend_authorized_client_t for each client to the service's list

1 0