sendme-version-0 dir fetches don't count toward geoip

diff --git a/src/core/or/or_circuit_st.h b/src/core/or/or_circuit_st.h
index d5a70079284333d2352f12cd0fe29cf3f3f5e013..382cb84d208682b8f013473640d310bddef2b710 100644 (file)
--- a/src/core/or/or_circuit_st.h
+++ b/src/core/or/or_circuit_st.h
@@ -81,6 +81,11 @@ struct or_circuit_t {
    * circuit. */
   bool used_legacy_circuit_handshake;
 
+  /** True if we received a version 0 sendme on this circuit, and it came
+   * on a legacy (CREATE_FAST) circuit so we allowed it. We track this
+   * state so we can avoid counting those directory requests for geoip. */
+  bool used_obsolete_sendme;
+
   /** Number of cells that were removed from circuit queue; reset every
    * time when writing buffer stats to disk. */
   uint32_t processed_cells;

diff --git a/src/core/or/sendme.c b/src/core/or/sendme.c
index 20f3e5a7144fec3e0a02628a583eaa3165eac4f9..1d481e7f05327bbee632f9878322164e36119bda 100644 (file)
--- a/src/core/or/sendme.c
+++ b/src/core/or/sendme.c
@@ -157,7 +157,7 @@ cell_version_can_be_handled(uint8_t cell_version)
  * send/recv cells on a circuit. If the SENDME is invalid, the circuit should
  * be marked for close by the caller. */
 STATIC bool
-sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
+sendme_is_valid(circuit_t *circ, const uint8_t *cell_payload,
                 size_t cell_payload_len)
 {
   uint8_t cell_version;
@@ -183,10 +183,13 @@ sendme_is_valid(const circuit_t *circ, const uint8_t *cell_payload,
 
   /* Validate that we can handle this cell version. */
   if (CIRCUIT_IS_ORCIRC(circ) &&
-      CONST_TO_OR_CIRCUIT(circ)->used_legacy_circuit_handshake &&
+      TO_OR_CIRCUIT(circ)->used_legacy_circuit_handshake &&
       cell_version == 0) {
     /* exception, allow v0 sendmes on circuits made with CREATE_FAST */
     log_info(LD_CIRC, "Permitting sendme version 0 on legacy circuit.");
+    /* Record this choice on the circuit, so we can avoid counting
+     * directory fetches on this circuit toward our geoip stats. */
+    TO_OR_CIRCUIT(circ)->used_obsolete_sendme = 1;
   } else if (!cell_version_can_be_handled(cell_version)) {
     goto invalid;
   }

diff --git a/src/core/or/sendme.h b/src/core/or/sendme.h
index bc1daef23d027582cf3b0d082c83587e5e96cb75..7c8c025fd33587365dc0643cf3ccb54abb8dca3e 100644 (file)
--- a/src/core/or/sendme.h
+++ b/src/core/or/sendme.h
@@ -70,7 +70,7 @@ STATIC bool cell_version_can_be_handled(uint8_t cell_version);
 
 STATIC ssize_t build_cell_payload_v1(const uint8_t *cell_digest,
                                      uint8_t *payload);
-STATIC bool sendme_is_valid(const circuit_t *circ,
+STATIC bool sendme_is_valid(circuit_t *circ,
                             const uint8_t *cell_payload,
                             size_t cell_payload_len);
 STATIC bool circuit_sendme_cell_is_next(int deliver_window,

diff --git a/src/feature/dircache/dirserv.c b/src/feature/dircache/dirserv.c
index b675e164a52f1088909dab6a4d6ee23147205478..5673d1ec063e4f1698641c58738facd00aba49eb 100644 (file)
--- a/src/feature/dircache/dirserv.c
+++ b/src/feature/dircache/dirserv.c
@@ -779,10 +779,15 @@ connection_dirserv_flushed_some(dir_connection_t *conn)
     tor_compress_free(conn->compress_state);
     conn->compress_state = NULL;
   }
+
+  /* only count networkstatus serves as successful when the spool runs dry */
   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) {
+    /* but as a special case, check if conn is on a circuit that used a
+     * version-0 sendme (bugs 41191 and 41192), because we don't want to
+     * count clients that should exit after they receive our consensus. */
+    if (!connection_dir_used_obsolete_sendme(conn) &&
+        tor_addr_parse(&addr, (TO_CONN(conn))->address) >= 0) {
       geoip_note_client_seen(GEOIP_CLIENT_NETWORKSTATUS,
                              &addr, NULL,
                              time(NULL));

diff --git a/src/feature/dircommon/directory.c b/src/feature/dircommon/directory.c
index 96c6e8e060b59995d7664abbeca63f8b3c767ca5..06fbfd8d5d4341e172eadae65747154caf1db1a9 100644 (file)
--- a/src/feature/dircommon/directory.c
+++ b/src/feature/dircommon/directory.c
@@ -264,6 +264,38 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn)
   return !channel_is_client(CONST_TO_OR_CIRCUIT(circ)->p_chan);
 }
 
+/** Did <b>conn</b> ever send us a version 0 sendme cell and we allowed
+ * it? Used to decide whether to count consensus fetches from it in our
+ * geoip stats.
+ *
+ * Note that this function might have false negatives in some cases, i.e.
+ * it could tell us that the conn never sent a v0 sendme when actually it
+ * did but its linked edge connection or OR connection got broken before
+ * we called this function. For our geoip stats these false negatives
+ * would mean overcounting users by including some of the v0-using
+ * clients.
+ *
+ * We think these false positives should be unlikely or maybe even
+ * impossible when called from connection_dirserv_flushed_some(), but
+ * be careful calling it from elsewhere.
+ * */
+bool
+connection_dir_used_obsolete_sendme(const dir_connection_t *conn)
+{
+  const edge_connection_t *edge_conn = NULL;
+  const circuit_t *circ = NULL;
+  bool used_obsolete_sendme = 0;
+  const connection_t *linked_conn = TO_CONN(conn)->linked_conn;
+  if (linked_conn)
+    edge_conn = CONST_TO_EDGE_CONN(linked_conn);
+  if (edge_conn)
+    circ = edge_conn->on_circuit;
+  if (circ && CIRCUIT_IS_ORCIRC(circ))
+    used_obsolete_sendme = CONST_TO_OR_CIRCUIT(circ)->used_obsolete_sendme;
+
+  return used_obsolete_sendme;
+}
+
 /** Parse an HTTP request line at the start of a headers string.  On failure,
  * return -1.  On success, set *<b>command_out</b> to a copy of the HTTP
  * command ("get", "post", etc), set *<b>url_out</b> to a copy of the URL, and

diff --git a/src/feature/dircommon/directory.h b/src/feature/dircommon/directory.h
index 7d861682bb03c8b9e8bc15953dea63552b52eaa8..ffb2a87257cca1d29fb6f73112172a0d6d33bba8 100644 (file)
--- a/src/feature/dircommon/directory.h
+++ b/src/feature/dircommon/directory.h
@@ -95,6 +95,7 @@ char *http_get_header(const char *headers, const char *which);
 
 int connection_dir_is_encrypted(const dir_connection_t *conn);
 bool connection_dir_is_anonymous(const dir_connection_t *conn);
+bool connection_dir_used_obsolete_sendme(const dir_connection_t *conn);
 int connection_dir_reached_eof(dir_connection_t *conn);
 int connection_dir_process_inbuf(dir_connection_t *conn);
 int connection_dir_finished_flushing(dir_connection_t *conn);