Phase 1 support for operator privileges #65256

ywangd · 2020-11-19T12:06:20Z

In some Elastic Stack environments, there is a distinction between the operator of the cluster infrastructure and the administrator of the cluster. This distinction cannot be supported currently because the "administrator" often has the superuser role which grants each and every privilege of the cluster.

This PR adds a new feature to protect a fixed set of APIs from the "administrator" even when it is a highly privileged user such as superuser. It enhances the Elasticsearch security model to have an additional layer of restriction in addition to the RBAC.

ywangd · 2020-11-24T04:21:00Z

@tvernum The PR is ready for an initial look. I added some essential tests which prove the feature is working. It is still intentional light on tests because I'd like to get confirmation on the general approach before spending too much time for the coverage.

I spent quite a bit time to ponder how to implement the test for ensuring every action is marked either "operator-only" or "non-operator" (It's an interesting exploration). I don't think there is an easy way of doing it, not possible with unit tests for sure. The main challenge is to ensure its current and future coverage for all possible actions. Most of the actions are from modules, including always on plugins (I collectively refer them as modules since that's how it's reported with GET _nodes). There is no way to cover all of them without actually running a cluster. Also this has to be an external cluster because an internal cluster does not run every module. But with an external cluster, we lose the ability to access node components at code level which means we have no way to query for all registered actions. So it seems to me that a possible approach is to create a test only plugin which registered an API that expose this information. The test will then query this new API and get a list of all actions. I didn't try it but it seems plausible in theory. If you could see another simpler solution, I would like to learn about it. I am also happy to have a synchronous discussion on this topic if you are interested (it's pretty intriguing). Btw, I thought about having a Gradle task to perform something like code scanning for all action names. But it's not feasible since some action names are not string literals and some action class are inner classes.

x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java

…only or not

ywangd · 2020-11-24T08:22:10Z

a possible approach is to create a test only plugin which registered an API that expose this information. I didn't try it but it seems plausible in theory.

This is now added. The complexity in fact is not high. Quite a bit boilerplate code to add an API but otherwise pretty minimal. This pattern could be quite useful for other things as well, such as what KnownActionsTests were trying to achieve, and ensure actions are categorised under the right named privileges, etc. So other than not being an unit test, this approach seems to be about right.

x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java

...ternalClusterTest/java/org/elasticsearch/xpack/security/operator/NonOperatorOnlyActions.java

...erTest/java/org/elasticsearch/xpack/security/operator/OperatorPrivilegesSingleNodeTests.java

x-pack/qa/operator-privileges-tests/build.gradle

...ator-privileges-tests/src/main/java/org/elasticsearch/example/actions/GetActionsRequest.java

...r-privileges-tests/src/main/java/org/elasticsearch/example/actions/RestGetActionsAction.java

…ecurity/operator/OperatorUserDescriptor.java Co-authored-by: Tim Vernum <[email protected]>

…rivileges-phase-one

ywangd · 2020-11-26T13:01:24Z

...ecurity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java

@@ -567,24 +582,23 @@ public void testTokenRestMissing() throws Exception {
        });
    }

-    public void authenticationInContextAndHeader() throws Exception {
+    public void testAuthenticationInContextAndHeader() throws Exception {


This test was skipped for a long time because of the naming convention mismatch. I fixed it but it's not 100% the same as it was because things evolved.

ywangd · 2020-11-26T13:04:07Z

...ugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

@@ -202,6 +205,16 @@ public void authorize(final Authentication authentication, final String action,
            // sometimes a request might be wrapped within another, which is the case for proxied
            // requests and concrete shard requests
            final TransportRequest unwrappedRequest = maybeUnwrapRequest(authentication, originalRequest, action, auditId);
+
+            // Check operator privileges
+            // TODO: audit?


I think auditing can be a follow up PR.

…ecurity/operator/FileOperatorUsersStore.java Co-authored-by: Tim Vernum <[email protected]>

tvernum

LGTM

...n/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java

…ecurity/operator/OperatorOnlyRegistry.java Co-authored-by: Tim Vernum <[email protected]>

…rivileges-phase-one

In some Elastic Stack environments, there is a distinction between the operator of the cluster infrastructure and the administrator of the cluster. This distinction cannot be supported currently because the "administrator" often has the superuser role which grants each and every privilege of the cluster. This PR adds a new feature to protect a fixed set of APIs from the "administrator" even when it is a highly privileged user such as superuser. It enhances the Elasticsearch security model to have an additional layer of restriction in addition to the RBAC. Co-authored-by: Tim Vernum <[email protected]>

This is a follow-up PR for #65256 to fix the xpack info and usage reports for operator privilegs. In summary, this PR ensures: * _xpack does not report operator privileges because it is categorised under security * _xpack/usage reports operator privileges status under the security section * _license/feature_usage reports last used time of operator privileges. It is up to the downstream to filter out this report if necessary.

Add documentation for operator privilegs. The docs cover features delivered by phase 1 (#65256) and 2 (#66684). Co-authored-by: Tim Vernum <[email protected]> Co-authored-by: lcawl <[email protected]> Co-authored-by: Adam Locke <[email protected]>

Add documentation for operator privilegs. The docs cover features delivered by phase 1 (elastic#65256) and 2 (elastic#66684). Co-authored-by: Tim Vernum <[email protected]> Co-authored-by: lcawl <[email protected]> Co-authored-by: Adam Locke <[email protected]>

Add documentation for operator privilegs. The docs cover features delivered by phase 1 (#65256) and 2 (#66684). Co-authored-by: Tim Vernum <[email protected]> Co-authored-by: lcawl <[email protected]> Co-authored-by: Adam Locke <[email protected]>

WIP: operator privileges initial working code with smoke tests

4966365

ywangd force-pushed the meta-101-operator-privileges-phase-one branch 2 times, most recently from 96ccc04 to ceef13c Compare November 23, 2020 23:55

WIP: working on tests

3d5f3b0

ywangd force-pushed the meta-101-operator-privileges-phase-one branch from ceef13c to 3d5f3b0 Compare November 24, 2020 01:31

ywangd added :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC >feature v7.11.0 v8.0.0 labels Nov 24, 2020

ywangd requested a review from tvernum November 24, 2020 04:21

ywangd commented Nov 24, 2020

View reviewed changes

x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java Show resolved Hide resolved

ywangd added 3 commits November 24, 2020 16:07

Remove unnecessary file

cd5f339

Use javaRestTest instead of test to align with recent changes

9834e47

Add a test plugin to ensure every action is declared either operator-…

9b63010

…only or not

ywangd added 2 commits November 24, 2020 20:01

spotless

be893ec

testingConventions test

2d66a05

tvernum reviewed Nov 25, 2020

View reviewed changes

ywangd and others added 9 commits November 26, 2020 00:25

Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…

1b28476

…ecurity/operator/OperatorUserDescriptor.java Co-authored-by: Tim Vernum <[email protected]>

address feedback

892840f

Simplify the plugin IT test with reflection

9314789

spotless

5dedd54

Refactor as suggested

901ecd2

Move plugin test package

b52c814

Fix authenticationService tests

5883923

Fix tests and telemetry

1b3344f

Merge remote-tracking branch 'origin/master' into meta-101-operator-p…

0e170d9

…rivileges-phase-one

ywangd commented Nov 26, 2020

View reviewed changes

ywangd and others added 4 commits November 30, 2020 22:54

Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…

c3fa2ba

…ecurity/operator/FileOperatorUsersStore.java Co-authored-by: Tim Vernum <[email protected]>

wip

4cf5b08

Address feedback

973e7a5

Fix import

2ae1bef

ywangd requested a review from tvernum November 30, 2020 12:25

ywangd added 2 commits December 1, 2020 09:10

Tweak

91d2ef0

fix test

6bddcff

tvernum approved these changes Dec 2, 2020

View reviewed changes

...n/security/src/main/java/org/elasticsearch/xpack/security/operator/OperatorOnlyRegistry.java Show resolved Hide resolved

ywangd and others added 2 commits December 3, 2020 15:31

Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…

e7042f9

…ecurity/operator/OperatorOnlyRegistry.java Co-authored-by: Tim Vernum <[email protected]>

Merge remote-tracking branch 'origin/master' into meta-101-operator-p…

5807b33

…rivileges-phase-one

ywangd merged commit def2f27 into elastic:master Dec 3, 2020

ywangd mentioned this pull request Dec 4, 2020

Fix xpack info and usage reports for operator privileges #65867

Merged

ywangd mentioned this pull request Dec 8, 2020

Do not restore autoscaling policy from snapshots #66023

Merged

ywangd mentioned this pull request Feb 15, 2021

Documentation for operator privileges #68964

Merged

This was referenced Mar 10, 2021

Documentation for operator privileges (#68964) #70193

Merged

Documentation for operator privileges (#68964) #70194

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phase 1 support for operator privileges #65256

Phase 1 support for operator privileges #65256

ywangd commented Nov 19, 2020 •

edited

Loading

ywangd commented Nov 24, 2020

ywangd commented Nov 24, 2020

ywangd Nov 26, 2020 •

edited

Loading

ywangd Nov 26, 2020

tvernum left a comment

Phase 1 support for operator privileges #65256

Phase 1 support for operator privileges #65256

Conversation

ywangd commented Nov 19, 2020 • edited Loading

ywangd commented Nov 24, 2020

ywangd commented Nov 24, 2020

ywangd Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

ywangd Nov 26, 2020

Choose a reason for hiding this comment

tvernum left a comment

Choose a reason for hiding this comment

ywangd commented Nov 19, 2020 •

edited

Loading

ywangd Nov 26, 2020 •

edited

Loading