Skip to content

Commit 076715d

Browse files
willyborankinRyanL1997stephen-crawford
authored
Permissions for REST admin user 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 Adds tests for these permissions. Signed-off-by: Andrey Pleskach <ples@aiven.io> Co-authored-by: Ryan Liang <109499885+RyanL1997@users.noreply.github.com> Co-authored-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
1 parent 76aa785 commit 076715d

46 files changed

Lines changed: 2522 additions & 942 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎config/roles.yml‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,20 @@ kibana_read_only:
99
# The security REST API access role is used to assign specific users access to change the security settings through the REST API.
1010
security_rest_api_access:
1111
reserved: true
12-
12+
13+
security_rest_api_full_access:
14+
reserved: true
15+
cluster_permissions:
16+
- 'restapi:admin/actiongroups'
17+
- 'restapi:admin/allowlist'
18+
- 'restapi:admin/internalusers'
19+
- 'restapi:admin/nodesdn'
20+
- 'restapi:admin/roles'
21+
- 'restapi:admin/rolesmapping'
22+
- 'restapi:admin/ssl/certs/info'
23+
- 'restapi:admin/ssl/certs/reload'
24+
- 'restapi:admin/tenants'
25+
1326
# Allows users to view monitors, destinations and alerts
1427
alerting_read_access:
1528
reserved: true

‎src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,6 @@
156156
import org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin;
157157
import org.opensearch.security.ssl.SslExceptionHandler;
158158
import org.opensearch.security.ssl.http.netty.ValidatingDispatcher;
159-
import org.opensearch.security.ssl.rest.SecuritySSLCertsInfoAction;
160-
import org.opensearch.security.ssl.rest.SecuritySSLReloadCertsAction;
161159
import org.opensearch.security.ssl.transport.DefaultPrincipalExtractor;
162160
import org.opensearch.security.ssl.transport.SecuritySSLNettyTransport;
163161
import org.opensearch.security.ssl.util.SSLConfigConstants;
@@ -466,18 +464,25 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
466464
if(!SSLConfig.isSslOnlyMode()) {
467465
handlers.add(new SecurityInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
468466
handlers.add(new SecurityHealthAction(settings, restController, Objects.requireNonNull(backendRegistry)));
469-
handlers.add(new SecuritySSLCertsInfoAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
470467
handlers.add(new DashboardsInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
471468
handlers.add(new TenantInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool),
472-
Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
473-
handlers.add(new SecurityConfigUpdateAction(settings, restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
474-
handlers.add(new SecurityWhoAmIAction(settings ,restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
475-
if (sslCertReloadEnabled) {
476-
handlers.add(new SecuritySSLReloadCertsAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
477-
}
478-
final Collection<RestHandler> apiHandlers = SecurityRestApiActions.getHandler(settings, configPath, restController, localClient, adminDns, cr, cs, principalExtractor, evaluator, threadPool, Objects.requireNonNull(auditLog));
479-
handlers.addAll(apiHandlers);
480-
log.debug("Added {} management rest handler(s)", apiHandlers.size());
469+
Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
470+
handlers.add(new SecurityConfigUpdateAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
471+
handlers.add(new SecurityWhoAmIAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
472+
handlers.addAll(
473+
SecurityRestApiActions.getHandler(
474+
settings,
475+
configPath,
476+
restController,
477+
localClient,
478+
adminDns,
479+
cr, cs, principalExtractor,
480+
evaluator,
481+
threadPool,
482+
Objects.requireNonNull(auditLog), sks,
483+
sslCertReloadEnabled)
484+
);
485+
log.debug("Added {} rest handler(s)", handlers.size());
481486
}
482487
}
483488

‎src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ public abstract class AbstractApiAction extends BaseRestHandler {
7575
final ThreadPool threadPool;
7676
protected String opendistroIndex;
7777
private final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator;
78+
protected final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator;
7879
protected final AuditLog auditLog;
7980
protected final Settings settings;
80-
private AdminDNs adminDNs;
8181

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

91-
this.adminDNs = adminDNs;
9291
this.cl = cl;
9392
this.cs = cs;
9493
this.threadPool = threadPool;
9594
this.restApiPrivilegesEvaluator = new RestApiPrivilegesEvaluator(settings, adminDNs, evaluator,
9695
principalExtractor, configPath, threadPool);
96+
this.restApiAdminPrivilegesEvaluator =
97+
new RestApiAdminPrivilegesEvaluator(threadPool.getThreadContext(), evaluator, adminDNs);
9798
this.auditLog = auditLog;
9899
}
99100

@@ -195,7 +196,12 @@ protected void handlePut(final RestChannel channel, final RestRequest request, f
195196
}
196197

197198
boolean existed = existingConfiguration.exists(name);
198-
existingConfiguration.putCObject(name, DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass()));
199+
final Object newContent = DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass());
200+
if (!hasPermissionsToCreate(existingConfiguration, newContent, getResourceName())) {
201+
forbidden(channel, "No permissions");
202+
return;
203+
}
204+
existingConfiguration.putCObject(name, newContent);
199205

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

@@ -216,6 +222,12 @@ protected void handlePost(final RestChannel channel, final RestRequest request,
216222
notImplemented(channel, Method.POST);
217223
}
218224

225+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
226+
final Object content,
227+
final String resourceName) throws IOException {
228+
return false;
229+
}
230+
219231
protected void handleGet(final RestChannel channel, RestRequest request, Client client, final JsonNode content)
220232
throws IOException{
221233

@@ -448,7 +460,6 @@ protected static XContentBuilder convertToJson(RestChannel channel, ToXContent t
448460
}
449461

450462
protected void response(RestChannel channel, RestStatus status, String message) {
451-
452463
try {
453464
final XContentBuilder builder = channel.newBuilder();
454465
builder.startObject();
@@ -558,8 +569,7 @@ public String getName() {
558569
protected abstract Endpoint getEndpoint();
559570

560571
protected boolean isSuperAdmin() {
561-
User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
562-
return adminDNs.isAdmin(user);
572+
return restApiAdminPrivilegesEvaluator.isCurrentUserRestApiAdminFor(getEndpoint());
563573
}
564574

565575
/**

‎src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ public AccountApiAction(Settings settings,
8383
this.threadContext = threadPool.getThreadContext();
8484
}
8585

86+
@Override
87+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
88+
final Object content,
89+
final String resourceName) {
90+
return true;
91+
}
92+
8693
@Override
8794
public List<Route> routes() {
8895
return routes;

‎src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,35 @@ protected void handlePut(RestChannel channel, RestRequest request, Client client
118118

119119
// Prevent the case where action group references to itself in the allowed_actions.
120120
final SecurityDynamicConfiguration<?> existingActionGroupsConfig = load(getConfigName(), false);
121-
existingActionGroupsConfig.putCObject(name, DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass()));
121+
final Object actionGroup = DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass());
122+
existingActionGroupsConfig.putCObject(name, actionGroup);
122123
if (hasActionGroupSelfReference(existingActionGroupsConfig, name)) {
123124
badRequestResponse(channel, name + " cannot be an allowed_action of itself");
124125
return;
125126
}
126-
127+
// prevent creation of groups for REST admin api
128+
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(actionGroup)) {
129+
forbidden(channel, "Not allowed");
130+
return;
131+
}
127132
super.handlePut(channel, request, client, content);
128133
}
134+
135+
@Override
136+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfiguration,
137+
final Object content,
138+
final String resourceName) throws IOException {
139+
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(content)) {
140+
return false;
141+
}
142+
return true;
143+
}
144+
145+
@Override
146+
protected boolean isReadOnly(SecurityDynamicConfiguration<?> existingConfiguration, String name) {
147+
if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(existingConfiguration.getCEntry(name))) {
148+
return true;
149+
}
150+
return super.isReadOnly(existingConfiguration, name);
151+
}
129152
}

‎src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,13 @@ public AllowlistApiAction(final Settings settings, final Path configPath, final
9898
super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog);
9999
}
100100

101+
@Override
102+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
103+
final Object content,
104+
final String resourceName) {
105+
return true;
106+
}
107+
101108
@Override
102109
protected void handleApiRequest(final RestChannel channel, final RestRequest request, final Client client) throws IOException {
103110
if (!isSuperAdmin()) {

‎src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,13 @@ public AuditApiAction(final Settings settings,
165165
}
166166
}
167167

168+
@Override
169+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
170+
final Object content,
171+
final String resourceName) {
172+
return true;
173+
}
174+
168175
@Override
169176
public List<Route> routes() {
170177
return routes;

‎src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.opensearch.security.dlic.rest.validation.NoOpValidator;
3535
import org.opensearch.security.privileges.PrivilegesEvaluator;
3636
import org.opensearch.security.securityconf.impl.CType;
37+
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
3738
import org.opensearch.security.ssl.transport.PrincipalExtractor;
3839
import org.opensearch.threadpool.ThreadPool;
3940

@@ -53,6 +54,13 @@ public AuthTokenProcessorAction(final Settings settings, final Path configPath,
5354
auditLog);
5455
}
5556

57+
@Override
58+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
59+
final Object content,
60+
final String resourceName) {
61+
return true;
62+
}
63+
5664
@Override
5765
public List<Route> routes() {
5866
return routes;

‎src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,6 @@ public enum Endpoint {
2828
VALIDATE,
2929
WHITELIST,
3030
ALLOWLIST,
31-
NODESDN;
31+
NODESDN,
32+
SSL;
3233
}

‎src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.opensearch.security.dlic.rest.validation.NoOpValidator;
3939
import org.opensearch.security.privileges.PrivilegesEvaluator;
4040
import org.opensearch.security.securityconf.impl.CType;
41+
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
4142
import org.opensearch.security.ssl.transport.PrincipalExtractor;
4243
import org.opensearch.threadpool.ThreadPool;
4344

@@ -58,6 +59,13 @@ public FlushCacheApiAction(final Settings settings, final Path configPath, final
5859
super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog);
5960
}
6061

62+
@Override
63+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
64+
final Object content,
65+
final String resourceName) {
66+
return true;
67+
}
68+
6169
@Override
6270
public List<Route> routes() {
6371
return routes;

0 commit comments

Comments
 (0)