diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java index 15497dc9d36..66380774a2f 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrail.java @@ -5,13 +5,15 @@ */ package org.elasticsearch.xpack.security.audit; +import org.elasticsearch.common.Nullable; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.xpack.security.user.User; +import org.elasticsearch.transport.TransportMessage; import org.elasticsearch.xpack.security.authc.AuthenticationToken; import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule; -import org.elasticsearch.transport.TransportMessage; +import org.elasticsearch.xpack.security.user.User; import java.net.InetAddress; +import java.util.Set; public interface AuditTrail { @@ -37,9 +39,25 @@ public interface AuditTrail { void authenticationFailed(String realm, AuthenticationToken token, RestRequest request); - void accessGranted(User user, String action, TransportMessage message); + /** + * Access was granted for some request. + * @param specificIndices if non-null then the action was authorized + * for all indices in this particular set of indices, otherwise + * the action was authorized for all indices to which it is + * related, if any + */ + void accessGranted(User user, String action, TransportMessage message, @Nullable Set specificIndices); - void accessDenied(User user, String action, TransportMessage message); + /** + * Access was denied for some request. + * @param specificIndices if non-null then the action was denied + * for at least one index in this particular set of indices, + * otherwise the action was denied for at least one index + * to which the request is related. If the request isn't + * related to any particular index then the request itself + * was denied. + */ + void accessDenied(User user, String action, TransportMessage message, @Nullable Set specificIndices); void tamperedRequest(RestRequest request); diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java index 615d86066a7..c39486ac77a 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/AuditTrailService.java @@ -5,10 +5,7 @@ */ package org.elasticsearch.xpack.security.audit; -import java.net.InetAddress; -import java.util.Collections; -import java.util.List; - +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.license.XPackLicenseState; @@ -18,6 +15,11 @@ import org.elasticsearch.xpack.security.authc.AuthenticationToken; import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule; import org.elasticsearch.xpack.security.user.User; +import java.net.InetAddress; +import java.util.Collections; +import java.util.List; +import java.util.Set; + public class AuditTrailService extends AbstractComponent implements AuditTrail { private final XPackLicenseState licenseState; @@ -130,19 +132,19 @@ public class AuditTrailService extends AbstractComponent implements AuditTrail { } @Override - public void accessGranted(User user, String action, TransportMessage message) { + public void accessGranted(User user, String action, TransportMessage message, @Nullable Set specificIndices) { if (licenseState.isAuditingAllowed()) { for (AuditTrail auditTrail : auditTrails) { - auditTrail.accessGranted(user, action, message); + auditTrail.accessGranted(user, action, message, specificIndices); } } } @Override - public void accessDenied(User user, String action, TransportMessage message) { + public void accessDenied(User user, String action, TransportMessage message, @Nullable Set specificIndices) { if (licenseState.isAuditingAllowed()) { for (AuditTrail auditTrail : auditTrails) { - auditTrail.accessDenied(user, action, message); + auditTrail.accessDenied(user, action, message, specificIndices); } } } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java index e89031f7cff..54657dd4fe1 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrail.java @@ -473,19 +473,21 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail, Cl } @Override - public void accessGranted(User user, String action, TransportMessage message) { + public void accessGranted(User user, String action, TransportMessage message, @Nullable Set specificIndices) { // special treatment for internal system actions - only log if explicitly told to if ((SystemUser.is(user) && SystemPrivilege.INSTANCE.predicate().test(action))) { if (events.contains(SYSTEM_ACCESS_GRANTED)) { try { - enqueue(message("access_granted", action, user, null, indices(message), message), "access_granted"); + Set indices = specificIndices == null ? indices(message) : specificIndices; + enqueue(message("access_granted", action, user, null, indices, message), "access_granted"); } catch (Exception e) { logger.warn("failed to index audit event: [access_granted]", e); } } } else if (events.contains(ACCESS_GRANTED) && XPackUser.is(user) == false) { try { - enqueue(message("access_granted", action, user, null, indices(message), message), "access_granted"); + Set indices = specificIndices == null ? indices(message) : specificIndices; + enqueue(message("access_granted", action, user, null, indices, message), "access_granted"); } catch (Exception e) { logger.warn("failed to index audit event: [access_granted]", e); } @@ -493,10 +495,11 @@ public class IndexAuditTrail extends AbstractComponent implements AuditTrail, Cl } @Override - public void accessDenied(User user, String action, TransportMessage message) { + public void accessDenied(User user, String action, TransportMessage message, @Nullable Set specificIndices) { if (events.contains(ACCESS_DENIED) && XPackUser.is(user) == false) { try { - enqueue(message("access_denied", action, user, null, indices(message), message), "access_denied"); + Set indices = specificIndices == null ? indices(message) : specificIndices; + enqueue(message("access_denied", action, user, null, indices, message), "access_denied"); } catch (Exception e) { logger.warn("failed to index audit event: [access_denied]", e); } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java index 762519ffe19..66e5146222d 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrail.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.security.audit.logfile; import org.apache.logging.log4j.Logger; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.network.NetworkAddress; @@ -252,12 +253,12 @@ public class LoggingAuditTrail extends AbstractComponent implements AuditTrail { } @Override - public void accessGranted(User user, String action, TransportMessage message) { + public void accessGranted(User user, String action, TransportMessage message, @Nullable Set specificIndices) { final boolean isSystem = (SystemUser.is(user) && SystemPrivilege.INSTANCE.predicate().test(action)) || XPackUser.is(user); final boolean logSystemAccessGranted = isSystem && events.contains(SYSTEM_ACCESS_GRANTED); final boolean shouldLog = logSystemAccessGranted || (isSystem == false && events.contains(ACCESS_GRANTED)); if (shouldLog) { - String indices = indicesString(message); + String indices = specificIndices == null ? indicesString(message) : collectionToCommaDelimitedString(specificIndices); if (indices != null) { logger.info("{}[transport] [access_granted]\t{}, {}, action=[{}], indices=[{}], request=[{}]", getPrefix(), originAttributes(message, clusterService.localNode(), threadContext), principal(user), action, indices, @@ -271,9 +272,9 @@ public class LoggingAuditTrail extends AbstractComponent implements AuditTrail { } @Override - public void accessDenied(User user, String action, TransportMessage message) { + public void accessDenied(User user, String action, TransportMessage message, @Nullable Set specificIndices) { if (events.contains(ACCESS_DENIED)) { - String indices = indicesString(message); + String indices = specificIndices == null ? indicesString(message) : collectionToCommaDelimitedString(specificIndices); if (indices != null) { logger.info("{}[transport] [access_denied]\t{}, {}, action=[{}], indices=[{}], request=[{}]", getPrefix(), originAttributes(message, clusterService.localNode(), threadContext), principal(user), action, indices, diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 17a79105066..c774f6b57e0 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -25,6 +25,7 @@ import org.elasticsearch.action.support.replication.TransportReplicationAction.C import org.elasticsearch.action.termvectors.MultiTermVectorsAction; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -146,10 +147,10 @@ public class AuthorizationService extends AbstractComponent { if (SystemUser.is(authentication.getUser())) { if (SystemUser.isAuthorized(action) && SystemUser.is(authentication.getUser())) { setIndicesAccessControl(IndicesAccessControl.ALLOW_ALL); - grant(authentication, action, request); + grant(authentication, action, request, null); return; } - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } // get the roles of the authenticated user, which may be different than the effective @@ -175,15 +176,15 @@ public class AuthorizationService extends AbstractComponent { ClusterPermission cluster = permission.cluster(); if (cluster.check(action) || checkSameUserPermissions(action, request, authentication)) { setIndicesAccessControl(IndicesAccessControl.ALLOW_ALL); - grant(authentication, action, request); + grant(authentication, action, request, null); return; } - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } // ok... this is not a cluster action, let's verify it's an indices action if (!IndexPrivilege.ACTION_MATCHER.test(action)) { - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } //composite actions are explicitly listed and will be authorized at the sub-request / shard level @@ -194,16 +195,16 @@ public class AuthorizationService extends AbstractComponent { } // we check if the user can execute the action, without looking at indices, which will be authorized at the shard level if (permission.indices().check(action)) { - grant(authentication, action, request); + grant(authentication, action, request, null); return; } - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } else if (isDelayedIndicesAction(action)) { /* We check now if the user can execute the action without looking at indices. * The action is itself responsible for checking if the user can access the * indices when it runs. */ if (permission.indices().check(action)) { - grant(authentication, action, request); + grant(authentication, action, request, null); /* Now that we know the user can run the action we need to build a function * that we can use later to fetch the user's actual permissions for an @@ -229,19 +230,25 @@ public class AuthorizationService extends AbstractComponent { return indicesOptions; } }; - ResolvedIndices resolvedIndices = - resolveIndexNames(authentication, action, proxy, finalRequest, metaData, authorizedIndices); + Set specificIndices = new HashSet<>(); + Collections.addAll(specificIndices, indices); + ResolvedIndices resolvedIndices; + try { + resolvedIndices = indicesAndAliasesResolver.resolve(proxy, metaData, authorizedIndices); + } catch (Exception e) { + denial(authentication, action, finalRequest, specificIndices); + throw e; + } Set localIndices = new HashSet<>(resolvedIndices.getLocal()); - IndicesAccessControl indicesAccessControl = - authorizeIndices(action, finalRequest, localIndices, authentication, finalPermission, metaData); - // NOCOMMIT we need to rework auditing so we provide the indices - grant(authentication, action, finalRequest); + IndicesAccessControl indicesAccessControl = authorizeIndices(action, finalRequest, localIndices, specificIndices, + authentication, finalPermission, metaData); + grant(authentication, action, finalRequest, specificIndices); return indicesAccessControl; }); return; } - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } else if (isTranslatedToBulkAction(action)) { if (request instanceof CompositeIndicesRequest == false) { throw new IllegalStateException("Bulk translated actions must implement " + CompositeIndicesRequest.class.getSimpleName() @@ -249,10 +256,10 @@ public class AuthorizationService extends AbstractComponent { } // we check if the user can execute the action, without looking at indices, which will be authorized at the shard level if (permission.indices().check(action)) { - grant(authentication, action, request); + grant(authentication, action, request, null); return; } - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } else if (TransportActionProxy.isProxyAction(action)) { // we authorize proxied actions once they are "unwrapped" on the next node if (TransportActionProxy.isProxyRequest(originalRequest) == false ) { @@ -260,12 +267,12 @@ public class AuthorizationService extends AbstractComponent { + action + "] is a proxy action"); } if (permission.indices().check(action)) { - grant(authentication, action, request); + grant(authentication, action, request, null); return; } else { - // we do this here in addition to the denial below since we might run into an assertion on scroll requrest below if we + // we do this here in addition to the denial below since we might run into an assertion on scroll request below if we // don't have permission to read cross cluster but wrap a scroll request. - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } } @@ -284,18 +291,18 @@ public class AuthorizationService extends AbstractComponent { // index and if they cannot, we can fail the request early before we allow the execution of the action and in // turn the shard actions if (SearchScrollAction.NAME.equals(action) && permission.indices().check(action) == false) { - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } else { // we store the request as a transient in the ThreadContext in case of a authorization failure at the shard // level. If authorization fails we will audit a access_denied message and will use the request to retrieve // information such as the index and the incoming address of the request - grant(authentication, action, request); + grant(authentication, action, request, null); return; } } else { assert false : "only scroll related requests are known indices api that don't support retrieving the indices they relate to"; - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } } @@ -305,7 +312,7 @@ public class AuthorizationService extends AbstractComponent { // If this request does not allow remote indices // then the user must have permission to perform this action on at least 1 local index if (allowsRemoteIndices == false && permission.indices().check(action) == false) { - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } final MetaData metaData = clusterService.state().metaData(); @@ -317,19 +324,20 @@ public class AuthorizationService extends AbstractComponent { // If this request does reference any remote indices // then the user must have permission to perform this action on at least 1 local index if (resolvedIndices.getRemote().isEmpty() && permission.indices().check(action) == false) { - throw denial(authentication, action, request); + throw denial(authentication, action, request, null); } //all wildcard expressions have been resolved and only the security plugin could have set '-*' here. //'-*' matches no indices so we allow the request to go through, which will yield an empty response if (resolvedIndices.isNoIndicesPlaceholder()) { setIndicesAccessControl(IndicesAccessControl.ALLOW_NO_INDICES); - grant(authentication, action, request); + grant(authentication, action, request, null); return; } final Set localIndices = new HashSet<>(resolvedIndices.getLocal()); - IndicesAccessControl indicesAccessControl = authorizeIndices(action, request, localIndices, authentication, permission, metaData); + IndicesAccessControl indicesAccessControl = authorizeIndices(action, request, localIndices, null, authentication, + permission, metaData); setIndicesAccessControl(indicesAccessControl); //if we are creating an index we need to authorize potential aliases created at the same time @@ -343,14 +351,14 @@ public class AuthorizationService extends AbstractComponent { } indicesAccessControl = permission.authorize("indices:admin/aliases", aliasesAndIndices, metaData, fieldPermissionsCache); if (!indicesAccessControl.isGranted()) { - throw denial(authentication, "indices:admin/aliases", request); + throw denial(authentication, "indices:admin/aliases", request, null); } // no need to re-add the indicesAccessControl in the context, // because the create index call doesn't do anything FLS or DLS } } - grant(authentication, action, originalRequest); + grant(authentication, action, originalRequest, null); } private ResolvedIndices resolveIndexNames(Authentication authentication, String action, Object indicesRequest, @@ -358,7 +366,7 @@ public class AuthorizationService extends AbstractComponent { try { return indicesAndAliasesResolver.resolve(indicesRequest, metaData, authorizedIndices); } catch (Exception e) { - auditTrail.accessDenied(authentication.getUser(), action, mainRequest); + auditTrail.accessDenied(authentication.getUser(), action, mainRequest, null); throw e; } } @@ -463,10 +471,10 @@ public class AuthorizationService extends AbstractComponent { * the {@link IndicesAccessControl} if they are. */ private IndicesAccessControl authorizeIndices(String action, TransportRequest request, Set localIndices, - Authentication authentication, Role permission, MetaData metaData) { + Set specificIndices, Authentication authentication, Role permission, MetaData metaData) { IndicesAccessControl indicesAccessControl = permission.authorize(action, localIndices, metaData, fieldPermissionsCache); if (!indicesAccessControl.isGranted()) { - throw denial(authentication, action, request); + throw denial(authentication, action, request, specificIndices); } if (indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME) != null && indicesAccessControl.getIndexPermissions(SecurityLifecycleService.SECURITY_INDEX_NAME).isGranted() @@ -477,7 +485,7 @@ public class AuthorizationService extends AbstractComponent { // purposes. These monitor requests also sometimes resolve indices concretely and then requests them logger.debug("user [{}] attempted to directly perform [{}] against the security index [{}]", authentication.getUser().principal(), action, SecurityLifecycleService.SECURITY_INDEX_NAME); - throw denial(authentication, action, request); + throw denial(authentication, action, request, specificIndices); } return indicesAccessControl; } @@ -526,8 +534,9 @@ public class AuthorizationService extends AbstractComponent { return ReservedRealm.TYPE.equals(realmType) || NativeRealm.TYPE.equals(realmType); } - ElasticsearchSecurityException denial(Authentication authentication, String action, TransportRequest request) { - auditTrail.accessDenied(authentication.getUser(), action, request); + ElasticsearchSecurityException denial(Authentication authentication, String action, TransportRequest request, + @Nullable Set specificIndices) { + auditTrail.accessDenied(authentication.getUser(), action, request, specificIndices); return denialException(authentication, action); } @@ -536,8 +545,8 @@ public class AuthorizationService extends AbstractComponent { return denialException(authentication, action); } - private void grant(Authentication authentication, String action, TransportRequest request) { - auditTrail.accessGranted(authentication.getUser(), action, request); + private void grant(Authentication authentication, String action, TransportRequest request, @Nullable Set specificIndices) { + auditTrail.accessGranted(authentication.getUser(), action, request, specificIndices); } private void grantRunAs(Authentication authentication, String action, TransportRequest request) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java index 071a4111f80..889f139642d 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListener.java @@ -90,7 +90,7 @@ public final class SecuritySearchOperationListener implements SearchOperationLis final boolean sameUser = samePrincipal && sameRealmType; if (sameUser == false) { - auditTrailService.accessDenied(current.getUser(), action, request); + auditTrailService.accessDenied(current.getUser(), action, request, null); throw new SearchContextMissingException(id); } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java index b7ec41b1209..e7787ea2243 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/AuditTrailServiceTests.java @@ -6,21 +6,24 @@ package org.elasticsearch.xpack.security.audit; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.rest.RestRequest; import org.elasticsearch.license.XPackLicenseState; -import org.elasticsearch.xpack.security.user.User; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.TransportMessage; import org.elasticsearch.xpack.security.authc.AuthenticationToken; import org.elasticsearch.xpack.security.transport.filter.IPFilter; import org.elasticsearch.xpack.security.transport.filter.SecurityIpFilterRule; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.transport.TransportMessage; +import org.elasticsearch.xpack.security.user.User; import org.junit.Before; import java.net.InetAddress; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static java.util.Collections.unmodifiableList; +import static java.util.Collections.unmodifiableSet; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -138,11 +141,12 @@ public class AuditTrailServiceTests extends ESTestCase { public void testAccessGranted() throws Exception { User user = new User("_username", "r1"); - service.accessGranted(user, "_action", message); + Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; + service.accessGranted(user, "_action", message, specificIndices); verify(licenseState).isAuditingAllowed(); if (isAuditingAllowed) { for (AuditTrail auditTrail : auditTrails) { - verify(auditTrail).accessGranted(user, "_action", message); + verify(auditTrail).accessGranted(user, "_action", message, specificIndices); } } else { verifyZeroInteractions(auditTrails.toArray((Object[]) new AuditTrail[auditTrails.size()])); @@ -151,11 +155,12 @@ public class AuditTrailServiceTests extends ESTestCase { public void testAccessDenied() throws Exception { User user = new User("_username", "r1"); - service.accessDenied(user, "_action", message); + Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; + service.accessDenied(user, "_action", message, specificIndices); verify(licenseState).isAuditingAllowed(); if (isAuditingAllowed) { for (AuditTrail auditTrail : auditTrails) { - verify(auditTrail).accessDenied(user, "_action", message); + verify(auditTrail).accessDenied(user, "_action", message, specificIndices); } } else { verifyZeroInteractions(auditTrails.toArray((Object[]) new AuditTrail[auditTrails.size()])); @@ -217,4 +222,18 @@ public class AuditTrailServiceTests extends ESTestCase { verifyZeroInteractions(auditTrails.toArray((Object[]) new AuditTrail[auditTrails.size()])); } } + + /** + * Generates a semi-believable random value for the specificIndices parameter sent + * to the {@link AuditTrail#accessGranted(User, String, TransportMessage, Set)} or + * {@link AuditTrail#accessDenied(User, String, TransportMessage, Set)} methods. + */ + public static Set randomSpecificIndices() { + Set specificIndices = new HashSet<>(); + int count = between(1, 10); + while (specificIndices.size() < count) { + specificIndices.add(randomAlphaOfLength(5)); + } + return unmodifiableSet(specificIndices); + } } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java index fd685b33db0..de0e345c200 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailMutedTests.java @@ -5,11 +5,6 @@ */ package org.elasticsearch.xpack.security.audit.index; -import java.net.InetAddress; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.atomic.AtomicBoolean; - import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; @@ -35,6 +30,12 @@ import org.elasticsearch.xpack.security.user.User; import org.junit.After; import org.junit.Before; +import java.net.InetAddress; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.elasticsearch.xpack.security.audit.AuditTrailServiceTests.randomSpecificIndices; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyZeroInteractions; @@ -173,7 +174,7 @@ public class IndexAuditTrailMutedTests extends ESTestCase { createAuditTrail(new String[] { "access_granted" }); TransportMessage message = mock(TransportMessage.class); User user = mock(User.class); - auditTrail.accessGranted(user, randomAlphaOfLengthBetween(6, 40), message); + auditTrail.accessGranted(user, randomAlphaOfLengthBetween(6, 40), message, randomBoolean() ? randomSpecificIndices() : null); assertThat(messageEnqueued.get(), is(false)); assertThat(clientCalled.get(), is(false)); @@ -184,7 +185,7 @@ public class IndexAuditTrailMutedTests extends ESTestCase { createAuditTrail(randomFrom(new String[] { "access_granted" }, null)); TransportMessage message = mock(TransportMessage.class); User user = SystemUser.INSTANCE; - auditTrail.accessGranted(user, "internal:foo", message); + auditTrail.accessGranted(user, "internal:foo", message, randomBoolean() ? randomSpecificIndices() : null); assertThat(messageEnqueued.get(), is(false)); assertThat(clientCalled.get(), is(false)); @@ -195,7 +196,7 @@ public class IndexAuditTrailMutedTests extends ESTestCase { createAuditTrail(new String[] { "access_denied" }); TransportMessage message = mock(TransportMessage.class); User user = mock(User.class); - auditTrail.accessDenied(user, randomAlphaOfLengthBetween(6, 40), message); + auditTrail.accessDenied(user, randomAlphaOfLengthBetween(6, 40), message, randomBoolean() ? randomSpecificIndices() : null); assertThat(messageEnqueued.get(), is(false)); assertThat(clientCalled.get(), is(false)); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java index cbaf1f5aab6..41d22545bd4 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/index/IndexAuditTrailTests.java @@ -67,6 +67,7 @@ import java.util.function.Function; import static org.elasticsearch.test.ESIntegTestCase.Scope.SUITE; import static org.elasticsearch.test.InternalTestCluster.clusterName; +import static org.elasticsearch.xpack.security.audit.AuditTrailServiceTests.randomSpecificIndices; import static org.elasticsearch.xpack.security.audit.index.IndexNameResolver.Rollover.DAILY; import static org.elasticsearch.xpack.security.audit.index.IndexNameResolver.Rollover.HOURLY; import static org.elasticsearch.xpack.security.audit.index.IndexNameResolver.Rollover.MONTHLY; @@ -472,7 +473,8 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { } else { user = new User("_username", new String[]{"r1"}); } - auditor.accessGranted(user, "_action", message); + Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; + auditor.accessGranted(user, "_action", message, specificIndices); SearchHit hit = getIndexedAuditMessage(enqueuedMessage.get()); assertAuditMessage(hit, "transport", "access_granted"); @@ -485,9 +487,15 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { assertEquals("_username", sourceMap.get("principal")); } assertEquals("_action", sourceMap.get("action")); - if (message instanceof IndicesRequest) { - List indices = (List) sourceMap.get("indices"); - assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); + List indices = (List) sourceMap.get("indices"); + if (specificIndices == null) { + if (message instanceof IndicesRequest) { + assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); + } else { + assertThat(indices, nullValue()); + } + } else { + assertThat(indices, containsInAnyOrder(specificIndices.toArray())); } assertEquals(sourceMap.get("request"), message.getClass().getSimpleName()); } @@ -495,7 +503,7 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { public void testSystemAccessGranted() throws Exception { initialize(new String[] { "system_access_granted" }, null); TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage(); - auditor.accessGranted(SystemUser.INSTANCE, "internal:_action", message); + auditor.accessGranted(SystemUser.INSTANCE, "internal:_action", message, null); SearchHit hit = getIndexedAuditMessage(enqueuedMessage.get()); assertAuditMessage(hit, "transport", "access_granted"); @@ -516,7 +524,8 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { } else { user = new User("_username", new String[]{"r1"}); } - auditor.accessDenied(user, "_action", message); + Set specificIndices = randomBoolean() ? randomSpecificIndices() : null; + auditor.accessDenied(user, "_action", message, specificIndices); SearchHit hit = getIndexedAuditMessage(enqueuedMessage.get()); Map sourceMap = hit.getSourceAsMap(); @@ -529,9 +538,15 @@ public class IndexAuditTrailTests extends SecurityIntegTestCase { assertEquals("_username", sourceMap.get("principal")); } assertEquals("_action", sourceMap.get("action")); - if (message instanceof IndicesRequest) { - List indices = (List) sourceMap.get("indices"); - assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); + List indices = (List) sourceMap.get("indices"); + if (specificIndices == null) { + if (message instanceof IndicesRequest) { + assertThat(indices, containsInAnyOrder((Object[]) ((IndicesRequest)message).indices())); + } else { + assertThat(indices, nullValue()); + } + } else { + assertThat(indices, containsInAnyOrder(specificIndices.toArray())); } assertEquals(sourceMap.get("request"), message.getClass().getSimpleName()); } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java index 22798e3b736..4cbabc3b9c5 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.FakeRestRequest.Builder; import org.elasticsearch.transport.TransportMessage; +import org.elasticsearch.xpack.security.audit.AuditTrail; import org.elasticsearch.xpack.security.audit.AuditUtil; import org.elasticsearch.xpack.security.authc.AuthenticationToken; import org.elasticsearch.xpack.security.rest.RemoteHostHeader; @@ -40,10 +41,13 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; +import static java.util.Collections.unmodifiableSet; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; @@ -331,7 +335,7 @@ public class LoggingAuditTrailTests extends ESTestCase { user = new User("_username", new String[]{"r1"}); } String userInfo = runAs ? "principal=[running as], run_by_principal=[_username]" : "principal=[_username]"; - auditTrail.accessGranted(user, "_action", message); + auditTrail.accessGranted(user, "_action", message, null); if (message instanceof IndicesRequest) { assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", " + userInfo + ", action=[_action], indices=[" + indices(message) + "], request=[MockIndicesRequest]"); @@ -340,11 +344,19 @@ public class LoggingAuditTrailTests extends ESTestCase { ", action=[_action], request=[MockMessage]"); } + // test specificIndices + CapturingLogger.output(logger.getName(), Level.INFO).clear(); + Set specificIndices = randomSpecificIndices(); + auditTrail.accessGranted(user, "_action", message, specificIndices); + assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", " + userInfo + + ", action=[_action], indices=[" + Strings.collectionToCommaDelimitedString(specificIndices) + + "], request=[" + message.getClass().getSimpleName() + "]"); + // test disabled CapturingLogger.output(logger.getName(), Level.INFO).clear(); settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.exclude", "access_granted").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessGranted(user, "_action", message); + auditTrail.accessGranted(user, "_action", message, randomBoolean() ? specificIndices : null); assertEmptyLog(logger); } @@ -353,13 +365,13 @@ public class LoggingAuditTrailTests extends ESTestCase { LoggingAuditTrail auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); TransportMessage message = randomBoolean() ? new MockMessage(threadContext) : new MockIndicesRequest(threadContext); String origins = LoggingAuditTrail.originAttributes(message, localNode, threadContext); - auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message); + auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message, null); assertEmptyLog(logger); // test enabled settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.include", "system_access_granted").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message); + auditTrail.accessGranted(SystemUser.INSTANCE, "internal:_action", message, null); if (message instanceof IndicesRequest) { assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", principal=[" + SystemUser.INSTANCE.principal() @@ -383,7 +395,7 @@ public class LoggingAuditTrailTests extends ESTestCase { user = new User("_username", new String[]{"r1"}); } String userInfo = runAs ? "principal=[running as], run_by_principal=[_username]" : "principal=[_username]"; - auditTrail.accessGranted(user, "internal:_action", message); + auditTrail.accessGranted(user, "internal:_action", message, null); if (message instanceof IndicesRequest) { assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", " + userInfo + ", action=[internal:_action], indices=[" + indices(message) + "], request=[MockIndicesRequest]"); @@ -396,7 +408,7 @@ public class LoggingAuditTrailTests extends ESTestCase { CapturingLogger.output(logger.getName(), Level.INFO).clear(); settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.exclude", "access_granted").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessGranted(user, "internal:_action", message); + auditTrail.accessGranted(user, "internal:_action", message, null); assertEmptyLog(logger); } @@ -413,7 +425,7 @@ public class LoggingAuditTrailTests extends ESTestCase { user = new User("_username", new String[]{"r1"}); } String userInfo = runAs ? "principal=[running as], run_by_principal=[_username]" : "principal=[_username]"; - auditTrail.accessDenied(user, "_action", message); + auditTrail.accessDenied(user, "_action", message, null); if (message instanceof IndicesRequest) { assertMsg(logger, Level.INFO, prefix + "[transport] [access_denied]\t" + origins + ", " + userInfo + ", action=[_action], indices=[" + indices(message) + "], request=[MockIndicesRequest]"); @@ -422,11 +434,19 @@ public class LoggingAuditTrailTests extends ESTestCase { ", action=[_action], request=[MockMessage]"); } + // test specificIndices + CapturingLogger.output(logger.getName(), Level.INFO).clear(); + Set specificIndices = randomSpecificIndices(); + auditTrail.accessDenied(user, "_action", message, specificIndices); + assertMsg(logger, Level.INFO, prefix + "[transport] [access_denied]\t" + origins + ", " + userInfo + + ", action=[_action], indices=[" + Strings.collectionToCommaDelimitedString(specificIndices) + + "], request=[" + message.getClass().getSimpleName() + "]"); + // test disabled CapturingLogger.output(logger.getName(), Level.INFO).clear(); settings = Settings.builder().put(settings).put("xpack.security.audit.logfile.events.exclude", "access_denied").build(); auditTrail = new LoggingAuditTrail(settings, clusterService, logger, threadContext); - auditTrail.accessDenied(user, "_action", message); + auditTrail.accessDenied(user, "_action", message, null); assertEmptyLog(logger); } @@ -708,6 +728,20 @@ public class LoggingAuditTrailTests extends ESTestCase { return Strings.collectionToCommaDelimitedString(AuditUtil.indices(message)); } + /** + * Generates a semi-believable random value for the specificIndices parameter sent + * to the {@link AuditTrail#accessGranted(User, String, TransportMessage, Set)} or + * {@link AuditTrail#accessDenied(User, String, TransportMessage, Set)} methods. + */ + private static Set randomSpecificIndices() { + Set specificIndices = new HashSet<>(); + int count = between(1, 10); + while (specificIndices.size() < count) { + specificIndices.add(randomAlphaOfLength(5)); + } + return unmodifiableSet(specificIndices); + } + private static class MockMessage extends TransportMessage { private MockMessage(ThreadContext threadContext) throws IOException { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 8606c42fdd0..bfa2a8e97b8 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.security.authz; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; @@ -116,6 +117,7 @@ import org.elasticsearch.xpack.security.user.SystemUser; import org.elasticsearch.xpack.security.user.User; import org.elasticsearch.xpack.security.user.XPackUser; import org.elasticsearch.xpack.sql.plugin.sql.action.SqlAction; +import org.elasticsearch.xpack.sql.plugin.sql.action.SqlRequest; import org.junit.Before; import java.util.ArrayList; @@ -127,6 +129,7 @@ import java.util.Map; import java.util.Set; import java.util.function.BiFunction; +import static java.util.Collections.singleton; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationExceptionRunAs; @@ -210,10 +213,10 @@ public class AuthorizationServiceTests extends ESTestCase { // A failure would throw an exception authorize(createAuthentication(SystemUser.INSTANCE), "indices:monitor/whatever", request); - verify(auditTrail).accessGranted(SystemUser.INSTANCE, "indices:monitor/whatever", request); + verify(auditTrail).accessGranted(SystemUser.INSTANCE, "indices:monitor/whatever", request, null); authorize(createAuthentication(SystemUser.INSTANCE), "internal:whatever", request); - verify(auditTrail).accessGranted(SystemUser.INSTANCE, "internal:whatever", request); + verify(auditTrail).accessGranted(SystemUser.INSTANCE, "internal:whatever", request, null); verifyNoMoreInteractions(auditTrail); } @@ -222,7 +225,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(SystemUser.INSTANCE), "indices:", request), "indices:", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(SystemUser.INSTANCE, "indices:", request); + verify(auditTrail).accessDenied(SystemUser.INSTANCE, "indices:", request, null); verifyNoMoreInteractions(auditTrail); } @@ -231,7 +234,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(SystemUser.INSTANCE), "cluster:admin/whatever", request), "cluster:admin/whatever", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/whatever", request); + verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/whatever", request, null); verifyNoMoreInteractions(auditTrail); } @@ -240,7 +243,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(SystemUser.INSTANCE), "cluster:admin/snapshot/status", request), "cluster:admin/snapshot/status", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/snapshot/status", request); + verify(auditTrail).accessDenied(SystemUser.INSTANCE, "cluster:admin/snapshot/status", request, null); verifyNoMoreInteractions(auditTrail); } @@ -251,7 +254,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), "indices:a", request), "indices:a", "test user"); - verify(auditTrail).accessDenied(user, "indices:a", request); + verify(auditTrail).accessDenied(user, "indices:a", request, null); verifyNoMoreInteractions(auditTrail); } @@ -261,7 +264,7 @@ public class AuthorizationServiceTests extends ESTestCase { User user = new User("test user"); mockEmptyMetaData(); authorize(createAuthentication(user), SearchAction.NAME, request); - verify(auditTrail).accessGranted(user, SearchAction.NAME, request); + verify(auditTrail).accessGranted(user, SearchAction.NAME, request, null); verifyNoMoreInteractions(auditTrail); } @@ -278,7 +281,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SearchAction.NAME, request), SearchAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SearchAction.NAME, request); + verify(auditTrail).accessDenied(user, SearchAction.NAME, request, null); verifyNoMoreInteractions(auditTrail); } @@ -294,7 +297,18 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SearchAction.NAME, request), SearchAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SearchAction.NAME, request); + verify(auditTrail).accessDenied(user, SearchAction.NAME, request, null); + verifyNoMoreInteractions(auditTrail); + } + + public void testUserWithNoRolesCannotSql() { + TransportRequest request = new SqlRequest(); + User user = new User("test user"); + mockEmptyMetaData(); + assertThrowsAuthorizationException( + () -> authorize(createAuthentication(user), SqlAction.NAME, request), + SqlAction.NAME, "test user"); + verify(auditTrail).accessDenied(user, SqlAction.NAME, request, null); verifyNoMoreInteractions(auditTrail); } @@ -310,18 +324,24 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), DeleteIndexAction.NAME, request), DeleteIndexAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, DeleteIndexAction.NAME, request); + verify(auditTrail).accessDenied(user, DeleteIndexAction.NAME, request, null); verifyNoMoreInteractions(auditTrail); } public void testUnknownRoleCausesDenial() { - TransportRequest request = new SearchRequest(); + @SuppressWarnings("unchecked") + Tuple tuple = randomFrom( + new Tuple<>(SearchAction.NAME, new SearchRequest()), + new Tuple<>(IndicesExistsAction.NAME, new IndicesExistsRequest()), + new Tuple<>(SqlAction.NAME, new SqlRequest())); + String action = tuple.v1(); + TransportRequest request = tuple.v2(); User user = new User("test user", "non-existent-role"); mockEmptyMetaData(); assertThrowsAuthorizationException( - () -> authorize(createAuthentication(user), "indices:a", request), - "indices:a", "test user"); - verify(auditTrail).accessDenied(user, "indices:a", request); + () -> authorize(createAuthentication(user), action, request), + action, "test user"); + verify(auditTrail).accessDenied(user, action, request, null); verifyNoMoreInteractions(auditTrail); } @@ -334,20 +354,26 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), "whatever", request), "whatever", "test user"); - verify(auditTrail).accessDenied(user, "whatever", request); + verify(auditTrail).accessDenied(user, "whatever", request, null); verifyNoMoreInteractions(auditTrail); } public void testThatRoleWithNoIndicesIsDenied() { - TransportRequest request = new IndicesExistsRequest("a"); + @SuppressWarnings("unchecked") + Tuple tuple = randomFrom( + new Tuple<>(SearchAction.NAME, new SearchRequest()), + new Tuple<>(IndicesExistsAction.NAME, new IndicesExistsRequest()), + new Tuple<>(SqlAction.NAME, new SqlRequest())); + String action = tuple.v1(); + TransportRequest request = tuple.v2(); User user = new User("test user", "no_indices"); roleMap.put("no_indices", new RoleDescriptor("a_role",null,null,null)); mockEmptyMetaData(); assertThrowsAuthorizationException( - () -> authorize(createAuthentication(user), "indices:a", request), - "indices:a", "test user"); - verify(auditTrail).accessDenied(user, "indices:a", request); + () -> authorize(createAuthentication(user), action, request), + action, "test user"); + verify(auditTrail).accessDenied(user, action, request, null); verifyNoMoreInteractions(auditTrail); } @@ -356,7 +382,7 @@ public class AuthorizationServiceTests extends ESTestCase { Tuple request = randomCompositeRequest(); authorize(createAuthentication(user), request.v1(), request.v2()); - verify(auditTrail).accessGranted(user, request.v1(), request.v2()); + verify(auditTrail).accessGranted(user, request.v1(), request.v2(), null); } public void testSearchAgainstEmptyCluster() { @@ -373,7 +399,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), SearchAction.NAME, searchRequest), SearchAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, SearchAction.NAME, searchRequest); + verify(auditTrail).accessDenied(user, SearchAction.NAME, searchRequest, null); verifyNoMoreInteractions(auditTrail); } @@ -382,7 +408,7 @@ public class AuthorizationServiceTests extends ESTestCase { SearchRequest searchRequest = new SearchRequest("does_not_exist") .indicesOptions(IndicesOptions.fromOptions(true, true, true, false)); authorize(createAuthentication(user), SearchAction.NAME, searchRequest); - verify(auditTrail).accessGranted(user, SearchAction.NAME, searchRequest); + verify(auditTrail).accessGranted(user, SearchAction.NAME, searchRequest, null); IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_KEY); IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(IndicesAndAliasesResolver.NO_INDEX_PLACEHOLDER); @@ -399,28 +425,28 @@ public class AuthorizationServiceTests extends ESTestCase { ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); authorize(createAuthentication(user), ClearScrollAction.NAME, clearScrollRequest); - verify(auditTrail).accessGranted(user, ClearScrollAction.NAME, clearScrollRequest); + verify(auditTrail).accessGranted(user, ClearScrollAction.NAME, clearScrollRequest, null); SearchScrollRequest searchScrollRequest = new SearchScrollRequest(); authorize(createAuthentication(user), SearchScrollAction.NAME, searchScrollRequest); - verify(auditTrail).accessGranted(user, SearchScrollAction.NAME, searchScrollRequest); + verify(auditTrail).accessGranted(user, SearchScrollAction.NAME, searchScrollRequest, null); // We have to use a mock request for other Scroll actions as the actual requests are package private to SearchTransportService TransportRequest request = mock(TransportRequest.class); authorize(createAuthentication(user), SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME, request); - verify(auditTrail).accessGranted(user, SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME, request); + verify(auditTrail).accessGranted(user, SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME, request, null); authorize(createAuthentication(user), SearchTransportService.FETCH_ID_SCROLL_ACTION_NAME, request); - verify(auditTrail).accessGranted(user, SearchTransportService.FETCH_ID_SCROLL_ACTION_NAME, request); + verify(auditTrail).accessGranted(user, SearchTransportService.FETCH_ID_SCROLL_ACTION_NAME, request, null); authorize(createAuthentication(user), SearchTransportService.QUERY_FETCH_SCROLL_ACTION_NAME, request); - verify(auditTrail).accessGranted(user, SearchTransportService.QUERY_FETCH_SCROLL_ACTION_NAME, request); + verify(auditTrail).accessGranted(user, SearchTransportService.QUERY_FETCH_SCROLL_ACTION_NAME, request, null); authorize(createAuthentication(user), SearchTransportService.QUERY_SCROLL_ACTION_NAME, request); - verify(auditTrail).accessGranted(user, SearchTransportService.QUERY_SCROLL_ACTION_NAME, request); + verify(auditTrail).accessGranted(user, SearchTransportService.QUERY_SCROLL_ACTION_NAME, request, null); authorize(createAuthentication(user), SearchTransportService.FREE_CONTEXT_SCROLL_ACTION_NAME, request); - verify(auditTrail).accessGranted(user, SearchTransportService.FREE_CONTEXT_SCROLL_ACTION_NAME, request); + verify(auditTrail).accessGranted(user, SearchTransportService.FREE_CONTEXT_SCROLL_ACTION_NAME, request, null); verifyNoMoreInteractions(auditTrail); } @@ -434,7 +460,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), "indices:a", request), "indices:a", "test user"); - verify(auditTrail).accessDenied(user, "indices:a", request); + verify(auditTrail).accessDenied(user, "indices:a", request, null); verifyNoMoreInteractions(auditTrail); verify(clusterService, times(1)).state(); verify(state, times(1)).metaData(); @@ -451,7 +477,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), CreateIndexAction.NAME, request), IndicesAliasesAction.NAME, "test user"); - verify(auditTrail).accessDenied(user, IndicesAliasesAction.NAME, request); + verify(auditTrail).accessDenied(user, IndicesAliasesAction.NAME, request, null); verifyNoMoreInteractions(auditTrail); verify(clusterService).state(); verify(state, times(1)).metaData(); @@ -467,7 +493,7 @@ public class AuthorizationServiceTests extends ESTestCase { authorize(createAuthentication(user), CreateIndexAction.NAME, request); - verify(auditTrail).accessGranted(user, CreateIndexAction.NAME, request); + verify(auditTrail).accessGranted(user, CreateIndexAction.NAME, request, null); verifyNoMoreInteractions(auditTrail); verify(clusterService).state(); verify(state, times(1)).metaData(); @@ -487,7 +513,7 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(anonymousUser), "indices:a", request), "indices:a", anonymousUser.principal()); - verify(auditTrail).accessDenied(anonymousUser, "indices:a", request); + verify(auditTrail).accessDenied(anonymousUser, "indices:a", request, null); verifyNoMoreInteractions(auditTrail); verify(clusterService, times(1)).state(); verify(state, times(1)).metaData(); @@ -510,7 +536,7 @@ public class AuthorizationServiceTests extends ESTestCase { ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class, () -> authorize(createAuthentication(anonymousUser), "indices:a", request)); assertAuthenticationException(securityException, containsString("action [indices:a] requires authentication")); - verify(auditTrail).accessDenied(anonymousUser, "indices:a", request); + verify(auditTrail).accessDenied(anonymousUser, "indices:a", request, null); verifyNoMoreInteractions(auditTrail); verify(clusterService, times(1)).state(); verify(state, times(1)).metaData(); @@ -529,7 +555,33 @@ public class AuthorizationServiceTests extends ESTestCase { () -> authorize(createAuthentication(user), GetIndexAction.NAME, request)); assertThat(nfe.getIndex(), is(notNullValue())); assertThat(nfe.getIndex().getName(), is("not-an-index-*")); - verify(auditTrail).accessDenied(user, GetIndexAction.NAME, request); + verify(auditTrail).accessDenied(user, GetIndexAction.NAME, request, null); + verifyNoMoreInteractions(auditTrail); + verify(clusterService).state(); + verify(state, times(1)).metaData(); + } + + public void testAuditTrailIsRecordedWhenIndexWildcardThrowsErrorDuringDelayed() { + IndicesOptions options = IndicesOptions.fromOptions(false, false, false, false); + TransportRequest request = new SqlRequest(); + ClusterState state = mockEmptyMetaData(); + User user = new User("test user", "a_all"); + roleMap.put("a_all", new RoleDescriptor("a_all",null, + new IndicesPrivileges[] { IndicesPrivileges.builder().indices("a").privileges("all").build() },null)); + + try (StoredContext context = threadContext.stashContext()) { + authorize(createAuthentication(user), SqlAction.NAME, request); + verify(auditTrail).accessGranted(user, SqlAction.NAME, request, null); + + BiFunction resolver = + threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_RESOLVER_KEY); + final Exception e = expectThrows( + ElasticsearchParseException.class, + () -> resolver.apply(options, new String[] {"<{>"})); + assertEquals("invalid dynamic name expression [{>]. date math placeholder is open ended", e.getMessage()); + verify(auditTrail).accessDenied(user, SqlAction.NAME, request, singleton("<{>")); + } + verifyNoMoreInteractions(auditTrail); verify(clusterService).state(); verify(state, times(1)).metaData(); @@ -600,7 +652,7 @@ public class AuthorizationServiceTests extends ESTestCase { () -> authorize(createAuthentication(user), "indices:a", request), "indices:a", "test user", "run as me"); verify(auditTrail).runAsGranted(user, "indices:a", request); - verify(auditTrail).accessDenied(user, "indices:a", request); + verify(auditTrail).accessDenied(user, "indices:a", request, null); verifyNoMoreInteractions(auditTrail); } @@ -623,7 +675,7 @@ public class AuthorizationServiceTests extends ESTestCase { authorize(createAuthentication(user), "indices:a", request); verify(auditTrail).runAsGranted(user, "indices:a", request); - verify(auditTrail).accessGranted(user, "indices:a", request); + verify(auditTrail).accessGranted(user, "indices:a", request, null); verifyNoMoreInteractions(auditTrail); } @@ -660,19 +712,37 @@ public class AuthorizationServiceTests extends ESTestCase { assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, request), action, "all_access_user"); - verify(auditTrail).accessDenied(user, action, request); + verify(auditTrail).accessDenied(user, action, request, null); verifyNoMoreInteractions(auditTrail); } // we should allow waiting for the health of the index or any index if the user has this permission - ClusterHealthRequest request = new ClusterHealthRequest(SecurityLifecycleService.SECURITY_INDEX_NAME); + TransportRequest request = new ClusterHealthRequest(SecurityLifecycleService.SECURITY_INDEX_NAME); authorize(createAuthentication(user), ClusterHealthAction.NAME, request); - verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request); + verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, null); + verifyNoMoreInteractions(auditTrail); // multiple indices request = new ClusterHealthRequest(SecurityLifecycleService.SECURITY_INDEX_NAME, "foo", "bar"); authorize(createAuthentication(user), ClusterHealthAction.NAME, request); - verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request); + verify(auditTrail).accessGranted(user, ClusterHealthAction.NAME, request, null); + verifyNoMoreInteractions(auditTrail); + + // Delayed request + try (StoredContext context = threadContext.stashContext()) { + request = new SqlRequest(); + authorize(createAuthentication(user), SqlAction.NAME, request); + verify(auditTrail).accessGranted(user, SqlAction.NAME, request, null); + + BiFunction resolver = + threadContext.getTransient(AuthorizationService.INDICES_PERMISSIONS_RESOLVER_KEY); + assertThrowsAuthorizationException( + () -> resolver.apply(IndicesOptions.strictSingleIndexNoExpandForbidClosed(), + new String[] {SecurityLifecycleService.SECURITY_INDEX_NAME}), + SqlAction.NAME, "all_access_user"); + verify(auditTrail).accessDenied(user, SqlAction.NAME, request, singleton(SecurityLifecycleService.SECURITY_INDEX_NAME)); + verifyNoMoreInteractions(auditTrail); + } SearchRequest searchRequest = new SearchRequest("_all"); authorize(createAuthentication(user), SearchAction.NAME, searchRequest); @@ -707,7 +777,7 @@ public class AuthorizationServiceTests extends ESTestCase { String action = requestTuple.v1(); TransportRequest request = requestTuple.v2(); authorize(createAuthentication(user), action, request); - verify(auditTrail).accessGranted(user, action, request); + verify(auditTrail).accessGranted(user, action, request, null); } } @@ -747,7 +817,7 @@ public class AuthorizationServiceTests extends ESTestCase { String action = requestTuple.v1(); TransportRequest request = requestTuple.v2(); authorize(createAuthentication(user), action, request); - verify(auditTrail).accessGranted(user, action, request); + verify(auditTrail).accessGranted(user, action, request, null); } } } @@ -766,12 +836,12 @@ public class AuthorizationServiceTests extends ESTestCase { String action = SearchAction.NAME; SearchRequest request = new SearchRequest("_all"); authorize(createAuthentication(XPackUser.INSTANCE), action, request); - verify(auditTrail).accessGranted(XPackUser.INSTANCE, action, request); + verify(auditTrail).accessGranted(XPackUser.INSTANCE, action, request, null); assertThat(request.indices(), arrayContaining(".security")); request = new SearchRequest("_all"); authorize(createAuthentication(superuser), action, request); - verify(auditTrail).accessGranted(superuser, action, request); + verify(auditTrail).accessGranted(superuser, action, request, null); assertThat(request.indices(), arrayContaining(".security")); } @@ -825,7 +895,7 @@ public class AuthorizationServiceTests extends ESTestCase { roleMap.put("no_indices", new RoleDescriptor("no_indices", null, null, null)); assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, request), action, "test user"); - verify(auditTrail).accessDenied(user, action, request); + verify(auditTrail).accessDenied(user, action, request, null); verifyNoMoreInteractions(auditTrail); } @@ -839,7 +909,7 @@ public class AuthorizationServiceTests extends ESTestCase { new IndicesPrivileges[] { IndicesPrivileges.builder().indices(randomBoolean() ? "a" : "index").privileges("all").build() }, null)); authorize(createAuthentication(user), action, request); - verify(auditTrail).accessGranted(user, action, request); + verify(auditTrail).accessGranted(user, action, request, null); verifyNoMoreInteractions(auditTrail); } @@ -892,6 +962,17 @@ public class AuthorizationServiceTests extends ESTestCase { () -> authorize(createAuthentication(userDenied), action, request), action, "userDenied"); } + public void testDelayedActionsAreImmediatelyRejectedWithoutPermissions() { + //if the user has no permission for composite actions against any index, the request fails straight-away in the main action + TransportRequest request = new SqlRequest(); + User user = new User("test user", "no_indices"); + roleMap.put("no_indices", new RoleDescriptor("no_indices", null, null, null)); + assertThrowsAuthorizationException( + () -> authorize(createAuthentication(user), SqlAction.NAME, request), SqlAction.NAME, "test user"); + verify(auditTrail).accessDenied(user, SqlAction.NAME, request, null); + verifyNoMoreInteractions(auditTrail); + } + public void testDelayedActionsAddResolver() { String action = SqlAction.NAME; MockIndicesRequest request = new MockIndicesRequest(IndicesOptions.strictExpandOpen(), "index"); @@ -913,25 +994,29 @@ public class AuthorizationServiceTests extends ESTestCase { try (StoredContext ctxRestore = threadContext.stashContext()) { authorize(createAuthentication(userAllowed), action, request); - // NOCOMMIT we need to test the audit trail but it needs to be improved to allow explicit indices. + verify(auditTrail).accessGranted(userAllowed, action, request, null); assertNotNull(getAccessControlResolver()); IndicesAccessControl iac = getAccessControlResolver().apply(IndicesOptions.strictExpandOpen(), new String[] {"index"}); assertTrue(iac.isGranted()); assertTrue(iac.getIndexPermissions("index").isGranted()); + verify(auditTrail).accessGranted(userAllowed, action, request, singleton("index")); } assertNull(getAccessControlResolver()); try (StoredContext ctxRestore = threadContext.stashContext()) { authorize(createAuthentication(userDenied), action, request); + verify(auditTrail).accessGranted(userDenied, action, request, null); assertNotNull(getAccessControlResolver()); assertThrowsAuthorizationException( () -> getAccessControlResolver().apply(IndicesOptions.strictExpandOpen(), new String[] {"index"}), action, "userDenied"); + verify(auditTrail).accessDenied(userDenied, action, request, singleton("index")); } assertNull(getAccessControlResolver()); try (StoredContext ctxRestore = threadContext.stashContext()) { authorize(createAuthentication(userSome), action, request); + verify(auditTrail).accessGranted(userSome, action, request, null); assertNotNull(getAccessControlResolver()); IndicesAccessControl iac = getAccessControlResolver().apply(IndicesOptions.strictExpandOpen(), new String[] {"a"}); assertTrue(iac.isGranted()); @@ -940,15 +1025,19 @@ public class AuthorizationServiceTests extends ESTestCase { iac = getAccessControlResolver().apply(IndicesOptions.strictExpandOpen(), new String[] {"b"}); assertTrue(iac.isGranted()); assertTrue(iac.getIndexPermissions("b").isGranted()); + verify(auditTrail).accessGranted(userSome, action, request, singleton("b")); iac = getAccessControlResolver().apply(IndicesOptions.strictExpandOpen(), new String[] {"a", "b"}); assertTrue(iac.isGranted()); assertTrue(iac.getIndexPermissions("a").isGranted()); assertTrue(iac.getIndexPermissions("b").isGranted()); + verify(auditTrail).accessGranted(userSome, action, request, singleton("a")); + verify(auditTrail).accessGranted(userSome, action, request, singleton("b")); assertThrowsAuthorizationException( () -> getAccessControlResolver().apply(IndicesOptions.strictExpandOpen(), new String[] {"index", "a", "b"}), action, "userSome"); + verify(auditTrail).accessDenied(userSome, action, request, new HashSet<>(Arrays.asList("index", "a", "b"))); } assertNull(getAccessControlResolver()); } @@ -1173,7 +1262,7 @@ public class AuthorizationServiceTests extends ESTestCase { roleMap.put("no_indices", new RoleDescriptor("no_indices", null, null, null)); assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, transportRequest), action, "test user"); - verify(auditTrail).accessDenied(user, action, proxiedRequest); + verify(auditTrail).accessDenied(user, action, proxiedRequest, null); verifyNoMoreInteractions(auditTrail); } @@ -1188,7 +1277,7 @@ public class AuthorizationServiceTests extends ESTestCase { TransportRequest transportRequest = TransportActionProxy.wrapRequest(node, clearScrollRequest); String action = TransportActionProxy.getProxyAction(SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME); authorize(createAuthentication(user), action, transportRequest); - verify(auditTrail).accessGranted(user, action, clearScrollRequest); + verify(auditTrail).accessGranted(user, action, clearScrollRequest, null); } public void testProxyRequestAuthenticationGranted() { @@ -1202,7 +1291,7 @@ public class AuthorizationServiceTests extends ESTestCase { TransportRequest transportRequest = TransportActionProxy.wrapRequest(node, clearScrollRequest); String action = TransportActionProxy.getProxyAction(SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME); authorize(createAuthentication(user), action, transportRequest); - verify(auditTrail).accessGranted(user, action, clearScrollRequest); + verify(auditTrail).accessGranted(user, action, clearScrollRequest, null); } public void testProxyRequestAuthenticationDeniedWithReadPrivileges() { @@ -1216,7 +1305,7 @@ public class AuthorizationServiceTests extends ESTestCase { String action = TransportActionProxy.getProxyAction(SearchTransportService.CLEAR_SCROLL_CONTEXTS_ACTION_NAME); assertThrowsAuthorizationException( () -> authorize(createAuthentication(user), action, transportRequest), action, "test user"); - verify(auditTrail).accessDenied(user, action, clearScrollRequest); + verify(auditTrail).accessDenied(user, action, clearScrollRequest, null); } private BiFunction getAccessControlResolver() { diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java index fb815144a24..4db787a2665 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authz/SecuritySearchOperationListenerTests.java @@ -116,7 +116,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { assertEquals(testSearchContext.id(), expected.id()); verify(licenseState, times(3)).isAuthAllowed(); verify(auditTrailService) - .accessDenied(authentication.getUser(), "action", request); + .accessDenied(authentication.getUser(), "action", request, null); } // another user running as the original user @@ -150,7 +150,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { assertEquals(testSearchContext.id(), expected.id()); verify(licenseState, times(5)).isAuthAllowed(); verify(auditTrailService) - .accessDenied(authentication.getUser(), "action", request); + .accessDenied(authentication.getUser(), "action", request, null); } } @@ -186,7 +186,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { SearchContextMissingException e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(original, differentRealmType, auditTrail, id, action, request)); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(differentRealmType.getUser(), action, request); + verify(auditTrail).accessDenied(differentRealmType.getUser(), action, request, null); // wrong user Authentication differentUser = @@ -194,7 +194,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(original, differentUser, auditTrail, id, action, request)); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(differentUser.getUser(), action, request); + verify(auditTrail).accessDenied(differentUser.getUser(), action, request, null); // run as different user Authentication diffRunAs = new Authentication(new User(new User("test2", "role"), new User("authenticated", "runas")), @@ -202,7 +202,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(original, diffRunAs, auditTrail, id, action, request)); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(diffRunAs.getUser(), action, request); + verify(auditTrail).accessDenied(diffRunAs.getUser(), action, request, null); // run as different looked up by type Authentication runAsDiffType = new Authentication(user, new RealmRef("realm", "file", "node"), @@ -210,7 +210,7 @@ public class SecuritySearchOperationListenerTests extends ESTestCase { e = expectThrows(SearchContextMissingException.class, () -> ensureAuthenticatedUserIsSame(runAs, runAsDiffType, auditTrail, id, action, request)); assertEquals(id, e.id()); - verify(auditTrail).accessDenied(runAsDiffType.getUser(), action, request); + verify(auditTrail).accessDenied(runAsDiffType.getUser(), action, request, null); } static class TestScrollSearchContext extends TestSearchContext { diff --git a/qa/sql-security/build.gradle b/qa/sql-security/build.gradle index b864c0bca20..627b3415ac5 100644 --- a/qa/sql-security/build.gradle +++ b/qa/sql-security/build.gradle @@ -15,6 +15,15 @@ integTestCluster { plugin ':x-pack-elasticsearch:plugin' setting 'xpack.ml.enabled', 'false' setting 'xpack.monitoring.enabled', 'false' + // Enabled audit logging so we can test it + setting 'xpack.security.audit.enabled', 'true' + setting 'xpack.security.audit.outputs', 'index' + // Only log the events we need so we don't have as much to sort through + setting 'xpack.security.audit.index.events.include', '[access_denied, access_granted]' + // Try and speed up audit logging without overwelming it + setting 'xpack.security.audit.index.flush_interval', '250ms' + setting 'xpack.security.audit.index.settings.index.number_of_shards', '1' + setting 'xpack.security.audit.index.settings.index.refresh_interval', '250ms' extraConfigFile 'x-pack/roles.yml', 'roles.yml' setupCommand 'setupUser#test_admin', 'bin/x-pack/users', 'useradd', 'test_admin', '-p', 'x-pack-test-password', '-r', 'superuser' @@ -35,6 +44,15 @@ task run(type: RunTask) { plugin ':x-pack-elasticsearch:plugin' setting 'xpack.ml.enabled', 'false' setting 'xpack.monitoring.enabled', 'false' + // Enabled audit logging so we can test it + setting 'xpack.security.audit.enabled', 'true' + setting 'xpack.security.audit.outputs', 'index' + // Only log the events we need so we don't have as much to sort through + setting 'xpack.security.audit.index.events.include', '[access_denied, access_granted]' + // Try and speed up the logging process without overwelming it + setting 'xpack.security.audit.index.flush_interval', '250ms' + setting 'xpack.security.audit.index.settings.index.number_of_shards', '1' + setting 'xpack.security.audit.index.settings.index.refresh_interval', '250ms' extraConfigFile 'x-pack/roles.yml', 'roles.yml' setupCommand 'setupUser#test_admin', 'bin/x-pack/users', 'useradd', 'test_admin', '-p', 'x-pack-test-password', '-r', 'superuser' diff --git a/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java b/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java index 4f0390bbe95..6341dc605f8 100644 --- a/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java +++ b/qa/sql-security/src/test/java/org/elasticsearch/xpack/sql/SqlSecurityIT.java @@ -11,6 +11,7 @@ import org.elasticsearch.client.http.Header; import org.elasticsearch.client.http.entity.ContentType; import org.elasticsearch.client.http.entity.StringEntity; import org.elasticsearch.client.http.message.BasicHeader; +import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; @@ -20,21 +21,29 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.NotEqualMessageBuilder; import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.xpack.sql.plugin.sql.action.SqlAction; import org.hamcrest.Matcher; +import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; import java.io.IOException; import java.io.InputStream; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.elasticsearch.xpack.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; import static org.hamcrest.Matchers.containsString; public class SqlSecurityIT extends ESRestTestCase { + private static boolean createdTestData = false; + private static boolean auditFailure = false; + /** * All tests run as a an administrative user but use * es-security-runas-user to become a less privileged user when needed. @@ -47,8 +56,21 @@ public class SqlSecurityIT extends ESRestTestCase { .build(); } + @Override + protected boolean preserveIndicesUponCompletion() { + /* We can't wipe the cluster between tests because that nukes the audit + * trail index which makes the auditing flaky. Instead we wipe all + * indices after the entire class is finished. */ + return true; + } + @Before public void createTestData() throws IOException { + if (createdTestData) { + /* Since we don't wipe the cluster between tests we only need to + * write the test data once. */ + return; + } StringBuilder bulk = new StringBuilder(); bulk.append("{\"index\":{\"_id\":\"1\"}\n"); bulk.append("{\"a\": 1, \"b\": 2, \"c\": 3}\n"); @@ -58,12 +80,35 @@ public class SqlSecurityIT extends ESRestTestCase { new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON)); } + @After + public void clearAuditLog() throws Exception { + try { + clearAuditEvents(); + } catch (ResponseException e) { + // 404 here just means we had no indexes + if (e.getResponse().getStatusLine().getStatusCode() != 404) { + throw e; + } + } + } + + @AfterClass + public static void wipeIndicesAfterTests() throws IOException { + try { + adminClient().performRequest("DELETE", "*"); + } catch (ResponseException e) { + // 404 here just means we had no indexes + if (e.getResponse().getStatusLine().getStatusCode() != 404) { + throw e; + } + } + } + // NOCOMMIT we're going to need to test jdbc and cli with these too! // NOCOMMIT we'll have to test scrolling as well // NOCOMMIT tests for describing a table and showing tables - // NOCOMMIT tests with audit trail - public void testSqlWorksAsAdmin() throws IOException { + public void testSqlWorksAsAdmin() throws Exception { Map expected = new HashMap<>(); Map columns = new HashMap<>(); columns.put("a", singletonMap("type", "long")); @@ -81,46 +126,84 @@ public class SqlSecurityIT extends ESRestTestCase { expected.put("rows", Arrays.asList(row1, row2)); expected.put("size", 2); assertResponse(expected, runSql("SELECT * FROM test ORDER BY a", null)); + assertAuditForSqlGranted("test_admin", "test"); } - public void testSqlWithFullAccess() throws IOException { + public void testSqlWithFullAccess() throws Exception { createUser("full_access", "read_test"); assertResponse(runSql("SELECT * FROM test ORDER BY a", null), runSql("SELECT * FROM test ORDER BY a", "full_access")); + assertAuditForSqlGranted("test_admin", "test"); + assertAuditForSqlGranted("full_access", "test"); } - public void testSqlNoAccess() throws IOException { + public void testSqlNoAccess() throws Exception { createUser("no_access", "read_nothing"); ResponseException e = expectThrows(ResponseException.class, () -> runSql("SELECT * FROM test", "no_access")); assertThat(e.getMessage(), containsString("403 Forbidden")); + assertAuditEvents(m -> "access_denied".equals(m.get("event_type")) + && m.get("indices") == null + && "no_access".equals(m.get("principal"))); } - public void testSqlWrongAccess() throws IOException { + public void testSqlWrongAccess() throws Exception { createUser("wrong_access", "read_something_else"); - ResponseException e = expectThrows(ResponseException.class, () -> runSql("SELECT * FROM test", "no_access")); + ResponseException e = expectThrows(ResponseException.class, () -> runSql("SELECT * FROM test", "wrong_access")); assertThat(e.getMessage(), containsString("403 Forbidden")); + assertAuditEvents( + /* This user has permission to run sql queries so they are + * given preliminary authorization. */ + m -> "access_granted".equals(m.get("event_type")) + && null == m.get("indices") + && "wrong_access".equals(m.get("principal")), + /* But as soon as they attempt to resolve an index that + * they don't have access to they get denied. */ + m -> "access_denied".equals(m.get("event_type")) + && singletonList("test").equals(m.get("indices")) + && "wrong_access".equals(m.get("principal"))); } - public void testSqlSingleFieldGranted() throws IOException { + public void testSqlSingleFieldGranted() throws Exception { createUser("only_a", "read_test_a"); assertResponse(runSql("SELECT a FROM test", null), runSql("SELECT * FROM test", "only_a")); + assertAuditForSqlGranted("test_admin", "test"); + assertAuditForSqlGranted("only_a", "test"); + clearAuditEvents(); expectBadRequest(() -> runSql("SELECT c FROM test", "only_a"), containsString("line 1:8: Unresolved item 'c'")); + /* The user has permission to query the index but one of the + * columns that they explicitly mention is hidden from them + * by field level access control. This *looks* like a successful + * query from the audit side because all the permissions checked + * out but it failed in SQL because it couldn't compile the + * query without the metadata for the missing field. */ + assertAuditForSqlGranted("only_a", "test"); } - public void testSqlSingleFieldExcepted() throws IOException { + public void testSqlSingleFieldExcepted() throws Exception { createUser("not_c", "read_test_a_and_b"); assertResponse(runSql("SELECT a, b FROM test", null), runSql("SELECT * FROM test", "not_c")); + assertAuditForSqlGranted("test_admin", "test"); + assertAuditForSqlGranted("not_c", "test"); + clearAuditEvents(); expectBadRequest(() -> runSql("SELECT c FROM test", "not_c"), containsString("line 1:8: Unresolved item 'c'")); + /* The user has permission to query the index but one of the + * columns that they explicitly mention is hidden from them + * by field level access control. This *looks* like a successful + * query from the audit side because all the permissions checked + * out but it failed in SQL because it couldn't compile the + * query without the metadata for the missing field. */ + assertAuditForSqlGranted("not_c", "test"); } - public void testSqlDocumentExclued() throws IOException { + public void testSqlDocumentExclued() throws Exception { createUser("no_3s", "read_test_without_c_3"); assertResponse(runSql("SELECT * FROM test WHERE c != 3", null), runSql("SELECT * FROM test", "no_3s")); + assertAuditForSqlGranted("no_3s", "test"); } private void expectBadRequest(ThrowingRunnable code, Matcher errorMessageMatcher) { @@ -142,6 +225,10 @@ public class SqlSecurityIT extends ESRestTestCase { Response response = client().performRequest("POST", "/_sql", emptyMap(), new StringEntity("{\"query\": \"" + sql + "\"}", ContentType.APPLICATION_JSON), headers); + return toMap(response); + } + + private Map toMap(Response response) throws IOException { try (InputStream content = response.getEntity().getContent()) { return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false); } @@ -156,4 +243,77 @@ public class SqlSecurityIT extends ESRestTestCase { client().performRequest("PUT", "/_xpack/security/user/" + name, emptyMap(), new StringEntity(user.string(), ContentType.APPLICATION_JSON)); } + + private void assertAuditForSqlGranted(String user, String index) throws Exception { + assertAuditEvents( + m -> "access_granted".equals(m.get("event_type")) + && m.get("indices") == null + && user.equals(m.get("principal")), + m -> "access_granted".equals(m.get("event_type")) + && singletonList(index).equals(m.get("indices")) + && user.equals(m.get("principal"))); + } + + /** + * Asserts that audit events have been logged that match all the provided checkers. + */ + @SafeVarargs + private final void assertAuditEvents(CheckedFunction, Boolean, Exception>... eventCheckers) throws Exception { + assumeFalse("Previous test had an audit-related failure. All subsequent audit related assertions are bogus because we can't " + + "guarantee that we fully cleaned up after the last test.", auditFailure); + try { + assertBusy(() -> { + XContentBuilder search = JsonXContent.contentBuilder().prettyPrint(); + search.startObject(); { + search.array("_source", "@timestamp", "indices", "principal", "event_type"); + search.startObject("query"); { + search.startObject("match").field("action", SqlAction.NAME).endObject(); + } + search.endObject(); + } + search.endObject(); + Map audit; + try { + audit = toMap(client().performRequest("POST", "/.security_audit_log-*/_search", + singletonMap("filter_path", "hits.hits._source"), new StringEntity(search.string(), ContentType.APPLICATION_JSON))); + } catch (ResponseException e) { + throw new AssertionError("ES failed to respond. Wrapping in assertion so we retry. Hopefully this is transient.", e); + } + Map hitsOuter = (Map) audit.get("hits"); + assertNotNull("expected some hits", hitsOuter); + List hits = (List) hitsOuter.get("hits"); + verifier: for (CheckedFunction, Boolean, Exception> eventChecker : eventCheckers) { + for (Object hit : hits) { + Map source = (Map)((Map) hit).get("_source"); + if (eventChecker.apply(source)) { + continue verifier; + } + } + fail("didn't find audit event we were looking for. found [" + hits + "]"); + } + }); + } catch (AssertionError e) { + auditFailure = true; + throw e; + } + } + + private void clearAuditEvents() throws Exception { + try { + assertBusy(() -> { + try { + adminClient().performRequest("POST", "/.security_audit_log-*/_delete_by_query", emptyMap(), + new StringEntity("{\"query\":{\"match_all\":{}}}", ContentType.APPLICATION_JSON)); + } catch (ResponseException e) { + if (e.getResponse().getStatusLine().getStatusCode() == 409) { + throw new AssertionError("Conflict while clearing audit log. Wrapping in assertion so we retry.", e); + } + throw e; + } + }); + } catch (AssertionError e) { + auditFailure = true; + throw e; + } + } }