From a25aff2ccc8e40a70439ea50ea2d06dcc7682f2b Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@dalby.cc>
Date: Fri, 6 Nov 2026 10:41:07 +0000
Subject: [PATCH] derp: reject non-200 DERP map responses

A JSON error page decoded to an empty map; a refresh then dropped
that URL's regions. Errors now name the URL.

Updates #3142
---
 hscontrol/derp/derp.go      | 12 +++++++++---
 hscontrol/derp/derp_test.go | 16 ++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/hscontrol/derp/derp.go b/hscontrol/derp/derp.go
index a57303b66..7ba9e2482 100644
--- a/hscontrol/derp/derp.go
+++ b/hscontrol/derp/derp.go
@@ -27,6 +27,7 @@ import (
 
 var (
 	errEmptyDERPMapFile = errors.New("DERP map file has no regions or homeparams")
+	errDERPMapStatus    = errors.New("unexpected HTTP status")
 
 	ErrRegionIDKey    = errors.New("doesn't match its key")
 	ErrRegionNoNodes  = errors.New("must specify at least one node in DERP Region")
@@ -130,21 +131,26 @@ func loadDERPMapFromURL(addr url.URL) (*tailcfg.DERPMap, error) {
 
 	defer resp.Body.Close()
 
+	// An error page can be JSON that decodes to an empty map.
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("DERP map %s: %w: %s", addr.Redacted(), errDERPMapStatus, resp.Status)
+	}
+
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("DERP map %s: %w", addr.Redacted(), err)
 	}
 
 	var derpMap tailcfg.DERPMap
 
 	err = json.Unmarshal(body, &derpMap)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("DERP map %s: %w", addr.Redacted(), err)
 	}
 
 	err = validateRegions(&derpMap)
 	if err != nil {
-		return nil, fmt.Errorf("DERP map %s: %w", addr.String(), err)
+		return nil, fmt.Errorf("DERP map %s: %w", addr.Redacted(), err)
 	}
 
 	return &derpMap, nil
diff --git a/hscontrol/derp/derp_test.go b/hscontrol/derp/derp_test.go
index 57f453726..e19ce9b86 100644
--- a/hscontrol/derp/derp_test.go
+++ b/hscontrol/derp/derp_test.go
@@ -499,3 +499,19 @@ func TestLoadDERPMapFromURLValidates(t *testing.T) {
 	_, err = loadDERPMapFromURL(*addr)
 	require.ErrorIs(t, err, ErrRegionIDKey)
 }
+
+// TestLoadDERPMapFromURLRejectsHTTPError: an error page that happens to be
+// JSON must not pass as an empty map, or a refresh drops that URL's regions.
+func TestLoadDERPMapFromURLRejectsHTTPError(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusServiceUnavailable)
+		_, _ = w.Write([]byte(`{"message": "down for maintenance"}`))
+	}))
+	defer srv.Close()
+
+	addr, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+
+	_, err = loadDERPMapFromURL(*addr)
+	require.ErrorIs(t, err, errDERPMapStatus)
+}