Skip to content

Commit a4bd99d

Browse files
samuelcostaeDarshitChanpurapeternied
committed
System index permissions (#2887)
System index permissions Signed-off-by: Sam <samuel.costa@eliatra.com> Signed-off-by: Sam <128482925+samuelcostae@users.noreply.github.com> Signed-off-by: Darshit Chanpura <dchanp@amazon.com> Signed-off-by: Peter Nied <peternied@hotmail.com> Co-authored-by: Darshit Chanpura <dchanp@amazon.com> Co-authored-by: Peter Nied <peternied@hotmail.com> (cherry picked from commit 1379234)
1 parent 552de1b commit a4bd99d

25 files changed

Lines changed: 2595 additions & 727 deletions

‎DEVELOPER_GUIDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ Checkstyle enforced several rules within this codebase. Sometimes exceptions wi
256256
257257
*Suppression All Checkstyle Rules*
258258
```
259-
// CS-SUPRESS-ALL: Legacy code to be deleted in Z.Y.X see http://github/issues/1234
259+
// CS-SUPPRESS-ALL: Legacy code to be deleted in Z.Y.X see http://github/issues/1234
260260
...
261261
// CS-ENFORCE-ALL
262262
```

‎src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
package org.opensearch.security.api;
1313

1414
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
15-
import org.apache.hc.core5.http.HttpStatus;
15+
import org.apache.http.HttpStatus;
1616
import org.junit.ClassRule;
1717
import org.junit.Test;
1818
import org.junit.runner.RunWith;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,6 +1781,14 @@ public List<Setting<?>> getSettings() {
17811781
Property.Filtered
17821782
)
17831783
);
1784+
settings.add(
1785+
Setting.boolSetting(
1786+
ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY,
1787+
ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_DEFAULT,
1788+
Property.NodeScope,
1789+
Property.Filtered
1790+
)
1791+
);
17841792
}
17851793

17861794
return settings;

‎src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,17 @@ public PrivilegesEvaluatorResponse evaluate(
314314
}
315315

316316
// Security index access
317-
if (securityIndexAccessEvaluator.evaluate(request, task, action0, requestedResolved, presponse).isComplete()) {
317+
if (securityIndexAccessEvaluator.evaluate(
318+
request,
319+
task,
320+
action0,
321+
requestedResolved,
322+
presponse,
323+
securityRoles,
324+
user,
325+
resolver,
326+
clusterService
327+
).isComplete()) {
318328
return presponse;
319329
}
320330

‎src/main/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluator.java‎

Lines changed: 202 additions & 71 deletions
Large diffs are not rendered by default.

‎src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,36 @@ public EvaluatedDlsFlsConfig getDlsFls(
448448
return new EvaluatedDlsFlsConfig(dlsQueries, flsFields, maskedFieldsMap);
449449
}
450450

451+
public boolean hasExplicitIndexPermission(
452+
Resolved resolved,
453+
User user,
454+
String[] actions,
455+
IndexNameExpressionResolver resolver,
456+
ClusterService cs
457+
) {
458+
final Set<String> indicesForRequest = new HashSet<>(resolved.getAllIndicesResolved(cs, resolver));
459+
if (indicesForRequest.isEmpty()) {
460+
// If no indices could be found on the request there is no way to check for the explicit permissions
461+
return false;
462+
}
463+
464+
final Set<String> explicitlyAllowedIndices = roles.stream()
465+
.map(role -> role.getAllResolvedPermittedIndices(resolved, user, actions, resolver, cs, true))
466+
.flatMap(Collection::stream)
467+
.collect(Collectors.toSet());
468+
469+
if (log.isDebugEnabled()) {
470+
log.debug(
471+
"ExplicitIndexPermission check indices for request {}, explicitly allowed indices {}",
472+
indicesForRequest.toString(),
473+
explicitlyAllowedIndices.toString()
474+
);
475+
}
476+
477+
indicesForRequest.removeAll(explicitlyAllowedIndices);
478+
return indicesForRequest.isEmpty();
479+
}
480+
451481
// opensearchDashboards special only, terms eval
452482
public Set<String> getAllPermittedIndicesForDashboards(
453483
Resolved resolved,
@@ -458,7 +488,7 @@ public Set<String> getAllPermittedIndicesForDashboards(
458488
) {
459489
Set<String> retVal = new HashSet<>();
460490
for (SecurityRole sr : roles) {
461-
retVal.addAll(sr.getAllResolvedPermittedIndices(Resolved._LOCAL_ALL, user, actions, resolver, cs));
491+
retVal.addAll(sr.getAllResolvedPermittedIndices(Resolved._LOCAL_ALL, user, actions, resolver, cs, false));
462492
retVal.addAll(resolved.getRemoteIndices());
463493
}
464494
return Collections.unmodifiableSet(retVal);
@@ -468,7 +498,7 @@ public Set<String> getAllPermittedIndicesForDashboards(
468498
public Set<String> reduce(Resolved resolved, User user, String[] actions, IndexNameExpressionResolver resolver, ClusterService cs) {
469499
Set<String> retVal = new HashSet<>();
470500
for (SecurityRole sr : roles) {
471-
retVal.addAll(sr.getAllResolvedPermittedIndices(resolved, user, actions, resolver, cs));
501+
retVal.addAll(sr.getAllResolvedPermittedIndices(resolved, user, actions, resolver, cs, false));
472502
}
473503
if (log.isDebugEnabled()) {
474504
log.debug("Reduced requested resolved indices {} to permitted indices {}.", resolved, retVal.toString());
@@ -536,7 +566,8 @@ private Set<String> getAllResolvedPermittedIndices(
536566
User user,
537567
String[] actions,
538568
IndexNameExpressionResolver resolver,
539-
ClusterService cs
569+
ClusterService cs,
570+
boolean matchExplicitly
540571
) {
541572

542573
final Set<String> retVal = new HashSet<>();
@@ -545,7 +576,11 @@ private Set<String> getAllResolvedPermittedIndices(
545576
boolean patternMatch = false;
546577
final Set<TypePerm> tperms = p.getTypePerms();
547578
for (TypePerm tp : tperms) {
548-
if (tp.typeMatcher.matchAny(resolved.getTypes())) {
579+
// if matchExplicitly is true we don't want to match against `*` pattern
580+
WildcardMatcher matcher = matchExplicitly && (tp.getPerms() == WildcardMatcher.ANY)
581+
? WildcardMatcher.NONE
582+
: tp.getTypeMatcher();
583+
if (matcher.matchAny(resolved.getTypes())) {
549584
patternMatch = tp.getPerms().matchAll(actions);
550585
}
551586
}

‎src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.concurrent.Executors;
3434
import java.util.concurrent.Future;
3535
import java.util.concurrent.TimeUnit;
36+
import java.util.function.Function;
3637
import java.util.regex.Pattern;
3738
import java.util.stream.Collectors;
3839

@@ -462,7 +463,7 @@ public Set<String> getAllPermittedIndicesForDashboards(
462463
) {
463464
Set<String> retVal = new HashSet<>();
464465
for (SecurityRole sr : roles) {
465-
retVal.addAll(sr.getAllResolvedPermittedIndices(Resolved._LOCAL_ALL, user, actions, resolver, cs));
466+
retVal.addAll(sr.getAllResolvedPermittedIndices(Resolved._LOCAL_ALL, user, actions, resolver, cs, Function.identity()));
466467
retVal.addAll(resolved.getRemoteIndices());
467468
}
468469
return Collections.unmodifiableSet(retVal);
@@ -472,7 +473,7 @@ public Set<String> getAllPermittedIndicesForDashboards(
472473
public Set<String> reduce(Resolved resolved, User user, String[] actions, IndexNameExpressionResolver resolver, ClusterService cs) {
473474
Set<String> retVal = new HashSet<>();
474475
for (SecurityRole sr : roles) {
475-
retVal.addAll(sr.getAllResolvedPermittedIndices(resolved, user, actions, resolver, cs));
476+
retVal.addAll(sr.getAllResolvedPermittedIndices(resolved, user, actions, resolver, cs, Function.identity()));
476477
}
477478
if (log.isDebugEnabled()) {
478479
log.debug("Reduced requested resolved indices {} to permitted indices {}.", resolved, retVal.toString());
@@ -497,10 +498,43 @@ public boolean impliesClusterPermissionPermission(String action) {
497498

498499
@Override
499500
public boolean hasExplicitClusterPermissionPermission(String action) {
500-
return roles.stream()
501-
.map(r -> r.clusterPerms == WildcardMatcher.ANY ? WildcardMatcher.NONE : r.clusterPerms)
502-
.filter(m -> m.test(action))
503-
.count() > 0;
501+
return roles.stream().map(r -> matchExplicitly(r.clusterPerms)).filter(m -> m.test(action)).count() > 0;
502+
}
503+
504+
private static WildcardMatcher matchExplicitly(final WildcardMatcher matcher) {
505+
return matcher == WildcardMatcher.ANY ? WildcardMatcher.NONE : matcher;
506+
}
507+
508+
@Override
509+
public boolean hasExplicitIndexPermission(
510+
final Resolved resolved,
511+
final User user,
512+
final String[] actions,
513+
final IndexNameExpressionResolver resolver,
514+
final ClusterService cs
515+
) {
516+
517+
final Set<String> indicesForRequest = new HashSet<>(resolved.getAllIndicesResolved(cs, resolver));
518+
if (indicesForRequest.isEmpty()) {
519+
// If no indices could be found on the request there is no way to check for the explicit permissions
520+
return false;
521+
}
522+
523+
final Set<String> explicitlyAllowedIndices = roles.stream()
524+
.map(role -> role.getAllResolvedPermittedIndices(resolved, user, actions, resolver, cs, SecurityRoles::matchExplicitly))
525+
.flatMap(Collection::stream)
526+
.collect(Collectors.toSet());
527+
528+
if (log.isDebugEnabled()) {
529+
log.debug(
530+
"ExplicitIndexPermission check indices for request {}, explicitly allowed indices {}",
531+
indicesForRequest.toString(),
532+
explicitlyAllowedIndices.toString()
533+
);
534+
}
535+
536+
indicesForRequest.removeAll(explicitlyAllowedIndices);
537+
return indicesForRequest.isEmpty();
504538
}
505539

506540
// rolespan
@@ -577,13 +611,14 @@ private Set<String> getAllResolvedPermittedIndices(
577611
User user,
578612
String[] actions,
579613
IndexNameExpressionResolver resolver,
580-
ClusterService cs
614+
ClusterService cs,
615+
Function<WildcardMatcher, WildcardMatcher> matcherModification
581616
) {
582617

583618
final Set<String> retVal = new HashSet<>();
584619
for (IndexPattern p : ipatterns) {
585620
// what if we cannot resolve one (for create purposes)
586-
final boolean patternMatch = p.getPerms().matchAll(actions);
621+
final boolean patternMatch = matcherModification.apply(p.getPerms()).matchAll(actions);
587622

588623
// final Set<TypePerm> tperms = p.getTypePerms();
589624
// for (TypePerm tp : tperms) {

‎src/main/java/org/opensearch/security/securityconf/SecurityRoles.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@ public interface SecurityRoles {
4141

4242
boolean hasExplicitClusterPermissionPermission(String action);
4343

44+
/**
45+
* Determines if the actions are explicitly granted for indices
46+
* @return if all indices in the request have an explicit grant for all actions
47+
*/
48+
boolean hasExplicitIndexPermission(
49+
Resolved resolved,
50+
User user,
51+
String[] actions,
52+
IndexNameExpressionResolver resolver,
53+
ClusterService cs
54+
);
55+
4456
Set<String> getRoleNames();
4557

4658
Set<String> reduce(
@@ -84,5 +96,4 @@ Set<String> getAllPermittedIndicesForDashboards(
8496
);
8597

8698
SecurityRoles filter(Set<String> roles);
87-
8899
}

‎src/main/java/org/opensearch/security/support/ConfigConstants.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,11 @@ public enum RolesMappingResolution {
309309
"opendistro_security_injected_roles_validation_header";
310310

311311
// System indices settings
312+
public static final String SYSTEM_INDEX_PERMISSION = "system:admin/system_index";
312313
public static final String SECURITY_SYSTEM_INDICES_ENABLED_KEY = "plugins.security.system_indices.enabled";
313314
public static final Boolean SECURITY_SYSTEM_INDICES_ENABLED_DEFAULT = false;
315+
public static final String SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY = "plugins.security.system_indices.permission.enabled";
316+
public static final Boolean SECURITY_SYSTEM_INDICES_PERMISSIONS_DEFAULT = false;
314317
public static final String SECURITY_SYSTEM_INDICES_KEY = "plugins.security.system_indices.indices";
315318
public static final List<String> SECURITY_SYSTEM_INDICES_DEFAULT = Collections.emptyList();
316319

‎src/test/java/org/opensearch/security/IntegrationTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,7 @@ public void testSecurityIndexSecurity() throws Exception {
10171017
encodeBasicHeader("nagilum", "nagilum")
10181018
);
10191019
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, res.getStatusCode());
1020+
10201021
res = rh.executePutRequest(
10211022
"*dis*rit*/_mapping?pretty",
10221023
"{\"properties\": {\"name\":{\"type\":\"text\"}}}",
@@ -1052,9 +1053,8 @@ public void testSecurityIndexSecurity() throws Exception {
10521053
encodeBasicHeader("nagilum", "nagilum")
10531054
);
10541055
Assert.assertEquals(HttpStatus.SC_FORBIDDEN, res.getStatusCode());
1055-
// res = rh.executePostRequest(".opendistro_security/_freeze", "",
1056-
// encodeBasicHeader("nagilum", "nagilum"));
1057-
// Assert.assertTrue(res.getStatusCode() >= 400);
1056+
res = rh.executePostRequest(".opendistro_security/_freeze", "", encodeBasicHeader("nagilum", "nagilum"));
1057+
Assert.assertEquals(400, res.getStatusCode());
10581058

10591059
String bulkBody = "{ \"index\" : { \"_index\" : \".opendistro_security\", \"_id\" : \"1\" } }\n"
10601060
+ "{ \"field1\" : \"value1\" }\n"

0 commit comments

Comments
 (0)