commit a4f46ff8ba43b1e635bc5a8543b9354e6de02e14 Author: teor <teor2345@gmail.com> Date: Wed Sep 7 13:29:07 2016 +1000 Refactor the hidden service code to use rend_service_path And make consequential changes to make it less error-prone. No behaviour change. --- src/or/rendservice.c | 113 +++++++++++++++++++++++++++------------------------ src/or/rendservice.h | 1 + 2 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index c91cd50..04894dd 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -108,6 +108,14 @@ struct rend_service_port_config_s { * rendezvous point before giving up? */ #define MAX_REND_TIMEOUT 30 +/* Hidden service directory file names: + * new file names should be added to rend_service_add_filenames_to_list() + * for sandboxing purposes. */ +static const char *private_key_fname = "private_key"; +static const char *hostname_fname = "hostname"; +static const char *client_keys_fname = "client_keys"; +static const char *sos_poison_fname = "onion_service_non_anonymous"; + /** Returns a escaped string representation of the service, <b>s</b>. */ static const char * @@ -950,23 +958,32 @@ rend_service_update_descriptor(rend_service_t *service) } } -static const char *sos_poison_fname = "non_anonymous_hidden_service"; - -/* Allocate and return a string containing the path to the single onion - * poison file in service. - * The caller must free this path. - * Returns NULL if there is no directory for service. */ +/* Allocate and return a string containing the path to file_name in + * service->directory. Asserts that service has a directory. + * This function will never return NULL. + * The caller must free this path. */ static char * -sos_poison_path(const rend_service_t *service) +rend_service_path(const rend_service_t *service, const char *file_name) { - char *poison_path; + char *file_path = NULL; tor_assert(service->directory); - tor_asprintf(&poison_path, "%s%s%s", - service->directory, PATH_SEPARATOR, sos_poison_fname); + /* Can never fail: asserts rather than leaving file_path NULL. */ + tor_asprintf(&file_path, "%s%s%s", + service->directory, PATH_SEPARATOR, file_name); + + return file_path; +} - return poison_path; +/* Allocate and return a string containing the path to the single onion + * service poison file in service->directory. Asserts that service has a + * directory. + * The caller must free this path. */ +STATIC char * +rend_service_sos_poison_path(const rend_service_t *service) +{ + return rend_service_path(service, sos_poison_fname); } /** Return True if hidden services <b>service> has been poisoned by single @@ -981,8 +998,7 @@ service_is_single_onion_poisoned(const rend_service_t *service) return 0; } - poison_fname = sos_poison_path(service); - tor_assert(poison_fname); + poison_fname = rend_service_sos_poison_path(service); fstatus = file_status(poison_fname); tor_free(poison_fname); @@ -1076,7 +1092,7 @@ poison_new_single_onion_hidden_service_dir(const rend_service_t *service) return -1; } - poison_fname = sos_poison_path(service); + poison_fname = rend_service_sos_poison_path(service); switch (file_status(poison_fname)) { case FN_DIR: @@ -1190,12 +1206,10 @@ rend_service_add_filenames_to_list(smartlist_t *lst, const rend_service_t *s) tor_assert(lst); tor_assert(s); tor_assert(s->directory); - smartlist_add_asprintf(lst, "%s"PATH_SEPARATOR"private_key", - s->directory); - smartlist_add_asprintf(lst, "%s"PATH_SEPARATOR"hostname", - s->directory); - smartlist_add_asprintf(lst, "%s"PATH_SEPARATOR"client_keys", - s->directory); + smartlist_add(lst, rend_service_path(s, private_key_fname)); + smartlist_add(lst, rend_service_path(s, hostname_fname)); + smartlist_add(lst, rend_service_path(s, client_keys_fname)); + smartlist_add(lst, rend_service_sos_poison_path(s)); } /** Add to <b>open_lst</b> every filename used by a configured hidden service, @@ -1239,7 +1253,7 @@ rend_service_derive_key_digests(struct rend_service_t *s) static int rend_service_load_keys(rend_service_t *s) { - char fname[512]; + char *fname = NULL; char buf[128]; cpd_check_t check_opts = CPD_CREATE; @@ -1248,7 +1262,7 @@ rend_service_load_keys(rend_service_t *s) } /* Check/create directory */ if (check_private_dir(s->directory, check_opts, get_options()->User) < 0) { - return -1; + goto err; } #ifndef _WIN32 if (s->dir_group_readable) { @@ -1260,34 +1274,23 @@ rend_service_load_keys(rend_service_t *s) #endif /* Load key */ - if (strlcpy(fname,s->directory,sizeof(fname)) >= sizeof(fname) || - strlcat(fname,PATH_SEPARATOR"private_key",sizeof(fname)) - >= sizeof(fname)) { - log_warn(LD_CONFIG, "Directory name too long to store key file: \"%s\".", - s->directory); - return -1; - } + fname = rend_service_path(s, private_key_fname); s->private_key = init_key_from_file(fname, 1, LOG_ERR, 0); + if (!s->private_key) - return -1; + goto err; if (rend_service_derive_key_digests(s) < 0) - return -1; + goto err; + tor_free(fname); /* Create service file */ - if (strlcpy(fname,s->directory,sizeof(fname)) >= sizeof(fname) || - strlcat(fname,PATH_SEPARATOR"hostname",sizeof(fname)) - >= sizeof(fname)) { - log_warn(LD_CONFIG, "Directory name too long to store hostname file:" - " \"%s\".", s->directory); - return -1; - } + fname = rend_service_path(s, hostname_fname); tor_snprintf(buf, sizeof(buf),"%s.onion\n", s->service_id); if (write_str_to_file(fname,buf,0)<0) { log_warn(LD_CONFIG, "Could not write onion address to hostname file."); - memwipe(buf, 0, sizeof(buf)); - return -1; + goto err; } #ifndef _WIN32 if (s->dir_group_readable) { @@ -1298,15 +1301,21 @@ rend_service_load_keys(rend_service_t *s) } #endif - memwipe(buf, 0, sizeof(buf)); - /* If client authorization is configured, load or generate keys. */ if (s->auth_type != REND_NO_AUTH) { - if (rend_service_load_auth_keys(s, fname) < 0) - return -1; + if (rend_service_load_auth_keys(s, fname) < 0) { + goto err; + } } - return 0; + int r = 0; + goto done; + err: + r = -1; + done: + memwipe(buf, 0, sizeof(buf)); + tor_free(fname); + return r; } /** Load and/or generate client authorization keys for the hidden service @@ -1316,7 +1325,7 @@ static int rend_service_load_auth_keys(rend_service_t *s, const char *hfname) { int r = 0; - char cfname[512]; + char *cfname = NULL; char *client_keys_str = NULL; strmap_t *parsed_clients = strmap_new(); FILE *cfile, *hfile; @@ -1326,12 +1335,7 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) char buf[1500]; /* Load client keys and descriptor cookies, if available. */ - if (tor_snprintf(cfname, sizeof(cfname), "%s"PATH_SEPARATOR"client_keys", - s->directory)<0) { - log_warn(LD_CONFIG, "Directory name too long to store client keys " - "file: \"%s\".", s->directory); - goto err; - } + cfname = rend_service_path(s, client_keys_fname); client_keys_str = read_file_to_str(cfname, RFTS_IGNORE_MISSING, NULL); if (client_keys_str) { if (rend_parse_client_keys(parsed_clients, client_keys_str) < 0) { @@ -1485,7 +1489,10 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) } strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); - memwipe(cfname, 0, sizeof(cfname)); + if (cfname) { + memwipe(cfname, 0, sizeof(cfname)); + tor_free(cfname); + } /* Clear stack buffers that held key-derived material. */ memwipe(buf, 0, sizeof(buf)); diff --git a/src/or/rendservice.h b/src/or/rendservice.h index ec32262..630191e 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -118,6 +118,7 @@ typedef struct rend_service_t { } rend_service_t; STATIC void rend_service_free(rend_service_t *service); +STATIC char *rend_service_sos_poison_path(const rend_service_t *service); #endif