Skip to content

Commit

Permalink
Resolving wildcard application names without prefix query (#96479)
Browse files Browse the repository at this point in the history
For application privileges, today we use prefix query to resolve
application names with trailing wildcard. However, prefix query is
considered to be expensive and can be disabled if the cluster setting
search.allow_expensive_queries is set to false. When that happens it
breaks authorization in a surprising way.

This PR adds conditional logic to fallback to in-memory filtering for
application names when expensive queries are disabled. It is not less
expensive. But it avoids the surprising breakage.

Resolves: #96465
  • Loading branch information
ywangd authored Jun 5, 2023
1 parent cc07775 commit deba772
Show file tree
Hide file tree
Showing 5 changed files with 351 additions and 28 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/96479.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 96479
summary: Resolving wildcard application names without prefix query
area: Authorization
type: bug
issues:
- 96465
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.authz.store;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.ActionFuture;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsAction;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.test.SecuritySingleNodeTestCase;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyAction;
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyResponse;
import org.elasticsearch.xpack.core.security.action.privilege.GetPrivilegesRequestBuilder;
import org.elasticsearch.xpack.core.security.action.privilege.GetPrivilegesResponse;
import org.elasticsearch.xpack.core.security.action.privilege.PutPrivilegesAction;
import org.elasticsearch.xpack.core.security.action.privilege.PutPrivilegesRequest;
import org.elasticsearch.xpack.core.security.action.role.PutRoleAction;
import org.elasticsearch.xpack.core.security.action.role.PutRoleRequestBuilder;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateAction;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequest;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateResponse;
import org.elasticsearch.xpack.core.security.action.user.PutUserAction;
import org.elasticsearch.xpack.core.security.action.user.PutUserRequestBuilder;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
import org.junit.Before;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Base64;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import static java.util.Collections.emptyMap;
import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD;
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING;
import static org.hamcrest.Matchers.arrayWithSize;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

public class NativePrivilegeStoreSingleNodeTests extends SecuritySingleNodeTestCase {

@Before
public void configureApplicationPrivileges() {
final PutPrivilegesRequest putPrivilegesRequest = new PutPrivilegesRequest();
final List<ApplicationPrivilegeDescriptor> applicationPrivilegeDescriptors = Arrays.asList(
new ApplicationPrivilegeDescriptor("myapp-1", "read", Set.of("action:read"), emptyMap()),
new ApplicationPrivilegeDescriptor("myapp-1", "write", Set.of("action:write"), emptyMap()),
new ApplicationPrivilegeDescriptor("myapp-2", "read", Set.of("action:read"), emptyMap()),
new ApplicationPrivilegeDescriptor("myapp-2", "write", Set.of("action:write"), emptyMap()),
new ApplicationPrivilegeDescriptor("yourapp-1", "read", Set.of("action:read"), emptyMap()),
new ApplicationPrivilegeDescriptor("yourapp-1", "write", Set.of("action:write"), emptyMap()),
new ApplicationPrivilegeDescriptor("yourapp-2", "read", Set.of("action:read"), emptyMap()),
new ApplicationPrivilegeDescriptor("yourapp-2", "write", Set.of("action:write"), emptyMap()),
new ApplicationPrivilegeDescriptor("randomapp", "read", Set.of("action:read"), emptyMap()),
new ApplicationPrivilegeDescriptor("randomapp", "write", Set.of("action:write"), emptyMap())
);
putPrivilegesRequest.setPrivileges(applicationPrivilegeDescriptors);
client().execute(PutPrivilegesAction.INSTANCE, putPrivilegesRequest).actionGet();
}

public void testResolvePrivilegesWorkWhenExpensiveQueriesAreDisabled() throws IOException {
// Disable expensive query
new ClusterUpdateSettingsRequestBuilder(client(), ClusterUpdateSettingsAction.INSTANCE).setTransientSettings(
Settings.builder().put(ALLOW_EXPENSIVE_QUERIES.getKey(), false)
).execute().actionGet();

try {
// Prove that expensive queries are indeed disabled
final ActionFuture<SearchResponse> future = client().prepareSearch(".security")
.setQuery(QueryBuilders.prefixQuery("application", "my"))
.execute();
final ElasticsearchException e = expectThrows(ElasticsearchException.class, future::actionGet);
assertThat(
e.getCause().getMessage(),
containsString("[prefix] queries cannot be executed when 'search.allow_expensive_queries' is set to false")
);

// Get privileges work with wildcard application name
final GetPrivilegesResponse getPrivilegesResponse = new GetPrivilegesRequestBuilder(client()).application("yourapp*")
.execute()
.actionGet();
assertThat(getPrivilegesResponse.privileges(), arrayWithSize(4));
assertThat(
Arrays.stream(getPrivilegesResponse.privileges())
.map(ApplicationPrivilegeDescriptor::getApplication)
.collect(Collectors.toUnmodifiableSet()),
containsInAnyOrder("yourapp-1", "yourapp-2")
);

// User role resolution works with wildcard application name
new PutRoleRequestBuilder(client(), PutRoleAction.INSTANCE).source("app_user_role", new BytesArray("""
{
"cluster": ["manage_own_api_key"],
"applications": [
{
"application": "myapp-*",
"privileges": ["read"],
"resources": ["shared*"]
},
{
"application": "yourapp-1",
"privileges": ["read", "write"],
"resources": ["public"]
}
]
}
"""), XContentType.JSON).execute().actionGet();

new PutUserRequestBuilder(client(), PutUserAction.INSTANCE).username("app_user")
.password(TEST_PASSWORD_SECURE_STRING, getFastStoredHashAlgoForTests())
.roles("app_user_role")
.execute()
.actionGet();

Client appUserClient;
appUserClient = client().filterWithHeader(
Map.of(
"Authorization",
"Basic " + Base64.getEncoder().encodeToString(("app_user:" + TEST_PASSWORD).getBytes(StandardCharsets.UTF_8))
)
);
if (randomBoolean()) {
final var createApiKeyRequest = new CreateApiKeyRequest();
createApiKeyRequest.setName(randomAlphaOfLength(5));
if (randomBoolean()) {
createApiKeyRequest.setRoleDescriptors(
List.of(
new RoleDescriptor(
randomAlphaOfLength(5),
null,
null,
new RoleDescriptor.ApplicationResourcePrivileges[] {
RoleDescriptor.ApplicationResourcePrivileges.builder()
.application("myapp-*")
.privileges("read")
.resources("shared-common-*")
.build(),
RoleDescriptor.ApplicationResourcePrivileges.builder()
.application("yourapp-*")
.privileges("write")
.resources("public")
.build() },
null,
null,
null,
null
)
)
);
}
final CreateApiKeyResponse createApiKeyResponse = appUserClient.execute(CreateApiKeyAction.INSTANCE, createApiKeyRequest)
.actionGet();
appUserClient = client().filterWithHeader(
Map.of(
"Authorization",
"ApiKey "
+ Base64.getEncoder()
.encodeToString(
(createApiKeyResponse.getId() + ":" + createApiKeyResponse.getKey()).getBytes(StandardCharsets.UTF_8)
)
)
);
}

final AuthenticateResponse authenticateResponse = appUserClient.execute(
AuthenticateAction.INSTANCE,
AuthenticateRequest.INSTANCE
).actionGet();
assertThat(authenticateResponse.authentication().getEffectiveSubject().getUser().principal(), equalTo("app_user"));
} finally {
// Reset setting since test suite expects things in a clean slate
new ClusterUpdateSettingsRequestBuilder(client(), ClusterUpdateSettingsAction.INSTANCE).setTransientSettings(
Settings.builder().putNull(ALLOW_EXPENSIVE_QUERIES.getKey())
).execute().actionGet();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,8 @@ Collection<Object> createComponents(
settings,
client,
systemIndices.getMainIndexManager(),
cacheInvalidatorRegistry
cacheInvalidatorRegistry,
clusterService
);
components.add(privilegeStore);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.action.support.GroupedActionListener;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.cache.Cache;
Expand All @@ -28,6 +29,7 @@
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.index.query.BoolQueryBuilder;
Expand All @@ -47,6 +49,7 @@
import org.elasticsearch.xpack.core.security.action.privilege.ClearPrivilegesCacheResponse;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
import org.elasticsearch.xpack.core.security.support.StringMatcher;
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
import org.elasticsearch.xpack.security.support.LockingAtomicCounter;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;
Expand All @@ -59,11 +62,13 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collector;
import java.util.stream.Collectors;

import static org.elasticsearch.core.Strings.format;
import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN;
Expand Down Expand Up @@ -103,17 +108,21 @@ public class NativePrivilegeStore {
private final Settings settings;
private final Client client;
private final SecurityIndexManager securityIndexManager;
private volatile boolean allowExpensiveQueries;
private final DescriptorsAndApplicationNamesCache descriptorsAndApplicationNamesCache;

public NativePrivilegeStore(
Settings settings,
Client client,
SecurityIndexManager securityIndexManager,
CacheInvalidatorRegistry cacheInvalidatorRegistry
CacheInvalidatorRegistry cacheInvalidatorRegistry,
ClusterService clusterService
) {
this.settings = settings;
this.client = client;
this.securityIndexManager = securityIndexManager;
this.allowExpensiveQueries = ALLOW_EXPENSIVE_QUERIES.get(settings);
clusterService.getClusterSettings().addSettingsUpdateConsumer(ALLOW_EXPENSIVE_QUERIES, this::setAllowExpensiveQueries);
final TimeValue ttl = CACHE_TTL_SETTING.get(settings);
if (ttl.getNanos() > 0) {
descriptorsAndApplicationNamesCache = new DescriptorsAndApplicationNamesCache(
Expand Down Expand Up @@ -193,7 +202,16 @@ private void innerGetPrivileges(Collection<String> applications, ActionListener<
ApplicationPrivilegeDescriptor.Fields.TYPE.getPreferredName(),
DOC_TYPE_VALUE
);
final QueryBuilder query = QueryBuilders.boolQuery().filter(typeQuery).filter(getApplicationNameQuery(applications));
final Tuple<QueryBuilder, Predicate<String>> applicationNameQueryAndPredicate = getApplicationNameQueryAndPredicate(
applications
);

final QueryBuilder query;
if (applicationNameQueryAndPredicate.v1() != null) {
query = QueryBuilders.boolQuery().filter(typeQuery).filter(applicationNameQueryAndPredicate.v1());
} else {
query = QueryBuilders.boolQuery().filter(typeQuery);
}

final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
try (ThreadContext.StoredContext ignore = client.threadPool().getThreadContext().stashWithOrigin(SECURITY_ORIGIN)) {
Expand All @@ -209,16 +227,16 @@ private void innerGetPrivileges(Collection<String> applications, ActionListener<
client,
request,
new ContextPreservingActionListener<>(supplier, listener),
hit -> buildPrivilege(hit.getId(), hit.getSourceRef())
hit -> buildPrivilege(hit.getId(), hit.getSourceRef(), applicationNameQueryAndPredicate.v2())
);
}
});
}
}

private static QueryBuilder getApplicationNameQuery(Collection<String> applications) {
private Tuple<QueryBuilder, Predicate<String>> getApplicationNameQueryAndPredicate(Collection<String> applications) {
if (applications.contains("*")) {
return QueryBuilders.existsQuery(APPLICATION.getPreferredName());
return new Tuple<>(QueryBuilders.existsQuery(APPLICATION.getPreferredName()), null);
}
final List<String> rawNames = new ArrayList<>(applications.size());
final List<String> wildcardNames = new ArrayList<>(applications.size());
Expand All @@ -234,26 +252,43 @@ private static QueryBuilder getApplicationNameQuery(Collection<String> applicati

TermsQueryBuilder termsQuery = rawNames.isEmpty() ? null : QueryBuilders.termsQuery(APPLICATION.getPreferredName(), rawNames);
if (wildcardNames.isEmpty()) {
return termsQuery;
return new Tuple<>(termsQuery, null);
}
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
if (termsQuery != null) {
boolQuery.should(termsQuery);
}
for (String wildcard : wildcardNames) {
final String prefix = wildcard.substring(0, wildcard.length() - 1);
boolQuery.should(QueryBuilders.prefixQuery(APPLICATION.getPreferredName(), prefix));

if (allowExpensiveQueries) {
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
if (termsQuery != null) {
boolQuery.should(termsQuery);
}
for (String wildcard : wildcardNames) {
final String prefix = wildcard.substring(0, wildcard.length() - 1);
boolQuery.should(QueryBuilders.prefixQuery(APPLICATION.getPreferredName(), prefix));
}
boolQuery.minimumShouldMatch(1);
return new Tuple<>(boolQuery, null);
} else {
logger.trace("expensive queries are not allowed, switching to filtering application names in memory");
return new Tuple<>(null, StringMatcher.of(applications));
}
boolQuery.minimumShouldMatch(1);
return boolQuery;
}

private static ApplicationPrivilegeDescriptor buildPrivilege(String docId, BytesReference source) {
private void setAllowExpensiveQueries(boolean allowExpensiveQueries) {
this.allowExpensiveQueries = allowExpensiveQueries;
}

private static ApplicationPrivilegeDescriptor buildPrivilege(
String docId,
BytesReference source,
@Nullable Predicate<String> applicationNamePredicate
) {
logger.trace("Building privilege from [{}] [{}]", docId, source == null ? "<<null>>" : source.utf8ToString());
if (source == null) {
return null;
}
final Tuple<String, String> name = nameFromDocId(docId);
if (applicationNamePredicate != null && false == applicationNamePredicate.test(name.v1())) {
return null;
}
try {
// EMPTY is safe here because we never use namedObject

Expand Down
Loading

0 comments on commit deba772

Please sign in to comment.