ipn: [serve] warn that foreground funnel won't work if shields are up (#14685)

We throw error early with a warning if users attempt to enable background funnel
for a node that does not allow incoming connections
(shields up), but if it done in foreground mode, we just silently fail
(the funnel command succeeds, but the connections are not allowed).
This change makes sure that we also error early in foreground mode.

Updates tailscale/tailscale#11049

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

ipn/serve.go

@@ -63,12 +63,12 @@ type ServeConfig struct {
 	// traffic is allowed, from trusted ingress peers.
 	AllowFunnel map[HostPort]bool `json:",omitempty"`
 
-	// Foreground is a map of an IPN Bus session ID to an alternate foreground
-	// serve config that's valid for the life of that WatchIPNBus session ID.
-	// This. This allows the config to specify ephemeral configs that are
-	// used in the CLI's foreground mode to ensure ungraceful shutdowns
-	// of either the client or the LocalBackend does not expose ports
-	// that users are not aware of.
+	// Foreground is a map of an IPN Bus session ID to an alternate foreground serve config that's valid for the
+	// life of that WatchIPNBus session ID. This allows the config to specify ephemeral configs that are used
+	// in the CLI's foreground mode to ensure ungraceful shutdowns of either the client or the LocalBackend does not
+	// expose ports that users are not aware of. In practice this contains any serve config set via 'tailscale
+	// serve' command run without the '--bg' flag. ServeConfig contained by Foreground is not expected itself to contain
+	// another Foreground block.
 	Foreground map[string]*ServeConfig `json:",omitempty"`
 
 	// ETag is the checksum of the serve config that's populated
@@ -389,8 +389,7 @@ func (sc *ServeConfig) RemoveTCPForwarding(port uint16) {
 // View version of ServeConfig.IsFunnelOn.
 func (v ServeConfigView) IsFunnelOn() bool { return v.ж.IsFunnelOn() }
 
-// IsFunnelOn reports whether if ServeConfig is currently allowing funnel
-// traffic for any host:port.
+// IsFunnelOn reports whether any funnel endpoint is currently enabled for this node.
 func (sc *ServeConfig) IsFunnelOn() bool {
 	if sc == nil {
 		return false
@@ -400,6 +399,11 @@ func (sc *ServeConfig) IsFunnelOn() bool {
 			return true
 		}
 	}
+	for _, conf := range sc.Foreground {
+		if conf.IsFunnelOn() {
+			return true
+		}
+	}
 	return false
 }
 

ipn/serve_test.go

@@ -182,3 +182,88 @@ func TestExpandProxyTargetDev(t *testing.T) {
 		})
 	}
 }
+
+func TestIsFunnelOn(t *testing.T) {
+	tests := []struct {
+		name string
+		sc   *ServeConfig
+		want bool
+	}{
+		{
+			name: "nil_config",
+		},
+		{
+			name: "empty_config",
+			sc:   &ServeConfig{},
+		},
+		{
+			name: "funnel_enabled_in_background",
+			sc: &ServeConfig{
+				AllowFunnel: map[HostPort]bool{
+					"tailnet.xyz:443": true,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "funnel_disabled_in_background",
+			sc: &ServeConfig{
+				AllowFunnel: map[HostPort]bool{
+					"tailnet.xyz:443": false,
+				},
+			},
+		},
+		{
+			name: "funnel_enabled_in_foreground",
+			sc: &ServeConfig{
+				Foreground: map[string]*ServeConfig{
+					"abc123": {
+						AllowFunnel: map[HostPort]bool{
+							"tailnet.xyz:443": true,
+						},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "funnel_disabled_in_both",
+			sc: &ServeConfig{
+				AllowFunnel: map[HostPort]bool{
+					"tailnet.xyz:443": false,
+				},
+				Foreground: map[string]*ServeConfig{
+					"abc123": {
+						AllowFunnel: map[HostPort]bool{
+							"tailnet.xyz:8443": false,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "funnel_enabled_in_both",
+			sc: &ServeConfig{
+				AllowFunnel: map[HostPort]bool{
+					"tailnet.xyz:443": true,
+				},
+				Foreground: map[string]*ServeConfig{
+					"abc123": {
+						AllowFunnel: map[HostPort]bool{
+							"tailnet.xyz:8443": true,
+						},
+					},
+				},
+			},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.sc.IsFunnelOn(); got != tt.want {
+				t.Errorf("ServeConfig.IsFunnelOn() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}