Opened 4 months ago

Closed 4 months ago

#25223 closed defect (fixed)

dos: dos_new_client_conn: Non-fatal assertion !(entry == NULL) failed

Reported by: dgoulet Owned by: dgoulet
Priority: High Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-dos, tor-relay, 033-must
Cc: Actual Points:
Parent ID: #24902 Points:
Reviewer: nickm Sponsor:

Description

I just got this report from a relay operator who got this on tor-0.3.3.2-alpha but that relay is an obfs4 bridge! DoS mitigation is not suppose to be running on bridges for now...

Feb 12 18:14:55.000 [notice] Tor 0.3.3.2-alpha (git-7b1d356bdb76607d) opening log file.
Feb 12 18:47:09.000 [warn] tor_bug_occurred_(): Bug: ../src/or/dos.c:679: dos_new_client_conn: Non-fatal assertion !(entry == NULL) failed. (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug: Non-fatal assertion !(entry == NULL) failed in dos_new_client_conn at ../src/or/dos.c:679. Stack trace: (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(log_backtrace+0x44) [0x55e0314a7de4] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(tor_bug_occurred_+0xb9) [0x55e0314c3479] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(channel_do_open_actions+0x1de) [0x55e0313e933e] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(channel_change_state_open+0x2a) [0x55e0313e93ba] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(channel_tls_handle_state_change_on_orconn+0x102) [0x55e0313ee2f2] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(connection_or_set_state_open+0x22) [0x55e031437872] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(channel_tls_handle_cell+0x185) [0x55e0313ee535] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(+0x115a01) [0x55e031434a01] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(+0x10c52e) [0x55e03142b52e] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(+0x537ae) [0x55e0313727ae] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(event_base_loop+0x6a0) [0x7f6f4e6c55a0] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(do_main_loop+0x2bc) [0x55e03137381c] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(tor_run_main+0x275) [0x55e031374f25] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(tor_main+0x3a) [0x55e03136e36a] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(main+0x19) [0x55e03136e0d9] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1) [0x7f6f4cc9f2b1] (on Tor 0.3.3.2-alpha )
Feb 12 18:47:09.000 [warn] Bug:     /usr/bin/tor(_start+0x2a) [0x55e03136e12a] (on Tor 0.3.3.2-alpha )
Feb 12 18:59:37.000 [warn] Onion service connection to [scrubbed] failed (connection refused)

Child Tickets

Change History (5)

comment:1 Changed 4 months ago by dgoulet

Ok I figure it out I think.

Bridge aren't suppose to have DoS mitigation on because of this in config.c:

  if (public_server_mode(options)) {
    /* If we are configured as a relay, initialize the subsystem. Even on HUP,
     * this is safe to call as it will load data from the current options
     * or/and the consensus. */
    dos_init();

So we don't init the DoS mitigation when reading the options. However, when we get a new consensus, we do set_dos_parameters() which can enable/disable it. See dos_consensus_has_changed().

Basically, we need to gate the set_dos_parameters() for public relays. We call that in dos_init() and when the consensus changes.

comment:2 Changed 4 months ago by dgoulet

Keywords: 033-must added

Some ticket dgoulet thinks must go in 033.

comment:3 Changed 4 months ago by dgoulet

Parent ID: #24902
Status: assignedneeds_review

Branch: bug25223_029_01

Based on ticket24902_029_05 branch. Once ACK, I'll merge this into that parent branch for the 029 backport.

comment:4 Changed 4 months ago by nickm

Status: needs_reviewmerge_ready

lgtm

comment:5 Changed 4 months ago by dgoulet

Resolution: fixed
Reviewer: nickm
Status: merge_readyclosed

Merged into #24902.

Note: See TracTickets for help on using tickets.