ipn/ipnlocal: move URL validation to LocalBackend

Updates tailscale/corp#7948

Signed-off-by: Maisem Ali <maisem@tailscale.com>

ipn/ipnlocal/local.go

@@ -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})
 	}
 }

ipn/ipnlocal/state_test.go

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