Handle a edge case for validation of API key role descriptors (#76959) (#76963)

ywangd · web-flow · commit 102cfe77dae7 · 2021-08-26T02:05:08.000-04:00
* Handle a edge case for validation of API key role descriptors (#76959) This PR fixes a BWC edge case: In a mixed cluster, e.g. rolling upgrade, API keys can sometimes fail to validate due to mismatch of role descriptors depending on where the request is initially authenticated. * checkstyle
diff --git a/x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java b/x-pack/plugin/security/qa/service-account/src/javaRestTest/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountIT.java
@@ -26,7 +26,9 @@
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
+import java.util.Base64;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -381,10 +383,30 @@ public void testManageOwnApiKey() throws IOException {
         createApiKeyRequest1.setOptions(requestOptions);
         final Response createApiKeyResponse1 = client().performRequest(createApiKeyRequest1);
         assertOK(createApiKeyResponse1);
-        final String apiKeyId1 = (String) responseAsMap(createApiKeyResponse1).get("id");
+        final Map<String, Object> createApiKeyResponseMap1 = responseAsMap(createApiKeyResponse1);
+        final String apiKeyId1 = (String) createApiKeyResponseMap1.get("id");
 
         assertApiKeys(apiKeyId1, "key-1", false, requestOptions);
 
+        final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString(
+            (apiKeyId1 + ":" + createApiKeyResponseMap1.get("api_key")).getBytes(StandardCharsets.UTF_8));
+
+        // API key can monitor cluster
+        final Request mainRequest = new Request("GET", "/");
+        mainRequest.setOptions(mainRequest.getOptions().toBuilder().addHeader(
+            "Authorization", "ApiKey " + base64ApiKeyKeyValue
+        ));
+        assertOK(client().performRequest(mainRequest));
+
+        // API key cannot get user
+        final Request getUserRequest = new Request("GET", "_security/user");
+        getUserRequest.setOptions(getUserRequest.getOptions().toBuilder().addHeader(
+            "Authorization", "ApiKey " + base64ApiKeyKeyValue
+        ));
+        final ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(getUserRequest));
+        assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(403));
+        assertThat(e.getMessage(), containsString("is unauthorized for API key"));
+
         final Request invalidateApiKeysRequest = new Request("DELETE", "_security/api_key");
         invalidateApiKeysRequest.setJsonEntity("{\"ids\":[\"" + apiKeyId1 + "\"],\"owner\":true}");
         invalidateApiKeysRequest.setOptions(requestOptions);
diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java
@@ -0,0 +1,138 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security.authc.apikey;
+
+import org.elasticsearch.ElasticsearchSecurityException;
+import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
+import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
+import org.elasticsearch.action.get.GetAction;
+import org.elasticsearch.action.get.GetRequest;
+import org.elasticsearch.action.get.GetResponse;
+import org.elasticsearch.action.main.MainAction;
+import org.elasticsearch.action.main.MainRequest;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.test.SecuritySingleNodeTestCase;
+import org.elasticsearch.test.XContentTestUtils;
+import org.elasticsearch.xpack.core.XPackSettings;
+import org.elasticsearch.xpack.core.security.action.CreateApiKeyAction;
+import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
+import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
+import org.elasticsearch.xpack.core.security.action.GrantApiKeyAction;
+import org.elasticsearch.xpack.core.security.action.GrantApiKeyRequest;
+import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenAction;
+import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenRequest;
+import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenResponse;
+import org.elasticsearch.xpack.core.security.action.user.PutUserAction;
+import org.elasticsearch.xpack.core.security.action.user.PutUserRequest;
+import org.elasticsearch.xpack.core.security.authc.support.Hasher;
+import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
+import org.elasticsearch.xpack.security.authc.service.ServiceAccountService;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.Map;
+
+import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
+
+public class ApiKeySingleNodeTests extends SecuritySingleNodeTestCase {
+
+    @Override
+    protected Settings nodeSettings() {
+        Settings.Builder builder = Settings.builder().put(super.nodeSettings());
+        builder.put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true);
+        return builder.build();
+    }
+
+    public void testCreatingApiKeyWithNoAccess() {
+        final PutUserRequest putUserRequest = new PutUserRequest();
+        final String username = randomAlphaOfLength(8);
+        putUserRequest.username(username);
+        final SecureString password = new SecureString("super-strong-password".toCharArray());
+        putUserRequest.passwordHash(Hasher.PBKDF2.hash(password));
+        putUserRequest.roles(Strings.EMPTY_ARRAY);
+        client().execute(PutUserAction.INSTANCE, putUserRequest).actionGet();
+
+        final GrantApiKeyRequest grantApiKeyRequest = new GrantApiKeyRequest();
+        grantApiKeyRequest.getGrant().setType("password");
+        grantApiKeyRequest.getGrant().setUsername(username);
+        grantApiKeyRequest.getGrant().setPassword(password);
+        grantApiKeyRequest.getApiKeyRequest().setName(randomAlphaOfLength(8));
+        grantApiKeyRequest.getApiKeyRequest().setRoleDescriptors(org.elasticsearch.core.List.of(
+            new RoleDescriptor("x", new String[] { "all" },
+                new RoleDescriptor.IndicesPrivileges[]{
+                    RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(true).build()
+                },
+                null, null, null, null, null)));
+        final CreateApiKeyResponse createApiKeyResponse = client().execute(GrantApiKeyAction.INSTANCE, grantApiKeyRequest).actionGet();
+
+        final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString(
+            (createApiKeyResponse.getId() + ":" + createApiKeyResponse.getKey().toString()).getBytes(StandardCharsets.UTF_8));
+
+        // No cluster access
+        final ElasticsearchSecurityException e1 = expectThrows(
+            ElasticsearchSecurityException.class,
+            () -> client().filterWithHeader(org.elasticsearch.core.Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue))
+                .execute(MainAction.INSTANCE, new MainRequest())
+                .actionGet());
+        assertThat(e1.status().getStatus(), equalTo(403));
+        assertThat(e1.getMessage(), containsString("is unauthorized for API key"));
+
+        // No index access
+        final ElasticsearchSecurityException e2 = expectThrows(
+            ElasticsearchSecurityException.class,
+            () -> client().filterWithHeader(org.elasticsearch.core.Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue))
+                .execute(CreateIndexAction.INSTANCE, new CreateIndexRequest(
+                    randomFrom(randomAlphaOfLengthBetween(3, 8), SECURITY_MAIN_ALIAS)))
+                .actionGet());
+        assertThat(e2.status().getStatus(), equalTo(403));
+        assertThat(e2.getMessage(), containsString("is unauthorized for API key"));
+    }
+
+    public void testServiceAccountApiKey() throws IOException {
+        final CreateServiceAccountTokenRequest createServiceAccountTokenRequest =
+            new CreateServiceAccountTokenRequest("elastic", "fleet-server", randomAlphaOfLength(8));
+        final CreateServiceAccountTokenResponse createServiceAccountTokenResponse =
+            client().execute(CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
+
+        final CreateApiKeyResponse createApiKeyResponse =
+            client()
+                .filterWithHeader(org.elasticsearch.core.Map.of("Authorization", "Bearer " + createServiceAccountTokenResponse.getValue()))
+                .execute(CreateApiKeyAction.INSTANCE, new CreateApiKeyRequest(randomAlphaOfLength(8), null, null))
+                .actionGet();
+
+        final Map<String, Object> apiKeyDocument = getApiKeyDocument(createApiKeyResponse.getId());
+
+        @SuppressWarnings("unchecked")
+        final Map<String, Object> fleetServerRoleDescriptor =
+            (Map<String, Object>) apiKeyDocument.get("limited_by_role_descriptors");
+        assertThat(fleetServerRoleDescriptor.size(), equalTo(1));
+        assertThat(fleetServerRoleDescriptor, hasKey("elastic/fleet-server"));
+
+        @SuppressWarnings("unchecked")
+        final Map<String, ?> descriptor = (Map<String, ?>) fleetServerRoleDescriptor.get("elastic/fleet-server");
+
+        final RoleDescriptor roleDescriptor = RoleDescriptor.parse("elastic/fleet-server",
+            XContentTestUtils.convertToXContent(descriptor, XContentType.JSON),
+            false,
+            XContentType.JSON);
+        assertThat(roleDescriptor, equalTo(ServiceAccountService.getServiceAccounts().get("elastic/fleet-server").roleDescriptor()));
+    }
+
+    private Map<String, Object> getApiKeyDocument(String apiKeyId) {
+        final GetResponse getResponse =
+            client().execute(GetAction.INSTANCE, new GetRequest(".security-7", apiKeyId)).actionGet();
+        return getResponse.getSource();
+    }
+}
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
@@ -88,6 +88,7 @@
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
+import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.user.User;
@@ -182,6 +183,18 @@ public class ApiKeyService {
     public static final Setting<TimeValue> DOC_CACHE_TTL_SETTING = Setting.timeSetting("xpack.security.authc.api_key.doc_cache.ttl",
         TimeValue.timeValueMinutes(5), TimeValue.timeValueMinutes(0), TimeValue.timeValueMinutes(15), Property.NodeScope);
 
+    // This following fixed role descriptor is for fleet-server BWC on and before 7.14.
+    // It is fixed and must NOT be updated when the fleet-server service account updates.
+    private static final BytesArray FLEET_SERVER_ROLE_DESCRIPTOR_BYTES_V_7_14 = new BytesArray(
+        "{\"elastic/fleet-server\":{\"cluster\":[\"monitor\",\"manage_own_api_key\"]," +
+            "\"indices\":[{\"names\":[\"logs-*\",\"metrics-*\",\"traces-*\",\"synthetics-*\"," +
+            "\".logs-endpoint.diagnostic.collection-*\"]," +
+            "\"privileges\":[\"write\",\"create_index\",\"auto_configure\"],\"allow_restricted_indices\":false}," +
+            "{\"names\":[\".fleet-*\"],\"privileges\":[\"read\",\"write\",\"monitor\",\"create_index\",\"auto_configure\"]," +
+            "\"allow_restricted_indices\":false}],\"applications\":[],\"run_as\":[],\"metadata\":{}," +
+            "\"transient_metadata\":{\"enabled\":true}}}"
+    );
+
     private final Clock clock;
     private final Client client;
     private final XPackLicenseState licenseState;
@@ -536,9 +549,15 @@ public Tuple<String, BytesReference> getApiKeyIdAndRoleBytes(Authentication auth
             .onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES) : "This method only applies to authentication objects created on or after v7.9.0";
 
         final Map<String, Object> metadata = authentication.getMetadata();
-        return new Tuple<>(
-            (String) metadata.get(API_KEY_ID_KEY),
-            (BytesReference) metadata.get(limitedBy ? API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY : API_KEY_ROLE_DESCRIPTORS_KEY));
+        final BytesReference bytesReference =
+            (BytesReference) metadata.get(limitedBy ? API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY : API_KEY_ROLE_DESCRIPTORS_KEY);
+        if (limitedBy && bytesReference.length() == 2 && "{}".equals(bytesReference.utf8ToString())) {
+            if (ServiceAccountSettings.REALM_NAME.equals(metadata.get(API_KEY_CREATOR_REALM_NAME))
+                && "elastic/fleet-server".equals(authentication.getUser().principal())) {
+                return new Tuple<>((String) metadata.get(API_KEY_ID_KEY), FLEET_SERVER_ROLE_DESCRIPTOR_BYTES_V_7_14);
+            }
+        }
+        return new Tuple<>((String) metadata.get(API_KEY_ID_KEY), bytesReference);
     }
 
     public static class ApiKeyRoleDescriptors {
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccounts.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccounts.java
@@ -45,7 +45,7 @@ final class ElasticServiceAccounts {
         ));
 
     static final Map<String, ServiceAccount> ACCOUNTS = List.of(FLEET_ACCOUNT).stream()
-        .collect(Collectors.toMap(a -> a.id().asPrincipal(), Function.identity()));;
+        .collect(Collectors.toMap(a -> a.id().asPrincipal(), Function.identity()));
 
     private ElasticServiceAccounts() {}
 
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/ApiKeyGenerator.java
@@ -14,13 +14,17 @@
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
 import org.elasticsearch.xpack.core.security.action.CreateApiKeyResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
+import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
 import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator;
 import org.elasticsearch.xpack.security.authc.ApiKeyService;
+import org.elasticsearch.xpack.security.authc.service.ServiceAccount;
+import org.elasticsearch.xpack.security.authc.service.ServiceAccountService;
 import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
 
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.Set;
 
 /**
  * Utility class for generating API keys for a provided {@link Authentication}.
@@ -48,20 +52,31 @@ public void generateApiKey(Authentication authentication, CreateApiKeyRequest re
                 "creating derived api keys requires an explicit role descriptor that is empty (has no privileges)"));
             return;
         }
-        rolesStore.getRoleDescriptors(new HashSet<>(Arrays.asList(authentication.getUser().roles())),
-            ActionListener.wrap(roleDescriptors -> {
-                    for (RoleDescriptor rd : roleDescriptors) {
-                        try {
-                            DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry);
-                        } catch (ElasticsearchException | IllegalArgumentException e) {
-                            listener.onFailure(e);
-                            return;
-                        }
-                    }
-                    apiKeyService.createApiKey(authentication, request, roleDescriptors, listener);
-                },
-                listener::onFailure));
 
+        final ActionListener<Set<RoleDescriptor>> roleDescriptorsListener = ActionListener.wrap(roleDescriptors -> {
+            for (RoleDescriptor rd : roleDescriptors) {
+                try {
+                    DLSRoleQueryValidator.validateQueryField(rd.getIndicesPrivileges(), xContentRegistry);
+                } catch (ElasticsearchException | IllegalArgumentException e) {
+                    listener.onFailure(e);
+                    return;
+                }
+            }
+            apiKeyService.createApiKey(authentication, request, roleDescriptors, listener);
+        }, listener::onFailure);
+
+        if (ServiceAccountSettings.REALM_NAME.equals(authentication.getSourceRealm().getName())) {
+            final ServiceAccount serviceAccount = ServiceAccountService.getServiceAccounts().get(authentication.getUser().principal());
+            if (serviceAccount == null) {
+                roleDescriptorsListener.onFailure(new ElasticsearchSecurityException(
+                    "the authentication is created by a service account that does not exist: ["
+                        + authentication.getUser().principal() + "]"));
+            } else {
+                roleDescriptorsListener.onResponse(org.elasticsearch.core.Set.of(serviceAccount.roleDescriptor()));
+            }
+        } else {
+            rolesStore.getRoleDescriptors(new HashSet<>(Arrays.asList(authentication.getUser().roles())), roleDescriptorsListener);
+        }
     }
 
     private boolean grantsAnyPrivileges(CreateApiKeyRequest request) {
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java
@@ -275,7 +275,7 @@ private void getRolesForApiKey(Authentication authentication, ActionListener<Rol
                     } else {
                         buildAndCacheRoleForApiKey(authentication, true, ActionListener.wrap(
                             limitedByRole -> roleActionListener.onResponse(
-                                limitedByRole == Role.EMPTY ? role : LimitedRole.createLimitedRole(role, limitedByRole)),
+                                LimitedRole.createLimitedRole(role, limitedByRole)),
                             roleActionListener::onFailure
                         ));
                     }
diff --git a/x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java b/x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java

Original file line number	Diff line number	Diff line change
`@@ -275,7 +275,7 @@ private void getRolesForApiKey(Authentication authentication, ActionListener<Rol`
`275`	`275`	`} else {`
`276`	`276`	`buildAndCacheRoleForApiKey(authentication, true, ActionListener.wrap(`
`277`	`277`	`limitedByRole -> roleActionListener.onResponse(`
`278`		`- limitedByRole == Role.EMPTY ? role : LimitedRole.createLimitedRole(role, limitedByRole)),`
	`278`	`+ LimitedRole.createLimitedRole(role, limitedByRole)),`
`279`	`279`	`roleActionListener::onFailure`
`280`	`280`	`));`
`281`	`281`	`}`