ui, server: remote_debugging.mode setting isn't being respected

[a-robinson]

I built cockroach from head (b6c934c), then started a 1 node insecure cluster on my gceworker. Without changing any cluster settings, I'm able to access all the debug pages over the internet on the VM's public IP, even though the setting defaults to local only. I then changed the setting to 'off' (set cluster setting server.remote_debugging.mode = 'off';) and I'm still able to access all of the debug pages.

[a-robinson]

So it turns out I misunderstood which pages count as debug pages. Only pages that are prefixed with /debug (not /#/debug) are currently treated as a debug page for the purposes of this setting. So that means that effectively the only things we're locking down are the tracing, logspy, and pprof endpoints. Those are indeed the most sensitive. The /_status/ranges/ and /_status/raft pages (and report pages like #/reports/range/), which expose the start/end keys of all ranges, are debatably sensitive, but you can already often get start/end keys from the logs, which are also accessible without remote debugging turned on.

Closing as user error.

[bdarnell]

Anything which can potentially return keys needs to be protected. We need to lock down the log endpoints as well as all the status/debug pages that can include keys (or we could keep those pages but hide the actual keys if the filter doesn't pass).

[a-robinson]

Alright, but that's no small task, and it's the case in all of our releases going back to v1.0.

[a-robinson]

cc @kuanluo @couchand, since disabling the logs pages based on the value of the server.remote_debugging.mode setting is a pretty user-visible change. Also, are there any other places in the UI where keys are displayed?

[kuanluo]

Thanks @a-robinson! I will let @couchand answer the key question more accurately - I didn't think we display any of them in the UI.

cc: @josueeee

[couchand]

The logs pages are the only place that's documented/linked from somewhere. If/when we choose to do #20690 the other debug pages will need to see more scrutiny.

The seems like one more reason that we should implement #6307/#18206 as soon as possible, but it's unlikely that will be backported to 2.0 or 1.x, so we should probably figure out something that we feel comfortable adding to each of those releases.

I'm fine with the idea that the logs pages and many (all?) the debug pages should be conditional on server.remote_debugging.mode, but it's worth considering the impact: this will be an impediment to debugging issues in the field (albeit a small, and probably worthy, one).

@bdarnell It sounds like this is a high priority from your perspective? Would you be available to spend some time with me to do an audit of which debug pages we need to be thinking about?

cc @dianasaur323 @piyush-singh

[bdarnell]

The ones I think we need to block are

• range status
• raft for all ranges
• everything under the "tracing endpoints" section (I think all the pprof stuff is already covered; if it's not it might be OK to leave it open)
• logs
• single-node specific: gossip, ranges, stacks
• sessions: local and all
• cluster-wide: all of these

[couchand]

@bdarnell thanks for that list. Of the examples you listed, only logs, range status, and raft need addressing for security purposes, since the others are "raw" debug pages that are already covered. But the others are important as far as the usability point raised below. After investigating a bit, I've noticed that my assumption about which endpoints are under the restrictions of server.remote_debugging.mode was wrong (a careful reading of #23555 (comment) provides the same information).

It looks like there are two sides to this issue:

• For the purposes of security, we need to make sure the endpoints used on each of those pages are inaccessible.
• For purposes of usability, we should hide links to the restricted pages and redirect or show an error page from them.

And we have two options for the criteria to apply: if it's not too much effort we could try to support local, or a simpler implementation would just treat local as off for this purpose.

[couchand]

Oh, one more point about usability. We don't currently seem to document the setting server.remote_debugging.mode. If this is going to affect the availability of a user-facing feature (the node logs), we should probably update documentation to point that out.

cc @Amruta-Ranade

[a-robinson]

I should have a PR out for this later today. I have the plumbing of source addresses through the grpc-gateway figured out and working for a couple endpoints, I just need to handle the rest of them. I don't really like the lack of fail-safes to keep us from forgetting this on new endpoints, but we could potentially add some sort of test that crawls all the debug pages looking for key-shaped strings.

We're also going to have to fix the #/node/1/logs page to display errors to the user. Currently it just continues spinning even when the attempt to load logs returns a 403.

Finally, I was worried about how this was going to affect the debug zip command, but I think we're ok to continue allowing everything that comes in directly via grpc, and only apply filters for requests that arrive on the HTTP port and go through the grpc gateway. Let me know if that sounds wrong to you.

[couchand]

@a-robinson Thanks for taking this on. If you knock out the server side of this, I'll add the updates for the admin UI to not break because of it.

You're right that debug zip falls under the gRPC security model rather than that of HTTP, so it's not affected by this issue.

I agree that it's worrisome how easy it is to mess this up. We need better visibility into what endpoints are available and what information they offer.

Really, we need to require a login for HTTP access just the same as we do for gRPC access.