VERSION.txt: this is v1.76.6

Signed-off-by: Jonathan Nobels <jonathan@tailscale.com>

VERSION.txt

@@ -1 +1 @@
-1.75.0
+1.76.6

health/health.go

@@ -1051,11 +1051,15 @@ func (t *Tracker) updateBuiltinWarnablesLocked() {
 				ArgDuration:       d.Round(time.Second).String(),
 			})
 		}
-	} else {
+	} else if homeDERP != 0 {
 		t.setUnhealthyLocked(noDERPConnectionWarnable, Args{
 			ArgDERPRegionID:   fmt.Sprint(homeDERP),
 			ArgDERPRegionName: t.derpRegionNameLocked(homeDERP),
 		})
+	} else {
+		// No DERP home yet determined yet. There's probably some
+		// other problem or things are just starting up.
+		t.setHealthyLocked(noDERPConnectionWarnable)
 	}
 
 	if !t.ipnWantRunning {

net/dns/resolver/forwarder.go

@@ -487,6 +487,10 @@ func (f *forwarder) sendDoH(ctx context.Context, urlBase string, c *http.Client,
 	defer hres.Body.Close()
 	if hres.StatusCode != 200 {
 		metricDNSFwdDoHErrorStatus.Add(1)
+		if hres.StatusCode/100 == 5 {
+			// Translate 5xx HTTP server errors into SERVFAIL DNS responses.
+			return nil, fmt.Errorf("%w: %s", errServerFailure, hres.Status)
+		}
 		return nil, errors.New(hres.Status)
 	}
 	if ct := hres.Header.Get("Content-Type"); ct != dohType {
@@ -916,10 +920,7 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo
 		metricDNSFwdDropBonjour.Add(1)
 		res, err := nxDomainResponse(query)
 		if err != nil {
-			f.logf("error parsing bonjour query: %v", err)
-			// Returning an error will cause an internal retry, there is
-			// nothing we can do if parsing failed. Just drop the packet.
-			return nil
+			return err
 		}
 		select {
 		case <-ctx.Done():
@@ -951,10 +952,7 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo
 
 			res, err := servfailResponse(query)
 			if err != nil {
-				f.logf("building servfail response: %v", err)
-				// Returning an error will cause an internal retry, there is
-				// nothing we can do if parsing failed. Just drop the packet.
-				return nil
+				return err
 			}
 			select {
 			case <-ctx.Done():
@@ -1053,6 +1051,7 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo
 						if verboseDNSForward() {
 							f.logf("forwarder response(%d, %v, %d) = %d, %v", fq.txid, typ, len(domain), len(res.bs), firstErr)
 						}
+						return nil
 					}
 				}
 				return firstErr

net/dns/resolver/forwarder_test.go

@@ -7,7 +7,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/binary"
-	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -450,7 +449,7 @@ func makeLargeResponse(tb testing.TB, domain string) (request, response []byte)
 	return
 }
 
-func runTestQuery(tb testing.TB, port uint16, request []byte, modify func(*forwarder)) ([]byte, error) {
+func runTestQuery(tb testing.TB, request []byte, modify func(*forwarder), ports ...uint16) ([]byte, error) {
 	netMon, err := netmon.New(tb.Logf)
 	if err != nil {
 		tb.Fatal(err)
@@ -464,8 +463,9 @@ func runTestQuery(tb testing.TB, port uint16, request []byte, modify func(*forwa
 		modify(fwd)
 	}
 
-	rr := resolverAndDelay{
-		name: &dnstype.Resolver{Addr: fmt.Sprintf("127.0.0.1:%d", port)},
+	resolvers := make([]resolverAndDelay, len(ports))
+	for i, port := range ports {
+		resolvers[i].name = &dnstype.Resolver{Addr: fmt.Sprintf("127.0.0.1:%d", port)}
 	}
 
 	rpkt := packet{
@@ -477,7 +477,7 @@ func runTestQuery(tb testing.TB, port uint16, request []byte, modify func(*forwa
 	rchan := make(chan packet, 1)
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	tb.Cleanup(cancel)
-	err = fwd.forwardWithDestChan(ctx, rpkt, rchan, rr)
+	err = fwd.forwardWithDestChan(ctx, rpkt, rchan, resolvers...)
 	select {
 	case res := <-rchan:
 		return res.bs, err
@@ -486,8 +486,62 @@ func runTestQuery(tb testing.TB, port uint16, request []byte, modify func(*forwa
 	}
 }
 
-func mustRunTestQuery(tb testing.TB, port uint16, request []byte, modify func(*forwarder)) []byte {
-	resp, err := runTestQuery(tb, port, request, modify)
+// makeTestRequest returns a new TypeA request for the given domain.
+func makeTestRequest(tb testing.TB, domain string) []byte {
+	tb.Helper()
+	name := dns.MustNewName(domain)
+	builder := dns.NewBuilder(nil, dns.Header{})
+	builder.StartQuestions()
+	builder.Question(dns.Question{
+		Name:  name,
+		Type:  dns.TypeA,
+		Class: dns.ClassINET,
+	})
+	request, err := builder.Finish()
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return request
+}
+
+// makeTestResponse returns a new Type A response for the given domain,
+// with the specified status code and zero or more addresses.
+func makeTestResponse(tb testing.TB, domain string, code dns.RCode, addrs ...netip.Addr) []byte {
+	tb.Helper()
+	name := dns.MustNewName(domain)
+	builder := dns.NewBuilder(nil, dns.Header{
+		Response:      true,
+		Authoritative: true,
+		RCode:         code,
+	})
+	builder.StartQuestions()
+	q := dns.Question{
+		Name:  name,
+		Type:  dns.TypeA,
+		Class: dns.ClassINET,
+	}
+	builder.Question(q)
+	if len(addrs) > 0 {
+		builder.StartAnswers()
+		for _, addr := range addrs {
+			builder.AResource(dns.ResourceHeader{
+				Name:  q.Name,
+				Class: q.Class,
+				TTL:   120,
+			}, dns.AResource{
+				A: addr.As4(),
+			})
+		}
+	}
+	response, err := builder.Finish()
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return response
+}
+
+func mustRunTestQuery(tb testing.TB, request []byte, modify func(*forwarder), ports ...uint16) []byte {
+	resp, err := runTestQuery(tb, request, modify, ports...)
 	if err != nil {
 		tb.Fatalf("error making request: %v", err)
 	}
@@ -516,7 +570,7 @@ func TestForwarderTCPFallback(t *testing.T) {
 		}
 	})
 
-	resp := mustRunTestQuery(t, port, request, nil)
+	resp := mustRunTestQuery(t, request, nil, port)
 	if !bytes.Equal(resp, largeResponse) {
 		t.Errorf("invalid response\ngot: %+v\nwant: %+v", resp, largeResponse)
 	}
@@ -554,7 +608,7 @@ func TestForwarderTCPFallbackTimeout(t *testing.T) {
 		}
 	})
 
-	resp := mustRunTestQuery(t, port, request, nil)
+	resp := mustRunTestQuery(t, request, nil, port)
 	if !bytes.Equal(resp, largeResponse) {
 		t.Errorf("invalid response\ngot: %+v\nwant: %+v", resp, largeResponse)
 	}
@@ -585,11 +639,11 @@ func TestForwarderTCPFallbackDisabled(t *testing.T) {
 		}
 	})
 
-	resp := mustRunTestQuery(t, port, request, func(fwd *forwarder) {
+	resp := mustRunTestQuery(t, request, func(fwd *forwarder) {
 		// Disable retries for this test.
 		fwd.controlKnobs = &controlknobs.Knobs{}
 		fwd.controlKnobs.DisableDNSForwarderTCPRetries.Store(true)
-	})
+	}, port)
 
 	wantResp := append([]byte(nil), largeResponse[:maxResponseBytes]...)
 
@@ -613,41 +667,10 @@ func TestForwarderTCPFallbackError(t *testing.T) {
 	const domain = "error-response.tailscale.com."
 
 	// Our response is a SERVFAIL
-	response := func() []byte {
-		name := dns.MustNewName(domain)
-
-		builder := dns.NewBuilder(nil, dns.Header{
-			Response: true,
-			RCode:    dns.RCodeServerFailure,
-		})
-		builder.StartQuestions()
-		builder.Question(dns.Question{
-			Name:  name,
-			Type:  dns.TypeA,
-			Class: dns.ClassINET,
-		})
-		response, err := builder.Finish()
-		if err != nil {
-			t.Fatal(err)
-		}
-		return response
-	}()
+	response := makeTestResponse(t, domain, dns.RCodeServerFailure)
 
 	// Our request is a single A query for the domain in the answer, above.
-	request := func() []byte {
-		builder := dns.NewBuilder(nil, dns.Header{})
-		builder.StartQuestions()
-		builder.Question(dns.Question{
-			Name:  dns.MustNewName(domain),
-			Type:  dns.TypeA,
-			Class: dns.ClassINET,
-		})
-		request, err := builder.Finish()
-		if err != nil {
-			t.Fatal(err)
-		}
-		return request
-	}()
+	request := makeTestRequest(t, domain)
 
 	var sawRequest atomic.Bool
 	port := runDNSServer(t, nil, response, func(isTCP bool, gotRequest []byte) {
@@ -657,14 +680,141 @@ func TestForwarderTCPFallbackError(t *testing.T) {
 		}
 	})
 
-	_, err := runTestQuery(t, port, request, nil)
+	resp, err := runTestQuery(t, request, nil, port)
 	if !sawRequest.Load() {
 		t.Error("did not see DNS request")
 	}
-	if err == nil {
-		t.Error("wanted error, got nil")
-	} else if !errors.Is(err, errServerFailure) {
-		t.Errorf("wanted errServerFailure, got: %v", err)
+	if err != nil {
+		t.Fatalf("wanted nil, got %v", err)
+	}
+	var parser dns.Parser
+	respHeader, err := parser.Start(resp)
+	if err != nil {
+		t.Fatalf("parser.Start() failed: %v", err)
+	}
+	if got, want := respHeader.RCode, dns.RCodeServerFailure; got != want {
+		t.Errorf("wanted %v, got %v", want, got)
+	}
+}
+
+// Test to ensure that if we have more than one resolver, and at least one of them
+// returns a successful response, we propagate it.
+func TestForwarderWithManyResolvers(t *testing.T) {
+	enableDebug(t)
+
+	const domain = "example.com."
+	request := makeTestRequest(t, domain)
+
+	tests := []struct {
+		name          string
+		responses     [][]byte // upstream responses
+		wantResponses [][]byte // we should receive one of these from the forwarder
+	}{
+		{
+			name: "Success",
+			responses: [][]byte{ // All upstream servers returned successful, but different, response.
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.2")),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.3")),
+			},
+			wantResponses: [][]byte{ // We may forward whichever response is received first.
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.2")),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.3")),
+			},
+		},
+		{
+			name: "ServFail",
+			responses: [][]byte{ // All upstream servers returned a SERVFAIL.
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+			},
+			wantResponses: [][]byte{
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+			},
+		},
+		{
+			name: "ServFail+Success",
+			responses: [][]byte{ // All upstream servers fail except for one.
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+			},
+			wantResponses: [][]byte{ // We should forward the successful response.
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+			},
+		},
+		{
+			name: "NXDomain",
+			responses: [][]byte{ // All upstream servers returned NXDOMAIN.
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeNameError),
+			},
+			wantResponses: [][]byte{
+				makeTestResponse(t, domain, dns.RCodeNameError),
+			},
+		},
+		{
+			name: "NXDomain+Success",
+			responses: [][]byte{ // All upstream servers returned NXDOMAIN except for one.
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+			},
+			wantResponses: [][]byte{ // However, only SERVFAIL are considered to be errors. Therefore, we may forward any response.
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+			},
+		},
+		{
+			name: "Refused",
+			responses: [][]byte{ // All upstream servers return different failures.
+				makeTestResponse(t, domain, dns.RCodeRefused),
+				makeTestResponse(t, domain, dns.RCodeRefused),
+				makeTestResponse(t, domain, dns.RCodeRefused),
+				makeTestResponse(t, domain, dns.RCodeRefused),
+				makeTestResponse(t, domain, dns.RCodeRefused),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+			},
+			wantResponses: [][]byte{ // Refused is not considered to be an error and can be forwarded.
+				makeTestResponse(t, domain, dns.RCodeRefused),
+				makeTestResponse(t, domain, dns.RCodeSuccess, netip.MustParseAddr("127.0.0.1")),
+			},
+		},
+		{
+			name: "MixFail",
+			responses: [][]byte{ // All upstream servers return different failures.
+				makeTestResponse(t, domain, dns.RCodeServerFailure),
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeRefused),
+			},
+			wantResponses: [][]byte{ // Both NXDomain and Refused can be forwarded.
+				makeTestResponse(t, domain, dns.RCodeNameError),
+				makeTestResponse(t, domain, dns.RCodeRefused),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ports := make([]uint16, len(tt.responses))
+			for i := range tt.responses {
+				ports[i] = runDNSServer(t, nil, tt.responses[i], func(isTCP bool, gotRequest []byte) {})
+			}
+			gotResponse, err := runTestQuery(t, request, nil, ports...)
+			if err != nil {
+				t.Fatalf("wanted nil, got %v", err)
+			}
+			responseOk := slices.ContainsFunc(tt.wantResponses, func(wantResponse []byte) bool {
+				return slices.Equal(gotResponse, wantResponse)
+			})
+			if !responseOk {
+				t.Errorf("invalid response\ngot: %+v\nwant: %+v", gotResponse, tt.wantResponses[0])
+			}
+		})
 	}
 }
 
@@ -713,7 +863,7 @@ func TestNXDOMAINIncludesQuestion(t *testing.T) {
 	port := runDNSServer(t, nil, response, func(isTCP bool, gotRequest []byte) {
 	})
 
-	res, err := runTestQuery(t, port, request, nil)
+	res, err := runTestQuery(t, request, nil, port)
 	if err != nil {
 		t.Fatal(err)
 	}

net/dns/resolver/tsdns.go

@@ -321,15 +321,7 @@ func (r *Resolver) Query(ctx context.Context, bs []byte, family string, from net
 		defer cancel()
 		err = r.forwarder.forwardWithDestChan(ctx, packet{bs, family, from}, responses)
 		if err != nil {
-			select {
-			// Best effort: use any error response sent by forwardWithDestChan.
-			// This is present in some errors paths, such as when all upstream
-			// DNS servers replied with an error.
-			case resp := <-responses:
-				return resp.bs, err
-			default:
-				return nil, err
-			}
+			return nil, err
 		}
 		return (<-responses).bs, nil
 	}

net/dns/resolver/tsdns_test.go

@@ -1503,8 +1503,8 @@ func TestServfail(t *testing.T) {
 	r.SetConfig(cfg)
 
 	pkt, err := syncRespond(r, dnspacket("test.site.", dns.TypeA, noEdns))
-	if !errors.Is(err, errServerFailure) {
-		t.Errorf("err = %v, want %v", err, errServerFailure)
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
 	}
 
 	wantPkt := []byte{

net/netcheck/netcheck.go

@@ -391,10 +391,11 @@ type probePlan map[string][]probe
 // sortRegions returns the regions of dm first sorted
 // from fastest to slowest (based on the 'last' report),
 // end in regions that have no data.
-func sortRegions(dm *tailcfg.DERPMap, last *Report) (prev []*tailcfg.DERPRegion) {
+func sortRegions(dm *tailcfg.DERPMap, last *Report, preferredDERP int) (prev []*tailcfg.DERPRegion) {
 	prev = make([]*tailcfg.DERPRegion, 0, len(dm.Regions))
 	for _, reg := range dm.Regions {
-		if reg.Avoid {
+		// include an otherwise avoid region if it is the current preferred region
+		if reg.Avoid && reg.RegionID != preferredDERP {
 			continue
 		}
 		prev = append(prev, reg)
@@ -419,9 +420,19 @@ func sortRegions(dm *tailcfg.DERPMap, last *Report) (prev []*tailcfg.DERPRegion)
 // a full report, all regions are scanned.)
 const numIncrementalRegions = 3
 
-// makeProbePlan generates the probe plan for a DERPMap, given the most
-// recent report and whether IPv6 is configured on an interface.
-func makeProbePlan(dm *tailcfg.DERPMap, ifState *netmon.State, last *Report) (plan probePlan) {
+// makeProbePlan generates the probe plan for a DERPMap, given the most recent
+// report and the current home DERP. preferredDERP is passed independently of
+// last (report) because last is currently nil'd to indicate a desire for a full
+// netcheck.
+//
+// TODO(raggi,jwhited): refactor the callers and this function to be more clear
+// about full vs. incremental netchecks, and remove the need for the history
+// hiding. This was avoided in an incremental change due to exactly this kind of
+// distant coupling.
+// TODO(raggi): change from "preferred DERP" from a historical report to "home
+// DERP" as in what DERP is the current home connection, this would further
+// reduce flap events.
+func makeProbePlan(dm *tailcfg.DERPMap, ifState *netmon.State, last *Report, preferredDERP int) (plan probePlan) {
 	if last == nil || len(last.RegionLatency) == 0 {
 		return makeProbePlanInitial(dm, ifState)
 	}
@@ -432,9 +443,34 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *netmon.State, last *Report) (pl
 	had4 := len(last.RegionV4Latency) > 0
 	had6 := len(last.RegionV6Latency) > 0
 	hadBoth := have6if && had4 && had6
-	for ri, reg := range sortRegions(dm, last) {
-		if ri == numIncrementalRegions {
-			break
+	// #13969 ensure that the home region is always probed.
+	// If a netcheck has unstable latency, such as a user with large amounts of
+	// bufferbloat or a highly congested connection, there are cases where a full
+	// netcheck may observe a one-off high latency to the current home DERP. Prior
+	// to the forced inclusion of the home DERP, this would result in an
+	// incremental netcheck following such an event to cause a home DERP move, with
+	// restoration back to the home DERP on the next full netcheck ~5 minutes later
+	// - which is highly disruptive when it causes shifts in geo routed subnet
+	// routers. By always including the home DERP in the incremental netcheck, we
+	// ensure that the home DERP is always probed, even if it observed a recenet
+	// poor latency sample. This inclusion enables the latency history checks in
+	// home DERP selection to still take effect.
+	// planContainsHome indicates whether the home DERP has been added to the probePlan,
+	// if there is no prior home, then there's no home to additionally include.
+	planContainsHome := preferredDERP == 0
+	for ri, reg := range sortRegions(dm, last, preferredDERP) {
+		regIsHome := reg.RegionID == preferredDERP
+		if ri >= numIncrementalRegions {
+			// planned at least numIncrementalRegions regions and that includes the
+			// last home region (or there was none), plan complete.
+			if planContainsHome {
+				break
+			}
+			// planned at least numIncrementalRegions regions, but not the home region,
+			// check if this is the home region, if not, skip it.
+			if !regIsHome {
+				continue
+			}
 		}
 		var p4, p6 []probe
 		do4 := have4if
@@ -445,7 +481,7 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *netmon.State, last *Report) (pl
 		tries := 1
 		isFastestTwo := ri < 2
 
-		if isFastestTwo {
+		if isFastestTwo || regIsHome {
 			tries = 2
 		} else if hadBoth {
 			// For dual stack machines, make the 3rd & slower nodes alternate
@@ -456,14 +492,15 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *netmon.State, last *Report) (pl
 				do4, do6 = false, true
 			}
 		}
-		if !isFastestTwo && !had6 {
+		if !regIsHome && !isFastestTwo && !had6 {
 			do6 = false
 		}
 
-		if reg.RegionID == last.PreferredDERP {
+		if regIsHome {
 			// But if we already had a DERP home, try extra hard to
 			// make sure it's there so we don't flip flop around.
 			tries = 4
+			planContainsHome = true
 		}
 
 		for try := 0; try < tries; try++ {
@@ -788,9 +825,10 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
 	c.curState = rs
 	last := c.last
 
-	// Even if we're doing a non-incremental update, we may want to try our
-	// preferred DERP region for captive portal detection. Save that, if we
-	// have it.
+	// Extract preferredDERP from the last report, if available. This will be used
+	// in captive portal detection and DERP flapping suppression. Ideally this would
+	// be the current active home DERP rather than the last report preferred DERP,
+	// but only the latter is presently available.
 	var preferredDERP int
 	if last != nil {
 		preferredDERP = last.PreferredDERP
@@ -847,7 +885,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
 
 	var plan probePlan
 	if opts == nil || !opts.OnlyTCP443 {
-		plan = makeProbePlan(dm, ifState, last)
+		plan = makeProbePlan(dm, ifState, last, preferredDERP)
 	}
 
 	// If we're doing a full probe, also check for a captive portal. We
@@ -940,7 +978,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
 			}
 		}
 		if len(need) > 0 {
-			if !opts.OnlyTCP443 {
+			if opts == nil || !opts.OnlyTCP443 {
 				// Kick off ICMP in parallel to HTTPS checks; we don't
 				// reuse the same WaitGroup for those probes because we
 				// need to close the underlying Pinger after a timeout

net/netcheck/netcheck_test.go

@@ -576,6 +576,40 @@ func TestMakeProbePlan(t *testing.T) {
 				"region-3-v4": []probe{p("3a", 4)},
 			},
 		},
+		{
+			// #13969: ensure that the prior/current home region is always included in
+			// probe plans, so that we don't flap between regions due to a single major
+			// netcheck having excluded the home region due to a spuriously high sample.
+			name:    "ensure_home_region_inclusion",
+			dm:      basicMap,
+			have6if: true,
+			last: &Report{
+				RegionLatency: map[int]time.Duration{
+					1: 50 * time.Millisecond,
+					2: 20 * time.Millisecond,
+					3: 30 * time.Millisecond,
+					4: 40 * time.Millisecond,
+				},
+				RegionV4Latency: map[int]time.Duration{
+					1: 50 * time.Millisecond,
+					2: 20 * time.Millisecond,
+				},
+				RegionV6Latency: map[int]time.Duration{
+					3: 30 * time.Millisecond,
+					4: 40 * time.Millisecond,
+				},
+				PreferredDERP: 1,
+			},
+			want: probePlan{
+				"region-1-v4": []probe{p("1a", 4), p("1a", 4, 60*ms), p("1a", 4, 220*ms), p("1a", 4, 330*ms)},
+				"region-1-v6": []probe{p("1a", 6), p("1a", 6, 60*ms), p("1a", 6, 220*ms), p("1a", 6, 330*ms)},
+				"region-2-v4": []probe{p("2a", 4), p("2b", 4, 24*ms)},
+				"region-2-v6": []probe{p("2a", 6), p("2b", 6, 24*ms)},
+				"region-3-v4": []probe{p("3a", 4), p("3b", 4, 36*ms)},
+				"region-3-v6": []probe{p("3a", 6), p("3b", 6, 36*ms)},
+				"region-4-v4": []probe{p("4a", 4)},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -583,7 +617,11 @@ func TestMakeProbePlan(t *testing.T) {
 				HaveV6: tt.have6if,
 				HaveV4: !tt.no4,
 			}
-			got := makeProbePlan(tt.dm, ifState, tt.last)
+			preferredDERP := 0
+			if tt.last != nil {
+				preferredDERP = tt.last.PreferredDERP
+			}
+			got := makeProbePlan(tt.dm, ifState, tt.last, preferredDERP)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("unexpected plan; got:\n%v\nwant:\n%v\n", got, tt.want)
 			}
@@ -756,7 +794,7 @@ func TestSortRegions(t *testing.T) {
 	report.RegionLatency[3] = time.Second * time.Duration(6)
 	report.RegionLatency[4] = time.Second * time.Duration(0)
 	report.RegionLatency[5] = time.Second * time.Duration(2)
-	sortedMap := sortRegions(unsortedMap, report)
+	sortedMap := sortRegions(unsortedMap, report, 0)
 
 	// Sorting by latency this should result in rid: 5, 2, 1, 3
 	// rid 4 with latency 0 should be at the end

net/sockstats/sockstats_tsgo.go

@@ -279,7 +279,13 @@ func setNetMon(netMon *netmon.Monitor) {
 		if ifName == "" {
 			return
 		}
-		ifIndex := state.Interface[ifName].Index
+		// DefaultRouteInterface and Interface are gathered at different points in time.
+		// Check for existence first, to avoid a nil pointer dereference.
+		iface, ok := state.Interface[ifName]
+		if !ok {
+			return
+		}
+		ifIndex := iface.Index
 		sockStats.mu.Lock()
 		defer sockStats.mu.Unlock()
 		// Ignore changes to unknown interfaces -- it would require

ssh/tailssh/incubator.go

@@ -210,8 +210,6 @@ type incubatorArgs struct {
 	debugTest          bool
 	isSELinuxEnforcing bool
 	encodedEnv         string
-	allowListEnvKeys   string
-	forwardedEnviron   []string
 }
 
 func parseIncubatorArgs(args []string) (incubatorArgs, error) {
@@ -246,31 +244,35 @@ func parseIncubatorArgs(args []string) (incubatorArgs, error) {
 		ia.gids = append(ia.gids, gid)
 	}
 
-	ia.forwardedEnviron = os.Environ()
+	return ia, nil
+}
+
+func (ia incubatorArgs) forwadedEnviron() ([]string, string, error) {
+	environ := os.Environ()
 	// pass through SSH_AUTH_SOCK environment variable to support ssh agent forwarding
-	ia.allowListEnvKeys = "SSH_AUTH_SOCK"
+	allowListKeys := "SSH_AUTH_SOCK"
 
 	if ia.encodedEnv != "" {
 		unquoted, err := strconv.Unquote(ia.encodedEnv)
 		if err != nil {
-			return ia, fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+			return nil, "", fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
 		}
 
 		var extraEnviron []string
 
 		err = json.Unmarshal([]byte(unquoted), &extraEnviron)
 		if err != nil {
-			return ia, fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
+			return nil, "", fmt.Errorf("unable to parse encodedEnv %q: %w", ia.encodedEnv, err)
 		}
 
-		ia.forwardedEnviron = append(ia.forwardedEnviron, extraEnviron...)
+		environ = append(environ, extraEnviron...)
 
 		for _, v := range extraEnviron {
-			ia.allowListEnvKeys = fmt.Sprintf("%s,%s", ia.allowListEnvKeys, strings.Split(v, "=")[0])
+			allowListKeys = fmt.Sprintf("%s,%s", allowListKeys, strings.Split(v, "=")[0])
 		}
 	}
 
-	return ia, nil
+	return environ, allowListKeys, nil
 }
 
 // beIncubator is the entrypoint to the `tailscaled be-child ssh` subcommand.
@@ -450,8 +452,13 @@ func tryExecLogin(dlogf logger.Logf, ia incubatorArgs) error {
 	loginArgs := ia.loginArgs(loginCmdPath)
 	dlogf("logging in with %+v", loginArgs)
 
+	environ, _, err := ia.forwadedEnviron()
+	if err != nil {
+		return err
+	}
+
 	// If Exec works, the Go code will not proceed past this:
-	err = unix.Exec(loginCmdPath, loginArgs, ia.forwardedEnviron)
+	err = unix.Exec(loginCmdPath, loginArgs, environ)
 
 	// If we made it here, Exec failed.
 	return err
@@ -484,9 +491,14 @@ func trySU(dlogf logger.Logf, ia incubatorArgs) (handled bool, err error) {
 		defer sessionCloser()
 	}
 
+	environ, allowListEnvKeys, err := ia.forwadedEnviron()
+	if err != nil {
+		return false, err
+	}
+
 	loginArgs := []string{
 		su,
-		"-w", ia.allowListEnvKeys,
+		"-w", allowListEnvKeys,
 		"-l",
 		ia.localUser,
 	}
@@ -498,7 +510,7 @@ func trySU(dlogf logger.Logf, ia incubatorArgs) (handled bool, err error) {
 	dlogf("logging in with %+v", loginArgs)
 
 	// If Exec works, the Go code will not proceed past this:
-	err = unix.Exec(su, loginArgs, ia.forwardedEnviron)
+	err = unix.Exec(su, loginArgs, environ)
 
 	// If we made it here, Exec failed.
 	return true, err
@@ -527,11 +539,16 @@ func findSU(dlogf logger.Logf, ia incubatorArgs) string {
 		return ""
 	}
 
+	_, allowListEnvKeys, err := ia.forwadedEnviron()
+	if err != nil {
+		return ""
+	}
+
 	// First try to execute su -w <allow listed env> -l <user> -c true
 	// to make sure su supports the necessary arguments.
 	err = exec.Command(
 		su,
-		"-w", ia.allowListEnvKeys,
+		"-w", allowListEnvKeys,
 		"-l",
 		ia.localUser,
 		"-c", "true",
@@ -558,10 +575,15 @@ func handleSSHInProcess(dlogf logger.Logf, ia incubatorArgs) error {
 		return err
 	}
 
+	environ, _, err := ia.forwadedEnviron()
+	if err != nil {
+		return err
+	}
+
 	args := shellArgs(ia.isShell, ia.cmd)
 	dlogf("running %s %q", ia.loginShell, args)
-	cmd := newCommand(ia.hasTTY, ia.loginShell, ia.forwardedEnviron, args)
-	err := cmd.Run()
+	cmd := newCommand(ia.hasTTY, ia.loginShell, environ, args)
+	err = cmd.Run()
 	if ee, ok := err.(*exec.ExitError); ok {
 		ps := ee.ProcessState
 		code := ps.ExitCode()

wgengine/magicsock/derp.go

@@ -158,10 +158,10 @@ func (c *Conn) maybeSetNearestDERP(report *netcheck.Report) (preferredDERP int)
 	} else {
 		connectedToControl = c.health.GetInPollNetMap()
 	}
+	c.mu.Lock()
+	myDerp := c.myDerp
+	c.mu.Unlock()
 	if !connectedToControl {
-		c.mu.Lock()
-		myDerp := c.myDerp
-		c.mu.Unlock()
 		if myDerp != 0 {
 			metricDERPHomeNoChangeNoControl.Add(1)
 			return myDerp
@@ -178,6 +178,11 @@ func (c *Conn) maybeSetNearestDERP(report *netcheck.Report) (preferredDERP int)
 		// one.
 		preferredDERP = c.pickDERPFallback()
 	}
+	if preferredDERP != myDerp {
+		c.logf(
+			"magicsock: home DERP changing from derp-%d [%dms] to derp-%d [%dms]",
+			c.myDerp, report.RegionLatency[myDerp].Milliseconds(), preferredDERP, report.RegionLatency[preferredDERP].Milliseconds())
+	}
 	if !c.setNearestDERP(preferredDERP) {
 		preferredDERP = 0
 	}