diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 4d6055bbd..3824d6f36 100644 --- a/ipn/ipnlocal/serve.go +++ b/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. diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index 6ee2181a0..0892545cc 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/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) diff --git a/ipn/localapi/serve.go b/ipn/localapi/serve.go index 56c8b486c..efbbde06f 100644 --- a/ipn/localapi/serve.go +++ b/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)