tor-commits June 2012

tor-commits@lists.torproject.org

17 participants
880 discussions

[tor/master] Fix indentation in rend_service_load_auth_keys
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit e5a61c5176352d02852dbe996be10fc51649debc Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 18 13:01:15 2012 -0400 Fix indentation in rend_service_load_auth_keys --- src/or/rendservice.c | 312 +++++++++++++++++++++++++------------------------- 1 files changed, 156 insertions(+), 156 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index dedddd8..69a86af 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -703,183 +703,183 @@ rend_service_load_auth_keys(rend_service_t *s) char service_id[16+1]; 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); + /* 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; + } + 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) { + log_warn(LD_CONFIG, "Previously stored client_keys file could not " + "be parsed."); goto err; + } else { + log_info(LD_CONFIG, "Parsed %d previously stored client entries.", + strmap_size(parsed_clients)); + tor_free(client_keys_str); } - 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) { - log_warn(LD_CONFIG, "Previously stored client_keys file could not " - "be parsed."); - goto err; - } else { - log_info(LD_CONFIG, "Parsed %d previously stored client entries.", - strmap_size(parsed_clients)); - tor_free(client_keys_str); - } - } + } - /* Prepare client_keys and hostname files. */ - if (!(cfile = start_writing_to_stdio_file(cfname, - OPEN_FLAGS_REPLACE | O_TEXT, - 0600, &open_cfile))) { - log_warn(LD_CONFIG, "Could not open client_keys file %s", - escaped(cfname)); - goto err; + /* Prepare client_keys and hostname files. */ + if (!(cfile = start_writing_to_stdio_file(cfname, + OPEN_FLAGS_REPLACE | O_TEXT, + 0600, &open_cfile))) { + log_warn(LD_CONFIG, "Could not open client_keys file %s", + escaped(cfname)); + goto err; + } + if (!(hfile = start_writing_to_stdio_file(fname, + OPEN_FLAGS_REPLACE | O_TEXT, + 0600, &open_hfile))) { + log_warn(LD_CONFIG, "Could not open hostname file %s", escaped(fname)); + goto err; + } + + /* Either use loaded keys for configured clients or generate new + * ones if a client is new. */ + SMARTLIST_FOREACH_BEGIN(s->clients, rend_authorized_client_t *, client) { + rend_authorized_client_t *parsed = + strmap_get(parsed_clients, client->client_name); + int written; + size_t len; + /* Copy descriptor cookie from parsed entry or create new one. */ + if (parsed) { + memcpy(client->descriptor_cookie, parsed->descriptor_cookie, + REND_DESC_COOKIE_LEN); + } else { + crypto_rand(client->descriptor_cookie, REND_DESC_COOKIE_LEN); } - if (!(hfile = start_writing_to_stdio_file(fname, - OPEN_FLAGS_REPLACE | O_TEXT, - 0600, &open_hfile))) { - log_warn(LD_CONFIG, "Could not open hostname file %s", escaped(fname)); - goto err; + if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1, + client->descriptor_cookie, + REND_DESC_COOKIE_LEN) < 0) { + log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); + strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); + /* Clear these here for the early error exit */ + memset(desc_cook_out, 0, sizeof(desc_cook_out)); + memset(service_id, 0, sizeof(service_id)); + return -1; } - - /* Either use loaded keys for configured clients or generate new - * ones if a client is new. */ - SMARTLIST_FOREACH_BEGIN(s->clients, rend_authorized_client_t *, client) { - rend_authorized_client_t *parsed = - strmap_get(parsed_clients, client->client_name); - int written; - size_t len; - /* Copy descriptor cookie from parsed entry or create new one. */ - if (parsed) { - memcpy(client->descriptor_cookie, parsed->descriptor_cookie, - REND_DESC_COOKIE_LEN); - } else { - crypto_rand(client->descriptor_cookie, REND_DESC_COOKIE_LEN); + /* Copy client key from parsed entry or create new one if required. */ + if (parsed && parsed->client_key) { + client->client_key = crypto_pk_dup_key(parsed->client_key); + } else if (s->auth_type == REND_STEALTH_AUTH) { + /* Create private key for client. */ + crypto_pk_t *prkey = NULL; + if (!(prkey = crypto_pk_new())) { + log_warn(LD_BUG,"Error constructing client key"); + goto err; } - if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1, - client->descriptor_cookie, - REND_DESC_COOKIE_LEN) < 0) { - log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); - strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); - /* Clear these here for the early error exit */ - memset(desc_cook_out, 0, sizeof(desc_cook_out)); - memset(service_id, 0, sizeof(service_id)); - return -1; + if (crypto_pk_generate_key(prkey)) { + log_warn(LD_BUG,"Error generating client key"); + crypto_pk_free(prkey); + goto err; } - /* Copy client key from parsed entry or create new one if required. */ - if (parsed && parsed->client_key) { - client->client_key = crypto_pk_dup_key(parsed->client_key); - } else if (s->auth_type == REND_STEALTH_AUTH) { - /* Create private key for client. */ - crypto_pk_t *prkey = NULL; - if (!(prkey = crypto_pk_new())) { - log_warn(LD_BUG,"Error constructing client key"); - goto err; - } - if (crypto_pk_generate_key(prkey)) { - log_warn(LD_BUG,"Error generating client key"); - crypto_pk_free(prkey); - goto err; - } - if (crypto_pk_check_key(prkey) <= 0) { - log_warn(LD_BUG,"Generated client key seems invalid"); - crypto_pk_free(prkey); - goto err; - } - client->client_key = prkey; + if (crypto_pk_check_key(prkey) <= 0) { + log_warn(LD_BUG,"Generated client key seems invalid"); + crypto_pk_free(prkey); + goto err; } - /* Add entry to client_keys file. */ - desc_cook_out[strlen(desc_cook_out)-1] = '\0'; /* Remove newline. */ - written = tor_snprintf(buf, sizeof(buf), - "client-name %s\ndescriptor-cookie %s\n", - client->client_name, desc_cook_out); - if (written < 0) { - log_warn(LD_BUG, "Could not write client entry."); + client->client_key = prkey; + } + /* Add entry to client_keys file. */ + desc_cook_out[strlen(desc_cook_out)-1] = '\0'; /* Remove newline. */ + written = tor_snprintf(buf, sizeof(buf), + "client-name %s\ndescriptor-cookie %s\n", + client->client_name, desc_cook_out); + if (written < 0) { + log_warn(LD_BUG, "Could not write client entry."); + goto err; + } + if (client->client_key) { + char *client_key_out = NULL; + if (crypto_pk_write_private_key_to_string(client->client_key, + &client_key_out, &len) != 0) { + log_warn(LD_BUG, "Internal error: " + "crypto_pk_write_private_key_to_string() failed."); goto err; } - if (client->client_key) { - char *client_key_out = NULL; - if (crypto_pk_write_private_key_to_string(client->client_key, - &client_key_out, &len) != 0) { - log_warn(LD_BUG, "Internal error: " - "crypto_pk_write_private_key_to_string() failed."); - goto err; - } - if (rend_get_service_id(client->client_key, service_id)<0) { - log_warn(LD_BUG, "Internal error: couldn't encode service ID."); - /* - * len is string length, not buffer length, but last byte is NUL - * anyway. - */ - memset(client_key_out, 0, len); - tor_free(client_key_out); - goto err; - } - written = tor_snprintf(buf + written, sizeof(buf) - written, - "client-key\n%s", client_key_out); + if (rend_get_service_id(client->client_key, service_id)<0) { + log_warn(LD_BUG, "Internal error: couldn't encode service ID."); + /* + * len is string length, not buffer length, but last byte is NUL + * anyway. + */ memset(client_key_out, 0, len); tor_free(client_key_out); - if (written < 0) { - log_warn(LD_BUG, "Could not write client entry."); - goto err; - } + goto err; } - - if (fputs(buf, cfile) < 0) { - log_warn(LD_FS, "Could not append client entry to file: %s", - strerror(errno)); + written = tor_snprintf(buf + written, sizeof(buf) - written, + "client-key\n%s", client_key_out); + memset(client_key_out, 0, len); + tor_free(client_key_out); + if (written < 0) { + log_warn(LD_BUG, "Could not write client entry."); 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) { - log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); - goto err; - } - desc_cook_out[strlen(desc_cook_out)-3] = '\0'; /* Remove A= and - newline. */ - tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n", - service_id, desc_cook_out, client->client_name); - } + if (fputs(buf, cfile) < 0) { + log_warn(LD_FS, "Could not append client entry to file: %s", + strerror(errno)); + goto err; + } - if (fputs(buf, hfile)<0) { - log_warn(LD_FS, "Could not append host entry to file: %s", - strerror(errno)); + /* 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) { + log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); goto err; } + desc_cook_out[strlen(desc_cook_out)-3] = '\0'; /* Remove A= and + newline. */ + tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n", + service_id, desc_cook_out, client->client_name); } - SMARTLIST_FOREACH_END(client); - - goto done; - err: - r = -1; - done: - tor_free(client_keys_str); - strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); - if (r<0) { - /* Clear these here for the early error exit */ - /* We have to clear buf because encoded keys can get written to it */ - memset(buf, 0, sizeof(buf)); - memset(desc_cook_out, 0, sizeof(desc_cook_out)); - memset(service_id, 0, sizeof(service_id)); - memset(extended_desc_cookie, 0, sizeof(extended_desc_cookie)); - if (open_cfile) - abort_writing_to_file(open_cfile); - if (open_hfile) - abort_writing_to_file(open_hfile); - return r; - } else { - finish_writing_to_file(open_cfile); - finish_writing_to_file(open_hfile); + + if (fputs(buf, hfile)<0) { + log_warn(LD_FS, "Could not append host entry to file: %s", + strerror(errno)); + goto err; } + } + SMARTLIST_FOREACH_END(client); + + goto done; + err: + r = -1; + done: + tor_free(client_keys_str); + strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); + if (r<0) { + /* Clear these here for the early error exit */ + /* We have to clear buf because encoded keys can get written to it */ + memset(buf, 0, sizeof(buf)); + memset(desc_cook_out, 0, sizeof(desc_cook_out)); + memset(service_id, 0, sizeof(service_id)); + memset(extended_desc_cookie, 0, sizeof(extended_desc_cookie)); + if (open_cfile) + abort_writing_to_file(open_cfile); + if (open_hfile) + abort_writing_to_file(open_hfile); + return r; + } else { + finish_writing_to_file(open_cfile); + finish_writing_to_file(open_hfile); + } /* * Clear stack buffers that held key-derived material; we do this here

1 0

[tor/master] Refactor exit path in rend_service_load_auth_keys
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit b8d1e8e3755e6d12bd6a12cc8e91353b55a64186 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 18 13:05:28 2012 -0400 Refactor exit path in rend_service_load_auth_keys Now it's an orthodox "goto err/done" exit path, and it isn't some screwy thing where we stick err/done at the end of a loop and duplicate our cleanup code. --- src/or/rendservice.c | 61 +++++++++++++++++++------------------------------ 1 files changed, 24 insertions(+), 37 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 69a86af..22f3b27 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -33,7 +33,8 @@ static int intro_point_should_expire_now(rend_intro_point_t *intro, time_t now); struct rend_service_t; static int rend_service_load_keys(struct rend_service_t *s); -static int rend_service_load_auth_keys(struct rend_service_t *s); +static int rend_service_load_auth_keys(struct rend_service_t *s, + const char *hfname); /** Represents the mapping from a virtual port of a rendezvous service to * a real port on some IP. @@ -670,6 +671,7 @@ rend_service_load_keys(rend_service_t *s) " \"%s\".", s->directory); return -1; } + 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."); @@ -680,7 +682,7 @@ rend_service_load_keys(rend_service_t *s) /* If client authorization is configured, load or generate keys. */ if (s->auth_type != REND_NO_AUTH) { - if (rend_service_load_auth_keys(s) < 0) + if (rend_service_load_auth_keys(s, fname) < 0) return -1; } @@ -688,12 +690,13 @@ rend_service_load_keys(rend_service_t *s) } /** Load and/or generate client authorization keys for the hidden service - * s. Return 0 on success, -1 on failure. */ + * s, which stores its hostname in hfname. Return 0 on success, + * -1 on failure. */ static int -rend_service_load_auth_keys(rend_service_t *s) +rend_service_load_auth_keys(rend_service_t *s, const char *hfname) { int r = 0; - char fname[512], cfname[512]; + char cfname[512]; char *client_keys_str = NULL; strmap_t *parsed_clients = strmap_new(); FILE *cfile, *hfile; @@ -731,10 +734,11 @@ rend_service_load_auth_keys(rend_service_t *s) escaped(cfname)); goto err; } - if (!(hfile = start_writing_to_stdio_file(fname, + + if (!(hfile = start_writing_to_stdio_file(hfname, OPEN_FLAGS_REPLACE | O_TEXT, 0600, &open_hfile))) { - log_warn(LD_CONFIG, "Could not open hostname file %s", escaped(fname)); + log_warn(LD_CONFIG, "Could not open hostname file %s", escaped(hfname)); goto err; } @@ -756,11 +760,7 @@ rend_service_load_auth_keys(rend_service_t *s) client->descriptor_cookie, REND_DESC_COOKIE_LEN) < 0) { log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); - strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); - /* Clear these here for the early error exit */ - memset(desc_cook_out, 0, sizeof(desc_cook_out)); - memset(service_id, 0, sizeof(service_id)); - return -1; + goto err; } /* Copy client key from parsed entry or create new one if required. */ if (parsed && parsed->client_key) { @@ -855,39 +855,26 @@ rend_service_load_auth_keys(rend_service_t *s) strerror(errno)); goto err; } - } - SMARTLIST_FOREACH_END(client); + } SMARTLIST_FOREACH_END(client); + + finish_writing_to_file(open_cfile); + finish_writing_to_file(open_hfile); goto done; err: r = -1; + if (open_cfile) + abort_writing_to_file(open_cfile); + if (open_hfile) + abort_writing_to_file(open_hfile); done: + /* XXXX zero these two as well */ tor_free(client_keys_str); strmap_free(parsed_clients, rend_authorized_client_strmap_item_free); - if (r<0) { - /* Clear these here for the early error exit */ - /* We have to clear buf because encoded keys can get written to it */ - memset(buf, 0, sizeof(buf)); - memset(desc_cook_out, 0, sizeof(desc_cook_out)); - memset(service_id, 0, sizeof(service_id)); - memset(extended_desc_cookie, 0, sizeof(extended_desc_cookie)); - if (open_cfile) - abort_writing_to_file(open_cfile); - if (open_hfile) - abort_writing_to_file(open_hfile); - return r; - } else { - finish_writing_to_file(open_cfile); - finish_writing_to_file(open_hfile); - } - /* - * Clear stack buffers that held key-derived material; we do this here - * for each iteration so that we don't have to put this before all of - * the early error exits (check/create directory, check service file, - * and so forth) that otherwise might have leftover key from the - * previous iteration on the stack. - */ + memset(cfname, 0, sizeof(cfname)); + + /* Clear stack buffers that held key-derived material. */ memset(buf, 0, sizeof(buf)); memset(desc_cook_out, 0, sizeof(desc_cook_out)); memset(service_id, 0, sizeof(service_id));

1 0

[tor/master] add bug number and explanation to changes/bug2385; call it a feature
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit dcb1d0a67baa35b9645b19f2cf44f8ba3d0b30d1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 18 13:20:02 2012 -0400 add bug number and explanation to changes/bug2385; call it a feature --- changes/bug2385 | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/changes/bug2385 b/changes/bug2385 index e7151f4..5d571d9 100644 --- a/changes/bug2385 +++ b/changes/bug2385 @@ -1,5 +1,9 @@ - o Minor bugfixes: - - Clear keys and key-derived material left on the stack in rendservice.c - and rendclient.c + o Minor features (security): + - Clear keys and key-derived material left on the stack in + rendservice.c and rendclient.c. This should make us more + forward-secure against cold-boot attacks and the like. Fix for + bug 2385. + - Check return value of crypto_pk_write_private_key_to_string() in - end_service_load_keys() + end_service_load_keys(). This should make us more forward-secure + against cold-boot attacks and the like. Fix for bug 2385.

1 0

[tor/master] Merge remote-tracking branch 'public/bug2385'
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit 888d5d08fe4efb52a5923726dcfb75036bf90714 Merge: aad71ee dcb1d0a Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 25 12:05:36 2012 -0400 Merge remote-tracking branch 'public/bug2385' changes/bug2385 | 9 + src/common/util.c | 10 + src/common/util.h | 1 + src/or/config.c | 2 +- src/or/rendclient.c | 46 ++++- src/or/rendservice.c | 506 ++++++++++++++++++++++++++++--------------------- src/or/rendservice.h | 2 +- 7 files changed, 348 insertions(+), 228 deletions(-)

1 0

[tor/master] Clear a couple more fields in rend_service_load_auth_keys
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit 4a8eaad7efd4e24e89f969f79d9bde1874f366d1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 18 13:13:53 2012 -0400 Clear a couple more fields in rend_service_load_auth_keys --- src/common/util.c | 10 ++++++++++ src/common/util.h | 1 + src/or/rendservice.c | 4 +++- 3 files changed, 14 insertions(+), 1 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 28ecff3..cb9a65d 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -655,6 +655,16 @@ fast_memcmpstart(const void *mem, size_t memlen, return fast_memcmp(mem, prefix, plen); } +/** Given a nul-terminated string s, set every character before the nul + * to zero. */ +void +tor_strclear(char *s) +{ + while (*s) { + *s++ = '\0'; + } +} + /** Return a pointer to the first char of s that is not whitespace and * not a comment, or to the terminating NUL if no such character exists. */ diff --git a/src/common/util.h b/src/common/util.h index a2b196c..5e52292 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -188,6 +188,7 @@ int strcasecmpstart(const char *s1, const char *s2) ATTR_NONNULL((1,2)); int strcmpend(const char *s1, const char *s2) ATTR_NONNULL((1,2)); int strcasecmpend(const char *s1, const char *s2) ATTR_NONNULL((1,2)); int fast_memcmpstart(const void *mem, size_t memlen, const char *prefix); +void tor_strclear(char *s); void tor_strstrip(char *s, const char *strip) ATTR_NONNULL((1,2)); long tor_parse_long(const char *s, int base, long min, diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 22f3b27..38268d4 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -139,7 +139,9 @@ rend_authorized_client_free(rend_authorized_client_t *client) return; if (client->client_key) crypto_pk_free(client->client_key); + tor_strclear(client->client_name); tor_free(client->client_name); + memset(client->descriptor_cookie, 0, sizeof(client->descriptor_cookie)); tor_free(client); } @@ -868,7 +870,7 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) if (open_hfile) abort_writing_to_file(open_hfile); done: - /* XXXX zero these two as well */ + tor_strclear(client_keys_str); tor_free(client_keys_str); strmap_free(parsed_clients, rend_authorized_client_strmap_item_free);

1 0

[tor/master] Resolve crash caused by format_helper_exit_status changes in #5557
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit b1ad1a1d0266a20bb0dac15e65abe7b65a74e8a0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Jun 23 15:30:01 2012 -0400 Resolve crash caused by format_helper_exit_status changes in #5557 Because the string output was no longer equal in length to HEX_ERRNO_SIZE, the write() call would add some extra spaces and maybe a NUL, and the NUL would trigger an assert in get_string_from_pipe. Fixes bug 6225; bug not in any released version of Tor. --- src/common/util.c | 29 ++++++++++++++++++++--------- src/common/util.h | 4 ++-- src/test/test_util.c | 16 +++++++++++----- 3 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index cb8ff85..8cb013e 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3264,8 +3264,11 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, * in the processs of starting the child process did the failure occur (see * CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of * errno when the failure occurred. + * + * On success return the number of characters added to hex_errno, not counting + * the terminating NUL; return -1 on error. */ -void +int format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno) { @@ -3273,6 +3276,7 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, int written, left; char *cur; size_t i; + int res = -1; /* Fill hex_errno with spaces, and a trailing newline (memset may not be signal handler safe, so we can't use it) */ @@ -3343,6 +3347,8 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, *cur++ = '\n'; *cur++ = '\0'; + res = cur - hex_errno - 1; + goto done; err: @@ -3353,7 +3359,7 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, *hex_errno = '\0'; done: - return; + return res; } /* Maximum number of file descriptors, if we cannot get it via sysconf() */ @@ -3695,15 +3701,20 @@ tor_spawn_background(const char *const filename, const char **argv, child_state = CHILD_STATE_FAILEXEC; error: - /* XXX: are we leaking fds from the pipe? */ + { + /* XXX: are we leaking fds from the pipe? */ + int n; - format_helper_exit_status(child_state, errno, hex_errno); + n = format_helper_exit_status(child_state, errno, hex_errno); - /* Write the error message. GCC requires that we check the return - value, but there is nothing we can do if it fails */ - /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */ - nbytes = write(STDOUT_FILENO, error_message, error_message_length); - nbytes = write(STDOUT_FILENO, hex_errno, sizeof(hex_errno)); + if (n >= 0) { + /* Write the error message. GCC requires that we check the return + value, but there is nothing we can do if it fails */ + /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */ + nbytes = write(STDOUT_FILENO, error_message, error_message_length); + nbytes = write(STDOUT_FILENO, hex_errno, n); + } + } (void) nbytes; diff --git a/src/common/util.h b/src/common/util.h index 6b7c6fb..a2ab0cc 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -473,8 +473,8 @@ void tor_process_handle_destroy(process_handle_t *process_handle, int format_hex_number_for_helper_exit_status(unsigned int x, char *buf, int max_len); -void format_helper_exit_status(unsigned char child_state, - int saved_errno, char *hex_errno); +int format_helper_exit_status(unsigned char child_state, + int saved_errno, char *hex_errno); /* Space for hex values of child state, a slash, saved_errno (with leading minus) and newline (no null) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index d71d280..fab9595 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2138,28 +2138,34 @@ test_util_exit_status(void *ptr) { /* Leave an extra byte for a \0 so we can do string comparison */ char hex_errno[HEX_ERRNO_SIZE + 1]; + int n; (void)ptr; clear_hex_errno(hex_errno); - format_helper_exit_status(0, 0, hex_errno); + n = format_helper_exit_status(0, 0, hex_errno); test_streq("0/0\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); + n = format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); test_streq("0/7FFFFFFF\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0xFF, -0x80000000, hex_errno); + n = format_helper_exit_status(0xFF, -0x80000000, hex_errno); test_streq("FF/-80000000\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0x7F, 0, hex_errno); + n = format_helper_exit_status(0x7F, 0, hex_errno); test_streq("7F/0\n", hex_errno); + test_eq(n, strlen(hex_errno)); clear_hex_errno(hex_errno); - format_helper_exit_status(0x08, -0x242, hex_errno); + n = format_helper_exit_status(0x08, -0x242, hex_errno); test_streq("8/-242\n", hex_errno); + test_eq(n, strlen(hex_errno)); done: ;

1 0

[tor/master] Don't assert in get_string_from_pipe() on len==0
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit ffd7189b3fc015ce47e6ab27ac85d4eab2183a2b Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Jun 23 15:35:43 2012 -0400 Don't assert in get_string_from_pipe() on len==0 We can treat this case as an EAGAIN (probably because of an unexpected internal NUL) rather than a crash-worthy problem. Fixes bug 6225, again. Bug not in any released version of Tor. --- src/common/util.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 8cb013e..e5b51b9 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -4386,7 +4386,10 @@ get_string_from_pipe(FILE *stream, char *buf_out, size_t count) } } else { len = strlen(buf_out); - tor_assert(len>0); + if (len == 0) { + /* this probably means we got a NUL at the start of the string. */ + return IO_STREAM_EAGAIN; + } if (buf_out[len - 1] == '\n') { /* Remove the trailing newline */

1 0

[tor/master] Merge remote-tracking branch 'public/bug6225' into maint-0.2.3
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit 2703e072a1056edd3b68a16e045175e5bd268301 Merge: a6688c5 ffd7189 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 25 11:51:19 2012 -0400 Merge remote-tracking branch 'public/bug6225' into maint-0.2.3 src/common/util.c | 34 ++++++++++++++++++++++++---------- src/common/util.h | 4 ++-- src/test/test_util.c | 16 +++++++++++----- 3 files changed, 37 insertions(+), 17 deletions(-)

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.3'
by nickm＠torproject.org 25 Jun '12

25 Jun '12

commit aad71eef1b8cf5e6dd74371abc012b342ec3f011 Merge: 8e5d3ca 2703e07 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Jun 25 11:59:33 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' src/common/util.c | 34 ++++++++++++++++++++++++---------- src/common/util.h | 4 ++-- src/test/test_util.c | 16 +++++++++++----- 3 files changed, 37 insertions(+), 17 deletions(-)

1 0

[tor/maint-0.2.3] Don't assert in get_string_from_pipe() on len==0
by nickm＠torproject.org 25 Jun '12

25 Jun '12

1 0

← Newer
1
...
19
20
21
22
23
24
25
...
88
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits June 2012