[tor-commits] [tor/master] sendme: Always close stream if deliver window is negative

[asn at torproject.org]

commit 0e6e800c89c7cfef255491179473e13de5f72d03
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed Jan 9 11:03:49 2019 -0500

    sendme: Always close stream if deliver window is negative
    
    Previously, we would only close the stream when our deliver window was
    negative at the circuit-level but _not_ at the stream-level when receiving a
    DATA cell.
    
    This commit adds an helper function connection_edge_end_close() which
    sends an END and then mark the stream for close for a given reason.
    
    That function is now used both in case the deliver window goes below zero for
    both circuit and stream level.
    
    Part of #26840
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 src/core/or/connection_edge.c | 19 +++++++++++++++++++
 src/core/or/connection_edge.h |  1 +
 src/core/or/relay.c           | 14 +++++---------
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/core/or/connection_edge.c b/src/core/or/connection_edge.c
index fa72fb771..8ed403456 100644
--- a/src/core/or/connection_edge.c
+++ b/src/core/or/connection_edge.c
@@ -4565,6 +4565,25 @@ circuit_clear_isolation(origin_circuit_t *circ)
   circ->socks_username_len = circ->socks_password_len = 0;
 }
 
+/** Send an END and mark for close the given edge connection conn using the
+ * given reason that has to be a stream reason.
+ *
+ * Note: We don't unattached the AP connection (if applicable) because we
+ * don't want to flush the remaining data. This function aims at ending
+ * everything quickly regardless of the connection state.
+ *
+ * This function can't fail and does nothing if conn is NULL. */
+void
+connection_edge_end_close(edge_connection_t *conn, uint8_t reason)
+{
+  if (!conn) {
+    return;
+  }
+
+  connection_edge_end(conn, reason);
+  connection_mark_for_close(TO_CONN(conn));
+}
+
 /** Free all storage held in module-scoped variables for connection_edge.c */
 void
 connection_edge_free_all(void)
diff --git a/src/core/or/connection_edge.h b/src/core/or/connection_edge.h
index 68d8b19a1..e82b6bd76 100644
--- a/src/core/or/connection_edge.h
+++ b/src/core/or/connection_edge.h
@@ -80,6 +80,7 @@ int connection_edge_process_inbuf(edge_connection_t *conn,
 int connection_edge_destroy(circid_t circ_id, edge_connection_t *conn);
 int connection_edge_end(edge_connection_t *conn, uint8_t reason);
 int connection_edge_end_errno(edge_connection_t *conn);
+void connection_edge_end_close(edge_connection_t *conn, uint8_t reason);
 int connection_edge_flushed_some(edge_connection_t *conn);
 int connection_edge_finished_flushing(edge_connection_t *conn);
 int connection_edge_finished_connecting(edge_connection_t *conn);
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 06e201f20..6ff053d8a 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -1550,17 +1550,12 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
       ++stats_n_data_cells_received;
 
       /* Update our circuit-level deliver window that we received a DATA cell.
-       * If the deliver window goes below 0, we end the connection due to a
-       * protocol failure. */
+       * If the deliver window goes below 0, we end the circuit and stream due
+       * to a protocol failure. */
       if (sendme_circuit_data_received(circ, layer_hint) < 0) {
         log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
                "(relay data) circ deliver_window below 0. Killing.");
-        if (conn) {
-          /* XXXX Do we actually need to do this?  Will killing the circuit
-           * not send an END and mark the stream for close as appropriate? */
-          connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL);
-          connection_mark_for_close(TO_CONN(conn));
-        }
+        connection_edge_end_close(conn, END_STREAM_REASON_TORPROTOCOL);
         return -END_CIRC_REASON_TORPROTOCOL;
       }
 
@@ -1590,11 +1585,12 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ,
 
       /* Update our stream-level deliver window that we just received a DATA
        * cell. Going below 0 means we have a protocol level error so the
-       * circuit is closed. */
+       * stream and circuit are closed. */
 
       if (sendme_stream_data_received(conn) < 0) {
         log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
                "(relay data) conn deliver_window below 0. Killing.");
+        connection_edge_end_close(conn, END_STREAM_REASON_TORPROTOCOL);
         return -END_CIRC_REASON_TORPROTOCOL;
       }
       /* Total all valid application bytes delivered */