Opened 11 days ago

Last modified 10 days ago

#31111 needs_review defect

negotiate one machine per index

Reported by: pulls Owned by:
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version: Tor: 0.4.1.3-alpha
Severity: Normal Keywords:
Cc: asn, mikeperry, gaba Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

As part of circuitpadding.c, in circpad_add_matching_machines(), the macros FOR_EACH_CIRCUIT_MACHINE_BEGIN and SMARTLIST_FOREACH_REVERSE_BEGIN currently expand to a for loop each. Below a slightly cropped (...) version of circpad_add_matching_machines():

circpad_add_matching_machines(origin_circuit_t *on_circ,
                              smartlist_t *machines_sl)
{
  ...
  FOR_EACH_CIRCUIT_MACHINE_BEGIN(i) {
    ...
    SMARTLIST_FOREACH_REVERSE_BEGIN(machines_sl,
                                    circpad_machine_spec_t *,
                                    machine) {
        ...
        if (circpad_negotiate_padding(on_circ, machine->machine_num,
                                  machine->target_hopnum,
                                  CIRCPAD_COMMAND_START) < 0) {
          log_info(LD_CIRC, "Padding not negotiated. Cleaning machine");
          circpad_circuit_machineinfo_free_idx(circ, i);
          circ->padding_machine[i] = NULL;
          on_circ->padding_negotiation_failed = 1;
        } else {
          /* Success. Don't try any more machines */
          return;
        }
      }
    } SMARTLIST_FOREACH_END(machine);
  } FOR_EACH_CIRCUIT_MACHINE_END;
}

The outer loop goes over each machine index (currently 2, set by CIRCPAD_MAX_MACHINES), while the inner loop looks for a suitable machine for that index to negotiate. As soon as one is found and negotiated, currently, the function returns without looking for a machine for later indices in the outer loop. The return should be replaced by a break to continue looking for a machine for the next index.

See https://github.com/torproject/tor/pull/1168 for a PR.

Child Tickets

Change History (3)

comment:1 Changed 11 days ago by teor

Status: newneeds_review

comment:2 Changed 11 days ago by nickm

Milestone: Tor: 0.4.2.x-final

comment:3 Changed 10 days ago by asn

I'd like for mike to take a look here. Do we want to add multiple machines in one circuit?

Note: See TracTickets for help on using tickets.