Opened 6 months ago

Closed 5 months ago

#25213 closed defect (implemented)

0.3.3.2-alpha - Non-fatal assertion !(node_awaiting_ipv6(get_options(), node)) failed.

Reported by: sjcjonker Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version: Tor: 0.3.3.2-alpha
Severity: Normal Keywords: ipv6, 033-must, regression, 033-triage-20180320, 033-included-20180320
Cc: teor Actual Points:
Parent ID: #20916 Points: 0.5
Reviewer: teor Sponsor:

Description

Running 2 tor relays, whilst restarting Tor going from 0.3.3.1-alpha-dev to 0.3.3.2-alpha one of the two nodes gave the following error for 3 other relays, being: Rea, fury and lowtideceviche

It's just running for 2 hours or so, the remaining functionality seems OK.

Feb 11 20:01:04 tornode1 Tor[23044]: tor_bug_occurred_(): Bug: ../src/or/policies.c:1008: fascist_firewall_choose_address_node: Non-fatal assertion !(node_awaiting_ipv6(get_options(), node)) failed. (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: Non-fatal assertion !(node_awaiting_ipv6(get_options(), node)) failed in fascist_firewall_choose_address_node at ../src/or/policies.c:1008. Stack trace: (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(log_backtrace+0x44) [0x55a3a4d5bde4] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(tor_bug_occurred_+0xb9) [0x55a3a4d77479] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(fascist_firewall_choose_address_node+0x182) [0x55a3a4c47682] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(extend_info_from_node+0x12f) [0x55a3a4caaa7f] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(+0xed980) [0x55a3a4cc0980] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(connection_ap_handshake_attach_circuit+0x261) [0x55a3a4cc0eb1] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(connection_ap_attach_pending+0x1a8) [0x55a3a4ce2808] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(do_main_loop+0x399) [0x55a3a4c278f9] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(tor_run_main+0x275) [0x55a3a4c28f25] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(tor_main+0x3a) [0x55a3a4c2236a] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(main+0x19) [0x55a3a4c220d9] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /lib/x86_64-linux-gnu/libc.so.6(libc_start_main+0xf1) [0x7f6eb6b052b1] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Bug: /usr/bin/tor(_start+0x2a) [0x55a3a4c2212a] (on Tor 0.3.3.2-alpha )
Feb 11 20:01:04 tornode1 Tor[23044]: Could not choose valid address for Rea
Feb 11 20:01:04 tornode1 Tor[23044]: Could not make a one-hop connection to $8DCB8A8CD726EBCE5CCC90581EB7A695D6B09904. Discarding this circuit.

Child Tickets

Change History (16)

comment:1 Changed 6 months ago by teor

Keywords: ipv6 032-backport-maybe added
Milestone: Tor: 0.3.3.x-final
Parent ID: #20916
Points: 0.5

This issue is probably related to one of the children of #20196, where we last changed the IPv6 consensus code.

comment:2 Changed 6 months ago by arma

The log message makes me think it's connecting asking for directory info (i.e. directory guard).

comment:3 Changed 6 months ago by nickm

Keywords: 033-maybe-must added

Mark some tickets as possibly belonging in 033-must.

comment:4 Changed 6 months ago by nickm

Keywords: 033-must added; 033-maybe-must removed

move 033-maybe-must into 033-must

comment:5 Changed 5 months ago by nickm

Keywords: regression added

comment:6 Changed 5 months ago by nickm

Keywords: 033-triage-20180320 added

Marking all tickets reached by current round of 033 triage.

comment:7 Changed 5 months ago by nickm

Keywords: 033-included-20180320 added

Mark 033-must tickets as triaged-in for 0.3.3

comment:8 Changed 5 months ago by nickm

Interesting. This is happening because extend_info_from_node() is calling fascist_firewall_choose_address_node() with a node that is node_awaiting_ipv6(). But earlier in extend_info_from_node(), it checks:

  if (node->ri == NULL && (node->rs == NULL || node->md == NULL))
    return NULL;

which suggests to me that the check above is inconsistent with node_awaiting_ipv6().

comment:9 Changed 5 months ago by nickm

So, this bug will fix itself once enough authorities upgrade to 0.3.3.1-alpha or later, since networkstatus_consensus_has_ipv6() implies that node_awaiting_ipv6() is never true. But maybe we don't want to wait so long.

I think the problem here is that node->ri is set, but we_use_microdescriptors_for_circuits() returns true and the node->md field is not set.

comment:10 Changed 5 months ago by nickm

b66b62fb7525cac1e18385e130249ce24f5c9fea is the problem here: it removes the node->ri check, but doesn't make a corresponding change to extend_info_from_node()

comment:11 Changed 5 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:12 Changed 5 months ago by nickm

Cc: teor added
Status: acceptedneeds_review

Please see branch bug25213_033 for a possible fix made by changing extend_info_from_node(). The alternative would be to change node_awaiting_ipv6(), but I'm not sure that's wise.

Teor, do you have time to take a look at this? It's small, and I think you know this logic well.

comment:13 Changed 5 months ago by dgoulet

Reviewer: teor

Assigning teor as Reviewer. @teor, if you have no time for this, just remove yourself as Reviewer and we'll triage it to someone else. No pressure, Thanks!

comment:14 Changed 5 months ago by teor

This looks like the correct fix.

(We should ignore routerinfos whenever we are using microdescriptors, so this change is correct. There's a lot of code that will need a similar fix eventually. Maybe some accessor functions would help? Or maybe we could just stop parsing descriptors when we use microdescriptors.)

comment:15 Changed 5 months ago by teor

Keywords: 032-backport-maybe removed
Status: needs_reviewmerge_ready

comment:16 Changed 5 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Okay; merging to 0.3.3 and forward. Thanks for the review!

Note: See TracTickets for help on using tickets.