#17674 closed defect (fixed)
circuit_handle_first_hop doesn't respect ExtendAllowPrivateAddresses
Reported by: | teor | Owned by: | |
---|---|---|---|
Priority: | Very High | Milestone: | Tor: 0.2.8.x-final |
Component: | Core Tor/Tor | Version: | |
Severity: | Major | Keywords: | tor-hs, 027-backport, 026-backport, security, tor-dos, 2016-bug-retrospective |
Cc: | asn | Actual Points: | |
Parent ID: | #17178 | Points: | |
Reviewer: | Sponsor: |
Description
circuit_extend checks ExtendAllowPrivateAddresses, but by then it's too late, we've already connected in circuit_handle_first_hop.
This seems to be a DoS risk.
onionskin_answer handles local connections as a special case using channel_is_local, so we might actually be making some that serve some useful purpose. (What is that purpose?)
Do we really need to allow connections to our own address from ourselves?
It might be a good idea to refuse to build circuits to ourselves in circuit_handle_first_hop if ExtendAllowPrivateAddresses is 0, and then see what falls over. Unfortunately, this can't be tested using chutney.
Child Tickets
Change History (13)
comment:1 Changed 4 years ago by
Keywords: | 027-backport 026-backport security added |
---|
comment:2 Changed 4 years ago by
This is a general case of the bug reported in #8976.
From IRC:
teor we believe whatever address and port are sent to us in rendezvous protocol versions 2 & 3 dgoulet oh rly!? teor without checking the consensus dgoulet I vaguely remember being a feature of tor that is being able to exit at an address that is _not_ an exit teor So for HS, this means that a three-hop circuit can be made to an arbitrary address dgoulet (or not in consensus) teor For RSOS, this means that a one-hop circuit can be made to an arbitrary address In either case, there should be a check for a private address asn i thought this was fixed by robert at some point teor Facebook's logs suggest it has not been, and I can't see it in the code asn but i see how it's worse for RSOS teor Certainly Tor will refuse to send cells, but it will still connect I don't think we need that feature, unless we sometimes connect to ourselves I think Robert fixed it by refusing to send cells to extend to a private address Which doesn't handle the RSOS one-hop case, or any other case where Tor connects directly to a private address
comment:3 Changed 4 years ago by
Cc: | asn added |
---|
comment:4 follow-up: 6 Changed 4 years ago by
Status: | new → needs_review |
---|
Please see my branch first-hop-no-private at https://github.com/teor2345/tor.git
It modifies circuit_handle_first_hop to refuse any connections to a private address with a protocol error, unless ExtendAllowPrivateAddresses is set.
This should catch this issue with relay extends, and hidden service / RSOS rendezvous from badly behaved clients.
I can't test it on chutney, so I need help testing it on the live network. (Ugh.)
comment:6 follow-up: 7 Changed 4 years ago by
Status: | needs_review → needs_information |
---|
Replying to teor:
Please see my branch first-hop-no-private at https://github.com/teor2345/tor.git
It modifies circuit_handle_first_hop to refuse any connections to a private address with a protocol error, unless ExtendAllowPrivateAddresses is set.
This should catch this issue with relay extends, and hidden service / RSOS rendezvous from badly behaved clients.
I've played around with this. I've modified a tor client to change the RP extend info to be a IP/PORT I control (nc -l PORT basically). The result, and somehow expected is that the remote HS second middle when trying to extend to the RP does a TLS connection to my IP/PORT.
I've modified an HS I own to always pick the second middle to be a relay that I controlled (thus could look at the log). I then changed the RP info to 127.0.0.1. I see that my pinned relay is logging me:
Dec 07 17:31:56.000 [warn] Client asked me to extend to a private address
Which is denied by circuit_extend
. So maybe the use case I've tested is wrong but it doesn't seem we go through "handle_first_hop" when extending to the RP (for hidden service). I would be curious why RSOS doesn't also use circuit_extend
then?
(All this testing required me to change the HS, client and middle relay so I might have gone wrong).
comment:7 follow-up: 8 Changed 4 years ago by
Replying to dgoulet:
Replying to teor:
Please see my branch first-hop-no-private at https://github.com/teor2345/tor.git
It modifies circuit_handle_first_hop to refuse any connections to a private address with a protocol error, unless ExtendAllowPrivateAddresses is set.
This should catch this issue with relay extends, and hidden service / RSOS rendezvous from badly behaved clients.
I've played around with this. I've modified a tor client to change the RP extend info to be a IP/PORT I control (nc -l PORT basically). The result, and somehow expected is that the remote HS second middle when trying to extend to the RP does a TLS connection to my IP/PORT.
This is the behaviour the patch is intended to prevent - we don't want clients being able to convince HSs to make a TLS connection to anywhere on the Internet via their rendezvous second middle relay.
I've modified an HS I own to always pick the second middle to be a relay that I controlled (thus could look at the log). I then changed the RP info to 127.0.0.1. I see that my pinned relay is logging me:
Dec 07 17:31:56.000 [warn] Client asked me to extend to a private addressWhich is denied by
circuit_extend
. So maybe the use case I've tested is wrong but it doesn't seem we go through "handle_first_hop" when extending to the RP (for hidden service). I would be curious why RSOS doesn't also usecircuit_extend
then?
The current behaviour of HSs is to make a TLS connection from the second middle, then refuse to send the extend cell in "circuit_extend".
The first hop uses "handle_first_hop". Subsequent hops use "circuit_extend".
Since a RSOS only makes one-hop rendezvous circuits, we need to stop this in "handle_first_hop" as well.
(All this testing required me to change the HS, client and middle relay so I might have gone wrong).
It looks like you're on the right track.
comment:8 Changed 4 years ago by
Replying to teor:
This is the behaviour the patch is intended to prevent - we don't want clients being able to convince HSs to make a TLS connection to anywhere on the Internet via their rendezvous second middle relay.
Oops, I meant: "a TLS connection to the relay's private network".
comment:9 Changed 4 years ago by
Status: | needs_information → needs_review |
---|
This looks like it's working from dgoulet's testing.
comment:10 Changed 4 years ago by
yes, this seems okay to me. Merged it!
(Please either reparent or close the child ticket, then close this one?)
comment:11 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Reparented #8976, there's still work to do there to catch bad rendezvous points early, before we build a two-hop path for them.
comment:12 Changed 4 years ago by
Keywords: | tor-dos added; dos removed |
---|
Canonicalize dos tag to tor-dos
comment:13 Changed 4 years ago by
Keywords: | 2016-bug-retrospective added |
---|
Marking these tickets (based on severity and hand-review) for inclusion in 2016 bug retrospective
This is a DoS/security issue which could be backported to 0.2.6 and 0.2.7.