From f52a6d1b8cf45e55f1d19303354b6f60eb7d151f Mon Sep 17 00:00:00 2001 From: shayne Date: Sat, 19 Nov 2022 09:42:14 -0500 Subject: [PATCH] cmd/tailscale/cli, ipn: move serve CLI funcs on to ServeConfig (#6401) Signed-off-by: Shayne Sweeney --- cmd/tailscale/cli/serve.go | 149 +++++++++++--------------------- cmd/tailscale/cli/serve_test.go | 4 +- ipn/serve.go | 135 +++++++++++++++++++++++++++++ ipn/store.go | 67 -------------- 4 files changed, 189 insertions(+), 166 deletions(-) create mode 100644 ipn/serve.go diff --git a/cmd/tailscale/cli/serve.go b/cmd/tailscale/cli/serve.go index fe24d4e9e..402b64da5 100644 --- a/cmd/tailscale/cli/serve.go +++ b/cmd/tailscale/cli/serve.go @@ -114,6 +114,19 @@ EXAMPLES } } +func (e *serveEnv) newFlags(name string, setup func(fs *flag.FlagSet)) *flag.FlagSet { + onError, out := flag.ExitOnError, Stderr + if e.testFlagOut != nil { + onError, out = flag.ContinueOnError, e.testFlagOut + } + fs := flag.NewFlagSet(name, onError) + fs.SetOutput(out) + if setup != nil { + setup(fs) + } + return fs +} + // serveEnv is the environment the serve command runs within. All I/O should be // done via serveEnv methods so that it can be faked out for tests. // @@ -133,6 +146,9 @@ type serveEnv struct { testStdout io.Writer } +// getSelfDNSName returns the DNS name of the current node. +// The trailing dot is removed. +// Returns an error if local client status fails. func (e *serveEnv) getSelfDNSName(ctx context.Context) (string, error) { st, err := e.getLocalClientStatus(ctx) if err != nil { @@ -141,6 +157,10 @@ func (e *serveEnv) getSelfDNSName(ctx context.Context) (string, error) { return strings.TrimSuffix(st.Self.DNSName, "."), nil } +// getLocalClientStatus calls LocalClient.Status, checks if +// Status is ready. +// Returns error if unable to reach tailscaled or if self node is nil. +// Exits if status is not running or starting. func (e *serveEnv) getLocalClientStatus(ctx context.Context) (*ipnstate.Status, error) { if e.testGetLocalClientStatus != nil { return e.testGetLocalClientStatus(ctx) @@ -160,19 +180,6 @@ func (e *serveEnv) getLocalClientStatus(ctx context.Context) (*ipnstate.Status, return st, nil } -func (e *serveEnv) newFlags(name string, setup func(fs *flag.FlagSet)) *flag.FlagSet { - onError, out := flag.ExitOnError, Stderr - if e.testFlagOut != nil { - onError, out = flag.ContinueOnError, e.testFlagOut - } - fs := flag.NewFlagSet(name, onError) - fs.SetOutput(out) - if setup != nil { - setup(fs) - } - return fs -} - func (e *serveEnv) getServeConfig(ctx context.Context) (*ipn.ServeConfig, error) { if e.testGetServeConfig != nil { return e.testGetServeConfig(ctx) @@ -187,13 +194,6 @@ func (e *serveEnv) setServeConfig(ctx context.Context, c *ipn.ServeConfig) error return localClient.SetServeConfig(ctx, c) } -func (e *serveEnv) stdout() io.Writer { - if e.testStdout != nil { - return e.testStdout - } - return os.Stdout -} - // validateServePort returns --serve-port flag value, // or an error if the port is not a valid port to serve on. func (e *serveEnv) validateServePort() (port uint16, err error) { @@ -221,12 +221,6 @@ func (e *serveEnv) runServe(ctx context.Context, args []string) error { return flag.ErrHelp } - srvPort, err := e.validateServePort() - if err != nil { - return err - } - srvPortStr := strconv.Itoa(int(srvPort)) - // Undocumented debug command (not using ffcli subcommands) to set raw // configs from stdin for now (2022-11-13). if len(args) == 1 && args[0] == "set-raw" { @@ -246,6 +240,12 @@ func (e *serveEnv) runServe(ctx context.Context, args []string) error { return flag.ErrHelp } + srvPort, err := e.validateServePort() + if err != nil { + return err + } + srvPortStr := strconv.Itoa(int(srvPort)) + mount, err := cleanMountPoint(args[0]) if err != nil { return err @@ -305,7 +305,7 @@ func (e *serveEnv) runServe(ctx context.Context, args []string) error { } hp := ipn.HostPort(net.JoinHostPort(dnsName, srvPortStr)) - if isTCPForwardingOnPort(sc, srvPort) { + if sc.IsTCPForwardingOnPort(srvPort) { fmt.Fprintf(os.Stderr, "error: cannot serve web; already serving TCP\n") return flag.ErrHelp } @@ -359,11 +359,11 @@ func (e *serveEnv) handleWebServeRemove(ctx context.Context, mount string) error if err != nil { return err } - if isTCPForwardingOnPort(sc, srvPort) { + if sc.IsTCPForwardingOnPort(srvPort) { return errors.New("cannot remove web handler; currently serving TCP") } hp := ipn.HostPort(net.JoinHostPort(dnsName, srvPortStr)) - if !httpHandlerExists(sc, hp, mount) { + if !sc.WebHandlerExists(hp, mount) { return errors.New("error: serve config does not exist") } // delete existing handler, then cascade delete if empty @@ -385,18 +385,6 @@ func (e *serveEnv) handleWebServeRemove(ctx context.Context, mount string) error return nil } -func httpHandlerExists(sc *ipn.ServeConfig, hp ipn.HostPort, mount string) bool { - h := getHTTPHandler(sc, hp, mount) - return h != nil -} - -func getHTTPHandler(sc *ipn.ServeConfig, hp ipn.HostPort, mount string) *ipn.HTTPHandler { - if sc != nil && sc.Web[hp] != nil { - return sc.Web[hp].Handlers[mount] - } - return nil -} - func cleanMountPoint(mount string) (string, error) { if mount == "" { return "", errors.New("mount point cannot be empty") @@ -447,39 +435,6 @@ func expandProxyTarget(target string) (string, error) { return url, nil } -// isTCPForwardingAny checks if any TCP port is being forwarded. -func isTCPForwardingAny(sc *ipn.ServeConfig) bool { - if sc == nil || len(sc.TCP) == 0 { - return false - } - for _, h := range sc.TCP { - if h.TCPForward != "" { - return true - } - } - return false -} - -// isTCPForwardingOnPort checks serve config to see if -// we're specifically forwarding TCP on the given port. -func isTCPForwardingOnPort(sc *ipn.ServeConfig, port uint16) bool { - if sc == nil || sc.TCP[port] == nil { - return false - } - return !sc.TCP[port].HTTPS -} - -// isServingWeb checks serve config to see if -// we're serving a web handler on the given port. -func isServingWeb(sc *ipn.ServeConfig, port uint16) bool { - if sc == nil || sc.Web == nil || sc.TCP == nil || - sc.TCP[port] == nil || sc.TCP[port].HTTPS == false { - // not listening on port - return false - } - return true -} - func allNumeric(s string) bool { for i := 0; i < len(s); i++ { if s[i] < '0' || s[i] > '9' { @@ -516,7 +471,7 @@ func (e *serveEnv) runServeStatus(ctx context.Context, args []string) error { if err != nil { return err } - if isTCPForwardingAny(sc) { + if sc.IsTCPForwardingAny() { if err := printTCPStatusTree(ctx, sc, st); err != nil { return err } @@ -540,6 +495,13 @@ func (e *serveEnv) runServeStatus(ctx context.Context, args []string) error { return nil } +func (e *serveEnv) stdout() io.Writer { + if e.testStdout != nil { + return e.testStdout + } + return os.Stdout +} + func printTCPStatusTree(ctx context.Context, sc *ipn.ServeConfig, st *ipnstate.Status) error { dnsName := strings.TrimSuffix(st.Self.DNSName, ".") for p, h := range sc.TCP { @@ -552,7 +514,7 @@ func printTCPStatusTree(ctx context.Context, sc *ipn.ServeConfig, st *ipnstate.S tlsStatus = "TLS terminated" } fStatus := "tailnet only" - if isFunnelOn(sc, hp) { + if sc.IsFunnelOn(hp) { fStatus = "Funnel on" } printf("|-- tcp://%s (%s, %s)\n", hp, tlsStatus, fStatus) @@ -570,7 +532,7 @@ func printWebStatusTree(sc *ipn.ServeConfig, hp ipn.HostPort) { return } fStatus := "tailnet only" - if isFunnelOn(sc, hp) { + if sc.IsFunnelOn(hp) { fStatus = "Funnel on" } host, portStr, _ := net.SplitHostPort(string(hp)) @@ -649,12 +611,15 @@ func (e *serveEnv) runServeTCP(ctx context.Context, args []string) error { fwdAddr := "127.0.0.1:" + portStr - if e.remove { - if isServingWeb(sc, srvPort) { - return errors.New("cannot remove TCP port; currently serving web") + if sc.IsServingWeb(srvPort) { + if e.remove { + return fmt.Errorf("unable to remove; serving web, not TCP forwarding on serve port %d", srvPort) } - if sc.TCP != nil && sc.TCP[srvPort] != nil && - sc.TCP[srvPort].TCPForward == fwdAddr { + return fmt.Errorf("cannot serve TCP; already serving web on %d", srvPort) + } + + if e.remove { + if ph := sc.GetTCPPortHandler(srvPort); ph != nil && ph.TCPForward == fwdAddr { delete(sc.TCP, srvPort) // clear map mostly for testing if len(sc.TCP) == 0 { @@ -662,15 +627,9 @@ func (e *serveEnv) runServeTCP(ctx context.Context, args []string) error { } return e.setServeConfig(ctx, sc) } - return errors.New("error: serve config does not exist") } - if isServingWeb(sc, srvPort) { - fmt.Fprintf(os.Stderr, "error: cannot serve TCP; already serving Web\n\n") - return flag.ErrHelp - } - mak.Set(&sc.TCP, srvPort, &ipn.TCPPortHandler{TCPForward: fwdAddr}) dnsName, err := e.getSelfDNSName(ctx) @@ -716,6 +675,9 @@ func (e *serveEnv) runServeFunnel(ctx context.Context, args []string) error { if err != nil { return err } + if sc == nil { + sc = new(ipn.ServeConfig) + } st, err := e.getLocalClientStatus(ctx) if err != nil { return fmt.Errorf("getting client status: %w", err) @@ -725,14 +687,11 @@ func (e *serveEnv) runServeFunnel(ctx context.Context, args []string) error { } dnsName := strings.TrimSuffix(st.Self.DNSName, ".") hp := ipn.HostPort(dnsName + ":" + srvPortStr) - if on && sc != nil && sc.AllowFunnel[hp] || - !on && (sc == nil || !sc.AllowFunnel[hp]) { + isFun := sc.IsFunnelOn(hp) + if on && isFun || !on && !isFun { // Nothing to do. return nil } - if sc == nil { - sc = &ipn.ServeConfig{} - } if on { mak.Set(&sc.AllowFunnel, hp, true) } else { @@ -747,7 +706,3 @@ func (e *serveEnv) runServeFunnel(ctx context.Context, args []string) error { } return nil } - -func isFunnelOn(sc *ipn.ServeConfig, hp ipn.HostPort) bool { - return sc != nil && sc.AllowFunnel[hp] -} diff --git a/cmd/tailscale/cli/serve_test.go b/cmd/tailscale/cli/serve_test.go index d3a271ce3..e3269b674 100644 --- a/cmd/tailscale/cli/serve_test.go +++ b/cmd/tailscale/cli/serve_test.go @@ -577,9 +577,9 @@ func TestServeConfigMutations(t *testing.T) { }, }, }) - add(step{ // try to start a tcp forwarder on the same port + add(step{ // try to start a tcp forwarder on the same serve port (443 default) command: cmd("tcp 5432"), - wantErr: exactErr(flag.ErrHelp, "flag.ErrHelp"), + wantErr: anyErr(), }) // And now run the steps above. diff --git a/ipn/serve.go b/ipn/serve.go new file mode 100644 index 000000000..93d6b077e --- /dev/null +++ b/ipn/serve.go @@ -0,0 +1,135 @@ +// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ipn + +// ServeConfigKey returns a StateKey that stores the +// JSON-encoded ServeConfig for a config profile. +func ServeConfigKey(profileID ProfileID) StateKey { + return StateKey("_serve/" + profileID) +} + +// ServeConfig is the JSON type stored in the StateStore for +// StateKey "_serve/$PROFILE_ID" as returned by ServeConfigKey. +type ServeConfig struct { + // TCP are the list of TCP port numbers that tailscaled should handle for + // the Tailscale IP addresses. (not subnet routers, etc) + TCP map[uint16]*TCPPortHandler `json:",omitempty"` + + // Web maps from "$SNI_NAME:$PORT" to a set of HTTP handlers + // keyed by mount point ("/", "/foo", etc) + Web map[HostPort]*WebServerConfig `json:",omitempty"` + + // AllowFunnel is the set of SNI:port values for which funnel + // traffic is allowed, from trusted ingress peers. + AllowFunnel map[HostPort]bool `json:",omitempty"` +} + +// HostPort is an SNI name and port number, joined by a colon. +// There is no implicit port 443. It must contain a colon. +type HostPort string + +// WebServerConfig describes a web server's configuration. +type WebServerConfig struct { + Handlers map[string]*HTTPHandler // mountPoint => handler +} + +// TCPPortHandler describes what to do when handling a TCP +// connection. +type TCPPortHandler struct { + // HTTPS, if true, means that tailscaled should handle this connection as an + // HTTPS request as configured by ServeConfig.Web. + // + // It is mutually exclusive with TCPForward. + HTTPS bool `json:",omitempty"` + + // TCPForward is the IP:port to forward TCP connections to. + // Whether or not TLS is terminated by tailscaled depends on + // TerminateTLS. + // + // It is mutually exclusive with HTTPS. + TCPForward string `json:",omitempty"` + + // TerminateTLS, if non-empty, means that tailscaled should terminate the + // TLS connections before forwarding them to TCPForward, permitting only the + // SNI name with this value. It is only used if TCPForward is non-empty. + // (the HTTPS mode uses ServeConfig.Web) + TerminateTLS string `json:",omitempty"` +} + +// HTTPHandler is either a path or a proxy to serve. +type HTTPHandler struct { + // Exactly one of the following may be set. + + Path string `json:",omitempty"` // absolute path to directory or file to serve + Proxy string `json:",omitempty"` // http://localhost:3000/, localhost:3030, 3030 + + Text string `json:",omitempty"` // plaintext to serve (primarily for testing) + + // TODO(bradfitz): bool to not enumerate directories? TTL on mapping for + // temporary ones? Error codes? Redirects? +} + +// WebHandlerExists checks if the ServeConfig Web handler exists for +// the given host:port and mount point. +func (sc *ServeConfig) WebHandlerExists(hp HostPort, mount string) bool { + h := sc.GetWebHandler(hp, mount) + return h != nil +} + +// GetWebHandler returns the HTTPHandler for the given host:port and mount point. +// Returns nil if the handler does not exist. +func (sc *ServeConfig) GetWebHandler(hp HostPort, mount string) *HTTPHandler { + if sc.Web[hp] != nil { + return sc.Web[hp].Handlers[mount] + } + return nil +} + +// GetTCPPortHandler returns the TCPPortHandler for the given port. +// If the port is not configured, nil is returned. +func (sc *ServeConfig) GetTCPPortHandler(port uint16) *TCPPortHandler { + return sc.TCP[port] +} + +// IsTCPForwardingAny checks if ServeConfig is currently forwarding +// in TCPForward mode on any port. +// This is exclusive of Web/HTTPS serving. +func (sc *ServeConfig) IsTCPForwardingAny() bool { + if len(sc.TCP) == 0 { + return false + } + for _, h := range sc.TCP { + if h.TCPForward != "" { + return true + } + } + return false +} + +// IsTCPForwardingOnPort checks if ServeConfig is currently forwarding +// in TCPForward mode on the given port. +// This is exclusive of Web/HTTPS serving. +func (sc *ServeConfig) IsTCPForwardingOnPort(port uint16) bool { + if sc.TCP[port] == nil { + return false + } + return !sc.TCP[port].HTTPS +} + +// IsServingWeb checks if ServeConfig is currently serving +// Web/HTTPS on the given port. +// This is exclusive of TCPForwarding. +func (sc *ServeConfig) IsServingWeb(port uint16) bool { + if sc.TCP[port] == nil { + return false + } + return sc.TCP[port].HTTPS +} + +// IsFunnelOn checks if ServeConfig is currently allowing +// funnel traffic on for the given host:port. +func (sc *ServeConfig) IsFunnelOn(hp HostPort) bool { + return sc.AllowFunnel[hp] +} diff --git a/ipn/store.go b/ipn/store.go index 4420a8e54..a59952c3f 100644 --- a/ipn/store.go +++ b/ipn/store.go @@ -90,70 +90,3 @@ func ReadStoreInt(store StateStore, id StateKey) (int64, error) { func PutStoreInt(store StateStore, id StateKey, val int64) error { return store.WriteState(id, fmt.Appendf(nil, "%d", val)) } - -// ServeConfigKey returns a StateKey that stores the -// JSON-encoded ServeConfig for a config profile. -func ServeConfigKey(profileID ProfileID) StateKey { - return StateKey("_serve/" + profileID) -} - -// ServeConfig is the JSON type stored in the StateStore for -// StateKey "_serve/$PROFILE_ID" as returned by ServeConfigKey. -type ServeConfig struct { - // TCP are the list of TCP port numbers that tailscaled should handle for - // the Tailscale IP addresses. (not subnet routers, etc) - TCP map[uint16]*TCPPortHandler `json:",omitempty"` - - // Web maps from "$SNI_NAME:$PORT" to a set of HTTP handlers - // keyed by mount point ("/", "/foo", etc) - Web map[HostPort]*WebServerConfig `json:",omitempty"` - - // AllowFunnel is the set of SNI:port values for which funnel - // traffic is allowed, from trusted ingress peers. - AllowFunnel map[HostPort]bool `json:",omitempty"` -} - -// HostPort is an SNI name and port number, joined by a colon. -// There is no implicit port 443. It must contain a colon. -type HostPort string - -// WebServerConfig describes a web server's configuration. -type WebServerConfig struct { - Handlers map[string]*HTTPHandler // mountPoint => handler -} - -// TCPPortHandler describes what to do when handling a TCP -// connection. -type TCPPortHandler struct { - // HTTPS, if true, means that tailscaled should handle this connection as an - // HTTPS request as configured by ServeConfig.Web. - // - // It is mutually exclusive with TCPForward. - HTTPS bool `json:",omitempty"` - - // TCPForward is the IP:port to forward TCP connections to. - // Whether or not TLS is terminated by tailscaled depends on - // TerminateTLS. - // - // It is mutually exclusive with HTTPS. - TCPForward string `json:",omitempty"` - - // TerminateTLS, if non-empty, means that tailscaled should terminate the - // TLS connections before forwarding them to TCPForward, permitting only the - // SNI name with this value. It is only used if TCPForward is non-empty. - // (the HTTPS mode uses ServeConfig.Web) - TerminateTLS string `json:",omitempty"` -} - -// HTTPHandler is either a path or a proxy to serve. -type HTTPHandler struct { - // Exactly one of the following may be set. - - Path string `json:",omitempty"` // absolute path to directory or file to serve - Proxy string `json:",omitempty"` // http://localhost:3000/, localhost:3030, 3030 - - Text string `json:",omitempty"` // plaintext to serve (primarily for testing) - - // TODO(bradfitz): bool to not enumerate directories? TTL on mapping for - // temporary ones? Error codes? Redirects? -}