From 3c97ab3c24ba4a133377c7ec6ec89cc6903ffb2e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 17 Sep 2019 09:09:36 -0400 Subject: [PATCH 1/2] Treat an unexpected constant-sized VERSIONS cell as a PROTOCOL_WARN. We previously used tor_fragile_assert() to declare that this case could not happen: VERSIONS cells are always supposed to be variable-sized, right? This is incorrect, though. On a v1 link protocol connection, all cells are fixed-sized. There aren't supposed to be any VERSIONS cells with this version of the protocol, but apparently, somebody was messing up. (The v1 link protocol is obsolete, so probably the implementer responsible didn't mean to be using it.) Fixes bug 31107. Bugfix on 0.2.4.4-alpha, when we introduced a tor_fragile_assert() for this case. --- changes/bug31107 | 4 ++++ src/or/channeltls.c | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 changes/bug31107 diff --git a/changes/bug31107 b/changes/bug31107 new file mode 100644 index 0000000000..9652927c30 --- /dev/null +++ b/changes/bug31107 @@ -0,0 +1,4 @@ + o Minor bugfixes (logging, protocol violations): + - Do not log a nonfatal assertion failure when receiving a VERSIONS + cell on a connection using the obsolete v1 link protocol. Log a + protocol_warn instead. Fixes bug 31107; bugfix on 0.2.4.4-alpha. diff --git a/src/or/channeltls.c b/src/or/channeltls.c index d44f719138..6f4e413dc6 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1098,7 +1098,15 @@ channel_tls_handle_cell(cell_t *cell, or_connection_t *conn) /* do nothing */ break; case CELL_VERSIONS: - tor_fragile_assert(); + /* A VERSIONS cell should always be a variable-length cell, and + * so should never reach this function (which handles constant-sized + * cells). But if the connection is using the (obsolete) v1 link + * protocol, all cells will be treated as constant-sized, and so + * it's possible we'll reach this code. + */ + log_fn(LOG_PROTOCOL_WARN, LD_CHANNEL, + "Received unexpected VERSIONS cell on a channel using link " + "protocol %d; ignoring.", conn->link_proto); break; case CELL_NETINFO: ++stats_n_netinfo_cells_processed; From f0e4120996d3a96252200ea5302dcde3af6e0bc0 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 15:38:33 -0400 Subject: [PATCH 2/2] Add a rate-limit to our warning about the disabled .exit notation This warning would previously be given every time we tried to open a connection to a foo.exit address, which could potentially be used to flood the logs. Now, we don't allow this warning to appear more than once every 15 minutes. Fixes bug 31466; bugfix on 0.2.2.1-alpha, when .exit was first deprecated. --- changes/ticket31466 | 5 +++++ src/or/connection_edge.c | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) create mode 100644 changes/ticket31466 diff --git a/changes/ticket31466 b/changes/ticket31466 new file mode 100644 index 0000000000..e535b4502e --- /dev/null +++ b/changes/ticket31466 @@ -0,0 +1,5 @@ + o Minor bugfixes (logging): + - Rate-limit our the logging message about the obsolete .exit notation. + Previously, there was no limit on this warning, which could potentially + be triggered many times by a hostile website. Fixes bug 31466; + bugfix on 0.2.2.1-alpha. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 7a97c632d1..5638d9a1be 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1186,9 +1186,11 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, * disallowed when they're coming straight from the client, but you're * allowed to have them in MapAddress commands and so forth. */ if (!strcmpend(socks->address, ".exit") && !options->AllowDotExit) { - log_warn(LD_APP, "The \".exit\" notation is disabled in Tor due to " - "security risks. Set AllowDotExit in your torrc to enable " - "it (at your own risk)."); + static ratelim_t exit_warning_limit = RATELIM_INIT(60*15); + log_fn_ratelim(&exit_warning_limit, LOG_WARN, LD_APP, + "The \".exit\" notation is disabled in Tor due to " + "security risks. Set AllowDotExit in your torrc to enable " + "it (at your own risk)."); control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", escaped(socks->address)); out->end_reason = END_STREAM_REASON_TORPROTOCOL;