From 6d71eda263a6c97484c975073979a005a879cf79 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Thu, 2 Feb 2017 13:58:20 +0200 Subject: [PATCH 1/9] prop224: Rename auth_required HS desc field to intro_auth_required. And remove "password" type from the list of intro auths. --- src/or/hs_descriptor.c | 37 +++++++++++++++++------------------ src/or/hs_descriptor.h | 5 ++--- src/or/parsecommon.h | 2 +- src/test/test_hs_cache.c | 4 ++-- src/test/test_hs_descriptor.c | 23 ++++++++++++---------- 5 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index f16a2fdc14..fc1f36efc7 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -27,7 +27,7 @@ #define str_lifetime "descriptor-lifetime" /* Constant string value for the encrypted part of the descriptor. */ #define str_create2_formats "create2-formats" -#define str_auth_required "authentication-required" +#define str_intro_auth_required "intro-auth-required" #define str_single_onion "single-onion-service" #define str_intro_point "introduction-point" #define str_ip_auth_key "auth-key" @@ -44,8 +44,7 @@ static const struct { hs_desc_auth_type_t type; const char *identifier; -} auth_types[] = { - { HS_DESC_AUTH_PASSWORD, "password" }, +} intro_auth_types[] = { { HS_DESC_AUTH_ED25519, "ed25519" }, /* Indicate end of array. */ { 0, NULL } @@ -65,7 +64,7 @@ static token_rule_t hs_desc_v3_token_table[] = { /* Descriptor ruleset for the encrypted section. */ static token_rule_t hs_desc_encrypted_v3_token_table[] = { T1_START(str_create2_formats, R3_CREATE2_FORMATS, CONCAT_ARGS, NO_OBJ), - T01(str_auth_required, R3_AUTHENTICATION_REQUIRED, ARGS, NO_OBJ), + T01(str_intro_auth_required, R3_INTRO_AUTH_REQUIRED, ARGS, NO_OBJ), T01(str_single_onion, R3_SINGLE_ONION_SERVICE, ARGS, NO_OBJ), END_OF_TABLE }; @@ -123,9 +122,9 @@ desc_encrypted_data_free_contents(hs_desc_encrypted_data_t *desc) return; } - if (desc->auth_types) { - SMARTLIST_FOREACH(desc->auth_types, char *, a, tor_free(a)); - smartlist_free(desc->auth_types); + if (desc->intro_auth_types) { + SMARTLIST_FOREACH(desc->intro_auth_types, char *, a, tor_free(a)); + smartlist_free(desc->intro_auth_types); } if (desc->intro_points) { SMARTLIST_FOREACH(desc->intro_points, hs_desc_intro_point_t *, ip, @@ -649,12 +648,12 @@ encode_encrypted_data(const hs_descriptor_t *desc, smartlist_add_asprintf(lines, "%s %d\n", str_create2_formats, ONION_HANDSHAKE_TYPE_NTOR); - if (desc->encrypted_data.auth_types && - smartlist_len(desc->encrypted_data.auth_types)) { + if (desc->encrypted_data.intro_auth_types && + smartlist_len(desc->encrypted_data.intro_auth_types)) { /* Put the authentication-required line. */ - char *buf = smartlist_join_strings(desc->encrypted_data.auth_types, " ", - 0, NULL); - smartlist_add_asprintf(lines, "%s %s\n", str_auth_required, buf); + char *buf = smartlist_join_strings(desc->encrypted_data.intro_auth_types, + " ", 0, NULL); + smartlist_add_asprintf(lines, "%s %s\n", str_intro_auth_required, buf); tor_free(buf); } @@ -894,14 +893,14 @@ decode_auth_type(hs_desc_encrypted_data_t *desc, const char *list) tor_assert(desc); tor_assert(list); - desc->auth_types = smartlist_new(); - smartlist_split_string(desc->auth_types, list, " ", 0, 0); + desc->intro_auth_types = smartlist_new(); + smartlist_split_string(desc->intro_auth_types, list, " ", 0, 0); /* Validate the types that we at least know about one. */ - SMARTLIST_FOREACH_BEGIN(desc->auth_types, const char *, auth) { - for (int idx = 0; auth_types[idx].identifier; idx++) { - if (!strncmp(auth, auth_types[idx].identifier, - strlen(auth_types[idx].identifier))) { + SMARTLIST_FOREACH_BEGIN(desc->intro_auth_types, const char *, auth) { + for (int idx = 0; intro_auth_types[idx].identifier; idx++) { + if (!strncmp(auth, intro_auth_types[idx].identifier, + strlen(intro_auth_types[idx].identifier))) { match = 1; break; } @@ -1572,7 +1571,7 @@ desc_decode_encrypted_v3(const hs_descriptor_t *desc, } /* Authentication type. It's optional but only once. */ - tok = find_opt_by_keyword(tokens, R3_AUTHENTICATION_REQUIRED); + tok = find_opt_by_keyword(tokens, R3_INTRO_AUTH_REQUIRED); if (tok) { if (!decode_auth_type(desc_encrypted_out, tok->args[0])) { log_warn(LD_REND, "Service descriptor authentication type has " diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h index b520d24471..6b888c1822 100644 --- a/src/or/hs_descriptor.h +++ b/src/or/hs_descriptor.h @@ -68,8 +68,7 @@ /* Type of authentication in the descriptor. */ typedef enum { - HS_DESC_AUTH_PASSWORD = 1, - HS_DESC_AUTH_ED25519 = 2, + HS_DESC_AUTH_ED25519 = 1 } hs_desc_auth_type_t; /* Type of encryption key in the descriptor. */ @@ -132,7 +131,7 @@ typedef struct hs_desc_encrypted_data_t { /* A list of authentication types that a client must at least support one * in order to contact the service. Contains NULL terminated strings. */ - smartlist_t *auth_types; + smartlist_t *intro_auth_types; /* Is this descriptor a single onion service? */ unsigned int single_onion_service : 1; diff --git a/src/or/parsecommon.h b/src/or/parsecommon.h index 15e9f7ae85..ec62bade92 100644 --- a/src/or/parsecommon.h +++ b/src/or/parsecommon.h @@ -157,7 +157,7 @@ typedef enum { R3_SUPERENCRYPTED, R3_SIGNATURE, R3_CREATE2_FORMATS, - R3_AUTHENTICATION_REQUIRED, + R3_INTRO_AUTH_REQUIRED, R3_SINGLE_ONION_SERVICE, R3_INTRODUCTION_POINT, R3_INTRO_AUTH_KEY, diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 1943d0ffac..64fc1c1e6e 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -93,8 +93,8 @@ helper_build_hs_desc(uint64_t revision_counter, uint32_t lifetime, /* Setup encrypted data section. */ desc->encrypted_data.create2_ntor = 1; - desc->encrypted_data.auth_types = smartlist_new(); - smartlist_add(desc->encrypted_data.auth_types, tor_strdup("ed25519")); + desc->encrypted_data.intro_auth_types = smartlist_new(); + smartlist_add(desc->encrypted_data.intro_auth_types, tor_strdup("ed25519")); desc->encrypted_data.intro_points = smartlist_new(); /* Add an intro point. */ smartlist_add(desc->encrypted_data.intro_points, diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 02a71aa473..4042e647da 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -105,9 +105,9 @@ helper_build_hs_desc(unsigned int no_ip, ed25519_public_key_t *signing_pubkey) /* Setup encrypted data section. */ desc->encrypted_data.create2_ntor = 1; - desc->encrypted_data.auth_types = smartlist_new(); + desc->encrypted_data.intro_auth_types = smartlist_new(); desc->encrypted_data.single_onion_service = 1; - smartlist_add(desc->encrypted_data.auth_types, tor_strdup("ed25519")); + smartlist_add(desc->encrypted_data.intro_auth_types, tor_strdup("ed25519")); desc->encrypted_data.intro_points = smartlist_new(); if (!no_ip) { /* Add four intro points. */ @@ -157,14 +157,17 @@ helper_compare_hs_desc(const hs_descriptor_t *desc1, desc2->encrypted_data.create2_ntor); /* Authentication type. */ - tt_int_op(!!desc1->encrypted_data.auth_types, ==, - !!desc2->encrypted_data.auth_types); - if (desc1->encrypted_data.auth_types && desc2->encrypted_data.auth_types) { - tt_int_op(smartlist_len(desc1->encrypted_data.auth_types), ==, - smartlist_len(desc2->encrypted_data.auth_types)); - for (int i = 0; i < smartlist_len(desc1->encrypted_data.auth_types); i++) { - tt_str_op(smartlist_get(desc1->encrypted_data.auth_types, i), OP_EQ, - smartlist_get(desc2->encrypted_data.auth_types, i)); + tt_int_op(!!desc1->encrypted_data.intro_auth_types, ==, + !!desc2->encrypted_data.intro_auth_types); + if (desc1->encrypted_data.intro_auth_types && + desc2->encrypted_data.intro_auth_types) { + tt_int_op(smartlist_len(desc1->encrypted_data.intro_auth_types), ==, + smartlist_len(desc2->encrypted_data.intro_auth_types)); + for (int i = 0; + i < smartlist_len(desc1->encrypted_data.intro_auth_types); + i++) { + tt_str_op(smartlist_get(desc1->encrypted_data.intro_auth_types, i),OP_EQ, + smartlist_get(desc2->encrypted_data.intro_auth_types, i)); } } From f8ac4bb9fd9f66d71579016470d2c34e8f24fbfb Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 6 Feb 2017 14:13:35 +0200 Subject: [PATCH 2/9] prop224: Rename desc->encrypted_blob to desc->superencrypted_blob --- src/or/hs_descriptor.c | 26 +++++++++++++------------- src/or/hs_descriptor.h | 8 ++++---- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index fc1f36efc7..cb3e934614 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -106,8 +106,8 @@ desc_plaintext_data_free_contents(hs_desc_plaintext_data_t *desc) return; } - if (desc->encrypted_blob) { - tor_free(desc->encrypted_blob); + if (desc->superencrypted_blob) { + tor_free(desc->superencrypted_blob); } tor_cert_free(desc->signing_key_cert); @@ -1066,27 +1066,27 @@ desc_decrypt_data_v3(const hs_descriptor_t *desc, char **decrypted_out) tor_assert(decrypted_out); tor_assert(desc); - tor_assert(desc->plaintext_data.encrypted_blob); + tor_assert(desc->plaintext_data.superencrypted_blob); /* Construction is as follow: SALT | ENCRYPTED_DATA | MAC */ if (!encrypted_data_length_is_valid( - desc->plaintext_data.encrypted_blob_size)) { + desc->plaintext_data.superencrypted_blob_size)) { goto err; } /* Start of the blob thus the salt. */ - salt = desc->plaintext_data.encrypted_blob; + salt = desc->plaintext_data.superencrypted_blob; /* Next is the encrypted data. */ - encrypted = desc->plaintext_data.encrypted_blob + + encrypted = desc->plaintext_data.superencrypted_blob + HS_DESC_ENCRYPTED_SALT_LEN; - encrypted_len = desc->plaintext_data.encrypted_blob_size - + encrypted_len = desc->plaintext_data.superencrypted_blob_size - (HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN); /* At the very end is the MAC. Make sure it's of the right size. */ { desc_mac = encrypted + encrypted_len; - size_t desc_mac_size = desc->plaintext_data.encrypted_blob_size - - (desc_mac - desc->plaintext_data.encrypted_blob); + size_t desc_mac_size = desc->plaintext_data.superencrypted_blob_size - + (desc_mac - desc->plaintext_data.superencrypted_blob); if (desc_mac_size != DIGEST256_LEN) { log_warn(LD_REND, "Service descriptor MAC length of encrypted data " "is invalid (%lu, expected %u)", @@ -1507,8 +1507,8 @@ desc_decode_plaintext_v3(smartlist_t *tokens, /* Copy the encrypted blob to the descriptor object so we can handle it * latter if needed. */ - desc->encrypted_blob = tor_memdup(tok->object_body, tok->object_size); - desc->encrypted_blob_size = tok->object_size; + desc->superencrypted_blob = tor_memdup(tok->object_body, tok->object_size); + desc->superencrypted_blob_size = tok->object_size; /* Extract signature and verify it. */ tok = find_by_keyword(tokens, R3_SIGNATURE); @@ -1653,7 +1653,7 @@ hs_desc_decode_encrypted(const hs_descriptor_t *desc, /* Calling this function without an encrypted blob to parse is a code flow * error. The plaintext parsing should never succeed in the first place * without an encrypted section. */ - tor_assert(desc->plaintext_data.encrypted_blob); + tor_assert(desc->plaintext_data.superencrypted_blob); /* Let's make sure we have a supported version as well. By correctly parsing * the plaintext, this should not fail. */ if (BUG(!hs_desc_is_supported_version(version))) { @@ -1905,6 +1905,6 @@ hs_desc_plaintext_obj_size(const hs_desc_plaintext_data_t *data) { tor_assert(data); return (sizeof(*data) + sizeof(*data->signing_key_cert) + - data->encrypted_blob_size); + data->superencrypted_blob_size); } diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h index 6b888c1822..3b5832bdf2 100644 --- a/src/or/hs_descriptor.h +++ b/src/or/hs_descriptor.h @@ -166,11 +166,11 @@ typedef struct hs_desc_plaintext_data_t { * has changed. Spec specifies this as a 8 bytes positive integer. */ uint64_t revision_counter; - /* Decoding only: The base64-decoded encrypted blob from the descriptor */ - uint8_t *encrypted_blob; + /* Decoding only: The b64-decoded superencrypted blob from the descriptor */ + uint8_t *superencrypted_blob; - /* Decoding only: Size of the encrypted_blob */ - size_t encrypted_blob_size; + /* Decoding only: Size of the superencrypted_blob */ + size_t superencrypted_blob_size; } hs_desc_plaintext_data_t; /* Service descriptor in its decoded form. */ From bb602f61972874aa03181ede877ee8e85ce0389d Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Wed, 8 Feb 2017 14:16:20 +0200 Subject: [PATCH 3/9] prop224: Prepare for superencrypted HS descriptors. - Refactor our HS desc crypto funcs to be able to differentiate between the superencrypted layer and the encrypted layer so that different crypto constants and padding is used in each layer. - Introduce some string constants. - Add some comments. --- src/or/hs_descriptor.c | 109 ++++++++++++++++++++++++++++++++++------- 1 file changed, 91 insertions(+), 18 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index cb3e934614..3b9ee8aa2d 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -4,6 +4,52 @@ /** * \file hs_descriptor.c * \brief Handle hidden service descriptor encoding/decoding. + * + * \details + * Here is a graphical depiction of an HS descriptor and its layers: + * + * +------------------------------------------------------+ + * |DESCRIPTOR HEADER: | + * | hs-descriptor 3 | + * | descriptor-lifetime 180 | + * | ... | + * | superencrypted | + * |+---------------------------------------------------+ | + * ||SUPERENCRYPTED LAYER (aka OUTER ENCRYPTED LAYER): | | + * || desc-auth-type x25519 | | + * || desc-auth-ephemeral-key | | + * || auth-client | | + * || auth-client | | + * || ... | | + * || encrypted | | + * ||+-------------------------------------------------+| | + * |||ENCRYPTED LAYER (aka INNER ENCRYPTED LAYER): || | + * ||| create2-formats || | + * ||| intro-auth-required || | + * ||| introduction-point || | + * ||| introduction-point || | + * ||| ... || | + * ||+-------------------------------------------------+| | + * |+---------------------------------------------------+ | + * +------------------------------------------------------+ + * + * The DESCRIPTOR HEADER section is completely unencrypted and contains generic + * descriptor metadata. + * + * The SUPERENCRYPTED LAYER section is the first layer of encryption, and it's + * encrypted using the blinded public key of the hidden service to protect + * against entities who don't know its onion address. The clients of the hidden + * service know its onion address and blinded public key, whereas third-parties + * (like HSDirs) don't know it (except if it's a public hidden service). + * + * The ENCRYPTED LAYER section is the second layer of encryption, and it's + * encrypted using the client authorization key material (if those exist). When + * client authorization is enabled, this second layer of encryption protects + * the descriptor content from unauthorized entities. If client authorization + * is disabled, this second layer of encryption does not provide any extra + * security but is still present. The plaintext of this layer contains all the + * information required to connect to the hidden service like its list of + * introduction points. **/ /* For unit tests.*/ @@ -23,6 +69,7 @@ #define str_desc_cert "descriptor-signing-key-cert" #define str_rev_counter "revision-counter" #define str_superencrypted "superencrypted" +#define str_encrypted "encrypted" #define str_signature "signature" #define str_lifetime "descriptor-lifetime" /* Constant string value for the encrypted part of the descriptor. */ @@ -36,9 +83,14 @@ #define str_intro_point_start "\n" str_intro_point " " /* Constant string value for the construction to encrypt the encrypted data * section. */ -#define str_enc_hsdir_data "hsdir-superencrypted-data" +#define str_enc_const_superencryption "hsdir-superencrypted-data" +#define str_enc_const_encryption "hsdir-encrypted-data" /* Prefix required to compute/verify HS desc signatures */ #define str_desc_sig_prefix "Tor onion service descriptor sig v3" +#define str_desc_auth_type "desc-auth-type" +#define str_desc_auth_key "desc-auth-ephemeral-key" +#define str_desc_auth_client "auth-client" +#define str_encrypted "encrypted" /* Authentication supported types. */ static const struct { @@ -393,7 +445,8 @@ build_secret_input(const hs_descriptor_t *desc, uint8_t *dst, size_t dstlen) static void build_kdf_key(const hs_descriptor_t *desc, const uint8_t *salt, size_t salt_len, - uint8_t *key_out, size_t key_out_len) + uint8_t *key_out, size_t key_out_len, + int is_superencrypted_layer) { uint8_t secret_input[HS_DESC_ENCRYPTED_SECRET_INPUT_LEN]; crypto_xof_t *xof; @@ -409,8 +462,16 @@ build_kdf_key(const hs_descriptor_t *desc, /* Feed our KDF. [SHAKE it like a polaroid picture --Yawning]. */ crypto_xof_add_bytes(xof, secret_input, sizeof(secret_input)); crypto_xof_add_bytes(xof, salt, salt_len); - crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_hsdir_data, - strlen(str_enc_hsdir_data)); + + /* Feed in the right string constant based on the desc layer */ + if (is_superencrypted_layer) { + crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_const_superencryption, + strlen(str_enc_const_superencryption)); + } else { + crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_const_encryption, + strlen(str_enc_const_encryption)); + } + /* Eat from our KDF. */ crypto_xof_squeeze_bytes(xof, key_out, key_out_len); crypto_xof_free(xof); @@ -425,7 +486,8 @@ build_secret_key_iv_mac(const hs_descriptor_t *desc, const uint8_t *salt, size_t salt_len, uint8_t *key_out, size_t key_len, uint8_t *iv_out, size_t iv_len, - uint8_t *mac_out, size_t mac_len) + uint8_t *mac_out, size_t mac_len, + int is_superencrypted_layer) { size_t offset = 0; uint8_t kdf_key[HS_DESC_ENCRYPTED_KDF_OUTPUT_LEN]; @@ -436,7 +498,8 @@ build_secret_key_iv_mac(const hs_descriptor_t *desc, tor_assert(iv_out); tor_assert(mac_out); - build_kdf_key(desc, salt, salt_len, kdf_key, sizeof(kdf_key)); + build_kdf_key(desc, salt, salt_len, kdf_key, sizeof(kdf_key), + is_superencrypted_layer); /* Copy the bytes we need for both the secret key and IV. */ memcpy(key_out, kdf_key, key_len); offset += key_len; @@ -529,7 +592,8 @@ build_plaintext_padding(const char *plaintext, size_t plaintext_len, * data. Return size of the encrypted data buffer. */ static size_t build_encrypted(const uint8_t *key, const uint8_t *iv, const char *plaintext, - size_t plaintext_len, uint8_t **encrypted_out) + size_t plaintext_len, uint8_t **encrypted_out, + int is_superencrypted_layer) { size_t encrypted_len; uint8_t *padded_plaintext, *encrypted; @@ -540,15 +604,21 @@ build_encrypted(const uint8_t *key, const uint8_t *iv, const char *plaintext, tor_assert(plaintext); tor_assert(encrypted_out); + /* If we are encrypting the middle layer of the descriptor, we need to first + pad the plaintext */ + if (is_superencrypted_layer) { + encrypted_len = build_plaintext_padding(plaintext, plaintext_len, + &padded_plaintext); + /* Extra precautions that we have a valid padding length. */ + tor_assert(!(encrypted_len % HS_DESC_PLAINTEXT_PADDING_MULTIPLE)); + } else { /* No padding required for inner layers */ + padded_plaintext = tor_memdup(plaintext, plaintext_len); + encrypted_len = plaintext_len; + } + /* This creates a cipher for AES. It can't fail. */ cipher = crypto_cipher_new_with_iv_and_bits(key, iv, HS_DESC_ENCRYPTED_BIT_SIZE); - /* This can't fail. */ - encrypted_len = build_plaintext_padding(plaintext, plaintext_len, - &padded_plaintext); - /* Extra precautions that we have a valie padding length. */ - tor_assert(encrypted_len <= HS_DESC_PADDED_PLAINTEXT_MAX_LEN); - tor_assert(!(encrypted_len % HS_DESC_PLAINTEXT_PADDING_MULTIPLE)); /* We use a stream cipher so the encrypted length will be the same as the * plaintext padded length. */ encrypted = tor_malloc_zero(encrypted_len); @@ -562,12 +632,13 @@ build_encrypted(const uint8_t *key, const uint8_t *iv, const char *plaintext, return encrypted_len; } -/* Encrypt the given plaintext buffer and using the descriptor to get the +/* Encrypt the given plaintext buffer using desc to get the * keys. Set encrypted_out with the encrypted data and return the length of - * it. */ + * it. is_superencrypted_layer is set if this is the outer encrypted + * layer of the descriptor. */ static size_t encrypt_descriptor_data(const hs_descriptor_t *desc, const char *plaintext, - char **encrypted_out) + char **encrypted_out, int is_superencrypted_layer) { char *final_blob; size_t encrypted_len, final_blob_len, offset = 0; @@ -588,11 +659,13 @@ encrypt_descriptor_data(const hs_descriptor_t *desc, const char *plaintext, build_secret_key_iv_mac(desc, salt, sizeof(salt), secret_key, sizeof(secret_key), secret_iv, sizeof(secret_iv), - mac_key, sizeof(mac_key)); + mac_key, sizeof(mac_key), + is_superencrypted_layer); /* Build the encrypted part that is do the actual encryption. */ encrypted_len = build_encrypted(secret_key, secret_iv, plaintext, - strlen(plaintext), &encrypted); + strlen(plaintext), &encrypted, + is_superencrypted_layer); memwipe(secret_key, 0, sizeof(secret_key)); memwipe(secret_iv, 0, sizeof(secret_iv)); /* This construction is specified in section 2.5 of proposal 224. */ From b2e37b87a71704aa5274a8c9d47a6740f5953cf4 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Wed, 8 Feb 2017 14:43:43 +0200 Subject: [PATCH 4/9] prop224: Implement encoding of superencrypted HS descriptor. Also, relaxed the checks of encrypted_data_length_is_valid() since now only one encrypted section has padding requirements and we don't actually care to check that all the padding is there. Consider starting code review from function encode_superencrypted_data(). --- src/or/hs_descriptor.c | 295 +++++++++++++++++++++++++++++++++-------- src/or/hs_descriptor.h | 19 +-- 2 files changed, 245 insertions(+), 69 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 3b9ee8aa2d..db158233b7 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -554,8 +554,8 @@ compute_padded_plaintext_length(size_t plaintext_len) tor_assert(plaintext_len <= (SIZE_T_CEILING - HS_DESC_PLAINTEXT_PADDING_MULTIPLE)); - /* Get the extra length we need to add. For example, if srclen is 234 bytes, - * this will expand to (2 * 128) == 256 thus an extra 22 bytes. */ + /* Get the extra length we need to add. For example, if srclen is 10200 + * bytes, this will expand to (2 * 10k) == 20k thus an extra 9800 bytes. */ plaintext_padded_len = CEIL_DIV(plaintext_len, HS_DESC_PLAINTEXT_PADDING_MULTIPLE) * HS_DESC_PLAINTEXT_PADDING_MULTIPLE; @@ -697,20 +697,89 @@ encrypt_descriptor_data(const hs_descriptor_t *desc, const char *plaintext, return final_blob_len; } -/* Take care of encoding the encrypted data section and then encrypting it - * with the descriptor's key. A newly allocated NUL terminated string pointer - * containing the encrypted encoded blob is put in encrypted_blob_out. Return - * 0 on success else a negative value. */ -static int -encode_encrypted_data(const hs_descriptor_t *desc, - char **encrypted_blob_out) +/* Create and return a string containing a fake client-auth entry. It's the + * responsibility of the caller to free the returned string. This function will + * never fail. */ +static char * +get_fake_auth_client_str(void) { - int ret = -1; - char *encoded_str, *encrypted_blob; - smartlist_t *lines = smartlist_new(); + char *auth_client_str = NULL; + /* We are gonna fill these arrays with fake base64 data. They are all double + * the size of their binary representation to fit the base64 overhead. */ + char client_id_b64[8*2]; + char iv_b64[16*2]; + char encrypted_cookie_b64[16*2]; + int retval; - tor_assert(desc); - tor_assert(encrypted_blob_out); + /* This is a macro to fill a field with random data and then base64 it. */ +#define FILL_WITH_FAKE_DATA_AND_BASE64(field) STMT_BEGIN \ + crypto_rand((char *)field, sizeof(field)); \ + retval = base64_encode_nopad(field##_b64, sizeof(field##_b64), \ + field, sizeof(field)); \ + tor_assert(retval > 0); \ + STMT_END + + { /* Get those fakes! */ + uint8_t client_id[8]; /* fake client-id */ + uint8_t iv[16]; /* fake IV (initialization vector) */ + uint8_t encrypted_cookie[16]; /* fake encrypted cookie */ + + FILL_WITH_FAKE_DATA_AND_BASE64(client_id); + FILL_WITH_FAKE_DATA_AND_BASE64(iv); + FILL_WITH_FAKE_DATA_AND_BASE64(encrypted_cookie); + } + + /* Build the final string */ + tor_asprintf(&auth_client_str, "%s %s %s %s", str_desc_auth_client, + client_id_b64, iv_b64, encrypted_cookie_b64); + +#undef FILL_WITH_FAKE_DATA_AND_BASE64 + + return auth_client_str; +} + +/** How many lines of "client-auth" we want in our descriptors; fake or not. */ +#define CLIENT_AUTH_ENTRIES_BLOCK_SIZE 16 + +/** Create the "client-auth" part of the descriptor and return a + * newly-allocated string with it. It's the responsibility of the caller to + * free the returned string. */ +static char * +get_fake_auth_client_lines(void) +{ + /* XXX: Client authorization is still not implemented, so all this function + does is make fake clients */ + int i = 0; + smartlist_t *auth_client_lines = smartlist_new(); + char *auth_client_lines_str = NULL; + + /* Make a line for each fake client */ + const int num_fake_clients = CLIENT_AUTH_ENTRIES_BLOCK_SIZE; + for (i = 0; i < num_fake_clients; i++) { + char *auth_client_str = get_fake_auth_client_str(); + tor_assert(auth_client_str); + smartlist_add(auth_client_lines, auth_client_str); + } + + /* Join all lines together to form final string */ + auth_client_lines_str = smartlist_join_strings(auth_client_lines, + "\n", 1, NULL); + /* Cleanup the mess */ + SMARTLIST_FOREACH(auth_client_lines, char *, a, tor_free(a)); + smartlist_free(auth_client_lines); + + return auth_client_lines_str; +} + +/* Create the inner layer of the descriptor (which includes the intro points, + * etc.). Return a newly-allocated string with the layer plaintext, or NULL if + * an error occured. It's the responsibility of the caller to free the returned + * string. */ +static char * +get_inner_encrypted_layer_plaintext(const hs_descriptor_t *desc) +{ + char *encoded_str = NULL; + smartlist_t *lines = smartlist_new(); /* Build the start of the section prior to the introduction points. */ { @@ -751,31 +820,159 @@ encode_encrypted_data(const hs_descriptor_t *desc, * then encrypt it. */ encoded_str = smartlist_join_strings(lines, "", 0, NULL); - /* Encrypt the section into an encrypted blob that we'll base64 encode - * before returning it. */ - { - char *enc_b64; - ssize_t enc_b64_len, ret_len, enc_len; + err: + SMARTLIST_FOREACH(lines, char *, l, tor_free(l)); + smartlist_free(lines); - enc_len = encrypt_descriptor_data(desc, encoded_str, &encrypted_blob); - tor_free(encoded_str); - /* Get the encoded size plus a NUL terminating byte. */ - enc_b64_len = base64_encode_size(enc_len, BASE64_ENCODE_MULTILINE) + 1; - enc_b64 = tor_malloc_zero(enc_b64_len); - /* Base64 the encrypted blob before returning it. */ - ret_len = base64_encode(enc_b64, enc_b64_len, encrypted_blob, enc_len, - BASE64_ENCODE_MULTILINE); - /* Return length doesn't count the NUL byte. */ - tor_assert(ret_len == (enc_b64_len - 1)); - tor_free(encrypted_blob); - *encrypted_blob_out = enc_b64; + return encoded_str; +} + +/* Create the middle layer of the descriptor, which includes the client auth + * data and the encrypted inner layer (provided as a base64 string at + * layer2_b64_ciphertext). Return a newly-allocated string with the + * layer plaintext, or NULL if an error occured. It's the responsibility of the + * caller to free the returned string. */ +static char * +get_outer_encrypted_layer_plaintext(const hs_descriptor_t *desc, + const char *layer2_b64_ciphertext) +{ + char *layer1_str = NULL; + smartlist_t *lines = smartlist_new(); + + /* XXX: Disclaimer: This function generates only _fake_ client auth + * data. Real client auth is not yet implemented, but client auth data MUST + * always be present in descriptors. In the future this function will be + * refactored to use real client auth data if they exist (#20700). */ + (void) *desc; + + /* Specify auth type */ + smartlist_add_asprintf(lines, "%s %s\n", str_desc_auth_type, "x25519"); + + { /* Create fake ephemeral x25519 key */ + char fake_key_base64[CURVE25519_BASE64_PADDED_LEN + 1]; + curve25519_keypair_t fake_x25519_keypair; + if (curve25519_keypair_generate(&fake_x25519_keypair, 0) < 0) { + goto done; + } + if (curve25519_public_to_base64(fake_key_base64, + &fake_x25519_keypair.pubkey) < 0) { + goto done; + } + smartlist_add_asprintf(lines, "%s %s\n", + str_desc_auth_key, fake_key_base64); + /* No need to memwipe any of these fake keys. They will go unused. */ } + + { /* Create fake auth-client lines. */ + char *auth_client_lines = get_fake_auth_client_lines(); + tor_assert(auth_client_lines); + smartlist_add(lines, auth_client_lines); + } + + /* create encrypted section */ + { + smartlist_add_asprintf(lines, + "%s\n" + "-----BEGIN MESSAGE-----\n" + "%s" + "-----END MESSAGE-----", + str_encrypted, layer2_b64_ciphertext); + } + + layer1_str = smartlist_join_strings(lines, "", 0, NULL); + + done: + SMARTLIST_FOREACH(lines, char *, a, tor_free(a)); + smartlist_free(lines); + + return layer1_str; +} + +/* Encrypt encoded_str into an encrypted blob and then base64 it before + * returning it. desc is provided to derive the encryption + * keys. is_superencrypted_layer is set if encoded_str is the + * middle (superencrypted) layer of the descriptor. It's the responsibility of + * the caller to free the returned string. */ +static char * +encrypt_desc_data_and_base64(const hs_descriptor_t *desc, + const char *encoded_str, + int is_superencrypted_layer) +{ + char *enc_b64; + ssize_t enc_b64_len, ret_len, enc_len; + char *encrypted_blob = NULL; + + enc_len = encrypt_descriptor_data(desc, encoded_str, &encrypted_blob, + is_superencrypted_layer); + /* Get the encoded size plus a NUL terminating byte. */ + enc_b64_len = base64_encode_size(enc_len, BASE64_ENCODE_MULTILINE) + 1; + enc_b64 = tor_malloc_zero(enc_b64_len); + /* Base64 the encrypted blob before returning it. */ + ret_len = base64_encode(enc_b64, enc_b64_len, encrypted_blob, enc_len, + BASE64_ENCODE_MULTILINE); + /* Return length doesn't count the NUL byte. */ + tor_assert(ret_len == (enc_b64_len - 1)); + tor_free(encrypted_blob); + + return enc_b64; +} + +/* Generate and encode the superencrypted portion of desc. This also + * involves generating the encrypted portion of the descriptor, and performing + * the superencryption. A newly allocated NUL-terminated string pointer + * containing the encrypted encoded blob is put in encrypted_blob_out. Return 0 + * on success else a negative value. */ +static int +encode_superencrypted_data(const hs_descriptor_t *desc, + char **encrypted_blob_out) +{ + int ret = -1; + char *layer2_str = NULL; + char *layer2_b64_ciphertext = NULL; + char *layer1_str = NULL; + char *layer1_b64_ciphertext = NULL; + + tor_assert(desc); + tor_assert(encrypted_blob_out); + + /* Func logic: We first create the inner layer of the descriptor (layer2). + * We then encrypt it and use it to create the middle layer of the descriptor + * (layer1). Finally we superencrypt the middle layer and return it to our + * caller. */ + + /* Create inner descriptor layer */ + layer2_str = get_inner_encrypted_layer_plaintext(desc); + if (!layer2_str) { + goto err; + } + + /* Encrypt and b64 the inner layer */ + layer2_b64_ciphertext = encrypt_desc_data_and_base64(desc, layer2_str, 0); + if (!layer2_b64_ciphertext) { + goto err; + } + + /* Now create middle descriptor layer given the inner layer */ + layer1_str = get_outer_encrypted_layer_plaintext(desc,layer2_b64_ciphertext); + if (!layer1_str) { + goto err; + } + + /* Encrypt and base64 the middle layer */ + layer1_b64_ciphertext = encrypt_desc_data_and_base64(desc, layer1_str, 1); + if (!layer1_b64_ciphertext) { + goto err; + } + /* Success! */ ret = 0; err: - SMARTLIST_FOREACH(lines, char *, l, tor_free(l)); - smartlist_free(lines); + tor_free(layer1_str); + tor_free(layer2_str); + tor_free(layer2_b64_ciphertext); + + *encrypted_blob_out = layer1_b64_ciphertext; return ret; } @@ -828,7 +1025,7 @@ desc_encode_v3(const hs_descriptor_t *desc, /* Build the superencrypted data section. */ { char *enc_b64_blob=NULL; - if (encode_encrypted_data(desc, &enc_b64_blob) < 0) { + if (encode_superencrypted_data(desc, &enc_b64_blob) < 0) { goto err; } smartlist_add_asprintf(lines, @@ -868,6 +1065,13 @@ desc_encode_v3(const hs_descriptor_t *desc, encoded_str = smartlist_join_strings(lines, "\n", 1, NULL); *encoded_out = encoded_str; + if (strlen(encoded_str) >= hs_cache_get_max_descriptor_size()) { + log_warn(LD_GENERAL, "We just made an HS descriptor that's too big (%d)." + "Failing.", (int)strlen(encoded_str)); + tor_free(encoded_str); + goto err; + } + /* XXX: Trigger a control port event. */ /* Success! */ @@ -1095,30 +1299,15 @@ cert_parse_and_validate(tor_cert_t **cert_out, const char *data, STATIC int encrypted_data_length_is_valid(size_t len) { - /* Check for the minimum length possible. */ - if (len < HS_DESC_ENCRYPTED_MIN_LEN) { + /* Make sure there is enough data for the salt and the mac. The equality is + there to ensure that there is at least one byte of encrypted data. */ + if (len <= HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN) { log_warn(LD_REND, "Length of descriptor's encrypted data is too small. " "Got %lu but minimum value is %d", - (unsigned long)len, HS_DESC_ENCRYPTED_MIN_LEN); + (unsigned long)len, HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN); goto err; } - /* Encrypted data has the salt and MAC concatenated to it so remove those - * from the validation calculation. */ - len -= HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN; - - /* Check that it's aligned on the block size of the crypto algorithm. */ - if (len % HS_DESC_PLAINTEXT_PADDING_MULTIPLE) { - log_warn(LD_REND, "Length of descriptor's encrypted data is invalid. " - "Got %lu which is not a multiple of %d.", - (unsigned long) len, HS_DESC_PLAINTEXT_PADDING_MULTIPLE); - goto err; - } - - /* XXX: Check maximum size. Will strongly depends on the maximum intro point - * allowed we decide on and probably if they will all have to use the legacy - * key which is bigger than the ed25519 key. */ - return 1; err: return 0; diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h index 3b5832bdf2..4e0e86681e 100644 --- a/src/or/hs_descriptor.h +++ b/src/or/hs_descriptor.h @@ -41,24 +41,11 @@ * the secret IV and MAC key length which is the length of H() output. */ #define HS_DESC_ENCRYPTED_KDF_OUTPUT_LEN \ CIPHER256_KEY_LEN + CIPHER_IV_LEN + DIGEST256_LEN -/* We need to pad the plaintext version of the encrypted data section before - * encryption and it has to be a multiple of this value. */ -#define HS_DESC_PLAINTEXT_PADDING_MULTIPLE 128 -/* XXX: Let's make sure this makes sense as an upper limit for the padded - * plaintext section. Then we should enforce it as now only an assert will be - * triggered if we are above it. */ -/* Once padded, this is the maximum length in bytes for the plaintext. */ -#define HS_DESC_PADDED_PLAINTEXT_MAX_LEN 8192 -/* Minimum length in bytes of the encrypted portion of the descriptor. */ -#define HS_DESC_ENCRYPTED_MIN_LEN \ - HS_DESC_ENCRYPTED_SALT_LEN + \ - HS_DESC_PLAINTEXT_PADDING_MULTIPLE + DIGEST256_LEN +/* Pad plaintext of superencrypted data section before encryption so that its + * length is a multiple of this value. */ +#define HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE 10000 /* Maximum length in bytes of a full hidden service descriptor. */ #define HS_DESC_MAX_LEN 50000 /* 50kb max size */ -/* The minimum amount of fields a descriptor should contain. The parsing of - * the fields are version specific so the only required field, as a generic - * view of a descriptor, is 1 that is the version field. */ -#define HS_DESC_PLAINTEXT_MIN_FIELDS 1 /* Key length for the descriptor symmetric encryption. As specified in the * protocol, we use AES-256 for the encrypted section of the descriptor. The From d0fe199269addb9cab5070691a8b3be186d49986 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Wed, 8 Feb 2017 14:42:12 +0200 Subject: [PATCH 5/9] prop224: Implement decoding of superencrypted HS descriptor. [Consider starting review from desc_decrypt_all() ] --- src/or/hs_descriptor.c | 230 +++++++++++++++++++++++++++++++++++------ src/or/hs_descriptor.h | 2 + src/or/parsecommon.h | 4 + 3 files changed, 202 insertions(+), 34 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index db158233b7..54788e60ae 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -113,6 +113,15 @@ static token_rule_t hs_desc_v3_token_table[] = { END_OF_TABLE }; +/* Descriptor ruleset for the superencrypted section. */ +static token_rule_t hs_desc_superencrypted_v3_token_table[] = { + T1_START(str_desc_auth_type, R3_DESC_AUTH_TYPE, GE(1), NO_OBJ), + T1(str_desc_auth_key, R3_DESC_AUTH_KEY, GE(1), NO_OBJ), + T1N(str_desc_auth_client, R3_DESC_AUTH_CLIENT, GE(3), NO_OBJ), + T1(str_encrypted, R3_ENCRYPTED, NO_ARGS, NEED_OBJ), + END_OF_TABLE +}; + /* Descriptor ruleset for the encrypted section. */ static token_rule_t hs_desc_encrypted_v3_token_table[] = { T1_START(str_create2_formats, R3_CREATE2_FORMATS, CONCAT_ARGS, NO_OBJ), @@ -1313,12 +1322,17 @@ encrypted_data_length_is_valid(size_t len) return 0; } -/* Decrypt the encrypted section of the descriptor using the given descriptor - * object desc. A newly allocated NUL terminated string is put in - * decrypted_out. Return the length of decrypted_out on success else 0 is - * returned and decrypted_out is set to NULL. */ +/** Decrypt an encrypted descriptor layer at encrypted_blob of size + * encrypted_blob_size. Use the descriptor object desc to + * generate the right decryption keys; set decrypted_out to the + * plaintext. If is_superencrypted_layer is set, this is the outter + * encrypted layer of the descriptor. */ static size_t -desc_decrypt_data_v3(const hs_descriptor_t *desc, char **decrypted_out) +decrypt_desc_layer(const hs_descriptor_t *desc, + const uint8_t *encrypted_blob, + size_t encrypted_blob_size, + int is_superencrypted_layer, + char **decrypted_out) { uint8_t *decrypted = NULL; uint8_t secret_key[HS_DESC_ENCRYPTED_KEY_LEN], secret_iv[CIPHER_IV_LEN]; @@ -1328,41 +1342,33 @@ desc_decrypt_data_v3(const hs_descriptor_t *desc, char **decrypted_out) tor_assert(decrypted_out); tor_assert(desc); - tor_assert(desc->plaintext_data.superencrypted_blob); + tor_assert(encrypted_blob); - /* Construction is as follow: SALT | ENCRYPTED_DATA | MAC */ - if (!encrypted_data_length_is_valid( - desc->plaintext_data.superencrypted_blob_size)) { + /* Construction is as follow: SALT | ENCRYPTED_DATA | MAC . + * Make sure we have enough space for all these things. */ + if (!encrypted_data_length_is_valid(encrypted_blob_size)) { goto err; } /* Start of the blob thus the salt. */ - salt = desc->plaintext_data.superencrypted_blob; - /* Next is the encrypted data. */ - encrypted = desc->plaintext_data.superencrypted_blob + - HS_DESC_ENCRYPTED_SALT_LEN; - encrypted_len = desc->plaintext_data.superencrypted_blob_size - - (HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN); + salt = encrypted_blob; - /* At the very end is the MAC. Make sure it's of the right size. */ - { - desc_mac = encrypted + encrypted_len; - size_t desc_mac_size = desc->plaintext_data.superencrypted_blob_size - - (desc_mac - desc->plaintext_data.superencrypted_blob); - if (desc_mac_size != DIGEST256_LEN) { - log_warn(LD_REND, "Service descriptor MAC length of encrypted data " - "is invalid (%lu, expected %u)", - (unsigned long) desc_mac_size, DIGEST256_LEN); - goto err; - } - } + /* Next is the encrypted data. */ + encrypted = encrypted_blob + HS_DESC_ENCRYPTED_SALT_LEN; + encrypted_len = encrypted_blob_size - + (HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN); + tor_assert(encrypted_len > 0); /* guaranteed by the check above */ + + /* And last comes the MAC. */ + desc_mac = encrypted_blob + encrypted_blob_size - DIGEST256_LEN; /* KDF construction resulting in a key from which the secret key, IV and MAC * key are extracted which is what we need for the decryption. */ build_secret_key_iv_mac(desc, salt, HS_DESC_ENCRYPTED_SALT_LEN, secret_key, sizeof(secret_key), secret_iv, sizeof(secret_iv), - mac_key, sizeof(mac_key)); + mac_key, sizeof(mac_key), + is_superencrypted_layer); /* Build MAC. */ build_mac(mac_key, sizeof(mac_key), salt, HS_DESC_ENCRYPTED_SALT_LEN, @@ -1392,7 +1398,7 @@ desc_decrypt_data_v3(const hs_descriptor_t *desc, char **decrypted_out) } { - /* Adjust length to remove NULL padding bytes */ + /* Adjust length to remove NUL padding bytes */ uint8_t *end = memchr(decrypted, 0, encrypted_len); result_len = encrypted_len; if (end) { @@ -1418,6 +1424,161 @@ desc_decrypt_data_v3(const hs_descriptor_t *desc, char **decrypted_out) return result_len; } +/* Basic validation that the superencrypted client auth portion of the + * descriptor is well-formed and recognized. Return True if so, otherwise + * return False. */ +static int +superencrypted_auth_data_is_valid(smartlist_t *tokens) +{ + /* XXX: This is just basic validation for now. When we implement client auth, + we can refactor this function so that it actually parses and saves the + data. */ + + { /* verify desc auth type */ + const directory_token_t *tok; + tok = find_by_keyword(tokens, R3_DESC_AUTH_TYPE); + tor_assert(tok->n_args >= 1); + if (strcmp(tok->args[0], "x25519")) { + return 0; + } + } + + { /* verify desc auth key */ + const directory_token_t *tok; + curve25519_public_key_t k; + tok = find_by_keyword(tokens, R3_DESC_AUTH_KEY); + tor_assert(tok->n_args >= 1); + if (curve25519_public_from_base64(&k, tok->args[0]) < 0) { + log_warn(LD_DIR, "Bogus desc auth key in HS desc"); + return 0; + } + } + + /* verify desc auth client items */ + SMARTLIST_FOREACH_BEGIN(tokens, const directory_token_t *, tok) { + if (tok->tp == R3_DESC_AUTH_CLIENT) { + tor_assert(tok->n_args >= 3); + } + } SMARTLIST_FOREACH_END(tok); + + return 1; +} + +/* Parse message, the plaintext of the superencrypted portion of an HS + * descriptor. Set encrypted_out to the encrypted blob, and return its + * size */ +STATIC size_t +decode_superencrypted(const char *message, size_t message_len, + uint8_t **encrypted_out) +{ + int retval = 0; + memarea_t *area = NULL; + smartlist_t *tokens = NULL; + + area = memarea_new(); + tokens = smartlist_new(); + if (tokenize_string(area, message, message + message_len, tokens, + hs_desc_superencrypted_v3_token_table, 0) < 0) { + log_warn(LD_REND, "Superencrypted portion is not parseable"); + goto err; + } + + /* Do some rudimentary validation of the authentication data */ + if (!superencrypted_auth_data_is_valid(tokens)) { + goto err; + } + + /* Extract the encrypted data section. */ + { + const directory_token_t *tok; + tok = find_by_keyword(tokens, R3_ENCRYPTED); + tor_assert(tok->object_body); + if (strcmp(tok->object_type, "MESSAGE") != 0) { + log_warn(LD_REND, "Desc superencrypted data section is invalid"); + goto err; + } + /* Make sure the length of the encrypted blob is valid. */ + if (!encrypted_data_length_is_valid(tok->object_size)) { + goto err; + } + + /* Copy the encrypted blob to the descriptor object so we can handle it + * latter if needed. */ + *encrypted_out = tor_memdup(tok->object_body, tok->object_size); + retval = tok->object_size; + } + + err: + SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); + smartlist_free(tokens); + if (area) { + memarea_drop_all(area); + } + + return retval; +} + +/* Decrypt both the superencrypted and the encrypted section of the descriptor + * using the given descriptor object desc. A newly allocated NUL + * terminated string is put in decrypted_out which contains the inner encrypted + * layer of the descriptor. Return the length of decrypted_out on success else + * 0 is returned and decrypted_out is set to NULL. */ +static size_t +desc_decrypt_all(const hs_descriptor_t *desc, char **decrypted_out) +{ + size_t decrypted_len = 0; + size_t encrypted_len = 0; + size_t superencrypted_len = 0; + char *superencrypted_plaintext = NULL; + uint8_t *encrypted_blob = NULL; + + /** Function logic: This function takes us from the descriptor header to the + * inner encrypted layer, by decrypting and decoding the middle descriptor + * layer. In the end we return the contents of the inner encrypted layer to + * our caller. */ + + /* 1. Decrypt middle layer of descriptor */ + superencrypted_len = decrypt_desc_layer(desc, + desc->plaintext_data.superencrypted_blob, + desc->plaintext_data.superencrypted_blob_size, + 1, + &superencrypted_plaintext); + if (!superencrypted_len) { + log_warn(LD_REND, "Decrypting superencrypted desc failed."); + goto err; + } + tor_assert(superencrypted_plaintext); + + /* 2. Parse "superencrypted" */ + encrypted_len = decode_superencrypted(superencrypted_plaintext, + superencrypted_len, + &encrypted_blob); + if (!encrypted_len) { + log_warn(LD_REND, "Decrypting encrypted desc failed."); + goto err; + } + tor_assert(encrypted_blob); + + /* 3. Decrypt "encrypted" and set decrypted_out */ + char *decrypted_desc; + decrypted_len = decrypt_desc_layer(desc, + encrypted_blob, encrypted_len, + 0, &decrypted_desc); + if (!decrypted_len) { + log_warn(LD_REND, "Decrypting encrypted desc failed."); + goto err; + } + tor_assert(decrypted_desc); + + *decrypted_out = decrypted_desc; + + err: + tor_free(superencrypted_plaintext); + tor_free(encrypted_blob); + + return decrypted_len; +} + /* Given the start of a section and the end of it, decode a single * introduction point from that section. Return a newly allocated introduction * point object containing the decoded data. Return NULL if the section can't @@ -1550,7 +1711,9 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start) tor_cert_free(cross_cert); SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t)); smartlist_free(tokens); - memarea_drop_all(area); + if (area) { + memarea_drop_all(area); + } return ip; } @@ -1804,10 +1967,9 @@ desc_decode_encrypted_v3(const hs_descriptor_t *desc, tor_assert(desc); tor_assert(desc_encrypted_out); - /* Decrypt the encrypted data that is located in the plaintext section in - * the descriptor as a blob of bytes. The following functions will use the - * keys found in the same section. */ - message_len = desc_decrypt_data_v3(desc, &message); + /* Decrypt the superencrypted data that is located in the plaintext section + * in the descriptor as a blob of bytes. */ + message_len = desc_decrypt_all(desc, &message); if (!message_len) { log_warn(LD_REND, "Service descriptor decryption failed."); goto err; diff --git a/src/or/hs_descriptor.h b/src/or/hs_descriptor.h index 4e0e86681e..711514f82e 100644 --- a/src/or/hs_descriptor.h +++ b/src/or/hs_descriptor.h @@ -228,6 +228,8 @@ STATIC int desc_sig_is_valid(const char *b64_sig, const ed25519_public_key_t *signing_pubkey, const char *encoded_desc, size_t encoded_len); STATIC void desc_intro_point_free(hs_desc_intro_point_t *ip); +STATIC size_t decode_superencrypted(const char *message, size_t message_len, + uint8_t **encrypted_out); #endif /* HS_DESCRIPTOR_PRIVATE */ #endif /* TOR_HS_DESCRIPTOR_H */ diff --git a/src/or/parsecommon.h b/src/or/parsecommon.h index ec62bade92..0af9dfb5a2 100644 --- a/src/or/parsecommon.h +++ b/src/or/parsecommon.h @@ -163,6 +163,10 @@ typedef enum { R3_INTRO_AUTH_KEY, R3_INTRO_ENC_KEY, R3_INTRO_ENC_KEY_CERTIFICATION, + R3_DESC_AUTH_TYPE, + R3_DESC_AUTH_KEY, + R3_DESC_AUTH_CLIENT, + R3_ENCRYPTED, R_IPO_IDENTIFIER, R_IPO_IP_ADDRESS, From 1f421d8d47e914bfc826616498c22efbb7289b2e Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Wed, 8 Feb 2017 14:22:22 +0200 Subject: [PATCH 6/9] prop224: Fix the HS descriptor unittests. - HS descriptors are now bigger than 10kb. - encrypted_data_length_is_valid() is not that strict now. --- src/test/test_hs_cache.c | 2 +- src/test/test_hs_descriptor.c | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 64fc1c1e6e..8dac3d1c4b 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -333,7 +333,7 @@ helper_fetch_desc_from_hsdir(const ed25519_public_key_t *blinded_key) size_t body_used = 0; fetch_from_buf_http(TO_CONN(conn)->outbuf, &headers, MAX_HEADERS_SIZE, - &received_desc, &body_used, 10000, 0); + &received_desc, &body_used, HS_DESC_MAX_LEN, 0); tor_free(headers); } diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 4042e647da..bd361be692 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -590,19 +590,11 @@ test_encrypted_data_len(void *arg) /* No length, error. */ ret = encrypted_data_length_is_valid(0); tt_int_op(ret, OP_EQ, 0); - /* Not a multiple of our encryption algorithm (thus no padding). It's - * suppose to be aligned on HS_DESC_PLAINTEXT_PADDING_MULTIPLE. */ - value = HS_DESC_PLAINTEXT_PADDING_MULTIPLE * 10 - 1; - ret = encrypted_data_length_is_valid(value); - tt_int_op(ret, OP_EQ, 0); /* Valid value. */ - value = HS_DESC_PADDED_PLAINTEXT_MAX_LEN + HS_DESC_ENCRYPTED_SALT_LEN + - DIGEST256_LEN; + value = HS_DESC_ENCRYPTED_SALT_LEN + DIGEST256_LEN + 1; ret = encrypted_data_length_is_valid(value); tt_int_op(ret, OP_EQ, 1); - /* XXX: Test maximum possible size. */ - done: ; } From 163596d9c25d4836dde37675372271d8358ffc81 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Wed, 8 Feb 2017 13:22:36 +0200 Subject: [PATCH 7/9] prop224: Move some utility crypto funcs to the top of the file. --- src/or/hs_descriptor.c | 258 ++++++++++++++++++++--------------------- 1 file changed, 129 insertions(+), 129 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 54788e60ae..7c43d604c2 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -195,6 +195,135 @@ desc_encrypted_data_free_contents(hs_desc_encrypted_data_t *desc) memwipe(desc, 0, sizeof(*desc)); } +/* Using a key, salt and encrypted payload, build a MAC and put it in mac_out. + * We use SHA3-256 for the MAC computation. + * This function can't fail. */ +static void +build_mac(const uint8_t *mac_key, size_t mac_key_len, + const uint8_t *salt, size_t salt_len, + const uint8_t *encrypted, size_t encrypted_len, + uint8_t *mac_out, size_t mac_len) +{ + crypto_digest_t *digest; + + const uint64_t mac_len_netorder = tor_htonll(mac_key_len); + const uint64_t salt_len_netorder = tor_htonll(salt_len); + + tor_assert(mac_key); + tor_assert(salt); + tor_assert(encrypted); + tor_assert(mac_out); + + digest = crypto_digest256_new(DIGEST_SHA3_256); + /* As specified in section 2.5 of proposal 224, first add the mac key + * then add the salt first and then the encrypted section. */ + + crypto_digest_add_bytes(digest, (const char *) &mac_len_netorder, 8); + crypto_digest_add_bytes(digest, (const char *) mac_key, mac_key_len); + crypto_digest_add_bytes(digest, (const char *) &salt_len_netorder, 8); + crypto_digest_add_bytes(digest, (const char *) salt, salt_len); + crypto_digest_add_bytes(digest, (const char *) encrypted, encrypted_len); + crypto_digest_get_digest(digest, (char *) mac_out, mac_len); + crypto_digest_free(digest); +} + +/* Using a given decriptor object, build the secret input needed for the + * KDF and put it in the dst pointer which is an already allocated buffer + * of size dstlen. */ +static void +build_secret_input(const hs_descriptor_t *desc, uint8_t *dst, size_t dstlen) +{ + size_t offset = 0; + + tor_assert(desc); + tor_assert(dst); + tor_assert(HS_DESC_ENCRYPTED_SECRET_INPUT_LEN <= dstlen); + + /* XXX use the destination length as the memcpy length */ + /* Copy blinded public key. */ + memcpy(dst, desc->plaintext_data.blinded_pubkey.pubkey, + sizeof(desc->plaintext_data.blinded_pubkey.pubkey)); + offset += sizeof(desc->plaintext_data.blinded_pubkey.pubkey); + /* Copy subcredential. */ + memcpy(dst + offset, desc->subcredential, sizeof(desc->subcredential)); + offset += sizeof(desc->subcredential); + /* Copy revision counter value. */ + set_uint64(dst + offset, tor_ntohll(desc->plaintext_data.revision_counter)); + offset += sizeof(uint64_t); + tor_assert(HS_DESC_ENCRYPTED_SECRET_INPUT_LEN == offset); +} + +/* Do the KDF construction and put the resulting data in key_out which is of + * key_out_len length. It uses SHAKE-256 as specified in the spec. */ +static void +build_kdf_key(const hs_descriptor_t *desc, + const uint8_t *salt, size_t salt_len, + uint8_t *key_out, size_t key_out_len, + int is_superencrypted_layer) +{ + uint8_t secret_input[HS_DESC_ENCRYPTED_SECRET_INPUT_LEN]; + crypto_xof_t *xof; + + tor_assert(desc); + tor_assert(salt); + tor_assert(key_out); + + /* Build the secret input for the KDF computation. */ + build_secret_input(desc, secret_input, sizeof(secret_input)); + + xof = crypto_xof_new(); + /* Feed our KDF. [SHAKE it like a polaroid picture --Yawning]. */ + crypto_xof_add_bytes(xof, secret_input, sizeof(secret_input)); + crypto_xof_add_bytes(xof, salt, salt_len); + + /* Feed in the right string constant based on the desc layer */ + if (is_superencrypted_layer) { + crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_const_superencryption, + strlen(str_enc_const_superencryption)); + } else { + crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_const_encryption, + strlen(str_enc_const_encryption)); + } + + /* Eat from our KDF. */ + crypto_xof_squeeze_bytes(xof, key_out, key_out_len); + crypto_xof_free(xof); + memwipe(secret_input, 0, sizeof(secret_input)); +} + +/* Using the given descriptor and salt, run it through our KDF function and + * then extract a secret key in key_out, the IV in iv_out and MAC in mac_out. + * This function can't fail. */ +static void +build_secret_key_iv_mac(const hs_descriptor_t *desc, + const uint8_t *salt, size_t salt_len, + uint8_t *key_out, size_t key_len, + uint8_t *iv_out, size_t iv_len, + uint8_t *mac_out, size_t mac_len, + int is_superencrypted_layer) +{ + size_t offset = 0; + uint8_t kdf_key[HS_DESC_ENCRYPTED_KDF_OUTPUT_LEN]; + + tor_assert(desc); + tor_assert(salt); + tor_assert(key_out); + tor_assert(iv_out); + tor_assert(mac_out); + + build_kdf_key(desc, salt, salt_len, kdf_key, sizeof(kdf_key), + is_superencrypted_layer); + /* Copy the bytes we need for both the secret key and IV. */ + memcpy(key_out, kdf_key, key_len); + offset += key_len; + memcpy(iv_out, kdf_key + offset, iv_len); + offset += iv_len; + memcpy(mac_out, kdf_key + offset, mac_len); + /* Extra precaution to make sure we are not out of bound. */ + tor_assert((offset + mac_len) == sizeof(kdf_key)); + memwipe(kdf_key, 0, sizeof(kdf_key)); +} + /* === ENCODING === */ /* Encode the given link specifier objects into a newly allocated string. @@ -423,135 +552,6 @@ encode_intro_point(const ed25519_public_key_t *sig_key, return encoded_ip; } -/* Using a given decriptor object, build the secret input needed for the - * KDF and put it in the dst pointer which is an already allocated buffer - * of size dstlen. */ -static void -build_secret_input(const hs_descriptor_t *desc, uint8_t *dst, size_t dstlen) -{ - size_t offset = 0; - - tor_assert(desc); - tor_assert(dst); - tor_assert(HS_DESC_ENCRYPTED_SECRET_INPUT_LEN <= dstlen); - - /* XXX use the destination length as the memcpy length */ - /* Copy blinded public key. */ - memcpy(dst, desc->plaintext_data.blinded_pubkey.pubkey, - sizeof(desc->plaintext_data.blinded_pubkey.pubkey)); - offset += sizeof(desc->plaintext_data.blinded_pubkey.pubkey); - /* Copy subcredential. */ - memcpy(dst + offset, desc->subcredential, sizeof(desc->subcredential)); - offset += sizeof(desc->subcredential); - /* Copy revision counter value. */ - set_uint64(dst + offset, tor_ntohll(desc->plaintext_data.revision_counter)); - offset += sizeof(uint64_t); - tor_assert(HS_DESC_ENCRYPTED_SECRET_INPUT_LEN == offset); -} - -/* Do the KDF construction and put the resulting data in key_out which is of - * key_out_len length. It uses SHAKE-256 as specified in the spec. */ -static void -build_kdf_key(const hs_descriptor_t *desc, - const uint8_t *salt, size_t salt_len, - uint8_t *key_out, size_t key_out_len, - int is_superencrypted_layer) -{ - uint8_t secret_input[HS_DESC_ENCRYPTED_SECRET_INPUT_LEN]; - crypto_xof_t *xof; - - tor_assert(desc); - tor_assert(salt); - tor_assert(key_out); - - /* Build the secret input for the KDF computation. */ - build_secret_input(desc, secret_input, sizeof(secret_input)); - - xof = crypto_xof_new(); - /* Feed our KDF. [SHAKE it like a polaroid picture --Yawning]. */ - crypto_xof_add_bytes(xof, secret_input, sizeof(secret_input)); - crypto_xof_add_bytes(xof, salt, salt_len); - - /* Feed in the right string constant based on the desc layer */ - if (is_superencrypted_layer) { - crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_const_superencryption, - strlen(str_enc_const_superencryption)); - } else { - crypto_xof_add_bytes(xof, (const uint8_t *) str_enc_const_encryption, - strlen(str_enc_const_encryption)); - } - - /* Eat from our KDF. */ - crypto_xof_squeeze_bytes(xof, key_out, key_out_len); - crypto_xof_free(xof); - memwipe(secret_input, 0, sizeof(secret_input)); -} - -/* Using the given descriptor and salt, run it through our KDF function and - * then extract a secret key in key_out, the IV in iv_out and MAC in mac_out. - * This function can't fail. */ -static void -build_secret_key_iv_mac(const hs_descriptor_t *desc, - const uint8_t *salt, size_t salt_len, - uint8_t *key_out, size_t key_len, - uint8_t *iv_out, size_t iv_len, - uint8_t *mac_out, size_t mac_len, - int is_superencrypted_layer) -{ - size_t offset = 0; - uint8_t kdf_key[HS_DESC_ENCRYPTED_KDF_OUTPUT_LEN]; - - tor_assert(desc); - tor_assert(salt); - tor_assert(key_out); - tor_assert(iv_out); - tor_assert(mac_out); - - build_kdf_key(desc, salt, salt_len, kdf_key, sizeof(kdf_key), - is_superencrypted_layer); - /* Copy the bytes we need for both the secret key and IV. */ - memcpy(key_out, kdf_key, key_len); - offset += key_len; - memcpy(iv_out, kdf_key + offset, iv_len); - offset += iv_len; - memcpy(mac_out, kdf_key + offset, mac_len); - /* Extra precaution to make sure we are not out of bound. */ - tor_assert((offset + mac_len) == sizeof(kdf_key)); - memwipe(kdf_key, 0, sizeof(kdf_key)); -} - -/* Using a key, salt and encrypted payload, build a MAC and put it in mac_out. - * We use SHA3-256 for the MAC computation. - * This function can't fail. */ -static void -build_mac(const uint8_t *mac_key, size_t mac_key_len, - const uint8_t *salt, size_t salt_len, - const uint8_t *encrypted, size_t encrypted_len, - uint8_t *mac_out, size_t mac_len) -{ - crypto_digest_t *digest; - - const uint64_t mac_len_netorder = tor_htonll(mac_key_len); - const uint64_t salt_len_netorder = tor_htonll(salt_len); - - tor_assert(mac_key); - tor_assert(salt); - tor_assert(encrypted); - tor_assert(mac_out); - - digest = crypto_digest256_new(DIGEST_SHA3_256); - /* As specified in section 2.5 of proposal 224, first add the mac key - * then add the salt first and then the encrypted section. */ - - crypto_digest_add_bytes(digest, (const char *) &mac_len_netorder, 8); - crypto_digest_add_bytes(digest, (const char *) mac_key, mac_key_len); - crypto_digest_add_bytes(digest, (const char *) &salt_len_netorder, 8); - crypto_digest_add_bytes(digest, (const char *) salt, salt_len); - crypto_digest_add_bytes(digest, (const char *) encrypted, encrypted_len); - crypto_digest_get_digest(digest, (char *) mac_out, mac_len); - crypto_digest_free(digest); -} - /* Given a source length, return the new size including padding for the * plaintext encryption. */ static size_t From e6b03151fb98a40f9f039424e3c3e8c99ce41371 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Tue, 14 Feb 2017 17:36:00 +0200 Subject: [PATCH 8/9] prop224: Add unittests for decode_superencrypted(). --- src/or/hs_descriptor.c | 2 + src/test/test_hs_descriptor.c | 103 ++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 7c43d604c2..d15c16054f 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -1439,6 +1439,7 @@ superencrypted_auth_data_is_valid(smartlist_t *tokens) tok = find_by_keyword(tokens, R3_DESC_AUTH_TYPE); tor_assert(tok->n_args >= 1); if (strcmp(tok->args[0], "x25519")) { + log_warn(LD_DIR, "Unrecognized desc auth type"); return 0; } } @@ -1485,6 +1486,7 @@ decode_superencrypted(const char *message, size_t message_len, /* Do some rudimentary validation of the authentication data */ if (!superencrypted_auth_data_is_valid(tokens)) { + log_warn(LD_REND, "Invalid auth data"); goto err; } diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index bd361be692..2a2188c2dc 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -15,6 +15,9 @@ #include "test.h" #include "torcert.h" +#include "test_helpers.h" +#include "log_test_helpers.h" + static hs_desc_intro_point_t * helper_build_intro_point(const ed25519_keypair_t *blinded_kp, time_t now, const char *addr, int legacy) @@ -1001,6 +1004,103 @@ test_desc_signature(void *arg) tor_free(data); } +/* bad desc auth type */ +const char bad_superencrypted_text1[] = "desc-auth-type scoobysnack\n" + "desc-auth-ephemeral-key A/O8DVtnUheb3r1JqoB8uJB7wxXL1XJX3eny4yB+eFA=\n" + "auth-client oiNrQB8WwKo S5D02W7vKgiWIMygrBl8RQ FB//SfOBmLEx1kViEWWL1g\n" + "encrypted\n" + "-----BEGIN MESSAGE-----\n" + "YmVpbmcgb24gbW91bnRhaW5zLCB0aGlua2luZyBhYm91dCBjb21wdXRlcnMsIGlzIG5vdC" + "BiYWQgYXQgYWxs\n" + "-----END MESSAGE-----\n"; + +/* bad ephemeral key */ +const char bad_superencrypted_text2[] = "desc-auth-type x25519\n" + "desc-auth-ephemeral-key differentalphabet\n" + "auth-client oiNrQB8WwKo S5D02W7vKgiWIMygrBl8RQ FB//SfOBmLEx1kViEWWL1g\n" + "encrypted\n" + "-----BEGIN MESSAGE-----\n" + "YmVpbmcgb24gbW91bnRhaW5zLCB0aGlua2luZyBhYm91dCBjb21wdXRlcnMsIGlzIG5vdC" + "BiYWQgYXQgYWxs\n" + "-----END MESSAGE-----\n"; + +/* bad encrypted msg */ +const char bad_superencrypted_text3[] = "desc-auth-type x25519\n" + "desc-auth-ephemeral-key A/O8DVtnUheb3r1JqoB8uJB7wxXL1XJX3eny4yB+eFA=\n" + "auth-client oiNrQB8WwKo S5D02W7vKgiWIMygrBl8RQ FB//SfOBmLEx1kViEWWL1g\n" + "encrypted\n" + "-----BEGIN MESSAGE-----\n" + "SO SMALL NOT GOOD\n" + "-----END MESSAGE-----\n"; + +const char correct_superencrypted_text[] = "desc-auth-type x25519\n" + "desc-auth-ephemeral-key A/O8DVtnUheb3r1JqoB8uJB7wxXL1XJX3eny4yB+eFA=\n" + "auth-client oiNrQB8WwKo S5D02W7vKgiWIMygrBl8RQ FB//SfOBmLEx1kViEWWL1g\n" + "auth-client Od09Qu636Qo /PKLzqewAdS/+0+vZC+MvQ dpw4NFo13zDnuPz45rxrOg\n" + "auth-client JRr840iGYN0 8s8cxYqF7Lx23+NducC4Qg zAafl4wPLURkuEjJreZq1g\n" + "encrypted\n" + "-----BEGIN MESSAGE-----\n" + "YmVpbmcgb24gbW91bnRhaW5zLCB0aGlua2luZyBhYm91dCBjb21wdXRlcnMsIGlzIG5vdC" + "BiYWQgYXQgYWxs\n" + "-----END MESSAGE-----\n"; + +const char correct_encrypted_plaintext[] = "being on mountains, " + "thinking about computers, is not bad at all"; + +static void +test_parse_hs_desc_superencrypted(void *arg) +{ + (void) arg; + int retval; + uint8_t *encrypted_out = NULL; + + { + setup_full_capture_of_logs(LOG_WARN); + retval = decode_superencrypted(bad_superencrypted_text1, + strlen(bad_superencrypted_text1), + &encrypted_out); + tt_int_op(retval, ==, 0); + tt_assert(!encrypted_out); + expect_log_msg_containing("Unrecognized desc auth type"); + teardown_capture_of_logs(); + } + + { + setup_full_capture_of_logs(LOG_WARN); + retval = decode_superencrypted(bad_superencrypted_text2, + strlen(bad_superencrypted_text2), + &encrypted_out); + tt_int_op(retval, ==, 0); + tt_assert(!encrypted_out); + expect_log_msg_containing("Bogus desc auth key in HS desc"); + teardown_capture_of_logs(); + } + + { + setup_full_capture_of_logs(LOG_WARN); + retval = decode_superencrypted(bad_superencrypted_text3, + strlen(bad_superencrypted_text3), + &encrypted_out); + tt_int_op(retval, ==, 0); + tt_assert(!encrypted_out); + expect_log_msg_containing("Length of descriptor\'s encrypted data " + "is too small."); + teardown_capture_of_logs(); + } + + /* Now finally the good one */ + retval = decode_superencrypted(correct_superencrypted_text, + strlen(correct_superencrypted_text), + &encrypted_out); + + tt_int_op(retval, ==, strlen(correct_encrypted_plaintext)); + tt_mem_op(encrypted_out, OP_EQ, correct_encrypted_plaintext, + strlen(correct_encrypted_plaintext)); + + done: + tor_free(encrypted_out); +} + struct testcase_t hs_descriptor[] = { /* Encoding tests. */ { "cert_encoding", test_cert_encoding, TT_FORK, @@ -1030,6 +1130,9 @@ struct testcase_t hs_descriptor[] = { { "desc_signature", test_desc_signature, TT_FORK, NULL, NULL }, + { "parse_hs_desc_superencrypted", test_parse_hs_desc_superencrypted, + TT_FORK, NULL, NULL }, + END_OF_TESTCASES }; From 61f318b1b0bb1cb55d4f54eab87518a1b093ba08 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Fri, 24 Feb 2017 12:44:34 +0200 Subject: [PATCH 9/9] prop224: Rename padding size def to something less confusing. People felt it could refer to the descriptor header section instead of the plaintext of the superencrypted section. --- src/or/hs_descriptor.c | 13 ++++++------- src/test/test_hs_descriptor.c | 10 +++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index d15c16054f..f441fbf03b 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -558,18 +558,17 @@ static size_t compute_padded_plaintext_length(size_t plaintext_len) { size_t plaintext_padded_len; + const int padding_block_length = HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE; /* Make sure we won't overflow. */ - tor_assert(plaintext_len <= - (SIZE_T_CEILING - HS_DESC_PLAINTEXT_PADDING_MULTIPLE)); + tor_assert(plaintext_len <= (SIZE_T_CEILING - padding_block_length)); /* Get the extra length we need to add. For example, if srclen is 10200 * bytes, this will expand to (2 * 10k) == 20k thus an extra 9800 bytes. */ - plaintext_padded_len = CEIL_DIV(plaintext_len, - HS_DESC_PLAINTEXT_PADDING_MULTIPLE) * - HS_DESC_PLAINTEXT_PADDING_MULTIPLE; + plaintext_padded_len = CEIL_DIV(plaintext_len, padding_block_length) * + padding_block_length; /* Can never be extra careful. Make sure we are _really_ padded. */ - tor_assert(!(plaintext_padded_len % HS_DESC_PLAINTEXT_PADDING_MULTIPLE)); + tor_assert(!(plaintext_padded_len % padding_block_length)); return plaintext_padded_len; } @@ -619,7 +618,7 @@ build_encrypted(const uint8_t *key, const uint8_t *iv, const char *plaintext, encrypted_len = build_plaintext_padding(plaintext, plaintext_len, &padded_plaintext); /* Extra precautions that we have a valid padding length. */ - tor_assert(!(encrypted_len % HS_DESC_PLAINTEXT_PADDING_MULTIPLE)); + tor_assert(!(encrypted_len % HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE)); } else { /* No padding required for inner layers */ padded_plaintext = tor_memdup(plaintext, plaintext_len); encrypted_len = plaintext_len; diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 2a2188c2dc..1a1b51169e 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -317,13 +317,13 @@ test_descriptor_padding(void *arg) /* Example: if l = 129, the ceiled division gives 2 and then multiplied by 128 * to give 256. With l = 127, ceiled division gives 1 then times 128. */ #define PADDING_EXPECTED_LEN(l) \ - CEIL_DIV(l, HS_DESC_PLAINTEXT_PADDING_MULTIPLE) * \ - HS_DESC_PLAINTEXT_PADDING_MULTIPLE + CEIL_DIV(l, HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE) * \ + HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE (void) arg; { /* test #1: no padding */ - plaintext_len = HS_DESC_PLAINTEXT_PADDING_MULTIPLE; + plaintext_len = HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE; plaintext = tor_malloc(plaintext_len); padded_len = build_plaintext_padding(plaintext, plaintext_len, &padded_plaintext); @@ -339,7 +339,7 @@ test_descriptor_padding(void *arg) } { /* test #2: one byte padding? */ - plaintext_len = HS_DESC_PLAINTEXT_PADDING_MULTIPLE - 1; + plaintext_len = HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE - 1; plaintext = tor_malloc(plaintext_len); padded_plaintext = NULL; padded_len = build_plaintext_padding(plaintext, plaintext_len, @@ -356,7 +356,7 @@ test_descriptor_padding(void *arg) } { /* test #3: Lots more bytes of padding? */ - plaintext_len = HS_DESC_PLAINTEXT_PADDING_MULTIPLE + 1; + plaintext_len = HS_DESC_SUPERENC_PLAINTEXT_PAD_MULTIPLE + 1; plaintext = tor_malloc(plaintext_len); padded_plaintext = NULL; padded_len = build_plaintext_padding(plaintext, plaintext_len,