diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index d87df2974..639fcc1a2 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -19,7 +19,6 @@ import ( "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" "tailscale.com/tstest" - "tailscale.com/types/key" "tailscale.com/types/persist" "tailscale.com/types/preftype" "tailscale.com/version/distro" @@ -632,7 +631,7 @@ func TestPrefsFromUpArgs(t *testing.T) { st: &ipnstate.Status{ TailscaleIPs: []netaddr.IP{netaddr.MustParseIP("100.105.106.107")}, }, - wantErr: `cannot use 100.105.106.107 as the exit node as it is a local IP address to this machine, did you mean --advertise-exit-node?`, + wantErr: `cannot use 100.105.106.107 as an exit node as it is a local IP address to this machine; did you mean --advertise-exit-node?`, }, { name: "warn_linux_netfilter_nodivert", @@ -897,136 +896,6 @@ var cmpIP = cmp.Comparer(func(a, b netaddr.IP) bool { return a == b }) -func TestExitNodeIPOfArg(t *testing.T) { - mustIP := netaddr.MustParseIP - tests := []struct { - name string - arg string - st *ipnstate.Status - want netaddr.IP - wantErr string - }{ - { - name: "ip_while_stopped_okay", - arg: "1.2.3.4", - st: &ipnstate.Status{ - BackendState: "Stopped", - }, - want: mustIP("1.2.3.4"), - }, - { - name: "ip_not_found", - arg: "1.2.3.4", - st: &ipnstate.Status{ - BackendState: "Running", - }, - wantErr: `no node found in netmap with IP 1.2.3.4`, - }, - { - name: "ip_not_exit", - arg: "1.2.3.4", - st: &ipnstate.Status{ - BackendState: "Running", - Peer: map[key.NodePublic]*ipnstate.PeerStatus{ - key.NewNode().Public(): { - TailscaleIPs: []netaddr.IP{mustIP("1.2.3.4")}, - }, - }, - }, - wantErr: `node 1.2.3.4 is not advertising an exit node`, - }, - { - name: "ip", - arg: "1.2.3.4", - st: &ipnstate.Status{ - BackendState: "Running", - Peer: map[key.NodePublic]*ipnstate.PeerStatus{ - key.NewNode().Public(): { - TailscaleIPs: []netaddr.IP{mustIP("1.2.3.4")}, - ExitNodeOption: true, - }, - }, - }, - want: mustIP("1.2.3.4"), - }, - { - name: "no_match", - arg: "unknown", - st: &ipnstate.Status{MagicDNSSuffix: ".foo"}, - wantErr: `invalid value "unknown" for --exit-node; must be IP or unique node name`, - }, - { - name: "name", - arg: "skippy", - st: &ipnstate.Status{ - MagicDNSSuffix: ".foo", - Peer: map[key.NodePublic]*ipnstate.PeerStatus{ - key.NewNode().Public(): { - DNSName: "skippy.foo.", - TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, - ExitNodeOption: true, - }, - }, - }, - want: mustIP("1.0.0.2"), - }, - { - name: "name_not_exit", - arg: "skippy", - st: &ipnstate.Status{ - MagicDNSSuffix: ".foo", - Peer: map[key.NodePublic]*ipnstate.PeerStatus{ - key.NewNode().Public(): { - DNSName: "skippy.foo.", - TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, - }, - }, - }, - wantErr: `node "skippy" is not advertising an exit node`, - }, - { - name: "ambiguous", - arg: "skippy", - st: &ipnstate.Status{ - MagicDNSSuffix: ".foo", - Peer: map[key.NodePublic]*ipnstate.PeerStatus{ - key.NewNode().Public(): { - DNSName: "skippy.foo.", - TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, - ExitNodeOption: true, - }, - key.NewNode().Public(): { - DNSName: "SKIPPY.foo.", - TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, - ExitNodeOption: true, - }, - }, - }, - wantErr: `ambiguous exit node name "skippy"`, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := exitNodeIPOfArg(tt.arg, tt.st) - if err != nil { - if err.Error() == tt.wantErr { - return - } - if tt.wantErr == "" { - t.Fatal(err) - } - t.Fatalf("error = %#q; want %#q", err, tt.wantErr) - } - if tt.wantErr != "" { - t.Fatalf("got %v; want error %#q", got, tt.wantErr) - } - if got != tt.want { - t.Fatalf("got %v; want %v", got, tt.want) - } - }) - } -} - func TestCleanUpArgs(t *testing.T) { c := qt.New(t) tests := []struct { diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index 5f7c543db..91436f808 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -31,7 +31,6 @@ import ( "tailscale.com/tailcfg" "tailscale.com/types/logger" "tailscale.com/types/preftype" - "tailscale.com/util/dnsname" "tailscale.com/version" "tailscale.com/version/distro" ) @@ -245,65 +244,6 @@ func calcAdvertiseRoutes(advertiseRoutes string, advertiseDefaultRoute bool) ([] return routes, nil } -// peerWithTailscaleIP returns the peer in st with the provided -// Tailscale IP. -func peerWithTailscaleIP(st *ipnstate.Status, ip netaddr.IP) (ps *ipnstate.PeerStatus, ok bool) { - for _, ps := range st.Peer { - for _, ip2 := range ps.TailscaleIPs { - if ip == ip2 { - return ps, true - } - } - } - return nil, false -} - -// exitNodeIPOfArg maps from a user-provided CLI flag value to an IP -// address they want to use as an exit node. -func exitNodeIPOfArg(arg string, st *ipnstate.Status) (ip netaddr.IP, err error) { - if arg == "" { - return ip, errors.New("invalid use of exitNodeIPOfArg with empty string") - } - ip, err = netaddr.ParseIP(arg) - if err == nil { - // If we're online already and have a netmap, double check that the IP - // address specified is valid. - if st.BackendState == "Running" { - ps, ok := peerWithTailscaleIP(st, ip) - if !ok { - return ip, fmt.Errorf("no node found in netmap with IP %v", ip) - } - if !ps.ExitNodeOption { - return ip, fmt.Errorf("node %v is not advertising an exit node", ip) - } - } - return ip, err - } - match := 0 - for _, ps := range st.Peer { - baseName := dnsname.TrimSuffix(ps.DNSName, st.MagicDNSSuffix) - if !strings.EqualFold(arg, baseName) { - continue - } - match++ - if len(ps.TailscaleIPs) == 0 { - return ip, fmt.Errorf("node %q has no Tailscale IP?", arg) - } - if !ps.ExitNodeOption { - return ip, fmt.Errorf("node %q is not advertising an exit node", arg) - } - ip = ps.TailscaleIPs[0] - } - switch match { - case 0: - return ip, fmt.Errorf("invalid value %q for --exit-node; must be IP or unique node name", arg) - case 1: - return ip, nil - default: - return ip, fmt.Errorf("ambiguous exit node name %q", arg) - } -} - // prefsFromUpArgs returns the ipn.Prefs for the provided args. // // Note that the parameters upArgs and warnf are named intentionally @@ -316,25 +256,10 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo return nil, err } - var exitNodeIP netaddr.IP - if upArgs.exitNodeIP != "" { - var err error - exitNodeIP, err = exitNodeIPOfArg(upArgs.exitNodeIP, st) - if err != nil { - return nil, err - } - } else if upArgs.exitNodeAllowLANAccess { + if upArgs.exitNodeIP == "" && upArgs.exitNodeAllowLANAccess { return nil, fmt.Errorf("--exit-node-allow-lan-access can only be used with --exit-node") } - if upArgs.exitNodeIP != "" { - for _, ip := range st.TailscaleIPs { - if exitNodeIP == ip { - return nil, fmt.Errorf("cannot use %s as the exit node as it is a local IP address to this machine, did you mean --advertise-exit-node?", upArgs.exitNodeIP) - } - } - } - var tags []string if upArgs.advertiseTags != "" { tags = strings.Split(upArgs.advertiseTags, ",") @@ -354,7 +279,17 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo prefs.ControlURL = upArgs.server prefs.WantRunning = true prefs.RouteAll = upArgs.acceptRoutes - prefs.ExitNodeIP = exitNodeIP + + if upArgs.exitNodeIP != "" { + if err := prefs.SetExitNodeIP(upArgs.exitNodeIP, st); err != nil { + var e ipn.ExitNodeLocalIPError + if errors.As(err, &e) { + return nil, fmt.Errorf("%w; did you mean --advertise-exit-node?", err) + } + return nil, err + } + } + prefs.ExitNodeAllowLANAccess = upArgs.exitNodeAllowLANAccess prefs.CorpDNS = upArgs.acceptDNS prefs.AllowSingleHosts = upArgs.singleRoutes diff --git a/ipn/prefs.go b/ipn/prefs.go index 330fd073d..f7d34cf53 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -7,6 +7,7 @@ package ipn import ( "bytes" "encoding/json" + "errors" "fmt" "io/ioutil" "log" @@ -18,10 +19,12 @@ import ( "inet.af/netaddr" "tailscale.com/atomicfile" + "tailscale.com/ipn/ipnstate" "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/types/persist" "tailscale.com/types/preftype" + "tailscale.com/util/dnsname" ) //go:generate go run tailscale.com/cmd/cloner -type=Prefs -output=prefs_clone.go @@ -31,6 +34,12 @@ import ( // The default control plane is the hosted version run by Tailscale.com. const DefaultControlURL = "https://controlplane.tailscale.com" +var ( + // ErrExitNodeIDAlreadySet is returned from (*Prefs).SetExitNodeIP when the + // Prefs.ExitNodeID field is already set. + ErrExitNodeIDAlreadySet = errors.New("cannot set ExitNodeIP when ExitNodeID is already set") +) + // IsLoginServerSynonym reports whether a URL is a drop-in replacement // for the primary Tailscale login server. func IsLoginServerSynonym(val interface{}) bool { @@ -467,6 +476,109 @@ func (p *Prefs) SetAdvertiseExitNode(runExit bool) { netaddr.IPPrefixFrom(netaddr.IPv6Unspecified(), 0)) } +// peerWithTailscaleIP returns the peer in st with the provided +// Tailscale IP. +func peerWithTailscaleIP(st *ipnstate.Status, ip netaddr.IP) (ps *ipnstate.PeerStatus, ok bool) { + for _, ps := range st.Peer { + for _, ip2 := range ps.TailscaleIPs { + if ip == ip2 { + return ps, true + } + } + } + return nil, false +} + +func isRemoteIP(st *ipnstate.Status, ip netaddr.IP) bool { + for _, selfIP := range st.TailscaleIPs { + if ip == selfIP { + return false + } + } + return true +} + +// ClearExitNode sets the ExitNodeID and ExitNodeIP to their zero values. +func (p *Prefs) ClearExitNode() { + p.ExitNodeID = "" + p.ExitNodeIP = netaddr.IP{} +} + +// ExitNodeLocalIPError is returned when the requested IP address for an exit +// node belongs to the local machine. +type ExitNodeLocalIPError struct { + hostOrIP string +} + +func (e ExitNodeLocalIPError) Error() string { + return fmt.Sprintf("cannot use %s as an exit node as it is a local IP address to this machine", e.hostOrIP) +} + +func exitNodeIPOfArg(s string, st *ipnstate.Status) (ip netaddr.IP, err error) { + if s == "" { + return ip, os.ErrInvalid + } + ip, err = netaddr.ParseIP(s) + if err == nil { + // If we're online already and have a netmap, double check that the IP + // address specified is valid. + if st.BackendState == "Running" { + ps, ok := peerWithTailscaleIP(st, ip) + if !ok { + return ip, fmt.Errorf("no node found in netmap with IP %v", ip) + } + if !ps.ExitNodeOption { + return ip, fmt.Errorf("node %v is not advertising an exit node", ip) + } + } + if !isRemoteIP(st, ip) { + return ip, ExitNodeLocalIPError{s} + } + return ip, nil + } + match := 0 + for _, ps := range st.Peer { + baseName := dnsname.TrimSuffix(ps.DNSName, st.MagicDNSSuffix) + if !strings.EqualFold(s, baseName) { + continue + } + match++ + if len(ps.TailscaleIPs) == 0 { + return ip, fmt.Errorf("node %q has no Tailscale IP?", s) + } + if !ps.ExitNodeOption { + return ip, fmt.Errorf("node %q is not advertising an exit node", s) + } + ip = ps.TailscaleIPs[0] + } + switch match { + case 0: + return ip, fmt.Errorf("invalid value %q for --exit-node; must be IP or unique node name", s) + case 1: + if !isRemoteIP(st, ip) { + return ip, ExitNodeLocalIPError{s} + } + return ip, nil + default: + return ip, fmt.Errorf("ambiguous exit node name %q", s) + } +} + +// SetExitNodeIP validates and sets the ExitNodeIP from a user-provided string +// specifying either an IP address or a MagicDNS base name ("foo", as opposed to +// "foo.bar.beta.tailscale.net"). This method does not mutate ExitNodeID and +// will fail if ExitNodeID is already set. +func (p *Prefs) SetExitNodeIP(s string, st *ipnstate.Status) error { + if !p.ExitNodeID.IsZero() { + return ErrExitNodeIDAlreadySet + } + ip, err := exitNodeIPOfArg(s, st) + if err == nil { + p.ExitNodeIP = ip + } + return err +} + // PrefsFromBytes deserializes Prefs from a JSON blob. If // enforceDefaults is true, Prefs.RouteAll and Prefs.AllowSingleHosts // are forced on. diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index d9204b901..043f7e557 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -17,6 +17,7 @@ import ( "go4.org/mem" "inet.af/netaddr" + "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" "tailscale.com/tstest" "tailscale.com/types/key" @@ -679,3 +680,133 @@ func TestPrefsExitNode(t *testing.T) { t.Errorf("routes = %d; want %d", got, want) } } + +func TestExitNodeIPOfArg(t *testing.T) { + mustIP := netaddr.MustParseIP + tests := []struct { + name string + arg string + st *ipnstate.Status + want netaddr.IP + wantErr string + }{ + { + name: "ip_while_stopped_okay", + arg: "1.2.3.4", + st: &ipnstate.Status{ + BackendState: "Stopped", + }, + want: mustIP("1.2.3.4"), + }, + { + name: "ip_not_found", + arg: "1.2.3.4", + st: &ipnstate.Status{ + BackendState: "Running", + }, + wantErr: `no node found in netmap with IP 1.2.3.4`, + }, + { + name: "ip_not_exit", + arg: "1.2.3.4", + st: &ipnstate.Status{ + BackendState: "Running", + Peer: map[key.NodePublic]*ipnstate.PeerStatus{ + key.NewNode().Public(): { + TailscaleIPs: []netaddr.IP{mustIP("1.2.3.4")}, + }, + }, + }, + wantErr: `node 1.2.3.4 is not advertising an exit node`, + }, + { + name: "ip", + arg: "1.2.3.4", + st: &ipnstate.Status{ + BackendState: "Running", + Peer: map[key.NodePublic]*ipnstate.PeerStatus{ + key.NewNode().Public(): { + TailscaleIPs: []netaddr.IP{mustIP("1.2.3.4")}, + ExitNodeOption: true, + }, + }, + }, + want: mustIP("1.2.3.4"), + }, + { + name: "no_match", + arg: "unknown", + st: &ipnstate.Status{MagicDNSSuffix: ".foo"}, + wantErr: `invalid value "unknown" for --exit-node; must be IP or unique node name`, + }, + { + name: "name", + arg: "skippy", + st: &ipnstate.Status{ + MagicDNSSuffix: ".foo", + Peer: map[key.NodePublic]*ipnstate.PeerStatus{ + key.NewNode().Public(): { + DNSName: "skippy.foo.", + TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, + ExitNodeOption: true, + }, + }, + }, + want: mustIP("1.0.0.2"), + }, + { + name: "name_not_exit", + arg: "skippy", + st: &ipnstate.Status{ + MagicDNSSuffix: ".foo", + Peer: map[key.NodePublic]*ipnstate.PeerStatus{ + key.NewNode().Public(): { + DNSName: "skippy.foo.", + TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, + }, + }, + }, + wantErr: `node "skippy" is not advertising an exit node`, + }, + { + name: "ambiguous", + arg: "skippy", + st: &ipnstate.Status{ + MagicDNSSuffix: ".foo", + Peer: map[key.NodePublic]*ipnstate.PeerStatus{ + key.NewNode().Public(): { + DNSName: "skippy.foo.", + TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, + ExitNodeOption: true, + }, + key.NewNode().Public(): { + DNSName: "SKIPPY.foo.", + TailscaleIPs: []netaddr.IP{mustIP("1.0.0.2")}, + ExitNodeOption: true, + }, + }, + }, + wantErr: `ambiguous exit node name "skippy"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := exitNodeIPOfArg(tt.arg, tt.st) + if err != nil { + if err.Error() == tt.wantErr { + return + } + if tt.wantErr == "" { + t.Fatal(err) + } + t.Fatalf("error = %#q; want %#q", err, tt.wantErr) + } + if tt.wantErr != "" { + t.Fatalf("got %v; want error %#q", got, tt.wantErr) + } + if got != tt.want { + t.Fatalf("got %v; want %v", got, tt.want) + } + }) + } +}