diff options
| -rw-r--r-- | changes/20460 | 4 | ||||
| -rw-r--r-- | changes/bug20484 | 5 | ||||
| -rw-r--r-- | changes/bug20529 | 4 | ||||
| -rw-r--r-- | changes/bug20587 | 1 | ||||
| -rw-r--r-- | changes/bug20588 | 2 | ||||
| -rw-r--r-- | changes/geoip-november2016 | 2 | ||||
| -rw-r--r-- | doc/tor.1.txt | 4 | ||||
| -rw-r--r-- | src/common/util.c | 12 | ||||
| -rw-r--r-- | src/or/rendservice.c | 76 | ||||
| -rw-r--r-- | src/test/test.h | 1 | ||||
| -rw-r--r-- | src/test/test_hs.c | 55 | ||||
| -rw-r--r-- | src/test/testing_common.c | 34 |
12 files changed, 139 insertions, 61 deletions
diff --git a/changes/20460 b/changes/20460 index d51ec3ad96..9fbb4a7986 100644 --- a/changes/20460 +++ b/changes/20460 @@ -1,4 +1,4 @@ - o Minor bugfixes (testing) + o Minor bugfixes (testing): - Use ECDHE ciphers instead of ECDH in tortls tests. LibreSSL has removed the ECDH ciphers which caused the tests to fail on - platforms which use it. Closes ticket 20460. + platforms which use it. Fixes bug 20460; bugfix on 0.2.8.1-alpha. diff --git a/changes/bug20484 b/changes/bug20484 new file mode 100644 index 0000000000..9a0b95cb39 --- /dev/null +++ b/changes/bug20484 @@ -0,0 +1,5 @@ + o Minor bugfixes (single onion services): + - Start correctly when creating a single onion service in a + directory that did not previously exist. Fixes bug 20484; bugfix on + 0.2.9.3-alpha. + diff --git a/changes/bug20529 b/changes/bug20529 new file mode 100644 index 0000000000..276be5b2b6 --- /dev/null +++ b/changes/bug20529 @@ -0,0 +1,4 @@ + o Minor bugfixes (hidden services): + - When configuring hidden services, check every hidden service directory's + permissions. Previously, we only checked the last hidden service. + Fixes bug 20529; bugfix on 13942 commit 85bfad1 in 0.2.6.2-alpha. diff --git a/changes/bug20587 b/changes/bug20587 index a05933427c..341b001363 100644 --- a/changes/bug20587 +++ b/changes/bug20587 @@ -1,4 +1,3 @@ - o Minor bugfixes (download timing): - When determining when to download a directory object, handle times after 2038 if the operating system supports that. (Someday this will be diff --git a/changes/bug20588 b/changes/bug20588 index be199b2de0..832ef81336 100644 --- a/changes/bug20588 +++ b/changes/bug20588 @@ -1,3 +1,3 @@ - o Minor bugfixes (portability): + o Minor features (portability): - Fix compilation with OpenSSL 1.1 and less commonly-used CPU architectures. Closes ticket 20588. diff --git a/changes/geoip-november2016 b/changes/geoip-november2016 index 5190ed66f4..b3f9913bb1 100644 --- a/changes/geoip-november2016 +++ b/changes/geoip-november2016 @@ -1,4 +1,4 @@ - o Minor features: + o Minor features (ge0oip): - Update geoip and geoip6 to the November 3 2016 Maxmind GeoLite2 Country database. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index bfae1c6323..97eeb9674d 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2898,6 +2898,10 @@ __HiddenServiceDirectory__**/client_keys**:: Authorization data for a hidden service that is only accessible by authorized clients. +__HiddenServiceDirectory__**/onion_service_non_anonymous**:: + This file is present if a hidden service key was created in + **HiddenServiceNonAnonymousMode**. + SEE ALSO -------- **torsocks**(1), **torify**(1) + diff --git a/src/common/util.c b/src/common/util.c index b1159e5334..cb5f12821e 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2191,11 +2191,13 @@ file_status(const char *fname) } } -/** Check whether <b>dirname</b> exists and is private. If yes return 0. If - * it does not exist, and <b>check</b>&CPD_CREATE is set, try to create it - * and return 0 on success. If it does not exist, and - * <b>check</b>&CPD_CHECK, and we think we can create it, return 0. Else - * return -1. If CPD_GROUP_OK is set, then it's okay if the directory +/** Check whether <b>dirname</b> exists and is private. If yes return 0. + * If <b>dirname</b> does not exist: + * - if <b>check</b>&CPD_CREATE, try to create it and return 0 on success. + * - if <b>check</b>&CPD_CHECK, and we think we can create it, return 0. + * - if <b>check</b>&CPD_CHECK is false, and the directory exists, return 0. + * - otherwise, return -1. + * If CPD_GROUP_OK is set, then it's okay if the directory * is group-readable, but in all cases we create the directory mode 0700. * If CPD_GROUP_READ is set, existing directory behaves as CPD_GROUP_OK and * if the directory is created it will use mode 0750 with group read diff --git a/src/or/rendservice.c b/src/or/rendservice.c index b6bf63d829..0819d8a713 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -73,6 +73,10 @@ static ssize_t rend_service_parse_intro_for_v3( size_t plaintext_len, char **err_msg_out); +static int rend_service_check_private_dir(const or_options_t *options, + const rend_service_t *s, + int create); + /** Represents the mapping from a virtual port of a rendezvous service to * a real port on some IP. */ @@ -472,6 +476,11 @@ rend_config_services(const or_options_t *options, int validate_only) for (line = options->RendConfigLines; line; line = line->next) { if (!strcasecmp(line->key, "HiddenServiceDir")) { if (service) { /* register the one we just finished parsing */ + if (rend_service_check_private_dir(options, service, 0) < 0) { + rend_service_free(service); + return -1; + } + if (validate_only) rend_service_free(service); else @@ -687,12 +696,7 @@ rend_config_services(const or_options_t *options, int validate_only) } } if (service) { - cpd_check_t check_opts = CPD_CHECK_MODE_ONLY|CPD_CHECK; - if (service->dir_group_readable) { - check_opts |= CPD_GROUP_READ; - } - - if (check_private_dir(service->directory, check_opts, options->User) < 0) { + if (rend_service_check_private_dir(options, service, 0) < 0) { rend_service_free(service); return -1; } @@ -1016,7 +1020,9 @@ service_is_single_onion_poisoned(const rend_service_t *service) fstatus = file_status(poison_fname); tor_free(poison_fname); - /* If this fname is occupied, the hidden service has been poisoned. */ + /* If this fname is occupied, the hidden service has been poisoned. + * fstatus can be FN_ERROR if the service directory does not exist, in that + * case, there is obviously no private key. */ if (fstatus == FN_FILE || fstatus == FN_EMPTY) { return 1; } @@ -1032,7 +1038,9 @@ rend_service_private_key_exists(const rend_service_t *service) char *private_key_path = rend_service_path(service, private_key_fname); const file_status_t private_key_status = file_status(private_key_path); tor_free(private_key_path); - /* Only non-empty regular private key files could have been used before. */ + /* Only non-empty regular private key files could have been used before. + * fstatus can be FN_ERROR if the service directory does not exist, in that + * case, there is obviously no private key. */ return private_key_status == FN_FILE; } @@ -1105,6 +1113,10 @@ poison_new_single_onion_hidden_service_dir(const rend_service_t *service) return -1; } + /* Make sure the directory exists */ + if (rend_service_check_private_dir(get_options(), service, 1) < 0) + return -1; + poison_fname = rend_service_sos_poison_path(service); switch (file_status(poison_fname)) { @@ -1260,6 +1272,37 @@ rend_service_derive_key_digests(struct rend_service_t *s) return 0; } +/** Make sure that the directory for <b>s</b> is private, using the config in + * <b>options</b>. + * If <b>create</b> is true: + * - if the directory exists, change permissions if needed, + * - if the directory does not exist, create it with the correct permissions. + * If <b>create</b> is false: + * - if the directory exists, check permissions, + * - if the directory does not exist, check if we think we can create it. + * Return 0 on success, -1 on failure. */ +static int +rend_service_check_private_dir(const or_options_t *options, + const rend_service_t *s, + int create) +{ + cpd_check_t check_opts = CPD_NONE; + if (create) { + check_opts |= CPD_CREATE; + } else { + check_opts |= CPD_CHECK_MODE_ONLY; + check_opts |= CPD_CHECK; + } + if (s->dir_group_readable) { + check_opts |= CPD_GROUP_READ; + } + /* Check/create directory */ + if (check_private_dir(s->directory, check_opts, options->User) < 0) { + return -1; + } + return 0; +} + /** Load and/or generate private keys for the hidden service <b>s</b>, * possibly including keys for client authorization. Return 0 on success, -1 * on failure. */ @@ -1268,24 +1311,9 @@ rend_service_load_keys(rend_service_t *s) { char *fname = NULL; char buf[128]; - cpd_check_t check_opts = CPD_CREATE; - if (s->dir_group_readable) { - check_opts |= CPD_GROUP_READ; - } - /* Check/create directory */ - if (check_private_dir(s->directory, check_opts, get_options()->User) < 0) { + if (rend_service_check_private_dir(get_options(), s, 1) < 0) goto err; - } -#ifndef _WIN32 - if (s->dir_group_readable) { - /* Only new dirs created get new opts, also enforce group read. */ - if (chmod(s->directory, 0750)) { - log_warn(LD_FS,"Unable to make %s group-readable.", - rend_service_escaped_dir(s)); - } - } -#endif /* Load key */ fname = rend_service_path(s, private_key_fname); diff --git a/src/test/test.h b/src/test/test.h index 15ccf5c379..49b4499594 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -73,6 +73,7 @@ {print_ = (I64_PRINTF_TYPE) value_;}, {}, TT_EXIT_TEST_FUNCTION) const char *get_fname(const char *name); +const char *get_fname_rnd(const char *name); struct crypto_pk_t *pk_generate(int idx); void init_pregenerated_keys(void); void free_pregenerated_keys(void); diff --git a/src/test/test_hs.c b/src/test/test_hs.c index 67ce1cfaed..318c549762 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -527,6 +527,11 @@ mock_get_options(void) return mock_options; } +/* arg can't be 0 (the test fails) or 2 (the test is skipped) */ +#define CREATE_HS_DIR_NONE ((intptr_t)0x04) +#define CREATE_HS_DIR1 ((intptr_t)0x08) +#define CREATE_HS_DIR2 ((intptr_t)0x10) + /* Test that single onion poisoning works. */ static void test_single_onion_poisoning(void *arg) @@ -537,15 +542,15 @@ test_single_onion_poisoning(void *arg) MOCK(get_options, mock_get_options); int ret = -1; - mock_options->DataDirectory = tor_strdup(get_fname("test_data_dir")); + intptr_t create_dir_mask = (intptr_t)arg; + /* Get directories with a random suffix so we can repeat the tests */ + mock_options->DataDirectory = tor_strdup(get_fname_rnd("test_data_dir")); rend_service_t *service_1 = tor_malloc_zero(sizeof(rend_service_t)); - char *dir1 = tor_strdup(get_fname("test_hs_dir1")); + char *dir1 = tor_strdup(get_fname_rnd("test_hs_dir1")); rend_service_t *service_2 = tor_malloc_zero(sizeof(rend_service_t)); - char *dir2 = tor_strdup(get_fname("test_hs_dir2")); + char *dir2 = tor_strdup(get_fname_rnd("test_hs_dir2")); smartlist_t *services = smartlist_new(); - (void) arg; - /* No services, no problem! */ mock_options->HiddenServiceSingleHopMode = 0; mock_options->HiddenServiceNonAnonymousMode = 0; @@ -558,22 +563,20 @@ test_single_onion_poisoning(void *arg) ret = rend_service_list_verify_single_onion_poison(services, mock_options); tt_assert(ret == 0); - /* Create directories for both services */ - -#ifdef _WIN32 - ret = mkdir(mock_options->DataDirectory); - tt_assert(ret == 0); - ret = mkdir(dir1); - tt_assert(ret == 0); - ret = mkdir(dir2); -#else - ret = mkdir(mock_options->DataDirectory, 0700); - tt_assert(ret == 0); - ret = mkdir(dir1, 0700); - tt_assert(ret == 0); - ret = mkdir(dir2, 0700); -#endif + /* Create the data directory, and, if the correct bit in arg is set, + * create a directory for that service. + * The data directory is required for the lockfile, which is used when + * loading keys. */ + ret = check_private_dir(mock_options->DataDirectory, CPD_CREATE, NULL); tt_assert(ret == 0); + if (create_dir_mask & CREATE_HS_DIR1) { + ret = check_private_dir(dir1, CPD_CREATE, NULL); + tt_assert(ret == 0); + } + if (create_dir_mask & CREATE_HS_DIR2) { + ret = check_private_dir(dir2, CPD_CREATE, NULL); + tt_assert(ret == 0); + } service_1->directory = dir1; service_2->directory = dir2; @@ -703,7 +706,7 @@ test_single_onion_poisoning(void *arg) tt_assert(ret < 0); done: - /* TODO: should we delete the directories here? */ + /* The test harness deletes the directories at exit */ rend_service_free(service_1); rend_service_free(service_2); smartlist_free(services); @@ -725,8 +728,14 @@ struct testcase_t hs_tests[] = { NULL, NULL }, { "hs_auth_cookies", test_hs_auth_cookies, TT_FORK, NULL, NULL }, - { "single_onion_poisoning", test_single_onion_poisoning, TT_FORK, - NULL, NULL }, + { "single_onion_poisoning_create_dir_none", test_single_onion_poisoning, + TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR_NONE) }, + { "single_onion_poisoning_create_dir1", test_single_onion_poisoning, + TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR1) }, + { "single_onion_poisoning_create_dir2", test_single_onion_poisoning, + TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR2) }, + { "single_onion_poisoning_create_dir_both", test_single_onion_poisoning, + TT_FORK, &passthrough_setup, (void*)(CREATE_HS_DIR1 | CREATE_HS_DIR2) }, END_OF_TESTCASES }; diff --git a/src/test/testing_common.c b/src/test/testing_common.c index deb11d0112..caeae13a38 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -102,18 +102,41 @@ setup_directory(void) temp_dir_setup_in_pid = getpid(); } -/** Return a filename relative to our testing temporary directory */ -const char * -get_fname(const char *name) +/** Return a filename relative to our testing temporary directory, based on + * name and suffix. If name is NULL, return the name of the testing temporary + * directory. */ +static const char * +get_fname_suffix(const char *name, const char *suffix) { static char buf[1024]; setup_directory(); if (!name) return temp_dir; - tor_snprintf(buf,sizeof(buf),"%s/%s",temp_dir,name); + tor_snprintf(buf,sizeof(buf),"%s/%s%s%s",temp_dir,name,suffix ? "_" : "", + suffix ? suffix : ""); return buf; } +/** Return a filename relative to our testing temporary directory. If name is + * NULL, return the name of the testing temporary directory. */ +const char * +get_fname(const char *name) +{ + return get_fname_suffix(name, NULL); +} + +/** Return a filename with a random suffix, relative to our testing temporary + * directory. If name is NULL, return the name of the testing temporary + * directory, without any suffix. */ +const char * +get_fname_rnd(const char *name) +{ + char rnd[256], rnd32[256]; + crypto_rand(rnd, RAND_PATH_BYTES); + base32_encode(rnd32, sizeof(rnd32), rnd, RAND_PATH_BYTES); + return get_fname_suffix(name, rnd32); +} + /* Remove a directory and all of its subdirectories */ static void rm_rf(const char *dir) @@ -158,6 +181,9 @@ remove_directory(void) static void * passthrough_test_setup(const struct testcase_t *testcase) { + /* Make sure the passthrough doesn't unintentionally fail or skip tests */ + tor_assert(testcase->setup_data); + tor_assert(testcase->setup_data != (void*)TT_SKIP); return testcase->setup_data; } static int |