Support audit ignore policy by index privileges

Addressing review comments + changing approach:
- use permission check instead of simple "checkIfGrants"
- adding more testing
This commit is contained in:
BigPandaToo 2021-02-15 21:38:25 +01:00
parent 35573c8bef
commit 67574b2f51
5 changed files with 978 additions and 109 deletions

View file

@ -57,9 +57,13 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
@Override
protected boolean extendedCheck(String action, TransportRequest request, Authentication authentication) {
if (request == null || authentication == null) {
if(authentication == null) {
//No valid credentials found
return false;
}
if (request == null) {
throw new IllegalArgumentException(
"manage own cluster permission check only supported in context of request and authentication");
"manage own cluster permission check only supported in context of an API key request");
}
if (request instanceof CreateApiKeyRequest) {

View file

@ -24,6 +24,9 @@ import java.util.HashSet;
import java.util.List;
import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailFilterTests.randomNonEmptyListOfFilteredClusterActions;
import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailFilterTests.randomNonEmptyListOfFilteredIndexActions;
import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrailFilterTests.randomNonEmptyListOfFilteredPrivileges;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.startsWith;
import static org.hamcrest.Matchers.is;
@ -97,13 +100,44 @@ public class AuditTrailSettingsUpdateTests extends SecurityIntegTestCase {
"xpack.security.audit.logfile.events.ignore_filters.invalid.realms",
"xpack.security.audit.logfile.events.ignore_filters.invalid.roles",
"xpack.security.audit.logfile.events.ignore_filters.invalid.indices",
"xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges"};
"xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges",
"xpack.security.audit.logfile.events.ignore_filters.invalid.cluster_privileges"};
settingsBuilder.put(randomFrom(allSettingsKeys), invalidLuceneRegex);
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> client().admin().cluster().prepareUpdateSettings().setTransientSettings(settingsBuilder.build()).get());
assertThat(e.getMessage(), containsString("invalid pattern [/invalid]"));
}
public void testInvalidPrivilegesFilterSettings() throws Exception {
final Settings.Builder settingsBuilder1 = Settings.builder();
settingsBuilder1.putList("xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges", "hkrgbkj");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> updateSettings(settingsBuilder1.build(), randomBoolean()));
assertThat(e.getMessage(), containsString("illegal value can't update"));
final Settings.Builder settingsBuilder2 = Settings.builder();
settingsBuilder1.putList("xpack.security.audit.logfile.events.ignore_filters.invalid.index_privileges", "hkrgbkj");
e = expectThrows(IllegalArgumentException.class,
() -> updateSettings(settingsBuilder2.build(), randomBoolean()));
assertThat(e.getMessage(), containsString("illegal value can't update"));
}
public void testInvalidBothPrivilegesFilterSettings() throws Exception {
final Settings.Builder settingsBuilder1 = Settings.builder();
settingsBuilder1.putList("xpack.security.audit.logfile.events.ignore_filters.BothPrivilegesFilter.index_privileges",
"read");
updateSettings(settingsBuilder1.build(), true);
final Settings.Builder settingsBuilder2 = Settings.builder();
settingsBuilder2.putList("xpack.security.audit.logfile.events.ignore_filters.BothPrivilegesFilter.cluster_privileges", "monitor");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> updateSettings(settingsBuilder2.build(), true));
assertThat(e.getMessage(), containsString("illegal value can't update"));
}
public void testDynamicHostSettings() {
final boolean persistent = randomBoolean();
final Settings.Builder settingsBuilder = Settings.builder();
@ -225,10 +259,16 @@ public class AuditTrailSettingsUpdateTests extends SecurityIntegTestCase {
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters." + policyName + ".indices", filteredIndices);
}
if (randomBoolean()) {
// filter by privileges
final List<String> filteredPrivileges = randomNonEmptyListOfFilteredNames();
// filter by index privileges
final List<String> filteredIndexActions = randomNonEmptyListOfFilteredIndexActions();
final List<String> filteredPrivileges = randomNonEmptyListOfFilteredPrivileges(filteredIndexActions);
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters." + policyName + ".index_privileges",
filteredPrivileges);
} else {
final List<String> filteredClusterActions = randomNonEmptyListOfFilteredClusterActions();
final List<String> filteredPrivileges = randomNonEmptyListOfFilteredPrivileges(filteredClusterActions);
settingsBuilder.putList("xpack.security.audit.logfile.events.ignore_filters." + policyName + ".cluster_privileges",
filteredPrivileges);
}
} while (settingsBuilder.build().isEmpty());

View file

@ -96,6 +96,7 @@ import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@ -300,14 +301,17 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
this.eventFilterPolicyRegistry.set(policyName, newPolicy);
}, (policyName, filtersList) -> {
EventFilterPolicy.parsePredicate(filtersList);
EventFilterPolicy.testtest(filtersList);
});
eventFilterPolicyRegistry.get(policyName).orElse(new EventFilterPolicy(policyName, settings)).
validateIndexPrivilege(filtersList); });
clusterService.getClusterSettings().addAffixUpdateConsumer(FILTER_POLICY_IGNORE_CLUSTER_PRIVILEGES, (policyName, filtersList) -> {
final Optional<EventFilterPolicy> policy = eventFilterPolicyRegistry.get(policyName);
final EventFilterPolicy newPolicy = policy.orElse(new EventFilterPolicy(policyName, settings)).
changeClusterPrivilegesFilter(filtersList);
this.eventFilterPolicyRegistry.set(policyName, newPolicy);
}, (policyName, filtersList) -> EventFilterPolicy.parsePredicate(filtersList));
}, (policyName, filtersList) -> {
EventFilterPolicy.parsePredicate(filtersList);
eventFilterPolicyRegistry.get(policyName).orElse(new EventFilterPolicy(policyName, settings)).
validateClusterPrivilege(filtersList); });
// this log filter ensures that audit events are not filtered out because of the log level
final LoggerContext ctx = LoggerContext.getContext(false);
MarkerFilter auditMarkerFilter = MarkerFilter.createFilter(AUDIT_MARKER.getName(), Result.ACCEPT, Result.NEUTRAL);
@ -1422,7 +1426,7 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
this.ignoreIndexPrivilegesPredicate = ignoreIndexPrivilegesPredicate;
this.ignoreClusterPrivilegesPredicate = ignoreClusterPrivilegesPredicate;
if (!ignoreIndexPrivilegesPredicate.test("") && !ignoreClusterPrivilegesPredicate.test("")) {
if (ignoreIndexPrivilegesPredicate.test("") == false && ignoreClusterPrivilegesPredicate.test("") == false) {
final String message = String.format(
Locale.ROOT,
"Both Index and Cluster privilege ignore filters are set for policy [%s]. " +
@ -1430,7 +1434,7 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
throw new IllegalArgumentException(message);
}
if (!ignoreClusterPrivilegesPredicate.test("")) {
if (ignoreClusterPrivilegesPredicate.test("") == false) {
ClusterPermission.Builder builder = ClusterPermission.builder();
String[] clusterPrivileges = ignoreClusterPrivilegesPredicate.toString().split("\\|");
for (String privilege : clusterPrivileges) {
@ -1441,10 +1445,11 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
this.clusterPermission = ClusterPermission.NONE;
}
if (!ignoreIndexPrivilegesPredicate.test("")) {
if (ignoreIndexPrivilegesPredicate.test("") == false) {
List<IndicesPermission.Group> groups = new ArrayList<>();
groups.add(new IndicesPermission.Group(IndexPrivilege.get(Set.of(ignoreIndexPrivilegesPredicate.toString().split("\\|"))),
FieldPermissions.DEFAULT, null, false, "*"));
Set<String> set = new HashSet<>(Arrays.stream(ignoreIndexPrivilegesPredicate.toString().split("\\|"))
.collect(Collectors.toSet()));
groups.add(new IndicesPermission.Group(IndexPrivilege.get(set), FieldPermissions.DEFAULT, null, false, "*"));
this.indexPermission = groups.isEmpty() ? IndicesPermission.NONE :
new IndicesPermission(groups.toArray(new IndicesPermission.Group[groups.size()]));
} else {
@ -1477,22 +1482,39 @@ public class LoggingAuditTrail implements AuditTrail, ClusterStateListener {
return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
ignoreRolesPredicate, ignoreIndicesPredicate, parsePredicate(filtersList), ignoreClusterPrivilegesPredicate);
} catch (IllegalArgumentException e) {
return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
ignoreRolesPredicate, ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, ignoreClusterPrivilegesPredicate);
return this;
}
}
private EventFilterPolicy changeClusterPrivilegesFilter(List<String> filtersList) {
return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate, ignoreRolesPredicate,
ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, parsePredicate(filtersList));
try {
return new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate, ignoreRolesPredicate,
ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, parsePredicate(filtersList));
} catch (IllegalArgumentException e) {
return this;
}
}
static Predicate<String> parsePredicate(List<String> l) {
return Automatons.predicate(emptyStringBuildsEmptyAutomaton(l));
}
static void testtest(List<String> l) {
throw new IllegalStateException("Cannot dynamically update SSL settings for the exporter");
private void validateIndexPrivilege(List<String> l) {
try {
new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
ignoreRolesPredicate, ignoreIndicesPredicate, parsePredicate(l), ignoreClusterPrivilegesPredicate);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Invalid setting for `index_privileges` audit policy filter found: " + e.getMessage());
}
}
private void validateClusterPrivilege(List<String> l) {
try {
new EventFilterPolicy(name, ignorePrincipalsPredicate, ignoreRealmsPredicate,
ignoreRolesPredicate, ignoreIndicesPredicate, ignoreIndexPrivilegesPredicate, parsePredicate(l));
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Invalid setting for `index_privileges` audit policy filter found: " + e.getMessage());
}
}
/**

View file

@ -155,6 +155,10 @@ public class AuthorizationService {
getAuthorizationEngine(authentication).getUserPrivileges(authentication, getAuthorizationInfoFromContext(), request, listener);
}
public static boolean isIndexAction(String action) {
return IndexPrivilege.ACTION_MATCHER.test(action);
}
private AuthorizationInfo getAuthorizationInfoFromContext() {
return Objects.requireNonNull(threadContext.getTransient(AUTHORIZATION_INFO_KEY), "authorization info is missing from context");
}
@ -574,10 +578,6 @@ public class AuthorizationService {
return new IllegalArgumentException(message);
}
public static boolean isIndexAction(String action) {
return IndexPrivilege.ACTION_MATCHER.test(action);
}
private static String getAction(BulkItemRequest item) {
final DocWriteRequest<?> docWriteRequest = item.request();
switch (docWriteRequest.opType()) {