Skip to content

Commit

Permalink
Tighten API key behaviour with DLS and incompatible license (elastic#…
Browse files Browse the repository at this point in the history
…78378) (elastic#79021)

* Tighten API key behaviour with DLS and incompatible license (elastic#78378)

When the cluster license is incompatible with DLS and FLS, instead of
failing open, API keys now fail closed by throwing an error if any of
the target indices has DLS/FLS protection.

* Fix for 7.x quirks

* more quirks
  • Loading branch information
ywangd authored Oct 13, 2021
1 parent b716a86 commit 1187088
Show file tree
Hide file tree
Showing 8 changed files with 201 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -66,6 +67,21 @@ public Collection<?> getDeniedIndices() {
.collect(Collectors.toSet()));
}

public boolean hasFieldOrDocumentLevelSecurity() {
return indexPermissions.values().stream().anyMatch(indexAccessControl ->
indexAccessControl.fieldPermissions.hasFieldLevelSecurity()
|| indexAccessControl.documentPermissions.hasDocumentLevelPermissions()
);
}

public List<String> getIndicesWithFieldOrDocumentLevelSecurity() {
return indexPermissions.entrySet().stream()
.filter(entry -> entry.getValue().fieldPermissions.hasFieldLevelSecurity()
|| entry.getValue().documentPermissions.hasDocumentLevelPermissions())
.map(Map.Entry::getKey)
.collect(Collectors.toList());
}

/**
* Encapsulates the field and document permissions for an index.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,12 @@ public final class IndicesPermission {
private final Map<String, Predicate<IndexAbstraction>> allowedIndicesMatchersForAction = new ConcurrentHashMap<>();

private final Group[] groups;
private final boolean hasFieldOrDocumentLevelSecurity;

public IndicesPermission(Group... groups) {
this.groups = groups;
this.hasFieldOrDocumentLevelSecurity = Arrays.stream(groups)
.anyMatch(g -> g.hasQuery() || g.fieldPermissions.hasFieldLevelSecurity());
}

private static StringMatcher indexMatcher(Collection<String> ordinaryIndices, Collection<String> restrictedIndices) {
Expand Down Expand Up @@ -86,6 +89,10 @@ public Predicate<IndexAbstraction> allowedIndicesMatcher(String action) {
return allowedIndicesMatchersForAction.computeIfAbsent(action, a -> Group.buildIndexMatcherPredicateForAction(a, groups));
}

public boolean hasFieldOrDocumentLevelSecurity() {
return hasFieldOrDocumentLevelSecurity;
}

/**
* Checks if the permission matches the provided action, without looking at indices.
* To be used in very specific cases where indices actions need to be authorized regardless of their indices.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public RunAsPermission runAs() {
throw new UnsupportedOperationException("cannot retrieve run_as permission on limited role");
}

@Override
public boolean hasFieldOrDocumentLevelSecurity() {
return super.hasFieldOrDocumentLevelSecurity() || limitedBy.hasFieldOrDocumentLevelSecurity();
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ public RunAsPermission runAs() {
return runAs;
}

public boolean hasFieldOrDocumentLevelSecurity() {
return indices.hasFieldOrDocumentLevelSecurity();
}

public static Builder builder(String... names) {
return new Builder(names);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.test.rest.yaml.ObjectPath;
import org.elasticsearch.xpack.security.authc.InternalRealms;

Expand Down Expand Up @@ -47,26 +49,42 @@ public void testWithBasicLicense() throws Exception {
public void testWithTrialLicense() throws Exception {
startTrial();
String accessToken = null;
String apiKeyCredentials = null;
String apiKeyCredentials1 = null;
String apiKeyCredentials2 = null;
boolean keyRoleHasDlsFls = false;
assertCreateIndex("index1");
assertCreateIndex("index2");
assertCreateIndex("index41");
assertCreateIndex("index42");
try {
checkLicenseType("trial");
checkSecurityEnabled(true);
checkAuthentication();
checkHasPrivileges();
checkIndexWrite();
accessToken = getAccessToken();
apiKeyCredentials = getApiKeyCredentials();
apiKeyCredentials1 = getApiKeyCredentials();
assertAuthenticateWithToken(accessToken, true);
assertAuthenticateWithApiKey(apiKeyCredentials, true);
assertAuthenticateWithApiKey(apiKeyCredentials1, true);
assertAddRoleWithDLS(true);
assertAddRoleWithFLS(true);
final Tuple<String, Boolean> tuple = assertCreateApiKeyWithDlsFls();
apiKeyCredentials2 = tuple.v1();
keyRoleHasDlsFls = tuple.v2();
assertReadWithApiKey(apiKeyCredentials2, "/index*/_search", true);
} finally {
revertTrial();
assertAuthenticateWithToken(accessToken, false);
assertAuthenticateWithApiKey(apiKeyCredentials, true);
assertAuthenticateWithApiKey(apiKeyCredentials1, true);
assertFailToGetToken();
assertAddRoleWithDLS(false);
assertAddRoleWithFLS(false);
// Any indices with DLS/FLS cannot be searched with the API key when the license is on Basic
assertReadWithApiKey(apiKeyCredentials2, "/index*/_search", false);
assertReadWithApiKey(apiKeyCredentials2, "/index1,index2/_search", false);
assertReadWithApiKey(apiKeyCredentials2, "/index41/_search", false == keyRoleHasDlsFls);
assertReadWithApiKey(apiKeyCredentials2, "/index42/_search", true);
assertReadWithApiKey(apiKeyCredentials2, "/index1/_doc/1", false);
}
}

Expand Down Expand Up @@ -247,6 +265,10 @@ private void assertAddRoleWithDLS(boolean shouldSucceed) throws IOException {
" \"names\": [ \"index1\", \"index2\" ],\n" +
" \"privileges\": [\"all\"],\n" +
" \"query\": \"{\\\"match\\\": {\\\"title\\\": \\\"foo\\\"}}\" \n" +
" },\n" +
" {\n" +
" \"names\": [ \"index41\", \"index42\" ],\n" +
" \"privileges\": [\"read\"]\n" +
" }\n" +
" ],\n" +
" \"run_as\": [ \"other_user\" ],\n" +
Expand All @@ -265,7 +287,7 @@ private void assertAddRoleWithDLS(boolean shouldSucceed) throws IOException {
}

private void assertAddRoleWithFLS(boolean shouldSucceed) throws IOException {
final Request addRole = new Request("POST", "/_security/role/dlsrole");
final Request addRole = new Request("POST", "/_security/role/flsrole");
addRole.setJsonEntity("{\n" +
" \"cluster\": [\"all\"],\n" +
" \"indices\": [\n" +
Expand All @@ -275,6 +297,10 @@ private void assertAddRoleWithFLS(boolean shouldSucceed) throws IOException {
" \"field_security\" : { // optional\n" +
" \"grant\" : [ \"title\", \"body\" ]\n" +
" }\n" +
" },\n" +
" {\n" +
" \"names\": [ \"index41\", \"index42\" ],\n" +
" \"privileges\": [\"read\"]\n" +
" }\n" +
" ],\n" +
" \"run_as\": [ \"other_user\" ],\n" +
Expand All @@ -291,4 +317,66 @@ private void assertAddRoleWithFLS(boolean shouldSucceed) throws IOException {
assertThat(e.getMessage(), containsString("current license is non-compliant for [field and document level security]"));
}
}

private void createUserWithDlsOrFlsRole() throws IOException {
final Request request = new Request("PUT", "/_security/user/dls_fls_user");
request.setJsonEntity("{\"password\":\"superstrongpassword\"," +
"\"roles\":[\"" + (randomBoolean() ? "dlsrole" : "flsrole") + "\"]}");
assertOK(adminClient().performRequest(request));
}

private Tuple<String, Boolean> assertCreateApiKeyWithDlsFls() throws IOException {
createUserWithDlsOrFlsRole();

final Request request = new Request("POST", "/_security/api_key");
final boolean keyRoleHasDlsFls = randomBoolean();
if (keyRoleHasDlsFls) {
if (randomBoolean()) {
request.setJsonEntity("{\"name\":\"my-key\",\"role_descriptors\":" +
"{\"a\":{\"indices\":[" +
"{\"names\":[\"index41\"],\"privileges\":[\"read\"]," +
"\"query\":{\"term\":{\"tag\":{\"value\":\"prod\"}}}}," +
"{\"names\":[\"index1\",\"index2\",\"index42\"],\"privileges\":[\"read\"]}" +
"]}}}");
} else {
request.setJsonEntity(
"{\"name\":\"my-key\",\"role_descriptors\":" +
"{\"a\":{\"indices\":[" +
"{\"names\":[\"index41\"],\"privileges\":[\"read\"]," +
"\"field_security\":{\"grant\":[\"tag\"]}}," +
"{\"names\":[\"index1\",\"index2\",\"index42\"],\"privileges\":[\"read\"]}" +
"]}}}");
}
} else {
request.setJsonEntity("{\"name\":\"my-key\",\"role_descriptors\":" +
"{\"a\":{\"indices\":[{\"names\":[\"index1\",\"index2\",\"index41\",\"index42\"],\"privileges\":[\"read\"]}]}}}");
}
request.setOptions(request.getOptions().toBuilder().addHeader("Authorization",
basicAuthHeaderValue("dls_fls_user", new SecureString("superstrongpassword".toCharArray()))));

final Response response = client().performRequest(request);
assertOK(response);
return new Tuple<>((String) responseAsMap(response).get("encoded"), keyRoleHasDlsFls);
}

private void assertCreateIndex(String indexName) throws IOException {
final Request request = new Request("PUT", indexName);
assertOK(adminClient().performRequest(request));
}

private void assertReadWithApiKey(String apiKeyCredentials, String path, boolean shouldSucceed) throws IOException {
final Request request = new Request("GET", path);
final RequestOptions.Builder options = request.getOptions().toBuilder();
options.addHeader(HttpHeaders.AUTHORIZATION, "ApiKey " + apiKeyCredentials);
request.setOptions(options);

if (shouldSucceed) {
assertOK(client().performRequest(request));
} else {
final ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(request));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(403));
assertThat(e.getMessage(), containsString("current license is non-compliant for [field and document level security]"));
assertThat(e.getMessage(), containsString("indices_with_dls_or_fls"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@
import org.elasticsearch.xpack.security.authz.accesscontrol.OptOutQueryCache;
import org.elasticsearch.xpack.security.authz.interceptor.BulkShardRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.IndicesAliasesRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.DlsFlsLicenseComplianceRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.RequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.ResizeRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.SearchRequestInterceptor;
Expand Down Expand Up @@ -624,7 +625,8 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
new SearchRequestInterceptor(threadPool, getLicenseState(), clusterService),
new ShardSearchRequestInterceptor(threadPool, getLicenseState(), clusterService),
new UpdateRequestInterceptor(threadPool, getLicenseState()),
new BulkShardRequestInterceptor(threadPool, getLicenseState())
new BulkShardRequestInterceptor(threadPool, getLicenseState()),
new DlsFlsLicenseComplianceRequestInterceptor(threadPool.getThreadContext(), getLicenseState())
));
}
requestInterceptors = Collections.unmodifiableSet(requestInterceptors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,13 @@ private static RBACAuthorizationInfo ensureRBAC(AuthorizationInfo authorizationI
return (RBACAuthorizationInfo) authorizationInfo;
}

public static Role maybeGetRBACEngineRole(AuthorizationInfo authorizationInfo) {
if (authorizationInfo instanceof RBACAuthorizationInfo) {
return ((RBACAuthorizationInfo) authorizationInfo).getRole();
}
return null;
}

private static boolean checkChangePasswordAction(Authentication authentication) {
// we need to verify that this user was authenticated by or looked up by a realm type that support password changes
// otherwise we open ourselves up to issues where a user in a different realm could be created with the same username
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* 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.interceptor;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.license.LicenseUtils;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.transport.TransportActionProxy;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
import org.elasticsearch.xpack.core.security.authz.permission.Role;
import org.elasticsearch.xpack.security.authz.RBACEngine;

import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.AUTHORIZATION_INFO_KEY;
import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.INDICES_PERMISSIONS_KEY;

public class DlsFlsLicenseComplianceRequestInterceptor implements RequestInterceptor {
private static final Logger logger = LogManager.getLogger(DlsFlsLicenseComplianceRequestInterceptor.class);

private final ThreadContext threadContext;
private final XPackLicenseState licenseState;

public DlsFlsLicenseComplianceRequestInterceptor(ThreadContext threadContext, XPackLicenseState licenseState) {
this.threadContext = threadContext;
this.licenseState = licenseState;
}

@Override
public void intercept(
AuthorizationEngine.RequestInfo requestInfo,
AuthorizationEngine authorizationEngine,
AuthorizationInfo authorizationInfo,
ActionListener<Void> listener) {

if (requestInfo.getRequest() instanceof IndicesRequest && false == TransportActionProxy.isProxyAction(requestInfo.getAction())) {
if (false == licenseState.isAllowed(XPackLicenseState.Feature.SECURITY_DLS_FLS)) {
final Role role = RBACEngine.maybeGetRBACEngineRole(threadContext.getTransient(AUTHORIZATION_INFO_KEY));
if (role == null || role.hasFieldOrDocumentLevelSecurity()) {
logger.trace("Role has DLS or FLS and license is incompatible. " +
"Checking for whether the request touches any indices that have DLS or FLS configured");
final IndicesAccessControl indicesAccessControl = threadContext.getTransient(INDICES_PERMISSIONS_KEY);
if (indicesAccessControl != null && indicesAccessControl.hasFieldOrDocumentLevelSecurity()) {
final ElasticsearchSecurityException licenseException =
LicenseUtils.newComplianceException("field and document level security");
licenseException.addMetadata(
"es.indices_with_dls_or_fls", indicesAccessControl.getIndicesWithFieldOrDocumentLevelSecurity());
listener.onFailure(licenseException);
return;
}
}
}
}
listener.onResponse(null);
}
}

0 comments on commit 1187088

Please sign in to comment.