From 296e71259106942ee1bb66dc1f1551e7b9c9b0b0 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Wed, 16 Nov 2022 23:36:01 +0500 Subject: [PATCH] tailcfg: add CapabilityDebug Updates tailscale/corp#7948 Signed-off-by: Maisem Ali --- ipn/ipnlocal/peerapi.go | 11 +++++++ ipn/ipnlocal/peerapi_test.go | 63 +++++++++++++++++++++++------------- tailcfg/tailcfg.go | 4 +++ 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index bf0c3c7d7..902e3cc3b 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -778,6 +778,17 @@ func (h *peerAPIHandler) canPutFile() bool { // canDebug reports whether h can debug this node (goroutines, metrics, // magicsock internal state, etc). func (h *peerAPIHandler) canDebug() bool { + // Reread the selfNode as it may have changed since the peerAPIServer + // was created. + // TODO(maisem): handle this in other places too. + nm := h.ps.b.NetMap() + if nm == nil || nm.SelfNode == nil { + return false + } + if !slices.Contains(nm.SelfNode.Capabilities, tailcfg.CapabilityDebug) { + // This node does not expose debug info. + return false + } return h.isSelf || h.peerHasCap(tailcfg.CapabilityDebugPeer) } diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 5b59141dc..83fd01163 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -25,6 +25,7 @@ import ( "tailscale.com/tailcfg" "tailscale.com/tstest" "tailscale.com/types/logger" + "tailscale.com/types/netmap" "tailscale.com/util/must" "tailscale.com/wgengine" "tailscale.com/wgengine/filter" @@ -113,6 +114,7 @@ func TestHandlePeerAPI(t *testing.T) { name string isSelf bool // the peer sending the request is owned by us capSharing bool // self node has file sharing capability + debugCap bool // self node has debug capability omitRoot bool // don't configure req *http.Request checks []check @@ -140,15 +142,24 @@ func TestHandlePeerAPI(t *testing.T) { ), }, { - name: "peer_api_goroutines_deny", - isSelf: false, - req: httptest.NewRequest("GET", "/v0/goroutines", nil), - checks: checks(httpStatus(403)), + name: "goroutines/deny-self-no-cap", + isSelf: true, + debugCap: false, + req: httptest.NewRequest("GET", "/v0/goroutines", nil), + checks: checks(httpStatus(403)), }, { - name: "peer_api_goroutines", - isSelf: true, - req: httptest.NewRequest("GET", "/v0/goroutines", nil), + name: "goroutines/deny-nonself", + isSelf: false, + debugCap: true, + req: httptest.NewRequest("GET", "/v0/goroutines", nil), + checks: checks(httpStatus(403)), + }, + { + name: "goroutines/accept-self", + isSelf: true, + debugCap: true, + req: httptest.NewRequest("GET", "/v0/goroutines", nil), checks: checks( httpStatus(200), bodyContains("ServeHTTP"), @@ -404,25 +415,28 @@ func TestHandlePeerAPI(t *testing.T) { ), }, { - name: "host-val/bad-ip", - isSelf: true, - req: httptest.NewRequest("GET", "http://12.23.45.66:1234/v0/env", nil), + name: "host-val/bad-ip", + isSelf: true, + debugCap: true, + req: httptest.NewRequest("GET", "http://12.23.45.66:1234/v0/env", nil), checks: checks( httpStatus(403), ), }, { - name: "host-val/no-port", - isSelf: true, - req: httptest.NewRequest("GET", "http://100.100.100.101/v0/env", nil), + name: "host-val/no-port", + isSelf: true, + debugCap: true, + req: httptest.NewRequest("GET", "http://100.100.100.101/v0/env", nil), checks: checks( httpStatus(403), ), }, { - name: "host-val/peer", - isSelf: true, - req: httptest.NewRequest("GET", "http://peer/v0/env", nil), + name: "host-val/peer", + isSelf: true, + debugCap: true, + req: httptest.NewRequest("GET", "http://peer/v0/env", nil), checks: checks( httpStatus(200), ), @@ -430,10 +444,16 @@ func TestHandlePeerAPI(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + selfNode := &tailcfg.Node{ + Addresses: []netip.Prefix{ + netip.MustParsePrefix("100.100.100.101/32"), + }, + } var e peerAPITestEnv lb := &LocalBackend{ logf: e.logBuf.Logf, capFileSharing: tt.capSharing, + netMap: &netmap.NetworkMap{SelfNode: selfNode}, } e.ph = &peerAPIHandler{ isSelf: tt.isSelf, @@ -441,14 +461,13 @@ func TestHandlePeerAPI(t *testing.T) { ComputedName: "some-peer-name", }, ps: &peerAPIServer{ - b: lb, - selfNode: &tailcfg.Node{ - Addresses: []netip.Prefix{ - netip.MustParsePrefix("100.100.100.101/32"), - }, - }, + b: lb, + selfNode: selfNode, }, } + if tt.debugCap { + e.ph.ps.selfNode.Capabilities = append(e.ph.ps.selfNode.Capabilities, tailcfg.CapabilityDebug) + } var rootDir string if !tt.omitRoot { rootDir = t.TempDir() diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 24d4093dd..580af8c7f 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -1654,12 +1654,16 @@ type Oauth2Token struct { const ( // These are the capabilities that the self node has as listed in // MapResponse.Node.Capabilities. + // + // We've since started referring to these as "Node Attributes" ("nodeAttrs" + // in the ACL policy file). CapabilityFileSharing = "https://tailscale.com/cap/file-sharing" CapabilityAdmin = "https://tailscale.com/cap/is-admin" CapabilitySSH = "https://tailscale.com/cap/ssh" // feature enabled/available CapabilitySSHRuleIn = "https://tailscale.com/cap/ssh-rule-in" // some SSH rule reach this node CapabilityDataPlaneAuditLogs = "https://tailscale.com/cap/data-plane-audit-logs" // feature enabled + CapabilityDebug = "https://tailscale.com/cap/debug" // exposes debug endpoints over the PeerAPI // Inter-node capabilities as specified in the MapResponse.PacketFilter[].CapGrants.