Don't count networkstatus serves until they finish

When we started serving a consensus document but the client didn't
receive all of it, we were still counting that as a success in our stats.

This mistake, which can be triggered for example by obsolete clients or
by DPI-based censorship, led to wildly inflated user counts because we
estimate total users in the world based on successful consensus fetches.

Fixes bug 41192; bugfix on 0.2.1.1-alpha.

diff --git a/changes/bug41192 b/changes/bug41192
new file mode 100644 (file)
index 0000000..5091667
--- /dev/null
+++ b/changes/bug41192
@@ -0,0 +1,9 @@
+  o Major bugfixes (directory servers):
+    - Don't count networkstatus serves until they finish. When we started
+      serving a consensus document but the client didn't receive all of
+      it, we were still counting that as a success in our stats. This
+      mistake, which can be triggered for example by obsolete clients
+      or by DPI-based censorship, led to wildly inflated user counts
+      because we estimate total users in the world based on successful
+      consensus fetches. Fixes bug 41192; bugfix on 0.2.1.1-alpha.
+

diff --git a/src/feature/dircache/dircache.c b/src/feature/dircache/dircache.c
index 6fe3d407915a5f0ab2d1ff526b85dd8d8a2a1e39..31a806eea1f43a9b77de565feb369600afcdf52f 100644 (file)
--- a/src/feature/dircache/dircache.c
+++ b/src/feature/dircache/dircache.c
@@ -968,12 +968,11 @@ handle_get_current_consensus(dir_connection_t *conn,
     goto done;
   }
 
+  /* Success: we are going to try serving it. */
+  conn->should_count_geoip_when_finished = 1;
+
   tor_addr_t addr;
   if (tor_addr_parse(&addr, (TO_CONN(conn))->address) >= 0) {
-    geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS,
-                           &addr, NULL,
-                           time(NULL));
-    geoip_note_ns_response(GEOIP_SUCCESS);
     /* Note that a request for a network status has started, so that we
      * can measure the download time later on. */
     if (conn->dirreq_id)

diff --git a/src/feature/dircache/dirserv.c b/src/feature/dircache/dirserv.c
index 2b5349923deae0d08edf1edbae7b19d5add1c2e1..b675e164a52f1088909dab6a4d6ee23147205478 100644 (file)
--- a/src/feature/dircache/dirserv.c
+++ b/src/feature/dircache/dirserv.c
@@ -15,6 +15,7 @@
 #include "feature/nodelist/routerlist.h"
 #include "feature/relay/router.h"
 #include "feature/relay/routermode.h"
+#include "feature/stats/geoip_stats.h"
 #include "feature/stats/predict_ports.h"
 
 #include "feature/dircache/cached_dir_st.h"
@@ -778,6 +779,17 @@ connection_dirserv_flushed_some(dir_connection_t *conn)
     tor_compress_free(conn->compress_state);
     conn->compress_state = NULL;
   }
+  if (conn->should_count_geoip_when_finished) {
+    /* only count successfully networkstatus serves when the spool runs dry */
+    tor_addr_t addr;
+    if (tor_addr_parse(&addr, (TO_CONN(conn))->address) >= 0) {
+      geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS,
+                             &addr, NULL,
+                             time(NULL));
+    }
+    geoip_note_ns_response(GEOIP_SUCCESS);
+    conn->should_count_geoip_when_finished = 0;
+  }
   return 0;
 }
 

diff --git a/src/feature/dircommon/dir_connection_st.h b/src/feature/dircommon/dir_connection_st.h
index 2a3dea5e6656a905e540cab2693abf454b50a64c..ee26fd362740a381a07b37bc780a063c0b74b0cc 100644 (file)
--- a/src/feature/dircommon/dir_connection_st.h
+++ b/src/feature/dircommon/dir_connection_st.h
@@ -61,6 +61,10 @@ struct dir_connection_t {
    * needs this for the incoming side, so it's moved here. */
   uint64_t dirreq_id;
 
+  /** 0 normally, 1 if we're serving a consensus and we're delaying counting
+   * geoip until we've served the final bytes. */
+  bool should_count_geoip_when_finished;
+
 #ifdef MEASUREMENTS_21206
   /** Number of RELAY_DATA cells received. */
   uint32_t data_cells_received;