From d94a978b320a56eaa7daee3c242b5261898ee0d5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 28 Oct 2007 19:48:16 +0000 Subject: [PATCH] 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 | 2 + src/or/or.h | 5 +- src/or/rendcommon.c | 187 +++++++++++++++++++++++++------------------ src/or/rendservice.c | 23 +++++- src/or/routerparse.c | 52 +++++++----- 5 files changed, 162 insertions(+), 107 deletions(-) diff --git a/ChangeLog b/ChangeLog index fb19a3acd0..f1c62164e4 100644 --- a/ChangeLog +++ b/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 diff --git a/src/or/or.h b/src/or/or.h index 00b6fca6e5..bba9785618 100644 --- a/src/or/or.h +++ b/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); diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index c89b7e7910..60f295e4a4 100644 --- a/src/or/rendcommon.c +++ b/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 _ent. + */ +/*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 desc. */ 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 service_id of length +/** Compute the descriptor ID for service_id of length * REND_SERVICE_ID_BINARY and secret_id_part of length * DIGEST_LEN, and write it to descriptor_id_out of length * DIGEST_LEN. */ @@ -78,17 +96,18 @@ rend_get_descriptor_id_bytes(char *descriptor_id_out, crypto_free_digest_env(digest); } -/* Compute the secret ID part for time_period of length - * REND_TIME_PERIOD_BINARY, descriptor_cookie of length +/** Compute the secret ID part for time_period, + * a descriptor_cookie of length * REND_DESC_COOKIE_BINARY which may also be NULL if no * descriptor_cookie shall be used, and replica, and write it to * secret_id_part 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 now plus a potentially - * intended deviation of one or more periods, and the first byte of - * service_id, and write it to time_period of length 4. */ -static void -get_time_period_bytes(char *time_period, time_t now, uint8_t deviation, - const char *service_id) +/** Return the time period for time now plus a potentially + * intended deviation of one or more periods, based on the first byte + * of service_id. */ +static uint32_t +get_time_period(time_t now, uint8_t deviation, const char *service_id) { - uint32_t host_order = - (uint32_t) + /* The time period is the number of REND_TIME_PERIOD_V2_DESC_VALIDITY + * 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 * now for service_id 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) - (now + ((uint8_t) *service_id) * REND_TIME_PERIOD_V2_DESC_VALIDITY / 256) - % REND_TIME_PERIOD_V2_DESC_VALIDITY; + ((uint32_t) + (now + ((uint8_t) *service_id) * REND_TIME_PERIOD_V2_DESC_VALIDITY / 256) + % REND_TIME_PERIOD_V2_DESC_VALIDITY); return result; } -/* Compute the binary desc_id for a given base32-encoded - * service_id and binary encoded descriptor_cookie of length - * 16 that may be NULL at time now for replica number +/** Compute the binary desc_id_out (DIGEST_LEN bytes long) for a given + * base32-encoded service_id and optional unencoded + * descriptor_cookie of length REND_DESC_COOKIE_BINARY, + * at time now for replica number * replica. desc_id needs to have DIGEST_LEN 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 desc, optionally encrypt them - * with descriptor_cookie of length 16 that may also be NULL, - * write them to a newly allocated string, and write a pointer to it to - * ipos_base64. Return 0 for success, -1 otherwise. */ +/* Encode the introduction points in desc, optionally encrypt them with + * an optional descriptor_cookie of length REND_DESC_COOKIE_BINARY, + * encode it in base64, and write it to a newly allocated string, and write a + * pointer to it to *ipos_base64. 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 *onion_key; + char id_base32[32 + 1]; /*XXXX020 should be a macro */ + 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(unenc); - return -1; + tor_free(onion_key); + goto done; } /* 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); - return -1; + goto done; } /* 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); - return -1; + goto done; } /* 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); - return -1; + goto done; } 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); - cipher = crypto_create_init_cipher(descriptor_cookie, 1); - enclen = crypto_cipher_encrypt_with_iv(cipher, enc, unenc_written + 16, - unenc, unenc_written); + char *enc = tor_malloc_zero(unenc_written + CIPHER_IV_LEN); + crypto_cipher_env_t *cipher = + 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); - return -1; + tor_free(enc); + 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) - < 0) { + if (base64_encode(*ipos_base64, unenc_written * 2, unenc, unenc_written)<0) { log_warn(LD_REND, "Could not encode introduction point string to " - "base64."); - tor_free(unenc); - tor_free(ipos_base64); - return -1; + "base64."); + goto done; } + r = 0; + done: + if (r<0) + tor_free(*ipos_base64); tor_free(unenc); - return 0; + return r; } /** Attempt to parse the given desc_str 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); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 3f5afcde12..2eb615a47b 100644 --- a/src/or/rendservice.c +++ b/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 _ent. +/** Helper: Release the storage held by the intro key in _ent. */ 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); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 98e46c8e4a..ab520c94ef 100644 --- a/src/or/routerparse.c +++ b/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), \ - NO_OBJ), + T1_START("rendezvous-service-descriptor", R_RENDEZVOUS_SERVICE_DESCRIPTOR, + 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 desc, - * write the parsed descriptor to the newly allocated parsed, the - * binary descriptor ID of length DIGEST_LEN to desc_id, the + * write the parsed descriptor to the newly allocated *parsed_out, the + * binary descriptor ID of length DIGEST_LEN to desc_id_out, the * encrypted introduction points to the newly allocated - * intro_points_encrypted, their encrypted size to - * intro_points_encrypted_size, the size of the encoded descriptor - * to encoded_size, and a pointer to the possibly next - * descriptor to next; return 0 for success (including validation) + * *intro_points_encrypted_out, their encrypted size to + * *intro_points_encrypted_size_out, the size of the encoded descriptor + * to *encoded_size_out, and a pointer to the possibly next + * descriptor to *next_now; 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); - memcpy(*intro_points_encrypted_out, tok->object_body, tok->object_size); + /* XXXX020 make sure it's "BEGIN MESSAGE", not "BEGIN SOMETHINGELSE" */ + *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(); - parsed->intro_keys = strmap_new(); + 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);