Ticket33072 042 01

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

@@ -2931,6 +2931,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

@@ -56,7 +56,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 5569
+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/app/config/config.c

@@ -672,6 +672,7 @@ static const config_var_t option_vars_[] = {
   V(User,                        STRING,   NULL),
   OBSOLETE("UserspaceIOCPBuffers"),
   V(AuthDirRejectRequestsUnderLoad, BOOL,  "1"),
+  V(AuthDirRejectUncompressedRequests, BOOL, "1"),
   V(AuthDirSharedRandomness,     BOOL,     "1"),
   V(AuthDirTestEd25519LinkKeys,  BOOL,     "1"),
   OBSOLETE("V1AuthoritativeDirectory"),

src/app/config/or_options_st.h

@@ -1015,6 +1015,10 @@ struct or_options_t {
    * regardless of bandwidth pressure or not. */
   int AuthDirRejectRequestsUnderLoad;
 
+  /** Bool (default: 1) Reject uncompressed directory requests. A 503 error
+   * code is returned. */
+  int AuthDirRejectUncompressedRequests;
+
   /** Bool (default: 1): Switch for the shared random protocol. Only
    * relevant to a directory authority. If off, the authority won't
    * participate in the protocol. If on (default), a flag is added to the

src/core/mainloop/connection.c

@@ -3190,8 +3190,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.
@@ -3206,14 +3209,25 @@ 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 have reached the limit thus deny answering. */
+    if (c_method == NO_METHOD &&
+        get_options()->AuthDirRejectUncompressedRequests) {
+      return true;
+    }
+    if (c_method != NO_METHOD) {
+      /* Always answer compressed request. */
+      return false;
+    }
     /* Are we configured to possibly reject requests under load? */
     if (!get_options()->AuthDirRejectRequestsUnderLoad) {
       /* Answer request no matter what. */

src/core/mainloop/connection.h

@@ -12,6 +12,8 @@
 #ifndef TOR_CONNECTION_H
 #define TOR_CONNECTION_H
 
+#include "lib/compress/compress.h"
+
 listener_connection_t *TO_LISTENER_CONN(connection_t *);
 
 struct buf_t;
@@ -197,7 +199,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 or_options_t *options);
 void connection_bucket_refill_all(time_t now,

src/feature/dircache/dircache.c

@@ -951,7 +951,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.");
@@ -1060,7 +1061,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;
     }
@@ -1119,7 +1121,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.");
@@ -1217,7 +1220,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.");
@@ -1314,7 +1318,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

@@ -336,6 +336,8 @@ test_bwmgt_dir_conn_global_write_low(void *arg)
   mock_options.AuthoritativeDir = 1;
   mock_options.V3AuthoritativeDir = 1;
   mock_options.UseDefaultFallbackDirs = 0;
+  mock_options.AuthDirRejectUncompressedRequests = 0;
+  mock_options.AuthDirRejectRequestsUnderLoad = 0;
 
   /* This will set our global bucket to 1 byte and thus we will hit the
    * banwdith limit in our test. */
@@ -359,7 +361,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
@@ -369,46 +371,55 @@ 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. */
+  mock_options.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: