r16237@catbus: nickm | 2007-10-28 15:45:25 -0400

Tidy v2 hidden service descriptor format code: fix memory leaks, fix reference problems, note magic numbers, note questions, remove redundant checks, remove a possible stack smashing bug when encoding a descriptor with no protocols supported.


svn:r12255

ChangeLog

@@ -7,6 +7,8 @@ Changes in version 0.2.0.10-alpha - 2007-1?-??
       command to establish the stream rather than the normal BEGIN. Now
       we can make anonymized begin_dir connections for (e.g.) more secure
       hidden service posting and fetching.
+    - Code to parse and generate new hidden service descriptor format
+      (From Karsten Loesing.)
 
   o Major bugfixes:
     - Stop servers from crashing if they set a Family option (or

src/or/or.h

@@ -3372,7 +3372,7 @@ int rend_client_send_introduction(origin_circuit_t *introcirc,
 /** Information used to connect to a hidden service. */
 typedef struct rend_service_descriptor_t {
   crypto_pk_env_t *pk; /**< This service's public key. */
-  int version; /**< 0 or 2. */
+  int version; /**< Version of the descriptor format: 0 or 2. */
   time_t timestamp; /**< Time when the descriptor was generated. */
   uint16_t protocols; /**< Bitmask: which rendezvous protocols are supported?
                        * (We allow bits '0', '1', and '2' to be set.) */
@@ -3388,6 +3388,9 @@ typedef struct rend_service_descriptor_t {
   extend_info_t **intro_point_extend_info;
   strmap_t *intro_keys; /**< map from intro node hexdigest to key; only
                          * used for versioned hidden service descriptors. */
+
+  /* XXXX020 Refactor n_intro_points, intro_points, intro_point_extend_info,
+   * and intro_keys into a list of intro points. */
 } rend_service_descriptor_t;
 
 int rend_cmp_service_ids(const char *one, const char *two);

src/or/rendcommon.c

@@ -19,6 +19,16 @@ rend_cmp_service_ids(const char *one, const char *two)
   return strcasecmp(one,two);
 }
 
+/** Helper: Release the storage held by the intro key in <b>_ent</b>.
+ */
+/*XXXX020 there's also one of these in rendservice.c */
+static void
+intro_key_free(void *_ent)
+{
+  crypto_pk_env_t *ent = _ent;
+  crypto_free_pk_env(ent);
+}
+
 /** Free the storage held by the service descriptor <b>desc</b>.
  */
 void
@@ -40,29 +50,37 @@ rend_service_descriptor_free(rend_service_descriptor_t *desc)
     }
     tor_free(desc->intro_point_extend_info);
   }
+  if (desc->intro_keys) {
+    strmap_free(desc->intro_keys, intro_key_free);
+  }
   tor_free(desc);
 }
 
-/* Length of a binary-encoded rendezvous service ID. */
+/** Length of a binary-encoded rendezvous service ID. */
+/*XXXX020 Rename to include "len" and maybe not "binary" */
 #define REND_SERVICE_ID_BINARY 10
 
-/* Length of the time period that is used to encode the secret ID part of
+/** Length of the time period that is used to encode the secret ID part of
  * versioned hidden service descriptors. */
+/*XXXX020 Rename to include "len" and maybe not "binary" */
 #define REND_TIME_PERIOD_BINARY 4
 
-/* Length of the descriptor cookie that is used for versioned hidden
+/** Length of the descriptor cookie that is used for versioned hidden
  * service descriptors. */
+/* XXXX020 rename to REND_DESC_COOKIE_(BINARY_)LEN */
 #define REND_DESC_COOKIE_BINARY 16
 
-/* Length of the replica number that is used to determine the secret ID
+/** Length of the replica number that is used to determine the secret ID
  * part of versioned hidden service descriptors. */
+/* XXXX020 rename to REND_REPLICA_(BINARY_)LEN */
 #define REND_REPLICA_BINARY 1
 
-/* Length of the base32-encoded secret ID part of versioned hidden service
+/** Length of the base32-encoded secret ID part of versioned hidden service
  * descriptors. */
+/*XXXX020 Rename to include "len" */
 #define REND_SECRET_ID_PART_BASE32 32
 
-/* Compute the descriptor ID for <b>service_id</b> of length
+/** Compute the descriptor ID for <b>service_id</b> of length
  * <b>REND_SERVICE_ID_BINARY</b> and <b>secret_id_part</b> of length
  * <b>DIGEST_LEN</b>, and write it to <b>descriptor_id_out</b> of length
  * <b>DIGEST_LEN</b>. */
@@ -78,17 +96,18 @@ rend_get_descriptor_id_bytes(char *descriptor_id_out,
   crypto_free_digest_env(digest);
 }
 
-/* Compute the secret ID part for <b>time_period</b> of length
+/** Compute the secret ID part for time_period,
- * <b>REND_TIME_PERIOD_BINARY</b>, <b>descriptor_cookie</b> of length
+ * a <b>descriptor_cookie</b> of length
  * <b>REND_DESC_COOKIE_BINARY</b> which may also be <b>NULL</b> if no
  * descriptor_cookie shall be used, and <b>replica</b>, and write it to
  * <b>secret_id_part</b> of length DIGEST_LEN. */
 static void
-get_secret_id_part_bytes(char *secret_id_part, const char *time_period,
+get_secret_id_part_bytes(char *secret_id_part, uint32_t time_period,
                          const char *descriptor_cookie, uint8_t replica)
 {
   crypto_digest_env_t *digest = crypto_new_digest_env();
-  crypto_digest_add_bytes(digest, time_period, REND_TIME_PERIOD_BINARY);
+  time_period = htonl(time_period);
+  crypto_digest_add_bytes(digest, (char*)&time_period, sizeof(uint32_t));
   if (descriptor_cookie) {
     crypto_digest_add_bytes(digest, descriptor_cookie,
                             REND_DESC_COOKIE_BINARY);
@@ -98,36 +117,37 @@ get_secret_id_part_bytes(char *secret_id_part, const char *time_period,
   crypto_free_digest_env(digest);
 }
 
-/* Compute the time period bytes for time <b>now</b> plus a potentially
+/** Return the time period for time <b>now</b> plus a potentially
- * intended <b>deviation</b> of one or more periods, and the first byte of
+ * intended <b>deviation</b> of one or more periods, based on the first byte
- * <b>service_id</b>, and write it to <b>time_period</b> of length 4. */
+ * of <b>service_id</b>. */
-static void
+static uint32_t
-get_time_period_bytes(char *time_period, time_t now, uint8_t deviation,
+get_time_period(time_t now, uint8_t deviation, const char *service_id)
-                      const char *service_id)
 {
-  uint32_t host_order =
+  /* The time period is the number of REND_TIME_PERIOD_V2_DESC_VALIDITY
-    (uint32_t)
+   * intervals that have passed since the epoch, offset slightly so that
+   * each service's time periods start and end at a fraction of that
+   * period based on their first byte. */
+  return (uint32_t)
     (now + ((uint8_t) *service_id) * REND_TIME_PERIOD_V2_DESC_VALIDITY / 256)
     / REND_TIME_PERIOD_V2_DESC_VALIDITY + deviation;
-  uint32_t network_order = htonl(host_order);
-  set_uint32(time_period, network_order);
 }
 
-/* Compute the time in seconds that a descriptor that is generated
+/** Compute the time in seconds that a descriptor that is generated
  * <b>now</b> for <b>service_id</b> will be valid. */
 static uint32_t
 get_seconds_valid(time_t now, const char *service_id)
 {
   uint32_t result = REND_TIME_PERIOD_V2_DESC_VALIDITY -
-    (uint32_t)
+    ((uint32_t)
      (now + ((uint8_t) *service_id) * REND_TIME_PERIOD_V2_DESC_VALIDITY / 256)
-    % REND_TIME_PERIOD_V2_DESC_VALIDITY;
+     % REND_TIME_PERIOD_V2_DESC_VALIDITY);
   return result;
 }
 
-/* Compute the binary <b>desc_id</b> for a given base32-encoded
+/** Compute the binary <b>desc_id_out</b> (DIGEST_LEN bytes long) for a given
- * <b>service_id</b> and binary encoded <b>descriptor_cookie</b> of length
+ * base32-encoded <b>service_id</b> and optional unencoded
- * 16 that may be <b>NULL</b> at time <b>now</b> for replica number
+ * <b>descriptor_cookie</b> of length REND_DESC_COOKIE_BINARY,
+ * at time <b>now</b> for replica number
  * <b>replica</b>. <b>desc_id</b> needs to have <b>DIGEST_LEN</b> bytes
  * free. Return 0 for success, -1 otherwise. */
 int
@@ -136,8 +156,8 @@ rend_compute_v2_desc_id(char *desc_id_out, const char *service_id,
                         uint8_t replica)
 {
   char service_id_binary[REND_SERVICE_ID_BINARY];
-  char time_period[REND_TIME_PERIOD_BINARY];
   char secret_id_part[DIGEST_LEN];
+  uint32_t time_period;
   if (!service_id ||
       strlen(service_id) != REND_SERVICE_ID_LEN) {
     log_warn(LD_REND, "Could not compute v2 descriptor ID: "
@@ -158,7 +178,7 @@ rend_compute_v2_desc_id(char *desc_id_out, const char *service_id,
     return -1;
   }
   /* Calculate current time-period. */
-  get_time_period_bytes(time_period, now, 0, service_id_binary);
+  time_period = get_time_period(now, 0, service_id_binary);
   /* Calculate secret-id-part = h(time-period + replica). */
   get_secret_id_part_bytes(secret_id_part, time_period, descriptor_cookie,
                            replica);
@@ -167,45 +187,46 @@ rend_compute_v2_desc_id(char *desc_id_out, const char *service_id,
   return 0;
 }
 
-/* Encode the introduction points in <b>desc</b>, optionally encrypt them
+/* Encode the introduction points in <b>desc</b>, optionally encrypt them with
- * with <b>descriptor_cookie</b> of length 16 that may also be <b>NULL</b>,
+ * an optional <b>descriptor_cookie</b> of length REND_DESC_COOKIE_BINARY,
- * write them to a newly allocated string, and write a pointer to it to
+ * encode it in base64, and write it to a newly allocated string, and write a
- * <b>ipos_base64</b>. Return 0 for success, -1 otherwise. */
+ * pointer to it to *<b>ipos_base64</b>. Return 0 for success, -1
+ * otherwise. */
 static int
 rend_encode_v2_intro_points(char **ipos_base64,
                             rend_service_descriptor_t *desc,
                             const char *descriptor_cookie)
 {
   size_t unenc_len;
-  char *unenc;
+  char *unenc = NULL;
   size_t unenc_written = 0;
-  char *enc;
-  int enclen;
   int i;
-  crypto_cipher_env_t *cipher;
+  int r = -1;
   /* Assemble unencrypted list of introduction points. */
+  *ipos_base64 = NULL;
   unenc_len = desc->n_intro_points * 1000; /* too long, but ok. */
   unenc = tor_malloc_zero(unenc_len);
   for (i = 0; i < desc->n_intro_points; i++) {
-    char id_base32[32 + 1];
+    char id_base32[32 + 1]; /*XXXX020 should be a macro */
-    char *onion_key;
+    char *onion_key = NULL;
     size_t onion_key_len;
     crypto_pk_env_t *intro_key;
-    char *service_key;
+    char *service_key = NULL;
+    char *addr = NULL;
     size_t service_key_len;
     int res;
-    char hex_digest[HEX_DIGEST_LEN+2];
+    char hex_digest[HEX_DIGEST_LEN+2]; /* includes $ and NUL. */
     /* Obtain extend info with introduction point details. */
     extend_info_t *info = desc->intro_point_extend_info[i];
     /* Encode introduction point ID. */
-    base32_encode(id_base32, 32 + 1, info->identity_digest, DIGEST_LEN);
+    base32_encode(id_base32, sizeof(id_base32),
+                  info->identity_digest, DIGEST_LEN);
     /* Encode onion key. */
     if (crypto_pk_write_public_key_to_string(info->onion_key, &onion_key,
                                              &onion_key_len) < 0) {
       log_warn(LD_REND, "Could not write onion key.");
-      if (onion_key) tor_free(onion_key);
+      tor_free(onion_key);
-      tor_free(unenc);
+      goto done;
-      return -1;
     }
     /* Encode intro key. */
     hex_digest[0] = '$';
@@ -217,12 +238,12 @@ rend_encode_v2_intro_points(char **ipos_base64,
       crypto_pk_write_public_key_to_string(intro_key, &service_key,
                                            &service_key_len) < 0) {
       log_warn(LD_REND, "Could not write intro key.");
-      if (service_key) tor_free(service_key);
+      tor_free(service_key);
       tor_free(onion_key);
-      tor_free(unenc);
+      goto done;
-      return -1;
     }
     /* Assemble everything for this introduction point. */
+    addr = tor_dup_addr(info->addr);
     res = tor_snprintf(unenc + unenc_written, unenc_len - unenc_written,
                          "introduction-point %s\n"
                          "ip-address %s\n"
@@ -230,17 +251,17 @@ rend_encode_v2_intro_points(char **ipos_base64,
                          "onion-key\n%s"
                          "service-key\n%s",
                        id_base32,
-                       tor_dup_addr(info->addr),
+                       addr,
                        info->port,
                        onion_key,
                        service_key);
+    tor_free(addr);
     tor_free(onion_key);
     tor_free(service_key);
     if (res < 0) {
       log_warn(LD_REND, "Not enough space for writing introduction point "
                         "string.");
-      tor_free(unenc);
+      goto done;
-      return -1;
     }
     /* Update total number of written bytes for unencrypted intro points. */
     unenc_written += res;
@@ -249,40 +270,42 @@ rend_encode_v2_intro_points(char **ipos_base64,
   if (unenc_len < unenc_written + 2) {
     log_warn(LD_REND, "Not enough space for finalizing introduction point "
                       "string.");
-    tor_free(unenc);
+    goto done;
-    return -1;
   }
   unenc[unenc_written++] = '\n';
   unenc[unenc_written++] = 0;
   /* If a descriptor cookie is passed, encrypt introduction points. */
   if (descriptor_cookie) {
-    enc = tor_malloc_zero(unenc_written + 16);
+    char *enc = tor_malloc_zero(unenc_written + CIPHER_IV_LEN);
-    cipher = crypto_create_init_cipher(descriptor_cookie, 1);
+    crypto_cipher_env_t *cipher =
-    enclen = crypto_cipher_encrypt_with_iv(cipher, enc, unenc_written + 16,
+      crypto_create_init_cipher(descriptor_cookie, 1);
+    int enclen = crypto_cipher_encrypt_with_iv(cipher, enc,
+                                               unenc_written + CIPHER_IV_LEN,
                                                unenc, unenc_written);
     crypto_free_cipher_env(cipher);
-    tor_free(unenc);
     if (enclen < 0) {
       log_warn(LD_REND, "Could not encrypt introduction point string.");
-      if (enc) tor_free(enc);
+      tor_free(enc);
-      return -1;
+      goto done;
     }
-    /* Replace original string by encrypted one. */
+    /* Replace original string with the encrypted one. */
+    tor_free(unenc);
     unenc = enc;
     unenc_written = enclen;
   }
   /* Base64-encode introduction points. */
   *ipos_base64 = tor_malloc_zero(unenc_written * 2);
-  if (base64_encode(*ipos_base64, unenc_written * 2, unenc, unenc_written)
+  if (base64_encode(*ipos_base64, unenc_written * 2, unenc, unenc_written)<0) {
-      < 0) {
     log_warn(LD_REND, "Could not encode introduction point string to "
              "base64.");
-    tor_free(unenc);
+    goto done;
-    tor_free(ipos_base64);
-    return -1;
   }
+  r = 0;
+ done:
+  if (r<0)
+    tor_free(*ipos_base64);
   tor_free(unenc);
-  return 0;
+  return r;
 }
 
 /** Attempt to parse the given <b>desc_str</b> and return true if this
@@ -290,9 +313,9 @@ rend_encode_v2_intro_points(char **ipos_base64,
 static int
 rend_desc_v2_is_parsable(const char *desc_str)
 {
-  rend_service_descriptor_t *test_parsed;
+  rend_service_descriptor_t *test_parsed = NULL;
   char test_desc_id[DIGEST_LEN];
-  char *test_intro_content;
+  char *test_intro_content = NULL;
   size_t test_intro_size;
   size_t test_encoded_size;
   const char *test_next;
@@ -301,7 +324,8 @@ rend_desc_v2_is_parsable(const char *desc_str)
                                          &test_intro_size,
                                          &test_encoded_size,
                                          &test_next, desc_str);
-  tor_free(test_parsed);
+  if (test_parsed)
+    rend_service_descriptor_free(test_parsed);
   tor_free(test_intro_content);
   return (res >= 0);
 }
@@ -322,7 +346,7 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
                            const char *descriptor_cookie, uint8_t period)
 {
   char service_id[DIGEST_LEN];
-  char time_period[REND_TIME_PERIOD_BINARY];
+  uint32_t time_period;
   char *ipos_base64 = NULL;
   int k;
   uint32_t seconds_valid;
@@ -333,14 +357,14 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
   /* Obtain service_id from public key. */
   crypto_pk_get_digest(desc->pk, service_id);
   /* Calculate current time-period. */
-  get_time_period_bytes(time_period, now, period, service_id);
+  time_period = get_time_period(now, period, service_id);
   /* Determine how many seconds the descriptor will be valid. */
   seconds_valid = period * REND_TIME_PERIOD_V2_DESC_VALIDITY +
                   get_seconds_valid(now, service_id);
   /* Assemble, possibly encrypt, and encode introduction points. */
   if (rend_encode_v2_intro_points(&ipos_base64, desc, descriptor_cookie) < 0) {
     log_warn(LD_REND, "Encoding of introduction points did not succeed.");
-    if (ipos_base64) tor_free(ipos_base64);
+    tor_free(ipos_base64);
     return -1;
   }
   /* Encode REND_NUMBER_OF_NON_CONSECUTIVE_REPLICAS descriptors. */
@@ -349,14 +373,14 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
     char secret_id_part_base32[REND_SECRET_ID_PART_BASE32 + 1];
     char *desc_id;
     char desc_id_base32[REND_DESC_ID_V2_BASE32 + 1];
-    char *permanent_key;
+    char *permanent_key = NULL;
     size_t permanent_key_len;
     char published[ISO_TIME_LEN+1];
     int i;
     char protocol_versions_string[16]; /* max len: "0,1,2,3,4,5,6,7\0" */
     size_t protocol_versions_written;
     size_t desc_len;
-    char *desc_str;
+    char *desc_str = NULL;
     int result = 0;
     size_t written = 0;
     char desc_digest[DIGEST_LEN];
@@ -375,7 +399,7 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
     if (crypto_pk_write_public_key_to_string(desc->pk, &permanent_key,
                                              &permanent_key_len) < 0) {
       log_warn(LD_BUG, "Could not write public key to string.");
-      if (permanent_key) tor_free(permanent_key);
+      tor_free(permanent_key);
       goto err;
     }
     /* Encode timestamp. */
@@ -389,7 +413,10 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
         protocol_versions_written += 2;
       }
     }
-    protocol_versions_string[protocol_versions_written - 1] = 0;
+    if (protocol_versions_written)
+      protocol_versions_string[protocol_versions_written - 1] = '\0';
+    else
+      protocol_versions_string[0]= '\0';
     /* Assemble complete descriptor. */
     desc_len = 2000 + desc->n_intro_points * 1000; /* far too long, but ok. */
     desc_str = tor_malloc_zero(desc_len);
@@ -412,14 +439,14 @@ rend_encode_v2_descriptors(smartlist_t *desc_strs_out,
     tor_free(permanent_key);
     if (result < 0) {
       log_warn(LD_BUG, "Descriptor ran out of room.");
-      if (desc_str) tor_free(desc_str);
+      tor_free(desc_str);
       goto err;
     }
     written = result;
     /* Add signature. */
     strlcpy(desc_str + written, "signature\n", desc_len - written);
     written += strlen(desc_str + written);
-    desc_str[written] = '\0';
+    desc_str[written] = '\0'; /* XXXX020 strlcpy always nul-terminates. */
     if (crypto_digest(desc_digest, desc_str, written) < 0) {
       log_warn(LD_BUG, "could not create digest.");
       tor_free(desc_str);

src/or/rendservice.c

@@ -74,7 +74,7 @@ num_rend_services(void)
   return smartlist_len(rend_service_list);
 }
 
-/** Release the storage held by the intro key in <b>_ent</b>.
+/** Helper: Release the storage held by the intro key in <b>_ent</b>.
  */
 static void
 intro_key_free(void *_ent)
@@ -302,12 +302,11 @@ rend_service_update_descriptor(rend_service_t *service)
   origin_circuit_t *circ;
   int i,n;
   routerinfo_t *router;
-
   if (service->desc) {
     rend_service_descriptor_free(service->desc);
     service->desc = NULL;
   }
-  d = service->desc = tor_malloc(sizeof(rend_service_descriptor_t));
+  d = service->desc = tor_malloc_zero(sizeof(rend_service_descriptor_t));
   d->pk = crypto_pk_dup_key(service->private_key);
   d->timestamp = time(NULL);
   d->version = 1;
@@ -317,7 +316,23 @@ rend_service_update_descriptor(rend_service_t *service)
   d->intro_point_extend_info = tor_malloc_zero(sizeof(extend_info_t*)*n);
   /* We support intro protocol 2 and protocol 0. */
   d->protocols = (1<<2) | (1<<0);
-  d->intro_keys = service->intro_keys;
+
+  if (service->intro_keys) {
+    /* We need to copy keys so that they're not deleted when we free the
+     * descriptor. */
+    strmap_iter_t *iter;
+    d->intro_keys = strmap_new();
+    for (iter = strmap_iter_init(service->intro_keys); !strmap_iter_done(iter);
+         iter = strmap_iter_next(service->intro_keys, iter)) {
+      const char *key;
+      void *val;
+      crypto_pk_env_t *k;
+      strmap_iter_get(iter, &key, &val);
+      k = val;
+      strmap_set(d->intro_keys, key, crypto_pk_dup_key(k));
+    }
+  }
+
   for (i=0; i < n; ++i) {
     const char *name = smartlist_get(service->intro_nodes, i);
     router = router_get_by_nickname(name, 1);

src/or/routerparse.c

@@ -315,22 +315,22 @@ static token_rule_t dir_key_certificate_table[] = {
 
 /** List of tokens allowable in rendezvous service descriptors */
 static token_rule_t desc_token_table[] = {
-  T1("rendezvous-service-descriptor", R_RENDEZVOUS_SERVICE_DESCRIPTOR, EQ(1), \
+  T1_START("rendezvous-service-descriptor", R_RENDEZVOUS_SERVICE_DESCRIPTOR,
-     NO_OBJ),
+           EQ(1), NO_OBJ),
   T1("version", R_VERSION, EQ(1), NO_OBJ),
   T1("permanent-key", R_PERMANENT_KEY, NO_ARGS, NEED_KEY_1024),
   T1("secret-id-part", R_SECRET_ID_PART, EQ(1), NO_OBJ),
   T1("publication-time", R_PUBLICATION_TIME, CONCAT_ARGS, NO_OBJ),
   T1("protocol-versions", R_PROTOCOL_VERSIONS, EQ(1), NO_OBJ),
   T1("introduction-points", R_INTRODUCTION_POINTS, NO_ARGS, NEED_OBJ),
-  T1("signature", R_SIGNATURE, NO_ARGS, NEED_OBJ),
+  T1_END("signature", R_SIGNATURE, NO_ARGS, NEED_OBJ),
   END_OF_TABLE
 };
 
 /** List of tokens allowed in the (encrypted) list of introduction points of
  * rendezvous service descriptors */
 static token_rule_t ipo_token_table[] = {
-  T1("introduction-point", R_IPO_IDENTIFIER, EQ(1), NO_OBJ),
+  T1_START("introduction-point", R_IPO_IDENTIFIER, EQ(1), NO_OBJ),
   T1("ip-address", R_IPO_IP_ADDRESS, EQ(1), NO_OBJ),
   T1("onion-port", R_IPO_ONION_PORT, EQ(1), NO_OBJ),
   T1("onion-key", R_IPO_ONION_KEY, NO_ARGS, NEED_KEY_1024),
@@ -3166,13 +3166,13 @@ sort_version_list(smartlist_t *versions, int remove_duplicates)
 }
 
 /** Parse and validate the ASCII-encoded v2 descriptor in <b>desc</b>,
- * write the parsed descriptor to the newly allocated <b>parsed</b>, the
+ * write the parsed descriptor to the newly allocated *<b>parsed_out</b>, the
- * binary descriptor ID of length DIGEST_LEN to <b>desc_id</b>, the
+ * binary descriptor ID of length DIGEST_LEN to <b>desc_id_out</b>, the
  * encrypted introduction points to the newly allocated
- * <b>intro_points_encrypted</b>, their encrypted size to
+ * *<b>intro_points_encrypted_out</b>, their encrypted size to
- * <b>intro_points_encrypted_size</b>, the size of the encoded descriptor
+ * *<b>intro_points_encrypted_size_out</b>, the size of the encoded descriptor
- * to <b>encoded_size</b>, and a pointer to the possibly next
+ * to *<b>encoded_size_out</b>, and a pointer to the possibly next
- * descriptor to <b>next</b>; return 0 for success (including validation)
+ * descriptor to *<b>next_now</b>; return 0 for success (including validation)
  * and -1 for failure.
  */
 int
@@ -3229,16 +3229,12 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
     goto err;
   }
   /* Check whether descriptor starts correctly. */
-  tok = smartlist_get(tokens, 0);
-  if (tok->tp != R_RENDEZVOUS_SERVICE_DESCRIPTOR) {
-    log_warn(LD_REND, "Entry does not start with "
-             "\"rendezvous-service-descriptor\"");
-    goto err;
-  }
   /* Parse base32-encoded descriptor ID. */
   tok = find_first_by_keyword(tokens, R_RENDEZVOUS_SERVICE_DESCRIPTOR);
   tor_assert(tok);
+  tor_assert(tok == smartlist_get(tokens, 0));
   tor_assert(tok->n_args == 1);
+  /*XXXX020 magic 32. */
   if (strlen(tok->args[0]) != 32 ||
       strspn(tok->args[0], BASE32_CHARS) != 32) {
     log_warn(LD_REND, "Invalid descriptor ID: '%s'", tok->args[0]);
@@ -3255,7 +3251,7 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
   tor_assert(tok);
   tor_assert(tok->n_args == 1);
   result->version = atoi(tok->args[0]);
-  if (result->version < 2) {
+  if (result->version < 2) { /*XXXX020 what if > 2? */
     log_warn(LD_REND, "Wrong descriptor version: %d", result->version);
     goto err;
   }
@@ -3268,6 +3264,7 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
   tok = find_first_by_keyword(tokens, R_SECRET_ID_PART);
   tor_assert(tok);
   tor_assert(tok->n_args == 1);
+  /* XXXX020 magic 32. */
   if (strlen(tok->args[0]) != 32 ||
       strspn(tok->args[0], BASE32_CHARS) != 32) {
     log_warn(LD_REND, "Invalid secret ID part: '%s'", tok->args[0]);
@@ -3295,16 +3292,19 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
   smartlist_split_string(versions, tok->args[0], ",",
                          SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
   for (i = 0; i < smartlist_len(versions); i++) {
+    /* XXXX020 validate the numbers here. */
     version = atoi(smartlist_get(versions, i));
     result->protocols |= 1 << version;
   }
+  SMARTLIST_FOREACH(versions, char *, cp, tor_free(cp));
   smartlist_free(versions);
   /* Parse encrypted introduction points. Don't verify. */
   tok = find_first_by_keyword(tokens, R_INTRODUCTION_POINTS);
   tor_assert(tok);
-  *intro_points_encrypted_out = tor_malloc_zero(tok->object_size);
+  /* XXXX020 make sure it's "BEGIN MESSAGE", not "BEGIN SOMETHINGELSE" */
-  memcpy(*intro_points_encrypted_out, tok->object_body, tok->object_size);
+  *intro_points_encrypted_out = tok->object_body;
   *intro_points_encrypted_size_out = tok->object_size;
+  tok->object_body = NULL; /* Prevent free. */
   /* Parse and verify signature. */
   tok = find_first_by_keyword(tokens, R_SIGNATURE);
   tor_assert(tok);
@@ -3351,7 +3351,7 @@ rend_decrypt_introduction_points(rend_service_descriptor_t *parsed,
                                  const char *intro_points_encrypted,
                                  size_t intro_points_encrypted_size)
 {
-  char *ipos_decrypted;
+  char *ipos_decrypted = NULL;
   const char **current_ipo;
   smartlist_t *intropoints;
   smartlist_t *tokens;
@@ -3375,7 +3375,7 @@ rend_decrypt_introduction_points(rend_service_descriptor_t *parsed,
                                              intro_points_encrypted_size);
     crypto_free_cipher_env(cipher);
     if (unenclen < 0) {
-      if (ipos_decrypted) tor_free(ipos_decrypted);
+      tor_free(ipos_decrypted);
       return -1;
     }
     intro_points_encrypted = ipos_decrypted;
@@ -3385,7 +3385,12 @@ rend_decrypt_introduction_points(rend_service_descriptor_t *parsed,
   current_ipo = (const char **)&intro_points_encrypted;
   intropoints = smartlist_create();
   tokens = smartlist_create();
+  if (parsed->intro_keys) {
+    log_warn(LD_BUG, "Parsing list of introduction points for the same "
+             "hidden service, twice.");
+  } else {
     parsed->intro_keys = strmap_new();
+  }
   while (!strcmpstart(*current_ipo, "introduction-point ")) {
     /* Determine end of string. */
     const char *eos = strstr(*current_ipo, "\nintroduction-point ");
@@ -3413,6 +3418,7 @@ rend_decrypt_introduction_points(rend_service_descriptor_t *parsed,
     /* Parse identifier. */
     tok = find_first_by_keyword(tokens, R_IPO_IDENTIFIER);
     tor_assert(tok);
+    /* XXXX020 magic 32. */
     if (base32_decode(info->identity_digest, DIGEST_LEN,
                       tok->args[0], 32) < 0) {
       log_warn(LD_REND, "Identity digest contains illegal characters: %s",
@@ -3434,6 +3440,7 @@ rend_decrypt_introduction_points(rend_service_descriptor_t *parsed,
     info->addr = ntohl(ip.s_addr);
     /* Parse onion port. */
     tok = find_first_by_keyword(tokens, R_IPO_ONION_PORT);
+    /* XXXX020 validate range. */
     info->port = (uint16_t) atoi(tok->args[0]);
     /* Parse onion key. */
     tok = find_first_by_keyword(tokens, R_IPO_ONION_KEY);
@@ -3447,6 +3454,7 @@ rend_decrypt_introduction_points(rend_service_descriptor_t *parsed,
     smartlist_add(intropoints, info);
   }
   /* Write extend infos to descriptor. */
+  /* XXXX020 what if intro_points (&tc) are already set? */
   parsed->n_intro_points = smartlist_len(intropoints);
   parsed->intro_point_extend_info =
     tor_malloc_zero(sizeof(extend_info_t *) * parsed->n_intro_points);