Resolve task 42: find where 19-char nicknames were getting truncated when read from certs, and fix it. Also audit use of MAX_NICKNAME_LEN; no other badness found, but some docs/code cleaned up a touch.

svn:r3244

6 files changed, 22 insertions, 14 deletions

diff --git a/src/common/tortls.c b/src/common/tortls.c
index 420b1963f7..86cdcec48f 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -586,8 +586,10 @@ tor_tls_peer_has_cert(tor_tls *tls)
   return 1;
 }
 
-/** Return the nickname (if any) that the peer connected on <b>tls</b>
- * claims to have.
+/** Write the nickname (if any) that the peer connected on <b>tls</b>
+ * claims to have into the first <b>buflen</b> characters of <b>buf</b>.
+ * Truncate the nickname if it is longer than buflen-1 characters.  Always
+ * NUL-terminate.  Return 0 on success, -1 on failure.
  */
 int
 tor_tls_get_peer_cert_nickname(tor_tls *tls, char *buf, size_t buflen)
diff --git a/src/or/config.c b/src/or/config.c
index 7d81ad4f97..7fc3f77beb 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -927,7 +927,7 @@ resolve_my_address(const char *address, uint32_t *addr)
 }
 
 /** Called when we don't have a nickname set.  Try to guess a good
- * nickname based on the hostname, and return it. */
+ * nickname based on the hostname, and return it in a newly allocated string. */
 static char *
 get_default_nickname(void)
 {
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 3520bd2ffc..2fed1f89a7 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -354,7 +354,7 @@ connection_tls_finish_handshake(connection_t *conn) {
     }
   }
   /* Okay; the other side is an OR or a post-0.0.8 OP (with a cert). */
-  if (tor_tls_get_peer_cert_nickname(conn->tls, nickname, MAX_NICKNAME_LEN)) {
+  if (tor_tls_get_peer_cert_nickname(conn->tls, nickname, sizeof(nickname))) {
     log_fn(LOG_WARN,"Other side (%s:%d) has a cert without a valid nickname. Closing.",
            conn->address, conn->port);
     return -1;
diff --git a/src/or/router.c b/src/or/router.c
index 30fe73448d..d91546d6ff 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -240,7 +240,8 @@ int init_keys(void) {
    */
   char keydir[512];
   char keydir2[512];
-  char fingerprint[FINGERPRINT_LEN+MAX_NICKNAME_LEN+3];
+  char fingerprint[FINGERPRINT_LEN+1];
+  char fingerprint_line[FINGERPRINT_LEN+MAX_NICKNAME_LEN+3];/*nickname fp\n\0 */
   char *cp;
   const char *tmp, *mydesc, *datadir;
   crypto_pk_env_t *prkey;
@@ -333,16 +334,17 @@ int init_keys(void) {
   /* 5. Dump fingerprint to 'fingerprint' */
   tor_snprintf(keydir,sizeof(keydir),"%s/fingerprint", datadir);
   log_fn(LOG_INFO,"Dumping fingerprint to %s...",keydir);
-  tor_assert(strlen(options->Nickname) <= MAX_NICKNAME_LEN);
-  strlcpy(fingerprint, options->Nickname, sizeof(fingerprint));
-  strlcat(fingerprint, " ", sizeof(fingerprint));
-  if (crypto_pk_get_fingerprint(get_identity_key(),
-                                fingerprint+strlen(fingerprint), 1)<0) {
+  if (crypto_pk_get_fingerprint(get_identity_key(), fingerprint, 1)<0) {
     log_fn(LOG_ERR, "Error computing fingerprint");
     return -1;
   }
-  strlcat(fingerprint, "\n", sizeof(fingerprint));
-  if (write_str_to_file(keydir, fingerprint, 0))
+  tor_assert(strlen(options->Nickname) <= MAX_NICKNAME_LEN);
+  if (tor_snprintf(fingerprint_line, sizeof(fingerprint_line),
+                   "%s %s\n",options->Nickname, fingerprint) < 0) {
+    log_fn(LOG_ERR, "Error writing fingerprint line");
+    return -1;
+  }
+  if (write_str_to_file(keydir, fingerprint_line, 0))
     return -1;
   if (!authdir_mode(options))
     return 0;
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 293797f6cf..6a68bb96e8 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -311,8 +311,8 @@ add_nickname_list_to_smartlist(smartlist_t *sl, const char *list, int warn_if_do
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
 
   SMARTLIST_FOREACH(nickname_list, const char *, nick, {
-    if (strlen(nick) > MAX_HEX_NICKNAME_LEN) {
-      log_fn(LOG_WARN,"Nickname too long; skipping");
+    if (!is_legal_nickname_or_hexdigest(nick)) {
+      log_fn(LOG_WARN,"Nickname %s is misformed; skipping", nick);
       continue;
     }
     router = router_get_by_nickname(nick);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 78fb316ff9..9886d0ac17 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -340,6 +340,10 @@ router_parse_routerlist_from_directory(const char *str,
     goto err;
 
   /* now we know tok->n_args == 1, so it's safe to access tok->args[0] */
+  if (!is_legal_nickname(tok->args[0])) {
+    log_fn(LOG_WARN, "Directory nickname '%s' is misformed", tok->args[0]);
+    goto err;
+  }
   strlcpy(dirnickname, tok->args[0], sizeof(dirnickname));
 
   SMARTLIST_FOREACH(tokens, directory_token_t *, tok, token_free(tok));