diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index eb1771ce4..0d665bf3c 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -1049,7 +1049,6 @@ func (h *Handler) serveWatchIPNBus(w http.ResponseWriter, r *http.Request) { http.Error(w, "not a flusher", http.StatusInternalServerError) return } - w.Header().Set("Content-Type", "application/json") var mask ipn.NotifyWatchOpt if s := r.FormValue("mask"); s != "" { @@ -1060,6 +1059,16 @@ func (h *Handler) serveWatchIPNBus(w http.ResponseWriter, r *http.Request) { } mask = ipn.NotifyWatchOpt(v) } + // Users with only read access must request private key filtering. If they + // don't filter out private keys, require write access. + if (mask & ipn.NotifyNoPrivateKeys) == 0 { + if !h.PermitWrite { + http.Error(w, "watch IPN bus access denied, must set ipn.NotifyNoPrivateKeys when not running as admin/root or operator", http.StatusForbidden) + return + } + } + + w.Header().Set("Content-Type", "application/json") ctx := r.Context() h.b.WatchNotifications(ctx, mask, f.Flush, func(roNotify *ipn.Notify) (keepGoing bool) { js, err := json.Marshal(roNotify) diff --git a/ipn/localapi/localapi_test.go b/ipn/localapi/localapi_test.go index b534c594f..3a348b8ee 100644 --- a/ipn/localapi/localapi_test.go +++ b/ipn/localapi/localapi_test.go @@ -5,7 +5,10 @@ package localapi import ( "bytes" + "context" "encoding/json" + "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -17,8 +20,13 @@ import ( "tailscale.com/client/tailscale/apitype" "tailscale.com/ipn" "tailscale.com/ipn/ipnlocal" + "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" + "tailscale.com/tsd" "tailscale.com/tstest" + "tailscale.com/types/logger" + "tailscale.com/types/logid" + "tailscale.com/wgengine" ) func TestValidHost(t *testing.T) { @@ -212,3 +220,92 @@ func TestShouldDenyServeConfigForGOOSAndUserContext(t *testing.T) { }) } } + +func TestServeWatchIPNBus(t *testing.T) { + tstest.Replace(t, &validLocalHostForTesting, true) + + tests := []struct { + desc string + permitRead, permitWrite bool + mask ipn.NotifyWatchOpt // extra bits in addition to ipn.NotifyInitialState + wantStatus int + }{ + { + desc: "no-permission", + permitRead: false, + permitWrite: false, + wantStatus: http.StatusForbidden, + }, + { + desc: "read-initial-state", + permitRead: true, + permitWrite: false, + wantStatus: http.StatusForbidden, + }, + { + desc: "read-initial-state-no-private-keys", + permitRead: true, + permitWrite: false, + mask: ipn.NotifyNoPrivateKeys, + wantStatus: http.StatusOK, + }, + { + desc: "read-initial-state-with-private-keys", + permitRead: true, + permitWrite: true, + wantStatus: http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + h := &Handler{ + PermitRead: tt.permitRead, + PermitWrite: tt.permitWrite, + b: newTestLocalBackend(t), + } + s := httptest.NewServer(h) + defer s.Close() + c := s.Client() + + ctx, cancel := context.WithCancel(context.Background()) + req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s/localapi/v0/watch-ipn-bus?mask=%d", s.URL, ipn.NotifyInitialState|tt.mask), nil) + if err != nil { + t.Fatal(err) + } + res, err := c.Do(req) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + // Cancel the context so that localapi stops streaming IPN bus + // updates. + cancel() + body, err := io.ReadAll(res.Body) + if err != nil && !errors.Is(err, context.Canceled) { + t.Fatal(err) + } + if res.StatusCode != tt.wantStatus { + t.Errorf("res.StatusCode=%d, want %d. body: %s", res.StatusCode, tt.wantStatus, body) + } + }) + } +} + +func newTestLocalBackend(t testing.TB) *ipnlocal.LocalBackend { + var logf logger.Logf = logger.Discard + sys := new(tsd.System) + store := new(mem.Store) + sys.Set(store) + eng, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set) + if err != nil { + t.Fatalf("NewFakeUserspaceEngine: %v", err) + } + t.Cleanup(eng.Close) + sys.Set(eng) + lb, err := ipnlocal.NewLocalBackend(logf, logid.PublicID{}, sys, 0) + if err != nil { + t.Fatalf("NewLocalBackend: %v", err) + } + return lb +}