Skip to content

Commit

Permalink
Separated DLS/FLS privilege evaluation from action privilege evaluati…
Browse files Browse the repository at this point in the history
…on (opensearch-project#4490)

Signed-off-by: Nils Bandener <[email protected]>
  • Loading branch information
nibix authored Jul 9, 2024
1 parent c00fdd4 commit dabff35
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1118,8 +1118,6 @@ public Collection<Object> createComponents(

final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);

// DLS-FLS is enabled if not client and not disabled and not SSL only.
final boolean dlsFlsEnabled = !SSLConfig.isSslOnlyMode();
evaluator = new PrivilegesEvaluator(
clusterService,
threadPool,
Expand All @@ -1130,7 +1128,6 @@ public Collection<Object> createComponents(
privilegesInterceptor,
cih,
irr,
dlsFlsEnabled,
namedXContentRegistry.get()
);

Expand Down Expand Up @@ -1169,6 +1166,9 @@ public Collection<Object> createComponents(
// Don't register if advanced modules is disabled in which case auditlog is instance of NullAuditLog
dcf.registerDCFListener(auditLog);
}
if (dlsFlsValve instanceof DlsFlsValveImpl) {
dcf.registerDCFListener(dlsFlsValve);
}

cr.setDynamicConfigFactory(dcf);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.opensearch.search.SearchHit;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.opensearch.security.privileges.DocumentAllowList;
import org.opensearch.security.privileges.PrivilegesEvaluationContext;
import org.opensearch.security.queries.QueryBuilderTraverser;
import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig;
Expand All @@ -74,11 +75,9 @@ public class DlsFilterLevelActionHandler {
);

public static boolean handle(
String action,
ActionRequest request,
ActionListener<?> listener,
PrivilegesEvaluationContext context,
EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
Resolved resolved,
ActionListener<?> listener,
Client nodeClient,
ClusterService clusterService,
IndicesService indicesService,
Expand All @@ -91,6 +90,9 @@ public static boolean handle(
return true;
}

String action = context.getAction();
ActionRequest request = context.getRequest();

if (action.startsWith("cluster:")
|| action.startsWith("indices:admin/template/")
|| action.startsWith("indices:admin/index_template/")) {
Expand All @@ -112,11 +114,9 @@ public static boolean handle(
}

return new DlsFilterLevelActionHandler(
action,
request,
listener,
context,
evaluatedDlsFlsConfig,
resolved,
listener,
nodeClient,
clusterService,
indicesService,
Expand All @@ -142,23 +142,21 @@ public static boolean handle(
private DocumentAllowList documentAllowlist;

DlsFilterLevelActionHandler(
String action,
ActionRequest request,
ActionListener<?> listener,
PrivilegesEvaluationContext context,
EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
Resolved resolved,
ActionListener<?> listener,
Client nodeClient,
ClusterService clusterService,
IndicesService indicesService,
IndexNameExpressionResolver resolver,
DlsQueryParser dlsQueryParser,
ThreadContext threadContext
) {
this.action = action;
this.request = request;
this.action = context.getAction();
this.request = context.getRequest();
this.listener = listener;
this.evaluatedDlsFlsConfig = evaluatedDlsFlsConfig;
this.resolved = resolved;
this.resolved = context.getResolvedRequest();
this.nodeClient = nodeClient;
this.clusterService = clusterService;
this.indicesService = indicesService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,16 @@

package org.opensearch.security.configuration;

import org.opensearch.action.ActionRequest;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.search.internal.SearchContext;
import org.opensearch.search.query.QuerySearchResult;
import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig;
import org.opensearch.security.privileges.PrivilegesEvaluationContext;
import org.opensearch.threadpool.ThreadPool;

public interface DlsFlsRequestValve {

boolean invoke(
String action,
ActionRequest request,
ActionListener<?> listener,
EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
Resolved resolved
);
boolean invoke(PrivilegesEvaluationContext context, ActionListener<?> listener);

void handleSearchContext(SearchContext context, ThreadPool threadPool, NamedXContentRegistry namedXContentRegistry);

Expand All @@ -52,13 +44,7 @@ boolean invoke(
public static class NoopDlsFlsRequestValve implements DlsFlsRequestValve {

@Override
public boolean invoke(
String action,
ActionRequest request,
ActionListener<?> listener,
EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
Resolved resolved
) {
public boolean invoke(PrivilegesEvaluationContext context, ActionListener<?> listener) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,18 @@
import org.opensearch.search.internal.SearchContext;
import org.opensearch.search.query.QuerySearchResult;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
import org.opensearch.security.privileges.PrivilegesEvaluationContext;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.securityconf.ConfigModel;
import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig;
import org.opensearch.security.support.Base64Helper;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.HeaderHelper;
import org.opensearch.security.support.SecurityUtils;
import org.opensearch.threadpool.ThreadPool;

import org.greenrobot.eventbus.Subscribe;

public class DlsFlsValveImpl implements DlsFlsRequestValve {

private static final String MAP_EXECUTION_HINT = "map";
Expand All @@ -91,6 +95,9 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve {
private final Mode mode;
private final DlsQueryParser dlsQueryParser;
private final IndexNameExpressionResolver resolver;
private final boolean dfmEmptyOverwritesAll;
private final NamedXContentRegistry namedXContentRegistry;
private volatile ConfigModel configModel;

public DlsFlsValveImpl(
Settings settings,
Expand All @@ -107,21 +114,29 @@ public DlsFlsValveImpl(
this.threadContext = threadContext;
this.mode = Mode.get(settings);
this.dlsQueryParser = new DlsQueryParser(namedXContentRegistry);
this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false);
this.namedXContentRegistry = namedXContentRegistry;
}

@Subscribe
public void onConfigModelChanged(ConfigModel configModel) {
this.configModel = configModel;
}

/**
*
* @param request
* @param listener
* @return false on error
*/
public boolean invoke(
String action,
ActionRequest request,
final ActionListener<?> listener,
EvaluatedDlsFlsConfig evaluatedDlsFlsConfig,
final Resolved resolved
) {
@Override
public boolean invoke(PrivilegesEvaluationContext context, final ActionListener<?> listener) {

EvaluatedDlsFlsConfig evaluatedDlsFlsConfig = configModel.getSecurityRoles()
.filter(context.getMappedRoles())
.getDlsFls(context.getUser(), dfmEmptyOverwritesAll, resolver, clusterService, namedXContentRegistry);

ActionRequest request = context.getRequest();
IndexResolverReplacer.Resolved resolved = context.getResolvedRequest();

if (log.isDebugEnabled()) {
log.debug(
Expand Down Expand Up @@ -288,7 +303,7 @@ public boolean invoke(
return false;
}

if (action.contains("plugins/replication")) {
if (context.getAction().contains("plugins/replication")) {
listener.onFailure(
new OpenSearchSecurityException(
"Cross Cluster Replication is not supported when FLS or DLS or Fieldmasking is activated",
Expand Down Expand Up @@ -324,11 +339,9 @@ public boolean invoke(

if (doFilterLevelDls && filteredDlsFlsConfig.hasDls()) {
return DlsFilterLevelActionHandler.handle(
action,
request,
listener,
context,
evaluatedDlsFlsConfig,
resolved,
listener,
nodeClient,
clusterService,
OpenSearchSecurityPlugin.GuiceHolder.getIndicesService(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import org.opensearch.security.configuration.CompatConfig;
import org.opensearch.security.configuration.DlsFlsRequestValve;
import org.opensearch.security.http.XFFResolver;
import org.opensearch.security.privileges.PrivilegesEvaluationContext;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.privileges.PrivilegesEvaluatorResponse;
import org.opensearch.security.resolver.IndexResolverReplacer;
Expand Down Expand Up @@ -378,7 +379,8 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
log.trace("Evaluate permissions for user: {}", user.getName());
}

final PrivilegesEvaluatorResponse pres = eval.evaluate(user, action, request, task, injectedRoles);
PrivilegesEvaluationContext context = eval.createContext(user, action, request, task, injectedRoles);
PrivilegesEvaluatorResponse pres = eval.evaluate(context);

if (log.isDebugEnabled()) {
log.debug(pres.toString());
Expand All @@ -387,7 +389,7 @@ private <Request extends ActionRequest, Response extends ActionResponse> void ap
if (pres.isAllowed()) {
auditLog.logGrantedPrivileges(action, request, task);
auditLog.logIndexEvent(action, request, task);
if (!dlsFlsValve.invoke(action, request, listener, pres.getEvaluatedDlsFlsConfig(), pres.getResolved())) {
if (!dlsFlsValve.invoke(context, listener)) {
return;
}
final CreateIndexRequestBuilder createIndexRequestBuilder = pres.getCreateIndexRequestBuilder();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/
package org.opensearch.security.privileges;

import com.google.common.collect.ImmutableSet;

import org.opensearch.action.ActionRequest;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.user.User;
import org.opensearch.tasks.Task;

/**
* Request-scoped context information for privilege evaluation.
*
* This class carries metadata about the request and provides caching facilities for data which might need to be
* evaluated several times per request.
*
* As this class is request-scoped, it is only used by a single thread. Thus, no thread synchronization mechanisms
* are necessary.
*/
public class PrivilegesEvaluationContext {
private final User user;
private final String action;
private final ActionRequest request;
private IndexResolverReplacer.Resolved resolvedRequest;
private final Task task;
private ImmutableSet<String> mappedRoles;
private final IndexResolverReplacer indexResolverReplacer;

public PrivilegesEvaluationContext(
User user,
ImmutableSet<String> mappedRoles,
String action,
ActionRequest request,
Task task,
IndexResolverReplacer indexResolverReplacer
) {
this.user = user;
this.mappedRoles = mappedRoles;
this.action = action;
this.request = request;
this.task = task;
this.indexResolverReplacer = indexResolverReplacer;
}

public User getUser() {
return user;
}

public String getAction() {
return action;
}

public ActionRequest getRequest() {
return request;
}

public IndexResolverReplacer.Resolved getResolvedRequest() {
IndexResolverReplacer.Resolved result = this.resolvedRequest;

if (result == null) {
result = indexResolverReplacer.resolveRequest(request);
this.resolvedRequest = result;
}

return result;
}

public Task getTask() {
return task;
}

public ImmutableSet<String> getMappedRoles() {
return mappedRoles;
}

/**
* Note: Ideally, mappedRoles would be an unmodifiable attribute. PrivilegesEvaluator however contains logic
* related to OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION which first validates roles and afterwards modifies
* them again. Thus, we need to be able to set this attribute.
*
* However, this method should be only used for this one particular phase. Normally, all roles should be determined
* upfront and stay constant during the whole privilege evaluation process.
*/
void setMappedRoles(ImmutableSet<String> mappedRoles) {
this.mappedRoles = mappedRoles;
}

}
Loading

0 comments on commit dabff35

Please sign in to comment.