ipn/ipnlocal: move URL validation to LocalBackend

Updates tailscale/corp#7948

Signed-off-by: Maisem Ali <maisem@tailscale.com>
pull/6350/head
Maisem Ali 2 years ago committed by Maisem Ali
parent 4caca8619e
commit 513780f4f8

@ -999,9 +999,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, readOnly bool
metricMapResponsePings.Add(1)
go c.answerPing(pr)
}
if !c.validPopBrowserURL(resp.PopBrowserURL) {
resp.PopBrowserURL = ""
}
if u := resp.PopBrowserURL; u != "" && u != sess.lastPopBrowserURL {
sess.lastPopBrowserURL = u
if c.popBrowser != nil {
@ -1705,28 +1702,6 @@ func (c *Direct) ReportHealthChange(sys health.Subsystem, sysErr error) {
res.Body.Close()
}
// validPopBrowserURL reports whether urlStr is a valid value for a
// control server to send in a MapResponse.PopUpBrowserURL field.
func (c *Direct) validPopBrowserURL(urlStr string) bool {
if urlStr == "" {
// Common case.
return true
}
u, err := url.Parse(urlStr)
if err != nil {
return false
}
switch u.Scheme {
case "https":
return true
case "http":
// If the control server is using plain HTTP (likely a dev server),
// then permit http://.
return strings.HasPrefix(c.serverURL, "http://")
}
return false
}
var (
metricMapRequestsActive = clientmetric.NewGauge("controlclient_map_requests_active")

@ -1726,8 +1726,34 @@ func (b *LocalBackend) popBrowserAuthNow() {
}
}
// validPopBrowserURL reports whether urlStr is a valid value for a
// control server to send in a *URL field.
// b.mu must *not* be held.
func (b *LocalBackend) validPopBrowserURL(urlStr string) bool {
if urlStr == "" {
// Common case.
return true
}
u, err := url.Parse(urlStr)
if err != nil {
return false
}
switch u.Scheme {
case "https":
return true
case "http":
b.mu.Lock()
serverURL := b.serverURL
b.mu.Unlock()
// If the control server is using plain HTTP (likely a dev server),
// then permit http://.
return strings.HasPrefix(serverURL, "http://")
}
return false
}
func (b *LocalBackend) tellClientToBrowseToURL(url string) {
if url != "" {
if url != "" && b.validPopBrowserURL(url) {
b.send(ipn.Notify{BrowseToURL: &url})
}
}

@ -404,7 +404,7 @@ func TestStateMachine(t *testing.T) {
// the user needs to visit a login URL.
t.Logf("\n\nLogin (url response)")
notifies.expect(1)
url1 := "http://localhost:1/1"
url1 := "https://localhost:1/1"
cc.send(nil, url1, false, nil)
{
cc.assertCalls()
@ -453,7 +453,7 @@ func TestStateMachine(t *testing.T) {
// Provide a new interactive login URL.
t.Logf("\n\nLogin2 (url response)")
notifies.expect(1)
url2 := "http://localhost:1/2"
url2 := "https://localhost:1/2"
cc.send(nil, url2, false, nil)
{
cc.assertCalls()
@ -811,7 +811,7 @@ func TestStateMachine(t *testing.T) {
t.Logf("\n\nLoginDifferent")
notifies.expect(1)
b.StartLoginInteractive()
url3 := "http://localhost:1/3"
url3 := "https://localhost:1/3"
cc.send(nil, url3, false, nil)
{
nn := notifies.drain(1)

Loading…
Cancel
Save