From 1a4200a9748b78366eafca46570bcf2092faf622 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 28 Oct 2021 12:58:39 +0100 Subject: [PATCH 1/2] [ML] Retain built-in ML roles granting Kibana privileges The machine_learning_admin and machine_learning_user roles in Elasticsearch also grant access to the ML pages in Kibana. At one time it was intended that this should change in 8.0, so that ML privileges in Kibana would be completely separate. However, our thinking has now changed. An administrator cannot give a user the Elasticsearch backend roles and expect Kibana privileges alone to then stop that user from using ML - the user could just switch to curl or even Kibana dev console (which uses backend privileges rather than Kibana privileges). So it's clearer what is really being permitted if the backend roles continue to allow access to the ML UI as well as the ML backend endpoints. There's nothing the user can see in the ML UI that they couldn't find out by calling ML Elasticsearch endpoints directly and rendering the responses in a more graphical way. --- .../security/authz/store/ReservedRolesStore.java | 16 ++++++++++++++-- .../xpack/ml/MlDailyMaintenanceService.java | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java index c745f0775e7cd..c3ec9ba1ae073 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java @@ -285,7 +285,13 @@ private static Map initializeReservedRoles() { .indices(".ml-annotations*") .privileges("view_index_metadata", "read", "write") .build() }, - // TODO: remove Kibana privileges from ML backend roles in 8.0.0 + // This role also grants Kibana privileges related to ML. + // This makes it completely clear to UI administrators that + // if they grant the Elasticsearch backend role to a user then + // they cannot expect Kibana privileges to stop that user from + // accessing ML functionality - the user could switch to curl + // or even Kibana dev console and call the ES endpoints directly + // bypassing the Kibana privileges layer entirely. new RoleDescriptor.ApplicationResourcePrivileges[] { RoleDescriptor.ApplicationResourcePrivileges.builder() .application("kibana-*") @@ -313,7 +319,13 @@ private static Map initializeReservedRoles() { .indices(".ml-annotations*") .privileges("view_index_metadata", "read", "write") .build() }, - // TODO: remove Kibana privileges from ML backend roles in 8.0.0 + // This role also grants Kibana privileges related to ML. + // This makes it completely clear to UI administrators that + // if they grant the Elasticsearch backend role to a user then + // they cannot expect Kibana privileges to stop that user from + // accessing ML functionality - the user could switch to curl + // or even Kibana dev console and call the ES endpoints directly + // bypassing the Kibana privileges layer entirely. new RoleDescriptor.ApplicationResourcePrivileges[] { RoleDescriptor.ApplicationResourcePrivileges.builder() .application("kibana-*") diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java index e14de7f6b9c01..9f8f8a595b25a 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java @@ -141,7 +141,7 @@ public void close() { stop(); } - private synchronized void scheduleNext() { + private synchronized void scheduleNext() { try { cancellable = threadPool.schedule(this::triggerTasks, schedulerProvider.get(), ThreadPool.Names.GENERIC); } catch (EsRejectedExecutionException e) { From 396e7165b6d7dda6a54d965c868c5e435650d7e4 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Thu, 28 Oct 2021 13:13:47 +0100 Subject: [PATCH 2/2] Revert accidental change --- .../org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java index 9f8f8a595b25a..e14de7f6b9c01 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlDailyMaintenanceService.java @@ -141,7 +141,7 @@ public void close() { stop(); } - private synchronized void scheduleNext() { + private synchronized void scheduleNext() { try { cancellable = threadPool.schedule(this::triggerTasks, schedulerProvider.get(), ThreadPool.Names.GENERIC); } catch (EsRejectedExecutionException e) {