circuit_handle_first_hop doesn't respect ExtendAllowPrivateAddresses

[teor]

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.

[teor]

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

[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

[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 can't test it on chutney, so I need help testing it on the live network. (Ugh.)

[nickm]

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

[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.

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).

[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.

[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".

[teor]

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

[nickm]

yes, this seems okay to me. Merged it!

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

[teor]

Reparented #8976, there's still work to do there to catch bad rendezvous points early, before we build a two-hop path for them.

[mikeperry]

Canonicalize dos tag to tor-dos

[nickm]

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