system actions are only logged at the correct audit level

Today, some system actions could be logged by default when the actions performed are
not internal. Additionally for internal actions, we never checked if the user was the system
user.

This adds a check to ensure the user is the System user and that the actions that are being
suppressed are known system actions.

Closes elastic/elasticsearch#902

Original commit: elastic/x-pack-elasticsearch@b107994692

src/main/java/org/elasticsearch/shield/audit/index/IndexAuditTrail.java

@@ -193,7 +193,7 @@ public class IndexAuditTrail implements AuditTrail {
         if (enabled.contains(Level.ACCESS_GRANTED)) {
             if (!principalIsAuditor(user.principal())) {
                 // special treatment for internal system actions - only log if explicitly told to
-                if (Privilege.SYSTEM.internalActionPredicate().apply(action)) {
+                if (user.isSystem() && Privilege.SYSTEM.predicate().apply(action)) {
                     if (enabled.contains(Level.SYSTEM_ACCESS_GRANTED)) {
                         try {
                             processor.submit(message("access_granted", action, user.principal(), null, indices(message), message));

src/main/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrail.java

@@ -140,7 +140,7 @@ public class LoggingAuditTrail implements AuditTrail {
         String indices = indices(message);
 
         // special treatment for internal system actions - only log on trace
-        if (Privilege.SYSTEM.internalActionPredicate().apply(action)) {
+        if (user.isSystem() && Privilege.SYSTEM.predicate().apply(action)) {
             if (logger.isTraceEnabled()) {
                 if (indices != null) {
                     logger.trace("{}[transport] [access_granted]\t{}, principal=[{}], action=[{}], indices=[{}], request=[{}]", prefix, originAttributes(message), user.principal(), action, indices, message.getClass().getSimpleName());

src/main/java/org/elasticsearch/shield/authz/Privilege.java

@@ -103,10 +103,6 @@ public abstract class Privilege<P extends Privilege<P>> {
             return PREDICATE;
         }
 
-        public Predicate<String> internalActionPredicate() {
-            return INTERNAL_PREDICATE;
-        }
-
         @Override
         public boolean implies(System other) {
             return true;

src/test/java/org/elasticsearch/shield/audit/index/IndexAuditTrailTests.java

@@ -80,17 +80,19 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest {
                 .build();
     }
 
-    private Settings mutedSettings(String... muted) {
+    private Settings mutedSettings(boolean systemEnabled, String... muted) {
         Settings.Builder builder = Settings.builder();
         for (String mute : muted) {
             builder.put("shield.audit.index.events." + mute, false);
         }
+
+        builder.put("shield.audit.index.events.system.access_granted", systemEnabled);
         return builder.build();
     }
 
-    private Settings settings(IndexNameResolver.Rollover rollover, String... muted) {
+    private Settings settings(IndexNameResolver.Rollover rollover, boolean systemEnabled, String... muted) {
         Settings.Builder builder = Settings.builder();
-        builder.put(mutedSettings(muted));
+        builder.put(mutedSettings(systemEnabled, muted));
         builder.put(commonSettings(rollover));
         return builder.build();
     }
@@ -110,9 +112,12 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest {
     }
 
     private void initialize(String... muted) {
+        initialize(false, muted);
+    }
 
+    private void initialize(boolean systemEnabled, String... muted) {
         rollover = randomFrom(HOURLY, DAILY, WEEKLY, MONTHLY);
-        Settings settings = settings(rollover, muted);
+        Settings settings = settings(rollover, systemEnabled, muted);
         remoteIndexing = randomBoolean();
 
         if (remoteIndexing) {
@@ -336,6 +341,29 @@ public class IndexAuditTrailTests extends ShieldIntegrationTest {
         getClient().prepareExists(resolveIndexName()).execute().actionGet();
     }
 
+    @Test
+    public void testSystemAccessGranted() throws Exception {
+        initialize(true);
+        TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage();
+        auditor.accessGranted(User.SYSTEM, "internal:_action", message);
+        awaitIndexCreation(resolveIndexName());
+
+        SearchHit hit = getIndexedAuditMessage();
+        assertAuditMessage(hit, "transport", "access_granted");
+        assertEquals("transport", hit.field("origin_type").getValue());
+        assertEquals(User.SYSTEM.principal(), hit.field("principal").getValue());
+        assertEquals("internal:_action", hit.field("action").getValue());
+    }
+
+    @Test(expected = IndexMissingException.class)
+    public void testSystemAccessGranted_Muted() throws Exception {
+        initialize();
+        TransportMessage message = randomBoolean() ? new RemoteHostMockMessage() : new LocalHostMockMessage();
+        auditor.accessGranted(User.SYSTEM, "internal:_action", message);
+        getClient().prepareExists(resolveIndexName()).execute().actionGet();
+        awaitIndexCreation(resolveIndexName());
+    }
+
     @Test
     public void testAccessDenied() throws Exception {
 

src/test/java/org/elasticsearch/shield/audit/logfile/LoggingAuditTrailTests.java

@@ -307,7 +307,7 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
             LoggingAuditTrail auditTrail = new LoggingAuditTrail(settings, logger);
             TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
             String origins = LoggingAuditTrail.originAttributes(message);
-            auditTrail.accessGranted(new User.Simple("_username", "r1"), "internal:_action", message);
+            auditTrail.accessGranted(User.SYSTEM, "internal:_action", message);
             switch (level) {
                 case ERROR:
                 case WARN:
@@ -317,9 +317,40 @@ public class LoggingAuditTrailTests extends ElasticsearchTestCase {
                     break;
                 case TRACE:
                     if (message instanceof IndicesRequest) {
-                        assertMsg(logger, Level.TRACE, prefix + "[transport] [access_granted]\t" + origins + ", principal=[_username], action=[internal:_action], indices=[idx1,idx2], request=[MockIndicesRequest]");
+                        assertMsg(logger, Level.TRACE, prefix + "[transport] [access_granted]\t" + origins + ", principal=[" + User.SYSTEM.principal() + "], action=[internal:_action], indices=[idx1,idx2], request=[MockIndicesRequest]");
                     } else {
-                        assertMsg(logger, Level.TRACE, prefix + "[transport] [access_granted]\t" + origins + ", principal=[_username], action=[internal:_action], request=[MockMessage]");
+                        assertMsg(logger, Level.TRACE, prefix + "[transport] [access_granted]\t" + origins + ", principal=[" + User.SYSTEM.principal() + "], action=[internal:_action], request=[MockMessage]");
+                    }
+            }
+        }
+    }
+
+    @Test
+    public void testAccessGranted_InternalSystemAction_NonSystemUser() throws Exception {
+        for (Level level : Level.values()) {
+            CapturingLogger logger = new CapturingLogger(level);
+            LoggingAuditTrail auditTrail = new LoggingAuditTrail(settings, logger);
+            TransportMessage message = randomBoolean() ? new MockMessage() : new MockIndicesRequest();
+            String origins = LoggingAuditTrail.originAttributes(message);
+            auditTrail.accessGranted(new User.Simple("_username"), "internal:_action", message);
+            switch (level) {
+                case ERROR:
+                case WARN:
+                    assertEmptyLog(logger);
+                    break;
+                case INFO:
+                    if (message instanceof IndicesRequest) {
+                        assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", principal=[_username], action=[internal:_action], indices=[idx1,idx2]");
+                    } else {
+                        assertMsg(logger, Level.INFO, prefix + "[transport] [access_granted]\t" + origins + ", principal=[_username], action=[internal:_action]");
+                    }
+                    break;
+                case DEBUG:
+                case TRACE:
+                    if (message instanceof IndicesRequest) {
+                        assertMsg(logger, Level.DEBUG, prefix + "[transport] [access_granted]\t" + origins + ", principal=[_username], action=[internal:_action], indices=[idx1,idx2], request=[MockIndicesRequest]");
+                    } else {
+                        assertMsg(logger, Level.DEBUG, prefix + "[transport] [access_granted]\t" + origins + ", principal=[_username], action=[internal:_action], request=[MockMessage]");
                     }
             }
         }