Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#5342 closed defect (fixed)

Never use a bridge-purpose node as an exit

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Wanoskarnet found this one. We should be sure that a client can never use a node that's been marked as a bridge anywhere but in the first position of its circuits. Additionally, we should probably treat every bridge's exit policy as "reject *:*"

Child Tickets

Change History (10)

comment:1 Changed 8 years ago by nickm

Hm. What parts of the codebase need to change? I am guessing that choose_random_node() and choose_good_exit_server_general() should be sufficient.

This is going to be tricky to do portably between 0.2.2 and 0.2.3 thanks to the node_t changes. we might have to just figure out which functions to change, then do two versions of those changes.

comment:2 Changed 8 years ago by Sebastian

We could be fine doing this on 0.2.3 I think, as long as we make the #4343 patch available on 0.2.2. It'll fix this more specific problem as well, I believe?

comment:3 Changed 8 years ago by nickm

Status: newneeds_review

See branch "bug5342" in my public repo. Looks like choose_random_node() already *has* this fix via add_running_routers_to_smartlist or whatever it is called.

The backport to 0.2.2 will be trivial once this is reviewed and tested.

comment:4 Changed 8 years ago by Sebastian

Hrm, does this mean that we cannot choose nodes as exit that we injected vial a controller?

comment:5 Changed 8 years ago by Sebastian

(Otherwise, this looks trivially correct. I also agree that router_choose_random_node() doesn't need a change. But the same question I had above applies there, too)

comment:6 in reply to:  4 Changed 8 years ago by nickm

Replying to Sebastian:

Hrm, does this mean that we cannot choose nodes as exit that we injected vial a controller?

It means that we won't choose them automatically; only if we're explicitly told to use them. That's consistent with the regular behavior for controller-injected nodes.

comment:7 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Backportd, merged, merged forward.

comment:8 Changed 8 years ago by arma

For the record, I believe we were also allowing controller-supplied descriptors to get chosen as exits even if they are ROUTER_PURPOSE_CONTROLLER. I added that point to the changelog entry.

comment:9 Changed 7 years ago by nickm

Keywords: tor-client added

comment:10 Changed 7 years ago by nickm

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