Browse Source

Merge branch 'mikeperry_bug26214-rebased_squashed' into maint-0.3.4

Nick Mathewson 5 years ago
parent
commit
bfcfeaed07
4 changed files with 42 additions and 3 deletions
  1. 3 0
      changes/bug26214
  2. 1 0
      src/or/or.h
  3. 22 2
      src/or/relay.c
  4. 16 1
      src/test/test_relaycell.c

+ 3 - 0
changes/bug26214

@@ -0,0 +1,3 @@
+  o Minor bugfixes (correctness, flow control):
+    - Upon receiving a stream-level SENDME cell, verify that our window has
+      not grown too large. Fixes bug 26214; bugfix on svn r54 (pre-0.0.1)

+ 1 - 0
src/or/or.h

@@ -929,6 +929,7 @@ typedef enum {
 /** Initial value on both sides of a stream transmission window when the
  * stream is initialized.  Measured in cells. */
 #define STREAMWINDOW_START 500
+#define STREAMWINDOW_START_MAX 500
 /** Amount to increment a stream window when we get a stream SENDME. */
 #define STREAMWINDOW_INCREMENT 50
 

+ 22 - 2
src/or/relay.c

@@ -1752,8 +1752,7 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
           circuit_resume_edge_reading(circ, layer_hint);
 
           /* We count circuit-level sendme's as valid delivered data because
-           * they are rate limited. Note that we cannot count stream
-           * sendme's because the other end could send as many as they like.
+           * they are rate limited.
            */
           if (CIRCUIT_IS_ORIGIN(circ)) {
             circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ),
@@ -1783,6 +1782,27 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
                  rh.stream_id);
         return 0;
       }
+
+      /* Don't allow the other endpoint to request more than our maximim
+       * (ie initial) stream SENDME window worth of data. Well-behaved
+       * stock clients will not request more than this max (as per the check
+       * in the while loop of connection_edge_consider_sending_sendme()).
+       */
+      if (conn->package_window + STREAMWINDOW_INCREMENT >
+          STREAMWINDOW_START_MAX) {
+        static struct ratelim_t stream_warn_ratelim = RATELIM_INIT(600);
+        log_fn_ratelim(&stream_warn_ratelim,LOG_PROTOCOL_WARN, LD_PROTOCOL,
+               "Unexpected stream sendme cell. Closing circ (window %d).",
+               conn->package_window);
+        return -END_CIRC_REASON_TORPROTOCOL;
+      }
+
+      /* At this point, the stream sendme is valid */
+      if (CIRCUIT_IS_ORIGIN(circ)) {
+        circuit_read_valid_data(TO_ORIGIN_CIRCUIT(circ),
+                                rh.length);
+      }
+
       conn->package_window += STREAMWINDOW_INCREMENT;
       log_debug(domain,"stream-level sendme, packagewindow now %d.",
                 conn->package_window);

+ 16 - 1
src/test/test_relaycell.c

@@ -240,11 +240,26 @@ test_circbw_relay(void *arg)
                                      circ->cpath);
   ASSERT_UNCOUNTED_BW();
 
-  /* Sendme on stream: not counted */
+  /* Sendme on valid stream: counted */
   ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0;
   PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234");
   connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn,
                                      circ->cpath);
+  ASSERT_COUNTED_BW();
+
+  /* Sendme on valid stream with full window: not counted */
+  ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0;
+  PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234");
+  edgeconn->package_window = 500;
+  connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), edgeconn,
+                                     circ->cpath);
+  ASSERT_UNCOUNTED_BW();
+
+  /* Sendme on unknown stream: not counted */
+  ENTRY_TO_CONN(entryconn)->outbuf_flushlen = 0;
+  PACK_CELL(1, RELAY_COMMAND_SENDME, "Data1234");
+  connection_edge_process_relay_cell(&cell, TO_CIRCUIT(circ), NULL,
+                                     circ->cpath);
   ASSERT_UNCOUNTED_BW();
 
   /* Sendme on circuit with full window: not counted */