cmd/tailscale/cli: validate advertised routes' IP address-vs-network bits

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

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)

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 {
+			ipp, err := netaddr.ParseIPPrefix(s) // parse it with other pawith both packages
-				log.Fatalf("%q is not a valid IP address or CIDR prefix", s)
+			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 {

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
 )

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=