Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security subject rebase #36

Draft
wants to merge 1 commit into
base: optimized-privilege-evaluation-security-subject-backup3
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1);

System.out.println("response: " + response.getBody());

assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
assertThat(response.getBody(), containsString(SystemIndexPlugin1.class.getCanonicalName()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

public class IndexDocumentIntoSystemIndexAction extends ActionType<IndexDocumentIntoSystemIndexResponse> {
public static final IndexDocumentIntoSystemIndexAction INSTANCE = new IndexDocumentIntoSystemIndexAction();
public static final String NAME = "mock:systemindex/index";
public static final String NAME = "cluster:mock/systemindex/index";

private IndexDocumentIntoSystemIndexAction() {
super(NAME, IndexDocumentIntoSystemIndexResponse::new);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

public class RunClusterHealthAction extends ActionType<RunClusterHealthResponse> {
public static final RunClusterHealthAction INSTANCE = new RunClusterHealthAction();
public static final String NAME = "mock:cluster/monitor/health";
public static final String NAME = "cluster:mock/monitor/health";

private RunClusterHealthAction() {
super(NAME, RunClusterHealthResponse::new);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ protected void doExecute(
String indexName = request.getIndexName();
String runAs = request.getRunAs();
Subject userSubject = identityService.getCurrentSubject();
System.out.println("User Subject: " + userSubject);
try {
contextSwitcher.runAs(() -> {
client.admin().indices().create(new CreateIndexRequest(indexName), ActionListener.wrap(r -> {
Expand All @@ -83,6 +84,7 @@ protected void doExecute(
.source("{\"content\":1}", XContentType.JSON),
ActionListener.wrap(r2 -> {
User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
System.out.println("Test User: " + user);
actionListener.onResponse(new IndexDocumentIntoSystemIndexResponse(true, user.getName()));
}, actionListener::onFailure)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,8 @@ public IndicesAndAliases(IndexSpec indexSpec, ActionSpec actionSpec, Statefulnes
Settings.EMPTY,
WellKnownActions.CLUSTER_ACTIONS,
WellKnownActions.INDEX_ACTIONS,
WellKnownActions.INDEX_ACTIONS
WellKnownActions.INDEX_ACTIONS,
Map.of()
);

if (statefulness == Statefulness.STATEFUL) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.util.Collection;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -57,7 +56,6 @@
import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS;
import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.RELOAD_CERTS_ACTION;
import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE;
import static org.junit.Assert.assertTrue;

/**
* Moved from https://github.com/opensearch-project/security/blob/54361468f5c4b3a57f3ecffaf1bbe8dccee562be/src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsTest.java
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -69,6 +70,7 @@
import org.opensearch.SpecialPermission;
import org.opensearch.Version;
import org.opensearch.action.ActionRequest;
import org.opensearch.action.bulk.BulkAction;
import org.opensearch.action.search.PitService;
import org.opensearch.action.search.SearchScrollAction;
import org.opensearch.action.support.ActionFilter;
Expand Down Expand Up @@ -2126,7 +2128,11 @@ public SecurityTokenManager getTokenManager() {

@Override
public PluginSubject getPluginSubject(Plugin plugin) {
return new ContextProvidingPluginSubject(threadPool, settings, plugin);
Set<String> clusterActions = new HashSet<>();
clusterActions.add(BulkAction.NAME);
PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin);
sf.updatePluginToClusterAction(subject.getPrincipal().getName(), clusterActions);
return subject;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ public void onFailure(Exception e) {
});
}
} else {
System.out.println("No permissions for " + user);
auditLog.logMissingPrivileges(action, request, task);
String err;
if (!pres.getMissingSecurityRoles().isEmpty()) {
Expand Down Expand Up @@ -529,6 +530,10 @@ private boolean checkImmutableIndices(Object request, ActionListener listener) {
return false;
}

public void updatePluginToClusterAction(String pluginIdentifier, Set<String> clusterActions) {
evalp.updatePluginToClusterActions(pluginIdentifier, clusterActions);
}

private boolean isRequestIndexImmutable(Object request) {
final IndexResolverReplacer.Resolved resolved = indexResolverReplacer.resolveRequest(request);
if (resolved.isLocalAll()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ public class ContextProvidingPluginSubject implements PluginSubject {
public ContextProvidingPluginSubject(ThreadPool threadPool, Settings settings, Plugin plugin) {
super();
this.threadPool = threadPool;
this.pluginPrincipal = new NamedPrincipal(plugin.getClass().getCanonicalName());
String principal = "plugin:" + plugin.getClass().getCanonicalName();
this.pluginPrincipal = new NamedPrincipal(principal);
// Convention for plugin username. Prefixed with 'plugin:'. ':' is forbidden from usernames, so this
// guarantees that a user with this username cannot be created by other means.
this.pluginUser = new User("plugin:" + pluginPrincipal.getName());
this.pluginUser = new User(principal);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ public ActionPrivileges(
Settings settings,
ImmutableSet<String> wellKnownClusterActions,
ImmutableSet<String> wellKnownIndexActions,
ImmutableSet<String> explicitlyRequiredIndexActions
ImmutableSet<String> explicitlyRequiredIndexActions,
Map<String, Set<String>> pluginToClusterActions
) {
this.cluster = new ClusterPrivileges(roles, actionGroups, wellKnownClusterActions);
this.cluster = new ClusterPrivileges(roles, actionGroups, wellKnownClusterActions, pluginToClusterActions);
this.index = new IndexPrivileges(roles, actionGroups, wellKnownIndexActions, explicitlyRequiredIndexActions);
this.roles = roles;
this.actionGroups = actionGroups;
Expand All @@ -142,7 +143,27 @@ public ActionPrivileges(
settings,
WellKnownActions.CLUSTER_ACTIONS,
WellKnownActions.INDEX_ACTIONS,
WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS
WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS,
Map.of()
);
}

public ActionPrivileges(
SecurityDynamicConfiguration<RoleV7> roles,
FlattenedActionGroups actionGroups,
Supplier<Map<String, IndexAbstraction>> indexMetadataSupplier,
Settings settings,
Map<String, Set<String>> pluginToClusterActions
) {
this(
roles,
actionGroups,
indexMetadataSupplier,
settings,
WellKnownActions.CLUSTER_ACTIONS,
WellKnownActions.INDEX_ACTIONS,
WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS,
pluginToClusterActions
);
}

Expand Down Expand Up @@ -375,6 +396,8 @@ static class ClusterPrivileges {
*/
private final ImmutableMap<String, WildcardMatcher> rolesToActionMatcher;

private final ImmutableMap<String, WildcardMatcher> usersToActionMatcher;

private final ImmutableSet<String> wellKnownClusterActions;

/**
Expand All @@ -388,14 +411,16 @@ static class ClusterPrivileges {
ClusterPrivileges(
SecurityDynamicConfiguration<RoleV7> roles,
FlattenedActionGroups actionGroups,
ImmutableSet<String> wellKnownClusterActions
ImmutableSet<String> wellKnownClusterActions,
Map<String, Set<String>> pluginToClusterActions
) {
DeduplicatingCompactSubSetBuilder<String> roleSetBuilder = new DeduplicatingCompactSubSetBuilder<>(
roles.getCEntries().keySet()
);
Map<String, DeduplicatingCompactSubSetBuilder.SubSetBuilder<String>> actionToRoles = new HashMap<>();
ImmutableSet.Builder<String> rolesWithWildcardPermissions = ImmutableSet.builder();
ImmutableMap.Builder<String, WildcardMatcher> rolesToActionMatcher = ImmutableMap.builder();
ImmutableMap.Builder<String, WildcardMatcher> usersToActionMatcher = ImmutableMap.builder();

for (Map.Entry<String, RoleV7> entry : roles.getCEntries().entrySet()) {
try {
Expand Down Expand Up @@ -445,13 +470,22 @@ static class ClusterPrivileges {
}
}

if (pluginToClusterActions != null) {
for (String pluginIdentifier : pluginToClusterActions.keySet()) {
Set<String> clusterActions = pluginToClusterActions.get(pluginIdentifier);
WildcardMatcher matcher = WildcardMatcher.from(clusterActions);
usersToActionMatcher.put(pluginIdentifier, matcher);
}
}

DeduplicatingCompactSubSetBuilder.Completed<String> completedRoleSetBuilder = roleSetBuilder.build();

this.actionToRoles = actionToRoles.entrySet()
.stream()
.collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, entry -> entry.getValue().build(completedRoleSetBuilder)));
this.rolesWithWildcardPermissions = rolesWithWildcardPermissions.build();
this.rolesToActionMatcher = rolesToActionMatcher.build();
this.usersToActionMatcher = usersToActionMatcher.build();
this.wellKnownClusterActions = wellKnownClusterActions;
}

Expand Down Expand Up @@ -485,6 +519,17 @@ PrivilegesEvaluatorResponse providesPrivilege(PrivilegesEvaluationContext contex
}
}

System.out.println("context: " + context);
System.out.println("usersToActionMatcher: " + usersToActionMatcher);

// 4: If plugin is performing the action, check if plugin has permission
if (context.getUser().isPluginUser()) {
WildcardMatcher matcher = this.usersToActionMatcher.get(context.getUser().getName());
if (matcher != null && matcher.test(action)) {
return PrivilegesEvaluatorResponse.ok();
}
}

return PrivilegesEvaluatorResponse.insufficient(action, context);
}

Expand Down Expand Up @@ -554,6 +599,16 @@ PrivilegesEvaluatorResponse providesAnyPrivilege(PrivilegesEvaluationContext con
}
}

// 4: If plugin is performing the action, check if plugin has permission
if (context.getUser().isPluginUser()) {
WildcardMatcher matcher = this.usersToActionMatcher.get(context.getUser().getName());
for (String action : actions) {
if (matcher != null && matcher.test(action)) {
return PrivilegesEvaluatorResponse.ok();
}
}
}

if (actions.size() == 1) {
return PrivilegesEvaluatorResponse.insufficient(actions.iterator().next(), context);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public class PrivilegesEvaluator {
private final boolean checkSnapshotRestoreWritePrivileges;

private final ClusterInfoHolder clusterInfoHolder;
private final ConfigurationRepository configurationRepository;
private ConfigModel configModel;
private final IndexResolverReplacer irr;
private final SnapshotRestoreEvaluator snapshotRestoreEvaluator;
Expand All @@ -153,6 +154,7 @@ public class PrivilegesEvaluator {
private DynamicConfigModel dcm;
private final NamedXContentRegistry namedXContentRegistry;
private final Settings settings;
private final Map<String, Set<String>> pluginToClusterActions;
private final AtomicReference<ActionPrivileges> actionPrivileges = new AtomicReference<>();

public PrivilegesEvaluator(
Expand All @@ -176,6 +178,7 @@ public PrivilegesEvaluator(

this.threadContext = threadContext;
this.privilegesInterceptor = privilegesInterceptor;
this.pluginToClusterActions = new HashMap<>();
this.clusterStateSupplier = clusterStateSupplier;
this.settings = settings;

Expand All @@ -192,6 +195,7 @@ public PrivilegesEvaluator(
termsAggregationEvaluator = new TermsAggregationEvaluator();
pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
this.namedXContentRegistry = namedXContentRegistry;
this.configurationRepository = configurationRepository;

if (configurationRepository != null) {
configurationRepository.subscribeOnChange(configMap -> {
Expand Down Expand Up @@ -228,11 +232,14 @@ void updateConfiguration(
? DynamicConfigFactory.addStatics(actionGroupsConfiguration.clone())
: DynamicConfigFactory.addStatics(SecurityDynamicConfiguration.empty(CType.ACTIONGROUPS));
FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsWithStatics);
System.out.println("updateConfiguration");
System.out.println("pluginToClusterActions: " + pluginToClusterActions);
ActionPrivileges actionPrivileges = new ActionPrivileges(
DynamicConfigFactory.addStatics(rolesConfiguration.clone()),
flattenedActionGroups,
() -> clusterStateSupplier.get().metadata().getIndicesLookup(),
settings
settings,
pluginToClusterActions
);
Metadata metadata = clusterStateSupplier.get().metadata();
actionPrivileges.updateStatefulIndexPrivileges(metadata.getIndicesLookup(), metadata.version());
Expand Down Expand Up @@ -389,23 +396,44 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)

// check snapshot/restore requests
if (snapshotRestoreEvaluator.evaluate(request, task, action0, clusterInfoHolder, presponse).isComplete()) {
return presponse;
if (!presponse.isAllowed()) {
return PrivilegesEvaluatorResponse.insufficient(action0, context);
} else {
return presponse;
}
}

System.out.println("Calling systemIndexAccessEvaluator.evaluate");
System.out.println("user: " + user);
System.out.println("action: " + action0);
// Security index access
if (systemIndexAccessEvaluator.evaluate(request, task, action0, requestedResolved, presponse, context, actionPrivileges, user)
.isComplete()) {
return presponse;
System.out.println("Returning presponse: " + presponse);
if (!presponse.isAllowed()) {
return PrivilegesEvaluatorResponse.insufficient(action0, context);
} else {
return presponse;
}
}
System.out.println("After systemIndexAccessEvaluator.evaluate");

// Protected index access
if (protectedIndexAccessEvaluator.evaluate(request, task, action0, requestedResolved, presponse, mappedRoles).isComplete()) {
return presponse;
if (!presponse.isAllowed()) {
return PrivilegesEvaluatorResponse.insufficient(action0, context);
} else {
return presponse;
}
}

// check access for point in time requests
if (pitPrivilegesEvaluator.evaluate(request, context, actionPrivileges, action0, presponse, irr).isComplete()) {
return presponse;
if (!presponse.isAllowed()) {
return PrivilegesEvaluatorResponse.insufficient(action0, context);
} else {
return presponse;
}
}

final boolean dnfofEnabled = dcm.isDnfofEnabled();
Expand Down Expand Up @@ -532,6 +560,7 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)

boolean dnfofPossible = dnfofEnabled && DNFOF_MATCHER.test(action0);

System.out.println("allIndexPermsRequired: " + allIndexPermsRequired);
presponse = actionPrivileges.hasIndexPrivilege(context, allIndexPermsRequired, requestedResolved);

if (presponse.isPartiallyOk()) {
Expand Down Expand Up @@ -839,4 +868,8 @@ private List<String> toString(List<AliasMetadata> aliases) {

return Collections.unmodifiableList(ret);
}

public void updatePluginToClusterActions(String pluginIdentifier, Set<String> clusterActions) {
pluginToClusterActions.put(pluginIdentifier, clusterActions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,6 @@ public String getPrivilegeMatrix() {
return result;
}

public boolean addMissingPrivileges(String action) {
return missingPrivileges.add(action);
}

public Set<String> getMissingSecurityRoles() {
return new HashSet<>(missingSecurityRoles);
}
Expand Down Expand Up @@ -192,6 +188,7 @@ public static PrivilegesEvaluatorResponse partiallyOk(
}

public static PrivilegesEvaluatorResponse insufficient(String missingPrivilege, PrivilegesEvaluationContext context) {
System.out.println("missingPrivilege: " + missingPrivilege);
PrivilegesEvaluatorResponse response = new PrivilegesEvaluatorResponse();
response.indexToActionCheckTable = CheckTable.create(ImmutableSet.of("_"), ImmutableSet.of(missingPrivilege));
return response;
Expand All @@ -201,6 +198,7 @@ public static PrivilegesEvaluatorResponse insufficient(
CheckTable<String, String> indexToActionCheckTable,
PrivilegesEvaluationContext context
) {
System.out.println("indexToActionCheckTable: " + indexToActionCheckTable);
PrivilegesEvaluatorResponse response = new PrivilegesEvaluatorResponse();
response.indexToActionCheckTable = indexToActionCheckTable;
return response;
Expand Down
Loading
Loading