Skip to content

Commit

Permalink
Point in time security changes (#2094)
Browse files Browse the repository at this point in the history
Description
For 'Delete PIT' and 'PIT segments' API, when PIT IDs are passed as part of request, this custom evaluator decode the PITs to indices and resolve the indices with user permissions.
If user has permission to all indices of PIT, then PIT is permitted to the user.
Only when the user has permissions for all PITs in the request, then we allow the operation.
For requests which operates on 'all' PITs, we skip the custom evaluator and evaluate via standard code

Alias and data stream behavior :
PIT IDs always contain the resolved indices ( underlying indices ) when saved.
Based on this,
For alias, user must have either 'index' or 'alias' permission for any PIT operation.
For data stream, user must have both 'data stream' AND 'backing indices of data stream' permission ( eg : data-stream-11 + .ds-my-data-stream11-000001 ) for any PIT operation.
With just data stream permission, user will be able to create pit but will not be able to use the PIT ID for other operations such as search without backing indices permission.

Signed-off-by: Bharathwaj G <[email protected]>

Signed-off-by: Bharathwaj G <[email protected]>
(cherry picked from commit 207cfcc)
  • Loading branch information
bharath-techie authored and github-actions[bot] committed Nov 2, 2022
1 parent 18cd908 commit e9d7322
Show file tree
Hide file tree
Showing 11 changed files with 465 additions and 9 deletions.
9 changes: 9 additions & 0 deletions config/roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,12 @@ snapshot_management_read_access:
- 'cluster:admin/opensearch/snapshot_management/policy/explain'
- 'cluster:admin/repository/get'
- 'cluster:admin/snapshot/get'

# Allows user to use point in time functionality
point_in_time_full_access:
reserved: true
index_permissions:
- index_patterns:
- '*'
allowed_actions:
- 'manage_point_in_time'
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import org.opensearch.Version;
import org.opensearch.action.ActionRequest;
import org.opensearch.action.ActionResponse;
import org.opensearch.action.search.PitService;
import org.opensearch.action.search.SearchScrollAction;
import org.opensearch.action.support.ActionFilter;
import org.opensearch.client.Client;
Expand Down Expand Up @@ -1161,12 +1162,15 @@ public static class GuiceHolder implements LifecycleComponent {
private static RemoteClusterService remoteClusterService;
private static IndicesService indicesService;

private static PitService pitService;

@Inject
public GuiceHolder(final RepositoriesService repositoriesService,
final TransportService remoteClusterService, IndicesService indicesService) {
final TransportService remoteClusterService, IndicesService indicesService, PitService pitService) {
GuiceHolder.repositoriesService = repositoriesService;
GuiceHolder.remoteClusterService = remoteClusterService.getRemoteClusterService();
GuiceHolder.indicesService = indicesService;
GuiceHolder.pitService = pitService;
}

public static RepositoriesService getRepositoriesService() {
Expand All @@ -1180,6 +1184,8 @@ public static RemoteClusterService getRemoteClusterService() {
public static IndicesService getIndicesService() {
return indicesService;
}

public static PitService getPitService() { return pitService; }

@Override
public void close() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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 java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import org.opensearch.action.ActionRequest;
import org.opensearch.action.admin.indices.segments.PitSegmentsRequest;
import org.opensearch.action.search.CreatePitRequest;
import org.opensearch.action.search.DeletePitRequest;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.security.OpenSearchSecurityPlugin;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.securityconf.SecurityRoles;
import org.opensearch.security.user.User;


/**
* This class evaluates privileges for point in time (Delete and List all) operations.
* For aliases - users must have either alias permission or backing index permissions
* For data streams - users must have access to backing indices permission + data streams permission.
*/
public class PitPrivilegesEvaluator {

public PrivilegesEvaluatorResponse evaluate(final ActionRequest request, final ClusterService clusterService,
final User user, final SecurityRoles securityRoles, final String action,
final IndexNameExpressionResolver resolver,
final PrivilegesEvaluatorResponse presponse,
final IndexResolverReplacer irr) {

if(!(request instanceof DeletePitRequest || request instanceof PitSegmentsRequest)) {
return presponse;
}
List<String> pitIds = new ArrayList<>();

if (request instanceof DeletePitRequest) {
DeletePitRequest deletePitRequest = (DeletePitRequest) request;
pitIds = deletePitRequest.getPitIds();
} else if(request instanceof PitSegmentsRequest) {
PitSegmentsRequest pitSegmentsRequest = (PitSegmentsRequest) request;
pitIds = pitSegmentsRequest.getPitIds();
}
// if request is for all PIT IDs, skip custom pit ids evaluation
if (pitIds.size() == 1 && "_all".equals(pitIds.get(0))) {
return presponse;
} else {
return handlePitsAccess(pitIds, clusterService, user, securityRoles,
action, resolver, presponse, irr);
}
}

/**
* Handle access for delete operation / pit segments operation where PIT IDs are explicitly passed
*/
private PrivilegesEvaluatorResponse handlePitsAccess(List<String> pitIds, ClusterService clusterService,
User user, SecurityRoles securityRoles, final String action,
IndexNameExpressionResolver resolver, PrivilegesEvaluatorResponse presponse,
final IndexResolverReplacer irr) {
Map<String, String[]> pitToIndicesMap = OpenSearchSecurityPlugin.
GuiceHolder.getPitService().getIndicesForPits(pitIds);
Set<String> pitIndices = new HashSet<>();
// add indices across all PITs to a set and evaluate if user has access to all indices
for(String[] indices: pitToIndicesMap.values()) {
pitIndices.addAll(Arrays.asList(indices));
}
Set<String> allPermittedIndices = getPermittedIndices(pitIndices, clusterService, user,
securityRoles, action, resolver, irr);
// Only if user has access to all PIT's indices, allow operation, otherwise continue evaluation in PrivilegesEvaluator.
if(allPermittedIndices.containsAll(pitIndices)) {
presponse.allowed = true;
presponse.markComplete();
}
return presponse;
}

/**
* This method returns list of permitted indices for the PIT indices passed
*/
private Set<String> getPermittedIndices(Set<String> pitIndices, ClusterService clusterService,
User user, SecurityRoles securityRoles, final String action,
IndexNameExpressionResolver resolver, final IndexResolverReplacer irr) {
String[] indicesArr = new String[pitIndices.size()];
CreatePitRequest req = new CreatePitRequest(new TimeValue(1, TimeUnit.DAYS), true,
pitIndices.toArray(indicesArr));
final IndexResolverReplacer.Resolved pitResolved = irr.resolveRequest(req);
return securityRoles.reduce(pitResolved,
user, new String[]{action}, resolver, clusterService);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public class PrivilegesEvaluator {
private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator;
private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator;
private final TermsAggregationEvaluator termsAggregationEvaluator;
private final PitPrivilegesEvaluator pitPrivilegesEvaluator;
private final boolean dlsFlsEnabled;
private final boolean dfmEmptyOverwritesAll;
private DynamicConfigModel dcm;
Expand Down Expand Up @@ -158,6 +159,7 @@ public PrivilegesEvaluator(final ClusterService clusterService, final ThreadPool
securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(settings, auditLog, irr);
protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog);
termsAggregationEvaluator = new TermsAggregationEvaluator();
pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
this.namedXContentRegistry = namedXContentRegistry;
this.dlsFlsEnabled = dlsFlsEnabled;
this.dfmEmptyOverwritesAll = settings.getAsBoolean(ConfigConstants.SECURITY_DFM_EMPTY_OVERRIDES_ALL, false);
Expand Down Expand Up @@ -282,6 +284,12 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin
return presponse;
}

// check access for point in time requests
if(pitPrivilegesEvaluator.evaluate(request, clusterService, user, securityRoles,
action0, resolver, presponse, irr).isComplete()) {
return presponse;
}

final boolean dnfofEnabled = dcm.isDnfofEnabled();

final boolean isTraceEnabled = log.isTraceEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,11 @@ public final static class Resolved {
private final boolean isLocalAll;
private final IndicesOptions indicesOptions;

private Resolved(final ImmutableSet<String> aliases,
final ImmutableSet<String> allIndices,
final ImmutableSet<String> originalRequested,
final ImmutableSet<String> remoteIndices,
IndicesOptions indicesOptions) {
public Resolved(final ImmutableSet<String> aliases,
final ImmutableSet<String> allIndices,
final ImmutableSet<String> originalRequested,
final ImmutableSet<String> remoteIndices,
IndicesOptions indicesOptions) {
this.aliases = aliases;
this.allIndices = allIndices;
this.originalRequested = originalRequested;
Expand Down
7 changes: 4 additions & 3 deletions src/main/resources/static_config/static_action_groups.yml
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,9 @@ manage_point_in_time:
static: true
allowed_actions:
- "indices:data/read/point_in_time/create"
- "cluster:admin/point_in_time/delete"
- "cluster:admin/point_in_time/read*"
- "indices:data/read/point_in_time/delete"
- "indices:data/read/point_in_time/readall"
- "indices:data/read/search"
- "indices:monitor/point_in_time/segments"
type: "cluster"
type: "index"
description: "Manage point in time actions"
Loading

0 comments on commit e9d7322

Please sign in to comment.