Browse Source

Merge remote-tracking branch 'tor-github/pr/1466' into maint-0.4.2

teor 4 years ago
parent
commit
c8aa3cc17d
3 changed files with 32 additions and 6 deletions
  1. 5 0
      changes/ticket31958
  2. 8 2
      src/feature/dircache/dircache.c
  3. 19 4
      src/feature/dircommon/directory.c

+ 5 - 0
changes/ticket31958

@@ -0,0 +1,5 @@
+  o Minor bugfixes (directory):
+    - When checking if a directory connection is anonymous, test if the circuit
+      was marked for close before looking at its channel. This avoids a BUG()
+      stacktrace in case it was previously closed. Fixes bug 31958; bugfix on
+      0.4.2.1-alpha.

+ 8 - 2
src/feature/dircache/dircache.c

@@ -1393,7 +1393,8 @@ handle_get_hs_descriptor_v3(dir_connection_t *conn,
   /* Reject non anonymous dir connections (which also tests if encrypted). We
    * do not allow single hop clients to query an HSDir. */
   if (!connection_dir_is_anonymous(conn)) {
-    write_short_http_response(conn, 404, "Not found");
+    write_short_http_response(conn, 503,
+                              "Rejecting single hop HS v3 descriptor request");
     goto done;
   }
 
@@ -1636,7 +1637,12 @@ directory_handle_command_post,(dir_connection_t *conn, const char *headers,
   /* Handle HS descriptor publish request. We force an anonymous connection
    * (which also tests for encrypted). We do not allow single-hop client to
    * post a descriptor onto an HSDir. */
-  if (connection_dir_is_anonymous(conn) && !strcmpstart(url, "/tor/hs/")) {
+  if (!strcmpstart(url, "/tor/hs/")) {
+    if (!connection_dir_is_anonymous(conn)) {
+      write_short_http_response(conn, 503,
+                                "Rejecting single hop HS descriptor post");
+      goto done;
+    }
     const char *msg = "HS descriptor stored successfully.";
 
     /* We most probably have a publish request for an HS descriptor. */

+ 19 - 4
src/feature/dircommon/directory.c

@@ -212,7 +212,8 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn)
    * be closed or marked for close. */
   if (linked_conn == NULL || linked_conn->magic != EDGE_CONNECTION_MAGIC ||
       conn->linked_conn_is_closed || conn->linked_conn->marked_for_close) {
-    log_info(LD_DIR, "Rejected HSDir request: not linked to edge");
+    log_debug(LD_DIR, "Directory connection is not anonymous: "
+                      "not linked to edge");
     return false;
   }
 
@@ -221,13 +222,27 @@ connection_dir_is_anonymous(const dir_connection_t *dir_conn)
 
   /* Can't be a circuit we initiated and without a circuit, no channel. */
   if (circ == NULL || CIRCUIT_IS_ORIGIN(circ)) {
-    log_info(LD_DIR, "Rejected HSDir request: not on OR circuit");
+    log_debug(LD_DIR, "Directory connection is not anonymous: "
+                      "not on OR circuit");
     return false;
   }
 
-  /* Get the previous channel to learn if it is a client or relay link. */
+  /* It is possible that the circuit was closed because one of the channel was
+   * closed or a DESTROY cell was received. Either way, this connection can
+   * not continue so return that it is not anonymous since we can not know for
+   * sure if it is. */
+  if (circ->marked_for_close) {
+    log_debug(LD_DIR, "Directory connection is not anonymous: "
+                      "circuit marked for close");
+    return false;
+  }
+
+  /* Get the previous channel to learn if it is a client or relay link. We
+   * BUG() because if the circuit is not mark for close, we ought to have a
+   * p_chan else we have a code flow issue. */
   if (BUG(CONST_TO_OR_CIRCUIT(circ)->p_chan == NULL)) {
-    log_info(LD_DIR, "Rejected HSDir request: no p_chan");
+    log_debug(LD_DIR, "Directory connection is not anonymous: "
+                      "no p_chan on circuit");
     return false;
   }