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

Implement resource permission evaluation in security #4638

Closed
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 @@ -167,6 +167,7 @@
import org.opensearch.security.identity.SecurityTokenManager;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.privileges.PrivilegesInterceptor;
import org.opensearch.security.privileges.ResourceAccessEvaluator;
import org.opensearch.security.privileges.RestLayerPrivilegesEvaluator;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.rest.DashboardsInfoAction;
Expand Down Expand Up @@ -246,6 +247,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile PrivilegesEvaluator evaluator;
private volatile UserService userService;
private volatile RestLayerPrivilegesEvaluator restLayerEvaluator;
private volatile ResourceAccessEvaluator resourceAccessEvaluator;
private volatile ConfigurationRepository cr;
private volatile AdminDNs adminDns;
private volatile ClusterService cs;
Expand Down Expand Up @@ -1139,6 +1141,8 @@ public Collection<Object> createComponents(

restLayerEvaluator = new RestLayerPrivilegesEvaluator(clusterService, threadPool);

resourceAccessEvaluator = new ResourceAccessEvaluator(clusterService, threadPool);

securityRestHandler = new SecurityRestFilter(
backendRegistry,
restLayerEvaluator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ public class PrivilegesEvaluator {
private final SnapshotRestoreEvaluator snapshotRestoreEvaluator;
private final SystemIndexAccessEvaluator systemIndexAccessEvaluator;
private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator;
private final ResourceAccessEvaluator resourceAccessEvaluator;
private final TermsAggregationEvaluator termsAggregationEvaluator;
private final PitPrivilegesEvaluator pitPrivilegesEvaluator;
private DynamicConfigModel dcm;
Expand Down Expand Up @@ -174,6 +175,7 @@ public PrivilegesEvaluator(
snapshotRestoreEvaluator = new SnapshotRestoreEvaluator(settings, auditLog);
systemIndexAccessEvaluator = new SystemIndexAccessEvaluator(settings, auditLog, irr);
protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog);
resourceAccessEvaluator = new ResourceAccessEvaluator();
termsAggregationEvaluator = new TermsAggregationEvaluator();
pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
this.namedXContentRegistry = namedXContentRegistry;
Expand Down Expand Up @@ -347,6 +349,10 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
return presponse;
}

if (resourceAccessEvaluator.evaluate(request, action0, securityRoles, user, clusterService).isComplete()) {
return presponse;
}

// check access for point in time requests
if (pitPrivilegesEvaluator.evaluate(request, clusterService, user, securityRoles, action0, resolver, presponse, irr).isComplete()) {
return presponse;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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 org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.greenrobot.eventbus.Subscribe;
import org.opensearch.OpenSearchSecurityException;
import org.opensearch.action.ActionRequest;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.core.common.transport.TransportAddress;
import org.opensearch.security.securityconf.ConfigModel;
import org.opensearch.security.securityconf.SecurityRoles;
import org.opensearch.security.user.User;

import java.util.List;
import java.util.Set;

public class ResourceAccessEvaluator {
protected final Logger log = LogManager.getLogger(this.getClass());
private ConfigModel configModel;

public ResourceAccessEvaluator() {}

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

boolean isInitialized() {
return configModel != null && configModel.getSecurityRoles() != null;
}

public PrivilegesEvaluatorResponse evaluate(final ActionRequest request,
final String action,
final SecurityRoles securityRoles,
final User user,
final ClusterService clusterService) {
if (!isInitialized()) {
throw new OpenSearchSecurityException("OpenSearch Security is not initialized.");
}

final PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse();

final boolean isDebugEnabled = log.isDebugEnabled();
if (isDebugEnabled) {
log.debug("Evaluate permissions for {} on {}", user, clusterService.localNode().getName());
log.debug("Action: {}", action);
log.debug("Resource: {}", request.getResources());
log.debug("Security roles: {}", securityRoles.toString());
}

List<String> resourcesRequested = request.getResources();
if (resourcesRequested == null || resourcesRequested.isEmpty()) {
presponse.allowed = true;
return presponse;
}
presponse.allowed = true;
for (String resource : resourcesRequested) {
if (!securityRoles.impliesResourcePermission(resource)) {
presponse.missingPrivileges.add(action);
presponse.allowed = false;
log.info(
stephen-crawford marked this conversation as resolved.
Show resolved Hide resolved
"No permission match for {} [Action [{}]] [RolesChecked {}]. No permissions for {}",
user,
action,
securityRoles.getRoleNames(),
presponse.missingPrivileges
);
}
}
return presponse;
}

Set<String> mapRoles(final User user, final TransportAddress caller) {
return this.configModel.mapSecurityRoles(user, caller);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import org.opensearch.security.securityconf.impl.v7.RoleMappingsV7;
import org.opensearch.security.securityconf.impl.v7.RoleV7;
import org.opensearch.security.securityconf.impl.v7.RoleV7.Index;
import org.opensearch.security.securityconf.impl.v7.RoleV7.Resource;
import org.opensearch.security.securityconf.impl.v7.TenantV7;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.WildcardMatcher;
Expand Down Expand Up @@ -179,6 +180,10 @@ public SecurityRole call() throws Exception {

}

for (final Resource permittedResources : securityRole.getValue().getResource_permissions()) {
_securityRole.addResourcePerms(permittedResources.getResource_patterns());
}

return _securityRole.build();
}
});
Expand Down Expand Up @@ -504,17 +509,24 @@ public boolean isPermittedOnSystemIndex(String indexName) {
}
return isPatternMatched && isPermitted;
}

@Override
public boolean impliesResourcePermission(String resource) {
return roles.stream().filter(r -> r.impliesClusterPermission(resource)).count() > 0;
}
}

public static class SecurityRole {
private final String name;
private final Set<IndexPattern> ipatterns;
private final WildcardMatcher clusterPerms;
private final WildcardMatcher resourcePerms;

public static final class Builder {
private final String name;
private final Set<String> clusterPerms = new HashSet<>();
private final Set<IndexPattern> ipatterns = new HashSet<>();
private final Set<String> resourcePerms = new HashSet<>();

public Builder(String name) {
this.name = Objects.requireNonNull(name);
Expand All @@ -532,21 +544,35 @@ public Builder addClusterPerms(Collection<String> clusterPerms) {
return this;
}

public Builder addResourcePerms(Collection<String> resourcePerms) {
if (resourcePerms != null) {
this.resourcePerms.addAll(resourcePerms);
}
return this;
}

public SecurityRole build() {
return new SecurityRole(name, ipatterns, WildcardMatcher.from(clusterPerms));
return new SecurityRole(name, ipatterns, WildcardMatcher.from(clusterPerms), WildcardMatcher.from(resourcePerms));
}
}

private SecurityRole(String name, Set<IndexPattern> ipatterns, WildcardMatcher clusterPerms) {
private SecurityRole(String name, Set<IndexPattern> ipatterns, WildcardMatcher clusterPerms, WildcardMatcher resourcePerms) {
this.name = Objects.requireNonNull(name);
this.ipatterns = ipatterns;
this.clusterPerms = clusterPerms;
this.resourcePerms = resourcePerms;
}

private boolean impliesClusterPermission(String action) {
return clusterPerms.test(action);
}

private boolean impliesResourcePermission(String action) {
return resourcePerms.test(action);
}



// get indices which are permitted for the given types and actions
// dnfof + opensearchDashboards special only
private Set<String> getAllResolvedPermittedIndices(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.util.Set;

import org.opensearch.action.ActionRequest;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.core.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -98,4 +99,6 @@ Set<String> getAllPermittedIndicesForDashboards(
SecurityRoles filter(Set<String> roles);

boolean isPermittedOnSystemIndex(String indexName);

boolean impliesResourcePermission(String resource);
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I just thought it seemed like it may be a nice thing to have since we were adding the new class. I can also remove it though and don't feel strongly about its inclusion.

import java.util.HashSet;
import java.util.List;
import java.util.Map.Entry;
Expand All @@ -51,6 +52,7 @@ public class RoleV7 implements Hideable, StaticDefinable {
private List<String> cluster_permissions = Collections.emptyList();
private List<Index> index_permissions = Collections.emptyList();
private List<Tenant> tenant_permissions = Collections.emptyList();
private List<Resource> resource_permissions = Collections.emptyList();

public RoleV7() {

Expand All @@ -63,6 +65,7 @@ public RoleV7(RoleV6 roleV6) {
this.cluster_permissions = roleV6.getCluster();
index_permissions = new ArrayList<>();
tenant_permissions = new ArrayList<>();
resource_permissions = new ArrayList<>();

for (Entry<String, RoleV6.Index> v6i : roleV6.getIndices().entrySet()) {
index_permissions.add(new Index(v6i.getKey(), v6i.getValue()));
Expand Down Expand Up @@ -225,6 +228,58 @@ public String toString() {

}

public static class Resource {

private String uniqueId;
private List<String> resource_patterns;
private Date lastModifiedAt;
private List<String> allowed_actions = Collections.emptyList();

public Resource(String resourceName, List<String> resourcePattern) {
super();
uniqueId = resourceName;
lastModifiedAt = new Date();
Set<String> tmpActions = new HashSet<>();
resource_patterns = resourcePattern;
allowed_actions = new ArrayList<>(tmpActions);
}

public Resource() {
super();
}

public List<String> getAllowed_actions() {
return allowed_actions;
}

public void setAllowed_actions(List<String> allowed_actions) {
lastModifiedAt = new Date();
this.allowed_actions = allowed_actions;
}

public List<String> getResource_patterns() {
return resource_patterns;
}

public void setResource_patterns(List<String> resource_patterns) {
lastModifiedAt = new Date();
this.resource_patterns = resource_patterns;
}

@Override
public String toString() {
return "Resource [uniqueId="
+ uniqueId
+ ", lastModifiedAt="
+ lastModifiedAt
+ ", resource_patterns="
+ resource_patterns
+ ", allowed_actions="
+ allowed_actions
+ "]";
}
}

public boolean isHidden() {
return hidden;
}
Expand Down Expand Up @@ -265,6 +320,14 @@ public void setTenant_permissions(List<Tenant> tenant_permissions) {
this.tenant_permissions = tenant_permissions;
}

public List<Resource> getResource_permissions() {
return resource_permissions;
}

public void setResource_permissions(List<Resource> resource_permissions) {
this.resource_permissions = resource_permissions;
}

public boolean isReserved() {
return reserved;
}
Expand Down
Loading