[tor-commits] [tor/master] dos: Change the DoS heartbeat line format

[nickm at torproject.org]

commit c96465259a71741eb90486056a62376c3475007e
Author: David Goulet <dgoulet at torproject.org>
Date:   Tue Feb 23 08:54:45 2021 -0500

    dos: Change the DoS heartbeat line format
    
    Fix a bug introduced in 94b56eaa7597e4a091a5b51d2c9032ea046631e3 which
    overwrite the connection message line.
    
    Furthermore, improve how we generate that line by using a smartlist and change
    the format so it is clearer of what is being rejected/detected and, if
    applicable, which option is disabled thus yielding no stats.
    
    Closes #40308
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 changes/changes40308   |  5 ++++
 src/core/or/dos.c      | 74 ++++++++++++++++++++++++--------------------------
 src/test/test_status.c |  4 ++-
 3 files changed, 43 insertions(+), 40 deletions(-)

diff --git a/changes/changes40308 b/changes/changes40308
new file mode 100644
index 0000000000..d2b91f9299
--- /dev/null
+++ b/changes/changes40308
@@ -0,0 +1,5 @@
+  o Minor feature (DoS log heartbeat):
+    - Change the DoS subsystem heartbeat line format so be more clear on what
+      has been detected/rejected and which option is disabled if any. Closes
+      ticket 40308.
+
diff --git a/src/core/or/dos.c b/src/core/or/dos.c
index ba4e5442d6..b00863c118 100644
--- a/src/core/or/dos.c
+++ b/src/core/or/dos.c
@@ -776,58 +776,54 @@ dos_should_refuse_single_hop_client(void)
 void
 dos_log_heartbeat(void)
 {
-  char *conn_msg = NULL;
-  char *cc_msg = NULL;
-  char *single_hop_client_msg = NULL;
-  char *circ_stats_msg = NULL;
-  char *hs_dos_intro2_msg = NULL;
+  smartlist_t *elems = smartlist_new();
 
   /* Stats number coming from relay.c append_cell_to_circuit_queue(). */
-  tor_asprintf(&circ_stats_msg,
-               " %" PRIu64 " circuits killed with too many cells.",
-               stats_n_circ_max_cell_reached);
+  smartlist_add_asprintf(elems,
+                         "%" PRIu64 " circuits killed with too many cells",
+                         stats_n_circ_max_cell_reached);
 
   if (dos_cc_enabled) {
-    tor_asprintf(&cc_msg,
-                 " %" PRIu64 " circuits rejected,"
-                 " %" PRIu32 " marked addresses.",
-                 cc_num_rejected_cells, cc_num_marked_addrs);
+    smartlist_add_asprintf(elems,
+                           "%" PRIu64 " circuits rejected, "
+                           "%" PRIu32 " marked addresses",
+                           cc_num_rejected_cells, cc_num_marked_addrs);
+  } else {
+    smartlist_add_asprintf(elems, "[DoSCircuitCreationEnabled disabled]");
   }
 
   if (dos_conn_enabled) {
-    tor_asprintf(&conn_msg,
-                 " %" PRIu64 " connections closed.",
-                 conn_num_addr_rejected);
-    tor_asprintf(&conn_msg,
-                 " %" PRIu64 " connect() connections closed.",
-                 conn_num_addr_connect_rejected);
+    smartlist_add_asprintf(elems,
+                           "%" PRIu64 " same address concurrent "
+                           "connections rejected", conn_num_addr_rejected);
+    smartlist_add_asprintf(elems,
+                           "%" PRIu64 " connections rejected",
+                           conn_num_addr_connect_rejected);
+  } else {
+    smartlist_add_asprintf(elems, "[DoSConnectionEnabled disabled]");
   }
 
   if (dos_should_refuse_single_hop_client()) {
-    tor_asprintf(&single_hop_client_msg,
-                 " %" PRIu64 " single hop clients refused.",
-                 num_single_hop_client_refused);
+    smartlist_add_asprintf(elems,
+                           "%" PRIu64 " single hop clients refused",
+                           num_single_hop_client_refused);
+  } else {
+    smartlist_add_asprintf(elems,
+                           "[DoSRefuseSingleHopClientRendezvous disabled]");
   }
 
   /* HS DoS stats. */
-  tor_asprintf(&hs_dos_intro2_msg,
-               " %" PRIu64 " INTRODUCE2 rejected.",
-               hs_dos_get_intro2_rejected_count());
-
-  log_notice(LD_HEARTBEAT,
-             "DoS mitigation since startup:%s%s%s%s%s",
-             circ_stats_msg,
-             (cc_msg != NULL) ? cc_msg : " [cc not enabled]",
-             (conn_msg != NULL) ? conn_msg : " [conn not enabled]",
-             (single_hop_client_msg != NULL) ? single_hop_client_msg : "",
-             (hs_dos_intro2_msg != NULL) ? hs_dos_intro2_msg : "");
-
-  tor_free(conn_msg);
-  tor_free(cc_msg);
-  tor_free(single_hop_client_msg);
-  tor_free(circ_stats_msg);
-  tor_free(hs_dos_intro2_msg);
-  return;
+  smartlist_add_asprintf(elems,
+                         "%" PRIu64 " INTRODUCE2 rejected",
+                         hs_dos_get_intro2_rejected_count());
+
+  char *msg = smartlist_join_strings(elems, ", ", 0, NULL);
+
+  log_notice(LD_HEARTBEAT, "DoS mitigation since startup: %s.", msg);
+
+  tor_free(msg);
+  SMARTLIST_FOREACH(elems, char *, e, tor_free(e));
+  smartlist_free(elems);
 }
 
 /* Called when a new client connection has been established on the given
diff --git a/src/test/test_status.c b/src/test/test_status.c
index b938b86326..223aeffb3b 100644
--- a/src/test/test_status.c
+++ b/src/test/test_status.c
@@ -360,7 +360,9 @@ test_status_hb_not_in_consensus(void *arg)
                  "initiated 0 and received 0 v4 connections; "
                  "initiated 0 and received 0 v5 connections.\n");
   expect_log_msg("DoS mitigation since startup: 0 circuits killed with "
-                 "too many cells. [cc not enabled] [conn not enabled] "
+                 "too many cells, [DoSCircuitCreationEnabled disabled], "
+                 "[DoSConnectionEnabled disabled], "
+                 "[DoSRefuseSingleHopClientRendezvous disabled], "
                  "0 INTRODUCE2 rejected.\n");
   tt_int_op(mock_saved_log_n_entries(), OP_EQ, 6);