From 26795da900d7b487b80e19b804475ad7b132188d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Mon, 22 May 2017 12:34:57 +0000 Subject: [PATCH 1/3] Don't add "Accept-Encoding" header if directory connection is anonymous. See: https://bugs.torproject.org/22305 --- src/or/directory.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index ef74c0fed4..5aab8a7328 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1677,6 +1677,7 @@ directory_send_command(dir_connection_t *conn, const char *payload = req->payload; const size_t payload_len = req->payload_len; const time_t if_modified_since = req->if_modified_since; + const int anonymized_connection = dirind_is_anon(req->indirection); char proxystring[256]; char hoststring[128]; @@ -1742,11 +1743,13 @@ directory_send_command(dir_connection_t *conn, proxystring[0] = 0; } - /* Add Accept-Encoding. */ - accept_encoding = accept_encoding_header(); - smartlist_add_asprintf(headers, "Accept-Encoding: %s\r\n", - accept_encoding); - tor_free(accept_encoding); + if (! anonymized_connection) { + /* Add Accept-Encoding. */ + accept_encoding = accept_encoding_header(); + smartlist_add_asprintf(headers, "Accept-Encoding: %s\r\n", + accept_encoding); + tor_free(accept_encoding); + } /* Add additional headers, if any */ { From 2b26ac139086c4ce3067b6d27644cb37808a9eb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Mon, 22 May 2017 14:42:18 +0000 Subject: [PATCH 2/3] Refactor error path handling in connection_dir_client_reached_eof(). This patch lifts the return value, rv, variable to the beginning of the function, adds a 'done' label for clean-up and function exit and makes the rest of the function use the rv value + goto done; instead of cleaning up in multiple places. See: https://bugs.torproject.org/22305 --- src/or/directory.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 5aab8a7328..495fd9c62a 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2204,8 +2204,8 @@ static int handle_response_upload_renddesc_v2(dir_connection_t *, static int connection_dir_client_reached_eof(dir_connection_t *conn) { - char *body; - char *headers; + char *body = NULL; + char *headers = NULL; char *reason = NULL; size_t body_len = 0; int status_code; @@ -2214,6 +2214,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) compress_method_t compression; int plausible; int skewed = 0; + int rv; int allow_partial = (conn->base_.purpose == DIR_PURPOSE_FETCH_SERVERDESC || conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO || conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC); @@ -2241,8 +2242,9 @@ connection_dir_client_reached_eof(dir_connection_t *conn) &compression, &reason) < 0) { log_warn(LD_HTTP,"Unparseable headers (server '%s:%d'). Closing.", conn->base_.address, conn->base_.port); - tor_free(body); tor_free(headers); - return -1; + + rv = -1; + goto done; } if (!reason) reason = tor_strdup("[no reason given]"); @@ -2316,8 +2318,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn) if ((ds = router_get_fallback_dirserver_by_digest(id_digest))) ds->fake_status.last_dir_503_at = now; - tor_free(body); tor_free(headers); tor_free(reason); - return -1; + rv = -1; + goto done; } plausible = body_is_plausible(body, body_len, conn->base_.purpose); @@ -2362,8 +2364,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn) log_fn(LOG_PROTOCOL_WARN, LD_HTTP, "Unable to decompress HTTP body (server '%s:%d').", conn->base_.address, conn->base_.port); - tor_free(body); tor_free(headers); tor_free(reason); - return -1; + rv = -1; + goto done; } if (new_body) { tor_free(body); @@ -2372,7 +2374,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) } } - int rv; response_handler_args_t args; memset(&args, 0, sizeof(args)); args.status_code = status_code; @@ -2421,6 +2422,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn) rv = -1; break; } + + done: tor_free(body); tor_free(headers); tor_free(reason); From 5a0eab68e1ca2c1f76ac3834b1e38938a5cb4546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20F=C3=A6r=C3=B8y?= Date: Mon, 22 May 2017 14:45:12 +0000 Subject: [PATCH 3/3] Ensure that only GZip and Zlib compression is handled for anonymous connections. See: https://bugs.torproject.org/22305 --- src/or/directory.c | 53 +++++++++++++++++++++++++++++++++++++++++++++- src/or/directory.h | 1 + 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/or/directory.c b/src/or/directory.c index 495fd9c62a..c13a98b9ac 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2219,6 +2219,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn) conn->base_.purpose == DIR_PURPOSE_FETCH_EXTRAINFO || conn->base_.purpose == DIR_PURPOSE_FETCH_MICRODESC); size_t received_bytes; + const int anonymized_connection = + purpose_needs_anonymity(conn->base_.purpose, + conn->router_purpose, + conn->requested_resource); received_bytes = connection_get_inbuf_len(TO_CONN(conn)); @@ -2347,13 +2351,29 @@ connection_dir_client_reached_eof(dir_connection_t *conn) description2, (compression>0 && guessed>0)?" Trying both.":""); } + /* Try declared compression first if we can. - * tor_compress_supports_method() also returns true for NO_METHOD. */ + * tor_compress_supports_method() also returns true for NO_METHOD. + * Ensure that the server is not sending us data compressed using a + * compression method that is not allowed for anonymous connections. */ + if (anonymized_connection && + ! allowed_anonymous_connection_compression_method(compression)) { + rv = -1; + goto done; + } + if (tor_compress_supports_method(compression)) tor_uncompress(&new_body, &new_len, body, body_len, compression, !allow_partial, LOG_PROTOCOL_WARN); + /* Okay, if that didn't work, and we think that it was compressed * differently, try that. */ + if (anonymized_connection && + ! allowed_anonymous_connection_compression_method(guessed)) { + rv = -1; + goto done; + } + if (!new_body && tor_compress_supports_method(guessed) && compression != guessed) tor_uncompress(&new_body, &new_len, body, body_len, guessed, @@ -3339,6 +3359,14 @@ static compress_method_t srv_meth_pref_streaming_compression[] = { NO_METHOD }; +/** Array of allowed compression methods to use (if supported) when receiving a + * response from a request that was required to be anonymous. */ +static compress_method_t client_meth_allowed_anonymous_compression[] = { + ZLIB_METHOD, + GZIP_METHOD, + NO_METHOD +}; + /** Parse the compression methods listed in an Accept-Encoding header h, * and convert them to a bitfield where compression method x is supported if * and only if 1 << x is set in the bitfield. */ @@ -3838,6 +3866,29 @@ find_best_compression_method(unsigned compression_methods, int stream) return NO_METHOD; } +/** Check if the given compression method is allowed for a connection that is + * supposed to be anonymous. Returns 1 if the compression method is allowed, + * otherwise 0. */ +STATIC int +allowed_anonymous_connection_compression_method(compress_method_t method) +{ + unsigned u; + + for (u = 0; u < ARRAY_LENGTH(client_meth_allowed_anonymous_compression); + ++u) { + compress_method_t allowed_method = + client_meth_allowed_anonymous_compression[u]; + + if (! tor_compress_supports_method(allowed_method)) + continue; + + if (method == allowed_method) + return 1; + } + + return 0; +} + /** Encodes the results of parsing a consensus request to figure out what * consensus, and possibly what diffs, the user asked for. */ typedef struct { diff --git a/src/or/directory.h b/src/or/directory.h index a015c7045d..6dc773439f 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -161,6 +161,7 @@ STATIC int handle_get_hs_descriptor_v3(dir_connection_t *conn, const struct get_handler_args_t *args); STATIC int directory_handle_command(dir_connection_t *conn); STATIC char *accept_encoding_header(void); +STATIC int allowed_anonymous_connection_compression_method(compress_method_t); #endif