Fetch certificates from the same directory as previous certificates

Improves the fix to #18963.

diff --git a/changes/bug18963 b/changes/bug18963
index 115ea01eba85b4323722dff24560246d0b5dc2ec..f122288b1de28b8581860c28784a568f0c0b26d0 100644 (file)
--- a/changes/bug18963
+++ b/changes/bug18963
@@ -1,4 +1,5 @@
   o Minor bugfix (bootstrap):
-    - Remember the directory we fetched the consensus from, and use
-      it to fetch authority certificates as well.
+    - Remember the directory we fetched the consensus or previous
+      certificates from, and use it to fetch future authority
+      certificates.
       Resolves ticket 18963; fix on #4483 in 0.2.8.1-alpha.

diff --git a/src/or/directory.c b/src/or/directory.c
index 4eeb3eae8ca6c4c6735914b3e0316ffed730da7e..373318f7e515b44397990339535c542f7e57ff24 100644 (file)
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -2063,7 +2063,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
     }
 
     if (src_code != -1) {
-      if (trusted_dirs_load_certs_from_string(body, src_code, 1)<0) {
+      if (trusted_dirs_load_certs_from_string(body, src_code, 1,
+                                              conn->identity_digest)<0) {
         log_warn(LD_DIR, "Unable to parse fetched certificates");
         /* if we fetched more than one and only some failed, the successful
          * ones got flushed to disk so it's safe to call this on them */

diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index f28bff92dbffb872e6d529f673b6d5e1696e21ec..4c64cc9c39464c7536acf28f43b307ea6d420455 100644 (file)
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -2916,7 +2916,8 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out)
     /* Hey, it's a new cert! */
     trusted_dirs_load_certs_from_string(
                                vote->cert->cache_info.signed_descriptor_body,
-                               TRUSTED_DIRS_CERTS_SRC_FROM_VOTE, 1 /*flush*/);
+                               TRUSTED_DIRS_CERTS_SRC_FROM_VOTE, 1 /*flush*/,
+                               NULL);
     if (!authority_cert_get_by_digests(vote->cert->cache_info.identity_digest,
                                        vote->cert->signing_key_digest)) {
       log_warn(LD_BUG, "We added a cert, but still couldn't find it.");

diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index cca45539aacecad785db837849e9909184007441..a3d11f4421db90d061a84c0e1f10e69937d70673 100644 (file)
--- a/src/or/networkstatus.c
+++ b/src/or/networkstatus.c
@@ -1506,8 +1506,8 @@ networkstatus_set_current_consensus_from_ns(networkstatus_t *c,
  * consensus, even if it comes from many days in the past.
  *
  * If source_dir is non-NULL, it's the identity digest for a directory that
- * we've just successfully retrieved a consensus from, so try it first to
- * fetch any missing certificates.
+ * we've just successfully retrieved a consensus or certificates from, so try
+ * it first to fetch any missing certificates.
  *
  * Return 0 on success, <0 on failure.  On failure, caller should increment
  * the failure count as appropriate.
@@ -1802,9 +1802,14 @@ networkstatus_set_current_consensus(const char *consensus,
 }
 
 /** Called when we have gotten more certificates: see whether we can
- * now verify a pending consensus. */
+ * now verify a pending consensus.
+ *
+ * If source_dir is non-NULL, it's the identity digest for a directory that
+ * we've just successfully retrieved certificates from, so try it first to
+ * fetch any missing certificates.
+ */
 void
-networkstatus_note_certs_arrived(void)
+networkstatus_note_certs_arrived(const char *source_dir)
 {
   int i;
   for (i=0; i<N_CONSENSUS_FLAVORS; ++i) {
@@ -1817,7 +1822,7 @@ networkstatus_note_certs_arrived(void)
                                  waiting_body,
                                  networkstatus_get_flavor_name(i),
                                  NSSET_WAS_WAITING_FOR_CERTS,
-                                 NULL)) {
+                                 source_dir)) {
         tor_free(waiting_body);
       }
     }

diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
index 03b041e5d63317ec53293b588777097b47037424..fbe2d8d7b5a5c5b7470d65b344183bb7789e133d 100644 (file)
--- a/src/or/networkstatus.h
+++ b/src/or/networkstatus.h
@@ -87,7 +87,7 @@ int networkstatus_set_current_consensus(const char *consensus,
                                         const char *flavor,
                                         unsigned flags,
                                         const char *source_dir);
-void networkstatus_note_certs_arrived(void);
+void networkstatus_note_certs_arrived(const char *source_dir);
 void routers_update_all_from_networkstatus(time_t now, int dir_version);
 void routers_update_status_from_consensus_networkstatus(smartlist_t *routers,
                                                         int reset_failures);

diff --git a/src/or/router.c b/src/or/router.c
index 68bcf1326ef13f5c309b098e0113800ae7ebd2ce..cbd94a7b8c293001d43f950461f30bdeac1328f4 100644 (file)
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1054,7 +1054,8 @@ init_keys(void)
     log_info(LD_DIR, "adding my own v3 cert");
     if (trusted_dirs_load_certs_from_string(
                       cert->cache_info.signed_descriptor_body,
-                      TRUSTED_DIRS_CERTS_SRC_SELF, 0)<0) {
+                      TRUSTED_DIRS_CERTS_SRC_SELF, 0,
+                      NULL)<0) {
       log_warn(LD_DIR, "Unable to parse my own v3 cert! Failing.");
       return -1;
     }

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 0f722d99dcfb297acf3cf7d94fe73c2a90811812..bba7d60f40f5c4e7edc2cca878860d502d78e716 100644 (file)
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -287,7 +287,7 @@ trusted_dirs_reload_certs(void)
     return 0;
   r = trusted_dirs_load_certs_from_string(
         contents,
-        TRUSTED_DIRS_CERTS_SRC_FROM_STORE, 1);
+        TRUSTED_DIRS_CERTS_SRC_FROM_STORE, 1, NULL);
   tor_free(contents);
   return r;
 }
@@ -317,16 +317,21 @@ already_have_cert(authority_cert_t *cert)
  * or TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_SK_DIGEST.  If <b>flush</b> is true, we
  * need to flush any changed certificates to disk now.  Return 0 on success,
  * -1 if any certs fail to parse.
+ *
+ * If source_dir is non-NULL, it's the identity digest for a directory that
+ * we've just successfully retrieved certificates from, so try it first to
+ * fetch any missing certificates.
  */
 
 int
 trusted_dirs_load_certs_from_string(const char *contents, int source,
-                                    int flush)
+                                    int flush, const char *source_dir)
 {
   dir_server_t *ds;
   const char *s, *eos;
   int failure_code = 0;
   int from_store = (source == TRUSTED_DIRS_CERTS_SRC_FROM_STORE);
+  int added_trusted_cert = 0;
 
   for (s = contents; *s; s = eos) {
     authority_cert_t *cert = authority_cert_parse_from_string(s, &eos);
@@ -386,6 +391,7 @@ trusted_dirs_load_certs_from_string(const char *contents, int source,
     }
 
     if (ds) {
+      added_trusted_cert = 1;
       log_info(LD_DIR, "Adding %s certificate for directory authority %s with "
                "signing key %s", from_store ? "cached" : "downloaded",
                ds->nickname, hex_str(cert->signing_key_digest,DIGEST_LEN));
@@ -430,8 +436,15 @@ trusted_dirs_load_certs_from_string(const char *contents, int source,
     trusted_dirs_flush_certs_to_disk();
 
   /* call this even if failure_code is <0, since some certs might have
-   * succeeded. */
-  networkstatus_note_certs_arrived();
+   * succeeded, but only pass source_dir if there were no failures,
+   * and at least one more authority certificate was added to the store.
+   * This avoids retrying a directory that's serving bad or entirely duplicate
+   * certificates. */
+  if (failure_code == 0 && added_trusted_cert) {
+    networkstatus_note_certs_arrived(source_dir);
+  } else {
+    networkstatus_note_certs_arrived(NULL);
+  }
 
   return failure_code;
 }
@@ -720,8 +733,8 @@ authority_cert_dl_looks_uncertain(const char *id_digest)
  * already have.
  *
  * If dir_hint is non-NULL, it's the identity digest for a directory that
- * we've just successfully retrieved a consensus from, so try it first to
- * fetch any missing certificates.
+ * we've just successfully retrieved a consensus or certificates from, so try
+ * it first to fetch any missing certificates.
  **/
 void
 authority_certs_fetch_missing(networkstatus_t *status, time_t now,

diff --git a/src/or/routerlist.h b/src/or/routerlist.h
index 534dbca5f97f24537fde846f27da49f4bc0e874b..d7af8c807a25c6bb1d1018a18ba444d748ba2e65 100644 (file)
--- a/src/or/routerlist.h
+++ b/src/or/routerlist.h
@@ -29,7 +29,7 @@ int trusted_dirs_reload_certs(void);
 #define TRUSTED_DIRS_CERTS_SRC_FROM_VOTE 4
 
 int trusted_dirs_load_certs_from_string(const char *contents, int source,
-                                        int flush);
+                                        int flush, const char *source_dir);
 void trusted_dirs_flush_certs_to_disk(void);
 authority_cert_t *authority_cert_get_newest_by_id(const char *id_digest);
 authority_cert_t *authority_cert_get_by_sk_digest(const char *sk_digest);

diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c
index 05657ca45205bdcb47d2440b56a81e54829b6c6f..1416b389aae4e357383cf39ce110922a547a632e 100644 (file)
--- a/src/test/test_dir_handle_get.c
+++ b/src/test/test_dir_handle_get.c
@@ -1237,7 +1237,7 @@ test_dir_handle_get_server_keys_all(void* data)
   base16_decode(ds->v3_identity_digest, DIGEST_LEN,
                 TEST_CERT_IDENT_KEY, HEX_DIGEST_LEN);
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   conn = dir_connection_new(tor_addr_family(&MOCK_TOR_ADDR));
 
@@ -1396,7 +1396,7 @@ test_dir_handle_get_server_keys_fp(void* data)
                 TEST_CERT_IDENT_KEY, HEX_DIGEST_LEN);
 
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   conn = dir_connection_new(tor_addr_family(&MOCK_TOR_ADDR));
   char req[71];
@@ -1468,7 +1468,7 @@ test_dir_handle_get_server_keys_sk(void* data)
   routerlist_free_all();
 
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   conn = dir_connection_new(tor_addr_family(&MOCK_TOR_ADDR));
   char req[71];
@@ -1550,7 +1550,7 @@ test_dir_handle_get_server_keys_fpsk(void* data)
   dir_server_add(ds);
 
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   conn = dir_connection_new(tor_addr_family(&MOCK_TOR_ADDR));
 
@@ -1606,7 +1606,7 @@ test_dir_handle_get_server_keys_busy(void* data)
   dir_server_add(ds);
 
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   MOCK(get_options, mock_get_options);
   MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock);
@@ -2344,7 +2344,7 @@ test_dir_handle_get_status_vote_next_authority(void* data)
   base16_decode(ds->v3_identity_digest, DIGEST_LEN,
                 TEST_CERT_IDENT_KEY, HEX_DIGEST_LEN);
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   init_mock_options();
   mock_options->AuthoritativeDir = 1;
@@ -2423,7 +2423,7 @@ test_dir_handle_get_status_vote_current_authority(void* data)
                 TEST_CERT_IDENT_KEY, HEX_DIGEST_LEN);
 
   tt_int_op(0, OP_EQ, trusted_dirs_load_certs_from_string(TEST_CERTIFICATE,
-    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1));
+    TRUSTED_DIRS_CERTS_SRC_DL_BY_ID_DIGEST, 1, NULL));
 
   init_mock_options();
   mock_options->AuthoritativeDir = 1;