From 70572b9abd660448777ebbee3dc71d7d70b37d8c Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 25 Nov 2019 15:55:12 +0200 Subject: [PATCH 1/6] hsv3: Implement permanent storage of auth credentials. - See hs_client_register_auth_credentials() for the entry point. - Also set the permanent flag for credentials we read from the filesystem. - Also add some missing documentation. --- src/feature/control/control_hs.c | 4 ++ src/feature/hs/hs_client.c | 95 +++++++++++++++++++++++++++++++- src/feature/hs/hs_client.h | 2 + 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/feature/control/control_hs.c b/src/feature/control/control_hs.c index 4c1d16a8c8..97938211d2 100644 --- a/src/feature/control/control_hs.c +++ b/src/feature/control/control_hs.c @@ -145,6 +145,10 @@ handle_control_onion_client_auth_add(control_connection_t *conn, /* It's a bug because the service addr has already been validated above */ control_printf_endreply(conn, 512, "Invalid v3 address \"%s\"", hsaddress); break; + case REGISTER_FAIL_PERMANENT_STORAGE: + control_printf_endreply(conn, 553, "Unable to store creds for \"%s\"", + hsaddress); + break; case REGISTER_SUCCESS_ALREADY_EXISTS: control_printf_endreply(conn, 251,"Client for onion existed and replaced"); break; diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 787b29b576..3c681dd85e 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1445,6 +1445,80 @@ client_dir_fetch_unexpected(dir_connection_t *dir_conn, const char *reason, NULL); } +/** Get the full filename for storing the client auth credentials for the + * service in onion_address. The base directory is dir. + * This function never returns NULL. */ +static char * +get_client_auth_creds_filename(const char *onion_address, + const char *dir) +{ + char *full_fname = NULL; + char *fname; + + tor_asprintf(&fname, "%s.auth_private", onion_address); + full_fname = hs_path_from_filename(dir, fname); + tor_free(fname); + + return full_fname; +} + +/** Permanently store the credentials in creds to disk. + * + * Return -1 if there was an error while storing the credentials, otherwise + * return 0. + */ +static int +store_permanent_client_auth_credentials( + const hs_client_service_authorization_t *creds) +{ + const or_options_t *options = get_options(); + char *full_fname = NULL; + char *file_contents = NULL; + char priv_key_b32[BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)+1]; + int retval = -1; + + tor_assert(creds->flags & CLIENT_AUTH_FLAG_IS_PERMANENT); + + /* We need ClientOnionAuthDir to be set, otherwise we can't proceed */ + if (!options->ClientOnionAuthDir) { + log_warn(LD_GENERAL, "Can't register permanent client auth credentials " + "for %s without ClientOnionAuthDir option. Discarding.", + creds->onion_address); + goto err; + } + + /* Make sure the directory exists and is private enough. */ + if (check_private_dir(options->ClientOnionAuthDir, 0, options->User) < 0) { + goto err; + } + + /* Get filename that we should store the credentials */ + full_fname = get_client_auth_creds_filename(creds->onion_address, + options->ClientOnionAuthDir); + + /* Encode client private key */ + base32_encode(priv_key_b32, sizeof(priv_key_b32), + (char*)creds->enc_seckey.secret_key, + sizeof(creds->enc_seckey.secret_key)); + + /* Get the full file contents and write it to disk! */ + tor_asprintf(&file_contents, "%s:descriptor:x25519:%s", + creds->onion_address, priv_key_b32); + if (write_str_to_file(full_fname, file_contents, 0) < 0) { + log_warn(LD_GENERAL, "Failed to write client auth creds file for %s!", + creds->onion_address); + goto err; + } + + retval = 0; + + err: + tor_free(file_contents); + tor_free(full_fname); + + return retval; +} + /** Register the credential creds as part of the client auth subsystem. * * Takes ownership of creds. @@ -1468,6 +1542,15 @@ hs_client_register_auth_credentials(hs_client_service_authorization_t *creds) return REGISTER_FAIL_BAD_ADDRESS; } + /* If we reach this point, the credentials will be stored one way or another: + * Make them permanent if the user asked us to. */ + if (creds->flags & CLIENT_AUTH_FLAG_IS_PERMANENT) { + if (store_permanent_client_auth_credentials(creds) < 0) { + client_service_authorization_free(creds); + return REGISTER_FAIL_PERMANENT_STORAGE; + } + } + old_creds = digest256map_get(client_auths, service_identity_pk.pubkey); if (old_creds) { digest256map_remove(client_auths, service_identity_pk.pubkey); @@ -1795,6 +1878,13 @@ auth_key_filename_is_valid(const char *filename) return ret; } +/** Parse the client auth credentials off a string in client_key_str + * based on the file format documented in the "Client side configuration" + * section of rend-spec-v3.txt. + * + * Return NULL if there was an error, otherwise return a newly allocated + * hs_client_service_authorization_t structure. + */ STATIC hs_client_service_authorization_t * parse_auth_file_content(const char *client_key_str) { @@ -1825,7 +1915,7 @@ parse_auth_file_content(const char *client_key_str) goto err; } - if (strlen(seckey_b32) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) { + if (strlen(seckey_b32) != BASE32_NOPAD_LEN(CURVE25519_SECKEY_LEN)) { log_warn(LD_REND, "Client authorization encoded base32 private key " "length is invalid: %s", seckey_b32); goto err; @@ -1842,6 +1932,9 @@ parse_auth_file_content(const char *client_key_str) } strncpy(auth->onion_address, onion_address, HS_SERVICE_ADDR_LEN_BASE32); + /* We are reading this from the disk, so set the permanent flag anyway. */ + auth->flags |= CLIENT_AUTH_FLAG_IS_PERMANENT; + /* Success. */ goto done; diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 04827ea92a..75a9111077 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -43,6 +43,8 @@ typedef enum { REGISTER_SUCCESS_AND_DECRYPTED, /* We failed to register these credentials, because of a bad HS address. */ REGISTER_FAIL_BAD_ADDRESS, + /* We failed to register these credentials, because of a bad HS address. */ + REGISTER_FAIL_PERMANENT_STORAGE, } hs_client_register_auth_status_t; /* Status code of client auth credential removal */ From c7c9899bc4d4d1b0ff6df31fe0526133b23cd8bd Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 25 Nov 2019 15:55:47 +0200 Subject: [PATCH 2/6] hsv3: Add tests for permanently storing auth credentials. Remove Permanent flag from old tests, and make a new test that does all the permanent things. --- src/test/test_hs_control.c | 157 ++++++++++++++++++++++++++++++++++++- 1 file changed, 154 insertions(+), 3 deletions(-) diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index 9279080329..0de2bca043 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -30,6 +30,17 @@ #include "test/test_helpers.h" +#ifdef HAVE_SYS_STAT_H +#include +#endif + +#ifdef _WIN32 +/* For mkdir() */ +#include +#else +#include +#endif /* defined(_WIN32) */ + /* mock ID digest and longname for node that's in nodelist */ #define HSDIR_EXIST_ID \ "\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA" \ @@ -235,7 +246,7 @@ test_hs_control_good_onion_client_auth_add(void *arg) /* Register first service */ args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " - "ClientName=bob Flags=Permanent"); + "ClientName=bob"); retval = handle_control_command(&conn, (uint32_t) strlen(args), args); tt_int_op(retval, OP_EQ, 0); @@ -267,7 +278,7 @@ test_hs_control_good_onion_client_auth_add(void *arg) digest256map_get(client_auths, service_identity_pk_2fv.pubkey); tt_assert(client_2fv); tt_str_op(client_2fv->nickname, OP_EQ, "bob"); - tt_int_op(client_2fv->flags, OP_EQ, CLIENT_AUTH_FLAG_IS_PERMANENT); + tt_int_op(client_2fv->flags, OP_EQ, 0); hs_client_service_authorization_t *client_jt4 = digest256map_get(client_auths, service_identity_pk_jt4.pubkey); @@ -286,7 +297,7 @@ test_hs_control_good_onion_client_auth_add(void *arg) #define VIEW_CORRECT_REPLY_NO_ADDR "250-ONION_CLIENT_AUTH_VIEW\r\n" \ "250-CLIENT x25519:eIIdIGoSZwI2Q/lSzpf92akGki5I+PZIDz37MA5BhlA=\r\n"\ "250-CLIENT x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " \ - "ClientName=bob Flags=Permanent\r\n" \ + "ClientName=bob\r\n" \ "250 OK\r\n" retval = handle_control_command(&conn, (uint32_t) strlen(args), args); @@ -466,6 +477,144 @@ test_hs_control_bad_onion_client_auth_add(void *arg) hs_client_free_all(); } +/** Test that we can correctly add permanent client auth credentials using the + * control port. */ +static void +test_hs_control_store_permanent_creds(void *arg) +{ + (void) arg; + + MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock); + + int retval; + ed25519_public_key_t service_identity_pk_2fv; + control_connection_t conn; + char *args = NULL; + char *cp1 = NULL; + char *creds_file_str = NULL; + char *creds_fname = NULL; + + size_t sz; + + { /* Setup the control conn */ + memset(&conn, 0, sizeof(control_connection_t)); + TO_CONN(&conn)->outbuf = buf_new(); + conn.current_cmd = tor_strdup("ONION_CLIENT_AUTH_ADD"); + } + + { /* Setup the services */ + retval = hs_parse_address( + "2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd", + &service_identity_pk_2fv, + NULL, NULL); + tt_int_op(retval, OP_EQ, 0); + } + + digest256map_t *client_auths = get_hs_client_auths_map(); + tt_assert(!client_auths); + + /* Try registering first service with no ClientOnionAuthDir set */ + args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " + "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " + "ClientName=bob Flags=Permanent"); + + retval = handle_control_command(&conn, (uint32_t) strlen(args), args); + tt_int_op(retval, OP_EQ, 0); + + /* Check control port response. This one should fail. */ + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, "553 Unable to store creds for " + "\"2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd\"\r\n"); + + { /* Setup ClientOnionAuthDir */ + int ret; + char *perm_creds_dir = tor_strdup(get_fname("permanent_credentials")); + + #ifdef _WIN32 + ret = mkdir(perm_creds_dir); + #else + ret = mkdir(perm_creds_dir, 0700); + #endif + tt_int_op(ret, OP_EQ, 0); + + get_options_mutable()->ClientOnionAuthDir = perm_creds_dir; + } + + tor_free(args); + tor_free(cp1); + + /* Try the control port command again. This time it should work! */ + args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " + "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " + "ClientName=bob Flags=Permanent"); + retval = handle_control_command(&conn, (uint32_t) strlen(args), args); + tt_int_op(retval, OP_EQ, 0); + + /* Check control port response */ + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, "250 OK\r\n"); + + /* Check file contents! */ + creds_fname = tor_strdup(get_fname("permanent_credentials/" + "2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd.auth_private")); + creds_file_str = read_file_to_str(creds_fname, RFTS_BIN, NULL); + + tt_assert(creds_file_str); + tt_str_op(creds_file_str, OP_EQ, + "2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd:descriptor:" + /* This is the base32 represenation of the base64 iJ1t... key above */ + "x25519:rcow3dfavmyanyqvhwnvnmfdqw34ydtrgv7jnelmqs4wi4uuxrca"); + + tor_free(args); + tor_free(cp1); + + /* Overwrite the credentials and check that they got overwrited. */ + args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " + "x25519:UDRvZLvcJo0QRLvDfkpgbtsqbkhIUQZyeo2FNBrgS18= " + "ClientName=fab Flags=Permanent"); + retval = handle_control_command(&conn, (uint32_t) strlen(args), args); + tt_int_op(retval, OP_EQ, 0); + + /* Check control port response: we replaced! */ + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, "251 Client for onion existed and replaced\r\n"); + + tor_free(creds_file_str); + + /* Check creds file contents again. See that the key got updated */ + creds_file_str = read_file_to_str(creds_fname, RFTS_BIN, NULL); + tt_assert(creds_file_str); + tt_str_op(creds_file_str, OP_EQ, + "2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd:descriptor:" + /* This is the base32 represenation of the base64 UDRv... key above */ + "x25519:ka2g6zf33qti2ecexpbx4stan3nsu3sijbiqm4t2rwctigxajnpq"); + + /* Now for our final act!!! Actually get the HS client subsystem to parse the + * whole directory and make sure that it extracted the right credential! */ + hs_config_client_authorization(get_options(), 0); + + client_auths = get_hs_client_auths_map(); + tt_assert(client_auths); + tt_uint_op(digest256map_size(client_auths), OP_EQ, 1); + + hs_client_service_authorization_t *client_2fv = + digest256map_get(client_auths, service_identity_pk_2fv.pubkey); + tt_assert(client_2fv); + tt_assert(!client_2fv->nickname); + tt_int_op(client_2fv->flags, OP_EQ, CLIENT_AUTH_FLAG_IS_PERMANENT); + tt_str_op(hex_str((char*)client_2fv->enc_seckey.secret_key, 32), OP_EQ, + "50346F64BBDC268D1044BBC37E4A606EDB2A6E48485106727A8D85341AE04B5F"); + + done: + tor_free(args); + tor_free(cp1); + buf_free(TO_CONN(&conn)->outbuf); + tor_free(conn.current_cmd); + tor_free(creds_fname); + tor_free(creds_file_str); + hs_client_free_all(); +} + struct testcase_t hs_control_tests[] = { { "hs_desc_event", test_hs_desc_event, TT_FORK, NULL, NULL }, @@ -475,6 +624,8 @@ struct testcase_t hs_control_tests[] = { { "hs_control_bad_onion_client_auth_add", test_hs_control_bad_onion_client_auth_add, TT_FORK, NULL, NULL }, + { "hs_control_store_permanent_creds", + test_hs_control_store_permanent_creds, TT_FORK, NULL, NULL }, END_OF_TESTCASES }; From 9395a0c765302f5f3fd15e8d9dd2e87404cea5ee Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 25 Nov 2019 17:22:52 +0200 Subject: [PATCH 3/6] hsv3: Remove support for client auth nicknames. Because the function that parses client auth credentials saved on disk (parse_auth_file_content()) is not future compatible, there is no way to add support for storing the nickname on the disk. Hence, nicknames cannot persist after Tor restart making them pretty much useless. In the future we can introduce nicknames by adding a new file format for client auth credentials, but this was not deemed worth doing at this stage. --- src/feature/control/control_hs.c | 14 +------------- src/feature/hs/hs_client.c | 4 ---- src/feature/hs/hs_client.h | 6 ------ src/test/test_hs_control.c | 15 +++++---------- 4 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/feature/control/control_hs.c b/src/feature/control/control_hs.c index 97938211d2..94940a7396 100644 --- a/src/feature/control/control_hs.c +++ b/src/feature/control/control_hs.c @@ -73,7 +73,6 @@ const control_cmd_syntax_t onion_client_auth_add_syntax = { * register the new client-side client auth credentials: * "ONION_CLIENT_AUTH_ADD" SP HSAddress * SP KeyType ":" PrivateKeyBlob - * [SP "ClientName=" Nickname] * [SP "Type=" TYPE] CRLF */ int @@ -112,14 +111,7 @@ handle_control_onion_client_auth_add(control_connection_t *conn, /* Now let's parse the remaining arguments (variable size) */ for (const config_line_t *line = args->kwargs; line; line = line->next) { - if (!strcasecmp(line->key, "ClientName")) { - if (strlen(line->value) > HS_CLIENT_AUTH_MAX_NICKNAME_LENGTH) { - control_write_endreply(conn, 512, "Too big 'ClientName' argument"); - goto err; - } - creds->nickname = tor_strdup(line->value); - - } else if (!strcasecmpstart(line->key, "Flags")) { + if (!strcasecmpstart(line->key, "Flags")) { smartlist_split_string(flags, line->value, ",", SPLIT_IGNORE_BLANK, 0); if (smartlist_len(flags) < 1) { control_write_endreply(conn, 512, "Invalid 'Flags' argument"); @@ -249,10 +241,6 @@ encode_client_auth_cred_for_control_port( smartlist_add_asprintf(control_line, "CLIENT x25519:%s", x25519_b64); - if (cred->nickname) { /* nickname is optional */ - smartlist_add_asprintf(control_line, " ClientName=%s", cred->nickname); - } - if (cred->flags) { /* flags are also optional */ if (cred->flags & CLIENT_AUTH_FLAG_IS_PERMANENT) { smartlist_add_asprintf(control_line, " Flags=Permanent"); diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 3c681dd85e..c4bfdd2d9c 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1832,10 +1832,6 @@ client_service_authorization_free_(hs_client_service_authorization_t *auth) return; } - if (auth->nickname) { - tor_free(auth->nickname); - } - memwipe(auth, 0, sizeof(*auth)); tor_free(auth); } diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h index 75a9111077..e4869a9619 100644 --- a/src/feature/hs/hs_client.h +++ b/src/feature/hs/hs_client.h @@ -60,9 +60,6 @@ typedef enum { /** Flag to set when a client auth is permanent (saved on disk). */ #define CLIENT_AUTH_FLAG_IS_PERMANENT (1<<0) -/** Max length of a client auth nickname */ -#define HS_CLIENT_AUTH_MAX_NICKNAME_LENGTH 255 - /** Client-side configuration of client authorization */ typedef struct hs_client_service_authorization_t { /** An curve25519 secret key used to compute decryption keys that @@ -72,9 +69,6 @@ typedef struct hs_client_service_authorization_t { /** An onion address that is used to connect to the onion service. */ char onion_address[HS_SERVICE_ADDR_LEN_BASE32+1]; - /* An optional nickname for this client */ - char *nickname; - /* Optional flags for this client. */ int flags; } hs_client_service_authorization_t; diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index 0de2bca043..572b7f3ab1 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -245,8 +245,7 @@ test_hs_control_good_onion_client_auth_add(void *arg) /* Register first service */ args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " - "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " - "ClientName=bob"); + "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= "); retval = handle_control_command(&conn, (uint32_t) strlen(args), args); tt_int_op(retval, OP_EQ, 0); @@ -277,13 +276,11 @@ test_hs_control_good_onion_client_auth_add(void *arg) hs_client_service_authorization_t *client_2fv = digest256map_get(client_auths, service_identity_pk_2fv.pubkey); tt_assert(client_2fv); - tt_str_op(client_2fv->nickname, OP_EQ, "bob"); tt_int_op(client_2fv->flags, OP_EQ, 0); hs_client_service_authorization_t *client_jt4 = digest256map_get(client_auths, service_identity_pk_jt4.pubkey); tt_assert(client_jt4); - tt_assert(!client_jt4->nickname); tt_int_op(client_jt4->flags, OP_EQ, 0); /* Now let's VIEW the auth credentials */ @@ -296,8 +293,7 @@ test_hs_control_good_onion_client_auth_add(void *arg) #define VIEW_CORRECT_REPLY_NO_ADDR "250-ONION_CLIENT_AUTH_VIEW\r\n" \ "250-CLIENT x25519:eIIdIGoSZwI2Q/lSzpf92akGki5I+PZIDz37MA5BhlA=\r\n"\ - "250-CLIENT x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " \ - "ClientName=bob\r\n" \ + "250-CLIENT x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ=\r\n" \ "250 OK\r\n" retval = handle_control_command(&conn, (uint32_t) strlen(args), args); @@ -516,7 +512,7 @@ test_hs_control_store_permanent_creds(void *arg) /* Try registering first service with no ClientOnionAuthDir set */ args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " - "ClientName=bob Flags=Permanent"); + "Flags=Permanent"); retval = handle_control_command(&conn, (uint32_t) strlen(args), args); tt_int_op(retval, OP_EQ, 0); @@ -546,7 +542,7 @@ test_hs_control_store_permanent_creds(void *arg) /* Try the control port command again. This time it should work! */ args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " "x25519:iJ1tjKCrMAbiFT2bVrCjhbfMDnE1fpaRbIS5ZHKUvEQ= " - "ClientName=bob Flags=Permanent"); + "Flags=Permanent"); retval = handle_control_command(&conn, (uint32_t) strlen(args), args); tt_int_op(retval, OP_EQ, 0); @@ -571,7 +567,7 @@ test_hs_control_store_permanent_creds(void *arg) /* Overwrite the credentials and check that they got overwrited. */ args = tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd " "x25519:UDRvZLvcJo0QRLvDfkpgbtsqbkhIUQZyeo2FNBrgS18= " - "ClientName=fab Flags=Permanent"); + "Flags=Permanent"); retval = handle_control_command(&conn, (uint32_t) strlen(args), args); tt_int_op(retval, OP_EQ, 0); @@ -600,7 +596,6 @@ test_hs_control_store_permanent_creds(void *arg) hs_client_service_authorization_t *client_2fv = digest256map_get(client_auths, service_identity_pk_2fv.pubkey); tt_assert(client_2fv); - tt_assert(!client_2fv->nickname); tt_int_op(client_2fv->flags, OP_EQ, CLIENT_AUTH_FLAG_IS_PERMANENT); tt_str_op(hex_str((char*)client_2fv->enc_seckey.secret_key, 32), OP_EQ, "50346F64BBDC268D1044BBC37E4A606EDB2A6E48485106727A8D85341AE04B5F"); From 763f33729062ba015b10369dc767db3408733ad4 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 25 Nov 2019 17:40:22 +0200 Subject: [PATCH 4/6] hsv3: Start refactoring hs_config_client_authorization(). - Remove key_dir which is useless. - Kill an indentation layer. We want to make it cleaner and slimmer so that we can reuse parts of it in the REMOVE command for removing the right client auth file. --- src/feature/hs/hs_client.c | 49 +++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index c4bfdd2d9c..0247a01998 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1957,7 +1957,6 @@ hs_config_client_authorization(const or_options_t *options, { int ret = -1; digest256map_t *auths = digest256map_new(); - char *key_dir = NULL; smartlist_t *file_list = NULL; char *client_key_str = NULL; char *client_key_file_path = NULL; @@ -1971,17 +1970,15 @@ hs_config_client_authorization(const or_options_t *options, goto end; } - key_dir = tor_strdup(options->ClientOnionAuthDir); - /* Make sure the directory exists and is private enough. */ - if (check_private_dir(key_dir, 0, options->User) < 0) { + if (check_private_dir(options->ClientOnionAuthDir, 0, options->User) < 0) { goto end; } - file_list = tor_listdir(key_dir); + file_list = tor_listdir(options->ClientOnionAuthDir); if (file_list == NULL) { log_warn(LD_REND, "Client authorization key directory %s can't be listed.", - key_dir); + options->ClientOnionAuthDir); goto end; } @@ -2000,7 +1997,8 @@ hs_config_client_authorization(const or_options_t *options, } /* Create a full path for a file. */ - client_key_file_path = hs_path_from_filename(key_dir, filename); + client_key_file_path = hs_path_from_filename(options->ClientOnionAuthDir, + filename); client_key_str = read_file_to_str(client_key_file_path, 0, NULL); /* Free the file path immediately after using it. */ tor_free(client_key_file_path); @@ -2015,36 +2013,37 @@ hs_config_client_authorization(const or_options_t *options, /* Free immediately after using it. */ tor_free(client_key_str); - if (auth) { - /* Parse the onion address to get an identity public key and use it - * as a key of global map in the future. */ - if (hs_parse_address(auth->onion_address, &identity_pk, - NULL, NULL) < 0) { - log_warn(LD_REND, "The onion address \"%s\" is invalid in " - "file %s", filename, auth->onion_address); - client_service_authorization_free(auth); - continue; - } + if (!auth) { + continue; + } - if (digest256map_get(auths, identity_pk.pubkey)) { + /* Parse the onion address to get an identity public key and use it + * as a key of global map in the future. */ + if (hs_parse_address(auth->onion_address, &identity_pk, + NULL, NULL) < 0) { + log_warn(LD_REND, "The onion address \"%s\" is invalid in " + "file %s", filename, auth->onion_address); + client_service_authorization_free(auth); + continue; + } + + if (digest256map_get(auths, identity_pk.pubkey)) { log_warn(LD_REND, "Duplicate authorization for the same hidden " - "service address %s.", + "service address %s.", safe_str_client_opts(options, auth->onion_address)); client_service_authorization_free(auth); goto end; - } - - digest256map_set(auths, identity_pk.pubkey, auth); - log_info(LD_REND, "Loaded a client authorization key file %s.", - filename); } + + digest256map_set(auths, identity_pk.pubkey, auth); + log_info(LD_REND, "Loaded a client authorization key file %s.", + filename); } SMARTLIST_FOREACH_END(filename); /* Success. */ ret = 0; end: - tor_free(key_dir); tor_free(client_key_str); tor_free(client_key_file_path); if (file_list) { From 8ed8707f0ab78e9c954dad870f0529369c01c518 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 25 Nov 2019 18:03:53 +0200 Subject: [PATCH 5/6] hsv3: Abstract parts of hs_config_client_authorization() into func. Now we have a function that reads a file and returns a credential. We need that for the REMOVE control port command. --- src/feature/hs/hs_client.c | 78 ++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 0247a01998..cb902290f9 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1569,6 +1569,50 @@ hs_client_register_auth_credentials(hs_client_service_authorization_t *creds) return retval; } +/** Load a client authorization file with filename that is stored under + * the global client auth directory, and return a newly-allocated credentials + * object if it parsed well. Otherwise, return NULL. + */ +static hs_client_service_authorization_t * +get_creds_from_client_auth_filename(const char *filename, + const or_options_t *options) +{ + hs_client_service_authorization_t *auth = NULL; + char *client_key_file_path = NULL; + char *client_key_str = NULL; + + log_info(LD_REND, "Loading a client authorization key file %s...", + filename); + + if (!auth_key_filename_is_valid(filename)) { + log_notice(LD_REND, "Client authorization unrecognized filename %s. " + "File must end in .auth_private. Ignoring.", + filename); + goto err; + } + + /* Create a full path for a file. */ + client_key_file_path = hs_path_from_filename(options->ClientOnionAuthDir, + filename); + + client_key_str = read_file_to_str(client_key_file_path, 0, NULL); + if (!client_key_str) { + log_warn(LD_REND, "The file %s cannot be read.", filename); + goto err; + } + + auth = parse_auth_file_content(client_key_str); + if (!auth) { + goto err; + } + + err: + tor_free(client_key_str); + tor_free(client_key_file_path); + + return auth; +} + /** Remove client auth credentials for the service hs_address. */ hs_client_removal_auth_status_t hs_client_remove_auth_credentials(const char *hsaddress) @@ -1958,8 +2002,6 @@ hs_config_client_authorization(const or_options_t *options, int ret = -1; digest256map_t *auths = digest256map_new(); smartlist_t *file_list = NULL; - char *client_key_str = NULL; - char *client_key_file_path = NULL; tor_assert(options); @@ -1982,37 +2024,11 @@ hs_config_client_authorization(const or_options_t *options, goto end; } - SMARTLIST_FOREACH_BEGIN(file_list, char *, filename) { - + SMARTLIST_FOREACH_BEGIN(file_list, const char *, filename) { hs_client_service_authorization_t *auth = NULL; ed25519_public_key_t identity_pk; - log_info(LD_REND, "Loading a client authorization key file %s...", - filename); - - if (!auth_key_filename_is_valid(filename)) { - log_notice(LD_REND, "Client authorization unrecognized filename %s. " - "File must end in .auth_private. Ignoring.", - filename); - continue; - } - - /* Create a full path for a file. */ - client_key_file_path = hs_path_from_filename(options->ClientOnionAuthDir, - filename); - client_key_str = read_file_to_str(client_key_file_path, 0, NULL); - /* Free the file path immediately after using it. */ - tor_free(client_key_file_path); - - /* If we cannot read the file, continue with the next file. */ - if (!client_key_str) { - log_warn(LD_REND, "The file %s cannot be read.", filename); - continue; - } - - auth = parse_auth_file_content(client_key_str); - /* Free immediately after using it. */ - tor_free(client_key_str); + auth = get_creds_from_client_auth_filename(filename, options); if (!auth) { continue; } @@ -2044,8 +2060,6 @@ hs_config_client_authorization(const or_options_t *options, ret = 0; end: - tor_free(client_key_str); - tor_free(client_key_file_path); if (file_list) { SMARTLIST_FOREACH(file_list, char *, s, tor_free(s)); smartlist_free(file_list); From 12305b6bb6a68640e011dc5e2f2557bdccc2ae22 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 25 Nov 2019 18:31:29 +0200 Subject: [PATCH 6/6] hsv3: ONION_CLIENT_AUTH_REMOVE now also removes the credential file. --- src/feature/hs/hs_client.c | 84 ++++++++++++++++++++++++++++++++++++++ src/test/test_hs_control.c | 23 ++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index cb902290f9..823a0a45a7 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1613,6 +1613,84 @@ get_creds_from_client_auth_filename(const char *filename, return auth; } +/* + * Remove the file in filename under the global client auth credential + * storage. + */ +static void +remove_client_auth_creds_file(const char *filename) +{ + char *creds_file_path = NULL; + const or_options_t *options = get_options(); + + creds_file_path = hs_path_from_filename(options->ClientOnionAuthDir, + filename); + if (tor_unlink(creds_file_path) != 0) { + log_warn(LD_REND, "Failed to remove client auth file (%s).", + creds_file_path); + goto end; + } + + log_warn(LD_REND, "Successfuly removed client auth file (%s).", + creds_file_path); + + end: + tor_free(creds_file_path); +} + +/** + * Find the filesystem file corresponding to the permanent client auth + * credentials in cred and remove it. + */ +static void +find_and_remove_client_auth_creds_file( + const hs_client_service_authorization_t *cred) +{ + smartlist_t *file_list = NULL; + const or_options_t *options = get_options(); + + tor_assert(cred->flags & CLIENT_AUTH_FLAG_IS_PERMANENT); + + if (!options->ClientOnionAuthDir) { + log_warn(LD_REND, "Found permanent credential but no ClientOnionAuthDir " + "configured. There is no file to be removed."); + goto end; + } + + file_list = tor_listdir(options->ClientOnionAuthDir); + if (file_list == NULL) { + log_warn(LD_REND, "Client authorization key directory %s can't be listed.", + options->ClientOnionAuthDir); + goto end; + } + + SMARTLIST_FOREACH_BEGIN(file_list, const char *, filename) { + hs_client_service_authorization_t *tmp_cred = NULL; + + tmp_cred = get_creds_from_client_auth_filename(filename, options); + if (!tmp_cred) { + continue; + } + + /* Find the right file for this credential */ + if (!strcmp(tmp_cred->onion_address, cred->onion_address)) { + /* Found it! Remove the file! */ + remove_client_auth_creds_file(filename); + /* cleanup and get out of here */ + client_service_authorization_free(tmp_cred); + break; + } + + client_service_authorization_free(tmp_cred); + } SMARTLIST_FOREACH_END(filename); + + end: + if (file_list) { + SMARTLIST_FOREACH(file_list, char *, s, tor_free(s)); + smartlist_free(file_list); + } +} + /** Remove client auth credentials for the service hs_address. */ hs_client_removal_auth_status_t hs_client_remove_auth_credentials(const char *hsaddress) @@ -1629,8 +1707,14 @@ hs_client_remove_auth_credentials(const char *hsaddress) hs_client_service_authorization_t *cred = NULL; cred = digest256map_remove(client_auths, service_identity_pk.pubkey); + /* digestmap_remove() returns the previously stored data if there were any */ if (cred) { + if (cred->flags & CLIENT_AUTH_FLAG_IS_PERMANENT) { + /* These creds are stored on disk: remove the corresponding file. */ + find_and_remove_client_auth_creds_file(cred); + } + client_service_authorization_free(cred); return REMOVAL_SUCCESS; } diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index 572b7f3ab1..d064f203a6 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -585,7 +585,7 @@ test_hs_control_store_permanent_creds(void *arg) /* This is the base32 represenation of the base64 UDRv... key above */ "x25519:ka2g6zf33qti2ecexpbx4stan3nsu3sijbiqm4t2rwctigxajnpq"); - /* Now for our final act!!! Actually get the HS client subsystem to parse the + /* Now for our next act!!! Actually get the HS client subsystem to parse the * whole directory and make sure that it extracted the right credential! */ hs_config_client_authorization(get_options(), 0); @@ -600,6 +600,27 @@ test_hs_control_store_permanent_creds(void *arg) tt_str_op(hex_str((char*)client_2fv->enc_seckey.secret_key, 32), OP_EQ, "50346F64BBDC268D1044BBC37E4A606EDB2A6E48485106727A8D85341AE04B5F"); + /* And now for the final act! Use the REMOVE control port command to remove + the credential, and ensure that the file has also been removed! */ + tor_free(conn.current_cmd); + tor_free(cp1); + tor_free(args); + + /* Ensure that the creds file exists */ + tt_int_op(file_status(creds_fname), OP_EQ, FN_FILE); + + /* Do the REMOVE */ + conn.current_cmd = tor_strdup("ONION_CLIENT_AUTH_REMOVE"); + args =tor_strdup("2fvhjskjet3n5syd6yfg5lhvwcs62bojmthr35ko5bllr3iqdb4ctdyd"); + retval = handle_control_command(&conn, (uint32_t) strlen(args), args); + tt_int_op(retval, OP_EQ, 0); + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, "250 OK\r\n"); + + /* Ensure that the file has been removed and the map is empty */ + tt_int_op(file_status(creds_fname), OP_EQ, FN_NOENT); + tt_uint_op(digest256map_size(client_auths), OP_EQ, 0); + done: tor_free(args); tor_free(cp1);