[tor-bugs] #26368 [Core Tor/Tor]: Consider circuit isolation when closing redundant intro points

[Tor Bug Tracker & Wiki]

#26368: Consider circuit isolation when closing redundant intro points
-------------------------------------------------+-------------------------
 Reporter:  sysrqb                               |          Owner:  neel
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  High                                 |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-hs, tor-client, 035-roadmap-     |  Actual Points:
  proposed, tbb-needs                            |
Parent ID:                                       |         Points:
 Reviewer:  dgoulet                              |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by neel):

 Copied from GitHub: When I check if a `oc` has at least one isolation
 flag, should I check for *any* isolation flag, or a specific one?

 If it is the former, I am thinking about something like this:

 {{{
 diff --git a/src/feature/rend/rendclient.c b/src/feature/rend/rendclient.c
 index 10b67ceda..dc2d33281 100644
 --- a/src/feature/rend/rendclient.c
 +++ b/src/feature/rend/rendclient.c
 @@ -361,10 +361,21 @@ rend_client_close_other_intros(const uint8_t
 *rend_pk_digest)
        origin_circuit_t *oc = TO_ORIGIN_CIRCUIT(c);
        if (oc->rend_data &&
            rend_circuit_pk_digest_eq(oc, rend_pk_digest)) {
 -        log_info(LD_REND|LD_CIRC, "Closing introduction circuit %d that
 we "
 -                 "built in parallel (Purpose %d).",
 oc->global_identifier,
 -                 c->purpose);
 -        circuit_mark_for_close(c, END_CIRC_REASON_IP_NOW_REDUNDANT);
 +        int has_isolation_flag = 0;
 +        for (edge_connection_t *oc_stream = oc->p_streams; oc_stream !=
 NULL;
 +             oc_stream = oc_stream->next_stream) {
 +          if (EDGE_TO_ENTRY_CONN(oc_stream)->entry_cfg.isolation_flags) {
 +            has_isolation_flag = 1;
 +            break;
 +          }
 +        }
 +
 +        if (!has_isolation_flag) {
 +          log_info(LD_REND|LD_CIRC, "Closing introduction circuit %d that
 we "
 +                   "built in parallel (Purpose %d).",
 oc->global_identifier,
 +                   c->purpose);
 +          circuit_mark_for_close(c, END_CIRC_REASON_IP_NOW_REDUNDANT);
 +        }
        }
      }
    }
 }}}

 The code in this comment seems much simpler than the code in the branch.
 Would that be okay? If so, the above code will replace the code in this
 branch.

 Also if the above code is okay, will we still need a unit test?

 If it is the latter, which flag should I check for?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/26368#comment:20>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online