diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index c3e00da109..dadabfb65a 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1118,8 +1118,6 @@ public Collection 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, @@ -1130,7 +1128,6 @@ public Collection createComponents( privilegesInterceptor, cih, irr, - dlsFlsEnabled, namedXContentRegistry.get() ); diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsRequestValve.java b/src/main/java/org/opensearch/security/configuration/DlsFlsRequestValve.java index 1152799bd5..e70f9c8024 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsRequestValve.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsRequestValve.java @@ -31,19 +31,12 @@ 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(String action, ActionRequest request, ActionListener listener, PrivilegesEvaluationContext context); void handleSearchContext(SearchContext context, ThreadPool threadPool, NamedXContentRegistry namedXContentRegistry); @@ -52,13 +45,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(String action, ActionRequest request, ActionListener listener, PrivilegesEvaluationContext context) { return true; } diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 855db9e896..66a66abb89 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -72,7 +72,9 @@ 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; @@ -80,6 +82,8 @@ 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"; @@ -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, @@ -107,6 +114,13 @@ 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; } /** @@ -115,13 +129,14 @@ public DlsFlsValveImpl( * @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(String action, ActionRequest request, final ActionListener listener, PrivilegesEvaluationContext context) { + + EvaluatedDlsFlsConfig evaluatedDlsFlsConfig = configModel.getSecurityRoles() + .filter(context.getMappedRoles()) + .getDlsFls(context.getUser(), dfmEmptyOverwritesAll, resolver, clusterService, namedXContentRegistry); + + IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); if (log.isDebugEnabled()) { log.debug( @@ -129,7 +144,7 @@ public boolean invoke( + request + "\nevaluatedDlsFlsConfig: " + evaluatedDlsFlsConfig - + "\nresolved: " + + "\ncontext: " + resolved + "\nmode: " + mode diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java index b9d4a73967..509a407196 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityFilter.java @@ -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; @@ -378,7 +379,15 @@ private void ap log.trace("Evaluate permissions for user: {}", user.getName()); } - final PrivilegesEvaluatorResponse pres = eval.evaluate(user, action, request, task, injectedRoles); + PrivilegesEvaluationContext context = null; + PrivilegesEvaluatorResponse pres; + + try { + context = eval.createContext(user, action, request, task, injectedRoles); + pres = eval.evaluate(context); + } catch (PrivilegesEvaluatorResponse.NotAllowedException e) { + pres = e.getResponse(); + } if (log.isDebugEnabled()) { log.debug(pres.toString()); @@ -387,7 +396,7 @@ private 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(action, request, listener, context)) { return; } final CreateIndexRequestBuilder createIndexRequestBuilder = pres.getCreateIndexRequestBuilder(); diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java index 7ddbb1e1b5..3d15315704 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java @@ -16,10 +16,13 @@ import com.google.common.collect.ImmutableSet; +import org.opensearch.action.ActionRequest; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.security.resolver.IndexResolverReplacer; import org.opensearch.security.support.WildcardMatcher; import org.opensearch.security.user.User; +import org.opensearch.tasks.Task; /** * Request-scoped context information for privilege evaluation. @@ -31,10 +34,11 @@ * are necessary. */ public class PrivilegesEvaluationContext { - private boolean resolveLocalAll = true; private final User user; private final String action; - private final Object request; + private final ActionRequest request; + private IndexResolverReplacer.Resolved resolvedRequest; + private final Task task; /** * This caches the ready to use WildcardMatcher instances for the current request. Many index patterns have @@ -43,23 +47,27 @@ public class PrivilegesEvaluationContext { */ private final Map renderedPatternTemplateCache = new HashMap<>(); private final ImmutableSet mappedRoles; - private final Supplier clusterStateSupplier; + private final IndexResolverReplacer indexResolverReplacer; private final IndexNameExpressionResolver indexNameExpressionResolver; public PrivilegesEvaluationContext( User user, ImmutableSet mappedRoles, String action, - Object request, + ActionRequest request, + Task task, Supplier clusterStateSupplier, + IndexResolverReplacer indexResolverReplacer, IndexNameExpressionResolver indexNameExpressionResolver ) { this.user = user; this.mappedRoles = mappedRoles; this.action = action; this.request = request; + this.task = task; this.clusterStateSupplier = clusterStateSupplier; + this.indexResolverReplacer = indexResolverReplacer; this.indexNameExpressionResolver = indexNameExpressionResolver; } @@ -88,10 +96,25 @@ public String getAction() { return action; } - public Object getRequest() { + 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 getMappedRoles() { return mappedRoles; } @@ -100,7 +123,16 @@ public PrivilegesEvaluationContext mappedRoles(ImmutableSet mappedRoles) if (this.mappedRoles != null && this.mappedRoles.equals(mappedRoles)) { return this; } else { - return new PrivilegesEvaluationContext(user, mappedRoles, action, request, clusterStateSupplier, indexNameExpressionResolver); + return new PrivilegesEvaluationContext( + user, + mappedRoles, + action, + request, + task, + clusterStateSupplier, + indexResolverReplacer, + indexNameExpressionResolver + ); } } diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 4d5cebe2ab..2c69872676 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -147,8 +147,6 @@ public class PrivilegesEvaluator { private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator; private final TermsAggregationEvaluator termsAggregationEvaluator; private final PitPrivilegesEvaluator pitPrivilegesEvaluator; - private final boolean dlsFlsEnabled; - private final boolean dfmEmptyOverwritesAll; private DynamicConfigModel dcm; private final NamedXContentRegistry namedXContentRegistry; private volatile ActionPrivileges actionPrivileges; @@ -163,7 +161,6 @@ public PrivilegesEvaluator( final PrivilegesInterceptor privilegesInterceptor, final ClusterInfoHolder clusterInfoHolder, final IndexResolverReplacer irr, - boolean dlsFlsEnabled, NamedXContentRegistry namedXContentRegistry ) { @@ -188,8 +185,6 @@ public PrivilegesEvaluator( termsAggregationEvaluator = new TermsAggregationEvaluator(); pitPrivilegesEvaluator = new PitPrivilegesEvaluator(); this.namedXContentRegistry = namedXContentRegistry; - this.dlsFlsEnabled = dlsFlsEnabled; - this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false); configurationRepository.subscribeOnChange(configMap -> { try { @@ -279,18 +274,45 @@ private void setUserInfoInThreadContext(User user) { } } - public PrivilegesEvaluatorResponse evaluate( - final User user, - String action0, - final ActionRequest request, - Task task, - final Set injectedRoles - ) { + public PrivilegesEvaluationContext createContext(User user, String action0, ActionRequest request, Task task, Set injectedRoles) + throws PrivilegesEvaluatorResponse.NotAllowedException { + if (!isInitialized()) { + throw new OpenSearchSecurityException("OpenSearch Security is not initialized."); + } + + TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); + ImmutableSet mappedRoles = ImmutableSet.copyOf((injectedRoles == null) ? mapRoles(user, caller) : injectedRoles); + final String injectedRolesValidationString = threadContext.getTransient( + ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION + ); + if (injectedRolesValidationString != null) { + HashSet injectedRolesValidationSet = new HashSet<>(Arrays.asList(injectedRolesValidationString.split(","))); + if (!mappedRoles.containsAll(injectedRolesValidationSet)) { + PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse(); + presponse.allowed = false; + presponse.missingSecurityRoles.addAll(injectedRolesValidationSet); + presponse.reason("Injected roles validation failed"); + log.info("Roles {} are not mapped to the user {}", injectedRolesValidationSet, user); + throw new PrivilegesEvaluatorResponse.NotAllowedException(presponse); + } + mappedRoles = ImmutableSet.copyOf(injectedRolesValidationSet); + } + + return new PrivilegesEvaluationContext(user, mappedRoles, action0, request, task, clusterService::state, irr, resolver); + } + + public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) { if (!isInitialized()) { throw new OpenSearchSecurityException("OpenSearch Security is not initialized."); } + String action0 = context.getAction(); + ImmutableSet mappedRoles = context.getMappedRoles(); + User user = context.getUser(); + ActionRequest request = context.getRequest(); + Task task = context.getTask(); + if (action0.startsWith("internal:indices/admin/upgrade")) { action0 = "indices:admin/upgrade"; } @@ -305,21 +327,6 @@ public PrivilegesEvaluatorResponse evaluate( PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse(); - final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); - ImmutableSet mappedRoles = ImmutableSet.copyOf((injectedRoles == null) ? mapRoles(user, caller) : injectedRoles); - final String injectedRolesValidationString = threadContext.getTransient( - ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION - ); - if (injectedRolesValidationString != null) { - HashSet injectedRolesValidationSet = new HashSet<>(Arrays.asList(injectedRolesValidationString.split(","))); - if (!mappedRoles.containsAll(injectedRolesValidationSet)) { - presponse.allowed = false; - presponse.missingSecurityRoles.addAll(injectedRolesValidationSet); - log.info("Roles {} are not mapped to the user {}", injectedRolesValidationSet, user); - return presponse; - } - mappedRoles = ImmutableSet.copyOf(injectedRolesValidationSet); - } presponse.resolvedSecurityRoles.addAll(mappedRoles); final SecurityRoles securityRoles = getSecurityRoles(mappedRoles); @@ -339,15 +346,6 @@ public PrivilegesEvaluatorResponse evaluate( throw new OpenSearchSecurityException("OpenSearch Security is not initialized: roles configuration is missing"); } - PrivilegesEvaluationContext context = new PrivilegesEvaluationContext( - user, - mappedRoles, - action0, - request, - clusterService::state, - resolver - ); - if (request instanceof BulkRequest && (Strings.isNullOrEmpty(user.getRequestedTenant()))) { // Shortcut for bulk actions. The details are checked on the lower level of the BulkShardRequests (Action // indices:data/write/bulk[s]). @@ -370,8 +368,7 @@ public PrivilegesEvaluatorResponse evaluate( return presponse; } - final Resolved requestedResolved = irr.resolveRequest(request); - presponse.resolved = requestedResolved; + final Resolved requestedResolved = context.getResolvedRequest(); if (isDebugEnabled) { log.debug("RequestedResolved : {}", requestedResolved); @@ -416,14 +413,6 @@ public PrivilegesEvaluatorResponse evaluate( log.trace("dnfof enabled? {}", dnfofEnabled); } - presponse.evaluatedDlsFlsConfig = getSecurityRoles(mappedRoles).getDlsFls( - user, - dfmEmptyOverwritesAll, - resolver, - clusterService, - namedXContentRegistry - ); - final boolean serviceAccountUser = user.isServiceAccount(); if (isClusterPerm(action0)) { if (serviceAccountUser) { @@ -500,7 +489,11 @@ public PrivilegesEvaluatorResponse evaluate( // final String[] allIndexPermsRequiredA = allIndexPermsRequired.toArray(new String[0]); if (isDebugEnabled) { - log.debug("Requested {} from {}", allIndexPermsRequired, caller); + log.debug( + "Requested {} from {}", + allIndexPermsRequired, + threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS) + ); } if (isDebugEnabled) { diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java index 26aaefe796..25b4a50594 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java @@ -33,8 +33,6 @@ import com.google.common.collect.ImmutableSet; import org.opensearch.action.admin.indices.create.CreateIndexRequestBuilder; -import org.opensearch.security.resolver.IndexResolverReplacer.Resolved; -import org.opensearch.security.securityconf.EvaluatedDlsFlsConfig; import com.selectivem.check.CheckTable; @@ -42,18 +40,12 @@ public class PrivilegesEvaluatorResponse { boolean allowed = false; Set missingSecurityRoles = new HashSet<>(); Set resolvedSecurityRoles = new HashSet<>(); - EvaluatedDlsFlsConfig evaluatedDlsFlsConfig; PrivilegesEvaluatorResponseState state = PrivilegesEvaluatorResponseState.PENDING; - Resolved resolved; CreateIndexRequestBuilder createIndexRequestBuilder; private Set onlyAllowedForIndices = ImmutableSet.of(); private CheckTable indexToActionCheckTable; private String reason; - public Resolved getResolved() { - return resolved; - } - /** * Returns true if the request can be fully allowed. See also isAllowedForSpecificIndices(). */ @@ -97,10 +89,6 @@ public Set getResolvedSecurityRoles() { return new HashSet<>(resolvedSecurityRoles); } - public EvaluatedDlsFlsConfig getEvaluatedDlsFlsConfig() { - return evaluatedDlsFlsConfig; - } - public CreateIndexRequestBuilder getCreateIndexRequestBuilder() { return createIndexRequestBuilder; } @@ -131,8 +119,6 @@ public String toString() { + onlyAllowedForIndices + ",\n" + (indexToActionCheckTable != null ? indexToActionCheckTable.toTableString("ok", "MISSING") : "") - + ",\nevaluatedDlsFlsConfig=" - + evaluatedDlsFlsConfig + "]"; } @@ -176,4 +162,27 @@ public static enum PrivilegesEvaluatorResponseState { COMPLETE; } + public static class NotAllowedException extends Exception { + private final PrivilegesEvaluatorResponse response; + + public NotAllowedException(PrivilegesEvaluatorResponse response) { + super(response.reason); + this.response = response; + } + + public NotAllowedException(PrivilegesEvaluatorResponse response, String message) { + super(message); + this.response = response; + } + + public NotAllowedException(PrivilegesEvaluatorResponse response, String message, Throwable cause) { + super(message, cause); + this.response = response; + } + + public PrivilegesEvaluatorResponse getResponse() { + return response; + } + } + } diff --git a/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java index 7b835ccd25..5f83f90cd7 100644 --- a/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java +++ b/src/test/java/org/opensearch/security/privileges/ActionPrivilegesTest.java @@ -582,6 +582,8 @@ static PrivilegesEvaluationContext ctx(String... roles) { null, null, null, + null, + null, new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)) ); } diff --git a/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java index aa474c9ae9..f9720cca1a 100644 --- a/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java @@ -156,7 +156,16 @@ public void setup( } PrivilegesEvaluationContext ctx(String action) { - return new PrivilegesEvaluationContext(user, ImmutableSet.of("role_a"), action, request, null, indexNameExpressionResolver); + return new PrivilegesEvaluationContext( + user, + ImmutableSet.of("role_a"), + action, + request, + null, + null, + null, + indexNameExpressionResolver + ); } @After