make dir parsing robust to invalid but well-formed descriptors

svn:r800

src/or/connection_or.c

@@ -275,6 +275,7 @@ static int connection_or_process_cell_from_inbuf(connection_t *conn) {
   char buf[CELL_NETWORK_SIZE];
   cell_t cell;
 
+loop:
   log_fn(LOG_DEBUG,"%d: starting, inbuf_datalen %d (%d pending in tls object).",
          conn->s,(int)buf_datalen(conn->inbuf),tor_tls_get_pending_bytes(conn->tls));
   if(buf_datalen(conn->inbuf) < CELL_NETWORK_SIZE) /* entire response available? */
@@ -282,14 +283,13 @@ static int connection_or_process_cell_from_inbuf(connection_t *conn) {
  
   connection_fetch_from_buf(buf, CELL_NETWORK_SIZE, conn);
  
-  /* retrieve cell info from buf (create the host-order struct from the network-order string) */
+  /* retrieve cell info from buf (create the host-order struct from the
+   * network-order string) */
   cell_unpack(&cell, buf);
  
   command_process_cell(&cell, conn);
 
-  /* CLEAR Shouldn't this be connection_or_process_inbuf at least? Or maybe
+  goto loop; /* process the remainder of the buffer */
-     just use a loop?  If not, doc why not. */
-  return connection_process_inbuf(conn); /* process the remainder of the buffer */
 }
 
 /*

src/or/dirserv.c

@@ -108,7 +108,8 @@ dirserv_parse_fingerprint_file(const char *fname)
   return -1;
 }    
 
-/* return 1 if router's identity and nickname match. */
+/* return 1 if router's identity and nickname match,
+ * -1 if they don't match, 0 if the nickname is not known. */
 int
 dirserv_router_fingerprint_is_known(const routerinfo_t *router)
 {
@@ -126,19 +127,19 @@ dirserv_router_fingerprint_is_known(const routerinfo_t *router)
   }
   
   if (!ent) { /* No such server known */
-    log_fn(LOG_WARN,"no fingerprint found for %s",router->nickname);
+    log_fn(LOG_INFO,"no fingerprint found for %s",router->nickname);
     return 0;
   }
   if (crypto_pk_get_fingerprint(router->identity_pkey, fp)) {
     log_fn(LOG_WARN,"error computing fingerprint");
-    return 0;
+    return -1;
   }
   if (0==strcasecmp(ent->fingerprint, fp)) {
     log_fn(LOG_DEBUG,"good fingerprint for %s",router->nickname);
     return 1; /* Right fingerprint. */
   } else {
     log_fn(LOG_WARN,"mismatched fingerprint for %s",router->nickname);
-    return 0; /* Wrong fingerprint. */
+    return -1; /* Wrong fingerprint. */
   }
 }
 
@@ -183,15 +184,21 @@ dirserv_free_descriptors()
   n_descriptors = 0;
 }
 
-/* Return 0 if descriptor added; -1 if descriptor rejected.  Updates *desc
+/* Return 0 if descriptor is well-formed; -1 if descriptor is not
- * to point after the descriptor if the descriptor is OK.
+ * well-formed.  Update *desc to point after the descriptor if the
+ * descriptor is well-formed.
+ */
+/* XXX down the road perhaps we should return 1 for accepted, 0 for
+ * well-formed but rejected, -1 for not-well-formed. So remote servers
+ * can know if their submission was accepted and not just whether it
+ * was well-formed. ...Or maybe we shouldn't give them that info?
  */
 int
 dirserv_add_descriptor(const char **desc)
 {
   descriptor_entry_t **desc_ent_ptr;
   routerinfo_t *ri = NULL;
-  int i;
+  int i, r;
   char *start, *end;
   char *desc_tmp = NULL, *cp;
   size_t desc_len;
@@ -221,14 +228,23 @@ dirserv_add_descriptor(const char **desc)
   }
   tor_free(desc_tmp);
   /* Okay.  Now check whether the fingerprint is recognized. */
-  if (!dirserv_router_fingerprint_is_known(ri)) {
+  r = dirserv_router_fingerprint_is_known(ri);
-    log_fn(LOG_WARN, "Identity is unrecognized for descriptor");
+  if(r<1) {
-    goto err;
+    if(r==0) {
+      log_fn(LOG_WARN, "Unknown nickname %s. Not adding.", ri->nickname);
+    } else {
+      log_fn(LOG_WARN, "Known nickname %s, wrong fingerprint. Not adding.", ri->nickname);
+    }
+    routerinfo_free(ri);
+    *desc = end;
+    return 0;
   }
   /* Is there too much clock skew? */
   if (ri->published_on > time(NULL)+ROUTER_ALLOW_SKEW) {
-    log_fn(LOG_WARN, "Publication time for nickname %s is too far in the future; possible clock skew.", ri->nickname);
+    log_fn(LOG_WARN, "Publication time for nickname %s is too far in the future; possible clock skew. Not adding", ri->nickname);
-    goto err;
+    routerinfo_free(ri);
+    *desc = end;
+    return 0;
   }
   /* Do we already have an entry for this router? */
   desc_ent_ptr = NULL;
@@ -244,8 +260,7 @@ dirserv_add_descriptor(const char **desc)
       /* We already have a newer descriptor */
       log_fn(LOG_INFO,"We already have a newer desc for nickname %s. Not adding.",ri->nickname);
       /* This isn't really an error; return. */
-      tor_free(desc_tmp);
+      routerinfo_free(ri);
-      if (ri) routerinfo_free(ri);
       *desc = end;
       return 0;
     }
@@ -254,6 +269,7 @@ dirserv_add_descriptor(const char **desc)
   } else {
     /* Add this at the end. */
     desc_ent_ptr = &descriptor_list[n_descriptors++];
+    /* XXX check if n_descriptors is too big */
   }
   
   (*desc_ent_ptr) = tor_malloc(sizeof(descriptor_entry_t));