diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 38c47867a..b15e90601 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -5,7 +5,9 @@ package ipnlocal import ( "context" + "crypto/sha256" "crypto/tls" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -33,6 +35,11 @@ import ( "tailscale.com/version" ) +// ErrETagMismatch signals that the given +// If-Match header does not match with the +// current etag of a resource. +var ErrETagMismatch = errors.New("etag mismatch") + // serveHTTPContextKey is the context.Value key for a *serveHTTPContext. type serveHTTPContextKey struct{} @@ -214,13 +221,15 @@ func (b *LocalBackend) updateServeTCPPortNetMapAddrListenersLocked(ports []uint1 } // SetServeConfig establishes or replaces the current serve config. -func (b *LocalBackend) SetServeConfig(config *ipn.ServeConfig) error { +// ETag is an optional parameter to enforce Optimistic Concurrency Control. +// If it is an empty string, then the config will be overwritten. +func (b *LocalBackend) SetServeConfig(config *ipn.ServeConfig, etag string) error { b.mu.Lock() defer b.mu.Unlock() - return b.setServeConfigLocked(config) + return b.setServeConfigLocked(config, etag) } -func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig) error { +func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig, etag string) error { prefs := b.pm.CurrentPrefs() if config.IsFunnelOn() && prefs.ShieldsUp() { return errors.New("Unable to turn on Funnel while shields-up is enabled") @@ -233,8 +242,24 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig) error { if !nm.SelfNode.Valid() { return errors.New("netMap SelfNode is nil") } - profileID := b.pm.CurrentProfile().ID - confKey := ipn.ServeConfigKey(profileID) + + // If etag is present, check that it has + // not changed from the last config. + 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. + previousCfg, err := json.Marshal(b.serveConfig) + if err != nil { + return fmt.Errorf("error encoding previous config: %w", err) + } + sum := sha256.Sum256(previousCfg) + previousEtag := hex.EncodeToString(sum[:]) + if etag != previousEtag { + return ErrETagMismatch + } + } var bs []byte if config != nil { @@ -244,6 +269,9 @@ func (b *LocalBackend) setServeConfigLocked(config *ipn.ServeConfig) error { } bs = j } + + profileID := b.pm.CurrentProfile().ID + confKey := ipn.ServeConfigKey(profileID) if err := b.store.WriteState(confKey, bs); err != nil { return fmt.Errorf("writing ServeConfig to StateStore: %w", err) } @@ -271,7 +299,7 @@ func (b *LocalBackend) DeleteForegroundSession(sessionID string) error { } sc := b.serveConfig.AsStruct() delete(sc.Foreground, sessionID) - return b.setServeConfigLocked(sc) + return b.setServeConfigLocked(sc, "") } func (b *LocalBackend) HandleIngressTCPConn(ingressPeer tailcfg.NodeView, target ipn.HostPort, srcAddr netip.AddrPort, getConnOrReset func() (net.Conn, bool), sendRST func()) { diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index f36ab955a..4fad5bb42 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -6,7 +6,11 @@ package ipnlocal import ( "bytes" "context" + "crypto/sha256" "crypto/tls" + "encoding/hex" + "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -24,6 +28,7 @@ import ( "tailscale.com/types/logid" "tailscale.com/types/netmap" "tailscale.com/util/cmpx" + "tailscale.com/util/mak" "tailscale.com/util/must" "tailscale.com/wgengine" ) @@ -169,50 +174,82 @@ func TestGetServeHandler(t *testing.T) { } } -func TestServeHTTPProxy(t *testing.T) { - sys := &tsd.System{} - e, err := wgengine.NewUserspaceEngine(t.Logf, wgengine.Config{SetSubsystem: sys.Set}) +func getEtag(t *testing.T, b any) string { + t.Helper() + bts, err := json.Marshal(b) if err != nil { t.Fatal(err) } - sys.Set(e) - sys.Set(new(mem.Store)) - b, err := NewLocalBackend(t.Logf, logid.PublicID{}, sys, 0) + sum := sha256.Sum256(bts) + return hex.EncodeToString(sum[:]) +} + +func TestServeConfigETag(t *testing.T) { + b := newTestBackend(t) + + // a nil config with initial etag should succeed + err := b.SetServeConfig(nil, getEtag(t, nil)) if err != nil { t.Fatal(err) } - defer b.Shutdown() - dir := t.TempDir() - b.SetVarRoot(dir) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - pm.currentProfile = &ipn.LoginProfile{ID: "id0"} - b.pm = pm + // a nil config with an invalid etag should fail + err = b.SetServeConfig(nil, "abc") + if !errors.Is(err, ErrETagMismatch) { + t.Fatal("expected an error but got nil") + } - b.netMap = &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - Name: "example.ts.net", - }).View(), - UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ - tailcfg.UserID(1): { - LoginName: "someone@example.com", - DisplayName: "Some One", - ProfilePicURL: "https://example.com/photo.jpg", - }, + // a new config with no etag should succeed + conf := &ipn.ServeConfig{ + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "example.ts.net:443": {Handlers: map[string]*ipn.HTTPHandler{ + "/": {Proxy: "http://127.0.0.1:3000"}, + }}, }, } - b.nodeByAddr = map[netip.Addr]tailcfg.NodeView{ - netip.MustParseAddr("100.150.151.152"): (&tailcfg.Node{ - ComputedName: "some-peer", - User: tailcfg.UserID(1), - }).View(), - netip.MustParseAddr("100.150.151.153"): (&tailcfg.Node{ - ComputedName: "some-tagged-peer", - Tags: []string{"tag:server", "tag:test"}, - User: tailcfg.UserID(1), - }).View(), + err = b.SetServeConfig(conf, getEtag(t, nil)) + 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") + } + conf = confView.AsStruct() + mak.Set(&conf.AllowFunnel, "example.ts.net:443", true) + + // replacing an existing config with an invalid etag should fail + err = b.SetServeConfig(conf, "invalid etag") + if !errors.Is(err, ErrETagMismatch) { + t.Fatalf("expected an etag mismatch error but got %v", err) + } + + // replacing an existing config with a valid etag should succeed + err = b.SetServeConfig(conf, etag) + if err != nil { + t.Fatal(err) + } + + // replacing an existing config with a previous etag should fail + err = b.SetServeConfig(nil, etag) + if !errors.Is(err, ErrETagMismatch) { + t.Fatalf("expected an etag mismatch error but got %v", err) } + // replacing an existing config with the new etag should succeed + newCfg := b.ServeConfig() + etag = getEtag(t, newCfg) + err = b.SetServeConfig(nil, etag) + if err != nil { + t.Fatal(err) + } +} + +func TestServeHTTPProxy(t *testing.T) { + b := newTestBackend(t) + // Start test serve endpoint. testServ := httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { @@ -232,7 +269,7 @@ func TestServeHTTPProxy(t *testing.T) { }}, }, } - if err := b.SetServeConfig(conf); err != nil { + if err := b.SetServeConfig(conf, ""); err != nil { t.Fatal(err) } @@ -309,6 +346,52 @@ func TestServeHTTPProxy(t *testing.T) { } } +func newTestBackend(t *testing.T) *LocalBackend { + sys := &tsd.System{} + e, err := wgengine.NewUserspaceEngine(t.Logf, wgengine.Config{SetSubsystem: sys.Set}) + if err != nil { + t.Fatal(err) + } + sys.Set(e) + sys.Set(new(mem.Store)) + b, err := NewLocalBackend(t.Logf, logid.PublicID{}, sys, 0) + if err != nil { + t.Fatal(err) + } + t.Cleanup(b.Shutdown) + dir := t.TempDir() + b.SetVarRoot(dir) + + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + pm.currentProfile = &ipn.LoginProfile{ID: "id0"} + b.pm = pm + + b.netMap = &netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{ + Name: "example.ts.net", + }).View(), + UserProfiles: map[tailcfg.UserID]tailcfg.UserProfile{ + tailcfg.UserID(1): { + LoginName: "someone@example.com", + DisplayName: "Some One", + ProfilePicURL: "https://example.com/photo.jpg", + }, + }, + } + b.nodeByAddr = map[netip.Addr]tailcfg.NodeView{ + netip.MustParseAddr("100.150.151.152"): (&tailcfg.Node{ + ComputedName: "some-peer", + User: tailcfg.UserID(1), + }).View(), + netip.MustParseAddr("100.150.151.153"): (&tailcfg.Node{ + ComputedName: "some-tagged-peer", + Tags: []string{"tag:server", "tag:test"}, + User: tailcfg.UserID(1), + }).View(), + } + return b +} + func TestServeFileOrDirectory(t *testing.T) { td := t.TempDir() writeFile := func(suffix, contents string) { diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 82bb28797..d0e4bc761 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -7,6 +7,8 @@ package localapi import ( "bytes" "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -830,9 +832,17 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) { http.Error(w, "serve config denied", http.StatusForbidden) return } - w.Header().Set("Content-Type", "application/json") config := h.b.ServeConfig() - json.NewEncoder(w).Encode(config) + 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) case "POST": if !h.PermitWrite { http.Error(w, "serve config denied", http.StatusForbidden) @@ -843,7 +853,12 @@ func (h *Handler) serveServeConfig(w http.ResponseWriter, r *http.Request) { writeErrorJSON(w, fmt.Errorf("decoding config: %w", err)) return } - if err := h.b.SetServeConfig(configIn); err != nil { + etag := r.Header.Get("If-Match") + if err := h.b.SetServeConfig(configIn, etag); err != nil { + if errors.Is(err, ipnlocal.ErrETagMismatch) { + http.Error(w, err.Error(), http.StatusPreconditionFailed) + return + } writeErrorJSON(w, fmt.Errorf("updating config: %w", err)) return }