Skip to content

Commit

Permalink
Fix security origin for TokenService#findActiveTokensFor... (#47418)
Browse files Browse the repository at this point in the history 
All internal searches (triggered by APIs) across the .security index
must be performed while "under the security origin". Otherwise,
the search is performed in the context of the caller which most
likely does not have privileges to search .security (hopefully).
This commit fixes this in the case of two methods in the
TokenService and corrects an overly done such context switch
in the ApiKeyService.

In addition, this makes all tests from the client/rest-high-level
module execute as an all mighty administrator,
but not a literal superuser.

Closes #47151
  • Loading branch information
@albertzaharovits
albertzaharovits committed Oct 21, 2019
1 parent afeee4b commit 9964d89
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 43 deletions.
5 changes: 4 additions & 1 deletion client/rest-high-level/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,11 @@ testClusters.integTest {

setting 'indices.lifecycle.poll_interval', '1000ms'
keystore 'xpack.security.transport.ssl.truststore.secure_password', 'testnode'
extraConfigFile 'roles.yml', file('roles.yml')
user username: System.getProperty('tests.rest.cluster.username', 'test_user'),
password: System.getProperty('tests.rest.cluster.password', 'test-password')
password: System.getProperty('tests.rest.cluster.password', 'test-password'),
role: System.getProperty('tests.rest.cluster.role', 'admin')
user username: 'admin_user', password: 'admin-password'

extraConfigFile nodeCert.name, nodeCert
extraConfigFile nodeTrustStore.name, nodeTrustStore
Expand Down
12 changes: 12 additions & 0 deletions client/rest-high-level/roles.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
admin:
cluster:
- all
indices:
- names: '*'
privileges:
- all
run_as: [ '*' ]
applications:
- application: '*'
privileges: [ '*' ]
resources: [ '*' ]
14 changes: 13 additions & 1 deletion ...h-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@
import org.elasticsearch.client.security.user.privileges.UserIndicesPrivileges;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.util.set.Sets;
import org.hamcrest.Matchers;

Expand All @@ -122,6 +124,8 @@
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;

import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
Expand All @@ -139,6 +143,14 @@

public class SecurityDocumentationIT extends ESRestHighLevelClientTestCase {

@Override
protected Settings restAdminSettings() {
String token = basicAuthHeaderValue("admin_user", new SecureString("admin-password".toCharArray()));
return Settings.builder()
.put(ThreadContext.PREFIX + ".Authorization", token)
.build();
}

public void testGetUsers() throws Exception {
final RestHighLevelClient client = highLevelClient();
String[] usernames = new String[] {"user1", "user2", "user3"};
Expand Down Expand Up @@ -739,7 +751,7 @@ public void testAuthenticate() throws Exception {
//end::authenticate-response

assertThat(user.getUsername(), is("test_user"));
assertThat(user.getRoles(), contains(new String[]{"superuser"}));
assertThat(user.getRoles(), contains(new String[]{"admin"}));
assertThat(user.getFullName(), nullValue());
assertThat(user.getEmail(), nullValue());
assertThat(user.getMetadata().isEmpty(), is(true));
Expand Down
41 changes: 22 additions & 19 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.action.index.IndexAction;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.ContextPreservingActionListener;
import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
import org.elasticsearch.action.update.UpdateRequest;
import org.elasticsearch.action.update.UpdateResponse;
Expand Down Expand Up @@ -92,6 +93,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
Expand Down Expand Up @@ -659,28 +661,29 @@ private void findApiKeys(final BoolQueryBuilder boolQuery, boolean filterOutInva
expiredQuery.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery("expiration_time")));
boolQuery.filter(expiredQuery);
}
final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(SECURITY_ORIGIN)) {
final SearchRequest request = client.prepareSearch(SECURITY_MAIN_ALIAS)
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
.setVersion(false)
.setSize(1000)
.setFetchSource(true)
.request();
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
.setVersion(false)
.setSize(1000)
.setFetchSource(true)
.request();
securityIndex.checkIndexVersionThenExecute(listener::onFailure,
() -> ScrollHelper.fetchAllByEntity(client, request, listener,
(SearchHit hit) -> {
Map<String, Object> source = hit.getSourceAsMap();
String name = (String) source.get("name");
String id = hit.getId();
Long creation = (Long) source.get("creation_time");
Long expiration = (Long) source.get("expiration_time");
Boolean invalidated = (Boolean) source.get("api_key_invalidated");
String username = (String) ((Map<String, Object>) source.get("creator")).get("principal");
String realm = (String) ((Map<String, Object>) source.get("creator")).get("realm");
return new ApiKey(name, id, Instant.ofEpochMilli(creation),
(expiration != null) ? Instant.ofEpochMilli(expiration) : null, invalidated, username, realm);
}));
() -> ScrollHelper.fetchAllByEntity(client, request, new ContextPreservingActionListener<>(supplier, listener),
(SearchHit hit) -> {
Map<String, Object> source = hit.getSourceAsMap();
String name = (String) source.get("name");
String id = hit.getId();
Long creation = (Long) source.get("creation_time");
Long expiration = (Long) source.get("expiration_time");
Boolean invalidated = (Boolean) source.get("api_key_invalidated");
String username = (String) ((Map<String, Object>) source.get("creator")).get("principal");
String realm = (String) ((Map<String, Object>) source.get("creator")).get("realm");
return new ApiKey(name, id, Instant.ofEpochMilli(creation),
(expiration != null) ? Instant.ofEpochMilli(expiration) : null, invalidated, username, realm);
}));
}
}

Expand Down
43 changes: 26 additions & 17 deletions ...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.ContextPreservingActionListener;
import org.elasticsearch.action.support.TransportActions;
import org.elasticsearch.action.support.WriteRequest.RefreshPolicy;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
Expand Down Expand Up @@ -135,6 +136,7 @@
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.elasticsearch.action.support.TransportActions.isShardNotAvailableException;
Expand Down Expand Up @@ -1240,17 +1242,20 @@ public void findActiveTokensForRealm(String realmName, @Nullable Predicate<Map<S
- TimeValue.timeValueHours(ExpiredTokenRemover.MAXIMUM_TOKEN_LIFETIME_HOURS).millis()))
)
);
final SearchRequest request = client.prepareSearch(indicesWithTokens.toArray(new String[0]))
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
.setVersion(false)
.setSize(1000)
.setFetchSource(true)
.request();
ScrollHelper.fetchAllByEntity(client, request, listener, (SearchHit hit) -> filterAndParseHit(hit, filter));
final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(SECURITY_ORIGIN)) {
final SearchRequest request = client.prepareSearch(indicesWithTokens.toArray(new String[0]))
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
.setVersion(false)
.setSize(1000)
.setFetchSource(true)
.request();
ScrollHelper.fetchAllByEntity(client, request, new ContextPreservingActionListener<>(supplier, listener),
(SearchHit hit) -> filterAndParseHit(hit, filter));
}
}
}, listener::onFailure));

}

/**
Expand Down Expand Up @@ -1284,14 +1289,18 @@ public void findActiveTokensForUser(String username, ActionListener<Collection<T
- TimeValue.timeValueHours(ExpiredTokenRemover.MAXIMUM_TOKEN_LIFETIME_HOURS).millis()))
)
);
final SearchRequest request = client.prepareSearch(indicesWithTokens.toArray(new String[0]))
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
.setVersion(false)
.setSize(1000)
.setFetchSource(true)
.request();
ScrollHelper.fetchAllByEntity(client, request, listener, (SearchHit hit) -> filterAndParseHit(hit, isOfUser(username)));
final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(SECURITY_ORIGIN)) {
final SearchRequest request = client.prepareSearch(indicesWithTokens.toArray(new String[0]))
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
.setVersion(false)
.setSize(1000)
.setFetchSource(true)
.request();
ScrollHelper.fetchAllByEntity(client, request, new ContextPreservingActionListener<>(supplier, listener),
(SearchHit hit) -> filterAndParseHit(hit, isOfUser(username)));
}
}
}, listener::onFailure));
}
Expand Down
Loading

0 comments on commit 9964d89

Please sign in to comment.