Add tracing for authorization #80815

ywangd · 2021-11-18T06:23:05Z

This PR adds tracing report for any authorizations except those for the _system user.

Can be turned off with xpack.security.authz.tracing: false
AuthZ is tied to relevant task by traceparent (it however does not
cover the first authZ)
x-opaque-id is auto configured at rest layer if not already exists.
This helps chain all relevant actions together.
ApmIT now has security enabled.

* Can be turned off with xpack.security.authz.tracing: false * AuthZ is tied to relevant task by traceparent (it however does not cover the first authZ) * x-opaque-id is auto configured at rest layer if not already exists. This helps chain all relevant actions together. * ApmIT now has security enabled.

ywangd · 2021-11-18T06:26:52Z

.../plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java

+                        // Populate x-opaque-id if not already exists to chain all related actions together
+                        if (authentication != null && false == SystemUser.is(authentication.getUser())) {
+                            if (threadContext.getHeader(Task.X_OPAQUE_ID) == null) {
+                                threadContext.putHeader(Task.X_OPAQUE_ID, UUIDs.base64UUID());
+                            }
+                        }


X-Opaque-Id automatically gets populate across related tasks and also cross nodes. This could be an alternative to chain all things together. It does not differentiate between parent and child. But it may not matter since the timestamp should tell most of the story?

I like capturing this, if we have it we might as well use it - if nothing else, it provides an easy way to go find the trace for a request you just made if you're running this on a live system.

Yes it is attached to every span in ApmTracer#onTraceStarted

ywangd · 2021-11-18T06:33:26Z

...k/plugin/apm-integration/src/internalClusterTest/java/org/elasticsearch/xpack/apm/ApmIT.java

+            APMTracer.APM_TOKEN_SETTING.getKey(),
+            System.getProperty("tests.apm.token", "")
+        );
+        builder.put("xpack.security.authz.tracing", true);


A new setting to enable/disable authZ tracing.

ywangd · 2021-11-18T06:49:03Z

...k/plugin/apm-integration/src/internalClusterTest/java/org/elasticsearch/xpack/apm/ApmIT.java

-        client().prepareSearch()
-            .setQuery(new RangeQueryBuilder("@timestamp").gt("2021-11-01"))
-            .setSearchType(SearchType.QUERY_THEN_FETCH)
-            .setPreFilterShardSize(1)
-            .execute()
-            .actionGet(10, TimeUnit.SECONDS);
+        final Request searchRequest = new Request("GET", "_search");
+        searchRequest.addParameter("search_type", "query_then_fetch");
+        searchRequest.addParameter("pre_filter_shard_size", "1");
+        searchRequest.setJsonEntity("{\"query\":{\"range\":{\"@timestamp\":{\"gt\":\"2021-11-01\"}}}}");
+        searchRequest.setOptions(
+            searchRequest.getOptions()
+                .toBuilder()
+                .addHeader(
+                    "Authorization",
+                    UsernamePasswordToken.basicAuthHeaderValue(
+                        SecuritySettingsSource.TEST_USER_NAME,
+                        new SecureString(SecuritySettingsSourceField.TEST_PASSWORD.toCharArray())
+                    )
+                )
+        );
+
+        final Response searchResponse = getRestClient().performRequest(searchRequest);


Issue the request at REST layer so it goes through all authentication/authorization layer. Otherwise the first task is created without auth being triggered.

elasticmachine · 2021-11-18T10:21:02Z

Pinging @elastic/es-distributed (Team:Distributed)

ywangd · 2021-11-18T10:21:10Z

@elasticmachine run elasticsearch-ci/part-2

…m/authz-tracing

gwbrown

LGTM! The concept of having separate flags for different parts of tracing is interesting, that might be a solution for something I've been trying to figure out.

gwbrown · 2021-11-18T21:04:08Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/AuthorizationTracer.java

+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+public class AuthorizationTracer {


We should consolidate and/or standardize these ThingTracer classes at some point. Not for this PR thought.

I agree. We could provide a common (base) bridging class between xpack module and APMTracer.

gwbrown · 2021-11-18T23:51:52Z

.../plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java

+                        // Populate x-opaque-id if not already exists to chain all related actions together
+                        if (authentication != null && false == SystemUser.is(authentication.getUser())) {
+                            if (threadContext.getHeader(Task.X_OPAQUE_ID) == null) {
+                                threadContext.putHeader(Task.X_OPAQUE_ID, UUIDs.base64UUID());
+                            }
+                        }


I like capturing this, if we have it we might as well use it - if nothing else, it provides an easy way to go find the trace for a request you just made if you're running this on a live system.

…m/authz-tracing

ywangd · 2021-11-19T01:20:28Z

Thanks for the review @gwbrown

ywangd · 2021-11-19T01:20:55Z

@elasticmachine run elasticsearch-ci/bwc

…m/authz-tracing

ywangd · 2021-11-19T03:16:19Z

@elasticmachine run elasticsearch-ci/part-2

ywangd requested review from tlrx, SylvainJuge, idegtiarenko, gwbrown, DaveCTurner and dimitris-athanasiou November 18, 2021 06:23

ywangd commented Nov 18, 2021

View reviewed changes

ywangd added 3 commits November 18, 2021 17:34

spotless

f6c44ea

fix tests

dd541d6

more spotless

b56d8d5

ywangd commented Nov 18, 2021

View reviewed changes

even more spotless

eae678c

ywangd added :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement labels Nov 18, 2021

elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Nov 18, 2021

ywangd added 2 commits November 18, 2021 22:45

Merge remote-tracking branch 'origin/feature/apm-integration' into ap…

09f3c2e

…m/authz-tracing

fix compilation

548c960

gwbrown approved these changes Nov 18, 2021

View reviewed changes

ywangd added 2 commits November 19, 2021 12:10

Merge remote-tracking branch 'origin/feature/apm-integration' into ap…

28504d9

…m/authz-tracing

format

789cc5e

Merge remote-tracking branch 'origin/feature/apm-integration' into ap…

a9fc7b8

…m/authz-tracing

ywangd merged commit f3f9835 into elastic:feature/apm-integration Nov 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tracing for authorization #80815

Add tracing for authorization #80815

ywangd commented Nov 18, 2021

ywangd Nov 18, 2021

gwbrown Nov 18, 2021

ywangd Nov 19, 2021

ywangd Nov 18, 2021

ywangd Nov 18, 2021

elasticmachine commented Nov 18, 2021

ywangd commented Nov 18, 2021

gwbrown left a comment

gwbrown Nov 18, 2021

ywangd Nov 19, 2021

gwbrown Nov 18, 2021

ywangd commented Nov 19, 2021

ywangd commented Nov 19, 2021

ywangd commented Nov 19, 2021

Add tracing for authorization #80815

Add tracing for authorization #80815

Conversation

ywangd commented Nov 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 18, 2021

ywangd commented Nov 18, 2021

gwbrown left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywangd commented Nov 19, 2021

ywangd commented Nov 19, 2021

ywangd commented Nov 19, 2021