Skip to content

Commit 5ff23cb

Browse files
bradfitzmaisem
bradfitz
authored and
maisem
committed
control/controlhttp: start port 443 fallback sooner if 80's stuck
Fixes tailscale#4544 Change-Id: I39877e71915ad48c6668351c45cd8e33e2f5dbae Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> (cherry picked from commit e38d3df)
1 parent 497fab5 commit 5ff23cb

File tree

1 file changed

+82
-31
lines changed

1 file changed

+82
-31
lines changed

‎control/controlhttp/client.go

Lines changed: 82 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"net/http"
3131
"net/http/httptrace"
3232
"net/url"
33+
"time"
3334

3435
"tailscale.com/control/controlbase"
3536
"tailscale.com/net/dnscache"
@@ -98,48 +99,98 @@ type dialParams struct {
9899
}
99100

100101
func (a *dialParams) dial() (*controlbase.Conn, error) {
101-
init, cont, err := controlbase.ClientDeferred(a.machineKey, a.controlKey, a.version)
102-
if err != nil {
103-
return nil, err
104-
}
102+
// Create one shared context used by both port 80 and port 443 dials.
103+
// If port 80 is still in flight when 443 returns, this deferred cancel
104+
// will stop the port 80 dial.
105+
ctx, cancel := context.WithCancel(a.ctx)
106+
defer cancel()
105107

106-
u := &url.URL{
108+
// u80 and u443 are the URLs we'll try to hit over HTTP or HTTPS,
109+
// respectively, in order to do the HTTP upgrade to a net.Conn over which
110+
// we'll speak Noise.
111+
u80 := &url.URL{
107112
Scheme: "http",
108113
Host: net.JoinHostPort(a.host, a.httpPort),
109114
Path: serverUpgradePath,
110115
}
111-
conn, httpErr := a.tryURL(u, init)
112-
if httpErr == nil {
113-
ret, err := cont(a.ctx, conn)
114-
if err != nil {
115-
conn.Close()
116-
return nil, err
117-
}
118-
return ret, nil
116+
u443 := &url.URL{
117+
Scheme: "https",
118+
Host: net.JoinHostPort(a.host, a.httpsPort),
119+
Path: serverUpgradePath,
119120
}
120-
121-
// Connecting over plain HTTP failed, assume it's an HTTP proxy
122-
// being difficult and see if we can get through over HTTPS.
123-
u.Scheme = "https"
124-
u.Host = net.JoinHostPort(a.host, a.httpsPort)
125-
init, cont, err = controlbase.ClientDeferred(a.machineKey, a.controlKey, a.version)
126-
if err != nil {
127-
return nil, err
121+
type tryURLRes struct {
122+
u *url.URL
123+
conn net.Conn
124+
cont controlbase.HandshakeContinuation
125+
err error
128126
}
129-
conn, tlsErr := a.tryURL(u, init)
130-
if tlsErr == nil {
131-
ret, err := cont(a.ctx, conn)
132-
if err != nil {
133-
conn.Close()
134-
return nil, err
127+
ch := make(chan tryURLRes) // must be unbuffered
128+
129+
try := func(u *url.URL) {
130+
res := tryURLRes{u: u}
131+
var init []byte
132+
init, res.cont, res.err = controlbase.ClientDeferred(a.machineKey, a.controlKey, a.version)
133+
if res.err == nil {
134+
res.conn, res.err = a.tryURL(ctx, u, init)
135+
}
136+
select {
137+
case ch <- res:
138+
case <-ctx.Done():
139+
if res.conn != nil {
140+
res.conn.Close()
141+
}
135142
}
136-
return ret, nil
137143
}
138144

139-
return nil, fmt.Errorf("all connection attempts failed (HTTP: %v, HTTPS: %v)", httpErr, tlsErr)
145+
// Start the plaintext HTTP attempt first.
146+
go try(u80)
147+
148+
// In case outbound port 80 blocked or MITM'ed poorly, start a backup timer
149+
// to dial port 443 if port 80 doesn't either succeed or fail quickly.
150+
try443Timer := time.AfterFunc(500*time.Millisecond, func() { try(u443) })
151+
defer try443Timer.Stop()
152+
153+
var err80, err443 error
154+
for {
155+
select {
156+
case <-ctx.Done():
157+
return nil, fmt.Errorf("connection attempts aborted by context: %w", ctx.Err())
158+
case res := <-ch:
159+
if res.err == nil {
160+
ret, err := res.cont(ctx, res.conn)
161+
if err != nil {
162+
res.conn.Close()
163+
return nil, err
164+
}
165+
return ret, nil
166+
}
167+
switch res.u {
168+
case u80:
169+
// Connecting over plain HTTP failed; assume it's an HTTP proxy
170+
// being difficult and see if we can get through over HTTPS.
171+
err80 = res.err
172+
// Stop the fallback timer and run it immediately. We don't use
173+
// Timer.Reset(0) here because on AfterFuncs, that can run it
174+
// again.
175+
if try443Timer.Stop() {
176+
go try(u443)
177+
} // else we lost the race and it started already which is what we want
178+
case u443:
179+
err443 = res.err
180+
default:
181+
panic("invalid")
182+
}
183+
if err80 != nil && err443 != nil {
184+
return nil, fmt.Errorf("all connection attempts failed (HTTP: %v, HTTPS: %v)", err80, err443)
185+
}
186+
}
187+
}
140188
}
141189

142-
func (a *dialParams) tryURL(u *url.URL, init []byte) (net.Conn, error) {
190+
// tryURL connects to u, and tries to upgrade it to a net.Conn.
191+
//
192+
// Only the provided ctx is used, not a.ctx.
193+
func (a *dialParams) tryURL(ctx context.Context, u *url.URL, init []byte) (net.Conn, error) {
143194
dns := &dnscache.Resolver{
144195
Forward: dnscache.Get().Forward,
145196
LookupIPFallback: dnsfallback.Lookup,
@@ -189,7 +240,7 @@ func (a *dialParams) tryURL(u *url.URL, init []byte) (net.Conn, error) {
189240
connCh <- info.Conn
190241
},
191242
}
192-
ctx := httptrace.WithClientTrace(a.ctx, &trace)
243+
ctx = httptrace.WithClientTrace(ctx, &trace)
193244
req := &http.Request{
194245
Method: "POST",
195246
URL: u,

0 commit comments

Comments
 (0)