Complicate the rules on WARN vs INFO in consensus verification

It's normal when bootstrapping to have a lot of different certs
missing, so we don't want missing certs to make us warn... unless
the certs we're missing are ones that we've tried to fetch a couple
of times and failed at.

May fix bug 1145.

diff --git a/changes/prettier-signature-log b/changes/prettier-signature-log
index cefaa381583318d38ec1622cf6ded5c5c3e5795f..c008514fb97344835928b42e080aa8d9c06342b9 100644 (file)
--- a/changes/prettier-signature-log
+++ b/changes/prettier-signature-log
@@ -2,4 +2,8 @@
     - 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.
+      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 a3e575ac97fde0711cf5fdd719ce2d5e1142b454..e46d08a7a5ba926c83057e639889660aa5107f17 100644 (file)
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -3484,6 +3484,14 @@ download_status_reset(download_status_t *dls)
   dls->next_attempt_at = time(NULL) + schedule[0];
 }
 
+/** Return the number of failures on <b>dls</b> 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 <b>was_extrainfo</b>)
  * fetches have failed (with uppercase fingerprints listed in <b>failed</b>,
  * either as descriptor digests or as identity digests based on

diff --git a/src/or/directory.h b/src/or/directory.h
index 36b4cf2b182f7dca1640a8aaa6d00e0a632ac956..6fd2c0beff02cf4aa86df9f40de1fe8085fd8a6d 100644 (file)
--- 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 d7a8f70dcc63b6022164b4bcb3b323f911ddc4ba..1b42918525efc917089b4fa754568efbaab76160 100644 (file)
--- 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,10 +540,12 @@ 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,

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 8808f56db90dc87b921311487b27fc5ee295c8ee..5bdc973b2c725aa6c5f412c108791e8d2b761ea8 100644 (file)
--- 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 <b>id_digest</b> 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 e31b07aef55d733b15a1bf81390ea9659cb0441b..e3e9ddd778a13219950ff726bd862eabe060a440 100644 (file)
--- 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);