[or-cvs] Resolve many XXXs and all DOCDOCs

Nick Mathewson nickm at seul.org
Tue Nov 9 18:22:21 UTC 2004


Update of /home/or/cvsroot/src/or
In directory moria.mit.edu:/tmp/cvs-serv23434/src/or

Modified Files:
	config.c connection_edge.c dirserv.c main.c or.h rephist.c 
	router.c routerparse.c test.c 
Log Message:
Resolve many XXXs and all DOCDOCs

Index: config.c
===================================================================
RCS file: /home/or/cvsroot/src/or/config.c,v
retrieving revision 1.236
retrieving revision 1.237
diff -u -d -r1.236 -r1.237
--- config.c	9 Nov 2004 17:15:17 -0000	1.236
+++ config.c	9 Nov 2004 18:22:16 -0000	1.237
@@ -165,7 +165,7 @@
 static int check_nickname_list(const char *lst, const char *name);
 
 static int parse_dir_server_line(const char *line, int validate_only);
-static int parse_redirect_line(or_options_t *options,
+static int parse_redirect_line(smartlist_t *result,
                                struct config_line_t *line);
 static int parse_log_severity_range(const char *range, int *min_out,
                                     int *max_out);
@@ -264,14 +264,27 @@
   close_temp_logs();
   add_callback_log(LOG_NOTICE, LOG_ERR, control_event_logmsg);
 
+  {
+    smartlist_t *sl = smartlist_create();
+    for (cl = options->RedirectExit; cl; cl = cl->next) {
+      if (parse_redirect_line(sl, cl)<0)
+        return -1;
+    }
+    set_exit_redirects(sl);
+  }
+
   /* Start backgrounding the process, if requested. */
+
+  /* XXXX We once had a reason to separate start_daemon and finish_daemon: It
+   *    let us have the parent process stick around until we were sure Tor was
+   *    started.  Should se make start_daemon get called earlier? -NM */
   if (options->RunAsDaemon) {
     start_daemon(options->DataDirectory);
   }
 
   /* Finish backgrounding the process */
   if(options->RunAsDaemon) {
-    /* XXXX Can we delay this any more? */
+    /* We may be calling this for the n'th time (on SIGHUP), but it's safe. */
     finish_daemon();
   }
 
@@ -599,7 +612,7 @@
       }
       break;
     case CONFIG_TYPE_UINT:
-      /* XXX This means every or_options_t uint or bool element
+      /* This means every or_options_t uint or bool element
        * needs to be an int. Not, say, a uint16_t or char. */
       tor_snprintf(buf, sizeof(buf), "%d", *(int*)value);
       result->value = tor_strdup(buf);
@@ -916,13 +929,6 @@
         break;
     }
   }
-  /* XXX this last part is an exception. can we make it not needed? */
-  if (options->RedirectExitList) {
-    SMARTLIST_FOREACH(options->RedirectExitList,
-                      exit_redirect_t *, p, tor_free(p));
-    smartlist_free(options->RedirectExitList);
-    options->RedirectExitList = NULL;
-  }
 }
 
 /** Copy storage held by <b>old</b> into a new or_options_t and return it. */
@@ -1180,16 +1186,8 @@
       result = -1;
   }
 
-  /* free options->RedirectExitList */
-  if (options->RedirectExitList) {
-    SMARTLIST_FOREACH(options->RedirectExitList,
-                      exit_redirect_t *, p, tor_free(p));
-    smartlist_free(options->RedirectExitList);
-  }
-
-  options->RedirectExitList = smartlist_create();
   for (cl = options->RedirectExit; cl; cl = cl->next) {
-    if (parse_redirect_line(options, cl)<0)
+    if (parse_redirect_line(NULL, cl)<0)
       result = -1;
   }
 
@@ -1729,17 +1727,16 @@
   }
 }
 
-/** Parse a single RedirectExit line's contents from <b>line</b>.  If they are
- *  valid, add an element to <b>options</b>-&gt;RedirectExitList and return 0.
+/** Parse a single RedirectExit line's contents from <b>line</b>.  If
+ *  they are valid, and <b>result</b> is not NULL, add an element to
+ *  <b>result</b> and return 0. Else if they are valid, return 0.
  *  Else return -1. */
 static int
-parse_redirect_line(or_options_t *options, struct config_line_t *line)
+parse_redirect_line(smartlist_t *result, struct config_line_t *line)
 {
   smartlist_t *elements = NULL;
   exit_redirect_t *r;
 
-  tor_assert(options);
-  tor_assert(options->RedirectExitList);
   tor_assert(line);
 
   r = tor_malloc_zero(sizeof(exit_redirect_t));
@@ -1773,7 +1770,10 @@
   SMARTLIST_FOREACH(elements, char *, cp, tor_free(cp));
   smartlist_free(elements);
   if (r) {
-    smartlist_add(options->RedirectExitList, r);
+    if (result)
+      smartlist_add(result, r);
+    else
+      tor_free(r);
     return 0;
   } else {
     return -1;
@@ -1840,6 +1840,9 @@
   return r;
 }
 
+
+/** Adjust or the value of options->DataDirectory, or fill it in if it's
+ * absent. Return 0 on success, -1 on failure. */
 static int
 normalize_data_directory(or_options_t *options) {
 #ifdef MS_WINDOWS
@@ -1861,6 +1864,11 @@
      log_fn(LOG_ERR,"Failed to expand filename '%s'.", d);
      return -1;
    }
+   if (!options->DataDirectory && !strcmp(fn,"/.tor")) {
+     /* If our homedir is /, we probably don't want to use it. */
+     /* XXXX Default to /var/lib/tor? */
+     log_fn(LOG_WARN, "Defaulting to %s, which may not be what you want", fn);
+   }
    tor_free(options->DataDirectory);
    options->DataDirectory = fn;
  }
@@ -1868,6 +1876,8 @@
 #endif
 }
 
+/** Check and normalize the value of options->DataDirectory; return 0 if it
+ * sane, -1 otherwise. */
 static int
 validate_data_directory(or_options_t *options) {
   if (normalize_data_directory(options) < 0)

Index: connection_edge.c
===================================================================
RCS file: /home/or/cvsroot/src/or/connection_edge.c,v
retrieving revision 1.226
retrieving revision 1.227
diff -u -d -r1.226 -r1.227
--- connection_edge.c	7 Nov 2004 01:33:05 -0000	1.226
+++ connection_edge.c	9 Nov 2004 18:22:16 -0000	1.227
@@ -13,6 +13,8 @@
 #include "tree.h"
 
 static struct exit_policy_t *socks_policy = NULL;
+/* List of exit_redirect_t */
+static smartlist_t *redirect_exit_list = NULL;
 
 static int connection_ap_handshake_process_socks(connection_t *conn);
 static void parse_socks_policy(void);
@@ -896,7 +898,8 @@
  * address, but <em>only</em> if it's a general exit stream. (Rendezvous
  * streams must not reveal what IP they connected to.)
  */
-void connection_exit_connect(connection_t *conn) {
+void
+connection_exit_connect(connection_t *conn) {
   unsigned char connected_payload[4];
   uint32_t addr;
   uint16_t port;
@@ -913,7 +916,8 @@
 
   addr = conn->addr;
   port = conn->port;
-  SMARTLIST_FOREACH(options->RedirectExitList, exit_redirect_t *, r,
+  if (redirect_exit_list) {
+    SMARTLIST_FOREACH(redirect_exit_list, exit_redirect_t *, r,
     {
       if ((addr&r->mask)==(r->addr&r->mask) &&
           (r->port_min <= port) && (port <= r->port_max)) {
@@ -928,6 +932,7 @@
         break;
       }
     });
+  }
 
   log_fn(LOG_DEBUG,"about to try connecting");
   switch(connection_connect(conn, conn->address, addr, port)) {
@@ -1198,6 +1203,19 @@
   strmap_foreach(client_dns_map, (strmap_foreach_fn)_remove_if_expired, &now);
 }
 
+
+/** Make connection redirection follow the provided list of
+ * exit_redirect_t */
+void
+set_exit_redirects(smartlist_t *lst)
+{
+  if (redirect_exit_list) {
+    SMARTLIST_FOREACH(redirect_exit_list, exit_redirect_t *, p, tor_free(p));
+    smartlist_free(redirect_exit_list);
+  }
+  redirect_exit_list = lst;
+}
+
 /*
   Local Variables:
   mode:c

Index: dirserv.c
===================================================================
RCS file: /home/or/cvsroot/src/or/dirserv.c,v
retrieving revision 1.116
retrieving revision 1.117
diff -u -d -r1.116 -r1.117
--- dirserv.c	9 Nov 2004 17:12:56 -0000	1.116
+++ dirserv.c	9 Nov 2004 18:22:16 -0000	1.117
@@ -119,6 +119,14 @@
              nickname, fingerprint);
       continue;
     }
+    if (0==strcasecmp(ent->nickname, DEFAULT_CLIENT_NICKNAME)) {
+      /* If you approved an OR called "client", then clients who use
+       * the default nickname could all be rejected.  That's no good. */
+      log(LOG_WARN,
+          "Authorizing a nickname '%s' would break many clients; skipping.",
+          DEFAULT_CLIENT_NICKNAME);
+      continue;
+    }
     for (i = 0; i < smartlist_len(fingerprint_list_new); ++i) {
       ent = smartlist_get(fingerprint_list_new, i);
       if (0==strcasecmp(ent->nickname, nickname)) {
@@ -388,8 +396,7 @@
   ent->desc_len = desc_len;
   ent->descriptor = tor_strndup(start,desc_len);
   ent->router = ri;
-  /* XXX008 is ent->verified useful/used for anything? */
-  ent->verified = verified; /* XXXX008 support other possibilities. */
+  ent->verified = verified;
   smartlist_add(descriptor_list, ent);
 
   *desc = end;
@@ -692,7 +699,9 @@
 static size_t cached_directory_z_len = 0;
 static time_t cached_directory_published = 0;
 
-/** DOCDOC */
+/** If we have no cached directory, or it is older than <b>when</b>, then
+ * replace it with <b>directory</b>, published at <b>when</b>.
+ */
 void dirserv_set_cached_directory(const char *directory, time_t when)
 {
   time_t now;

Index: main.c
===================================================================
RCS file: /home/or/cvsroot/src/or/main.c,v
retrieving revision 1.362
retrieving revision 1.363
diff -u -d -r1.362 -r1.363
--- main.c	9 Nov 2004 08:01:39 -0000	1.362
+++ main.c	9 Nov 2004 18:22:16 -0000	1.363
@@ -1058,7 +1058,8 @@
   printf("%s %s\n", nickname?nickname:"client", buf);
 }
 
-/** DOCDOC **/
+/** Entry point for password hashing: take the desired password from
+ * the command line, and print its salted hash to stdout. **/
 static void do_hash_password(void)
 {
 

Index: or.h
===================================================================
RCS file: /home/or/cvsroot/src/or/or.h,v
retrieving revision 1.471
retrieving revision 1.472
diff -u -d -r1.471 -r1.472
--- or.h	9 Nov 2004 17:15:17 -0000	1.471
+++ or.h	9 Nov 2004 18:22:16 -0000	1.472
@@ -432,6 +432,9 @@
 /* legal characters in a nickname */
 #define LEGAL_NICKNAME_CHARACTERS "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
 
+/** Name to use in client TLS certificates if no nickname is given.*/
+#define DEFAULT_CLIENT_NICKNAME "client"
+
 #define SOCKS4_NETWORK_LEN 8
 
 /*
@@ -1210,6 +1213,7 @@
 int client_dns_incr_failures(const char *address);
 void client_dns_set_entry(const char *address, uint32_t val);
 void client_dns_clean(void);
+void set_exit_redirects(smartlist_t *lst);
 
 /********************************* connection_or.c ***************************/
 

Index: rephist.c
===================================================================
RCS file: /home/or/cvsroot/src/or/rephist.c,v
retrieving revision 1.37
retrieving revision 1.38
diff -u -d -r1.37 -r1.38
--- rephist.c	7 Nov 2004 01:33:06 -0000	1.37
+++ rephist.c	9 Nov 2004 18:22:16 -0000	1.38
@@ -511,9 +511,11 @@
 }
 
 /**
- * DOCDOC
+ * Allocate and return lines for representing this server's bandwidth
+ * history in its descriptor.
  */
-char *rep_hist_get_bandwidth_lines(void)
+char *
+rep_hist_get_bandwidth_lines(void)
 {
   char *buf, *cp;
   char t[ISO_TIME_LEN+1];
@@ -521,7 +523,7 @@
   bw_array_t *b;
   size_t len;
 
-  /* opt (read|write)-history yyyy-mm-dd HH:MM:SS (xxx s) n,n,n,n,n... */
+  /* opt (read|write)-history yyyy-mm-dd HH:MM:SS (n s) n,n,n,n,n... */
   len = (60+12*NUM_TOTALS)*2;
   buf = tor_malloc_zero(len);
   cp = buf;

Index: router.c
===================================================================
RCS file: /home/or/cvsroot/src/or/router.c,v
retrieving revision 1.115
retrieving revision 1.116
diff -u -d -r1.115 -r1.116
--- router.c	9 Nov 2004 07:29:05 -0000	1.115
+++ router.c	9 Nov 2004 18:22:17 -0000	1.116
@@ -247,8 +247,7 @@
     if (crypto_pk_generate_key(prkey))
       return -1;
     set_identity_key(prkey);
-    /* XXX NM: do we have a convention for what client's Nickname is?
-     * No.  Let me propose one: */
+    /* Create a TLS context; default the client nickname to "client". */
     if (tor_tls_context_new(get_identity_key(), 1,
                             options->Nickname ? options->Nickname : "client",
                             MAX_SSL_KEY_LIFETIME) < 0) {

Index: routerparse.c
===================================================================
RCS file: /home/or/cvsroot/src/or/routerparse.c,v
retrieving revision 1.74
retrieving revision 1.75
diff -u -d -r1.74 -r1.75
--- routerparse.c	9 Nov 2004 10:38:42 -0000	1.74
+++ routerparse.c	9 Nov 2004 18:22:17 -0000	1.75
@@ -345,12 +345,6 @@
   smartlist_free(tokens);
   tokens = NULL;
 
-  if(!get_options()->AuthoritativeDir) {
-    /* Now that we know the signature is okay, cache the directory. */
-    /* XXXX009 extract published time if possible. */
-    dirserv_set_cached_directory(str, time(NULL));
-  }
-
   /* Now that we know the signature is okay, check the version. */
   if (check_version)
     check_software_version_against_directory(str, get_options()->IgnoreVersion);
@@ -393,6 +387,12 @@
      goto err;
   }
 
+  if(!get_options()->AuthoritativeDir) {
+    /* Now that we know the signature is okay, and we have a
+     * publication time, cache the directory. */
+    dirserv_set_cached_directory(str, published_on);
+  }
+
   if (!(tok = find_first_by_keyword(tokens, K_RECOMMENDED_SOFTWARE))) {
     log_fn(LOG_WARN, "Missing recommended-software line from directory.");
     goto err;
@@ -863,14 +863,22 @@
   if (!(tok = find_first_by_keyword(tokens, K_ONION_KEY))) {
     log_fn(LOG_WARN, "Missing onion key"); goto err;
   }
-  /* XXX Check key length */
+  if (crypto_pk_keysize(tok->key) != PK_BYTES) {
+    log_fn(LOG_WARN, "Wrong size on onion key: %d bits!",
+           crypto_pk_keysize(tok->key)*8);
+    goto err;
+  }
   router->onion_pkey = tok->key;
   tok->key = NULL; /* Prevent free */
 
   if (!(tok = find_first_by_keyword(tokens, K_SIGNING_KEY))) {
     log_fn(LOG_WARN, "Missing identity key"); goto err;
   }
-  /* XXX Check key length */
+  if (crypto_pk_keysize(tok->key) != PK_BYTES) {
+    log_fn(LOG_WARN, "Wrong size on identity key: %d bits!",
+           crypto_pk_keysize(tok->key)*8);
+    goto err;
+  }
   router->identity_pkey = tok->key;
   tok->key = NULL; /* Prevent free */
   if (crypto_pk_get_digest(router->identity_pkey,router->identity_digest)){
@@ -1420,7 +1428,8 @@
   return tor_version_compare(&router_version, &cutoff_version) >= 0;
 }
 
-/** DOCDOC */
+/** Parse a tor version from <b>s</b>, and store the result in <b>out</b>.
+ * Return 0 on success, -1 on failure. */
 int tor_version_parse(const char *s, tor_version_t *out)
 {
   char *eos=NULL, *cp=NULL;

Index: test.c
===================================================================
RCS file: /home/or/cvsroot/src/or/test.c,v
retrieving revision 1.143
retrieving revision 1.144
diff -u -d -r1.143 -r1.144
--- test.c	7 Nov 2004 22:58:16 -0000	1.143
+++ test.c	9 Nov 2004 18:22:17 -0000	1.144
@@ -727,6 +727,19 @@
   test_streq(v, "val");
   test_streq(cp, "");
 
+  /* Test for strcmpstart and strcmpend. */
+  test_assert(strcmpstart("abcdef", "abcdef")==0);
+  test_assert(strcmpstart("abcdef", "abc")==0);
+  test_assert(strcmpstart("abcdef", "abd")<0);
+  test_assert(strcmpstart("abcdef", "abb")>0);
+  test_assert(strcmpstart("ab", "abb")<0);
+
+  test_assert(strcmpend("abcdef", "abcdef")==0);
+  test_assert(strcmpend("abcdef", "def")==0);
+  test_assert(strcmpend("abcdef", "deg")<0);
+  test_assert(strcmpend("abcdef", "dee")>0);
+  test_assert(strcmpend("ab", "abb")<0);
+
   /* XXXX test older functions. */
   smartlist_free(sl);
 }



More information about the tor-commits mailing list