From 14b1c7a66e6d186a40da99da773a692ef48b603c Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Tue, 28 Jun 2016 14:12:18 +1000 Subject: [PATCH 1/4] Refactor connection_or_client_learned_peer_id for bug18812 No behavioural change. Also clarify some comments. --- src/or/connection_or.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index ea49bdba7..19a171820 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1698,9 +1698,14 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, * or renegotiation. For v3 handshakes, this is right after we get a * certificate chain in a CERTS cell. * - * If we want any particular ID before, record the one we got. + * If we did not know the ID before, record the one we got. * - * If we wanted an ID, but we didn't get it, log a warning and return -1. + * If we wanted an ID, but we didn't get the one we expected, log a message + * and return -1. + * On relays: + * - log a protocol warning whenever the fingerprints don't match; + * On clients: + * - if a relay's fingerprint doesn't match, log a warning; * * If we're testing reachability, remember what we learned. * @@ -1711,7 +1716,6 @@ connection_or_client_learned_peer_id(or_connection_t *conn, const uint8_t *peer_id) { const or_options_t *options = get_options(); - int severity = server_mode(options) ? LOG_PROTOCOL_WARN : LOG_WARN; if (tor_digest_is_zero(conn->identity_digest)) { connection_or_set_identity_digest(conn, (const char*)peer_id); @@ -1736,6 +1740,15 @@ connection_or_client_learned_peer_id(or_connection_t *conn, base16_encode(seen, sizeof(seen), (const char*)peer_id, DIGEST_LEN); base16_encode(expected, sizeof(expected), conn->identity_digest, DIGEST_LEN); + int severity; + + if (server_mode(options)) { + severity = LOG_PROTOCOL_WARN; + } else { + /* a relay has changed its fingerprint from the one in the consensus */ + severity = LOG_WARN; + } + log_fn(severity, LD_HANDSHAKE, "Tried connecting to router at %s:%d, but identity key was not " "as expected: wanted %s but got %s.", From 812fd416eff4fa7326cbd4bd46ff0f5801b9034c Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Tue, 28 Jun 2016 14:14:04 +1000 Subject: [PATCH 2/4] Make it clear that fallbacks include authorities Comment-only change --- src/or/routerlist.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index d49814f05..d7faf7882 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1384,6 +1384,7 @@ router_get_trusteddirserver_by_digest(const char *digest) * key hashes to digest, or NULL if no such fallback is in the list of * fallback_dir_servers. (fallback_dir_servers is affected by the FallbackDir * and UseDefaultFallbackDirs torrc options.) + * The list of fallback directories includes the list of authorities. */ dir_server_t * router_get_fallback_dirserver_by_digest(const char *digest) @@ -1407,6 +1408,7 @@ router_get_fallback_dirserver_by_digest(const char *digest) * or 0 if no such fallback is in the list of fallback_dir_servers. * (fallback_dir_servers is affected by the FallbackDir and * UseDefaultFallbackDirs torrc options.) + * The list of fallback directories includes the list of authorities. */ int router_digest_is_fallback_dir(const char *digest) From 608c12baaf820c33246052e23fd0c65459ed1c5c Mon Sep 17 00:00:00 2001 From: "teor (Tim Wilson-Brown)" Date: Tue, 28 Jun 2016 14:15:11 +1000 Subject: [PATCH 3/4] Resolve bug18812 by logging fallback key changes at info level --- changes/bug18812 | 4 ++++ src/or/connection_or.c | 36 ++++++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 changes/bug18812 diff --git a/changes/bug18812 b/changes/bug18812 new file mode 100644 index 000000000..793e1102f --- /dev/null +++ b/changes/bug18812 @@ -0,0 +1,4 @@ + o Minor bugfixes (bootstrap): + - When a fallback changes its fingerprint from the hard-coded + fingerprint, log a less severe, more explanatory log message. + Fixes bug 18812; bugfix on 0.2.8.1-alpha. Patch by teor. diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 19a171820..5c4461f59 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -31,6 +31,7 @@ #include "geoip.h" #include "main.h" #include "link_handshake.h" +#include "microdesc.h" #include "networkstatus.h" #include "nodelist.h" #include "reasons.h" @@ -1706,6 +1707,9 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn, * - log a protocol warning whenever the fingerprints don't match; * On clients: * - if a relay's fingerprint doesn't match, log a warning; + * - if we don't have updated relay fingerprints from a recent consensus, and + * a fallback directory mirror's hard-coded fingerprint has changed, log an + * info explaining that we will try another fallback. * * If we're testing reachability, remember what we learned. * @@ -1740,19 +1744,43 @@ connection_or_client_learned_peer_id(or_connection_t *conn, base16_encode(seen, sizeof(seen), (const char*)peer_id, DIGEST_LEN); base16_encode(expected, sizeof(expected), conn->identity_digest, DIGEST_LEN); + const int using_hardcoded_fingerprints = + !networkstatus_get_reasonably_live_consensus(time(NULL), + usable_consensus_flavor()); + const int is_fallback_fingerprint = router_digest_is_fallback_dir( + conn->identity_digest); + const int is_authority_fingerprint = router_digest_is_trusted_dir( + conn->identity_digest); int severity; + const char *extra_log = ""; if (server_mode(options)) { severity = LOG_PROTOCOL_WARN; } else { - /* a relay has changed its fingerprint from the one in the consensus */ - severity = LOG_WARN; + if (using_hardcoded_fingerprints) { + /* We need to do the checks in this order, because the list of + * fallbacks includes the list of authorities */ + if (is_authority_fingerprint) { + severity = LOG_WARN; + } else if (is_fallback_fingerprint) { + /* we expect a small number of fallbacks to change from their + * hard-coded fingerprints over the life of a release */ + severity = LOG_INFO; + extra_log = " Tor will try a different fallback."; + } else { + /* it's a bridge, it's either a misconfiguration, or unexpected */ + severity = LOG_WARN; + } + } else { + /* a relay has changed its fingerprint from the one in the consensus */ + severity = LOG_WARN; + } } log_fn(severity, LD_HANDSHAKE, "Tried connecting to router at %s:%d, but identity key was not " - "as expected: wanted %s but got %s.", - conn->base_.address, conn->base_.port, expected, seen); + "as expected: wanted %s but got %s.%s", + conn->base_.address, conn->base_.port, expected, seen, extra_log); entry_guard_register_connect_status(conn->identity_digest, 0, 1, time(NULL)); control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, From bc9a0f82b3436dad2aa09c1b68c0c230751d48dd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 28 Jun 2016 11:14:42 -0400 Subject: [PATCH 4/4] whitespace fixes --- src/or/connection_or.c | 2 +- src/or/routerlist.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 5c4461f59..9730e1a95 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1750,7 +1750,7 @@ connection_or_client_learned_peer_id(or_connection_t *conn, const int is_fallback_fingerprint = router_digest_is_fallback_dir( conn->identity_digest); const int is_authority_fingerprint = router_digest_is_trusted_dir( - conn->identity_digest); + conn->identity_digest); int severity; const char *extra_log = ""; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 432c64d53..49d76807b 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1408,7 +1408,7 @@ router_get_fallback_dirserver_by_digest(const char *digest) * or 0 if no such fallback is in the list of fallback_dir_servers. * (fallback_dir_servers is affected by the FallbackDir and * UseDefaultFallbackDirs torrc options.) - * The list of fallback directories includes the list of authorities. + * The list of fallback directories includes the list of authorities. */ int router_digest_is_fallback_dir(const char *digest)