Merge pull request #67 from tailscale/danderson/adv-routes

ipn: add AdvertiseRoutes to Prefs.

control/controlclient/direct.go

@@ -39,6 +39,21 @@ type Persist struct {
 	LoginName         string
 }
 
+func (p *Persist) Equals(p2 *Persist) bool {
+	if p == nil && p2 == nil {
+		return true
+	}
+	if p == nil || p2 == nil {
+		return false
+	}
+
+	return p.PrivateMachineKey.Equal(p2.PrivateMachineKey) &&
+		p.PrivateNodeKey.Equal(p2.PrivateNodeKey) &&
+		p.OldPrivateNodeKey.Equal(p2.OldPrivateNodeKey) &&
+		p.Provider == p2.Provider &&
+		p.LoginName == p2.LoginName
+}
+
 func (p *Persist) Pretty() string {
 	var mk, ok, nk wgcfg.Key
 	if !p.PrivateMachineKey.IsZero() {

control/controlclient/persist_test.go

@@ -0,0 +1,98 @@
+// Copyright (c) 2020 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 controlclient
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/tailscale/wireguard-go/wgcfg"
+)
+
+func TestPersistEqual(t *testing.T) {
+	persistHandles := []string{"PrivateMachineKey", "PrivateNodeKey", "OldPrivateNodeKey", "Provider", "LoginName"}
+	if have := fieldsOf(reflect.TypeOf(Persist{})); !reflect.DeepEqual(have, persistHandles) {
+		t.Errorf("Persist.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
+			have, persistHandles)
+	}
+
+	newPrivate := func() wgcfg.PrivateKey {
+		k, err := wgcfg.NewPrivateKey()
+		if err != nil {
+			panic(err)
+		}
+		return k
+	}
+	k1 := newPrivate()
+	tests := []struct {
+		a, b *Persist
+		want bool
+	}{
+		{nil, nil, true},
+		{nil, &Persist{}, false},
+		{&Persist{}, nil, false},
+		{&Persist{}, &Persist{}, true},
+
+		{
+			&Persist{PrivateMachineKey: k1},
+			&Persist{PrivateMachineKey: newPrivate()},
+			false,
+		},
+		{
+			&Persist{PrivateMachineKey: k1},
+			&Persist{PrivateMachineKey: k1},
+			true,
+		},
+
+		{
+			&Persist{PrivateNodeKey: k1},
+			&Persist{PrivateNodeKey: newPrivate()},
+			false,
+		},
+		{
+			&Persist{PrivateNodeKey: k1},
+			&Persist{PrivateNodeKey: k1},
+			true,
+		},
+
+		{
+			&Persist{OldPrivateNodeKey: k1},
+			&Persist{OldPrivateNodeKey: newPrivate()},
+			false,
+		},
+		{
+			&Persist{OldPrivateNodeKey: k1},
+			&Persist{OldPrivateNodeKey: k1},
+			true,
+		},
+
+		{
+			&Persist{Provider: "google"},
+			&Persist{Provider: "o365"},
+			false,
+		},
+		{
+			&Persist{Provider: "google"},
+			&Persist{Provider: "google"},
+			true,
+		},
+
+		{
+			&Persist{LoginName: "foo@tailscale.com"},
+			&Persist{LoginName: "bar@tailscale.com"},
+			false,
+		},
+		{
+			&Persist{LoginName: "foo@tailscale.com"},
+			&Persist{LoginName: "foo@tailscale.com"},
+			true,
+		},
+	}
+	for i, test := range tests {
+		if got := test.a.Equals(test.b); got != test.want {
+			t.Errorf("%d. Equals = %v; want %v", i, got, test.want)
+		}
+	}
+}

ipn/prefs.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"log"
+	"net"
 	"os"
 	"path/filepath"
 
@@ -17,13 +18,39 @@ import (
 	"tailscale.com/control/controlclient"
 )
 
+// Prefs are the user modifiable settings of the Tailscale node agent.
 type Prefs struct {
-	RouteAll         bool
+	// RouteAll specifies whether to accept subnet and default routes
+	// advertised by other nodes on the Tailscale network.
+	RouteAll bool
+	// AllowSingleHosts specifies whether to install routes for each
+	// node IP on the tailscale network, in addition to a route for
+	// the whole network.
+	//
+	// TODO(danderson): why do we have this? It dumps a lot of stuff
+	// into the routing table, and a single network route _should_ be
+	// all that we need. But when I turn this off in my tailscaled,
+	// packets stop flowing. What's up with that?
 	AllowSingleHosts bool
-	CorpDNS          bool
-	WantRunning      bool
-	NotepadURLs      bool
-	UsePacketFilter  bool
+	// CorpDNS specifies whether to install the Tailscale network's
+	// DNS configuration, if it exists.
+	CorpDNS bool
+	// WantRunning indicates whether networking should be active on
+	// this node.
+	WantRunning bool
+	// UsePacketFilter indicates whether to enforce centralized ACLs
+	// on this node. If false, all traffic in and out of this node is
+	// allowed.
+	UsePacketFilter bool
+	// AdvertiseRoutes specifies CIDR prefixes to advertise into the
+	// Tailscale network as reachable through the current node.
+	AdvertiseRoutes []*net.IPNet
+
+	// NotepadURLs is a debugging setting that opens OAuth URLs in
+	// notepad.exe on Windows, rather than loading them in a browser.
+	//
+	// TODO(danderson): remove?
+	NotepadURLs bool
 
 	// The Persist field is named 'Config' in the file for backward
 	// compatibility with earlier versions.
@@ -34,32 +61,57 @@ type Prefs struct {
 }
 
 // IsEmpty reports whether p is nil or pointing to a Prefs zero value.
-func (uc *Prefs) IsEmpty() bool { return uc == nil || *uc == Prefs{} }
+func (p *Prefs) IsEmpty() bool { return p == nil || p.Equals(&Prefs{}) }
 
-func (uc *Prefs) Pretty() string {
-	var ucp string
-	if uc.Persist != nil {
-		ucp = uc.Persist.Pretty()
+func (p *Prefs) Pretty() string {
+	var pp string
+	if p.Persist != nil {
+		pp = p.Persist.Pretty()
 	} else {
-		ucp = "Persist=nil"
+		pp = "Persist=nil"
 	}
-	return fmt.Sprintf("Prefs{ra=%v mesh=%v dns=%v want=%v notepad=%v pf=%v %v}",
-		uc.RouteAll, uc.AllowSingleHosts, uc.CorpDNS, uc.WantRunning,
-		uc.NotepadURLs, uc.UsePacketFilter, ucp)
+	return fmt.Sprintf("Prefs{ra=%v mesh=%v dns=%v want=%v notepad=%v pf=%v routes=%v %v}",
+		p.RouteAll, p.AllowSingleHosts, p.CorpDNS, p.WantRunning,
+		p.NotepadURLs, p.UsePacketFilter, p.AdvertiseRoutes, pp)
 }
 
-func (uc *Prefs) ToBytes() []byte {
-	data, err := json.MarshalIndent(uc, "", "\t")
+func (p *Prefs) ToBytes() []byte {
+	data, err := json.MarshalIndent(p, "", "\t")
 	if err != nil {
 		log.Fatalf("Prefs marshal: %v\n", err)
 	}
 	return data
 }
 
-func (uc *Prefs) Equals(uc2 *Prefs) bool {
-	b1 := uc.ToBytes()
-	b2 := uc2.ToBytes()
-	return bytes.Equal(b1, b2)
+func (p *Prefs) Equals(p2 *Prefs) bool {
+	if p == nil && p2 == nil {
+		return true
+	}
+	if p == nil || p2 == nil {
+		return false
+	}
+
+	return p != nil && p2 != nil &&
+		p.RouteAll == p2.RouteAll &&
+		p.AllowSingleHosts == p2.AllowSingleHosts &&
+		p.CorpDNS == p2.CorpDNS &&
+		p.WantRunning == p2.WantRunning &&
+		p.NotepadURLs == p2.NotepadURLs &&
+		p.UsePacketFilter == p2.UsePacketFilter &&
+		compareIPNets(p.AdvertiseRoutes, p2.AdvertiseRoutes) &&
+		p.Persist.Equals(p2.Persist)
+}
+
+func compareIPNets(a, b []*net.IPNet) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if !a[i].IP.Equal(b[i].IP) || !bytes.Equal(a[i].Mask, b[i].Mask) {
+			return false
+		}
+	}
+	return true
 }
 
 func NewPrefs() Prefs {
@@ -76,45 +128,45 @@ func NewPrefs() Prefs {
 }
 
 func PrefsFromBytes(b []byte, enforceDefaults bool) (Prefs, error) {
-	uc := NewPrefs()
+	p := NewPrefs()
 	if len(b) == 0 {
-		return uc, nil
+		return p, nil
 	}
 	persist := &controlclient.Persist{}
 	err := json.Unmarshal(b, persist)
 	if err == nil && (persist.Provider != "" || persist.LoginName != "") {
 		// old-style relaynode config; import it
-		uc.Persist = persist
+		p.Persist = persist
 	} else {
-		err = json.Unmarshal(b, &uc)
+		err = json.Unmarshal(b, &p)
 		if err != nil {
 			log.Printf("Prefs parse: %v: %v\n", err, b)
 		}
 	}
 	if enforceDefaults {
-		uc.RouteAll = true
-		uc.AllowSingleHosts = true
+		p.RouteAll = true
+		p.AllowSingleHosts = true
 	}
-	return uc, err
+	return p, err
 }
 
-func (uc *Prefs) Copy() *Prefs {
-	uc2, err := PrefsFromBytes(uc.ToBytes(), false)
+func (p *Prefs) Copy() *Prefs {
+	p2, err := PrefsFromBytes(p.ToBytes(), false)
 	if err != nil {
 		log.Fatalf("Prefs was uncopyable: %v\n", err)
 	}
-	return &uc2
+	return &p2
 }
 
 func LoadPrefs(filename string, enforceDefaults bool) *Prefs {
 	log.Printf("Loading prefs %v\n", filename)
 	data, err := ioutil.ReadFile(filename)
-	uc := NewPrefs()
+	p := NewPrefs()
 	if err != nil {
 		log.Printf("Read: %v: %v\n", filename, err)
 		goto fail
 	}
-	uc, err = PrefsFromBytes(data, enforceDefaults)
+	p, err = PrefsFromBytes(data, enforceDefaults)
 	if err != nil {
 		log.Printf("Parse: %v: %v\n", filename, err)
 		goto fail
@@ -122,8 +174,8 @@ func LoadPrefs(filename string, enforceDefaults bool) *Prefs {
 	goto post
 fail:
 	log.Printf("failed to load config. Generating a new one.\n")
-	uc = NewPrefs()
-	uc.WantRunning = true
+	p = NewPrefs()
+	p.WantRunning = true
 post:
 	// Update: we changed our minds :)
 	// Versabank would like to persist the setting across reboots, for now,
@@ -136,15 +188,15 @@ post:
 		// know how, rebooting will fix it.
 		// We still persist WantRunning just in case we change our minds on
 		// this topic.
-		uc.WantRunning = true
+		p.WantRunning = true
 	}
-	log.Printf("Loaded prefs %v %v\n", filename, uc.Pretty())
-	return &uc
+	log.Printf("Loaded prefs %v %v\n", filename, p.Pretty())
+	return &p
 }
 
-func SavePrefs(filename string, uc *Prefs) {
-	log.Printf("Saving prefs %v %v\n", filename, uc.Pretty())
-	data := uc.ToBytes()
+func SavePrefs(filename string, p *Prefs) {
+	log.Printf("Saving prefs %v %v\n", filename, p.Pretty())
+	data := p.ToBytes()
 	os.MkdirAll(filepath.Dir(filename), 0700)
 	if err := atomicfile.WriteFile(filename, data, 0666); err != nil {
 		log.Printf("SavePrefs: %v\n", err)

ipn/prefs_test.go

@@ -5,11 +5,168 @@
 package ipn
 
 import (
+	"net"
+	"reflect"
 	"testing"
 
 	"tailscale.com/control/controlclient"
 )
 
+func fieldsOf(t reflect.Type) (fields []string) {
+	for i := 0; i < t.NumField(); i++ {
+		fields = append(fields, t.Field(i).Name)
+	}
+	return
+}
+
+func TestPrefsEqual(t *testing.T) {
+	prefsHandles := []string{"RouteAll", "AllowSingleHosts", "CorpDNS", "WantRunning", "UsePacketFilter", "AdvertiseRoutes", "NotepadURLs", "Persist"}
+	if have := fieldsOf(reflect.TypeOf(Prefs{})); !reflect.DeepEqual(have, prefsHandles) {
+		t.Errorf("Prefs.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
+			have, prefsHandles)
+	}
+
+	nets := func(strs ...string) (ns []*net.IPNet) {
+		for _, s := range strs {
+			_, n, err := net.ParseCIDR(s)
+			if err != nil {
+				panic(err)
+			}
+			ns = append(ns, n)
+		}
+		return ns
+	}
+	tests := []struct {
+		a, b *Prefs
+		want bool
+	}{
+		{
+			&Prefs{},
+			nil,
+			false,
+		},
+		{
+			nil,
+			&Prefs{},
+			false,
+		},
+		{
+			&Prefs{},
+			&Prefs{},
+			true,
+		},
+
+		{
+			&Prefs{RouteAll: true},
+			&Prefs{RouteAll: false},
+			false,
+		},
+		{
+			&Prefs{RouteAll: true},
+			&Prefs{RouteAll: true},
+			true,
+		},
+
+		{
+			&Prefs{AllowSingleHosts: true},
+			&Prefs{AllowSingleHosts: false},
+			false,
+		},
+		{
+			&Prefs{AllowSingleHosts: true},
+			&Prefs{AllowSingleHosts: true},
+			true,
+		},
+
+		{
+			&Prefs{CorpDNS: true},
+			&Prefs{CorpDNS: false},
+			false,
+		},
+		{
+			&Prefs{CorpDNS: true},
+			&Prefs{CorpDNS: true},
+			true,
+		},
+
+		{
+			&Prefs{WantRunning: true},
+			&Prefs{WantRunning: false},
+			false,
+		},
+		{
+			&Prefs{WantRunning: true},
+			&Prefs{WantRunning: true},
+			true,
+		},
+
+		{
+			&Prefs{NotepadURLs: true},
+			&Prefs{NotepadURLs: false},
+			false,
+		},
+		{
+			&Prefs{NotepadURLs: true},
+			&Prefs{NotepadURLs: true},
+			true,
+		},
+
+		{
+			&Prefs{UsePacketFilter: true},
+			&Prefs{UsePacketFilter: false},
+			false,
+		},
+		{
+			&Prefs{UsePacketFilter: true},
+			&Prefs{UsePacketFilter: true},
+			true,
+		},
+
+		{
+			&Prefs{AdvertiseRoutes: nil},
+			&Prefs{AdvertiseRoutes: []*net.IPNet{}},
+			true,
+		},
+		{
+			&Prefs{AdvertiseRoutes: []*net.IPNet{}},
+			&Prefs{AdvertiseRoutes: []*net.IPNet{}},
+			true,
+		},
+		{
+			&Prefs{AdvertiseRoutes: nets("192.168.0.0/24", "10.1.0.0/16")},
+			&Prefs{AdvertiseRoutes: nets("192.168.1.0/24", "10.2.0.0/16")},
+			false,
+		},
+		{
+			&Prefs{AdvertiseRoutes: nets("192.168.0.0/24", "10.1.0.0/16")},
+			&Prefs{AdvertiseRoutes: nets("192.168.0.0/24", "10.2.0.0/16")},
+			false,
+		},
+		{
+			&Prefs{AdvertiseRoutes: nets("192.168.0.0/24", "10.1.0.0/16")},
+			&Prefs{AdvertiseRoutes: nets("192.168.0.0/24", "10.1.0.0/16")},
+			true,
+		},
+
+		{
+			&Prefs{Persist: &controlclient.Persist{}},
+			&Prefs{Persist: &controlclient.Persist{LoginName: "dave"}},
+			false,
+		},
+		{
+			&Prefs{Persist: &controlclient.Persist{LoginName: "dave"}},
+			&Prefs{Persist: &controlclient.Persist{LoginName: "dave"}},
+			true,
+		},
+	}
+	for i, tt := range tests {
+		got := tt.a.Equals(tt.b)
+		if got != tt.want {
+			t.Errorf("%d. Equal = %v; want %v", i, got, tt.want)
+		}
+	}
+}
+
 func checkPrefs(t *testing.T, p Prefs) {
 	var err error
 	var p2, p2c Prefs
@@ -29,7 +186,7 @@ func checkPrefs(t *testing.T, p Prefs) {
 	}
 	p2 = p
 	p2.RouteAll = true
-	if p == p2 {
+	if p.Equals(&p2) {
 		t.Fatalf("p == p2\n")
 	}
 	p2b, err = PrefsFromBytes(p2.ToBytes(), false)