ipn/conffile: reject null services and endpoints in Services config files (#21647)

A JSON null for a service or for an endpoint target unmarshals as a nil
pointer, and loadConfigV0 dereferenced it without a check. This made
"tailscale serve set-config" panic. Return an error that names the
service or endpoint instead.

Fixes #21534

Signed-off-by: Brendan Creane <bcreane@gmail.com>

ipn/conffile/serveconf.go

@@ -261,6 +261,9 @@ func loadConfigV0(json []byte, forService string) (*ServicesConfigFile, error) {
 		}
 	}
 	for svcName, svc := range scf.Services {
+		if svc == nil {
+			return nil, fmt.Errorf("service %q: must not be null", svcName)
+		}
 		if forService == "" && svc.Version != "" {
 			return nil, errors.New("services cannot be versioned separately from config file")
 		}
@@ -274,6 +277,9 @@ func loadConfigV0(json []byte, forService string) (*ServicesConfigFile, error) {
 		foundTUN := false
 		foundNonTUN := false
 		for ppr, target := range svc.Endpoints {
+			if target == nil {
+				return nil, fmt.Errorf("service %q: endpoint %q: must not be null", svcName, ppr.String())
+			}
 			if target.Protocol == "TUN" {
 				if ppr.Proto != 0 || ppr.Ports != tailcfg.PortRangeAny {
 					return nil, fmt.Errorf("service %q: destination \"TUN\" can only be used with source \"*\"", svcName)

ipn/conffile/serveconf_test.go

@@ -6,6 +6,8 @@
 package conffile
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"tailscale.com/tailcfg"
@@ -106,3 +108,44 @@ func TestTargetUnixSocketRoundtrip(t *testing.T) {
 		})
 	}
 }
+
+func TestLoadServicesConfigNull(t *testing.T) {
+	tests := []struct {
+		name       string
+		forService string
+		config     string
+		wantErr    string
+	}{
+		{
+			name:    "null_service",
+			config:  `{"version":"0.0.1","services":{"svc:a":null}}`,
+			wantErr: `service "svc:a": must not be null`,
+		},
+		{
+			name:    "null_endpoint",
+			config:  `{"version":"0.0.1","services":{"svc:a":{"endpoints":{"tcp:443":null}}}}`,
+			wantErr: `service "svc:a": endpoint "tcp:443": must not be null`,
+		},
+		{
+			name:       "null_endpoint_for_service",
+			forService: "svc:a",
+			config:     `{"version":"0.0.1","endpoints":{"tcp:443":null}}`,
+			wantErr:    `service "svc:a": endpoint "tcp:443": must not be null`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			path := filepath.Join(t.TempDir(), "config.json")
+			if err := os.WriteFile(path, []byte(tt.config), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			_, err := LoadServicesConfig(path, tt.forService)
+			if err == nil {
+				t.Fatalf("LoadServicesConfig succeeded; want error %q", tt.wantErr)
+			}
+			if err.Error() != tt.wantErr {
+				t.Errorf("LoadServicesConfig error = %q; want %q", err, tt.wantErr)
+			}
+		})
+	}
+}