Opened 4 months ago

Last modified 4 weeks ago

#27100 assigned enhancement

report connection to PT SOCKS proxy separately from OR connection

Reported by: catalyst Owned by: ahf
Priority: Medium Milestone: Tor: 0.4.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 035-roadmap-subtask, 035-triaged-in-20180711, socks, pt, s8-bootstrap
Cc: catalyst, mcs, brade, intrigeri, rl1987 Actual Points:
Parent ID: #28018 Points:
Reviewer: Sponsor: Sponsor8

Description

Right now when acting as a PT client, we don't distinguish between connecting to the SOCKS port of a PT proxy and connecting to the OR that's behind the proxy. This means that we lose some intermediate progress reporting that can help users understand what might be going wrong.

Child Tickets

Change History (16)

comment:1 Changed 4 months ago by intrigeri

Cc: intrigeri added

comment:2 Changed 4 months ago by catalyst

It looks like we might be able to hook in near connection_or_finished_connecting() or connection_proxy_connect(). Right now we call control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0) near the top of connection_or_finished_connecting() before we have begun to talk to the proxy, if any.

comment:3 Changed 4 months ago by rl1987

Cc: rl1987 added

comment:4 Changed 4 months ago by rl1987

Keywords: socks pt added

comment:5 Changed 4 months ago by catalyst

Keywords: s8-bootstrap added
Sponsor: Sponsor8

comment:6 Changed 4 months ago by catalyst

Type: defectenhancement

comment:7 Changed 3 months ago by catalyst

Milestone: Tor: 0.3.5.x-finalTor: 0.3.6.x-final

Move my post-freeze 0.3.5 items to 0.3.6.

comment:8 Changed 7 weeks ago by dgoulet

Parent ID: #25502#28018

This doesn't seem to have anything to do with #25502 but rather a change to help with bootstrap status of *tor* which from the roadmap looks like #28018.

comment:9 Changed 7 weeks ago by catalyst

This depends on some work in #27167. Otherwise, we would have to add more special cases in control_event_bootstrap() to disambiguate first connection vs application circuit connection.

comment:10 in reply to:  description ; Changed 7 weeks ago by dcf

Replying to catalyst:

Right now when acting as a PT client, we don't distinguish between connecting to the SOCKS port of a PT proxy and connecting to the OR that's behind the proxy.

Not only PT proxies, but also plain Socks4Proxy/Socks5Proxy/HTTPProxy, if I'm not mistaken.

comment:11 in reply to:  10 Changed 6 weeks ago by catalyst

Replying to dcf:

Replying to catalyst:

Right now when acting as a PT client, we don't distinguish between connecting to the SOCKS port of a PT proxy and connecting to the OR that's behind the proxy.

Not only PT proxies, but also plain Socks4Proxy/Socks5Proxy/HTTPProxy, if I'm not mistaken.

Yeah, that code doesn't distinguish PT proxies from firewall bypass proxies. I think it's still an improvement to start tracking proxy connection status even if we don't disambiguate for now.

comment:12 Changed 6 weeks ago by ahf

Owner: changed from catalyst to ahf

This is slightly related to some of the stuff I'm already working on for S8. Taking over this one.

comment:13 Changed 6 weeks ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:14 Changed 4 weeks ago by mcs

In the context of #27239, I have been reviewing tickets related to the bootstrap status reporting improvements and thinking about how Tor Launcher / Tor Browser will be affected. For this ticket, is the resulting change likely to be the addition of a new bootstrap phase? Or something else?

comment:15 Changed 4 weeks ago by catalyst

I'm still looking at how best to do this.

It looks like connection_or_finished_connecting() is called when the TCP connect() succeeds, whether it's directly to a relay or to the TCP listener of a proxy (which could be a PT proxy or a firewall-bypass proxy). It looks like the call control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0) is like many other parts of the bootstrap reporting in that it reports an anticipated next step, even if there might be multiple intermediate unreported steps yet to happen.

It's probably better to report the (TLS?) handshaking when it actually starts to happen, which is probably down in connection_tls_start_handshake().

There already seems to be somewhat of a state machine in connection_or.h already, making connection_or_change_state() a potentially useful place to hook in.

comment:16 Changed 4 weeks ago by catalyst

Proxy connections have their own small state machine, with the state variable in the proxy_state field of a connection_t. Currently, connection.c changes that field directly. If we wrap those state changes in a function, that function could publish notifications that we could then use in bootstrap reporting. There aren't many states a proxy connection goes through during connection setup, but maybe it's still enough to improve the bootstrap UX.

In terms of being comprehensible to a non-expert user, we might want to distinguish between firewall-bypass proxies and PT proxies. (I'm still not completely sure how those compose together.)

Note: See TracTickets for help on using tickets.