document Admin UI security update #6640

taroface · 2020-02-19T19:31:05Z

Fixes #6331
Fixes #6517
Fixes #6580
Fixes #6617

This PR does the following on 19.2 and 19.1:

Adds an "Admin UI access" subheading to Admin UI Overview page explaining why we limit access & listing the restricted Admin UI sections
- They are called "secure areas" to emphasize that this is a security feature
- The list of "privileged information" in the table may not be complete
Describes the two status-quo workarounds for core users provided in the doc
Adds an info callout & link back to "Admin UI access" on the relevant Admin UI pages
Updates tutorials in which we suggest checking any of the restricted Admin UI pages
- These tutorials generally emphasize speed (of starting, monitoring, testing a cluster) over conceptual thoroughness. As such, I specifically use the SQL workaround that doesn't require an enterprise license.

Should the above also apply to 2.1?

cockroach-teamcity · 2020-02-19T19:31:12Z

This change is

cockroach-teamcity · 2020-02-19T19:35:29Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/b483b39f5496ccc811e48d2a7efd00d08c81b68f/

Edited pages:

mattcrdb · 2020-02-19T21:49:46Z

I think we need product to determine whether the RBAC workaround should be documented.

https://cockroachlabs.slack.com/archives/C4UHCCUB0/p1582144057100000

mattcrdb · 2020-02-20T15:55:53Z

Looks like we're going to allow granting admin role for core, which means we can document the workaround until the work is done at which point once backported we can update the documentation.

taroface · 2020-02-20T21:50:27Z

@mattcrdb Thank you for clarifying! I'll create a separate issue to track the above.

Can you confirm whether that workaround is corrected as documented here?

mattcrdb · 2020-02-24T15:08:32Z

Yep!

INSERT INTO system.role_members (role, member, "isAdmin") VALUES ('admin', 'roach', true) looks correct. roach being the name of the sql user.

knz · 2020-02-24T16:11:41Z

I would recommend avoiding the workaround using INSERT and instead recommend GRANT admin TO ... as per cockroachdb/cockroach#45325 thank you

mattcrdb · 2020-02-24T16:15:01Z

I believe we intend to change the documentation once your patch is released. I guess it depends when this doc PR gets approved and when your change will be released. If we'd rather hold off on this and just want until @knz 's changes then let's do that. I agree we should avoid redundancy.

knz · 2020-02-24T16:16:49Z

If two steps of doc changes are acceptable, I'm OK with that. That will provide some guidance for users while the other PR gets reviewed.

mattcrdb · 2020-02-24T16:24:11Z

That's the plan! cc @jseldess are two-step doc changes acceptable? It would help allow us to point users to our documentation for the time being.

jseldess · 2020-02-24T17:57:40Z

I'm in favor two-step, getting this out now to help current users and then iterating once Raphael's PR is merged and part of a release. @taroface, thoughts?

taroface · 2020-02-24T18:23:32Z

According to this PR, the GRANT/REVOKE update will be backported. Since that PR is approved and I assume the changes will be applied soon, perhaps I can just update this docs PR with the new workaround. I just need confirmation on which versions are getting the backported update. This will take additional time, so best to publish the context & solution asap.

taroface · 2020-02-25T19:26:07Z

@knz Would love to have your approval on the context & workaround documented here!

After cockroachdb/cockroach#45275 is merged, I have a separate issue #6649 to change all INSERT INTO lines to the corresponding GRANT ROLE line.

I don't expect cockroachdb/cockroach#43893 to really impact this update,
since our various 'getting started' docs recommend that visitors create a new SQL user with a password in order to access the Admin UI. Let me know if you disagree.

taroface · 2020-02-26T17:38:58Z

@mattcrdb It looks like Raphael is on PTO. Would you be able to look at the updates here and confirm whether they match with the current guidance you're providing customers?

knz

@taroface I am actually reviewing this PR.

However, it seems like you have botched the PR by including many commits from other people in it, not just your changes. This makes the PR impossible to review (there are a hundred files listed to review in reviewable). Can you clean up the commit list and then ping me again? I'll have another look.

Reviewed 8 of 108 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jseldess, @knz, and @mattcrdb)

taroface · 2020-02-26T18:32:52Z

@knz Fixed. That was an awful error. My apologies!

cockroach-teamcity · 2020-02-26T18:36:14Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/70ae8a672587426abdf444f9ab2595af1c1941fb/

Edited pages:

knz

Looks good overall! Just two minor nits.

Reviewed 27 of 108 files at r1, 77 of 79 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jseldess, @mattcrdb, and @taroface)

v19.1/admin-ui-debug-pages.md, line 29 at r2 (raw file):

Network Latency | Check latencies between all nodes in your cluster. | All users
Data Distribution and Zone Configs | View the distribution of table data across nodes and verify zone configuration. | [`admin` users only on secure clusters](admin-ui-overview.html#admin-ui-access)
Cluster Settings | View all cluster settings and their configured values. | All users

for cluster settings, non-admin users would see less information/details than admin users. Not sure how to include that however.

v19.2/admin-ui-debug-pages.md, line 29 at r2 (raw file):

Network Latency | Check latencies between all nodes in your cluster. | All users
Data Distribution and Zone Configs | View the distribution of table data across nodes and verify zone configuration. | [`admin` users only on secure clusters](admin-ui-overview.html#admin-ui-access)
Cluster Settings | View all cluster settings and their configured values. | All users

see my previous comment

v20.1/admin-ui-debug-pages.md, line 29 at r2 (raw file):

Network Latency | Check latencies between all nodes in your cluster. | All users
Data Distribution and Zone Configs | View the distribution of table data across nodes and verify zone configuration. | [`admin` users only on secure clusters](admin-ui-overview.html#admin-ui-access)
Cluster Settings | View all cluster settings and their configured values. | All users

ditto previous comment

v20.1/admin-ui-jobs-page.md, line 7 at r2 (raw file):

{{site.data.alerts.callout_info}}
On a secure cluster, this area of the Admin UI can only be accessed by an `admin` user. See [Admin UI access](admin-ui-overview.html#admin-ui-access).

This is not true in 20.1. In that version, non-admin users can see their own jobs; admin users can see everything.

v20.1/admin-ui-overview.md, line 48 at r2 (raw file):

[Database Details](admin-ui-databases-page.html) | Stored table data	
[Statements Details](admin-ui-statements-page.html) | SQL statements
[Jobs Details](admin-ui-jobs-page.html) | SQL statements and operational details

see previous comment

taroface

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jseldess, @knz, and @mattcrdb)

v19.1/admin-ui-debug-pages.md, line 29 at r2 (raw file):

Previously, knz (kena) wrote…

for cluster settings, non-admin users would see less information/details than admin users. Not sure how to include that however.

Done.

v19.2/admin-ui-debug-pages.md, line 29 at r2 (raw file):

Previously, knz (kena) wrote…

see my previous comment

Done.

v20.1/admin-ui-debug-pages.md, line 29 at r2 (raw file):

Previously, knz (kena) wrote…

ditto previous comment

Done.

v20.1/admin-ui-jobs-page.md, line 7 at r2 (raw file):

Previously, knz (kena) wrote…

This is not true in 20.1. In that version, non-admin users can see their own jobs; admin users can see everything.

Done. Also removed Jobs from the "secure area" table in the 20.1 admin-ui-overview.

v20.1/admin-ui-overview.md, line 48 at r2 (raw file):

Previously, knz (kena) wrote…

see previous comment

Oh, already done!

knz

nice thanks!

Reviewed 8 of 8 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jseldess, @knz, and @mattcrdb)

knz · 2020-02-26T19:35:16Z

FYI the change to enable GRANT admin for non-licensed users is merged for 20.1, 19.1 and 19.2 already.

cockroach-teamcity · 2020-02-26T19:35:36Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/3273c55489d6ce364a84f04d5a91f3737e145bb8/

Edited pages:

jseldess

LGTM, with some comments. Please apply my comments across different versions, as appropriate.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz, @mattcrdb, and @taroface)

v19.1/admin-ui-overview.md, line 4 at r3 (raw file):

title: Admin UI Overview
summary: Use the Admin UI to monitor and optimize cluster performance.
toc: false

Let's remove toc: false so the headings show up in an on-page TOC.

v19.1/admin-ui-overview.md, line 9 at r3 (raw file):
Let's end with:

... and helps you to optimize cluster performance.

v19.1/admin-ui-overview.md, line 10 at r3 (raw file):

The CockroachDB Admin UI provides details about your cluster and database configuration, and helps you optimize cluster performance by monitoring the following areas:

Let's add an h2 here:

## Admin UI areas

v19.1/admin-ui-overview.md, line 31 at r3 (raw file):

The Admin UI also provides details about the way data is **Distributed**, the state of specific **Queues**, and metrics for **Slow Queries**, but these details are largely internal and intended for use by CockroachDB developers.

## Admin UI access

This is info is great. I wonder if we could incorporate this into the main table of areas? Fine as is, but I'm just curious if we attempted a consolidated view.

v19.1/admin-ui-overview.md, line 35 at r3 (raw file):

On insecure clusters, all areas of the Admin UI are accessible to all users.

On secure clusters, certain areas of the Admin UI can only be accessed by [`admin` users](authorization.html). These areas display information from privileged HTTP endpoints that operate with `root` user permissions.

Do you want to link to a specific section of that page?

v19.1/admin-ui-overview.md, line 42 at r3 (raw file):

The default `root` user is a member of the `admin` role, but on CockroachDB clusters prior to v20.1, the Admin UI cannot be accessed by `root`. To access the secure Admin UI areas, [grant a user membership to the `admin` role](grant-roles.html) using an [enterprise license](enterprise-licensing.html#obtain-a-license) (a trial license can be used).

If you don't have an enterprise license, use this command to manually create a secondary `admin` user: `INSERT INTO system.role_members (role, member, "isAdmin") VALUES ('admin', '<sql_user>', true)`

On smaller screens, the INSERT INTO... is not going to wrap well:

Is it possible to put that inside code block within the callout instead?

v19.2/secure-a-cluster.md, line 311 at r3 (raw file):

    <img src="{{ 'images/v19.2/admin_ui_overview_dashboard_3_nodes.png' | relative_url }}" alt="CockroachDB Admin UI" style="border:1px solid #eee;max-width:100%" />

5. On secure clusters, [certain pages of the Admin UI](admin-ui-overview.html#admin-ui-access) can only be accessed by `admin` users.

Do you think it would be more intuitive to start with this rather than tack it on once they're already in the UI?

v19.2/secure-a-cluster.md, line 313 at r3 (raw file):

5. On secure clusters, [certain pages of the Admin UI](admin-ui-overview.html#admin-ui-access) can only be accessed by `admin` users. 

    Run the [cockroach sql](cockroach-sql.html) command against node 1:

Need to place backticks around cockroach sql.

taroface

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jseldess, @knz, and @mattcrdb)

v19.1/admin-ui-overview.md, line 4 at r3 (raw file):