Skip to content

Commit

Permalink
Initial support of permissions for REST admin user
Browse files Browse the repository at this point in the history
Added granular permissions for all REST API actions in OpenSearch to be individually assigned.

Permissions are:
    - 'restapi:admin/actiongroups' - allow full access to actiongroups
    - 'restapi:admin/allowlist' - allow full access to allowlist
    - 'restapi:admin/internalusers'- allow full access to internalusers
    - 'restapi:admin/nodesdn'- allow full access to nodesdn
    - 'restapi:admin/roles' - allow full access to roles
    - 'restapi:admin/rolesmapping' - allow full access to roles mappings
    - 'restapi:admin/ssl/certs/info' - allow full access to certs info
    - 'restapi:admin/ssl/certs/reload' - allow full access to certs reload
    - 'restapi:admin/tenants' - allow full access to tenants

Signed-off-by: Andrey Pleskach <[email protected]>
  • Loading branch information
willyborankin committed Jan 23, 2023
1 parent e4b7361 commit b9bc363
Show file tree
Hide file tree
Showing 20 changed files with 705 additions and 495 deletions.
16 changes: 15 additions & 1 deletion config/roles.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,21 @@ kibana_read_only:
# The security REST API access role is used to assign specific users access to change the security settings through the REST API.
security_rest_api_access:
reserved: true


security_rest_api_full_access:
reserved: true
hidden: true
cluster_permissions:
- 'restapi:admin/actiongroups'
- 'restapi:admin/allowlist'
- 'restapi:admin/internalusers'
- 'restapi:admin/nodesdn'
- 'restapi:admin/roles'
- 'restapi:admin/rolesmapping'
- 'restapi:admin/ssl/certs/info'
- 'restapi:admin/ssl/certs/reload'
- 'restapi:admin/tenants'

# Allows users to view monitors, destinations and alerts
alerting_read_access:
reserved: true
Expand Down
29 changes: 17 additions & 12 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,6 @@
import org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin;
import org.opensearch.security.ssl.SslExceptionHandler;
import org.opensearch.security.ssl.http.netty.ValidatingDispatcher;
import org.opensearch.security.ssl.rest.SecuritySSLCertsInfoAction;
import org.opensearch.security.ssl.rest.SecuritySSLReloadCertsAction;
import org.opensearch.security.ssl.transport.DefaultPrincipalExtractor;
import org.opensearch.security.ssl.transport.SecuritySSLNettyTransport;
import org.opensearch.security.ssl.util.SSLConfigConstants;
Expand Down Expand Up @@ -466,18 +464,25 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
if(!SSLConfig.isSslOnlyMode()) {
handlers.add(new SecurityInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
handlers.add(new SecurityHealthAction(settings, restController, Objects.requireNonNull(backendRegistry)));
handlers.add(new SecuritySSLCertsInfoAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
handlers.add(new DashboardsInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
handlers.add(new TenantInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool),
Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
handlers.add(new SecurityConfigUpdateAction(settings, restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
handlers.add(new SecurityWhoAmIAction(settings ,restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
if (sslCertReloadEnabled) {
handlers.add(new SecuritySSLReloadCertsAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
}
final Collection<RestHandler> apiHandlers = SecurityRestApiActions.getHandler(settings, configPath, restController, localClient, adminDns, cr, cs, principalExtractor, evaluator, threadPool, Objects.requireNonNull(auditLog));
handlers.addAll(apiHandlers);
log.debug("Added {} management rest handler(s)", apiHandlers.size());
Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
handlers.add(new SecurityConfigUpdateAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
handlers.add(new SecurityWhoAmIAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
handlers.addAll(
SecurityRestApiActions.getHandler(
settings,
configPath,
restController,
localClient,
adminDns,
cr, cs, principalExtractor,
evaluator,
threadPool,
Objects.requireNonNull(auditLog), sks,
sslCertReloadEnabled)
);
log.debug("Added {} rest handler(s)", handlers.size());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ public abstract class AbstractApiAction extends BaseRestHandler {
final ThreadPool threadPool;
protected String opendistroIndex;
private final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator;
protected final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator;
protected final AuditLog auditLog;
protected final Settings settings;
private AdminDNs adminDNs;

protected AbstractApiAction(final Settings settings, final Path configPath, final RestController controller,
final Client client, final AdminDNs adminDNs, final ConfigurationRepository cl,
Expand All @@ -88,12 +88,13 @@ protected AbstractApiAction(final Settings settings, final Path configPath, fina
this.opendistroIndex = settings.get(ConfigConstants.SECURITY_CONFIG_INDEX_NAME,
ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX);

this.adminDNs = adminDNs;
this.cl = cl;
this.cs = cs;
this.threadPool = threadPool;
this.restApiPrivilegesEvaluator = new RestApiPrivilegesEvaluator(settings, adminDNs, evaluator,
principalExtractor, configPath, threadPool);
this.restApiAdminPrivilegesEvaluator =
new RestApiAdminPrivilegesEvaluator(threadPool.getThreadContext(), evaluator, adminDNs);
this.auditLog = auditLog;
}

Expand Down Expand Up @@ -195,7 +196,12 @@ protected void handlePut(final RestChannel channel, final RestRequest request, f
}

boolean existed = existingConfiguration.exists(name);
existingConfiguration.putCObject(name, DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass()));
final Object newContent = DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass());
if (!hasPermissionsToCreate(existingConfiguration, newContent, getResourceName())) {
forbidden(channel, "No permissions");
return;
}
existingConfiguration.putCObject(name, newContent);

saveAnUpdateConfigs(client, request, getConfigName(), existingConfiguration, new OnSucessActionListener<IndexResponse>(channel) {

Expand All @@ -212,6 +218,12 @@ public void onResponse(IndexResponse response) {

}

protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
final Object content,
final String resourceName) throws IOException {
return true;
}

protected void handlePost(final RestChannel channel, final RestRequest request, final Client client, final JsonNode content) throws IOException {
notImplemented(channel, Method.POST);
}
Expand Down Expand Up @@ -448,7 +460,6 @@ protected static XContentBuilder convertToJson(RestChannel channel, ToXContent t
}

protected void response(RestChannel channel, RestStatus status, String message) {

try {
final XContentBuilder builder = channel.newBuilder();
builder.startObject();
Expand Down Expand Up @@ -558,8 +569,7 @@ public String getName() {
protected abstract Endpoint getEndpoint();

protected boolean isSuperAdmin() {
User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
return adminDNs.isAdmin(user);
return restApiAdminPrivilegesEvaluator.isCurrentUserRestApiAdminFor(getEndpoint());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,33 @@ protected void handlePut(RestChannel channel, RestRequest request, Client client

// Prevent the case where action group references to itself in the allowed_actions.
final SecurityDynamicConfiguration<?> existingActionGroupsConfig = load(getConfigName(), false);
existingActionGroupsConfig.putCObject(name, DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass()));
final Object actionGroup = DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass());
existingActionGroupsConfig.putCObject(name, actionGroup);
if (hasActionGroupSelfReference(existingActionGroupsConfig, name)) {
badRequestResponse(channel, name + " cannot be an allowed_action of itself");
return;
}

// prevent creation of groups for REST admin api
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(actionGroup)) {
forbidden(channel, "Not allowed");
return;
}
super.handlePut(channel, request, client, content);
}

@Override
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfiguration, final Object content, final String resourceName) throws IOException {
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(content)) {
return false;
}
return super.hasPermissionsToCreate(dynamicConfiguration, content, resourceName);
}

@Override
protected boolean isReadOnly(SecurityDynamicConfiguration<?> existingConfiguration, String name) {
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(existingConfiguration.getCEntry(name))) {
return true;
}
return super.isReadOnly(existingConfiguration, name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ public enum Endpoint {
VALIDATE,
WHITELIST,
ALLOWLIST,
NODESDN;
NODESDN,
SSL;
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public abstract class PatchableResourceApiAction extends AbstractApiAction {
protected final Logger log = LogManager.getLogger(this.getClass());

public PatchableResourceApiAction(Settings settings, Path configPath, RestController controller, Client client,
AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
AuditLog auditLog) {
AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
AuditLog auditLog) {
super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool,
auditLog);
}
Expand Down Expand Up @@ -146,7 +146,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client

if (!validator.validate()) {
request.params().clear();
badRequestResponse(channel, validator);
badRequestResponse(channel, validator);
return;
}

Expand All @@ -156,7 +156,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client
, existingConfiguration.getVersion(), existingConfiguration.getSeqNo(), existingConfiguration.getPrimaryTerm());

if (existingConfiguration.getCType().equals(CType.ACTIONGROUPS)) {
if(hasActionGroupSelfReference(mdc, name)) {
if (hasActionGroupSelfReference(mdc, name)) {
badRequestResponse(channel, name + " cannot be an allowed_action of itself");
return;
}
Expand All @@ -172,6 +172,10 @@ public void onResponse(IndexResponse response) {
});
}

protected boolean verifyPermissionFor(final JsonNode jsonNode, final Method method) {
return true;
}

private void handleBulkPatch(RestChannel channel, RestRequest request, Client client,
SecurityDynamicConfiguration<?> existingConfiguration, ObjectNode existingAsObjectNode, JsonNode jsonPatch) throws IOException {

Expand All @@ -188,7 +192,6 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
for (String resourceName : existingConfiguration.getCEntries().keySet()) {
JsonNode oldResource = existingAsObjectNode.get(resourceName);
JsonNode patchedResource = patchedAsJsonNode.get(resourceName);

if (oldResource != null && !oldResource.equals(patchedResource) && !isWriteable(channel, existingConfiguration, resourceName)) {
return;
}
Expand All @@ -206,7 +209,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
if(originalValidator != null) {
if (!originalValidator.validate()) {
request.params().clear();
badRequestResponse(channel, originalValidator);
badRequestResponse(channel, originalValidator);
return;
}
}
Expand All @@ -222,7 +225,13 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl

if (!validator.validate()) {
request.params().clear();
badRequestResponse(channel, validator);
badRequestResponse(channel, validator);
return;
}
final Object newContent = DefaultObjectMapper.readTree(patchedResource, existingConfiguration.getImplementingClass());
if (!hasPermissionsToCreate(existingConfiguration, newContent, resourceName)) {
request.params().clear();
forbidden(channel, "No permissions");
return;
}
}
Expand Down
Loading

0 comments on commit b9bc363

Please sign in to comment.