From 7a57ab179391949a8027cc14f60d15ecaea3a227 Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Sat, 14 Mar 2020 10:56:52 -0500 Subject: [PATCH 1/8] tailcfg: add a per-peer KeepAlive field --- tailcfg/tailcfg.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index cd2fef1bf..00ecaa74e 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -134,6 +134,8 @@ type Node struct { Created time.Time LastSeen *time.Time `json:",omitempty"` + KeepAlive bool // open and keep open a connection to this peer + MachineAuthorized bool // TODO(crawshaw): replace with MachineStatus // NOTE: any new fields containing pointers in this type From 290f83e9f64817b61151520e6fab244f46b842e0 Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Sat, 14 Mar 2020 11:03:00 -0500 Subject: [PATCH 2/8] tailcfg: fix test Signed-off-by: David Crawshaw --- tailcfg/tailcfg_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 32f30dbdb..3bca304c1 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -160,7 +160,7 @@ func TestHostinfoEqual(t *testing.T) { } func TestNodeEqual(t *testing.T) { - nodeHandles := []string{"ID", "Name", "User", "Key", "KeyExpiry", "Machine", "Addresses", "AllowedIPs", "Endpoints", "DERP", "Hostinfo", "Created", "LastSeen", "MachineAuthorized"} + nodeHandles := []string{"ID", "Name", "User", "Key", "KeyExpiry", "Machine", "Addresses", "AllowedIPs", "Endpoints", "DERP", "Hostinfo", "Created", "LastSeen", "KeepAlive", "MachineAuthorized"} if have := fieldsOf(reflect.TypeOf(Node{})); !reflect.DeepEqual(have, nodeHandles) { t.Errorf("Node.Equal check might be out of sync\nfields: %q\nhandled: %q\n", have, nodeHandles) From 8712164a0a6ea6d8b5abd69dc4348aa25de4662c Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Sat, 14 Mar 2020 10:57:12 -0500 Subject: [PATCH 3/8] controlclient: use per-peer KeepAlive signal Signed-off-by: David Crawshaw --- control/controlclient/netmap.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/control/controlclient/netmap.go b/control/controlclient/netmap.go index ac1c44081..7a5c36eea 100644 --- a/control/controlclient/netmap.go +++ b/control/controlclient/netmap.go @@ -16,7 +16,6 @@ import ( "github.com/tailscale/wireguard-go/wgcfg" "tailscale.com/tailcfg" - "tailscale.com/version" "tailscale.com/wgengine/filter" ) @@ -328,8 +327,7 @@ func (nm *NetworkMap) _WireGuardConfig(uflags int, dnsOverride []wgcfg.IP, allEn aips = append(aips, aip) } fmt.Fprintf(buf, "AllowedIPs = %s\n", strings.Join(aips, ", ")) - doKeepAlives := !version.IsMobile() - if doKeepAlives { + if peer.KeepAlive { fmt.Fprintf(buf, "PersistentKeepalive = 25\n") } } From 1b2be3f1c8fae3a3c0057356b1757544ad75879b Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Sat, 14 Mar 2020 14:07:22 -0500 Subject: [PATCH 4/8] controlclient: test peer keepalive directive Signed-off-by: David Crawshaw --- control/controlclient/auto_test.go | 47 ++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/control/controlclient/auto_test.go b/control/controlclient/auto_test.go index 7ebcfb302..d4a090a37 100644 --- a/control/controlclient/auto_test.go +++ b/control/controlclient/auto_test.go @@ -348,6 +348,53 @@ func TestControl(t *testing.T) { c3.checkNoStatus(t) c4.checkNoStatus(t) }) + + t.Run("set hostinfo", func(t *testing.T) { + c1.SetHostinfo(&tailcfg.Hostinfo{ + BackendLogID: "set-hostinfo-test", + OS: "linux", + }) + c1.waitStatus(t, stateSynchronized) + c2NetMap := c2.status(t).New.NetMap + if len(c2NetMap.Peers) != 1 { + t.Fatalf("wrong number of peers: %v", c2NetMap.Peers) + } + peer := c2NetMap.Peers[0] + if !peer.KeepAlive { + t.Errorf("peer KeepAlive=false, want true") + } + if peer.Hostinfo.OS != "linux" { + t.Errorf("peer hostinfo does not have OS: %v", peer.Hostinfo) + } + + c2.SetHostinfo(&tailcfg.Hostinfo{ + BackendLogID: "set-hostinfo-test", + OS: "iOS", + }) + c1NetMap = c1.status(t).New.NetMap + c2NetMap = c2.status(t).New.NetMap + if len(c1NetMap.Peers) != 1 { + t.Fatalf("wrong number of peers: %v", c1NetMap.Peers) + } + if len(c2NetMap.Peers) != 1 { + t.Fatalf("wrong number of peers: %v", c2NetMap.Peers) + } + peer = c1NetMap.Peers[0] + if peer.KeepAlive { + t.Errorf("peer KeepAlive=true, want false") + } + if peer.Hostinfo.OS != "iOS" { + t.Errorf("peer hostinfo does not have OS: %v", peer.Hostinfo) + } + peer = c2NetMap.Peers[0] + if peer.KeepAlive { + t.Errorf("peer KeepAlive=true, want false") + } + if peer.Hostinfo.OS != "linux" { + t.Errorf("peer hostinfo does not have OS: %v", peer.Hostinfo) + } + + }) } func hasStringsSuffix(list, suffix []string) bool { From d348b9450555e4de09affdbd2ab3ebcbfa9a074a Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 15 Mar 2020 22:19:45 -0700 Subject: [PATCH 5/8] stun, stunner: clarify an error log message more But two earlier changes mean this doesn't show up anymore anyway. But if it does, it'll be a nice message. --- stun/stun.go | 2 +- stunner/stunner.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stun/stun.go b/stun/stun.go index f0b114406..d6b6e7856 100644 --- a/stun/stun.go +++ b/stun/stun.go @@ -124,7 +124,7 @@ func ParseBindingRequest(b []byte) (TxID, error) { var ( ErrNotSTUN = errors.New("response is not a STUN packet") - ErrNotSuccessResponse = errors.New("STUN response error") + ErrNotSuccessResponse = errors.New("STUN packet is not a response") ErrMalformedAttrs = errors.New("STUN response has malformed attributes") ErrNotBindingRequest = errors.New("STUN request not a binding request") ErrWrongSoftware = errors.New("STUN request came from non-Tailscale software") diff --git a/stunner/stunner.go b/stunner/stunner.go index 00e8318a9..534f617f9 100644 --- a/stunner/stunner.go +++ b/stunner/stunner.go @@ -110,7 +110,7 @@ func (s *Stunner) Receive(p []byte, fromAddr *net.UDPAddr) { // check probe coming in late. Ignore. return } - s.logf("stunner: received bad STUN response: %v", err) + s.logf("stunner: received unexpected STUN message response from %v: %v", fromAddr, err) return } r, ok := s.removeTX(tx) From 5aafe0ee96d214353b0b48ac37624616bae35772 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 15 Mar 2020 22:27:36 -0700 Subject: [PATCH 6/8] cmd/tailscale: don't crash on too many non-flag args --- cmd/tailscale/tailscale.go | 3 +-- version/version.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/tailscale/tailscale.go b/cmd/tailscale/tailscale.go index 57db63be8..a7a8fef98 100644 --- a/cmd/tailscale/tailscale.go +++ b/cmd/tailscale/tailscale.go @@ -18,7 +18,6 @@ import ( "syscall" "github.com/apenwarr/fixconsole" - "github.com/pborman/getopt/v2" "github.com/peterbourgon/ff/v2/ffcli" "github.com/tailscale/wireguard-go/wgcfg" "tailscale.com/ipn" @@ -120,7 +119,7 @@ var upArgs = struct { func runUp(ctx context.Context, args []string) error { pol := logpolicy.New("tailnode.log.tailscale.io") if len(args) > 0 { - log.Fatalf("too many non-flag arguments: %#v", getopt.Args()[0]) + log.Fatalf("too many non-flag arguments: %q", args) } defer pol.Close() diff --git a/version/version.go b/version/version.go index 3d7537d5e..b35558a7d 100644 --- a/version/version.go +++ b/version/version.go @@ -7,5 +7,5 @@ // Package version provides the version that the binary was built at. package version -const LONG = "date.20200311" +const LONG = "date.20200315" const SHORT = LONG // TODO: unused; remove SHORT? Make it a func? From 8de67844bd9d639c04bda5dbc15982027d7e127f Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 15 Mar 2020 22:40:41 -0700 Subject: [PATCH 7/8] cmd/tailscale: make failure message when tailscaled down less technical --- cmd/tailscale/tailscale.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/tailscale/tailscale.go b/cmd/tailscale/tailscale.go index a7a8fef98..96751d5d3 100644 --- a/cmd/tailscale/tailscale.go +++ b/cmd/tailscale/tailscale.go @@ -148,7 +148,7 @@ func runUp(ctx context.Context, args []string) error { c, err := safesocket.Connect(upArgs.socket, 41112) if err != nil { - log.Fatalf("safesocket.Connect: %v\n", err) + log.Fatalf("Failed to connect to connect to tailscaled. (safesocket.Connect: %v)\n", err) } clientToServer := func(b []byte) { ipn.WriteMsg(c, b) From 8f9fa6a842b7f15f9ce14079e11ac78ae6847425 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 15 Mar 2020 22:41:50 -0700 Subject: [PATCH 8/8] logtail: minor style/simplification changes --- logtail/logtail.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/logtail/logtail.go b/logtail/logtail.go index 77c6ca023..f63948e48 100644 --- a/logtail/logtail.go +++ b/logtail/logtail.go @@ -169,7 +169,8 @@ func (l *logger) drainPending() (res []byte) { entries := 0 var batchDone bool - for buf.Len() < 1<<18 && !batchDone { + const maxLen = 256 << 10 + for buf.Len() < maxLen && !batchDone { b, err := l.buffer.TryReadLine() if err == io.EOF { break @@ -292,7 +293,7 @@ func (l *logger) upload(ctx context.Context, body []byte) (uploaded bool, err er if resp.StatusCode != 200 { uploaded = resp.StatusCode == 400 // the server saved the logs anyway b, _ := ioutil.ReadAll(resp.Body) - return uploaded, fmt.Errorf("log upload of %d bytes %s failed %d: %q", len(body), compressedNote, resp.StatusCode, string(b)) + return uploaded, fmt.Errorf("log upload of %d bytes %s failed %d: %q", len(body), compressedNote, resp.StatusCode, b) } return true, nil } @@ -402,10 +403,8 @@ func (l *logger) Write(buf []byte) (int, error) { } else { // The log package always line-terminates logs, // so this is an uncommon path. - bufnl := make([]byte, len(buf)+1) - copy(bufnl, buf) - bufnl[len(bufnl)-1] = '\n' - l.stderr.Write(bufnl) + withNL := append(buf[:len(buf):len(buf)], '\n') + l.stderr.Write(withNL) } } b := l.encode(buf)