Skip to content

Commit d676716

Browse files
willyborankinwillyborankin
authored
Rest admin permissions (opensearch-project#2411)
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>
1 parent 75b6c30 commit d676716

Some content is hidden

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

46 files changed

+2521
-941
lines changed

‎config/roles.yml

+14-1
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

+17-12
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

+16-6
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

+7
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

+25-2
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

+7
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

+7
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

+8
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

+2-1
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

+8
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;

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

+7
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,13 @@ public InternalUsersApiAction(final Settings settings, final Path configPath, fi
7878
auditLog);
7979
}
8080

81+
@Override
82+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
83+
final Object content,
84+
final String resourceName) {
85+
return true;
86+
}
87+
8188
@Override
8289
public List<Route> routes() {
8390
return routes;

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

+7
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ protected Endpoint getEndpoint() {
9292
return Endpoint.MIGRATE;
9393
}
9494

95+
@Override
96+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
97+
final Object content,
98+
final String resourceName) {
99+
return true;
100+
}
101+
95102
@SuppressWarnings("unchecked")
96103
@Override
97104
protected void handlePost(RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException {

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

+7
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ public NodesDnApiAction(final Settings settings, final Path configPath, final Re
7777
this.staticNodesDnFromEsYml = settings.getAsList(ConfigConstants.SECURITY_NODES_DN, Collections.emptyList());
7878
}
7979

80+
@Override
81+
protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
82+
final Object content,
83+
final String resourceName) {
84+
return true;
85+
}
86+
8087
@Override
8188
public List<Route> routes() {
8289
if (settings.getAsBoolean(ConfigConstants.SECURITY_NODES_DN_DYNAMIC_CONFIG_ENABLED, false)) {

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

+13-8
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ public abstract class PatchableResourceApiAction extends AbstractApiAction {
5555
protected final Logger log = LogManager.getLogger(this.getClass());
5656

5757
public PatchableResourceApiAction(Settings settings, Path configPath, RestController controller, Client client,
58-
AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
59-
PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
60-
AuditLog auditLog) {
58+
AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
59+
PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
60+
AuditLog auditLog) {
6161
super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool,
6262
auditLog);
6363
}
@@ -146,7 +146,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client
146146

147147
if (!validator.validate()) {
148148
request.params().clear();
149-
badRequestResponse(channel, validator);
149+
badRequestResponse(channel, validator);
150150
return;
151151
}
152152

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

158158
if (existingConfiguration.getCType().equals(CType.ACTIONGROUPS)) {
159-
if(hasActionGroupSelfReference(mdc, name)) {
159+
if (hasActionGroupSelfReference(mdc, name)) {
160160
badRequestResponse(channel, name + " cannot be an allowed_action of itself");
161161
return;
162162
}
@@ -188,7 +188,6 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
188188
for (String resourceName : existingConfiguration.getCEntries().keySet()) {
189189
JsonNode oldResource = existingAsObjectNode.get(resourceName);
190190
JsonNode patchedResource = patchedAsJsonNode.get(resourceName);
191-
192191
if (oldResource != null && !oldResource.equals(patchedResource) && !isWriteable(channel, existingConfiguration, resourceName)) {
193192
return;
194193
}
@@ -206,7 +205,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
206205
if(originalValidator != null) {
207206
if (!originalValidator.validate()) {
208207
request.params().clear();
209-
badRequestResponse(channel, originalValidator);
208+
badRequestResponse(channel, originalValidator);
210209
return;
211210
}
212211
}
@@ -222,7 +221,13 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
222221

223222
if (!validator.validate()) {
224223
request.params().clear();
225-
badRequestResponse(channel, validator);
224+
badRequestResponse(channel, validator);
225+
return;
226+
}
227+
final Object newContent = DefaultObjectMapper.readTree(patchedResource, existingConfiguration.getImplementingClass());
228+
if (!hasPermissionsToCreate(existingConfiguration, newContent, resourceName)) {
229+
request.params().clear();
230+
forbidden(channel, "No permissions");
226231
return;
227232
}
228233
}

0 commit comments

Comments
 (0)