commit d5a23ce115bf89046cc0e9deb64b288113c8dd92 Author: John Brooks <john.brooks@dereferenced.net> Date: Mon Apr 13 21:35:06 2015 -0600 Move rend auth cookie en-/decoding to a function Tor stores client authorization cookies in two slightly different forms. The service's client_keys file has the standard base64-encoded cookie, including two chars of padding. The hostname file and the client remove the two padding chars, and store an auth type flag in the unused bits. The distinction makes no sense. Refactor all decoding to use the same function, which will accept either form, and use a helper function for encoding the truncated format. --- src/or/rendclient.c | 34 +++------------- src/or/rendcommon.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/or/rendcommon.h | 8 ++++ src/or/rendservice.c | 37 +++++++---------- src/or/routerparse.c | 22 +++-------- src/test/test_hs.c | 63 +++++++++++++++++++++++++++++ 6 files changed, 208 insertions(+), 66 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index a39e518..57c7b31 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -1465,12 +1465,10 @@ rend_parse_service_authorization(const or_options_t *options, strmap_t *parsed = strmap_new(); smartlist_t *sl = smartlist_new(); rend_service_authorization_t *auth = NULL; - char descriptor_cookie_tmp[REND_DESC_COOKIE_LEN+2]; - char descriptor_cookie_base64ext[REND_DESC_COOKIE_LEN_BASE64+2+1]; + char *err_msg = NULL; for (line = options->HidServAuth; line; line = line->next) { char *onion_address, *descriptor_cookie; - int auth_type_val = 0; auth = NULL; SMARTLIST_FOREACH(sl, char *, c, tor_free(c);); smartlist_clear(sl); @@ -1499,31 +1497,13 @@ rend_parse_service_authorization(const or_options_t *options, } /* Parse descriptor cookie. */ descriptor_cookie = smartlist_get(sl, 1); - if (strlen(descriptor_cookie) != REND_DESC_COOKIE_LEN_BASE64) { - log_warn(LD_CONFIG, "Authorization cookie has wrong length: '%s'", - descriptor_cookie); + if (rend_auth_decode_cookie(descriptor_cookie, auth->descriptor_cookie, + &auth->auth_type, &err_msg) < 0) { + tor_assert(err_msg); + log_warn(LD_CONFIG, "%s", err_msg); + tor_free(err_msg); goto err; } - /* Add trailing zero bytes (AA) to make base64-decoding happy. */ - tor_snprintf(descriptor_cookie_base64ext, - REND_DESC_COOKIE_LEN_BASE64+2+1, - "%sAA", descriptor_cookie); - if (base64_decode(descriptor_cookie_tmp, sizeof(descriptor_cookie_tmp), - descriptor_cookie_base64ext, - strlen(descriptor_cookie_base64ext)) < 0) { - log_warn(LD_CONFIG, "Decoding authorization cookie failed: '%s'", - descriptor_cookie); - goto err; - } - auth_type_val = (((uint8_t)descriptor_cookie_tmp[16]) >> 4) + 1; - if (auth_type_val < 1 || auth_type_val > 2) { - log_warn(LD_CONFIG, "Authorization cookie has unknown authorization " - "type encoded."); - goto err; - } - auth->auth_type = auth_type_val == 1 ? REND_BASIC_AUTH : REND_STEALTH_AUTH; - memcpy(auth->descriptor_cookie, descriptor_cookie_tmp, - REND_DESC_COOKIE_LEN); if (strmap_get(parsed, auth->onion_address)) { log_warn(LD_CONFIG, "Duplicate authorization for the same hidden " "service."); @@ -1546,8 +1526,6 @@ rend_parse_service_authorization(const or_options_t *options, } else { strmap_free(parsed, rend_service_authorization_strmap_item_free); } - memwipe(descriptor_cookie_tmp, 0, sizeof(descriptor_cookie_tmp)); - memwipe(descriptor_cookie_base64ext, 0, sizeof(descriptor_cookie_base64ext)); return res; } diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 22599e9..a9fdf84 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -936,3 +936,113 @@ rend_data_client_create(const char *onion_address, const char *desc_id, return NULL; } +/* Length of the 'extended' auth cookie used to encode auth type before + * base64 encoding. */ +#define REND_DESC_COOKIE_LEN_EXT (REND_DESC_COOKIE_LEN + 1) +/* Length of the zero-padded auth cookie when base64 encoded. These two + * padding bytes always (A=) are stripped off of the returned cookie. */ +#define REND_DESC_COOKIE_LEN_EXT_BASE64 (REND_DESC_COOKIE_LEN_BASE64 + 2) + +/** Encode a client authorization descriptor cookie. + * The result of this function is suitable for use in the HidServAuth + * option. The trailing padding characters are removed, and the + * auth type is encoded into the cookie. + * + * Returns a new base64-encoded cookie. This function cannot fail. + * The caller is responsible for freeing the returned value. + */ +char * +rend_auth_encode_cookie(const uint8_t *cookie_in, rend_auth_type_t auth_type) +{ + uint8_t extended_cookie[REND_DESC_COOKIE_LEN_EXT]; + char *cookie_out = tor_malloc_zero(REND_DESC_COOKIE_LEN_EXT_BASE64 + 1); + int re; + + tor_assert(cookie_in); + + memcpy(extended_cookie, cookie_in, REND_DESC_COOKIE_LEN); + extended_cookie[REND_DESC_COOKIE_LEN] = ((int)auth_type - 1) << 4; + re = base64_encode(cookie_out, REND_DESC_COOKIE_LEN_EXT_BASE64 + 1, + (const char *) extended_cookie, REND_DESC_COOKIE_LEN_EXT, + 0); + tor_assert(re == REND_DESC_COOKIE_LEN_EXT_BASE64); + + /* Remove the trailing 'A='. Auth type is encoded in the high bits + * of the last byte, so the last base64 character will always be zero + * (A). This is subtly different behavior from base64_encode_nopad. */ + cookie_out[REND_DESC_COOKIE_LEN_BASE64] = '\0'; + memwipe(extended_cookie, 0, sizeof(extended_cookie)); + return cookie_out; +} + +/** Decode a base64-encoded client authorization descriptor cookie. + * The descriptor_cookie can be truncated to REND_DESC_COOKIE_LEN_BASE64 + * characters (as given to clients), or may include the two padding + * characters (as stored by the service). + * + * The result is stored in REND_DESC_COOKIE_LEN bytes of cookie_out. + * The rend_auth_type_t decoded from the cookie is stored in the + * optional auth_type_out parameter. + * + * Return 0 on success, or -1 on error. The caller is responsible for + * freeing the returned err_msg. + */ +int +rend_auth_decode_cookie(const char *cookie_in, uint8_t *cookie_out, + rend_auth_type_t *auth_type_out, char **err_msg_out) +{ + uint8_t descriptor_cookie_decoded[REND_DESC_COOKIE_LEN_EXT + 1] = { 0 }; + char descriptor_cookie_base64ext[REND_DESC_COOKIE_LEN_EXT_BASE64 + 1]; + const char *descriptor_cookie = cookie_in; + char *err_msg = NULL; + int auth_type_val = 0; + int res = -1; + int decoded_len; + + size_t len = strlen(descriptor_cookie); + if (len == REND_DESC_COOKIE_LEN_BASE64) { + /* Add a trailing zero byte to make base64-decoding happy. */ + tor_snprintf(descriptor_cookie_base64ext, + sizeof(descriptor_cookie_base64ext), + "%sA=", descriptor_cookie); + descriptor_cookie = descriptor_cookie_base64ext; + } else if (len != REND_DESC_COOKIE_LEN_EXT_BASE64) { + tor_asprintf(&err_msg, "Authorization cookie has wrong length: %s", + escaped(cookie_in)); + goto err; + } + + decoded_len = base64_decode((char *) descriptor_cookie_decoded, + sizeof(descriptor_cookie_decoded), + descriptor_cookie, + REND_DESC_COOKIE_LEN_EXT_BASE64); + if (decoded_len != REND_DESC_COOKIE_LEN && + decoded_len != REND_DESC_COOKIE_LEN_EXT) { + tor_asprintf(&err_msg, "Authorization cookie has invalid characters: %s", + escaped(cookie_in)); + goto err; + } + + if (auth_type_out) { + auth_type_val = (descriptor_cookie_decoded[REND_DESC_COOKIE_LEN] >> 4) + 1; + if (auth_type_val < 1 || auth_type_val > 2) { + tor_asprintf(&err_msg, "Authorization cookie type is unknown: %s", + escaped(cookie_in)); + goto err; + } + *auth_type_out = auth_type_val == 1 ? REND_BASIC_AUTH : REND_STEALTH_AUTH; + } + + memcpy(cookie_out, descriptor_cookie_decoded, REND_DESC_COOKIE_LEN); + res = 0; + err: + if (err_msg_out) { + *err_msg_out = err_msg; + } else { + tor_free(err_msg); + } + memwipe(descriptor_cookie_decoded, 0, sizeof(descriptor_cookie_decoded)); + memwipe(descriptor_cookie_base64ext, 0, sizeof(descriptor_cookie_base64ext)); + return res; +} + diff --git a/src/or/rendcommon.h b/src/or/rendcommon.h index 3b2f86d..983cd73 100644 --- a/src/or/rendcommon.h +++ b/src/or/rendcommon.h @@ -67,5 +67,13 @@ rend_data_t *rend_data_service_create(const char *onion_address, const char *pk_digest, const uint8_t *cookie, rend_auth_type_t auth_type); + +char *rend_auth_encode_cookie(const uint8_t *cookie_in, + rend_auth_type_t auth_type); +int rend_auth_decode_cookie(const char *cookie_in, + uint8_t *cookie_out, + rend_auth_type_t *auth_type_out, + char **err_msg_out); + #endif diff --git a/src/or/rendservice.c b/src/or/rendservice.c index fbc228a..e6bceed 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1157,7 +1157,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) strmap_t *parsed_clients = strmap_new(); FILE *cfile, *hfile; open_file_t *open_cfile = NULL, *open_hfile = NULL; - char extended_desc_cookie[REND_DESC_COOKIE_LEN+1]; char desc_cook_out[3*REND_DESC_COOKIE_LEN_BASE64+1]; char service_id[16+1]; char buf[1500]; @@ -1211,6 +1210,8 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) } else { crypto_rand((char *) client->descriptor_cookie, REND_DESC_COOKIE_LEN); } + /* For compatibility with older tor clients, this does not + * truncate the padding characters, unlike rend_auth_encode_cookie. */ if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1, (char *) client->descriptor_cookie, REND_DESC_COOKIE_LEN, 0) < 0) { @@ -1273,6 +1274,8 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) log_warn(LD_BUG, "Could not write client entry."); goto err; } + } else { + strlcpy(service_id, s->service_id, sizeof(service_id)); } if (fputs(buf, cfile) < 0) { @@ -1281,27 +1284,18 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) goto err; } - /* Add line to hostname file. */ - if (s->auth_type == REND_BASIC_AUTH) { - /* Remove == signs (newline has been removed above). */ - desc_cook_out[strlen(desc_cook_out)-2] = '\0'; - tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n", - s->service_id, desc_cook_out, client->client_name); - } else { - memcpy(extended_desc_cookie, client->descriptor_cookie, - REND_DESC_COOKIE_LEN); - extended_desc_cookie[REND_DESC_COOKIE_LEN] = - ((int)s->auth_type - 1) << 4; - if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1, - extended_desc_cookie, - REND_DESC_COOKIE_LEN+1, 0) < 0) { - log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); - goto err; - } - desc_cook_out[strlen(desc_cook_out)-2] = '\0'; /* Remove A=. */ - tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n", - service_id, desc_cook_out, client->client_name); + /* Add line to hostname file. This is not the same encoding as in + * client_keys. */ + char *encoded_cookie = rend_auth_encode_cookie(client->descriptor_cookie, + s->auth_type); + if (!encoded_cookie) { + log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); + goto err; } + tor_snprintf(buf, sizeof(buf), "%s.onion %s # client: %s\n", + service_id, encoded_cookie, client->client_name); + memwipe(encoded_cookie, 0, strlen(encoded_cookie)); + tor_free(encoded_cookie); if (fputs(buf, hfile)<0) { log_warn(LD_FS, "Could not append host entry to file: %s", @@ -1333,7 +1327,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) memwipe(buf, 0, sizeof(buf)); memwipe(desc_cook_out, 0, sizeof(desc_cook_out)); memwipe(service_id, 0, sizeof(service_id)); - memwipe(extended_desc_cookie, 0, sizeof(extended_desc_cookie)); return r; } diff --git a/src/or/routerparse.c b/src/or/routerparse.c index c2206f1..35f76cd 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5290,6 +5290,7 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr) directory_token_t *tok; const char *current_entry = NULL; memarea_t *area = NULL; + char *err_msg = NULL; if (!ckstr || strlen(ckstr) == 0) return -1; tokens = smartlist_new(); @@ -5300,7 +5301,6 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr) while (!strcmpstart(current_entry, "client-name ")) { rend_authorized_client_t *parsed_entry; size_t len; - char descriptor_cookie_tmp[REND_DESC_COOKIE_LEN+2]; /* Determine end of string. */ const char *eos = strstr(current_entry, "\nclient-name "); if (!eos) @@ -5356,23 +5356,13 @@ rend_parse_client_keys(strmap_t *parsed_clients, const char *ckstr) /* Parse descriptor cookie. */ tok = find_by_keyword(tokens, C_DESCRIPTOR_COOKIE); tor_assert(tok->n_args == 1); - if (strlen(tok->args[0]) != REND_DESC_COOKIE_LEN_BASE64 + 2) { - log_warn(LD_REND, "Descriptor cookie has illegal length: %s", - escaped(tok->args[0])); - goto err; - } - /* The size of descriptor_cookie_tmp needs to be REND_DESC_COOKIE_LEN+2, - * because a base64 encoding of length 24 does not fit into 16 bytes in all - * cases. */ - if (base64_decode(descriptor_cookie_tmp, sizeof(descriptor_cookie_tmp), - tok->args[0], strlen(tok->args[0])) - != REND_DESC_COOKIE_LEN) { - log_warn(LD_REND, "Descriptor cookie contains illegal characters: " - "%s", escaped(tok->args[0])); + if (rend_auth_decode_cookie(tok->args[0], parsed_entry->descriptor_cookie, + NULL, &err_msg) < 0) { + tor_assert(err_msg); + log_warn(LD_REND, "%s", err_msg); + tor_free(err_msg); goto err; } - memcpy(parsed_entry->descriptor_cookie, descriptor_cookie_tmp, - REND_DESC_COOKIE_LEN); } result = strmap_size(parsed_clients); goto done; diff --git a/src/test/test_hs.c b/src/test/test_hs.c index 126e211..ea5253a 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -435,6 +435,67 @@ test_hs_rend_data(void *arg) rend_data_free(client_dup); } +/* Test encoding and decoding service authorization cookies */ +static void +test_hs_auth_cookies(void *arg) +{ +#define TEST_COOKIE_RAW ((const uint8_t *) "abcdefghijklmnop") +#define TEST_COOKIE_ENCODED "YWJjZGVmZ2hpamtsbW5vcA" +#define TEST_COOKIE_ENCODED_STEALTH "YWJjZGVmZ2hpamtsbW5vcB" +#define TEST_COOKIE_ENCODED_INVALID "YWJjZGVmZ2hpamtsbW5vcD" + + char *encoded_cookie; + uint8_t raw_cookie[REND_DESC_COOKIE_LEN]; + rend_auth_type_t auth_type; + char *err_msg; + int re; + + (void)arg; + + /* Test that encoding gives the expected result */ + encoded_cookie = rend_auth_encode_cookie(TEST_COOKIE_RAW, REND_BASIC_AUTH); + tt_str_op(encoded_cookie, OP_EQ, TEST_COOKIE_ENCODED); + tor_free(encoded_cookie); + + encoded_cookie = rend_auth_encode_cookie(TEST_COOKIE_RAW, REND_STEALTH_AUTH); + tt_str_op(encoded_cookie, OP_EQ, TEST_COOKIE_ENCODED_STEALTH); + tor_free(encoded_cookie); + + /* Decoding should give the original value */ + re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED, raw_cookie, &auth_type, + &err_msg); + tt_assert(!re); + tt_assert(!err_msg); + tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN); + tt_int_op(auth_type, OP_EQ, REND_BASIC_AUTH); + memset(raw_cookie, 0, sizeof(raw_cookie)); + + re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED_STEALTH, raw_cookie, + &auth_type, &err_msg); + tt_assert(!re); + tt_assert(!err_msg); + tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN); + tt_int_op(auth_type, OP_EQ, REND_STEALTH_AUTH); + memset(raw_cookie, 0, sizeof(raw_cookie)); + + /* Decoding with padding characters should also work */ + re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED "==", raw_cookie, NULL, + &err_msg); + tt_assert(!re); + tt_assert(!err_msg); + tt_mem_op(raw_cookie, OP_EQ, TEST_COOKIE_RAW, REND_DESC_COOKIE_LEN); + + /* Decoding with an unknown type should fail */ + re = rend_auth_decode_cookie(TEST_COOKIE_ENCODED_INVALID, raw_cookie, + &auth_type, &err_msg); + tt_int_op(re, OP_LT, 0); + tt_assert(err_msg); + tor_free(err_msg); + + done: + return; +} + struct testcase_t hs_tests[] = { { "hs_rend_data", test_hs_rend_data, TT_FORK, NULL, NULL }, @@ -445,6 +506,8 @@ struct testcase_t hs_tests[] = { { "pick_bad_tor2web_rendezvous_node", test_pick_bad_tor2web_rendezvous_node, TT_FORK, NULL, NULL }, + { "hs_auth_cookies", test_hs_auth_cookies, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };