From 3e493e0417abca7fb5648411d53d5a817732eae5 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Fri, 31 Jul 2020 12:40:49 -0700
Subject: [PATCH] wgengine: fix lazy wireguard config bug on sent packet
 minute+ later

A comparison operator was backwards.

The bad case went:

* device A send packet to B at t=1s
* B gets added to A's wireguard config
* B gets packet

(5 minutes pass)

* some other activity happens, causing B to expire
  to be removed from A's network map, since it's
  been over 5 minutes since sent or received activity
* device A sends packet to B at t=5m1s
* normally, B would get added back, but the old send
  time was not zero (we sent earlier!) and the time
  comparison was backwards, so we never regenerated
  the wireguard config.

This also refactors the code for legibility and moves constants up
top, with comments.
---
 wgengine/userspace.go | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index 544cc2ae2..dcfadd477 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -69,6 +69,16 @@ const (
 	// (This includes peers that have never been idle, which
 	// effectively have infinite idleness)
 	lazyPeerIdleThreshold = 5 * time.Minute
+
+	// packetSendTimeUpdateFrequency controls how often we record
+	// the time that we wrote a packet to an IP address.
+	packetSendTimeUpdateFrequency = 10 * time.Second
+
+	// packetSendRecheckWireguardThreshold controls how long we can go
+	// between packet sends to an IP before checking to see
+	// whether this IP address needs to be added back to the
+	// Wireguard peer oconfig.
+	packetSendRecheckWireguardThreshold = 1 * time.Minute
 )
 
 type userspaceEngine struct {
@@ -763,12 +773,19 @@ func (e *userspaceEngine) updateActivityMapsLocked(trackDisco []tailcfg.DiscoKey
 		if fn == nil {
 			// This is the func that gets run on every outgoing packet for tracked IPs:
 			fn = func() {
-				now, old := time.Now().Unix(), atomic.LoadInt64(timePtr)
-				if old > now-10 {
-					return
+				now := time.Now().Unix()
+				old := atomic.LoadInt64(timePtr)
+
+				// How long's it been since we last sent a packet?
+				// For our first packet, old is Unix epoch time 0 (1970).
+				elapsedSec := now - old
+
+				if elapsedSec >= int64(packetSendTimeUpdateFrequency/time.Second) {
+					atomic.StoreInt64(timePtr, now)
 				}
-				atomic.StoreInt64(timePtr, now)
-				if old == 0 || (now-old) <= 60 {
+				// On a big jump, assume we might no longer be in the wireguard
+				// config and go check.
+				if elapsedSec >= int64(packetSendRecheckWireguardThreshold/time.Second) {
 					e.wgLock.Lock()
 					defer e.wgLock.Unlock()
 					e.maybeReconfigWireguardLocked()