diff --git a/changes/prettier-signature-log b/changes/prettier-signature-log
new file mode 100644
index 0000000000..c008514fb9
--- /dev/null
+++ b/changes/prettier-signature-log
@@ -0,0 +1,9 @@
+ o Minor features
+ - Make the formerly ugly "2 unknown, 7 missing key, 0 good, 0 bad,
+ 2 no signature, 4 required" messages easier to read, and make sure
+ they get logged at the same severity as the messages explaining
+ which keys are which. Fixes bug 1290.
+ - Don't warn when we have a consensus that we can't verify because
+ of missing certificates, unless those certificates are ones
+ that we have been trying and failing to download. Fixes bug 1145.
+
diff --git a/src/or/directory.c b/src/or/directory.c
index 97ee0c9fbc..ffa312bc41 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -3533,6 +3533,14 @@ download_status_reset(download_status_t *dls)
dls->next_attempt_at = time(NULL) + schedule[0];
}
+/** Return the number of failures on dls since the last success (if
+ * any). */
+int
+download_status_get_n_failures(const download_status_t *dls)
+{
+ return dls->n_download_failures;
+}
+
/** Called when one or more routerdesc (or extrainfo, if was_extrainfo)
* fetches have failed (with uppercase fingerprints listed in failed,
* either as descriptor digests or as identity digests based on
diff --git a/src/or/directory.h b/src/or/directory.h
index 36b4cf2b18..6fd2c0beff 100644
--- a/src/or/directory.h
+++ b/src/or/directory.h
@@ -104,5 +104,7 @@ download_status_mark_impossible(download_status_t *dl)
dl->n_download_failures = IMPOSSIBLE_TO_DOWNLOAD;
}
+int download_status_get_n_failures(const download_status_t *dls);
+
#endif
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index e985679c4e..3ac7b805a0 100644
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -464,7 +464,7 @@ networkstatus_check_consensus_signature(networkstatus_t *consensus,
int warn)
{
int n_good = 0;
- int n_missing_key = 0;
+ int n_missing_key = 0, n_dl_failed_key = 0;
int n_bad = 0;
int n_unknown = 0;
int n_no_signature = 0;
@@ -482,7 +482,7 @@ networkstatus_check_consensus_signature(networkstatus_t *consensus,
voter) {
int good_here = 0;
int bad_here = 0;
- int missing_key_here = 0;
+ int missing_key_here = 0, dl_failed_key_here = 0;
SMARTLIST_FOREACH_BEGIN(voter->sigs, document_signature_t *, sig) {
if (!sig->good_signature && !sig->bad_signature &&
sig->signature) {
@@ -502,11 +502,15 @@ networkstatus_check_consensus_signature(networkstatus_t *consensus,
} else if (!cert || cert->expires < now) {
smartlist_add(need_certs_from, voter);
++missing_key_here;
+ if (authority_cert_dl_looks_uncertain(sig->identity_digest))
+ ++dl_failed_key_here;
continue;
}
if (networkstatus_check_document_signature(consensus, sig, cert) < 0) {
smartlist_add(need_certs_from, voter);
++missing_key_here;
+ if (authority_cert_dl_looks_uncertain(sig->identity_digest))
+ ++dl_failed_key_here;
continue;
}
}
@@ -519,9 +523,11 @@ networkstatus_check_consensus_signature(networkstatus_t *consensus,
++n_good;
else if (bad_here)
++n_bad;
- else if (missing_key_here)
+ else if (missing_key_here) {
++n_missing_key;
- else
+ if (dl_failed_key_here)
+ ++n_dl_failed_key;
+ } else
++n_no_signature;
} SMARTLIST_FOREACH_END(voter);
@@ -534,39 +540,71 @@ networkstatus_check_consensus_signature(networkstatus_t *consensus,
smartlist_add(missing_authorities, ds);
});
- if (warn > 1 || (warn >= 0 && n_good < n_required))
+ if (warn > 1 || (warn >= 0 &&
+ (n_good + n_missing_key - n_dl_failed_key < n_required))) {
severity = LOG_WARN;
- else
+ } else {
severity = LOG_INFO;
+ }
if (warn >= 0) {
SMARTLIST_FOREACH(unrecognized, networkstatus_voter_info_t *, voter,
{
- log_info(LD_DIR, "Consensus includes unrecognized authority '%s' "
- "at %s:%d (contact %s; identity %s)",
+ log(severity, LD_DIR, "Consensus includes unrecognized authority "
+ "'%s' at %s:%d (contact %s; identity %s)",
voter->nickname, voter->address, (int)voter->dir_port,
voter->contact?voter->contact:"n/a",
hex_str(voter->identity_digest, DIGEST_LEN));
});
SMARTLIST_FOREACH(need_certs_from, networkstatus_voter_info_t *, voter,
{
- log_info(LD_DIR, "Looks like we need to download a new certificate "
- "from authority '%s' at %s:%d (contact %s; identity %s)",
+ log(severity, LD_DIR, "Looks like we need to download a new "
+ "certificate from authority '%s' at %s:%d (contact %s; "
+ "identity %s)",
voter->nickname, voter->address, (int)voter->dir_port,
voter->contact?voter->contact:"n/a",
hex_str(voter->identity_digest, DIGEST_LEN));
});
SMARTLIST_FOREACH(missing_authorities, trusted_dir_server_t *, ds,
{
- log_info(LD_DIR, "Consensus does not include configured "
+ log(severity, LD_DIR, "Consensus does not include configured "
"authority '%s' at %s:%d (identity %s)",
ds->nickname, ds->address, (int)ds->dir_port,
hex_str(ds->v3_identity_digest, DIGEST_LEN));
});
- log(severity, LD_DIR,
- "%d unknown, %d missing key, %d good, %d bad, %d no signature, "
- "%d required", n_unknown, n_missing_key, n_good, n_bad,
- n_no_signature, n_required);
+ {
+ smartlist_t *sl = smartlist_create();
+ char *cp;
+ tor_asprintf(&cp, "A consensus needs %d good signatures from recognized "
+ "authorities for us to accept it. This one has %d.",
+ n_required, n_good);
+ smartlist_add(sl,cp);
+ if (n_no_signature) {
+ tor_asprintf(&cp, "%d of the authorities we know didn't sign it.",
+ n_no_signature);
+ smartlist_add(sl,cp);
+ }
+ if (n_unknown) {
+ tor_asprintf(&cp, "It has %d signatures from authorities we don't "
+ "recognize.", n_unknown);
+ smartlist_add(sl,cp);
+ }
+ if (n_bad) {
+ tor_asprintf(&cp, "%d of the signatures on it didn't verify "
+ "correctly.", n_bad);
+ smartlist_add(sl,cp);
+ }
+ if (n_missing_key) {
+ tor_asprintf(&cp, "We were unable to check %d of the signatures, "
+ "because we were missing the keys.", n_missing_key);
+ smartlist_add(sl,cp);
+ }
+ cp = smartlist_join_strings(sl, " ", 0, NULL);
+ log(severity, LD_DIR, "%s", cp);
+ tor_free(cp);
+ SMARTLIST_FOREACH(sl, char *, c, tor_free(c));
+ smartlist_free(sl);
+ }
}
smartlist_free(unrecognized);
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 8808f56db9..5bdc973b2c 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -440,6 +440,23 @@ authority_cert_dl_failed(const char *id_digest, int status)
download_status_failed(&cl->dl_status, status);
}
+/** Return true iff when we've been getting enough failures when trying to
+ * download the certificate with ID digest id_digest that we're willing
+ * to start bugging the user about it. */
+int
+authority_cert_dl_looks_uncertain(const char *id_digest)
+{
+#define N_AUTH_CERT_DL_FAILURES_TO_BUG_USER 2
+ cert_list_t *cl;
+ int n_failures;
+ if (!trusted_dir_certs ||
+ !(cl = digestmap_get(trusted_dir_certs, id_digest)))
+ return 0;
+
+ n_failures = download_status_get_n_failures(&cl->dl_status);
+ return n_failures >= N_AUTH_CERT_DL_FAILURES_TO_BUG_USER;
+}
+
/** How many times will we try to fetch a certificate before giving up? */
#define MAX_CERT_DL_FAILURES 8
diff --git a/src/or/routerlist.h b/src/or/routerlist.h
index e31b07aef5..e3e9ddd778 100644
--- a/src/or/routerlist.h
+++ b/src/or/routerlist.h
@@ -24,6 +24,7 @@ void authority_cert_get_all(smartlist_t *certs_out);
void authority_cert_dl_failed(const char *id_digest, int status);
void authority_certs_fetch_missing(networkstatus_t *status, time_t now);
int router_reload_router_list(void);
+int authority_cert_dl_looks_uncertain(const char *id_digest);
smartlist_t *router_get_trusted_dir_servers(void);
routerstatus_t *router_pick_directory_server(authority_type_t type, int flags);