Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4212 closed defect (fixed)

Circuit in CIRCUIT_PURPOSE_C_INTRODUCING with no rend_data

Reported by: rransom Owned by: rransom
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-hs
Cc: katmagic, Sebastian Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

With a somewhat recent Git master (commit 2725a88d5e3ec362a4f866f53c26ca20f943eb49):

Oct 09 19:13:41.000 [info] circuit_expire_building(): Abandoning circ [redacted] (state 3:open, purpose 6)
Oct 09 19:13:41.000 [info] internal (high-uptime) circ (length 4): [redacted](open) [redacted](open) [redacted](open) [redacted](open)
Oct 09 19:13:41.000 [err] _circuit_mark_for_close(): Bug: circuitlist.c:1222: _circuit_mark_for_close: Assertion ocirc->rend_data failed; aborting.

(CIRCUIT_PURPOSE_C_INTRODUCING == 6)

The assertion occurred in the following chunk of code, added to help fix #3825:

 } else if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCING &&
             reason != END_STREAM_REASON_TIMEOUT) {
    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
    tor_assert(ocirc->build_state->chosen_exit);
    tor_assert(ocirc->rend_data);
    log_info(LD_REND, "Failed intro circ %s to %s "
             "(building circuit to intro point). "
             "Marking intro point as possibly unreachable.",
             safe_str_client(ocirc->rend_data->onion_address),
           safe_str_client(build_state_get_exit_nickname(ocirc->build_state)));
    rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
                                           ocirc->rend_data,
                                           INTRO_POINT_FAILURE_UNREACHABLE);

But we should never have a circuit with that purpose with its rend_data field unset.

Child Tickets

Change History (10)

comment:1 Changed 8 years ago by rransom

Sebastian found that rend_client_reextend_intro_circuit launches a new circuit with purpose CIRCUIT_PURPOSE_C_INTRODUCING with no rend_data set. As far as I can tell, that's not the bug that caused this crash.

Also, the circuit created in that function can never be used by the HS code. connection_ap_handshake_attach_circuit will call circuit_get_open_circ_or_launch to obtain an intro circ for the HS, and will create a new intro circ with its rend_data field set properly.

comment:2 in reply to:  1 Changed 8 years ago by rransom

Owner: set to rransom
Status: newassigned

Replying to rransom:

Sebastian found that rend_client_reextend_intro_circuit launches a new circuit with purpose CIRCUIT_PURPOSE_C_INTRODUCING with no rend_data set. As far as I can tell, that's not the bug that caused this crash.

That is the bug that caused this crash.

comment:3 Changed 8 years ago by rransom

Status: assignedneeds_review

See bug4212-v2 ( https://git.torproject.org/rransom/tor.git bug4212-v2 ) for a fix. This patch should be applied to maint-0.2.2 and master .

comment:4 Changed 8 years ago by Sebastian

The patch should return -1 instead of 0, because we're in the failure case here and marked circ for close.

comment:5 in reply to:  4 Changed 8 years ago by rransom

Status: needs_reviewassigned

Replying to Sebastian:

The patch should return -1 instead of 0, because we're in the failure case here and marked circ for close.

No. The only caller of rend_client_reextend_intro_circuit whose behaviour is affected by its result interprets a non-zero result as indicating a permanent failure; this case isn't a permanent failure. rend_client_reextend_intro_circuit's documentation comment needs to be changed to reflect this.

This function's other caller only requires that it never return -2.

comment:6 Changed 8 years ago by rransom

Status: assignedneeds_review

I've pushed a fix for rend_client_reextend_intro_circuit's documentation comment.

comment:7 Changed 8 years ago by nickm

Code okay to me if it is tested and known to work. Dithering between merging to 0.2.2 immediately or merging to master and backporting if nothing breaks.

comment:8 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merging to 0.2.2 and master. Thanks!

comment:9 Changed 7 years ago by nickm

Keywords: tor-hs added

comment:10 Changed 7 years ago by nickm

Component: Tor Hidden ServicesTor
Note: See TracTickets for help on using tickets.