Remove implicit index monitor privilege

docs/reference/migration/migrate_7_0/api.asciidoc

@@ -126,3 +126,15 @@ removed.
 
 The `_termvector` endpoint was deprecated in 2.0 and has now been removed.
 The endpoint `_termvectors` (plural) should be used instead.
+
+[float]
+==== When {security-features} are enabled, index monitoring APIs over restricted indices are not authorized implicitly anymore
+
+Restricted indices (currently only `.security-6` and `.security`) are special internal
+indices that require setting the `allow_restricted_indices` flag on every index
+permission that covers them. If this flag is `false` (default) the permission
+will not cover these and actions against them will not be authorized.
+However, the monitoring APIs were the only exception to this rule. This exception
+has been forfeited and index monitoring privileges have to be granted explicitly,
+using the `allow_restricted_indices` flag on the permission (as any other index
+privilege).

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java

@@ -259,12 +259,8 @@ private boolean check(String action) {
 
         private boolean check(String action, String index) {
             assert index != null;
-            return check(action) && (indexNameMatcher.test(index)
-                    && (allowRestrictedIndices
-                            // all good if it is not restricted
-                            || (false == RestrictedIndicesNames.NAMES_SET.contains(index))
-                            // allow monitor as a special case, even for restricted
-                            || IndexPrivilege.MONITOR.predicate().test(action)));
+            return check(action) && indexNameMatcher.test(index)
+                    && (allowRestrictedIndices || (false == RestrictedIndicesNames.NAMES_SET.contains(index)));
         }
 
         boolean hasQuery() {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java

@@ -144,6 +144,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
 import static org.hamcrest.Matchers.hasEntry;
@@ -563,9 +564,33 @@ public void testRemoteMonitoringCollectorRole() {
         assertThat(remoteMonitoringAgentRole.indices().allowedIndicesMatcher(IndexAction.NAME)
                 .test(randomFrom(RestrictedIndicesNames.INTERNAL_SECURITY_INDEX, RestrictedIndicesNames.SECURITY_INDEX_NAME)), is(false));
 
+        assertMonitoringOnRestrictedIndices(remoteMonitoringAgentRole);
+
         assertNoAccessAllowed(remoteMonitoringAgentRole, RestrictedIndicesNames.NAMES_SET);
     }
 
+    private void assertMonitoringOnRestrictedIndices(Role role) {
+        final Settings indexSettings = Settings.builder().put("index.version.created", Version.CURRENT).build();
+        final MetaData metaData = new MetaData.Builder()
+                .put(new IndexMetaData.Builder(RestrictedIndicesNames.INTERNAL_SECURITY_INDEX)
+                        .settings(indexSettings)
+                        .numberOfShards(1)
+                        .numberOfReplicas(0)
+                        .putAlias(new AliasMetaData.Builder(RestrictedIndicesNames.SECURITY_INDEX_NAME).build())
+                        .build(), true)
+                .build();
+        final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY);
+        final List<String> indexMonitoringActionNamesList = Arrays.asList(IndicesStatsAction.NAME, IndicesSegmentsAction.NAME,
+                GetSettingsAction.NAME, IndicesShardStoresAction.NAME, UpgradeStatusAction.NAME, RecoveryAction.NAME);
+        for (final String indexMonitoringActionName : indexMonitoringActionNamesList) {
+            final Map<String, IndexAccessControl> authzMap = role.indices().authorize(indexMonitoringActionName,
+                    Sets.newHashSet(RestrictedIndicesNames.INTERNAL_SECURITY_INDEX, RestrictedIndicesNames.SECURITY_INDEX_NAME), metaData,
+                    fieldPermissionsCache);
+            assertThat(authzMap.get(RestrictedIndicesNames.INTERNAL_SECURITY_INDEX).isGranted(), is(true));
+            assertThat(authzMap.get(RestrictedIndicesNames.SECURITY_INDEX_NAME).isGranted(), is(true));
+        }
+    }
+
     public void testReportingUserRole() {
         final TransportRequest request = mock(TransportRequest.class);
 

x-pack/plugin/security/src/test/java/org/elasticsearch/integration/MultipleIndicesPermissionsTests.java

@@ -7,14 +7,27 @@
 
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.action.DocWriteResponse;
+import org.elasticsearch.action.admin.indices.recovery.RecoveryResponse;
+import org.elasticsearch.action.admin.indices.segments.IndicesSegmentResponse;
+import org.elasticsearch.action.admin.indices.settings.get.GetSettingsResponse;
+import org.elasticsearch.action.admin.indices.shards.IndicesShardStoresResponse;
+import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse;
+import org.elasticsearch.action.admin.indices.upgrade.get.UpgradeStatusResponse;
 import org.elasticsearch.action.index.IndexResponse;
 import org.elasticsearch.action.search.MultiSearchResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.client.Client;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.RequestOptions;
+import org.elasticsearch.client.Response;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.SecurityIntegTestCase;
 import org.elasticsearch.test.SecuritySettingsSource;
+import org.elasticsearch.xpack.core.security.authc.support.Hasher;
+import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
+import org.junit.After;
+import org.junit.Before;
 
 import java.util.Collections;
 
@@ -25,11 +38,29 @@
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 
 public class MultipleIndicesPermissionsTests extends SecurityIntegTestCase {
 
     protected static final SecureString USERS_PASSWD = new SecureString("passwd".toCharArray());
 
+    @Before
+    public void waitForSecurityIndexWritable() throws Exception {
+        // adds a dummy user to the native realm to force .security index creation
+        securityClient().preparePutUser("dummy_user", "password".toCharArray(), Hasher.BCRYPT, "missing_role").get();
+        assertSecurityIndexActive();
+    }
+
+    @After
+    public void cleanupSecurityIndex() throws Exception {
+        super.deleteSecurityIndex();
+    }
+
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected String configRoles() {
         return SecuritySettingsSource.TEST_ROLE + ":\n" +
@@ -49,6 +80,12 @@ protected String configRoles() {
                 "    - names: 'a'\n" +
                 "      privileges: [all]\n" +
                 "\n" +
+                "role_monitor_all_unrestricted_indices:\n" +
+                "  cluster: [monitor]\n" +
+                "  indices:\n" +
+                "    - names: '*'\n" +
+                "      privileges: [monitor]\n" +
+                "\n" +
                 "role_b:\n" +
                 "  indices:\n" +
                 "    - names: 'b'\n" +
@@ -60,14 +97,16 @@ protected String configUsers() {
         final String usersPasswdHashed = new String(getFastStoredHashAlgoForTests().hash(USERS_PASSWD));
         return SecuritySettingsSource.CONFIG_STANDARD_USER +
             "user_a:" + usersPasswdHashed + "\n" +
-            "user_ab:" + usersPasswdHashed + "\n";
+            "user_ab:" + usersPasswdHashed + "\n" +
+            "user_monitor:" + usersPasswdHashed + "\n";
     }
 
     @Override
     protected String configUsersRoles() {
         return SecuritySettingsSource.CONFIG_STANDARD_USER_ROLES +
                 "role_a:user_a,user_ab\n" +
-                "role_b:user_ab\n";
+                "role_b:user_ab\n" +
+                "role_monitor_all_unrestricted_indices:user_monitor\n";
     }
 
     public void testSingleRole() throws Exception {
@@ -127,6 +166,71 @@ public void testSingleRole() throws Exception {
         assertHitCount(searchResponse, 1);
     }
 
+    public void testMonitorRestrictedWildcards() throws Exception {
+
+        IndexResponse indexResponse = index("foo", "type", jsonBuilder()
+                .startObject()
+                .field("name", "value")
+                .endObject());
+        assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
+
+        indexResponse = index("foobar", "type", jsonBuilder()
+                .startObject()
+                .field("name", "value")
+                .endObject());
+        assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
+
+        indexResponse = index("foobarfoo", "type", jsonBuilder()
+                .startObject()
+                .field("name", "value")
+                .endObject());
+        assertEquals(DocWriteResponse.Result.CREATED, indexResponse.getResult());
+
+        refresh();
+
+        final Client client = client()
+                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user_monitor", USERS_PASSWD)));
+
+        final GetSettingsResponse getSettingsResponse = client.admin().indices().prepareGetSettings(randomFrom("*", "_all", "foo*")).get();
+        assertThat(getSettingsResponse.getIndexToSettings().size(), is(3));
+        assertThat(getSettingsResponse.getIndexToSettings().containsKey("foo"), is(true));
+        assertThat(getSettingsResponse.getIndexToSettings().containsKey("foobar"), is(true));
+        assertThat(getSettingsResponse.getIndexToSettings().containsKey("foobarfoo"), is(true));
+
+        final IndicesShardStoresResponse indicesShardsStoresResponse = client.admin().indices()
+                .prepareShardStores(randomFrom("*", "_all", "foo*")).setShardStatuses("all").get();
+        assertThat(indicesShardsStoresResponse.getStoreStatuses().size(), is(3));
+        assertThat(indicesShardsStoresResponse.getStoreStatuses().containsKey("foo"), is(true));
+        assertThat(indicesShardsStoresResponse.getStoreStatuses().containsKey("foobar"), is(true));
+        assertThat(indicesShardsStoresResponse.getStoreStatuses().containsKey("foobarfoo"), is(true));
+
+        final UpgradeStatusResponse upgradeStatusResponse = client.admin().indices().prepareUpgradeStatus(randomFrom("*", "_all", "foo*"))
+                .get();
+        assertThat(upgradeStatusResponse.getIndices().size(), is(3));
+        assertThat(upgradeStatusResponse.getIndices().keySet(), containsInAnyOrder("foo", "foobar", "foobarfoo"));
+
+        final IndicesStatsResponse indicesStatsResponse = client.admin().indices().prepareStats(randomFrom("*", "_all", "foo*")).get();
+        assertThat(indicesStatsResponse.getIndices().size(), is(3));
+        assertThat(indicesStatsResponse.getIndices().keySet(), containsInAnyOrder("foo", "foobar", "foobarfoo"));
+
+        final IndicesSegmentResponse indicesSegmentResponse = client.admin().indices().prepareSegments("*").get();
+        assertThat(indicesSegmentResponse.getIndices().size(), is(3));
+        assertThat(indicesSegmentResponse.getIndices().keySet(), containsInAnyOrder("foo", "foobar", "foobarfoo"));
+
+        final RecoveryResponse indicesRecoveryResponse = client.admin().indices().prepareRecoveries("*").get();
+        assertThat(indicesRecoveryResponse.shardRecoveryStates().size(), is(3));
+        assertThat(indicesRecoveryResponse.shardRecoveryStates().keySet(), containsInAnyOrder("foo", "foobar", "foobarfoo"));
+
+        // test _cat/indices with wildcards that cover unauthorized indices (".security" in this case)  
+        RequestOptions.Builder optionsBuilder = RequestOptions.DEFAULT.toBuilder();
+        optionsBuilder.addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue("user_monitor", USERS_PASSWD));
+        RequestOptions options = optionsBuilder.build();
+        Request catIndicesRequest = new Request("GET", "/_cat/indices/" + randomFrom("*", "_all", "foo*"));
+        catIndicesRequest.setOptions(options);
+        Response catIndicesResponse = getRestClient().performRequest(catIndicesRequest);
+        assertThat(catIndicesResponse.getStatusLine().getStatusCode() < 300, is(true));
+    }
+
     public void testMultipleRoles() throws Exception {
         IndexResponse indexResponse = index("a", "type", jsonBuilder()
                 .startObject()

x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java

@@ -78,6 +78,7 @@ public class SecuritySettingsSource extends NodeConfigurationSource {
                     "  cluster: [ ALL ]\n" +
                     "  indices:\n" +
                     "    - names: '*'\n" +
+                    "      allow_restricted_indices: true\n" +
                     "      privileges: [ ALL ]\n";
 
     private final Path parentFolder;