Opened 6 years ago

Closed 2 years ago

#11307 closed defect (wontfix)

connection_handle_event_cb() should handle orconns correctly even when not in OR_CONN_STATE_CONNECTING

Reported by: andrea Owned by: andrea
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version: Tor: 0.2.5.3-alpha
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This code is in connection_handle_event_cb():

if (conn->type == CONN_TYPE_OR &&
    conn->state == OR_CONN_STATE_CONNECTING) {
  connection_or_connect_failed(TO_OR_CONN(conn),
                               errno_to_orconn_end_reason(socket_error),
                               tor_socket_strerror(socket_error));
}

It should be something like this:

if (conn->type == CONN_TYPE_OR) {
  if (conn->state == OR_CONN_STATE_CONNECTING) {
    connection_or_connect_failed(TO_OR_CONN(conn),
                                errno_to_orconn_end_reason(socket_error),
                                tor_socket_strerror(socket_error));
  } else {
    connection_or_close_for_error(TO_OR_CONN(conn));
  }
}

As it stands, if conn->state != OR_CONN_STATE_CONNECTING this code will incorrectly treat orconns as generic conns and call connection_mark_for_close() on them without properly notifying the channel layer.

Note that since this code is specific to bufferevents which do not currently work, this bug cannot be demonstrated in any working build of Tor, so I'm assigning it to the 0.2.?? milestone.

Created pursuant to connection_mark_for_close() audit task #7472.

Child Tickets

Change History (6)

comment:1 Changed 6 years ago by andrea

Status: newneeds_review

Proposed fix in my bug11307 branch; ability to test is limited due to brokenness of bufferevents, which currently don't even build without --disable-unittests.

comment:2 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:3 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:4 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:5 Changed 2 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Severity: Normal

comment:6 Changed 2 years ago by nickm

Resolution: wontfix
Status: needs_reviewclosed

waaaait, this is bufferevents code.

Note: See TracTickets for help on using tickets.