Ticket33072 043 02

changes/ticket33072

@@ -0,0 +1,4 @@
+  o Minor bugfix (directory authority):
+    - Always allow compressed directory requests and introduce option
+      AuthDirRejectUncompressedRequests to control that behavior. Fixes bug
+      33072; bugfix on 0.1.2.5-alpha.

doc/tor.1.txt

@@ -3090,6 +3090,10 @@ on the public Tor network.
     bandwidth pressure (reaching the configured limit if any). Relays will
     always tried to be answered even if this is on. (Default: 1)
 
+[[AuthDirRejectUncompressedRequests]] **AuthDirRejectUncompressedRequests** **0**|**1**::
+    If set, the directory authority will reject every uncompressed directory
+    requests that is requests not ending with ".z" and lacking
+    "Accept-Encoding". (Default: 1)
 
 HIDDEN SERVICE OPTIONS
 ----------------------

scripts/maint/practracker/exceptions.txt

@@ -61,7 +61,7 @@ problem dependency-violation /src/core/crypto/onion_crypto.c 5
 problem dependency-violation /src/core/crypto/onion_fast.c 1
 problem dependency-violation /src/core/crypto/onion_tap.c 3
 problem dependency-violation /src/core/crypto/relay_crypto.c 9
-problem file-size /src/core/mainloop/connection.c 5577
+problem file-size /src/core/mainloop/connection.c 5694
 problem include-count /src/core/mainloop/connection.c 62
 problem function-size /src/core/mainloop/connection.c:connection_free_minimal() 185
 problem function-size /src/core/mainloop/connection.c:connection_listener_new() 324

src/core/mainloop/connection.c

@@ -3277,8 +3277,11 @@ connection_bucket_write_limit(connection_t *conn, time_t now)
  * shouldn't send <b>attempt</b> bytes of low-priority directory stuff
  * out to <b>conn</b>.
  *
- * If we are a directory authority, always answer dir requests thus true is
- * always returned.
+ * If we are a directory authority, false is returned (indicating that we
+ * should answer the request) if one of these conditions is met:
+ *  - Connection is from a known relay (address is looked up).
+ *  - AuthDirRejectRequestsUnderLoad is set to 0.
+ *  - Compression is being used for the request (looking at c_method).
  *
  * Note: There are a lot of parameters we could use here:
  * - global_relayed_write_bucket. Low is bad.
@@ -3293,14 +3296,21 @@ connection_bucket_write_limit(connection_t *conn, time_t now)
  *   that's harder to quantify and harder to keep track of.
  */
 bool
-connection_dir_is_global_write_low(const connection_t *conn, size_t attempt)
+connection_dir_is_global_write_low(const connection_t *conn, size_t attempt,
+                                   const compress_method_t c_method)
 {
   size_t smaller_bucket =
     MIN(token_bucket_rw_get_write(&global_bucket),
         token_bucket_rw_get_write(&global_relayed_bucket));
 
   /* Special case for authorities (directory only). */
   if (authdir_mode_v3(get_options())) {
+    /* If this requests is uncompressed and we are configured to reject those,
+     * indicate that we have reached the limit thus deny answering. */
+    if (c_method == NO_METHOD &&
+        dirauth_should_reject_uncompressed_requests()) {
+      return true;
+    }
     /* Are we configured to possibly reject requests under load? */
     if (!dirauth_should_reject_requests_under_load()) {
       /* Answer request no matter what. */

src/core/mainloop/connection.h

@@ -12,6 +12,7 @@
 #ifndef TOR_CONNECTION_H
 #define TOR_CONNECTION_H
 
+#include "lib/compress/compress.h"
 #include "lib/smartlist_core/smartlist_core.h"
 #include "lib/log/log.h"
 
@@ -220,7 +221,8 @@ void connection_mark_all_noncontrol_connections(void);
 
 ssize_t connection_bucket_write_limit(struct connection_t *conn, time_t now);
 bool connection_dir_is_global_write_low(const struct connection_t *conn,
-                                        size_t attempt);
+                                        size_t attempt,
+                                        const compress_method_t c_method);
 void connection_bucket_init(void);
 void connection_bucket_adjust(const struct or_options_t *options);
 void connection_bucket_refill_all(time_t now,

src/feature/dirauth/dirauth_config.c

@@ -54,6 +54,14 @@ dirauth_should_reject_requests_under_load(void)
   return !!dirauth_get_options()->AuthDirRejectRequestsUnderLoad;
 }
 
+/** Return true iff we are configured to reject uncompressed directory
+ * requests. */
+bool
+dirauth_should_reject_uncompressed_requests(void)
+{
+  return !!dirauth_get_options()->AuthDirRejectUncompressedRequests;
+}
+
 /**
  * Legacy validation/normalization function for the dirauth mode options in
  * options. Uses old_options as the previous options.

src/feature/dirauth/dirauth_config.h

@@ -36,6 +36,7 @@ int options_act_dirauth_stats(const struct or_options_t *old_options,
                               bool *print_notice_out);
 
 bool dirauth_should_reject_requests_under_load(void);
+bool dirauth_should_reject_uncompressed_requests(void);
 
 extern const struct config_format_t dirauth_options_fmt;
 
@@ -81,6 +82,7 @@ options_validate_dirauth_mode(const struct or_options_t *old_options,
   (((void)(old_options)),((void)(print_notice_out)),0)
 
 #define dirauth_should_reject_requests_under_load() (false)
+#define dirauth_should_reject_uncompressed_requests() (false)
 
 #endif /* defined(HAVE_MODULE_DIRAUTH) */
 

src/feature/dirauth/dirauth_options.inc

@@ -102,4 +102,8 @@ CONF_VAR(VersioningAuthoritativeDirectory, BOOL, 0, "0")
  * pressure or not. */
 CONF_VAR(AuthDirRejectRequestsUnderLoad, BOOL, 0, "1")
 
+/** Boolean: If set, reject uncompressed directory requests. A 503 error code
+* is returned. */
+CONF_VAR(AuthDirRejectUncompressedRequests, BOOL, 0, "1")
+
 END_CONF_STRUCT(dirauth_options_t)

src/feature/dircache/dircache.c

@@ -957,7 +957,8 @@ handle_get_current_consensus(dir_connection_t *conn,
     goto done;
   }
 
-  if (connection_dir_is_global_write_low(TO_CONN(conn), size_guess)) {
+  if (connection_dir_is_global_write_low(TO_CONN(conn), size_guess,
+                                         compress_method)) {
     log_debug(LD_DIRSERV,
               "Client asked for network status lists, but we've been "
               "writing too many bytes lately. Sending 503 Dir busy.");
@@ -1066,7 +1067,8 @@ handle_get_status_vote(dir_connection_t *conn, const get_handler_args_t *args)
         }
       });
 
-    if (connection_dir_is_global_write_low(TO_CONN(conn), estimated_len)) {
+    if (connection_dir_is_global_write_low(TO_CONN(conn), estimated_len,
+                                           compress_method)) {
       write_short_http_response(conn, 503, "Directory busy, try again later");
       goto vote_done;
     }
@@ -1125,7 +1127,8 @@ handle_get_microdesc(dir_connection_t *conn, const get_handler_args_t *args)
       write_short_http_response(conn, 404, "Not found");
       goto done;
     }
-    if (connection_dir_is_global_write_low(TO_CONN(conn), size_guess)) {
+    if (connection_dir_is_global_write_low(TO_CONN(conn), size_guess,
+                                           compress_method)) {
       log_info(LD_DIRSERV,
                "Client asked for server descriptors, but we've been "
                "writing too many bytes lately. Sending 503 Dir busy.");
@@ -1223,7 +1226,8 @@ handle_get_descriptor(dir_connection_t *conn, const get_handler_args_t *args)
         msg = "Not found";
       write_short_http_response(conn, 404, msg);
     } else {
-      if (connection_dir_is_global_write_low(TO_CONN(conn), size_guess)) {
+      if (connection_dir_is_global_write_low(TO_CONN(conn), size_guess,
+                                             compress_method)) {
         log_info(LD_DIRSERV,
                  "Client asked for server descriptors, but we've been "
                  "writing too many bytes lately. Sending 503 Dir busy.");
@@ -1320,7 +1324,8 @@ handle_get_keys(dir_connection_t *conn, const get_handler_args_t *args)
                       len += c->cache_info.signed_descriptor_len);
 
     if (connection_dir_is_global_write_low(TO_CONN(conn),
-                                compress_method != NO_METHOD ? len/2 : len)) {
+                                compress_method != NO_METHOD ? len/2 : len,
+                                compress_method)) {
       write_short_http_response(conn, 503, "Directory busy, try again later");
       goto keys_done;
     }

src/test/test_bwmgt.c

@@ -334,6 +334,7 @@ test_bwmgt_dir_conn_global_write_low(void *arg)
   nodelist_set_consensus(dummy_ns);
 
   dirauth_opts = tor_malloc_zero(sizeof(dirauth_options_t));
+  dirauth_opts->AuthDirRejectUncompressedRequests = 0;
   dirauth_opts->AuthDirRejectRequestsUnderLoad = 0;
   dirauth_set_options(dirauth_opts);
 
@@ -367,7 +368,7 @@ test_bwmgt_dir_conn_global_write_low(void *arg)
    * that our limit is _not_ low. */
   addr_family = tor_addr_parse(&conn->addr, "1.1.1.1");
   tt_int_op(addr_family, OP_EQ, AF_INET);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 0);
 
   /* Now, we will reject requests under load so try again a non authority non
@@ -377,53 +378,63 @@ test_bwmgt_dir_conn_global_write_low(void *arg)
 
   addr_family = tor_addr_parse(&conn->addr, "1.1.1.1");
   tt_int_op(addr_family, OP_EQ, AF_INET);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 1);
 
   /* Now, lets try with a connection address from moria1. It should always
    * pass even though our limit is too low. */
   addr_family = tor_addr_parse(&conn->addr, "128.31.0.39");
   tt_int_op(addr_family, OP_EQ, AF_INET);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 0);
 
   /* IPv6 testing of gabelmoo. */
   addr_family = tor_addr_parse(&conn->addr, "[2001:638:a000:4140::ffff:189]");
   tt_int_op(addr_family, OP_EQ, AF_INET6);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 0);
 
   /* Lets retry with a known relay address. It should pass. Possible due to
    * our consensus setting above. */
   memcpy(&conn->addr, &relay_addr, sizeof(tor_addr_t));
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 0);
 
   /* Lets retry with a random IP that is not an authority nor a relay. */
   addr_family = tor_addr_parse(&conn->addr, "1.2.3.4");
   tt_int_op(addr_family, OP_EQ, AF_INET);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 0);
 
+  /* Compress method should always let us through. */
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, ZLIB_METHOD);
+  tt_int_op(ret, OP_EQ, 0);
+
+  /* Lets configure ourselves to reject uncompressed requests. */
+  dirauth_opts->AuthDirRejectUncompressedRequests = 1;
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
+  tt_int_op(ret, OP_EQ, 1);
+
   /* Finally, just make sure it still denies an IP if we are _not_ a v3
    * directory authority. */
   mock_options.V3AuthoritativeDir = 0;
   addr_family = tor_addr_parse(&conn->addr, "1.2.3.4");
   tt_int_op(addr_family, OP_EQ, AF_INET);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 1);
 
   /* Random IPv6 should not be allowed. */
   addr_family = tor_addr_parse(&conn->addr, "[CAFE::ACAB]");
   tt_int_op(addr_family, OP_EQ, AF_INET6);
-  ret = connection_dir_is_global_write_low(conn, INT_MAX);
+  ret = connection_dir_is_global_write_low(conn, INT_MAX, NO_METHOD);
   tt_int_op(ret, OP_EQ, 1);
 
  done:
   connection_free_minimal(conn);
   routerstatus_free(rs); routerinfo_free(ri); microdesc_free(md);
   smartlist_clear(dummy_ns->routerstatus_list);
   networkstatus_vote_free(dummy_ns);
+  tor_free(dirauth_opts);
 
   UNMOCK(get_estimated_address_per_node);
   UNMOCK(networkstatus_get_latest_consensus);