Notice v3 cert parsing failures

If any the v3 certs we download are unparseable, we should actually
notice the failure so we don't retry indefinitely. Bugfix on 0.2.0.x;
reported by "rotator".

3 files changed, 19 insertions, 6 deletions

diff --git a/ChangeLog b/ChangeLog
index ac7beb79ea..9b93d3f361 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -39,6 +39,11 @@ Changes in version 0.2.2.1-alpha - 2009-0?-??
       as only certain clients who must have this information sooner should
       set this option.
 
+  o Minor bugfixes:
+    - If any the v3 certs we download are unparseable, we should actually
+      notice the failure so we don't retry indefinitely. Bugfix on
+      0.2.0.x; reported by "rotator".
+
   o Deprecated and removed features:
     - The controller no longer accepts the old obsolete "addr-mappings/"
       or "unregistered-servers-" GETINFO values.
diff --git a/src/or/directory.c b/src/or/directory.c
index 976c08c19a..93046489f0 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1649,6 +1649,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
              "'%s:%d'",(int) body_len, conn->_base.address, conn->_base.port);
     if (trusted_dirs_load_certs_from_string(body, 0, 1)<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 */
       connection_dir_download_cert_failed(conn, status_code);
     } else {
       directory_info_has_arrived(now, 0);
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 42b385b101..0a32f78a69 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -156,21 +156,24 @@ already_have_cert(authority_cert_t *cert)
 
 /** Load a bunch of new key certificates from the string <b>contents</b>.  If
  * <b>from_store</b> is true, the certificates are from the cache, and we
- * don't need to flush them to disk.  If <b>from_store</b> is false, we need
- * to flush any changed certificates to disk.  Return 0 on success, -1 on
- * failure. */
+ * don't need to flush them to disk. 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. */
 int
 trusted_dirs_load_certs_from_string(const char *contents, int from_store,
                                     int flush)
 {
   trusted_dir_server_t *ds;
   const char *s, *eos;
+  int failure_code = 0;
 
   for (s = contents; *s; s = eos) {
     authority_cert_t *cert = authority_cert_parse_from_string(s, &eos);
     cert_list_t *cl;
-    if (!cert)
+    if (!cert) {
+      failure_code = -1;
       break;
+    }
     ds = trusteddirserver_get_by_v3_auth_digest(
                                        cert->cache_info.identity_digest);
     log_debug(LD_DIR, "Parsed certificate for %s",
@@ -224,7 +227,7 @@ trusted_dirs_load_certs_from_string(const char *contents, int from_store,
            ds->dir_port != cert->dir_port)) {
         char *a = tor_dup_ip(cert->addr);
         log_notice(LD_DIR, "Updating address for directory authority %s "
-                   "from %s:%d to %s:%d based on in certificate.",
+                   "from %s:%d to %s:%d based on certificate.",
                    ds->nickname, ds->address, (int)ds->dir_port,
                    a, cert->dir_port);
         tor_free(a);
@@ -241,8 +244,11 @@ trusted_dirs_load_certs_from_string(const char *contents, int from_store,
   if (flush)
     trusted_dirs_flush_certs_to_disk();
 
+  /* call this even if failure_code is <0, since some certs might have
+   * succeeded. */
   networkstatus_note_certs_arrived();
-  return 0;
+
+  return failure_code;
 }
 
 /** Save all v3 key certificates to the cached-certs file. */