From 87cbc067c23278e8ed23927e2086376cd53652a1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 10 Aug 2020 08:10:15 -0700 Subject: [PATCH] cmd/tailscale/cli: validate advertised routes' IP address-vs-network bits Signed-off-by: Brad Fitzpatrick --- cmd/tailscale/cli/cli.go | 9 +++++++-- cmd/tailscale/cli/up.go | 31 ++++++++++++++++++------------- go.mod | 2 +- go.sum | 2 ++ 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/cmd/tailscale/cli/cli.go b/cmd/tailscale/cli/cli.go index ebb220591..ab8deb118 100644 --- a/cmd/tailscale/cli/cli.go +++ b/cmd/tailscale/cli/cli.go @@ -76,6 +76,11 @@ change in the future. return err } +func fatalf(format string, a ...interface{}) { + log.SetFlags(0) + log.Fatalf(format, a...) +} + var rootArgs struct { socket string } @@ -84,9 +89,9 @@ func connect(ctx context.Context) (net.Conn, *ipn.BackendClient, context.Context c, err := safesocket.Connect(rootArgs.socket, 41112) if err != nil { if runtime.GOOS != "windows" && rootArgs.socket == "" { - log.Fatalf("--socket cannot be empty") + fatalf("--socket cannot be empty") } - log.Fatalf("Failed to connect to connect to tailscaled. (safesocket.Connect: %v)\n", err) + fatalf("Failed to connect to connect to tailscaled. (safesocket.Connect: %v)\n", err) } clientToServer := func(b []byte) { ipn.WriteMsg(c, b) diff --git a/cmd/tailscale/cli/up.go b/cmd/tailscale/cli/up.go index f792cc31d..8c26d7ca2 100644 --- a/cmd/tailscale/cli/up.go +++ b/cmd/tailscale/cli/up.go @@ -18,6 +18,7 @@ import ( "github.com/peterbourgon/ff/v2/ffcli" "github.com/tailscale/wireguard-go/wgcfg" + "inet.af/netaddr" "tailscale.com/ipn" "tailscale.com/tailcfg" "tailscale.com/version" @@ -110,7 +111,7 @@ func isBSD(s string) bool { return s == "dragonfly" || s == "freebsd" || s == "netbsd" || s == "openbsd" } -func warning(format string, args ...interface{}) { +func warnf(format string, args ...interface{}) { fmt.Printf("Warning: "+format+"\n", args...) } @@ -129,16 +130,16 @@ func checkIPForwarding() { bs, err := exec.Command("sysctl", "-n", key).Output() if err != nil { - warning("couldn't check %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) + warnf("couldn't check %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) return } on, err := strconv.ParseBool(string(bytes.TrimSpace(bs))) if err != nil { - warning("couldn't parse %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) + warnf("couldn't parse %s (%v).\nSubnet routes won't work without IP forwarding.", key, err) return } if !on { - warning("%s is disabled. Subnet routes won't work.", key) + warnf("%s is disabled. Subnet routes won't work.", key) } } @@ -149,15 +150,19 @@ func runUp(ctx context.Context, args []string) error { var routes []wgcfg.CIDR if upArgs.advertiseRoutes != "" { - checkIPForwarding() advroutes := strings.Split(upArgs.advertiseRoutes, ",") for _, s := range advroutes { cidr, ok := parseIPOrCIDR(s) - if !ok { - log.Fatalf("%q is not a valid IP address or CIDR prefix", s) + ipp, err := netaddr.ParseIPPrefix(s) // parse it with other pawith both packages + if !ok || err != nil { + fatalf("%q is not a valid IP address or CIDR prefix", s) + } + if ipp != ipp.Masked() { + fatalf("%s has non-address bits set; expected %s", ipp, ipp.Masked()) } routes = append(routes, cidr) } + checkIPForwarding() } var tags []string @@ -166,13 +171,13 @@ func runUp(ctx context.Context, args []string) error { for _, tag := range tags { err := tailcfg.CheckTag(tag) if err != nil { - log.Fatalf("tag: %q: %s", tag, err) + fatalf("tag: %q: %s", tag, err) } } } if len(upArgs.hostname) > 256 { - log.Fatalf("hostname too long: %d bytes (max 256)", len(upArgs.hostname)) + fatalf("hostname too long: %d bytes (max 256)", len(upArgs.hostname)) } // TODO(apenwarr): fix different semantics between prefs and uflags @@ -195,12 +200,12 @@ func runUp(ctx context.Context, args []string) error { prefs.NetfilterMode = router.NetfilterOn case "nodivert": prefs.NetfilterMode = router.NetfilterNoDivert - warning("netfilter=nodivert; add iptables calls to ts-* chains manually.") + warnf("netfilter=nodivert; add iptables calls to ts-* chains manually.") case "off": prefs.NetfilterMode = router.NetfilterOff - warning("netfilter=off; configure iptables yourself.") + warnf("netfilter=off; configure iptables yourself.") default: - log.Fatalf("invalid value --netfilter-mode: %q", upArgs.netfilterMode) + fatalf("invalid value --netfilter-mode: %q", upArgs.netfilterMode) } } @@ -215,7 +220,7 @@ func runUp(ctx context.Context, args []string) error { AuthKey: upArgs.authKey, Notify: func(n ipn.Notify) { if n.ErrMessage != nil { - log.Fatalf("backend error: %v\n", *n.ErrMessage) + fatalf("backend error: %v\n", *n.ErrMessage) } if s := n.State; s != nil { switch *s { diff --git a/go.mod b/go.mod index b6b3562db..a833cc7c1 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,6 @@ require ( golang.org/x/time v0.0.0-20191024005414-555d28b269f0 golang.org/x/tools v0.0.0-20191216052735-49a3e744a425 honnef.co/go/tools v0.0.1-2020.1.4 - inet.af/netaddr v0.0.0-20200718043157-99321d6ad24c + inet.af/netaddr v0.0.0-20200810144936-56928fe48a98 rsc.io/goversion v1.2.0 ) diff --git a/go.sum b/go.sum index a7d689a41..3db39980e 100644 --- a/go.sum +++ b/go.sum @@ -168,5 +168,7 @@ honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= inet.af/netaddr v0.0.0-20200718043157-99321d6ad24c h1:si3Owrfem175Ry6gKqnh59eOXxDojyBTIHxUKuvK/Eo= inet.af/netaddr v0.0.0-20200718043157-99321d6ad24c/go.mod h1:qqYzz/2whtrbWJvt+DNWQyvekNN4ePQZcg2xc2/Yjww= +inet.af/netaddr v0.0.0-20200810144936-56928fe48a98 h1:bWyWDZP0l6VnQ1TDKf6yNwuiEDV6Q3q1Mv34m+lzT1I= +inet.af/netaddr v0.0.0-20200810144936-56928fe48a98/go.mod h1:qqYzz/2whtrbWJvt+DNWQyvekNN4ePQZcg2xc2/Yjww= rsc.io/goversion v1.2.0 h1:SPn+NLTiAG7w30IRK/DKp1BjvpWabYgxlLp/+kx5J8w= rsc.io/goversion v1.2.0/go.mod h1:Eih9y/uIBS3ulggl7KNJ09xGSLcuNaLgmvvqa07sgfo=