Opened 21 months ago

Closed 21 months ago

Last modified 16 months ago

#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 21 months ago by teor

Keywords: 027-backport 026-backport security added

This is a DoS/security issue which could be backported to 0.2.6 and 0.2.7.

comment:2 Changed 21 months ago by teor

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 21 months ago by asn

Cc: asn added

comment:4 Changed 21 months ago by teor

Status: newneeds_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:5 Changed 21 months ago by nickm

This does need testing, but it looks okay to me.

comment:6 in reply to:  4 ; Changed 21 months ago by dgoulet

Status: needs_reviewneeds_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 in reply to:  6 ; Changed 21 months ago by teor

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 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?

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 in reply to:  7 Changed 21 months ago by teor

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 21 months ago by teor

Status: needs_informationneeds_review

This looks like it's working from dgoulet's testing.

comment:10 Changed 21 months ago by nickm

yes, this seems okay to me. Merged it!

(Please either reparent or close the child ticket, then close this one?)

comment:11 Changed 21 months ago by teor

Resolution: fixed
Status: needs_reviewclosed

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 17 months ago by mikeperry

Keywords: tor-dos added; dos removed

Canonicalize dos tag to tor-dos

comment:13 Changed 16 months ago by nickm

Keywords: 2016-bug-retrospective added

Marking these tickets (based on severity and hand-review) for inclusion in 2016 bug retrospective

Note: See TracTickets for help on using tickets.