ipn/ipnlocal/serve: error when PeerCaps serialisation fails

Also consolidates variable and header naming and amends the
CLI behavior
* multiple app-caps have to be specified as comma-separated
  list
* simple regex-based validation of app capability names is
  carried out during flag parsing

Signed-off-by: Gesa Stupperich <gesa@tailscale.com>

cmd/tailscale/cli/serve_v2.go

@@ -20,6 +20,7 @@ import (
 	"os/signal"
 	"path"
 	"path/filepath"
+	"regexp"
 	"slices"
 	"sort"
 	"strconv"
@@ -100,12 +101,25 @@ type acceptAppCapsFlag struct {
 	Value *[]tailcfg.PeerCapability
 }
 
+// An application capability name has the form {domain}/{name}.
+// Both parts must use the (simplified) FQDN label character set.
+// The "name" can contain forward slashes.
+// \pL = Unicode Letter, \pN = Unicode Number, - = Hyphen
+var validAppCap = regexp.MustCompile(`^([\pL\pN-]+\.)+[\pL\pN-]+\/[\pL\pN-/]+$`)
+
 // Set appends s to the list of appCaps to accept.
 func (u *acceptAppCapsFlag) Set(s string) error {
 	if s == "" {
 		return nil
 	}
-	*u.Value = append(*u.Value, tailcfg.PeerCapability(s))
+	appCaps := strings.Split(s, ",")
+	for _, appCap := range appCaps {
+		appCap = strings.TrimSpace(appCap)
+		if !validAppCap.MatchString(appCap) {
+			return fmt.Errorf("%q does not match the form {domain}/{name}, where domain must be a fully qualified domain name", s)
+		}
+		*u.Value = append(*u.Value, tailcfg.PeerCapability(appCap))
+	}
 	return nil
 }
 
@@ -221,7 +235,7 @@ func newServeV2Command(e *serveEnv, subcmd serveMode) *ffcli.Command {
 			fs.UintVar(&e.https, "https", 0, "Expose an HTTPS server at the specified port (default mode)")
 			if subcmd == serve {
 				fs.UintVar(&e.http, "http", 0, "Expose an HTTP server at the specified port")
-				fs.Var(&acceptAppCapsFlag{Value: &e.acceptAppCaps}, "accept-app-caps", "App capability to forward to the server (can be specified multiple times)")
+				fs.Var(&acceptAppCapsFlag{Value: &e.acceptAppCaps}, "accept-app-caps", "App capabilities to forward to the server (specify multiple capabilities with a comma-separated list)")
 			}
 			fs.UintVar(&e.tcp, "tcp", 0, "Expose a TCP forwarder to forward raw TCP packets at the specified port")
 			fs.UintVar(&e.tlsTerminatedTCP, "tls-terminated-tcp", 0, "Expose a TCP forwarder to forward TLS-terminated TCP packets at the specified port")

cmd/tailscale/cli/serve_v2_test.go

@@ -875,7 +875,7 @@ func TestServeDevConfigMutations(t *testing.T) {
 					},
 				},
 				{
-					command: cmd("serve --bg --accept-app-caps=example.com/cap/foo --accept-app-caps=example.com/cap/bar 3000"),
+					command: cmd("serve --bg --accept-app-caps=example.com/cap/foo,example.com/cap/bar 3000"),
 					want: &ipn.ServeConfig{
 						TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}},
 						Web: map[ipn.HostPort]*ipn.WebServerConfig{
@@ -904,6 +904,15 @@ func TestServeDevConfigMutations(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "invalid_accept_caps_invalid_app_cap",
+			steps: []step{
+				{
+					command: cmd("serve --bg --accept-app-caps=example/cap/foo 3000"), // should be {domain.tld}/{name}
+					wantErr: anyErr(),
+				},
+			},
+		},
 	}
 
 	for _, group := range groups {
@@ -1220,6 +1229,108 @@ func TestSrcTypeFromFlags(t *testing.T) {
 	}
 }
 
+func TestAcceptSetAppCapsFlag(t *testing.T) {
+	testCases := []struct {
+		name          string
+		inputs        []string
+		expectErr     bool
+		expectedValue []tailcfg.PeerCapability
+	}{
+		{
+			name:          "valid_simple",
+			inputs:        []string{"example.com/name"},
+			expectErr:     false,
+			expectedValue: []tailcfg.PeerCapability{"example.com/name"},
+		},
+		{
+			name:          "valid_unicode",
+			inputs:        []string{"bücher.de/something"},
+			expectErr:     false,
+			expectedValue: []tailcfg.PeerCapability{"bücher.de/something"},
+		},
+		{
+			name:          "more_valid_unicode",
+			inputs:        []string{"example.tw/某某某"},
+			expectErr:     false,
+			expectedValue: []tailcfg.PeerCapability{"example.tw/某某某"},
+		},
+		{
+			name:          "valid_path_slashes",
+			inputs:        []string{"domain.com/path/to/name"},
+			expectErr:     false,
+			expectedValue: []tailcfg.PeerCapability{"domain.com/path/to/name"},
+		},
+		{
+			name:          "valid_multiple_sets",
+			inputs:        []string{"one.com/foo", "two.com/bar"},
+			expectErr:     false,
+			expectedValue: []tailcfg.PeerCapability{"one.com/foo", "two.com/bar"},
+		},
+		{
+			name:          "valid_empty_string",
+			inputs:        []string{""},
+			expectErr:     false,
+			expectedValue: nil, // Empty string should be a no-op and not append anything.
+		},
+		{
+			name:          "invalid_path_chars",
+			inputs:        []string{"domain.com/path_with_underscore"},
+			expectErr:     true,
+			expectedValue: nil, // Slice should remain empty.
+		},
+		{
+			name:          "valid_subdomain",
+			inputs:        []string{"sub.domain.com/name"},
+			expectErr:     false,
+			expectedValue: []tailcfg.PeerCapability{"sub.domain.com/name"},
+		},
+		{
+			name:          "invalid_no_path",
+			inputs:        []string{"domain.com/"},
+			expectErr:     true,
+			expectedValue: nil,
+		},
+		{
+			name:          "invalid_no_domain",
+			inputs:        []string{"/path/only"},
+			expectErr:     true,
+			expectedValue: nil,
+		},
+		{
+			name:          "some_invalid_some_valid",
+			inputs:        []string{"one.com/foo", "bad/bar", "two.com/baz"},
+			expectErr:     true,
+			expectedValue: []tailcfg.PeerCapability{"one.com/foo"}, // Parsing will stop after first error
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var v []tailcfg.PeerCapability
+			flag := &acceptAppCapsFlag{Value: &v}
+
+			var err error
+			for _, s := range tc.inputs {
+				err = flag.Set(s)
+				if err != nil {
+					break
+				}
+			}
+
+			if tc.expectErr && err == nil {
+				t.Errorf("expected an error, but got none")
+			}
+			if !tc.expectErr && err != nil {
+				t.Errorf("did not expect an error, but got: %v", err)
+			}
+
+			if !reflect.DeepEqual(tc.expectedValue, v) {
+				t.Errorf("unexpected value, got: %q, want: %q", v, tc.expectedValue)
+			}
+		})
+	}
+}
+
 func TestCleanURLPath(t *testing.T) {
 	tests := []struct {
 		input    string

ipn/ipnlocal/serve.go

@@ -81,7 +81,8 @@ type serveHTTPContext struct {
 
 	// provides funnel-specific context, nil if not funneled
 	Funnel *funnelFlow
-	PeerCapsFilter views.Slice[tailcfg.PeerCapability]
+	// AppCapabilities lists all PeerCapabilities that should be forwarded by serve
+	AppCapabilities views.Slice[tailcfg.PeerCapability]
 }
 
 // funnelFlow represents a funneled connection initiated via IngressPeer
@@ -805,10 +806,11 @@ func (rp *reverseProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		r.Out.Host = r.In.Host
 		addProxyForwardedHeaders(r)
 		rp.lb.addTailscaleIdentityHeaders(r)
-		rp.lb.addTailscaleGrantHeader(r)
+		if err := rp.lb.addAppCapabilitiesHeader(r); err != nil {
-	}}
+			http.Error(w, err.Error(), http.StatusInternalServerError)
-
+			return
-	// There is no way to autodetect h2c as per RFC 9113
+		}
+	}} // There is no way to autodetect h2c as per RFC 9113
 	// https://datatracker.ietf.org/doc/html/rfc9113#name-starting-http-2.
 	// However, we assume that http:// proxy prefix in combination with the
 	// protoccol being HTTP/2 is sufficient to detect h2c for our needs. Only use this for
@@ -930,24 +932,25 @@ func encTailscaleHeaderValue(v string) string {
 	return mime.QEncoding.Encode("utf-8", v)
 }
 
-func (b *LocalBackend) addTailscaleGrantHeader(r *httputil.ProxyRequest) {
+func (b *LocalBackend) addAppCapabilitiesHeader(r *httputil.ProxyRequest) error {
-	r.Out.Header.Del("Tailscale-App-Capabilities")
+	const appCapabilitiesHeaderName = "Tailscale-App-Capabilities"
+	r.Out.Header.Del(appCapabilitiesHeaderName)
 
 	c, ok := serveHTTPContextKey.ValueOk(r.Out.Context())
 	if !ok || c.Funnel != nil {
-		return
+		return nil
 	}
-	filter := c.PeerCapsFilter
+	acceptCaps := c.AppCapabilities
-	if filter.IsNil() {
+	if acceptCaps.IsNil() {
-		return
+		return nil
 	}
 	peerCaps := b.PeerCaps(c.SrcAddr.Addr())
 	if peerCaps == nil {
-		return
+		return nil
 	}
 
-	peerCapsFiltered := make(map[tailcfg.PeerCapability][]tailcfg.RawMessage, filter.Len())
+	peerCapsFiltered := make(map[tailcfg.PeerCapability][]tailcfg.RawMessage, acceptCaps.Len())
-	for _, cap := range filter.AsSlice() {
+	for _, cap := range acceptCaps.AsSlice() {
 		if peerCaps.HasCapability(cap) {
 			peerCapsFiltered[cap] = peerCaps[cap]
 		}
@@ -956,10 +959,11 @@ func (b *LocalBackend) addTailscaleGrantHeader(r *httputil.ProxyRequest) {
 	peerCapsSerialized, err := json.Marshal(peerCapsFiltered)
 	if err != nil {
 		b.logf("serve: failed to serialize filtered PeerCapMap: %v", err)
-		return
+		return fmt.Errorf("unable to process app capabilities")
 	}
 
-	r.Out.Header.Set("Tailscale-App-Capabilities", encTailscaleHeaderValue(string(peerCapsSerialized)))
+	r.Out.Header.Set(appCapabilitiesHeaderName, encTailscaleHeaderValue(string(peerCapsSerialized)))
+	return nil
 }
 
 // serveWebHandler is an http.HandlerFunc that maps incoming requests to the
@@ -990,7 +994,7 @@ func (b *LocalBackend) serveWebHandler(w http.ResponseWriter, r *http.Request) {
 		if !ok {
 			return
 		}
-		c.PeerCapsFilter = h.AcceptAppCaps()
+		c.AppCapabilities = h.AcceptAppCaps()
 		h := p.(http.Handler)
 		// Trim the mount point from the URL path before proxying. (#6571)
 		if r.URL.Path != "/" {