From f69435f4e5ea3fe95882aeef3532cf28dc984c3d Mon Sep 17 00:00:00 2001 From: Brendan Creane Date: Wed, 10 Jun 2026 13:50:25 -0700 Subject: [PATCH] ipn/conffile,cmd/tailscale/cli: add serve front-end protocol to config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The services config file Target stored a single protocol, so get-config recorded only the back-end scheme and silently downgraded HTTPS/TLS- terminated listeners on re-apply. Target now records Front and Backend separately — shorthand string when the back-end is the front-end's default, {front,backend} object otherwise — and rejects incoherent front/back pairs at parse time. Fixes #19724 Fixes #18381 Signed-off-by: Brendan Creane --- cmd/tailscale/cli/serve_v2.go | 146 +++++++--- cmd/tailscale/cli/serve_v2_test.go | 409 ++++++++++++++++++++++++++++- ipn/conffile/serveconf.go | 403 +++++++++++++++++++++------- ipn/conffile/serveconf_test.go | 279 ++++++++++++++++++++ 4 files changed, 1115 insertions(+), 122 deletions(-) create mode 100644 ipn/conffile/serveconf_test.go diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index 5b1676ed2..efc25f228 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -686,7 +686,7 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er if serviceConfig.Tun { mak.Set(&sdf.Endpoints, &tailcfg.ProtoPortRange{Ports: tailcfg.PortRangeAny}, &conffile.Target{ - Protocol: conffile.ProtoTUN, + Front: conffile.ProtoTUN, Destination: "", DestinationPorts: tailcfg.PortRange{}, }) @@ -696,11 +696,11 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er sniName := fmt.Sprintf("%s.%s", svcName.WithoutPrefix(), magicDNSSuffix) ppr := tailcfg.ProtoPortRange{Proto: int(ipproto.TCP), Ports: tailcfg.PortRange{First: port, Last: port}} if config.TCPForward != "" { - var proto conffile.ServiceProtocol + var front conffile.ServiceProtocol if config.TerminateTLS != "" { - proto = conffile.ProtoTLSTerminatedTCP + front = conffile.ProtoTLSTerminatedTCP } else { - proto = conffile.ProtoTCP + front = conffile.ProtoTCP } destHost, destPortStr, err := net.SplitHostPort(config.TCPForward) if err != nil { @@ -711,7 +711,8 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er return nil, fmt.Errorf("parse port %q: %w", destPortStr, err) } mak.Set(&sdf.Endpoints, &ppr, &conffile.Target{ - Protocol: proto, + Front: front, + Backend: conffile.ProtoTCP, Destination: destHost, DestinationPorts: tailcfg.PortRange{First: uint16(destPort), Last: uint16(destPort)}, }) @@ -725,20 +726,58 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er if !ok { return nil, fmt.Errorf("service %q: root handler not set", svcName) } + // The config file format maps a single endpoint per port, so it + // can only represent the root ("/") mount. Fail loudly rather + // than silently dropping additional mounts on export. + if len(handlers.Handlers) > 1 { + var mounts []string + for m := range handlers.Handlers { + if m != "/" { + mounts = append(mounts, m) + } + } + sort.Strings(mounts) + return nil, fmt.Errorf("service %q: port %d serves mount points (%s) that the config file format cannot represent; only the root \"/\" mount is supported", svcName, port, strings.Join(mounts, ", ")) + } + // Front is the listener protocol that tailscaled terminates, + // derived from the TCP port handler. It is independent of the + // backend proxy scheme (Backend); recording only the backend + // scheme dropped the HTTPS listener (see #18381). + front := conffile.ProtoHTTP + if config.HTTPS { + front = conffile.ProtoHTTPS + } if defaultHandler.Path != "" { mak.Set(&sdf.Endpoints, &ppr, &conffile.Target{ - Protocol: conffile.ProtoFile, + Front: front, + Backend: conffile.ProtoFile, Destination: defaultHandler.Path, DestinationPorts: tailcfg.PortRange{}, }) } else if defaultHandler.Proxy != "" { - proto, rest, ok := strings.Cut(defaultHandler.Proxy, "://") + proxy := defaultHandler.Proxy + // The config file format can only express http/https/ + // https+insecure web back-ends with an explicit host:port. + // Unix sockets (stored as "unix:", no "://"), portless + // remote hosts, and any other scheme have no faithful + // representation, so fail loudly rather than emitting a + // config that would not round-trip (or that MarshalJSON + // would later reject). + if strings.HasPrefix(proxy, "unix:") { + return nil, fmt.Errorf("service %q: unix socket proxy targets cannot be represented in the config file format", svcName) + } + backend, rest, ok := strings.Cut(proxy, "://") if !ok { - return nil, fmt.Errorf("service %q: invalid proxy handler %q", svcName, defaultHandler.Proxy) + return nil, fmt.Errorf("service %q: proxy handler %q is not representable in the config file format", svcName, proxy) + } + switch conffile.ServiceProtocol(backend) { + case conffile.ProtoHTTP, conffile.ProtoHTTPS, conffile.ProtoHTTPSInsecure: + default: + return nil, fmt.Errorf("service %q: proxy scheme %q is not representable in the config file format", svcName, backend) } host, portStr, err := net.SplitHostPort(rest) if err != nil { - return nil, fmt.Errorf("service %q: invalid proxy handler %q: %w", svcName, defaultHandler.Proxy, err) + return nil, fmt.Errorf("service %q: proxy handler %q is not representable in the config file format (need host:port): %w", svcName, proxy, err) } port, err := strconv.ParseUint(portStr, 10, 16) @@ -747,10 +786,24 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er } mak.Set(&sdf.Endpoints, &ppr, &conffile.Target{ - Protocol: conffile.ServiceProtocol(proto), + Front: front, + Backend: conffile.ServiceProtocol(backend), Destination: host, DestinationPorts: tailcfg.PortRange{First: uint16(port), Last: uint16(port)}, }) + } else { + // The config file format can only express path (file) and + // proxy targets. A Text or Redirect root handler has no + // representable endpoint, so fail loudly rather than + // silently dropping it on export. + switch { + case defaultHandler.Text != "": + return nil, fmt.Errorf("service %q: text handlers cannot be represented in the config file format", svcName) + case defaultHandler.Redirect != "": + return nil, fmt.Errorf("service %q: redirect handlers cannot be represented in the config file format", svcName) + default: + return nil, fmt.Errorf("service %q: root handler has no path or proxy target to export", svcName) + } } } } @@ -759,45 +812,65 @@ func (e *serveEnv) runServeGetConfig(ctx context.Context, args []string) (err er } var j []byte + // skippedErr is set (for --all) when one or more services could not be + // represented; the partial config is still written, but the command exits + // nonzero so the gap is not silently ignored by a backup script. + var skippedErr error if e.allServices && forSingleService { return errors.New("cannot specify both --all and --service") } else if e.allServices { var scf conffile.ServicesConfigFile - scf.Version = "0.0.1" + var skipped []string for svcName, serviceConfig := range sc.Services { sdf, err := handleService(svcName, serviceConfig) if err != nil { - return err + // For a bulk export, skip the unrepresentable service and warn + // rather than aborting the whole backup; the nonzero exit below + // signals that the result is incomplete. + fmt.Fprintf(e.stderr(), "Warning: skipping service %s: %v\n", svcName, err) + skipped = append(skipped, svcName.String()) + continue } mak.Set(&scf.Services, svcName, sdf) } + // The format changed meaning in 0.0.2 (see conffile), so every export is + // stamped 0.0.2; an older client then rejects it with a clear version + // error instead of misreading it. + scf.Version = conffile.ConfigVersionV2 j, err = json.MarshalIndent(scf, "", " ") if err != nil { return err } + if len(skipped) > 0 { + sort.Strings(skipped) + skippedErr = fmt.Errorf("%d service(s) could not be represented and were skipped: %s", len(skipped), strings.Join(skipped, ", ")) + } } else if forSingleService { serviceConfig, ok := sc.Services[e.service] if !ok { - j = []byte("{}") - } else { - sdf, err := handleService(e.service, serviceConfig) - if err != nil { - return err - } - sdf.Version = "0.0.1" - j, err = json.MarshalIndent(sdf, "", " ") - if err != nil { - return err - } + // Emitting "{}" here produces a file that set-config later rejects + // for lacking a "version" field; fail with a clear message instead. + return fmt.Errorf("service %q is not configured on this node", e.service) + } + sdf, err := handleService(e.service, serviceConfig) + if err != nil { + return err + } + sdf.Version = conffile.ConfigVersionV2 + j, err = json.MarshalIndent(sdf, "", " ") + if err != nil { + return err } } else { return errors.New("must specify either --service=svc: or --all") } j = append(j, '\n') - _, err = e.stdout().Write(j) - return err + if _, err = e.stdout().Write(j); err != nil { + return err + } + return skippedErr } // serveConfigDocsURL documents the Services configuration file format that set-config prefers @@ -848,10 +921,13 @@ func (e *serveEnv) runServeSetConfig(ctx context.Context, args []string) (err er if forSingleService { forService = e.service.String() } - scf, err := conffile.LoadServicesConfig(filename, forService) + scf, warnings, err := conffile.LoadServicesConfig(filename, forService) if err != nil { return fmt.Errorf("could not read config from file %q: %w", filename, err) } + for _, w := range warnings { + fmt.Fprintf(e.stderr(), "Warning: %s\n", w) + } st, err := e.getLocalClientStatusWithoutPeers(ctx) if err != nil { @@ -900,7 +976,7 @@ func (e *serveEnv) runServeSetConfig(ctx context.Context, args []string) (err er // scf.Services is nil for the legacy format, making this loop a no-op then. for name, details := range scf.Services { for ppr, ep := range details.Endpoints { - if ep.Protocol == conffile.ProtoTUN { + if ep.Front == conffile.ProtoTUN { err := e.setServe(sc, name.String(), serveTypeTUN, 0, "", "", false, magicDNSSuffix, nil, 0 /* proxy protocol */) if err != nil { return err @@ -912,20 +988,28 @@ func (e *serveEnv) runServeSetConfig(ctx context.Context, args []string) (err er if ppr.Proto != int(ipproto.TCP) { return fmt.Errorf("service %q: source ports must be TCP", name) } - serveType, _ := serveTypeFromConfString(ep.Protocol) + // The front-end (listener) protocol selects the serve type; + // the back-end protocol is preserved in the target URL passed to + // setServe so a TLS listener can still proxy to a plain-HTTP backend + // (and vice versa). See #18381. + serveType, ok := serveTypeFromConfString(ep.Front) + if !ok { + return fmt.Errorf("service %q: unsupported front-end protocol %q", name, ep.Front) + } for port := ppr.Ports.First; port <= ppr.Ports.Last; port++ { var target string - if ep.Protocol == conffile.ProtoFile { + if ep.Backend == conffile.ProtoFile { target = ep.Destination } else { // map source port range 1-1 to destination port range destPort := ep.DestinationPorts.First + (port - ppr.Ports.First) portStr := fmt.Sprint(destPort) - target = fmt.Sprintf("%s://%s", ep.Protocol, net.JoinHostPort(ep.Destination, portStr)) + target = fmt.Sprintf("%s://%s", ep.Backend, net.JoinHostPort(ep.Destination, portStr)) } err := e.setServe(sc, name.String(), serveType, port, "/", target, false, magicDNSSuffix, nil, 0 /* proxy protocol */) if err != nil { - return fmt.Errorf("service %q: %w", name, err) + return fmt.Errorf("service %q port %d (front %s, backend %s -> %s): %w", + name, port, ep.Front, ep.Backend, target, err) } } } diff --git a/cmd/tailscale/cli/serve_v2_test.go b/cmd/tailscale/cli/serve_v2_test.go index 994f443b6..cf314632f 100644 --- a/cmd/tailscale/cli/serve_v2_test.go +++ b/cmd/tailscale/cli/serve_v2_test.go @@ -21,6 +21,7 @@ "github.com/google/go-cmp/cmp" "github.com/peterbourgon/ff/v3/ffcli" "tailscale.com/ipn" + "tailscale.com/ipn/conffile" "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" "tailscale.com/types/views" @@ -2538,7 +2539,7 @@ func TestRunServeSetConfig(t *testing.T) { lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} var stdout, stderr bytes.Buffer e := &serveEnv{lc: lc, allServices: true, testStdout: &stdout, testStderr: &stderr} - path := writeTmpServeConfig(t, `{"version":"0.0.1","services":{"svc:foo":{"endpoints":{"tcp:443":"https://localhost:8000"}}}}`) + path := writeTmpServeConfig(t, `{"version":"0.0.2","services":{"svc:foo":{"endpoints":{"tcp:443":"https://localhost:8000"}}}}`) if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { t.Fatal(err) @@ -2561,7 +2562,7 @@ func TestRunServeSetConfig(t *testing.T) { lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}} var stdout, stderr bytes.Buffer e := &serveEnv{lc: lc, service: fooSvc, testStdout: &stdout, testStderr: &stderr} - path := writeTmpServeConfig(t, `{"version":"0.0.1","endpoints":{"tcp:443":"https://localhost:8000"}}`) + path := writeTmpServeConfig(t, `{"version":"0.0.2","endpoints":{"tcp:443":"https://localhost:8000"}}`) if err := e.runServeSetConfig(context.Background(), []string{path}); err != nil { t.Fatal(err) @@ -2574,3 +2575,407 @@ func TestRunServeSetConfig(t *testing.T) { } }) } + +// TestServeGetConfigMissingService verifies that get-config for a single +// service that isn't configured on the node fails with a clear error rather +// than emitting "{}", which set-config would later reject for lacking a +// "version" field. +func TestServeGetConfigMissingService(t *testing.T) { + lc := &fakeLocalServeClient{ + config: &ipn.ServeConfig{}, + prefs: &ipn.Prefs{}, + } + var stdout bytes.Buffer + e := &serveEnv{lc: lc, service: tailcfg.ServiceName("svc:missing"), testStdout: &stdout} + + err := e.runServeGetConfig(context.Background(), nil) + if err == nil { + t.Fatalf("runServeGetConfig returned nil error; want error. stdout=%q", stdout.String()) + } + if !strings.Contains(err.Error(), "svc:missing") { + t.Errorf("error = %q; want it to name the missing service", err) + } + if stdout.Len() != 0 { + t.Errorf("expected no stdout output on error; got %q", stdout.String()) + } +} + +// TestServeConfigRoundTripFrontProto verifies that get-config / set-config +// preserve the front-end (listener) protocol of a Service web handler. A +// service fronted by HTTPS that proxies to a plain-HTTP backend must round-trip +// as HTTPS rather than collapsing to HTTP. Regression test for #18381. +func TestServeConfigRoundTripFrontProto(t *testing.T) { + const svc = tailcfg.ServiceName("svc:awesome") + const sni = "awesome.test.ts.net" + + tests := []struct { + name string + https bool // front-end is HTTPS (else HTTP) + port uint16 // service port + proxy string // backend proxy stored in the handler + wantContains []string // substrings expected in the exported endpoint JSON + wantVersion string // expected "version" in the exported config + }{ + { + // The #18381 case: HTTPS listener, plain-HTTP backend. Exported as + // the shorthand string with the front-end scheme; stays version V1. + name: "https-front-http-backend", + https: true, + port: 443, + proxy: "http://127.0.0.1:30123", + wantContains: []string{`"tcp:443": "https://127.0.0.1:30123"`}, + wantVersion: "0.0.2", // always V2 now + }, + { + name: "http-front-http-backend", + https: false, + port: 80, + proxy: "http://127.0.0.1:30123", + wantContains: []string{`"tcp:80": "http://127.0.0.1:30123"`}, + wantVersion: "0.0.2", // always V2 now + }, + { + // HTTPS listener proxying to a TLS backend: the backend scheme + // differs from the front-end default, so it round-trips via the + // object form rather than being downgraded to plain HTTP. The object + // form bumps the file to version V2. + name: "https-front-https-backend", + https: true, + port: 443, + proxy: "https://127.0.0.1:8443", + wantContains: []string{`"front": "https"`, `"backend": "https://127.0.0.1:8443"`}, + wantVersion: "0.0.2", + }, + { + // HTTPS listener proxying to a TLS backend with an untrusted cert. + // This is a shorthand, so it stays version V1. + name: "https-front-https-insecure-backend", + https: true, + port: 443, + proxy: "https+insecure://127.0.0.1:8443", + wantContains: []string{`"tcp:443": "https+insecure://127.0.0.1:8443"`}, + wantVersion: "0.0.2", // always V2 now + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + webKey := ipn.HostPort(fmt.Sprintf("%s:%d", sni, tt.port)) + initial := &ipn.ServeConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + svc: { + TCP: map[uint16]*ipn.TCPPortHandler{ + tt.port: {HTTP: !tt.https, HTTPS: tt.https}, + }, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + webKey: {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: tt.proxy}, + }}, + }, + }, + }, + } + + lc := &fakeLocalServeClient{ + config: initial, + prefs: &ipn.Prefs{AdvertiseServices: []string{svc.String()}}, + } + + // Export with get-config --all. + var stdout bytes.Buffer + getEnv := &serveEnv{lc: lc, allServices: true, testStdout: &stdout} + if err := getEnv.runServeGetConfig(context.Background(), nil); err != nil { + t.Fatalf("runServeGetConfig: %v", err) + } + got := stdout.String() + for _, want := range tt.wantContains { + if !strings.Contains(got, want) { + t.Fatalf("exported config = %s\nwant substring %q", got, want) + } + } + if wantVer := fmt.Sprintf("%q: %q", "version", tt.wantVersion); !strings.Contains(got, wantVer) { + t.Fatalf("exported config = %s\nwant %s", got, wantVer) + } + + // Round-trip it back through set-config --all. + dir := t.TempDir() + file := filepath.Join(dir, "services.json") + if err := os.WriteFile(file, stdout.Bytes(), 0600); err != nil { + t.Fatal(err) + } + // SetServeConfig replaces lc.config; start from a clean slate so the + // resulting config reflects only the import. + lc.config = &ipn.ServeConfig{} + setEnv := &serveEnv{lc: lc, allServices: true} + if err := setEnv.runServeSetConfig(context.Background(), []string{file}); err != nil { + t.Fatalf("runServeSetConfig: %v", err) + } + + svcCfg := lc.config.Services[svc] + if svcCfg == nil { + t.Fatalf("service %q missing after import; config=%+v", svc, lc.config) + } + th := svcCfg.TCP[tt.port] + if th == nil { + t.Fatalf("no TCP handler on port %d after import; config=%+v", tt.port, svcCfg) + } + if th.HTTPS != tt.https || th.HTTP != !tt.https { + t.Errorf("after round-trip port %d: HTTP=%v HTTPS=%v; want HTTP=%v HTTPS=%v", + tt.port, th.HTTP, th.HTTPS, !tt.https, tt.https) + } + // The backend proxy must round-trip losslessly, preserving its + // scheme (http / https / https+insecure). + if h := svcCfg.Web[webKey].Handlers["/"]; h.Proxy != tt.proxy { + t.Errorf("after round-trip: backend proxy = %q; want %q", h.Proxy, tt.proxy) + } + }) + } +} + +// TestServeTypeFromConfString locks the front-end-protocol -> serveType mapping +// that runServeSetConfig relies on. The ok=false cases cover the branch that +// rejects an unmapped front protocol (serve_v2.go) instead of silently applying +// the zero serveType. Every protocol accepted by conffile.validFront must map +// here with ok=true, or that "unsupported front-end protocol" guard would +// reject a config the parser considers valid. +func TestServeTypeFromConfString(t *testing.T) { + tests := []struct { + in conffile.ServiceProtocol + want serveType + wantOk bool + }{ + {conffile.ProtoHTTP, serveTypeHTTP, true}, + {conffile.ProtoHTTPS, serveTypeHTTPS, true}, + {conffile.ProtoHTTPSInsecure, serveTypeHTTPS, true}, + {conffile.ProtoFile, serveTypeHTTPS, true}, + {conffile.ProtoTCP, serveTypeTCP, true}, + {conffile.ProtoTLSTerminatedTCP, serveTypeTLSTerminatedTCP, true}, + {conffile.ProtoTUN, serveTypeTUN, true}, + {conffile.ServiceProtocol("bogus"), -1, false}, + {conffile.ServiceProtocol(""), -1, false}, + } + for _, tt := range tests { + got, ok := serveTypeFromConfString(tt.in) + if got != tt.want || ok != tt.wantOk { + t.Errorf("serveTypeFromConfString(%q) = (%v, %v); want (%v, %v)", + tt.in, got, ok, tt.want, tt.wantOk) + } + } +} + +// TestServeSetConfigRejectsBadFront verifies that a Service endpoint with an +// unknown front-end protocol is rejected rather than silently misapplied. The +// object form is parsed by conffile, so the rejection currently happens at +// parse time; the explicit serveTypeFromConfString guard in runServeSetConfig +// is defense-in-depth for that same invariant. +func TestServeSetConfigRejectsBadFront(t *testing.T) { + const config = `{ + "version": "0.0.1", + "services": { + "svc:awesome": { + "endpoints": { + "tcp:443": {"front": "bogus", "backend": "http://127.0.0.1:80"} + } + } + } + }` + dir := t.TempDir() + file := filepath.Join(dir, "services.json") + if err := os.WriteFile(file, []byte(config), 0600); err != nil { + t.Fatal(err) + } + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}, prefs: &ipn.Prefs{}} + setEnv := &serveEnv{lc: lc, allServices: true} + err := setEnv.runServeSetConfig(context.Background(), []string{file}) + if err == nil { + t.Fatal("runServeSetConfig succeeded; want error for bogus front protocol") + } + if !strings.Contains(err.Error(), "bogus") { + t.Errorf("error = %v; want it to mention the bad protocol %q", err, "bogus") + } +} + +// TestServeSetConfigV2Object verifies that set-config applies a version "0.0.2" +// file written in the object form, preserving a TLS back-end behind an HTTPS +// listener (the combination the shorthand string cannot express). +func TestServeSetConfigV2Object(t *testing.T) { + const config = `{ + "version": "0.0.2", + "services": { + "svc:awesome": { + "endpoints": { + "tcp:443": {"front": "https", "backend": "https://127.0.0.1:8443"} + } + } + } + }` + file := filepath.Join(t.TempDir(), "services.json") + if err := os.WriteFile(file, []byte(config), 0600); err != nil { + t.Fatal(err) + } + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}, prefs: &ipn.Prefs{}} + setEnv := &serveEnv{lc: lc, allServices: true} + if err := setEnv.runServeSetConfig(context.Background(), []string{file}); err != nil { + t.Fatalf("runServeSetConfig: %v", err) + } + svcCfg := lc.config.Services["svc:awesome"] + if svcCfg == nil { + t.Fatalf("service not applied; config=%+v", lc.config) + } + if th := svcCfg.TCP[443]; th == nil || !th.HTTPS { + t.Errorf("port 443: want HTTPS listener; got %+v", th) + } + if h := svcCfg.Web[ipn.HostPort("awesome.test.ts.net:443")].Handlers["/"]; h.Proxy != "https://127.0.0.1:8443" { + t.Errorf("backend proxy = %q; want %q", h.Proxy, "https://127.0.0.1:8443") + } +} + +// TestServeGetConfigRejectsNonRootMount verifies that get-config fails loudly +// when a service has handlers on a mount other than "/", which the per-port +// config file format cannot represent, rather than silently dropping them. +func TestServeGetConfigRejectsNonRootMount(t *testing.T) { + const svc = tailcfg.ServiceName("svc:awesome") + cfg := &ipn.ServeConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + svc: { + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "awesome.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + "/api": {Proxy: "http://127.0.0.1:3001"}, + }}, + }, + }, + }, + } + lc := &fakeLocalServeClient{config: cfg, prefs: &ipn.Prefs{AdvertiseServices: []string{svc.String()}}} + var stdout bytes.Buffer + e := &serveEnv{lc: lc, service: svc, testStdout: &stdout} + err := e.runServeGetConfig(context.Background(), nil) + if err == nil { + t.Fatalf("runServeGetConfig succeeded; want error for non-root mount. stdout=%q", stdout.String()) + } + if !strings.Contains(err.Error(), "/api") { + t.Errorf("error = %v; want it to name the /api mount", err) + } +} + +// TestServeGetConfigAllSkipsUnrepresentable verifies that get-config --all skips +// a service it can't represent (warning to stderr, nonzero exit) while still +// exporting the others, so one bad service doesn't block backing up the rest. +func TestServeGetConfigAllSkipsUnrepresentable(t *testing.T) { + good := tailcfg.ServiceName("svc:good") + bad := tailcfg.ServiceName("svc:bad") + cfg := &ipn.ServeConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + good: { + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "good.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + }}, + }, + }, + bad: { // non-root mount -> unrepresentable + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "bad.test.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + "/api": {Proxy: "http://127.0.0.1:3001"}, + }}, + }, + }, + }, + } + lc := &fakeLocalServeClient{config: cfg, prefs: &ipn.Prefs{AdvertiseServices: []string{good.String(), bad.String()}}} + var stdout, stderr bytes.Buffer + e := &serveEnv{lc: lc, allServices: true, testStdout: &stdout, testStderr: &stderr} + err := e.runServeGetConfig(context.Background(), nil) + if err == nil { + t.Fatal("runServeGetConfig --all succeeded; want nonzero exit when a service is skipped") + } + if out := stdout.String(); !strings.Contains(out, "svc:good") || strings.Contains(out, "svc:bad") { + t.Errorf("exported config = %s\nwant it to include svc:good and omit svc:bad", out) + } + if !strings.Contains(stderr.String(), "svc:bad") { + t.Errorf("stderr = %q; want a warning naming the skipped svc:bad", stderr.String()) + } +} + +// TestServeSetConfigWarnsOnLegacyHTTPS verifies that reading an "https://" +// shorthand from a 0.0.1 file preserves its legacy TLS-back-end meaning and +// warns, rather than silently switching it to a plain-HTTP back-end. +func TestServeSetConfigWarnsOnLegacyHTTPS(t *testing.T) { + const config = `{"version":"0.0.1","services":{"svc:web":{"endpoints":{"tcp:443":"https://127.0.0.1:8443"}}}}` + file := filepath.Join(t.TempDir(), "services.json") + if err := os.WriteFile(file, []byte(config), 0600); err != nil { + t.Fatal(err) + } + lc := &fakeLocalServeClient{config: &ipn.ServeConfig{}, prefs: &ipn.Prefs{}} + var stderr bytes.Buffer + e := &serveEnv{lc: lc, allServices: true, testStderr: &stderr} + if err := e.runServeSetConfig(context.Background(), []string{file}); err != nil { + t.Fatalf("runServeSetConfig: %v", err) + } + if !strings.Contains(stderr.String(), "https://") { + t.Errorf("stderr = %q; want a migration warning about the https:// shorthand", stderr.String()) + } + svcCfg := lc.config.Services["svc:web"] + if svcCfg == nil { + t.Fatalf("service not applied; config=%+v", lc.config) + } + if h := svcCfg.Web[ipn.HostPort("web.test.ts.net:443")].Handlers["/"]; h.Proxy != "http://127.0.0.1:8443" { + t.Errorf("backend proxy = %q; want %q (0.0.1 https:// read with the fixed plain-HTTP meaning)", h.Proxy, "http://127.0.0.1:8443") + } +} + +// TestServeGetConfigRejectsUnrepresentableHandler verifies that get-config +// fails loudly when a service root handler has no representation in the config +// file format (Text/Redirect), rather than silently emitting an endpoint-less +// service that would lose the handler on round-trip. +func TestServeGetConfigRejectsUnrepresentableHandler(t *testing.T) { + const svc = tailcfg.ServiceName("svc:awesome") + const sni = "awesome.test.ts.net" + webKey := ipn.HostPort(fmt.Sprintf("%s:443", sni)) + + tests := []struct { + name string + handler *ipn.HTTPHandler + wantText string + }{ + {"text", &ipn.HTTPHandler{Text: "hello"}, "text"}, + {"redirect", &ipn.HTTPHandler{Redirect: "https://elsewhere.test.ts.net/"}, "redirect"}, + {"unix-proxy", &ipn.HTTPHandler{Proxy: "unix:/run/app.sock"}, "unix"}, + {"unsupported-scheme-proxy", &ipn.HTTPHandler{Proxy: "tcp://127.0.0.1:22"}, "scheme"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lc := &fakeLocalServeClient{ + config: &ipn.ServeConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + svc: { + TCP: map[uint16]*ipn.TCPPortHandler{443: {HTTPS: true}}, + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + webKey: {Handlers: map[string]*ipn.HTTPHandler{"/": tt.handler}}, + }, + }, + }, + }, + prefs: &ipn.Prefs{AdvertiseServices: []string{svc.String()}}, + } + // Use a single-service export: --all now skips an unrepresentable + // service with a stderr warning, whereas --service surfaces the + // specific reason as an error (see TestServeGetConfigAllSkips* for + // the --all path). + var stdout bytes.Buffer + getEnv := &serveEnv{lc: lc, service: svc, testStdout: &stdout} + err := getEnv.runServeGetConfig(context.Background(), nil) + if err == nil { + t.Fatalf("runServeGetConfig succeeded; want error for %s handler. output=%s", tt.name, stdout.String()) + } + if !strings.Contains(err.Error(), tt.wantText) { + t.Errorf("error = %v; want it to mention %q", err, tt.wantText) + } + }) + } +} diff --git a/ipn/conffile/serveconf.go b/ipn/conffile/serveconf.go index b58badc53..24d4521d2 100644 --- a/ipn/conffile/serveconf.go +++ b/ipn/conffile/serveconf.go @@ -21,19 +21,33 @@ "tailscale.com/util/mak" ) -// LegacyVersion is the sentinel [ServicesConfigFile.Version] used to mark a -// config that was loaded from the legacy raw [ipn.ServeConfig] format (a -// version-less file, such as "tailscale serve status --json" output). When -// Version is LegacyVersion, [ServicesConfigFile.Legacy] is set and Services is -// nil. It is never written to disk; the on-disk format always uses "0.0.1". -const LegacyVersion = "0.0.0" +// Config file format versions. +const ( + // LegacyVersion is the sentinel [ServicesConfigFile.Version] used to mark a + // config that was loaded from the legacy raw [ipn.ServeConfig] format (a + // version-less file, such as "tailscale serve status --json" output). When + // Version is LegacyVersion, [ServicesConfigFile.Legacy] is set and Services + // is nil. It is never written to disk. + LegacyVersion = "0.0.0" + // ConfigVersionV1 is the original format: every endpoint Target is a + // shorthand string. + ConfigVersionV1 = "0.0.1" + // ConfigVersionV2 adds the [Target] object form for endpoints whose back-end + // protocol differs from the front-end's default. It also fixes the meaning of + // the bare "https://" shorthand: it now names the front-end listener (an HTTPS + // front with a plain-HTTP back-end), whereas a 0.0.1 file used it for a TLS + // back-end. Because that meaning changed, get-config stamps every export V2 so + // an older client rejects it with a version error rather than misreading it. + ConfigVersionV2 = "0.0.2" +) // ServicesConfigFile is the config file format for services configuration. type ServicesConfigFile struct { - // Version is "0.0.1" for the declarative services configuration file - // format, or [LegacyVersion] ("0.0.0") when this value was produced by - // [LoadServicesConfig] from a legacy raw ipn.ServeConfig file (in which - // case Legacy is set instead of Services). + // Version is [ConfigVersionV1] ("0.0.1"), or [ConfigVersionV2] ("0.0.2") + // when an endpoint uses the object form (see [Target]), or [LegacyVersion] + // ("0.0.0") when produced by [LoadServicesConfig] from a legacy raw + // ipn.ServeConfig file (in which case Legacy is set instead of Services). + // Always present. Version string `json:"version"` Services map[tailcfg.ServiceName]*ServiceDetailsFile `json:"services,omitzero"` @@ -47,16 +61,20 @@ type ServicesConfigFile struct { // ServiceDetailsFile is the config syntax for an individual Tailscale Service. type ServiceDetailsFile struct { - // Version is always "0.0.1", set if and only if this is not inside a - // [ServiceConfigFile]. + // Version is "0.0.1", or "0.0.2" when an endpoint uses the object form (see + // [Target]); set if and only if this is not inside a [ServicesConfigFile]. Version string `json:"version,omitzero"` // Endpoints are sets of reverse proxy mappings from ProtoPortRanges on a - // Service to Targets (proto+destination+port) on remote destinations (or - // localhost). - // For example, "tcp:443" -> "tcp://localhost:8000" is an endpoint definition - // mapping traffic on the TCP port 443 of the Service to port 8080 on localhost. - // The Proto in the key must be populated. + // Service to Targets on remote destinations (or localhost). The key's Proto + // must be TCP; the front-end (listener) and back-end protocols are carried + // by the [Target] value. + // For example, "tcp:443" -> "tcp://localhost:8000" maps TCP traffic on port + // 443 of the Service to port 8000 on localhost, and "tcp:443" -> + // "https://localhost:8000" terminates HTTPS on port 443 and proxies to a + // plain-HTTP back-end on localhost:8000. See [Target] for the full syntax, + // including the object form used when the back-end protocol differs from the + // front-end's default. // As a special case, if the only mapping provided is "*" -> "TUN", that // enables TUN/L3 mode, where packets are delivered to the Tailscale network // interface with the understanding that the user will deal with them manually. @@ -82,105 +100,290 @@ type ServiceDetailsFile struct { ) // Target is a destination for traffic to go to when it arrives at a Tailscale -// Service host. +// Service host. It records two independent protocols: the front-end protocol +// that tailscaled terminates for the listener ([Target.Front]) and the back-end +// protocol used to reach the destination ([Target.Backend]). +// +// In JSON a Target is written one of two ways: +// +// - As a shorthand string "://:" whenever the back-end +// protocol is the one implied by the front-end (see [defaultBackend]): for +// example "https://127.0.0.1:8000" is an HTTPS listener proxying to a +// plain-HTTP back-end, and "tcp://127.0.0.1:22" is a TCP listener. The +// special strings "TUN" and "file://" are also shorthands. +// +// Note that the shorthand scheme names the FRONT-end (listener) protocol, +// not the back-end: "https://127.0.0.1:8000" terminates TLS at the listener +// and then connects to the back-end over plaintext HTTP. To proxy to a TLS +// back-end you must use the object form below; there is no shorthand for it. +// +// - As an object {"front": "", "backend": "://:"} +// when the back-end protocol differs from the implied default, for example +// an HTTPS listener proxying to a TLS back-end: +// {"front": "https", "backend": "https+insecure://127.0.0.1:8443"}. +// +// As a special case, Front == ProtoTUN activates "TUN mode" where packets are +// delivered to the Tailscale TUN interface and then manually handled by the +// user; it has no back-end. type Target struct { - // The protocol over which to communicate with the Destination. - // Protocol == ProtoTUN is a special case, activating "TUN mode" where - // packets are delivered to the Tailscale TUN interface and then manually - // handled by the user. - Protocol ServiceProtocol + // Front is the front-end (listener) protocol that tailscaled terminates: + // one of ProtoHTTP, ProtoHTTPS, ProtoTCP, ProtoTLSTerminatedTCP, or + // ProtoTUN. + Front ServiceProtocol - // If Protocol is ProtoFile, then Destination is a file path. - // If Protocol is ProtoTUN, then Destination is empty. + // Backend is the protocol used to reach Destination: one of ProtoHTTP, + // ProtoHTTPS, ProtoHTTPSInsecure, ProtoTCP, or ProtoFile. It is empty when + // Front is ProtoTUN. + Backend ServiceProtocol + + // If Backend is ProtoFile, then Destination is a file path. + // If Front is ProtoTUN, then Destination is empty. // Otherwise, it is a host. Destination string - // If Protocol is not ProtoFile or ProtoTUN, then DestinationPorts is the + // If Backend is neither ProtoFile nor empty, then DestinationPorts is the // set of ports on which to connect to the host referred to by Destination. DestinationPorts tailcfg.PortRange } +// targetObject is the object (non-shorthand) JSON representation of a [Target]. +type targetObject struct { + Front ServiceProtocol `json:"front"` + Backend string `json:"backend"` +} + +// defaultBackend returns the back-end protocol implied by a front-end protocol +// when a Target is written using the shorthand string form. A Target whose +// Backend equals this value can be written as a shorthand string; otherwise it +// must be written as an object so the back-end protocol is not lost. +func defaultBackend(front ServiceProtocol) ServiceProtocol { + switch front { + case ProtoHTTP, ProtoHTTPS: + return ProtoHTTP + case ProtoTCP, ProtoTLSTerminatedTCP: + return ProtoTCP + } + return "" +} + +// validFront reports whether p is a valid front-end (listener) protocol. +func validFront(p ServiceProtocol) bool { + switch p { + case ProtoHTTP, ProtoHTTPS, ProtoTCP, ProtoTLSTerminatedTCP, ProtoTUN: + return true + } + return false +} + +// validBackendForFront reports whether backend is a legal back-end protocol for +// the given front-end (listener) protocol. It is the single source of truth for +// front/back-end coherence and tracks the subset of the serve apply path that +// the config file format can represent: +// +// - Web fronts (HTTP, HTTPS) proxy via applyWebServe. Its +// ExpandProxyTargetValue allowlist also includes "unix", but unix-socket +// back-ends have no representation in the config file format, so the +// representable web back-ends are {http, https, https+insecure} plus a file +// back-end served as a static root. +// - TCP fronts (TCP, TLS-terminated TCP) forward via applyTCPServe, whose +// allowlist is {tcp} only. +// - TUN has no back-end. +// +// A combination this rejects would otherwise parse cleanly but fail later with +// a confusing low-level error from ExpandProxyTargetValue (see #19724). +func validBackendForFront(front, backend ServiceProtocol) bool { + switch front { + case ProtoHTTP, ProtoHTTPS: + switch backend { + case ProtoHTTP, ProtoHTTPS, ProtoHTTPSInsecure, ProtoFile: + return true + } + case ProtoTCP, ProtoTLSTerminatedTCP: + return backend == ProtoTCP + case ProtoTUN: + return backend == "" + } + return false +} + +// parseTarget parses a "://" spec (or "file://") into +// its protocol, destination, and ports. +func parseTarget(str string) (proto ServiceProtocol, dest string, ports tailcfg.PortRange, err error) { + p, rest, found := strings.Cut(str, "://") + if !found { + return "", "", tailcfg.PortRange{}, errors.New("handler not of form ://") + } + switch ServiceProtocol(p) { + case ProtoFile: + return ProtoFile, path.Clean(rest), tailcfg.PortRange{}, nil + case ProtoHTTP, ProtoHTTPS, ProtoHTTPSInsecure, ProtoTCP, ProtoTLSTerminatedTCP: + host, portRange, err := tailcfg.ParseHostPortRange(rest) + if err != nil { + return "", "", tailcfg.PortRange{}, err + } + return ServiceProtocol(p), host, portRange, nil + } + return "", "", tailcfg.PortRange{}, errors.New("unsupported protocol") +} + // UnmarshalJSON implements [jsonv1.Unmarshaler]. func (t *Target) UnmarshalJSON(buf []byte) error { return jsonv2.Unmarshal(buf, t) } -// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom]. +// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom]. It accepts either the +// shorthand string form or the {"front","backend"} object form. func (t *Target) UnmarshalJSONFrom(dec *jsontext.Decoder) error { - var str string - if err := jsonv2.UnmarshalDecode(dec, &str); err != nil { - return err + switch dec.PeekKind() { + case '"': + var str string + if err := jsonv2.UnmarshalDecode(dec, &str); err != nil { + return err + } + return t.fromShorthand(str) + case '{': + var o targetObject + if err := jsonv2.UnmarshalDecode(dec, &o, jsonv2.RejectUnknownMembers(true)); err != nil { + return err + } + return t.fromObject(o) + default: + return errors.New("endpoint target must be a string or an object") } +} - // The TUN case does not look like a standard :// arrangement, - // so handled separately. +// fromShorthand populates t from the shorthand string form. The scheme names +// the front-end listener for real front protocols; for the back-end-only +// schemes (https+insecure, file) the front is HTTPS and the scheme names the +// back-end. +func (t *Target) fromShorthand(str string) error { + // The TUN case does not look like a standard :// arrangement, + // so it is handled separately. if str == "TUN" { - t.Protocol = ProtoTUN + t.Front = ProtoTUN + t.Backend = "" t.Destination = "" t.DestinationPorts = tailcfg.PortRangeAny return nil } - proto, rest, found := strings.Cut(str, "://") - if !found { - return errors.New("handler not of form ://") + proto, dest, ports, err := parseTarget(str) + if err != nil { + return err } - - switch ServiceProtocol(proto) { + switch proto { + case ProtoHTTP, ProtoHTTPS, ProtoTCP, ProtoTLSTerminatedTCP: + t.Front = proto + t.Backend = defaultBackend(proto) + case ProtoHTTPSInsecure: + t.Front = ProtoHTTPS + t.Backend = ProtoHTTPSInsecure case ProtoFile: - target := path.Clean(rest) - t.Protocol = ProtoFile - t.Destination = target - t.DestinationPorts = tailcfg.PortRange{} - case ProtoHTTP, ProtoHTTPS, ProtoHTTPSInsecure, ProtoTCP, ProtoTLSTerminatedTCP: - host, portRange, err := tailcfg.ParseHostPortRange(rest) - if err != nil { - return err - } - t.Protocol = ServiceProtocol(proto) - t.Destination = host - t.DestinationPorts = portRange + t.Front = ProtoHTTPS + t.Backend = ProtoFile default: return errors.New("unsupported protocol") } - + if !validBackendForFront(t.Front, t.Backend) { + return fmt.Errorf("front protocol %q cannot proxy to a %q back-end", t.Front, t.Backend) + } + t.Destination = dest + t.DestinationPorts = ports return nil } -func (t *Target) MarshalText() ([]byte, error) { - var out string - switch t.Protocol { - case ProtoFile: - out = fmt.Sprintf("%s://%s", t.Protocol, t.Destination) - case ProtoTUN: - out = "TUN" - case ProtoHTTP, ProtoHTTPS, ProtoHTTPSInsecure, ProtoTCP, ProtoTLSTerminatedTCP: - out = fmt.Sprintf("%s://%s", t.Protocol, net.JoinHostPort(t.Destination, t.DestinationPorts.String())) - default: - return nil, errors.New("unsupported protocol") +// fromObject populates t from the object form. +func (t *Target) fromObject(o targetObject) error { + if !validFront(o.Front) || o.Front == ProtoTUN { + return fmt.Errorf("invalid front protocol %q", o.Front) } - return []byte(out), nil + proto, dest, ports, err := parseTarget(o.Backend) + if err != nil { + return err + } + if !validBackendForFront(o.Front, proto) { + return fmt.Errorf("front protocol %q cannot proxy to a %q back-end", o.Front, proto) + } + t.Front = o.Front + t.Backend = proto + t.Destination = dest + t.DestinationPorts = ports + return nil +} + +// MarshalJSON implements [jsonv1.Marshaler]. It emits the shorthand string form +// when the back-end protocol is the one implied by the front-end, and the +// object form otherwise so that the back-end protocol is preserved. +func (t *Target) MarshalJSON() ([]byte, error) { + if s, ok := t.shorthand(); ok { + return jsonv2.Marshal(s) + } + // Never emit an object form we would refuse to read back: the back-end must + // be coherent with the front (this also rejects an invalid front or a TUN + // front, neither of which has an object form). + if !validBackendForFront(t.Front, t.Backend) { + return nil, fmt.Errorf("front protocol %q cannot proxy to a %q back-end", t.Front, t.Backend) + } + return jsonv2.Marshal(targetObject{Front: t.Front, Backend: t.backendSpec()}) +} + +// backendSpec renders the back-end as a "://" spec. +func (t *Target) backendSpec() string { + if t.Backend == ProtoFile { + return fmt.Sprintf("%s://%s", ProtoFile, t.Destination) + } + return fmt.Sprintf("%s://%s", t.Backend, net.JoinHostPort(t.Destination, t.DestinationPorts.String())) +} + +// shorthand returns the shorthand string form of t and whether it is +// representable as a shorthand (i.e. the back-end is the default for the front, +// or one of the back-end-only schemes paired with an HTTPS front). +func (t *Target) shorthand() (string, bool) { + switch t.Front { + case ProtoTUN: + return "TUN", true + case ProtoHTTP, ProtoHTTPS: + switch { + case t.Backend == ProtoFile && t.Front == ProtoHTTPS: + return fmt.Sprintf("%s://%s", ProtoFile, t.Destination), true + case t.Backend == ProtoHTTPSInsecure && t.Front == ProtoHTTPS: + return fmt.Sprintf("%s://%s", ProtoHTTPSInsecure, net.JoinHostPort(t.Destination, t.DestinationPorts.String())), true + case t.Backend == "" || t.Backend == defaultBackend(t.Front): + return fmt.Sprintf("%s://%s", t.Front, net.JoinHostPort(t.Destination, t.DestinationPorts.String())), true + } + return "", false + case ProtoTCP, ProtoTLSTerminatedTCP: + if t.Backend == "" || t.Backend == defaultBackend(t.Front) { + return fmt.Sprintf("%s://%s", t.Front, net.JoinHostPort(t.Destination, t.DestinationPorts.String())), true + } + return "", false + } + return "", false } // LoadServicesConfig loads a serve config file as a [ServicesConfigFile]. // // If the file has a top-level "version" field it is parsed as that versioned -// declarative format. Otherwise it is treated as a legacy raw [ipn.ServeConfig] -// (such as "tailscale serve status --json" emits): the returned -// ServicesConfigFile has Version [LegacyVersion] and its Legacy field set to the -// parsed raw config, with Services left nil. +// declarative format (ConfigVersionV1 or ConfigVersionV2). Otherwise it is +// treated as a legacy raw [ipn.ServeConfig] (such as "tailscale serve status +// --json" emits): the returned ServicesConfigFile has Version LegacyVersion and +// its Legacy field set to the parsed raw config, with Services left nil. +// +// It also returns any non-fatal migration warnings (e.g. an "https://" +// shorthand read from a ConfigVersionV1 file, whose meaning changed in +// ConfigVersionV2) for the caller to surface to the user. // // forService is used only for the versioned Services configuration file format. -func LoadServicesConfig(filename string, forService string) (*ServicesConfigFile, error) { +func LoadServicesConfig(filename string, forService string) (cfg *ServicesConfigFile, warnings []string, err error) { data, err := os.ReadFile(filename) if err != nil { - return nil, err + return nil, nil, err } var json []byte if hujsonStandardize != nil { json, err = hujsonStandardize(data) if err != nil { - return nil, err + return nil, nil, err } } else { json = data @@ -189,7 +392,7 @@ func LoadServicesConfig(filename string, forService string) (*ServicesConfigFile Version string `json:"version"` } if err = jsonv2.Unmarshal(json, &ver); err != nil { - return nil, fmt.Errorf("could not parse config file version: %w", err) + return nil, nil, fmt.Errorf("could not parse config file version: %w", err) } if ver.Version == "" { // No "version" field. This is either the legacy raw ipn.ServeConfig @@ -207,7 +410,7 @@ func LoadServicesConfig(filename string, forService string) (*ServicesConfigFile } if err := jsonv2.Unmarshal(json, &probe); err == nil && (len(probe.Services) > 0 || len(probe.Endpoints) > 0) { - return nil, errors.New(`config file looks like a Services configuration file but is missing the required "version" field`) + return nil, nil, errors.New(`config file looks like a Services configuration file but is missing the required "version" field`) } // Legacy raw ipn.ServeConfig: parse leniently (like set-raw and // TS_SERVE_CONFIG) so "serve status --json" round-trips keep working. @@ -215,67 +418,89 @@ func LoadServicesConfig(filename string, forService string) (*ServicesConfigFile // sentinel so the public function signature stays stable. legacy := new(ipn.ServeConfig) if err := jsonv2.Unmarshal(json, legacy); err != nil { - return nil, fmt.Errorf("could not parse serve config: %w", err) + return nil, nil, fmt.Errorf("could not parse serve config: %w", err) } - return &ServicesConfigFile{Version: LegacyVersion, Legacy: legacy}, nil + return &ServicesConfigFile{Version: LegacyVersion, Legacy: legacy}, nil, nil } - if ver.Version != "0.0.1" { - return nil, fmt.Errorf("unsupported config file version %q", ver.Version) + switch ver.Version { + case ConfigVersionV1, ConfigVersionV2: + // Both versions share a parse path; the object form is self-describing, + // so a file is accepted regardless of which version it declares. The + // version is passed through so the meaning of an "https://" shorthand + // can be resolved per-version (see loadConfigV0). + return loadConfigV0(json, forService, ver.Version) } - return loadConfigV0(json, forService) + return nil, nil, fmt.Errorf("unsupported config file version %q", ver.Version) } -func loadConfigV0(json []byte, forService string) (*ServicesConfigFile, error) { +// loadConfigV0 parses and validates a 0.0.x config. version is the declared +// file version, used to resolve the meaning of an "https://" shorthand: in +// 0.0.1 it named a TLS back-end, whereas in 0.0.2 it names an HTTPS front end +// with a plain-HTTP back-end. For a 0.0.1 file each such endpoint is rewritten +// to its legacy meaning and a migration warning is returned. +func loadConfigV0(json []byte, forService, version string) (cfg *ServicesConfigFile, warnings []string, err error) { var scf ServicesConfigFile if svcName := tailcfg.AsServiceName(forService); svcName != "" { var sdf ServiceDetailsFile err := jsonv2.Unmarshal(json, &sdf, jsonv2.RejectUnknownMembers(true)) if err != nil { - return nil, err + return nil, nil, err } mak.Set(&scf.Services, svcName, &sdf) } else { err := jsonv2.Unmarshal(json, &scf, jsonv2.RejectUnknownMembers(true)) if err != nil { - return nil, err + return nil, nil, err } } for svcName, svc := range scf.Services { if forService == "" && svc.Version != "" { - return nil, errors.New("services cannot be versioned separately from config file") + return nil, nil, errors.New("services cannot be versioned separately from config file") } if err := svcName.Validate(); err != nil { - return nil, err + return nil, nil, err } if svc.Endpoints == nil { - return nil, fmt.Errorf("service %q: missing \"endpoints\" field", svcName) + return nil, nil, fmt.Errorf("service %q: missing \"endpoints\" field", svcName) } var sourcePorts []tailcfg.PortRange foundTUN := false foundNonTUN := false for ppr, target := range svc.Endpoints { - if target.Protocol == "TUN" { + // The bare "https://" shorthand is read with the fixed meaning (an + // HTTPS front with a plain-HTTP back-end) regardless of version. In a + // 0.0.1 file it formerly named a TLS back-end, so warn: this is the one + // shorthand whose meaning changed, and a 0.0.1 file that intended a TLS + // back-end will now connect in plaintext until it is re-exported with + // the back-end recorded explicitly. The object form did not exist in + // 0.0.1, so a real 0.0.1 file only reaches this state via the shorthand. + if version == ConfigVersionV1 && target.Front == ProtoHTTPS && target.Backend == ProtoHTTP { + warnings = append(warnings, fmt.Sprintf( + "service %q: endpoint %q uses the \"https://\" shorthand from a %s file; its meaning changed in %s and it is now read as an HTTPS front end with a plain-HTTP back-end. If you intended a TLS back-end, set it explicitly and re-export with this client.", + svcName, ppr.String(), ConfigVersionV1, ConfigVersionV2)) + } + if target.Front == ProtoTUN { if ppr.Proto != 0 || ppr.Ports != tailcfg.PortRangeAny { - return nil, fmt.Errorf("service %q: destination \"TUN\" can only be used with source \"*\"", svcName) + return nil, nil, fmt.Errorf("service %q: destination \"TUN\" can only be used with source \"*\"", svcName) } foundTUN = true } else { if ppr.Ports.Last-ppr.Ports.First != target.DestinationPorts.Last-target.DestinationPorts.First { - return nil, fmt.Errorf("service %q: source and destination port ranges must be of equal size", svcName.String()) + return nil, nil, fmt.Errorf("service %q: source and destination port ranges must be of equal size", svcName.String()) } foundNonTUN = true } if foundTUN && foundNonTUN { - return nil, fmt.Errorf("service %q: cannot mix TUN mode with non-TUN mode", svcName) + return nil, nil, fmt.Errorf("service %q: cannot mix TUN mode with non-TUN mode", svcName) } if pr := findOverlappingRange(sourcePorts, ppr.Ports); pr != nil { - return nil, fmt.Errorf("service %q: source port ranges %q and %q overlap", svcName, pr.String(), ppr.Ports.String()) + return nil, nil, fmt.Errorf("service %q: source port ranges %q and %q overlap", svcName, pr.String(), ppr.Ports.String()) } sourcePorts = append(sourcePorts, ppr.Ports) } } - return &scf, nil + return &scf, warnings, nil } // findOverlappingRange finds and returns a reference to a [tailcfg.PortRange] diff --git a/ipn/conffile/serveconf_test.go b/ipn/conffile/serveconf_test.go new file mode 100644 index 000000000..f8d0a71d4 --- /dev/null +++ b/ipn/conffile/serveconf_test.go @@ -0,0 +1,279 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_serve + +package conffile + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + jsonv2 "github.com/go-json-experiment/json" + "tailscale.com/tailcfg" +) + +func port(p uint16) tailcfg.PortRange { return tailcfg.PortRange{First: p, Last: p} } + +// TestTargetMarshal verifies that a Target marshals to the shorthand string +// form when the back-end is the front-end's implied default, and to the object +// form otherwise so the back-end protocol is preserved. +func TestTargetMarshal(t *testing.T) { + tests := []struct { + name string + target Target + want string + }{ + { + name: "https-front-http-backend", + target: Target{Front: ProtoHTTPS, Backend: ProtoHTTP, Destination: "127.0.0.1", DestinationPorts: port(30123)}, + want: `"https://127.0.0.1:30123"`, + }, + { + name: "http-front-http-backend", + target: Target{Front: ProtoHTTP, Backend: ProtoHTTP, Destination: "127.0.0.1", DestinationPorts: port(3000)}, + want: `"http://127.0.0.1:3000"`, + }, + { + name: "https-front-https-backend", + target: Target{Front: ProtoHTTPS, Backend: ProtoHTTPS, Destination: "127.0.0.1", DestinationPorts: port(8443)}, + want: `{"front":"https","backend":"https://127.0.0.1:8443"}`, + }, + { + name: "https-front-https-insecure-backend", + target: Target{Front: ProtoHTTPS, Backend: ProtoHTTPSInsecure, Destination: "127.0.0.1", DestinationPorts: port(8443)}, + want: `"https+insecure://127.0.0.1:8443"`, + }, + { + name: "http-front-https-backend", + target: Target{Front: ProtoHTTP, Backend: ProtoHTTPS, Destination: "127.0.0.1", DestinationPorts: port(8443)}, + want: `{"front":"http","backend":"https://127.0.0.1:8443"}`, + }, + { + name: "https-front-file-backend", + target: Target{Front: ProtoHTTPS, Backend: ProtoFile, Destination: "/var/www"}, + want: `"file:///var/www"`, + }, + { + name: "http-front-file-backend", + target: Target{Front: ProtoHTTP, Backend: ProtoFile, Destination: "/var/www"}, + want: `{"front":"http","backend":"file:///var/www"}`, + }, + { + name: "tcp", + target: Target{Front: ProtoTCP, Backend: ProtoTCP, Destination: "127.0.0.1", DestinationPorts: port(22)}, + want: `"tcp://127.0.0.1:22"`, + }, + { + name: "tls-terminated-tcp", + target: Target{Front: ProtoTLSTerminatedTCP, Backend: ProtoTCP, Destination: "127.0.0.1", DestinationPorts: port(22)}, + want: `"tls-terminated-tcp://127.0.0.1:22"`, + }, + { + name: "tun", + target: Target{Front: ProtoTUN, DestinationPorts: tailcfg.PortRangeAny}, + want: `"TUN"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // serve get-config marshals with the standard library. + got, err := json.Marshal(&tt.target) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if string(got) != tt.want { + t.Errorf("Marshal = %s; want %s", got, tt.want) + } + }) + } +} + +// TestTargetUnmarshal verifies parsing of both the shorthand string form +// (including the back-end-only schemes) and the object form. +func TestTargetUnmarshal(t *testing.T) { + tests := []struct { + name string + in string + want Target + }{ + { + name: "https-shorthand-defaults-to-http-backend", + in: `"https://127.0.0.1:30123"`, + want: Target{Front: ProtoHTTPS, Backend: ProtoHTTP, Destination: "127.0.0.1", DestinationPorts: port(30123)}, + }, + { + name: "http-shorthand", + in: `"http://127.0.0.1:3000"`, + want: Target{Front: ProtoHTTP, Backend: ProtoHTTP, Destination: "127.0.0.1", DestinationPorts: port(3000)}, + }, + { + name: "https-insecure-shorthand-implies-https-front", + in: `"https+insecure://127.0.0.1:8443"`, + want: Target{Front: ProtoHTTPS, Backend: ProtoHTTPSInsecure, Destination: "127.0.0.1", DestinationPorts: port(8443)}, + }, + { + name: "file-shorthand-implies-https-front", + in: `"file:///var/www"`, + want: Target{Front: ProtoHTTPS, Backend: ProtoFile, Destination: "/var/www"}, + }, + { + name: "tcp-shorthand", + in: `"tcp://127.0.0.1:22"`, + want: Target{Front: ProtoTCP, Backend: ProtoTCP, Destination: "127.0.0.1", DestinationPorts: port(22)}, + }, + { + name: "tls-terminated-tcp-shorthand", + in: `"tls-terminated-tcp://127.0.0.1:22"`, + want: Target{Front: ProtoTLSTerminatedTCP, Backend: ProtoTCP, Destination: "127.0.0.1", DestinationPorts: port(22)}, + }, + { + name: "tun", + in: `"TUN"`, + want: Target{Front: ProtoTUN, DestinationPorts: tailcfg.PortRangeAny}, + }, + { + name: "object-https-backend", + in: `{"front":"https","backend":"https://127.0.0.1:8443"}`, + want: Target{Front: ProtoHTTPS, Backend: ProtoHTTPS, Destination: "127.0.0.1", DestinationPorts: port(8443)}, + }, + { + name: "object-http-front-file-backend", + in: `{"front":"http","backend":"file:///var/www"}`, + want: Target{Front: ProtoHTTP, Backend: ProtoFile, Destination: "/var/www"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got Target + if err := jsonv2.Unmarshal([]byte(tt.in), &got); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if got != tt.want { + t.Errorf("Unmarshal(%s) = %+v; want %+v", tt.in, got, tt.want) + } + }) + } +} + +func TestTargetUnmarshalErrors(t *testing.T) { + for _, in := range []string{ + `"bogus"`, // no :// + `"gopher://127.0.0.1:70"`, // unsupported protocol + `{"front":"file","backend":"http://127.0.0.1:1"}`, // file is not a valid front + `{"front":"TUN","backend":"http://127.0.0.1:1"}`, // TUN has no object form + `42`, // not a string or object + // Incoherent front/back-end combinations: these parse field-by-field + // but would fail at apply time, so they're rejected up front (#19724). + `{"front":"tcp","backend":"http://127.0.0.1:80"}`, // TCP front needs a tcp back-end + `{"front":"tcp","backend":"file:///var/www"}`, // TCP front can't serve a file + `{"front":"https","backend":"tcp://127.0.0.1:22"}`, // web front can't proxy raw tcp + `{"front":"tls-terminated-tcp","backend":"https://127.0.0.1:1"}`, // TCP front needs a tcp back-end + } { + var got Target + if err := jsonv2.Unmarshal([]byte(in), &got); err == nil { + t.Errorf("Unmarshal(%s) = nil error; want error", in) + } + } +} + +// TestLoadServicesConfigMigration pins how the loader reads the one shorthand +// whose meaning changed between 0.0.1 and 0.0.2: a bare "https://" endpoint. It +// is read with the fixed meaning in both (an HTTPS front with a plain-HTTP +// back-end), but a 0.0.1 file gets a warning because that is the version where +// the meaning changed (it formerly named a TLS back-end). +func TestLoadServicesConfigMigration(t *testing.T) { + load := func(t *testing.T, body string) (*ServicesConfigFile, []string) { + t.Helper() + f := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(f, []byte(body), 0600); err != nil { + t.Fatal(err) + } + cfg, warnings, err := LoadServicesConfig(f, "") + if err != nil { + t.Fatalf("LoadServicesConfig: %v", err) + } + return cfg, warnings + } + backend := func(t *testing.T, cfg *ServicesConfigFile) ServiceProtocol { + t.Helper() + for _, sdf := range cfg.Services { + for _, tgt := range sdf.Endpoints { + return tgt.Backend + } + } + t.Fatal("no endpoint found") + return "" + } + + // A pre-change 0.0.1 file: "https://" named a TLS back-end. + const httpsBody = `{"version":%q,"services":{"svc:web":{"endpoints":{"tcp:443":"https://127.0.0.1:8443"}}}}` + + t.Run("v1-uses-fixed-http-backend-and-warns", func(t *testing.T) { + cfg, warnings := load(t, fmt.Sprintf(httpsBody, "0.0.1")) + if got := backend(t, cfg); got != ProtoHTTP { + t.Errorf("backend = %q; want %q (0.0.1 https:// read with the fixed front-naming meaning)", got, ProtoHTTP) + } + if len(warnings) != 1 { + t.Fatalf("warnings = %v; want exactly one migration warning", warnings) + } + if !strings.Contains(warnings[0], "svc:web") || !strings.Contains(warnings[0], "https://") { + t.Errorf("warning = %q; want it to name the service and the https:// shorthand", warnings[0]) + } + }) + + t.Run("v2-uses-http-backend-no-warning", func(t *testing.T) { + cfg, warnings := load(t, fmt.Sprintf(httpsBody, "0.0.2")) + if got := backend(t, cfg); got != ProtoHTTP { + t.Errorf("backend = %q; want %q (new meaning)", got, ProtoHTTP) + } + if len(warnings) != 0 { + t.Errorf("warnings = %v; want none for a 0.0.2 file", warnings) + } + }) + + t.Run("v1-http-shorthand-unchanged-no-warning", func(t *testing.T) { + cfg, warnings := load(t, `{"version":"0.0.1","services":{"svc:web":{"endpoints":{"tcp:80":"http://127.0.0.1:3000"}}}}`) + if got := backend(t, cfg); got != ProtoHTTP { + t.Errorf("backend = %q; want %q", got, ProtoHTTP) + } + if len(warnings) != 0 { + t.Errorf("warnings = %v; want none for an http:// shorthand", warnings) + } + }) +} + +// TestLoadServicesConfigVersion verifies that the loader accepts both 0.0.1 and +// 0.0.2 (parsing the object form under either, leniently) and rejects a missing +// or unknown version. +func TestLoadServicesConfigVersion(t *testing.T) { + for _, tt := range []struct { + name string + body string + wantErr bool + }{ + {"v1-shorthand", `{"version":"0.0.1","services":{"svc:web":{"endpoints":{"tcp:443":"https://127.0.0.1:3000"}}}}`, false}, + {"v2-object", `{"version":"0.0.2","services":{"svc:web":{"endpoints":{"tcp:443":{"front":"https","backend":"https://127.0.0.1:8443"}}}}}`, false}, + {"v2-shorthand", `{"version":"0.0.2","services":{"svc:web":{"endpoints":{"tcp:443":"https://127.0.0.1:3000"}}}}`, false}, + {"v1-with-object-is-lenient", `{"version":"0.0.1","services":{"svc:web":{"endpoints":{"tcp:443":{"front":"https","backend":"https://127.0.0.1:8443"}}}}}`, false}, + {"missing-version", `{"services":{"svc:web":{"endpoints":{"tcp:443":"https://127.0.0.1:3000"}}}}`, true}, + {"unknown-version", `{"version":"9.9.9","services":{"svc:web":{"endpoints":{"tcp:443":"https://127.0.0.1:3000"}}}}`, true}, + } { + t.Run(tt.name, func(t *testing.T) { + f := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(f, []byte(tt.body), 0600); err != nil { + t.Fatal(err) + } + _, _, err := LoadServicesConfig(f, "") + if (err != nil) != tt.wantErr { + t.Fatalf("LoadServicesConfig err = %v; wantErr = %v", err, tt.wantErr) + } + }) + } +}