Skip to content

Commit 13c2b80

Browse files
albertzaharovitsalbertzaharovits
committed
Ensure authz operation overrides transient authz headers (elastic#61621)
AuthorizationService#authorize uses the thread context to carry the result of the authorisation as transient headers. The listener argument to the `authorize` method must necessarily observe the header values. This PR makes it so that the authorisation transient headers (`_indices_permissions` and `_authz_info`, but NOT `_originating_action_name`) of the child action override the ones of the parent action. Co-authored-by: Tim Vernum [email protected]
1 parent e559f3c commit 13c2b80

File tree

11 files changed

+309
-95
lines changed

11 files changed

+309
-95
lines changed

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

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.io.IOException;
3636
import java.nio.charset.StandardCharsets;
3737
import java.util.ArrayList;
38+
import java.util.Collection;
3839
import java.util.Collections;
3940
import java.util.EnumSet;
4041
import java.util.HashMap;
@@ -177,12 +178,41 @@ public StoredContext stashAndMergeHeaders(Map<String, String> headers) {
177178
* @param preserveResponseHeaders if set to <code>true</code> the response headers of the restore thread will be preserved.
178179
*/
179180
public StoredContext newStoredContext(boolean preserveResponseHeaders) {
180-
final ThreadContextStruct context = threadLocal.get();
181-
return () -> {
182-
if (preserveResponseHeaders && threadLocal.get() != context) {
183-
threadLocal.set(context.putResponseHeaders(threadLocal.get().responseHeaders));
181+
return newStoredContext(preserveResponseHeaders, Collections.emptyList());
182+
}
183+
184+
/**
185+
* Just like {@link #stashContext()} but no default context is set. Instead, the {@code transientHeadersToClear} argument can be used
186+
* to clear specific transient headers in the new context. All headers (with the possible exception of {@code responseHeaders}) are
187+
* restored by closing the returned {@link StoredContext}.
188+
*
189+
* @param preserveResponseHeaders if set to <code>true</code> the response headers of the restore thread will be preserved.
190+
*/
191+
public StoredContext newStoredContext(boolean preserveResponseHeaders, Collection<String> transientHeadersToClear) {
192+
final ThreadContextStruct originalContext = threadLocal.get();
193+
// clear specific transient headers from the current context
194+
Map<String, Object> newTransientHeaders = null;
195+
for (String transientHeaderToClear : transientHeadersToClear) {
196+
if (originalContext.transientHeaders.containsKey(transientHeaderToClear)) {
197+
if (newTransientHeaders == null) {
198+
newTransientHeaders = new HashMap<>(originalContext.transientHeaders);
199+
}
200+
newTransientHeaders.remove(transientHeaderToClear);
201+
}
202+
}
203+
if (newTransientHeaders != null) {
204+
ThreadContextStruct threadContextStruct = new ThreadContextStruct(originalContext.requestHeaders,
205+
originalContext.responseHeaders, newTransientHeaders, originalContext.isSystemContext,
206+
originalContext.warningHeadersSize);
207+
threadLocal.set(threadContextStruct);
208+
}
209+
// this is the context when this method returns
210+
final ThreadContextStruct newContext = threadLocal.get();
211+
return () -> {
212+
if (preserveResponseHeaders && threadLocal.get() != newContext) {
213+
threadLocal.set(originalContext.putResponseHeaders(threadLocal.get().responseHeaders));
184214
} else {
185-
threadLocal.set(context);
215+
threadLocal.set(originalContext);
186216
}
187217
};
188218
}
@@ -483,7 +513,7 @@ private ThreadContextStruct putRequest(String key, String value) {
483513
return new ThreadContextStruct(newRequestHeaders, responseHeaders, transientHeaders, isSystemContext);
484514
}
485515

486-
private void putSingleHeader(String key, String value, Map<String, String> newHeaders) {
516+
private <T> void putSingleHeader(String key, T value, Map<String, T> newHeaders) {
487517
if (newHeaders.putIfAbsent(key, value) != null) {
488518
throw new IllegalArgumentException("value for key [" + key + "] already present");
489519
}
@@ -571,12 +601,9 @@ private ThreadContextStruct putResponse(final String key, final String value, fi
571601
return new ThreadContextStruct(requestHeaders, newResponseHeaders, transientHeaders, isSystemContext, newWarningHeaderSize);
572602
}
573603

574-
575604
private ThreadContextStruct putTransient(String key, Object value) {
576605
Map<String, Object> newTransient = new HashMap<>(this.transientHeaders);
577-
if (newTransient.putIfAbsent(key, value) != null) {
578-
throw new IllegalArgumentException("value for key [" + key + "] already present");
579-
}
606+
putSingleHeader(key, value, newTransient);
580607
return new ThreadContextStruct(requestHeaders, responseHeaders, newTransient, isSystemContext);
581608
}
582609

‎server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.test.ESTestCase;
2525

2626
import java.io.IOException;
27+
import java.util.Arrays;
2728
import java.util.Collections;
2829
import java.util.HashMap;
2930
import java.util.List;
@@ -55,6 +56,78 @@ public void testStashContext() {
5556
assertEquals("1", threadContext.getHeader("default"));
5657
}
5758

59+
public void testNewContextWithClearedTransients() {
60+
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
61+
threadContext.putTransient("foo", "bar");
62+
threadContext.putTransient("bar", "baz");
63+
threadContext.putHeader("foo", "bar");
64+
threadContext.putHeader("baz", "bar");
65+
threadContext.addResponseHeader("foo", "bar");
66+
threadContext.addResponseHeader("bar", "qux");
67+
68+
// this is missing or null
69+
if (randomBoolean()) {
70+
threadContext.putTransient("acme", null);
71+
}
72+
73+
// foo is the only existing transient header that is cleared
74+
try (ThreadContext.StoredContext stashed = threadContext.newStoredContext(false, randomFrom(Arrays.asList("foo", "foo"),
75+
Arrays.asList("foo"), Arrays.asList("foo", "acme")))) {
76+
// only the requested transient header is cleared
77+
assertNull(threadContext.getTransient("foo"));
78+
// missing header is still missing
79+
assertNull(threadContext.getTransient("acme"));
80+
// other headers are preserved
81+
assertEquals("baz", threadContext.getTransient("bar"));
82+
assertEquals("bar", threadContext.getHeader("foo"));
83+
assertEquals("bar", threadContext.getHeader("baz"));
84+
assertEquals("bar", threadContext.getResponseHeaders().get("foo").get(0));
85+
assertEquals("qux", threadContext.getResponseHeaders().get("bar").get(0));
86+
87+
// try override stashed header
88+
threadContext.putTransient("foo", "acme");
89+
assertEquals("acme", threadContext.getTransient("foo"));
90+
// add new headers
91+
threadContext.putTransient("baz", "bar");
92+
threadContext.putHeader("bar", "baz");
93+
threadContext.addResponseHeader("baz", "bar");
94+
threadContext.addResponseHeader("foo", "baz");
95+
}
96+
97+
// original is restored (it is not overridden)
98+
assertEquals("bar", threadContext.getTransient("foo"));
99+
// headers added inside the stash are NOT preserved
100+
assertNull(threadContext.getTransient("baz"));
101+
assertNull(threadContext.getHeader("bar"));
102+
assertNull(threadContext.getResponseHeaders().get("baz"));
103+
// original headers are restored
104+
assertEquals("bar", threadContext.getHeader("foo"));
105+
assertEquals("bar", threadContext.getHeader("baz"));
106+
assertEquals("bar", threadContext.getResponseHeaders().get("foo").get(0));
107+
assertEquals(1, threadContext.getResponseHeaders().get("foo").size());
108+
assertEquals("qux", threadContext.getResponseHeaders().get("bar").get(0));
109+
110+
// test stashed missing header stays missing
111+
try (ThreadContext.StoredContext stashed = threadContext.newStoredContext(randomBoolean(), randomFrom(Arrays.asList("acme", "acme"),
112+
Arrays.asList("acme")))) {
113+
assertNull(threadContext.getTransient("acme"));
114+
threadContext.putTransient("acme", "foo");
115+
}
116+
assertNull(threadContext.getTransient("acme"));
117+
118+
// test preserved response headers
119+
try (ThreadContext.StoredContext stashed = threadContext.newStoredContext(true, randomFrom(Arrays.asList("foo", "foo"),
120+
Arrays.asList("foo"), Arrays.asList("foo", "acme")))) {
121+
threadContext.addResponseHeader("baz", "bar");
122+
threadContext.addResponseHeader("foo", "baz");
123+
}
124+
assertEquals("bar", threadContext.getResponseHeaders().get("foo").get(0));
125+
assertEquals("baz", threadContext.getResponseHeaders().get("foo").get(1));
126+
assertEquals(2, threadContext.getResponseHeaders().get("foo").size());
127+
assertEquals("bar", threadContext.getResponseHeaders().get("baz").get(0));
128+
assertEquals(1, threadContext.getResponseHeaders().get("baz").size());
129+
}
130+
58131
public void testStashWithOrigin() {
59132
final String origin = randomAlphaOfLengthBetween(4, 16);
60133
final ThreadContext threadContext = new ThreadContext(Settings.EMPTY);

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationServiceField.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,18 @@
55
*/
66
package org.elasticsearch.xpack.core.security.authz;
77

8+
import java.util.Arrays;
9+
import java.util.Collection;
10+
811
public final class AuthorizationServiceField {
12+
913
public static final String INDICES_PERMISSIONS_KEY = "_indices_permissions";
14+
public static final String ORIGINATING_ACTION_KEY = "_originating_action_name";
15+
public static final String AUTHORIZATION_INFO_KEY = "_authz_info";
16+
17+
// Most often, transient authorisation headers are scoped (i.e. set, read and cleared) for the authorisation and execution
18+
// of individual actions (i.e. there is a different scope between the parent and the child actions)
19+
public static final Collection<String> ACTION_SCOPE_AUTHORIZATION_KEYS = Arrays.asList(INDICES_PERMISSIONS_KEY, AUTHORIZATION_INFO_KEY);
1020

1121
private AuthorizationServiceField() {}
1222
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.EmptyAuthorizationInfo;
5353
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.IndexAuthorizationResult;
5454
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.RequestInfo;
55-
import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
5655
import org.elasticsearch.xpack.core.security.authz.ResolvedIndices;
5756
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
5857
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
@@ -85,14 +84,16 @@
8584

8685
import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext;
8786
import static org.elasticsearch.xpack.core.security.SecurityField.setting;
87+
import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.ACTION_SCOPE_AUTHORIZATION_KEYS;
88+
import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.AUTHORIZATION_INFO_KEY;
89+
import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.INDICES_PERMISSIONS_KEY;
90+
import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.ORIGINATING_ACTION_KEY;
8891
import static org.elasticsearch.xpack.core.security.support.Exceptions.authorizationError;
8992
import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME;
9093

9194
public class AuthorizationService extends AbstractComponent {
9295
public static final Setting<Boolean> ANONYMOUS_AUTHORIZATION_EXCEPTION_SETTING =
9396
Setting.boolSetting(setting("authc.anonymous.authz_exception"), true, Property.NodeScope);
94-
public static final String ORIGINATING_ACTION_KEY = "_originating_action_name";
95-
public static final String AUTHORIZATION_INFO_KEY = "_authz_info";
9697
private static final AuthorizationInfo SYSTEM_AUTHZ_INFO =
9798
() -> Collections.singletonMap(PRINCIPAL_ROLES_FIELD_NAME, new String[] { SystemUser.ROLE_NAME });
9899

@@ -160,39 +161,51 @@ private AuthorizationInfo getAuthorizationInfoFromContext() {
160161
*/
161162
public void authorize(final Authentication authentication, final String action, final TransportRequest originalRequest,
162163
final ActionListener<Void> listener) throws ElasticsearchSecurityException {
163-
// prior to doing any authorization lets set the originating action in the context only
164-
putTransientIfNonExisting(ORIGINATING_ACTION_KEY, action);
165-
166-
String auditId = AuditUtil.extractRequestId(threadContext);
167-
if (auditId == null) {
168-
// We would like to assert that there is an existing request-id, but if this is a system action, then that might not be
169-
// true because the request-id is generated during authentication
170-
if (isInternalUser(authentication.getUser(), authentication.getVersion()) != false) {
171-
auditId = AuditUtil.getOrGenerateRequestId(threadContext);
172-
} else {
173-
auditTrail.tamperedRequest(null, authentication.getUser(), action, originalRequest);
174-
final String message = "Attempt to authorize action [" + action + "] for [" + authentication.getUser().principal()
175-
+ "] without an existing request-id";
176-
assert false : message;
177-
listener.onFailure(new ElasticsearchSecurityException(message));
164+
/* authorization fills in certain transient headers, which must be observed in the listener (action handler execution)
165+
* as well, but which must not bleed across different action context (eg parent-child action contexts).
166+
* <p>
167+
* Therefore we begin by clearing the existing ones up, as they might already be set during the authorization of a
168+
* previous parent action that ran under the same thread context (also on the same node).
169+
* When the returned {@code StoredContext} is closed, ALL the original headers are restored.
170+
*/
171+
try (ThreadContext.StoredContext ignore = threadContext.newStoredContext(false,
172+
ACTION_SCOPE_AUTHORIZATION_KEYS)) { // this does not clear {@code AuthorizationServiceField.ORIGINATING_ACTION_KEY}
173+
// prior to doing any authorization lets set the originating action in the thread context
174+
// the originating action is the current action if no originating action has yet been set in the current thread context
175+
// if there is already an original action, that stays put (eg. the current action is a child action)
176+
putTransientIfNonExisting(ORIGINATING_ACTION_KEY, action);
177+
178+
String auditId = AuditUtil.extractRequestId(threadContext);
179+
if (auditId == null) {
180+
// We would like to assert that there is an existing request-id, but if this is a system action, then that might not be
181+
// true because the request-id is generated during authentication
182+
if (isInternalUser(authentication.getUser(), authentication.getVersion()) != false) {
183+
auditId = AuditUtil.getOrGenerateRequestId(threadContext);
184+
} else {
185+
auditTrail.tamperedRequest(null, authentication.getUser(), action, originalRequest);
186+
final String message = "Attempt to authorize action [" + action + "] for [" + authentication.getUser().principal()
187+
+ "] without an existing request-id";
188+
assert false : message;
189+
listener.onFailure(new ElasticsearchSecurityException(message));
190+
}
178191
}
179-
}
180192

181-
// sometimes a request might be wrapped within another, which is the case for proxied
182-
// requests and concrete shard requests
183-
final TransportRequest unwrappedRequest = maybeUnwrapRequest(authentication, originalRequest, action, auditId);
184-
if (SystemUser.is(authentication.getUser())) {
185-
// this never goes async so no need to wrap the listener
186-
authorizeSystemUser(authentication, action, auditId, unwrappedRequest, listener);
187-
} else {
188-
final String finalAuditId = auditId;
189-
final RequestInfo requestInfo = new RequestInfo(authentication, unwrappedRequest, action);
190-
final ActionListener<AuthorizationInfo> authzInfoListener = wrapPreservingContext(ActionListener.wrap(
191-
authorizationInfo -> {
192-
putTransientIfNonExisting(AUTHORIZATION_INFO_KEY, authorizationInfo);
193-
maybeAuthorizeRunAs(requestInfo, finalAuditId, authorizationInfo, listener);
194-
}, listener::onFailure), threadContext);
195-
getAuthorizationEngine(authentication).resolveAuthorizationInfo(requestInfo, authzInfoListener);
193+
// sometimes a request might be wrapped within another, which is the case for proxied
194+
// requests and concrete shard requests
195+
final TransportRequest unwrappedRequest = maybeUnwrapRequest(authentication, originalRequest, action, auditId);
196+
if (SystemUser.is(authentication.getUser())) {
197+
// this never goes async so no need to wrap the listener
198+
authorizeSystemUser(authentication, action, auditId, unwrappedRequest, listener);
199+
} else {
200+
final String finalAuditId = auditId;
201+
final RequestInfo requestInfo = new RequestInfo(authentication, unwrappedRequest, action);
202+
final ActionListener<AuthorizationInfo> authzInfoListener = wrapPreservingContext(ActionListener.wrap(
203+
authorizationInfo -> {
204+
threadContext.putTransient(AUTHORIZATION_INFO_KEY, authorizationInfo);
205+
maybeAuthorizeRunAs(requestInfo, finalAuditId, authorizationInfo, listener);
206+
}, listener::onFailure), threadContext);
207+
getAuthorizationEngine(authentication).resolveAuthorizationInfo(requestInfo, authzInfoListener);
208+
}
196209
}
197210
}
198211

@@ -237,7 +250,7 @@ private void authorizeAction(final RequestInfo requestInfo, final String request
237250
if (ClusterPrivilege.ACTION_MATCHER.test(action)) {
238251
final ActionListener<AuthorizationResult> clusterAuthzListener =
239252
wrapPreservingContext(new AuthorizationResultListener<>(result -> {
240-
putTransientIfNonExisting(AuthorizationServiceField.INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL);
253+
threadContext.putTransient(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL);
241254
listener.onResponse(null);
242255
}, listener::onFailure, requestInfo, requestId, authzInfo), threadContext);
243256
authzEngine.authorizeClusterAction(requestInfo, authzInfo, clusterAuthzListener);
@@ -281,8 +294,7 @@ private void handleIndexActionAuthorizationResult(final IndexAuthorizationResult
281294
final TransportRequest request = requestInfo.getRequest();
282295
final String action = requestInfo.getAction();
283296
if (result.getIndicesAccessControl() != null) {
284-
putTransientIfNonExisting(AuthorizationServiceField.INDICES_PERMISSIONS_KEY,
285-
result.getIndicesAccessControl());
297+
threadContext.putTransient(INDICES_PERMISSIONS_KEY, result.getIndicesAccessControl());
286298
}
287299
//if we are creating an index we need to authorize potential aliases created at the same time
288300
if (IndexPrivilege.CREATE_INDEX_MATCHER.test(action)) {
@@ -371,8 +383,8 @@ private AuthorizationEngine getAuthorizationEngineForUser(final User user, final
371383
private void authorizeSystemUser(final Authentication authentication, final String action, final String requestId,
372384
final TransportRequest request, final ActionListener<Void> listener) {
373385
if (SystemUser.isAuthorized(action)) {
374-
putTransientIfNonExisting(AuthorizationServiceField.INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL);
375-
putTransientIfNonExisting(AUTHORIZATION_INFO_KEY, SYSTEM_AUTHZ_INFO);
386+
threadContext.putTransient(INDICES_PERMISSIONS_KEY, IndicesAccessControl.ALLOW_ALL);
387+
threadContext.putTransient(AUTHORIZATION_INFO_KEY, SYSTEM_AUTHZ_INFO);
376388
auditTrail.accessGranted(requestId, authentication, action, request, SYSTEM_AUTHZ_INFO);
377389
listener.onResponse(null);
378390
} else {

0 commit comments

Comments
 (0)