August 2017 - tor-commits - lists.torproject.org

[tor/maint-0.3.1] Merge branch 'bug19418_029' into maint-0.3.1
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 6069c829f90576c04a0802d2a108378a2bbabb7a Merge: 10b242822 418f3d629 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 25 11:38:24 2017 -0400 Merge branch 'bug19418_029' into maint-0.3.1 changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-)

1 0

[tor/master] Merge branch 'bug19418_029' into maint-0.3.1
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 6069c829f90576c04a0802d2a108378a2bbabb7a Merge: 10b242822 418f3d629 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 25 11:38:24 2017 -0400 Merge branch 'bug19418_029' into maint-0.3.1 changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-)

1 0

[tor/master] Merge branch 'maint-0.3.1'
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit b91dce9454c3db669a07968b25a4fa03d8526917 Merge: 0de3147bf 6069c829f Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 25 11:39:38 2017 -0400 Merge branch 'maint-0.3.1' changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-)

1 0

[tor/master] Make sure we always wind up checking i2d_*'s output.
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 418f3d6298beb27e050618e2f59e01d6d3b2f45b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 9 09:24:16 2017 -0400 Make sure we always wind up checking i2d_*'s output. The biggest offender here was sometimes not checking the output of crypto_pk_get_digest. Fixes bug 19418. Reported by Guido Vranken. --- changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-) diff --git a/changes/bug19418 b/changes/bug19418 new file mode 100644 index 000000000..fb5f6ad5d --- /dev/null +++ b/changes/bug19418 @@ -0,0 +1,7 @@ + o Minor bugfixes (robustness, error handling): + - Improve our handling of the cases where OpenSSL encounters a + memory error while encoding keys and certificates. We haven't + observed these happening in the wild, but if they do happen, + we now detect and respond better. Fixes bug 19418; bugfix + on all versions of Tor. Reported by Guido Vranken. + diff --git a/src/common/tortls.c b/src/common/tortls.c index d61cc2e58..bd41e0b7f 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -648,12 +648,7 @@ MOCK_IMPL(STATIC tor_x509_cert_t *, length = i2d_X509(x509_cert, &buf); cert = tor_malloc_zero(sizeof(tor_x509_cert_t)); if (length <= 0 || buf == NULL) { - /* LCOV_EXCL_START for the same reason as the exclusion above */ - tor_free(cert); - log_err(LD_CRYPTO, "Couldn't get length of encoded x509 certificate"); - X509_free(x509_cert); - return NULL; - /* LCOV_EXCL_STOP */ + goto err; } cert->encoded_len = (size_t) length; cert->encoded = tor_malloc(length); @@ -668,13 +663,25 @@ MOCK_IMPL(STATIC tor_x509_cert_t *, if ((pkey = X509_get_pubkey(x509_cert)) && (rsa = EVP_PKEY_get1_RSA(pkey))) { crypto_pk_t *pk = crypto_new_pk_from_rsa_(rsa); - crypto_pk_get_common_digests(pk, &cert->pkey_digests); + if (crypto_pk_get_common_digests(pk, &cert->pkey_digests) < 0) { + crypto_pk_free(pk); + EVP_PKEY_free(pkey); + goto err; + } + cert->pkey_digests_set = 1; crypto_pk_free(pk); EVP_PKEY_free(pkey); } return cert; + err: + /* LCOV_EXCL_START for the same reason as the exclusion above */ + tor_free(cert); + log_err(LD_CRYPTO, "Couldn't wrap encoded X509 certificate."); + X509_free(x509_cert); + return NULL; + /* LCOV_EXCL_STOP */ } /** Return a new copy of cert. */ diff --git a/src/or/connection_or.c b/src/or/connection_or.c index dadfdc438..49d48d72e 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1552,7 +1552,9 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, } if (identity_rcvd) { - crypto_pk_get_digest(identity_rcvd, digest_rcvd_out); + if (crypto_pk_get_digest(identity_rcvd, digest_rcvd_out) < 0) { + return -1; + } } else { memset(digest_rcvd_out, 0, DIGEST_LEN); } diff --git a/src/or/hibernate.c b/src/or/hibernate.c index e3c80b5f1..82196ef2c 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -587,7 +587,10 @@ accounting_set_wakeup_time(void) char buf[ISO_TIME_LEN+1]; format_iso_time(buf, interval_start_time); - crypto_pk_get_digest(get_server_identity_key(), digest); + if (crypto_pk_get_digest(get_server_identity_key(), digest) < 0) { + log_err(LD_BUG, "Error getting our key's digest."); + tor_assert(0); + } d_env = crypto_digest_new(); crypto_digest_add_bytes(d_env, buf, ISO_TIME_LEN); diff --git a/src/or/rendclient.c b/src/or/rendclient.c index a93bc94a9..e54818c54 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -263,6 +263,11 @@ rend_client_send_introduction(origin_circuit_t *introcirc, klen = crypto_pk_asn1_encode(extend_info->onion_key, tmp+v3_shift+7+DIGEST_LEN+2, sizeof(tmp)-(v3_shift+7+DIGEST_LEN+2)); + if (klen < 0) { + log_warn(LD_BUG,"Internal error: can't encode public key."); + status = -2; + goto perm_err; + } set_uint16(tmp+v3_shift+7+DIGEST_LEN, htons(klen)); memcpy(tmp+v3_shift+7+DIGEST_LEN+2+klen, rendcirc->rend_data->rend_cookie, REND_COOKIE_LEN); diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index d9d39b1f1..c01ce8f1d 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -475,7 +475,10 @@ rend_encode_v2_descriptors(smartlist_t *descs_out, tor_assert(descriptor_cookie); } /* Obtain service_id from public key. */ - crypto_pk_get_digest(service_key, service_id); + if (crypto_pk_get_digest(service_key, service_id) < 0) { + log_warn(LD_BUG, "Couldn't compute service key digest."); + return -1; + } /* Calculate current time-period. */ time_period = get_time_period(now, period, service_id); /* Determine how many seconds the descriptor will be valid. */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 4d04da02a..d4441b63c 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2689,7 +2689,14 @@ rend_service_decrypt_intro( /* Check that this cell actually matches this service key */ /* first DIGEST_LEN bytes of request is intro or service pk digest */ - crypto_pk_get_digest(key, (char *)key_digest); + if (crypto_pk_get_digest(key, (char *)key_digest) < 0) { + if (err_msg_out) + *err_msg_out = tor_strdup("Couldn't compute RSA digest."); + log_warn(LD_BUG, "Couldn't compute key digest."); + status = -7; + goto err; + } + if (tor_memneq(key_digest, intro->pk, DIGEST_LEN)) { if (err_msg_out) { base32_encode(service_id, REND_SERVICE_ID_LEN_BASE32 + 1, diff --git a/src/or/router.c b/src/or/router.c index 6d3a32a60..f7528c22d 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -212,7 +212,11 @@ set_server_identity_key(crypto_pk_t *k) { crypto_pk_free(server_identitykey); server_identitykey = k; - crypto_pk_get_digest(server_identitykey, server_identitykey_digest); + if (crypto_pk_get_digest(server_identitykey, + server_identitykey_digest) < 0) { + log_err(LD_BUG, "Couldn't compute our own identity key digest."); + tor_assert(0); + } } /** Make sure that we have set up our identity keys to match or not match as @@ -871,8 +875,12 @@ init_keys(void) } cert = get_my_v3_authority_cert(); if (cert) { - crypto_pk_get_digest(get_my_v3_authority_cert()->identity_key, - v3_digest); + if (crypto_pk_get_digest(get_my_v3_authority_cert()->identity_key, + v3_digest) < 0) { + log_err(LD_BUG, "Couldn't compute my v3 authority identity key " + "digest."); + return -1; + } v3_digest_set = 1; } } diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c index ca32228fc..5a41d6261 100644 --- a/src/or/routerkeys.c +++ b/src/or/routerkeys.c @@ -1079,7 +1079,9 @@ make_tap_onion_key_crosscert(const crypto_pk_t *onion_key, uint8_t signed_data[DIGEST_LEN + ED25519_PUBKEY_LEN]; *len_out = 0; - crypto_pk_get_digest(rsa_id_key, (char*)signed_data); + if (crypto_pk_get_digest(rsa_id_key, (char*)signed_data) < 0) { + return NULL; + } memcpy(signed_data + DIGEST_LEN, master_id_key->pubkey, ED25519_PUBKEY_LEN); int r = crypto_pk_private_sign(onion_key, diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 521e237be..bfd027043 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5967,7 +5967,10 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out, "v2 rendezvous service descriptor") < 0) goto err; /* Verify that descriptor ID belongs to public key and secret ID part. */ - crypto_pk_get_digest(result->pk, public_key_hash); + if (crypto_pk_get_digest(result->pk, public_key_hash) < 0) { + log_warn(LD_REND, "Unable to compute rend descriptor public key digest"); + goto err; + } rend_get_descriptor_id_bytes(test_desc_id, public_key_hash, secret_id_part); if (tor_memneq(desc_id_out, test_desc_id, DIGEST_LEN)) {

1 0

[tor/release-0.3.1] Merge branch 'bug19418_029' into maint-0.3.1
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 6069c829f90576c04a0802d2a108378a2bbabb7a Merge: 10b242822 418f3d629 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 25 11:38:24 2017 -0400 Merge branch 'bug19418_029' into maint-0.3.1 changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-)

1 0

[tor/release-0.3.1] Make sure we always wind up checking i2d_*'s output.
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 418f3d6298beb27e050618e2f59e01d6d3b2f45b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 9 09:24:16 2017 -0400 Make sure we always wind up checking i2d_*'s output. The biggest offender here was sometimes not checking the output of crypto_pk_get_digest. Fixes bug 19418. Reported by Guido Vranken. --- changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-) diff --git a/changes/bug19418 b/changes/bug19418 new file mode 100644 index 000000000..fb5f6ad5d --- /dev/null +++ b/changes/bug19418 @@ -0,0 +1,7 @@ + o Minor bugfixes (robustness, error handling): + - Improve our handling of the cases where OpenSSL encounters a + memory error while encoding keys and certificates. We haven't + observed these happening in the wild, but if they do happen, + we now detect and respond better. Fixes bug 19418; bugfix + on all versions of Tor. Reported by Guido Vranken. + diff --git a/src/common/tortls.c b/src/common/tortls.c index d61cc2e58..bd41e0b7f 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -648,12 +648,7 @@ MOCK_IMPL(STATIC tor_x509_cert_t *, length = i2d_X509(x509_cert, &buf); cert = tor_malloc_zero(sizeof(tor_x509_cert_t)); if (length <= 0 || buf == NULL) { - /* LCOV_EXCL_START for the same reason as the exclusion above */ - tor_free(cert); - log_err(LD_CRYPTO, "Couldn't get length of encoded x509 certificate"); - X509_free(x509_cert); - return NULL; - /* LCOV_EXCL_STOP */ + goto err; } cert->encoded_len = (size_t) length; cert->encoded = tor_malloc(length); @@ -668,13 +663,25 @@ MOCK_IMPL(STATIC tor_x509_cert_t *, if ((pkey = X509_get_pubkey(x509_cert)) && (rsa = EVP_PKEY_get1_RSA(pkey))) { crypto_pk_t *pk = crypto_new_pk_from_rsa_(rsa); - crypto_pk_get_common_digests(pk, &cert->pkey_digests); + if (crypto_pk_get_common_digests(pk, &cert->pkey_digests) < 0) { + crypto_pk_free(pk); + EVP_PKEY_free(pkey); + goto err; + } + cert->pkey_digests_set = 1; crypto_pk_free(pk); EVP_PKEY_free(pkey); } return cert; + err: + /* LCOV_EXCL_START for the same reason as the exclusion above */ + tor_free(cert); + log_err(LD_CRYPTO, "Couldn't wrap encoded X509 certificate."); + X509_free(x509_cert); + return NULL; + /* LCOV_EXCL_STOP */ } /** Return a new copy of cert. */ diff --git a/src/or/connection_or.c b/src/or/connection_or.c index dadfdc438..49d48d72e 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1552,7 +1552,9 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, } if (identity_rcvd) { - crypto_pk_get_digest(identity_rcvd, digest_rcvd_out); + if (crypto_pk_get_digest(identity_rcvd, digest_rcvd_out) < 0) { + return -1; + } } else { memset(digest_rcvd_out, 0, DIGEST_LEN); } diff --git a/src/or/hibernate.c b/src/or/hibernate.c index e3c80b5f1..82196ef2c 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -587,7 +587,10 @@ accounting_set_wakeup_time(void) char buf[ISO_TIME_LEN+1]; format_iso_time(buf, interval_start_time); - crypto_pk_get_digest(get_server_identity_key(), digest); + if (crypto_pk_get_digest(get_server_identity_key(), digest) < 0) { + log_err(LD_BUG, "Error getting our key's digest."); + tor_assert(0); + } d_env = crypto_digest_new(); crypto_digest_add_bytes(d_env, buf, ISO_TIME_LEN); diff --git a/src/or/rendclient.c b/src/or/rendclient.c index a93bc94a9..e54818c54 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -263,6 +263,11 @@ rend_client_send_introduction(origin_circuit_t *introcirc, klen = crypto_pk_asn1_encode(extend_info->onion_key, tmp+v3_shift+7+DIGEST_LEN+2, sizeof(tmp)-(v3_shift+7+DIGEST_LEN+2)); + if (klen < 0) { + log_warn(LD_BUG,"Internal error: can't encode public key."); + status = -2; + goto perm_err; + } set_uint16(tmp+v3_shift+7+DIGEST_LEN, htons(klen)); memcpy(tmp+v3_shift+7+DIGEST_LEN+2+klen, rendcirc->rend_data->rend_cookie, REND_COOKIE_LEN); diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index d9d39b1f1..c01ce8f1d 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -475,7 +475,10 @@ rend_encode_v2_descriptors(smartlist_t *descs_out, tor_assert(descriptor_cookie); } /* Obtain service_id from public key. */ - crypto_pk_get_digest(service_key, service_id); + if (crypto_pk_get_digest(service_key, service_id) < 0) { + log_warn(LD_BUG, "Couldn't compute service key digest."); + return -1; + } /* Calculate current time-period. */ time_period = get_time_period(now, period, service_id); /* Determine how many seconds the descriptor will be valid. */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 4d04da02a..d4441b63c 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2689,7 +2689,14 @@ rend_service_decrypt_intro( /* Check that this cell actually matches this service key */ /* first DIGEST_LEN bytes of request is intro or service pk digest */ - crypto_pk_get_digest(key, (char *)key_digest); + if (crypto_pk_get_digest(key, (char *)key_digest) < 0) { + if (err_msg_out) + *err_msg_out = tor_strdup("Couldn't compute RSA digest."); + log_warn(LD_BUG, "Couldn't compute key digest."); + status = -7; + goto err; + } + if (tor_memneq(key_digest, intro->pk, DIGEST_LEN)) { if (err_msg_out) { base32_encode(service_id, REND_SERVICE_ID_LEN_BASE32 + 1, diff --git a/src/or/router.c b/src/or/router.c index 6d3a32a60..f7528c22d 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -212,7 +212,11 @@ set_server_identity_key(crypto_pk_t *k) { crypto_pk_free(server_identitykey); server_identitykey = k; - crypto_pk_get_digest(server_identitykey, server_identitykey_digest); + if (crypto_pk_get_digest(server_identitykey, + server_identitykey_digest) < 0) { + log_err(LD_BUG, "Couldn't compute our own identity key digest."); + tor_assert(0); + } } /** Make sure that we have set up our identity keys to match or not match as @@ -871,8 +875,12 @@ init_keys(void) } cert = get_my_v3_authority_cert(); if (cert) { - crypto_pk_get_digest(get_my_v3_authority_cert()->identity_key, - v3_digest); + if (crypto_pk_get_digest(get_my_v3_authority_cert()->identity_key, + v3_digest) < 0) { + log_err(LD_BUG, "Couldn't compute my v3 authority identity key " + "digest."); + return -1; + } v3_digest_set = 1; } } diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c index ca32228fc..5a41d6261 100644 --- a/src/or/routerkeys.c +++ b/src/or/routerkeys.c @@ -1079,7 +1079,9 @@ make_tap_onion_key_crosscert(const crypto_pk_t *onion_key, uint8_t signed_data[DIGEST_LEN + ED25519_PUBKEY_LEN]; *len_out = 0; - crypto_pk_get_digest(rsa_id_key, (char*)signed_data); + if (crypto_pk_get_digest(rsa_id_key, (char*)signed_data) < 0) { + return NULL; + } memcpy(signed_data + DIGEST_LEN, master_id_key->pubkey, ED25519_PUBKEY_LEN); int r = crypto_pk_private_sign(onion_key, diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 521e237be..bfd027043 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5967,7 +5967,10 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out, "v2 rendezvous service descriptor") < 0) goto err; /* Verify that descriptor ID belongs to public key and secret ID part. */ - crypto_pk_get_digest(result->pk, public_key_hash); + if (crypto_pk_get_digest(result->pk, public_key_hash) < 0) { + log_warn(LD_REND, "Unable to compute rend descriptor public key digest"); + goto err; + } rend_get_descriptor_id_bytes(test_desc_id, public_key_hash, secret_id_part); if (tor_memneq(desc_id_out, test_desc_id, DIGEST_LEN)) {

1 0

[tor/release-0.3.1] Merge branch 'maint-0.3.1' into release-0.3.1
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 1dd0be75c2119ab5265725641778bf4d77df4d60 Merge: 882dd4de0 6069c829f Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 25 11:39:38 2017 -0400 Merge branch 'maint-0.3.1' into release-0.3.1 changes/bug19418 | 7 +++++++ src/common/tortls.c | 21 ++++++++++++++------- src/or/connection_or.c | 4 +++- src/or/hibernate.c | 5 ++++- src/or/rendclient.c | 5 +++++ src/or/rendcommon.c | 5 ++++- src/or/rendservice.c | 9 ++++++++- src/or/router.c | 14 +++++++++++--- src/or/routerkeys.c | 4 +++- src/or/routerparse.c | 5 ++++- 10 files changed, 63 insertions(+), 16 deletions(-)

1 0

[tor/master] fix wide lines
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 0de3147bf1b7a76b99ced69bde6e2169bb9a0caa Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 25 11:34:42 2017 -0400 fix wide lines --- src/or/hs_service.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 04248eaa0..a207fe331 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -1761,7 +1761,7 @@ rotate_all_descriptors(time_t now) int service_entered_overlap = service_just_entered_overlap_mode(service, overlap_mode_is_active); int service_left_overlap = service_just_left_overlap_mode(service, - overlap_mode_is_active); + overlap_mode_is_active); /* This should not be possible */ if (BUG(service_entered_overlap && service_left_overlap)) { return; @@ -1785,10 +1785,10 @@ rotate_all_descriptors(time_t now) } if (service_entered_overlap) { - /* We just entered overlap period: recompute all HSDir indices. We need to - * do this otherwise nodes can get stuck with old HSDir indices until we - * fetch a new consensus, and we might need to reupload our desc before - * that. */ + /* We just entered overlap period: recompute all HSDir indices. We need + * to do this otherwise nodes can get stuck with old HSDir indices until + * we fetch a new consensus, and we might need to reupload our desc + * before that. */ /* XXX find a better place than rotate_all_descriptors() to do this */ nodelist_recompute_all_hsdir_indices(); }

1 0

[tor/master] prop224: Refactor descriptor rotation logic.
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit 8b8e39e04b365eefd7d69a488c04405cd6371645 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 24 16:16:44 2017 +0300 prop224: Refactor descriptor rotation logic. The problem was that when we went from overlap mode to non-overlap mode, we were not wiping the 'desc_next' descriptor and instead we left it on the service. This meant that all functions that iterated service descriptors were also inspecting the useless 'desc_next' descriptor that should have been deleted. This commit refactors rotate_all_descriptors() so that it rotates descriptor both when entering overlap mode and also when leaving it. --- src/or/hs_service.c | 90 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 0d0db1cd6..e530a10a1 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -1714,10 +1714,35 @@ rotate_service_descriptors(hs_service_t *service) service->desc_next = NULL; } -/* Rotate descriptors for each service if needed. If we are just entering - * the overlap period, rotate them that is point the previous descriptor to - * the current and cleanup the previous one. A non existing current - * descriptor will trigger a descriptor build for the next time period. */ +/** Return true if service **just** entered overlap mode. */ +static int +service_just_entered_overlap_mode(const hs_service_t *service, + int overlap_mode_is_active) +{ + if (overlap_mode_is_active && !service->state.in_overlap_period) { + return 1; + } + + return 0; +} + +/** Return true if service **just** left overlap mode. */ +static int +service_just_left_overlap_mode(const hs_service_t *service, + int overlap_mode_is_active) +{ + if (!overlap_mode_is_active && service->state.in_overlap_period) { + return 1; + } + + return 0; +} + +/* Rotate descriptors for each service if needed. If we are just entering or + * leaving the overlap period, rotate them that is point the previous + * descriptor to the current and cleanup the previous one. A non existing + * current descriptor will trigger a descriptor build for the next time + * period. */ STATIC void rotate_all_descriptors(time_t now) { @@ -1725,35 +1750,56 @@ rotate_all_descriptors(time_t now) * be wise, to rotate service descriptors independently to hide that all * those descriptors are on the same tor instance */ + int overlap_mode_is_active = hs_overlap_mode_is_active(NULL, now); + FOR_EACH_SERVICE_BEGIN(service) { - /* We are _not_ in the overlap period so skip rotation. */ - if (!hs_overlap_mode_is_active(NULL, now)) { + int service_entered_overlap = service_just_entered_overlap_mode(service, + overlap_mode_is_active); + int service_left_overlap = service_just_left_overlap_mode(service, + overlap_mode_is_active); + /* This should not be possible */ + if (BUG(service_entered_overlap && service_left_overlap)) { + return; + } + + /* No changes in overlap mode: nothing to do here */ + if (!service_entered_overlap && !service_left_overlap) { + return; + } + + /* To get down there means that some change happened to overlap mode */ + tor_assert(service_entered_overlap || service_left_overlap); + + /* Update the overlap marks on this service */ + if (service_entered_overlap) { + /* It's the first time the service encounters the overlap period so flag + * it in order to make sure we don't rotate at next check. */ + service->state.in_overlap_period = 1; + } else if (service_left_overlap) { service->state.in_overlap_period = 0; - continue; } - /* We've entered the overlap period already so skip rotation. */ - if (service->state.in_overlap_period) { - continue; + + if (service_entered_overlap) { + /* We just entered overlap period: recompute all HSDir indices. We need to + * do this otherwise nodes can get stuck with old HSDir indices until we + * fetch a new consensus, and we might need to reupload our desc before + * that. */ + /* XXX find a better place than rotate_all_descriptors() to do this */ + nodelist_recompute_all_hsdir_indices(); } - /* It's the first time the service encounters the overlap period so flag - * it in order to make sure we don't rotate at next check. */ - service->state.in_overlap_period = 1; - /* We just entered overlap period: recompute all HSDir indices. We need to - * do this otherwise nodes can get stuck with old HSDir indices until we - * fetch a new consensus, and we might need to reupload our desc before - * that. */ - /* XXX find a better place than rotate_all_descriptors() to do this */ - nodelist_recompute_all_hsdir_indices(); + /* If we just entered or left overlap mode, rotate our descriptors */ + log_info(LD_REND, "We just %s overlap period. About to rotate %s " + "descriptors (%p / %p).", + service_entered_overlap ? "entered" : "left", + safe_str_client(service->onion_address), + service->desc_current, service->desc_next); /* If we have a next descriptor lined up, rotate the descriptors so that it * becomes current. */ if (service->desc_next) { rotate_service_descriptors(service); } - log_info(LD_REND, "We've just entered the overlap period. Service %s " - "descriptors have been rotated!", - safe_str_client(service->onion_address)); } FOR_EACH_SERVICE_END; }

1 0

[tor/master] prop224: Move service_desc_hsdirs_changed() and make it static.
by nickm＠torproject.org 25 Aug '17

25 Aug '17

commit e07b677bd9d557d2ab15474ce96c825a5a034a7f Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Aug 24 19:32:33 2017 +0300 prop224: Move service_desc_hsdirs_changed() and make it static. That function could be static but needed to be moved to the top. --- src/or/hs_service.c | 94 ++++++++++++++++++++++++++--------------------------- src/or/hs_service.h | 7 ++-- 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 0fc5a9144..04248eaa0 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2360,6 +2360,53 @@ upload_descriptor_to_all(const hs_service_t *service, return; } +/** The set of HSDirs have changed: check if the change affects our descriptor + * HSDir placement, and if it does, reupload the desc. */ +STATIC int +service_desc_hsdirs_changed(const hs_service_t *service, + const hs_service_descriptor_t *desc) +{ + int retval = 0; + smartlist_t *responsible_dirs = smartlist_new(); + smartlist_t *b64_responsible_dirs = smartlist_new(); + + /* No desc upload has happened yet: it will happen eventually */ + if (!desc->previous_hsdirs || !smartlist_len(desc->previous_hsdirs)) { + goto done; + } + + /* Get list of responsible hsdirs */ + hs_get_responsible_hsdirs(&desc->blinded_kp.pubkey, desc->time_period_num, + service->desc_next == desc, 0, responsible_dirs); + + /* Make a second list with their b64ed identity digests, so that we can + * compare it with out previous list of hsdirs */ + SMARTLIST_FOREACH_BEGIN(responsible_dirs, const routerstatus_t *, hsdir_rs) { + char b64_digest[BASE64_DIGEST_LEN+1] = {0}; + digest_to_base64(b64_digest, hsdir_rs->identity_digest); + smartlist_add_strdup(b64_responsible_dirs, b64_digest); + } SMARTLIST_FOREACH_END(hsdir_rs); + + /* Sort this new smartlist so that we can compare it with the other one */ + smartlist_sort_strings(b64_responsible_dirs); + + /* Check whether the set of HSDirs changed */ + if (!smartlist_strings_eq(b64_responsible_dirs, desc->previous_hsdirs)) { + log_warn(LD_GENERAL, "Received new dirinfo and set of hsdirs changed!"); + retval = 1; + } else { + log_warn(LD_GENERAL, "No change in hsdir set!"); + } + + done: + smartlist_free(responsible_dirs); + + SMARTLIST_FOREACH(b64_responsible_dirs, char*, s, tor_free(s)); + smartlist_free(b64_responsible_dirs); + + return retval; +} + /* Return 1 if the given descriptor from the given service can be uploaded * else return 0 if it can not. */ static int @@ -2751,53 +2798,6 @@ service_add_fnames_to_list(const hs_service_t *service, smartlist_t *list) smartlist_add(list, hs_path_from_filename(s_dir, fname)); } -/** The set of HSDirs have changed: check if the change affects our descriptor - * HSDir placement, and if it does, reupload the desc. */ -int -service_desc_hsdirs_changed(const hs_service_t *service, - const hs_service_descriptor_t *desc) -{ - int retval = 0; - smartlist_t *responsible_dirs = smartlist_new(); - smartlist_t *b64_responsible_dirs = smartlist_new(); - - /* No desc upload has happened yet: it will happen eventually */ - if (!desc->previous_hsdirs || !smartlist_len(desc->previous_hsdirs)) { - goto done; - } - - /* Get list of responsible hsdirs */ - hs_get_responsible_hsdirs(&desc->blinded_kp.pubkey, desc->time_period_num, - service->desc_next == desc, 0, responsible_dirs); - - /* Make a second list with their b64ed identity digests, so that we can - * compare it with out previous list of hsdirs */ - SMARTLIST_FOREACH_BEGIN(responsible_dirs, const routerstatus_t *, hsdir_rs) { - char b64_digest[BASE64_DIGEST_LEN+1] = {0}; - digest_to_base64(b64_digest, hsdir_rs->identity_digest); - smartlist_add_strdup(b64_responsible_dirs, b64_digest); - } SMARTLIST_FOREACH_END(hsdir_rs); - - /* Sort this new smartlist so that we can compare it with the other one */ - smartlist_sort_strings(b64_responsible_dirs); - - /* Check whether the set of HSDirs changed */ - if (!smartlist_strings_eq(b64_responsible_dirs, desc->previous_hsdirs)) { - log_info(LD_GENERAL, "Received new dirinfo and set of hsdirs changed!"); - retval = 1; - } else { - log_debug(LD_GENERAL, "No change in hsdir set!"); - } - - done: - smartlist_free(responsible_dirs); - - SMARTLIST_FOREACH(b64_responsible_dirs, char*, s, tor_free(s)); - smartlist_free(b64_responsible_dirs); - - return retval; -} - /* ========== */ /* Public API */ /* ========== */ diff --git a/src/or/hs_service.h b/src/or/hs_service.h index d902e0b2c..57717fc92 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -280,10 +280,6 @@ int hs_service_receive_introduce2(origin_circuit_t *circ, void hs_service_intro_circ_has_closed(origin_circuit_t *circ); -int service_desc_hsdirs_changed(const hs_service_t *service, - const hs_service_descriptor_t *desc); - - #ifdef HS_SERVICE_PRIVATE #ifdef TOR_UNIT_TESTS @@ -357,6 +353,9 @@ STATIC void service_desc_schedule_upload(hs_service_descriptor_t *desc, time_t now, int descriptor_changed); +STATIC int service_desc_hsdirs_changed(const hs_service_t *service, + const hs_service_descriptor_t *desc); + #endif /* TOR_UNIT_TESTS */ #endif /* HS_SERVICE_PRIVATE */

1 0