Remotely triggerable circuit destruction by path bias code

TBB 3.5 - lot of Unable to connect errors TBB user reports:

Additional info:
 - I am on 3.5.1 now and have the same issues
 - it seems thing work fine if I use the default torrc file

What I change is to add about 20 MapAddress lines to block ad servers.

I use the same list on 2.3.25 and there it causes no problems.

changed milestone to %Tor: 0.2.4.x-final

added 2016-bug-retrospective component::core tor/tor milestone::Tor: 0.2.4.x-final priority::high regression resolution::fixed status::closed tor-client type::defect labels

Trac:
Summary: Tor 0.2.4.x breaks something if many MapAddress defined to Tor 0.2.4.x breaks something if some MapAddress defined

Are there instructions on how to reproduce this?

Trac:
Keywords: client regression deleted, tor-client regression added
Milestone: N/A to Tor: 0.2.4.x-final

Trac:
Status: new to needs_information

Bug reporter sent examples with Mapaddress. I'll wait for confirmation before posting full answer.

Trac:
Priority: normal to major
Summary: Tor 0.2.4.x breaks something if some MapAddress defined to Remotely triggerable circuit destruction by path biase code

Trac:
Summary: Remotely triggerable circuit destruction by path biase code to Remotely triggerable circuit destruction by path bias code

Client sends begin cell with something-trigger.

Exit relay process it and does: connection_exit_connect:

  log_debug(LD_EXIT,"about to try connecting");
  switch (connection_connect(conn, conn->address, addr, port, &socket_error)) {
    case -1: {
      int reason = errno_to_stream_end_reason(socket_error);
      connection_edge_end(edge_conn, reason);

errno_to_stream_end_reason:

    E_CASE(EBADF):
    E_CASE(EFAULT):
    E_CASE(EINVAL):
    S_CASE(EISCONN):
    S_CASE(ENOTSOCK):
    S_CASE(EPROTONOSUPPORT):
    S_CASE(EAFNOSUPPORT):
    E_CASE(EACCES):
    S_CASE(ENOTCONN):
    S_CASE(ENETUNREACH):
      return END_STREAM_REASON_INTERNAL;

Exit sent relay_end cell.

Client process this cell by connection_ap_process_end_not_open.

  if (rh->length > 0) {
    if (reason == END_STREAM_REASON_TORPROTOCOL ||
        reason == END_STREAM_REASON_INTERNAL ||
        reason == END_STREAM_REASON_DESTROY) {
      /* All three of these reasons could mean a failed tag
       * hit the exit and it complained. Do not probe.
       * Fail the circuit. */
      circ->path_state = PATH_STATE_USE_FAILED;
      log_warn(LD_APP,"Got '%d' end reason.", reason);
      return -END_CIRC_REASON_TORPROTOCOL;

Client destroys circuit, detaches all streams and close them.

First, it's bug for exit side to count some of those reasons as internal. (EHOSTUNREACH counted as END_STREAM_REASON_NOROUTE while ENETUNREACH is internal for some reason)

Second, it's bug in general to use stream end reason for path bias detection purpose. Without warns (except non informative "connection_edge_process_relay_cell (at origin) failed."), without chance to reattach not yet connected streams to new circuit, with destroying all already attached streams. With reasons that chose by exit under external pressing.

That was series of minor bugs, now it's solid remotely triggerable circuit destruction.

To be clear, it's circuit destruction that's triggerable by the exit node, right? But the exit node can already trigger circuit destruction by sending a DESTROY cell. The real problematic case is if the user can be tricked into sending something that causes an ENETUNREACH response from the exit node.

In any case, we should ENETUNREACH to give NOROUTE. There's a patch for that as "bug10777_noroute_024"

If a third party can trigger this, we need to remove the case END_STREAM_REASON_INTERNAL case from connection_ap_process_end_notopen, treating it as neither a path-bias success nor a path-bias failure. There's a patch for that as "bug10777_nointernal_024."

Mike, I am leaning towards merging both. Please let me know if this makes path bias useless.

Also, there's maybe a third bug: If the user triggered this by using MapAddress to map advertising networks to some netblock we should have recognized as private., that should probably have taken effect and caused the stream to get blocked connection to a private address before the RELAY_BEGIN cell is ever sent. (Was it a private network block, or something else?)

Trac:
Status: needs_information to needs_review
Cc: N/A to mikeperry

Path bias detection code that used stream end reasons is bug. Every stream end reason can be triggered by third party pressure.

Was it a private network block, or something else? It was broadcast address. You can't to block all broadcast addresses because client can't to know all of them for exit relay and x.x.x.255 not always means broadcast address.

And you no need broadcast address only to trigger this bug. If exit relay process ICMP codes you can to answer with need code for controllable address. Then it's not only about ENETUNREACH.

Replying to cypherpunks:

Path bias detection code that used stream end reasons is bug. Every stream end reason can be triggered by third party pressure.

Including TORPROTOCOL and DESTROY? How?

END_STREAM_REASON_DESTROY can't happens actually in wire, relay can't to send it. Every place it assigns end_reason to END_STREAM_REASON_DESTROY circuit already destroyed and CELL_DESTROY sent. It's never working checks of END_STREAM_REASON_DESTROY by client.

END_STREAM_REASON_TORPROTOCOL is non triggerable by 3rd party, it seems. At least I didn't found way to trigger relay to send it, (still looking in code though). Not sure it's usable for path bias detection code however. But that probably another bug.

Are there instructions on how to reproduce this?

Additional information from the bug reporter:

The problem also happens with unmodified torrc. It seems this way it
happens less often.

It is mostly entries like this, to block ad servers:

MapAddress ad-emea.doubleclick.net 255.255.255.255
MapAddress oglasi.slo-tech.com 255.255.255.255
MapAddress ads.poraba.com 255.255.255.255
MapAddress googleads.g.doubleclick.net 255.255.255.255
MapAddress pagead2.googlesyndication.com 255.255.255.255
MapAddress d2.zedo.com 255.255.255.255
MapAddress cdn2.adexprt.com 255.255.255.255
MapAddress d7.zedo.com 255.255.255.255
MapAddress cdn1.clkads.com 255.255.255.255
MapAddress cdn.adnxs.com 255.255.255.255
MapAddress tpc.googlesyndication.com 255.255.255.255
MapAddress ssl.google-analytics.com 255.255.255.255

Why EACCES is internal reason?

Why EACCES is internal reason.

You're right, it shouldn't be. Neither should EPERM:

       EACCES, EPERM
              The user tried to connect to a broadcast address without  having
              the  socket  broadcast  flag  enabled  or the connection request
              failed because of a local firewall rule.

I pushed an updated bug10777_netunreach_024. Any other cases?

The branches to review are still "bug10777_netunreach_024" and "bug10777_nointernal_024".

bug10777_netunreach_024 seems ok. However, I am not a fan of bug10777_nointernal_024.. This branch would prevent us from detecting that entire class of route manipulation (tagging the initial relay cells after a circuit was established).

Specifically, in circuit_receive_relay_cell(), we use END_STREAM_REASON_INTERNAL to mean that the relay cell was not recognized, which is exactly what we'd see for a simple XOR tag that was meant to be undone by a cooperating exit node, but hit an honest exit node instead.

Replying to mikeperry:

bug10777_netunreach_024 seems ok. However, I am not a fan of bug10777_nointernal_024.. This branch would prevent us from detecting that entire class of route manipulation (tagging the initial relay cells after a circuit was established).

Specifically, in circuit_receive_relay_cell(), we use END_STREAM_REASON_INTERNAL to mean that the relay cell was not recognized, which is exactly what we'd see for a simple XOR tag that was meant to be undone by a cooperating exit node, but hit an honest exit node instead.

Actually, no I'm wrong. We only return INTERNAL if the decryption itself fails, not for unrecognized. Still digging for other cases...