Preserve request headers in a mixed version cluster by ywangd · Pull Request #79412 · elastic/elasticsearch

ywangd · 2021-10-19T01:56:08Z

When rewriting authentication for requests crossing nodes of different
versions, we now preserve all request headers except the authentication
one which needs to be rewritten. Previously all other request headers
were dropped and it caused issue like an operator user not being
recognised on the remote node. Other now preserved headers include audit
and system index access. This new behaviour is more correct because we
would never drop these headers if the nodes are on the same version.

Resolves: #79354

When rewriting authentication for requests crossing nodes of different versions, we now preserve all request headers except the authentication one which needs to be rewritten. Previously all other request headers were dropped and it caused issue like an operator user not being recognised on the remote node. Other now preserved headers include audit and system index access. This new behaviour is more correct because we would never drop these headers if the nodes are on the same version. Resolves: elastic#79354

elasticmachine · 2021-10-19T01:56:11Z

Pinging @elastic/es-security (Team:Security)

ywangd · 2021-10-19T02:02:06Z

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

    public void executeAfterRewritingAuthentication(Consumer<StoredContext> consumer, Version version) {
+        // Preserve request headers other than authentication
+        final Map<String, String> existingRequestHeaders = threadContext.getRequestHeadersOnly();
        final StoredContext original = threadContext.newStoredContext(true);
        final Authentication authentication = getAuthentication();
        try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
            setAuthentication(new Authentication(authentication.getUser(), authentication.getAuthenticatedBy(),
                authentication.getLookedUpBy(), version, authentication.getAuthenticationType(),
                rewriteMetadataForApiKeyRoleDescriptors(version, authentication)));
+            existingRequestHeaders.forEach((k, v) -> {
+                if (false == AuthenticationField.AUTHENTICATION_KEY.equals(k)) {
+                    threadContext.putHeader(k, v);
+                }
+            });
            consumer.accept(original);
        }
    }


This method does not preserve response headers because the original threadContext is wrapped in a restorable that does not propogate the response headers:

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

Lines 105 to 107 in 6192cb2

securityContext.executeAfterRewritingAuthentication(original -> sendWithUser(connection, action, request, options,

new ContextRestoreResponseHandler<>(threadPool.getThreadContext().wrapRestorable(original), handler), sender),

minVersion);

elasticsearch/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java

Lines 255 to 257 in 6192cb2

public Supplier<StoredContext> wrapRestorable(StoredContext storedContext) {

return () -> {

StoredContext context = newStoredContext(false);

But I believe the response headers are actually preserved without this rewriting, i.e. when nodes are on the same version:

elasticsearch/server/src/main/java/org/elasticsearch/transport/TransportService.java

Lines 712 to 713 in 6192cb2

Supplier<ThreadContext.StoredContext> storedContextSupplier = threadPool.getThreadContext().newRestorableContext(true);

ContextRestoreResponseHandler<T> responseHandler = new ContextRestoreResponseHandler<>(storedContextSupplier, handler);

I don't think it needs to be addressed in this PR since it's been there for a long while and no issue has been raised AFAIK. But maybe in a follow-up?

tvernum

LGTM

x-pack/qa/rolling-upgrade/build.gradle

...ling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/130_operator_privileges.yml

Co-authored-by: Tim Vernum <tim@adjective.org>

elasticsearchmachine · 2021-10-19T08:51:38Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts
❌	7.15	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 79412

When rewriting authentication for requests crossing nodes of different versions, we now preserve all request headers except the authentication one which needs to be rewritten. Previously all other request headers were dropped and it caused issue like an operator user not being recognised on the remote node. Other now preserved headers include audit and system index access. This new behaviour is more correct because we would never drop these headers if the nodes are on the same version. Resolves: elastic#79354

* Preserve request headers in a mixed version cluster (#79412) When rewriting authentication for requests crossing nodes of different versions, we now preserve all request headers except the authentication one which needs to be rewritten. Previously all other request headers were dropped and it caused issue like an operator user not being recognised on the remote node. Other now preserved headers include audit and system index access. This new behaviour is more correct because we would never drop these headers if the nodes are on the same version. Resolves: #79354 * for for 7.x quirks

When rewriting authentication for requests crossing nodes of different versions, we now preserve all request headers except the authentication one which needs to be rewritten. Previously all other request headers were dropped and it caused issue like an operator user not being recognised on the remote node. Other now preserved headers include audit and system index access. This new behaviour is more correct because we would never drop these headers if the nodes are on the same version. Resolves: #79354 Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* upstream/master: Validate tsdb's routing_path (elastic#79384) Adjust the BWC version for the return200ForClusterHealthTimeout field (elastic#79436) API for adding and removing indices from a data stream (elastic#79279) Exposing the ability to log deprecated settings at non-critical level (elastic#79107) Convert operator privilege license object to LicensedFeature (elastic#79407) Mute SnapshotBasedIndexRecoveryIT testSeqNoBasedRecoveryIsUsedAfterPrimaryFailOver (elastic#79456) Create cache files with CREATE_NEW & SPARSE options (elastic#79371) Revert "[ML] Use a new annotations index for future annotations (elastic#79151)" [ML] Use a new annotations index for future annotations (elastic#79151) [ML] Removing legacy code from ML/transform auditor (elastic#79434) Fix rate agg with custom `_doc_count` (elastic#79346) Optimize SLM Policy Queries (elastic#79341) Fix execution of exists query within nested queries on field with doc_values disabled (elastic#78841) Stricter UpdateSettingsRequest parsing on the REST layer (elastic#79227) Do not release snapshot file download permit during recovery retries (elastic#79409) Preserve request headers in a mixed version cluster (elastic#79412) Adjust versions after elastic#79044 backport to 7.x (elastic#79424) Mute BulkByScrollUsesAllScrollDocumentsAfterConflictsIntegTests (elastic#79429) Fail on SSPL licensed x-pack sources (elastic#79348) # Conflicts: # server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java

In elastic#79412 we fixed a bug that request headers got dropped when the request is sent across to a node of different version. The fix is to restore all existing request headers during the threadContext rewriting. However, there are headers that are always automatically preserved by the ThreadContext infrastructure, e.g. x-opaque-id. This causes failures when the code tries to re-add the x-opaque-id header since it already exists. An example of this issue is for CCS where the remote cluster is often on a different version compared to the local cluster. Resolves: elastic#79412

In #79412 we fixed a bug that request headers got dropped when the request is sent across to a node of different version. The fix is to restore all existing request headers during the threadContext rewriting. However, there are headers that are always automatically preserved by the ThreadContext infrastructure, e.g. x-opaque-id. This causes failures when the code tries to re-add the x-opaque-id header since it already exists. An example of this issue is for CCS where the remote cluster is often on a different version compared to the local cluster. Resolves: #79412

…79973) In elastic#79412 we fixed a bug that request headers got dropped when the request is sent across to a node of different version. The fix is to restore all existing request headers during the threadContext rewriting. However, there are headers that are always automatically preserved by the ThreadContext infrastructure, e.g. x-opaque-id. This causes failures when the code tries to re-add the x-opaque-id header since it already exists. An example of this issue is for CCS where the remote cluster is often on a different version compared to the local cluster. Resolves: elastic#79412

…79985) In #79412 we fixed a bug that request headers got dropped when the request is sent across to a node of different version. The fix is to restore all existing request headers during the threadContext rewriting. However, there are headers that are always automatically preserved by the ThreadContext infrastructure, e.g. x-opaque-id. This causes failures when the code tries to re-add the x-opaque-id header since it already exists. An example of this issue is for CCS where the remote cluster is often on a different version compared to the local cluster. Resolves: #79412

…79984) In #79412 we fixed a bug that request headers got dropped when the request is sent across to a node of different version. The fix is to restore all existing request headers during the threadContext rewriting. However, there are headers that are always automatically preserved by the ThreadContext infrastructure, e.g. x-opaque-id. This causes failures when the code tries to re-add the x-opaque-id header since it already exists. An example of this issue is for CCS where the remote cluster is often on a different version compared to the local cluster. Resolves: #79412

…79983) In #79412 we fixed a bug that request headers got dropped when the request is sent across to a node of different version. The fix is to restore all existing request headers during the threadContext rewriting. However, there are headers that are always automatically preserved by the ThreadContext infrastructure, e.g. x-opaque-id. This causes failures when the code tries to re-add the x-opaque-id header since it already exists. An example of this issue is for CCS where the remote cluster is often on a different version compared to the local cluster. Resolves: #79412 Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

ywangd added 2 commits October 19, 2021 12:03

wip

d7517d0

ywangd added >bug :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.0.0 v7.16.0 v7.15.2 labels Oct 19, 2021

ywangd requested a review from tvernum October 19, 2021 01:56

elasticmachine added the Team:Security Meta label for security team label Oct 19, 2021

ywangd commented Oct 19, 2021

View reviewed changes

checkstyle

e18a76d

tvernum approved these changes Oct 19, 2021

View reviewed changes

Apply suggestions from code review

5cf0bf0

Co-authored-by: Tim Vernum <tim@adjective.org>

ywangd added the auto-backport-and-merge label Oct 19, 2021

ywangd merged commit 75319f4 into elastic:master Oct 19, 2021

jakelandis added v8.0.0-beta1 and removed v8.0.0 labels Oct 27, 2021

Dosant mentioned this pull request Oct 27, 2021

x-opaque-id header breaks CCS in mixed version cluster #79948

Closed

ywangd mentioned this pull request Oct 28, 2021

Skip automatically preserved request headers when rewriting #79973

Merged

ywangd added v8.0.0 and removed v8.0.0-beta1 v8.0.0 labels Oct 28, 2021

ywangd added the v8.0.0-beta1 label Oct 28, 2021

ywangd mentioned this pull request Oct 28, 2021

Skip automatically preserved request headers when rewriting (#79973) #79984

Merged

ywangd mentioned this pull request Oct 28, 2021

Skip automatically preserved request headers when rewriting (#79973) #79985

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve request headers in a mixed version cluster#79412

Preserve request headers in a mixed version cluster#79412
ywangd merged 4 commits intoelastic:masterfrom
ywangd:es-79354-rewrite-security-headers

ywangd commented Oct 19, 2021

Uh oh!

elasticmachine commented Oct 19, 2021

Uh oh!

ywangd Oct 19, 2021 •

edited

Loading

Uh oh!

tvernum left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticsearchmachine commented Oct 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	securityContext.executeAfterRewritingAuthentication(original -> sendWithUser(connection, action, request, options,
	new ContextRestoreResponseHandler<>(threadPool.getThreadContext().wrapRestorable(original), handler), sender),
	minVersion);

	public Supplier<StoredContext> wrapRestorable(StoredContext storedContext) {
	return () -> {
	StoredContext context = newStoredContext(false);

	Supplier<ThreadContext.StoredContext> storedContextSupplier = threadPool.getThreadContext().newRestorableContext(true);
	ContextRestoreResponseHandler<T> responseHandler = new ContextRestoreResponseHandler<>(storedContextSupplier, handler);

Conversation

ywangd commented Oct 19, 2021

Uh oh!

elasticmachine commented Oct 19, 2021

Uh oh!

ywangd Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvernum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticsearchmachine commented Oct 19, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ywangd Oct 19, 2021 •

edited

Loading