ipn/ipnlocal: allow retrieval of serve config ETags from local API

This change adds API to ipn.LocalBackend to retrieve the ETag when
querying for the current serve config. This allows consumers of
ipn.LocalBackend.SetServeConfig to utilize the concurrency control
offered by ETags. Previous to this change, utilizing serve config ETags
required copying the local backend's internal ETag calcuation.

The local API server was previously copying the local backend's ETag
calculation as described above. With this change, the local API server
now uses the new ETag retrieval function instead. Serve config ETags are
therefore now opaque to clients, in line with best practices.

Fixes tailscale/corp#35857
Signed-off-by: Harry Harpham <harry@tailscale.com>

ipn/ipnlocal/serve.go

@@ -293,6 +293,15 @@ func (b *LocalBackend) updateServeTCPPortNetMapAddrListenersLocked(ports []uint1
 	}
 }
 
+func generateServeConfigETag(sc ipn.ServeConfigView) (string, error) {
+	j, err := json.Marshal(sc)
+	if err != nil {
+		return "", fmt.Errorf("encoding config: %w", err)
+	}
+	sum := sha256.Sum256(j)
+	return hex.EncodeToString(sum[:]), nil
+}
+
 // SetServeConfig establishes or replaces the current serve config.
 // ETag is an optional parameter to enforce Optimistic Concurrency Control.
 // If it is an empty string, then the config will be overwritten.
@@ -327,17 +336,11 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string
 	// not changed from the last config.
 	prevConfig := b.serveConfig
 	if etag != "" {
-		// Note that we marshal b.serveConfig
-		// and not use b.lastServeConfJSON as that might
-		// be a Go nil value, which produces a different
-		// checksum from a JSON "null" value.
-		prevBytes, err := json.Marshal(prevConfig)
+		prevETag, err := generateServeConfigETag(prevConfig)
 		if err != nil {
-			return fmt.Errorf("error encoding previous config: %w", err)
+			return fmt.Errorf("generating ETag for previous config: %w", err)
 		}
-		sum := sha256.Sum256(prevBytes)
-		previousEtag := hex.EncodeToString(sum[:])
-		if etag != previousEtag {
+		if etag != prevETag {
 			return ErrETagMismatch
 		}
 	}
@@ -392,6 +395,20 @@ func (b *LocalBackend) ServeConfig() ipn.ServeConfigView {
 	return b.serveConfig
 }
 
+// ServeConfigETag provides a view of the current serve mappings and an ETag,
+// which can later be provided to [LocalBackend.SetServeConfig] to implement
+// Optimistic Concurrency Control.
+//
+// If serving is not configured, the returned view is not Valid.
+func (b *LocalBackend) ServeConfigETag() (scv ipn.ServeConfigView, etag string, err error) {
+	sc := b.ServeConfig()
+	etag, err = generateServeConfigETag(sc)
+	if err != nil {
+		return ipn.ServeConfigView{}, "", fmt.Errorf("generating ETag: %w", err)
+	}
+	return sc, etag, nil
+}
+
 // DeleteForegroundSession deletes a ServeConfig's foreground session
 // in the LocalBackend if it exists. It also ensures check, delete, and
 // set operations happen within the same mutex lock to avoid any races.

ipn/ipnlocal/serve_test.go

@@ -9,9 +9,7 @@ import (
 	"bytes"
 	"cmp"
 	"context"
-	"crypto/sha256"
 	"crypto/tls"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -222,16 +220,6 @@ func TestGetServeHandler(t *testing.T) {
 	}
 }
 
-func getEtag(t *testing.T, b any) string {
-	t.Helper()
-	bts, err := json.Marshal(b)
-	if err != nil {
-		t.Fatal(err)
-	}
-	sum := sha256.Sum256(bts)
-	return hex.EncodeToString(sum[:])
-}
-
 // TestServeConfigForeground tests the inter-dependency
 // between a ServeConfig and a WatchIPNBus:
 // 1. Creating a WatchIPNBus returns a sessionID, that
@@ -544,8 +532,14 @@ func TestServeConfigServices(t *testing.T) {
 func TestServeConfigETag(t *testing.T) {
 	b := newTestBackend(t)
 
-	// a nil config with initial etag should succeed
-	err := b.SetServeConfig(nil, getEtag(t, nil))
+	// the etag should be valid even when there is no config
+	_, emptyStateETag, err := b.ServeConfigETag()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// a nil config with the empty-state etag should succeed
+	err = b.SetServeConfig(nil, emptyStateETag)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -556,7 +550,7 @@ func TestServeConfigETag(t *testing.T) {
 		t.Fatal("expected an error but got nil")
 	}
 
-	// a new config with no etag should succeed
+	// a new config with the empty-state etag should succeed
 	conf := &ipn.ServeConfig{
 		Web: map[ipn.HostPort]*ipn.WebServerConfig{
 			"example.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{
@@ -564,15 +558,14 @@ func TestServeConfigETag(t *testing.T) {
 			}},
 		},
 	}
-	err = b.SetServeConfig(conf, getEtag(t, nil))
+	err = b.SetServeConfig(conf, emptyStateETag)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	confView := b.ServeConfig()
-	etag := getEtag(t, confView)
-	if etag == "" {
-		t.Fatal("expected to get an etag but got an empty string")
+	confView, etag, err := b.ServeConfigETag()
+	if err != nil {
+		t.Fatal(err)
 	}
 	conf = confView.AsStruct()
 	mak.Set(&conf.AllowFunnel, "example.ts.net:443", true)
@@ -596,8 +589,10 @@ func TestServeConfigETag(t *testing.T) {
 	}
 
 	// replacing an existing config with the new etag should succeed
-	newCfg := b.ServeConfig()
-	etag = getEtag(t, newCfg)
+	_, etag, err = b.ServeConfigETag()
+	if err != nil {
+		t.Fatal(err)
+	}
 	err = b.SetServeConfig(nil, etag)
 	if err != nil {
 		t.Fatal(err)

ipn/localapi/serve.go

@@ -6,8 +6,6 @@
 package localapi
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -31,14 +29,16 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, "serve config denied", http.StatusForbidden)
 			return
 		}
-		config := h.b.ServeConfig()
+		config, etag, err := h.b.ServeConfigETag()
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
 		bts, err := json.Marshal(config)
 		if err != nil {
 			http.Error(w, "error encoding config: "+err.Error(), http.StatusInternalServerError)
 			return
 		}
-		sum := sha256.Sum256(bts)
-		etag := hex.EncodeToString(sum[:])
 		w.Header().Set("Etag", etag)
 		w.Header().Set("Content-Type", "application/json")
 		w.Write(bts)