From f95eef8d5779c054b1673ded7a83b1f34fc0af88 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Wed, 16 Oct 2024 18:16:41 -0500 Subject: [PATCH 01/43] support monitor_stats privilege for remote cluster privileges --- .../main/java/org/elasticsearch/TransportVersions.java | 1 + .../xpack/core/security/authz/RoleDescriptor.java | 4 ++-- .../authz/permission/RemoteClusterPermissions.java | 4 +++- .../authz/privilege/ClusterPrivilegeResolver.java | 10 +++++++++- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index d1d423dcc5405..800e007a40091 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -247,6 +247,7 @@ static TransportVersion def(int id) { public static final TransportVersion ML_INFERENCE_ATTACH_TO_EXISTSING_DEPLOYMENT = def(8_771_00_0); public static final TransportVersion CONVERT_FAILURE_STORE_OPTIONS_TO_SELECTOR_OPTIONS_INTERNALLY = def(8_772_00_0); public static final TransportVersion REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_773_00_0); + public static final TransportVersion ROLE_MONITOR_STATS = def(8_774_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 1a8839fa0fa4a..895e3211b85cf 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -828,13 +828,13 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName currentFieldName = parser.currentName(); } else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { privileges = readStringArray(roleName, parser, false); - if (privileges.length != 1 + if (privileges.length > 2 || RemoteClusterPermissions.getSupportedRemoteClusterPermissions() .contains(privileges[0].trim().toLowerCase(Locale.ROOT)) == false) { throw new ElasticsearchParseException( "failed to parse remote_cluster for role [{}]. " + RemoteClusterPermissions.getSupportedRemoteClusterPermissions() - + " is the only value allowed for [{}] within [remote_cluster]", + + " are the only value allowed for [{}] within [remote_cluster]", roleName, currentFieldName ); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 2960c5aaa53e7..ad312ec98bae7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -68,7 +68,9 @@ public class RemoteClusterPermissions implements NamedWriteable, ToXContentObjec // package private non-final for testing static Map> allowedRemoteClusterPermissions = Map.of( TransportVersions.ROLE_REMOTE_CLUSTER_PRIVS, - Set.of(ClusterPrivilegeResolver.MONITOR_ENRICH.name()) + Set.of(ClusterPrivilegeResolver.MONITOR_ENRICH.name()), + TransportVersions.ROLE_MONITOR_STATS, + Set.of(ClusterPrivilegeResolver.MONITOR_STATS.name()) ); public static final RemoteClusterPermissions NONE = new RemoteClusterPermissions(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java index 3d1b378f4f51e..80fc375d5a741 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java @@ -110,6 +110,8 @@ public class ClusterPrivilegeResolver { private static final Set MONITOR_WATCHER_PATTERN = Set.of("cluster:monitor/xpack/watcher/*"); private static final Set MONITOR_ROLLUP_PATTERN = Set.of("cluster:monitor/xpack/rollup/*"); private static final Set MONITOR_ENRICH_PATTERN = Set.of("cluster:monitor/xpack/enrich/*", "cluster:admin/xpack/enrich/get"); + // intentionally cluster:monitor/stats* to match cluster:monitor/stats, cluster:monitor/stats[n] and cluster:monitor/stats/remote + private static final Set MONITOR_STATS_PATTERN = Set.of("cluster:monitor/stats*"); private static final Set ALL_CLUSTER_PATTERN = Set.of( "cluster:*", @@ -208,7 +210,11 @@ public class ClusterPrivilegeResolver { // esql enrich "cluster:monitor/xpack/enrich/esql/resolve_policy", "cluster:internal:data/read/esql/open_exchange", - "cluster:internal:data/read/esql/exchange" + "cluster:internal:data/read/esql/exchange", + // cluster stats for remote clusters + "cluster:monitor/stats/remote", + "cluster:monitor/stats", + "cluster:monitor/stats[n]" ); private static final Set CROSS_CLUSTER_REPLICATION_PATTERN = Set.of( RemoteClusterService.REMOTE_CLUSTER_HANDSHAKE_ACTION_NAME, @@ -243,6 +249,7 @@ public class ClusterPrivilegeResolver { public static final NamedClusterPrivilege MONITOR_WATCHER = new ActionClusterPrivilege("monitor_watcher", MONITOR_WATCHER_PATTERN); public static final NamedClusterPrivilege MONITOR_ROLLUP = new ActionClusterPrivilege("monitor_rollup", MONITOR_ROLLUP_PATTERN); public static final NamedClusterPrivilege MONITOR_ENRICH = new ActionClusterPrivilege("monitor_enrich", MONITOR_ENRICH_PATTERN); + public static final NamedClusterPrivilege MONITOR_STATS = new ActionClusterPrivilege("monitor_stats", MONITOR_STATS_PATTERN); public static final NamedClusterPrivilege MANAGE = new ActionClusterPrivilege("manage", ALL_CLUSTER_PATTERN, ALL_SECURITY_PATTERN); public static final NamedClusterPrivilege MANAGE_INFERENCE = new ActionClusterPrivilege("manage_inference", MANAGE_INFERENCE_PATTERN); public static final NamedClusterPrivilege MANAGE_ML = new ActionClusterPrivilege("manage_ml", MANAGE_ML_PATTERN); @@ -424,6 +431,7 @@ public class ClusterPrivilegeResolver { MONITOR_WATCHER, MONITOR_ROLLUP, MONITOR_ENRICH, + MONITOR_STATS, MANAGE, MANAGE_CONNECTOR, MANAGE_INFERENCE, From 89aa2085c08c569b00adfc03fae55ae94b02b794 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 15:07:21 -0500 Subject: [PATCH 02/43] fix doc test --- docs/reference/rest-api/security/bulk-create-roles.asciidoc | 2 +- .../reference/rest-api/security/get-builtin-privileges.asciidoc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/reference/rest-api/security/bulk-create-roles.asciidoc b/docs/reference/rest-api/security/bulk-create-roles.asciidoc index e4b6ef7f765c2..32eb495f7d280 100644 --- a/docs/reference/rest-api/security/bulk-create-roles.asciidoc +++ b/docs/reference/rest-api/security/bulk-create-roles.asciidoc @@ -315,7 +315,7 @@ The result would then have the `errors` field set to `true` and hold the error f "details": { "my_admin_role": { <4> "type": "action_request_validation_exception", - "reason": "Validation Failed: 1: unknown cluster privilege [bad_cluster_privilege]. a privilege must be either one of the predefined cluster privilege names [manage_own_api_key,manage_data_stream_global_retention,monitor_data_stream_global_retention,none,cancel_task,cross_cluster_replication,cross_cluster_search,delegate_pki,grant_api_key,manage_autoscaling,manage_index_templates,manage_logstash_pipelines,manage_oidc,manage_saml,manage_search_application,manage_search_query_rules,manage_search_synonyms,manage_service_account,manage_token,manage_user_profile,monitor_connector,monitor_enrich,monitor_inference,monitor_ml,monitor_rollup,monitor_snapshot,monitor_text_structure,monitor_watcher,post_behavioral_analytics_event,read_ccr,read_connector_secrets,read_fleet_secrets,read_ilm,read_pipeline,read_security,read_slm,transport_client,write_connector_secrets,write_fleet_secrets,create_snapshot,manage_behavioral_analytics,manage_ccr,manage_connector,manage_enrich,manage_ilm,manage_inference,manage_ml,manage_rollup,manage_slm,manage_watcher,monitor_data_frame_transforms,monitor_transform,manage_api_key,manage_ingest_pipelines,manage_pipeline,manage_data_frame_transforms,manage_transform,manage_security,monitor,manage,all] or a pattern over one of the available cluster actions;" + "reason": "Validation Failed: 1: unknown cluster privilege [bad_cluster_privilege]. a privilege must be either one of the predefined cluster privilege names [manage_own_api_key,manage_data_stream_global_retention,monitor_data_stream_global_retention,none,cancel_task,cross_cluster_replication,cross_cluster_search,delegate_pki,grant_api_key,manage_autoscaling,manage_index_templates,manage_logstash_pipelines,manage_oidc,manage_saml,manage_search_application,manage_search_query_rules,manage_search_synonyms,manage_service_account,manage_token,manage_user_profile,monitor_connector,monitor_enrich,monitor_inference,monitor_ml,monitor_rollup,monitor_snapshot,monitor_stats,monitor_text_structure,monitor_watcher,post_behavioral_analytics_event,read_ccr,read_connector_secrets,read_fleet_secrets,read_ilm,read_pipeline,read_security,read_slm,transport_client,write_connector_secrets,write_fleet_secrets,create_snapshot,manage_behavioral_analytics,manage_ccr,manage_connector,manage_enrich,manage_ilm,manage_inference,manage_ml,manage_rollup,manage_slm,manage_watcher,monitor_data_frame_transforms,monitor_transform,manage_api_key,manage_ingest_pipelines,manage_pipeline,manage_data_frame_transforms,manage_transform,manage_security,monitor,manage,all] or a pattern over one of the available cluster actions;" } } } diff --git a/docs/reference/rest-api/security/get-builtin-privileges.asciidoc b/docs/reference/rest-api/security/get-builtin-privileges.asciidoc index 8435f5539ab9d..8755ce456195b 100644 --- a/docs/reference/rest-api/security/get-builtin-privileges.asciidoc +++ b/docs/reference/rest-api/security/get-builtin-privileges.asciidoc @@ -111,6 +111,7 @@ A successful call returns an object with "cluster", "index", and "remote_cluster "monitor_ml", "monitor_rollup", "monitor_snapshot", + "monitor_stats", "monitor_text_structure", "monitor_transform", "monitor_watcher", From 32017a77e30afe3fa3e78ea53ab4869ffe714bef Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 15:51:49 -0500 Subject: [PATCH 03/43] more trivial test fixes --- x-pack/plugin/build.gradle | 1 + .../xpack/core/security/authz/RoleDescriptor.java | 1 + .../security/authz/permission/RemoteClusterPermissions.java | 3 ++- .../xpack/core/security/action/role/PutRoleRequestTests.java | 2 +- .../authz/permission/RemoteClusterPermissionsTests.java | 2 +- .../xpack/security/authz/store/FileRolesStoreTests.java | 2 +- .../resources/rest-api-spec/test/privileges/11_builtin.yml | 2 +- 7 files changed, 8 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 3e5aaea43a9b9..df021c173b4aa 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -85,5 +85,6 @@ tasks.named("yamlRestCompatTestTransform").configure({ task -> task.skipTest("wildcard/30_ignore_above_synthetic_source/wildcard field type ignore_above", "Temporary until backported") task.skipTest("inference/inference_crud/Test get all", "Assertions on number of inference models break due to default configs") task.skipTest("esql/60_usage/Basic ESQL usage output (telemetry)", "The telemetry output changed. We dropped a column. That's safe.") + task.replaceValueInLength("cluster", 62, "Test get builtin privileges") // new builtin privileges have been added }) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 895e3211b85cf..454e340384fc4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -827,6 +827,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { + //TODO: fix this! privileges = readStringArray(roleName, parser, false); if (privileges.length > 2 || RemoteClusterPermissions.getSupportedRemoteClusterPermissions() diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index ad312ec98bae7..cf64e01aab4ef 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; /** @@ -76,7 +77,7 @@ public class RemoteClusterPermissions implements NamedWriteable, ToXContentObjec public static final RemoteClusterPermissions NONE = new RemoteClusterPermissions(); public static Set getSupportedRemoteClusterPermissions() { - return allowedRemoteClusterPermissions.values().stream().flatMap(Set::stream).collect(Collectors.toSet()); + return allowedRemoteClusterPermissions.values().stream().flatMap(Set::stream).collect(Collectors.toCollection(TreeSet::new)); } public RemoteClusterPermissions(StreamInput in) throws IOException { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestTests.java index 97255502bc7be..239d48ca9c2e1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequestTests.java @@ -104,7 +104,7 @@ public void testValidationErrorWithUnknownRemoteClusterPrivilegeName() { } request.putRemoteCluster(remoteClusterPermissions); assertValidationError("Invalid remote_cluster permissions found. Please remove the following: [", request); - assertValidationError("Only [monitor_enrich] are allowed", request); + assertValidationError("Only [monitor_enrich, monitor_stats] are allowed", request); } public void testValidationErrorWithEmptyClustersInRemoteIndices() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index 394455879bbdf..46d3ef0ff0215 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -181,7 +181,7 @@ public void testValidate() { // random values not allowed IllegalArgumentException error = expectThrows(IllegalArgumentException.class, () -> remoteClusterPermission.validate()); assertTrue(error.getMessage().contains("Invalid remote_cluster permissions found. Please remove the following:")); - assertTrue(error.getMessage().contains("Only [monitor_enrich] are allowed")); + assertTrue(error.getMessage().contains("Only [monitor_enrich, monitor_stats] are allowed")); new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" })) .validate(); // no error diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java index 0a2c40d2a257a..8f1feecfab7c9 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java @@ -388,7 +388,7 @@ public void testParseFileWithRemoteIndicesAndCluster() throws IllegalAccessExcep events.get(4), startsWith( "failed to parse remote_cluster for role [invalid_role_bad_priv_remote_clusters]. " - + "[monitor_enrich] is the only value allowed for [privileges] within [remote_cluster]. skipping role..." + + "[monitor_enrich, monitor_stats] is the only value allowed for [privileges] within [remote_cluster]. skipping role..." ) ); } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml index ef8fab9ca7b6d..d03e6925cab1f 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/privileges/11_builtin.yml @@ -15,5 +15,5 @@ setup: # This is fragile - it needs to be updated every time we add a new cluster/index privilege # I would much prefer we could just check that specific entries are in the array, but we don't have # an assertion for that - - length: { "cluster" : 61 } + - length: { "cluster" : 62 } - length: { "index" : 22 } From 10dda913d7da2a916f08aa9fa385f651f00471db Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:01:39 -0500 Subject: [PATCH 04/43] fix plural error message --- .../xpack/core/security/authz/RoleDescriptor.java | 11 ++++++----- .../security/authz/store/FileRolesStoreTests.java | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 454e340384fc4..9771b750d4236 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -48,6 +48,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.XContentHelper.createParserNotCompressed; @@ -827,15 +828,15 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { - //TODO: fix this! privileges = readStringArray(roleName, parser, false); - if (privileges.length > 2 - || RemoteClusterPermissions.getSupportedRemoteClusterPermissions() - .contains(privileges[0].trim().toLowerCase(Locale.ROOT)) == false) { + if (Collections.disjoint( + RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), + Arrays.stream(privileges).map(String::toLowerCase).collect(Collectors.toSet()) + )) { throw new ElasticsearchParseException( "failed to parse remote_cluster for role [{}]. " + RemoteClusterPermissions.getSupportedRemoteClusterPermissions() - + " are the only value allowed for [{}] within [remote_cluster]", + + " are the only values allowed for [{}] within [remote_cluster]", roleName, currentFieldName ); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java index 8f1feecfab7c9..64e7f6c888ce6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java @@ -388,7 +388,8 @@ public void testParseFileWithRemoteIndicesAndCluster() throws IllegalAccessExcep events.get(4), startsWith( "failed to parse remote_cluster for role [invalid_role_bad_priv_remote_clusters]. " - + "[monitor_enrich, monitor_stats] is the only value allowed for [privileges] within [remote_cluster]. skipping role..." + + "[monitor_enrich, monitor_stats] are the only values allowed for [privileges] within [remote_cluster]. " + + "skipping role..." ) ); } From a720c829a6d08a4ddd26d8098e01e13fbd46ac00 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:06:09 -0500 Subject: [PATCH 05/43] fix RestGetUserPrivilegesActionTests --- .../rest/action/user/RestGetUserPrivilegesActionTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestGetUserPrivilegesActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestGetUserPrivilegesActionTests.java index e17d651a19748..5b91b774cc435 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestGetUserPrivilegesActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestGetUserPrivilegesActionTests.java @@ -213,7 +213,7 @@ public void testBuildResponse() throws Exception { ,"remote_cluster":[ { "privileges":[ - "monitor_enrich" + "monitor_enrich", "monitor_stats" ], "clusters":[ "remote-1" @@ -221,7 +221,7 @@ public void testBuildResponse() throws Exception { }, { "privileges":[ - "monitor_enrich" + "monitor_enrich", "monitor_stats" ], "clusters":[ "remote-2", From 1f7c1897d03544f86ad3d244ffa8f0db7a9e8bb4 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:17:12 -0500 Subject: [PATCH 06/43] more test fixes --- .../xpack/security/authz/RBACEngineTests.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index d71c2b0d19074..406651366990a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -1313,7 +1313,7 @@ public void testBuildUserPrivilegeResponse() { .addRemoteClusterPermissions( new RemoteClusterPermissions().addGroup( new RemoteClusterPermissionGroup( - RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0]), + new String[] {"monitor_enrich"}, new String[] { "remote-1" } ) ) @@ -1383,24 +1383,25 @@ public void testBuildUserPrivilegeResponse() { RemoteClusterPermissions remoteClusterPermissions = response.getRemoteClusterPermissions(); String[] allRemoteClusterPermissions = RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0]); - assert allRemoteClusterPermissions.length == 1 - : "if more remote cluster permissions are added this test needs to be updated to ensure the correct remotes receive the " - + "correct permissions. "; - // 2 groups with 3 aliases + assertThat(response.getRemoteClusterPermissions().groups(), iterableWithSize(2)); - assertEquals( - 3, - response.getRemoteClusterPermissions() - .groups() - .stream() - .map(RemoteClusterPermissionGroup::remoteClusterAliases) - .flatMap(Arrays::stream) - .distinct() - .count() + //remote-1 has monitor_enrich permission + //remote-2 and remote-3 have all permissions + assertThat( + response.getRemoteClusterPermissions().groups(), + containsInAnyOrder( + new RemoteClusterPermissionGroup(new String[] {"monitor_enrich"}, new String[] { "remote-1" }), + new RemoteClusterPermissionGroup(allRemoteClusterPermissions, new String[] { "remote-2", "remote-3" }) + ) + ); + + // ensure that all permissions are valid for the current transport version + assertThat( + Arrays.asList(remoteClusterPermissions.privilegeNames("remote-1", TransportVersion.current())), + hasItem("monitor_enrich") ); for (String permission : RemoteClusterPermissions.getSupportedRemoteClusterPermissions()) { - assertThat(Arrays.asList(remoteClusterPermissions.privilegeNames("remote-1", TransportVersion.current())), hasItem(permission)); assertThat(Arrays.asList(remoteClusterPermissions.privilegeNames("remote-2", TransportVersion.current())), hasItem(permission)); assertThat(Arrays.asList(remoteClusterPermissions.privilegeNames("remote-3", TransportVersion.current())), hasItem(permission)); } From 5e3f7a79447bd2cf2ed9f8de6829b8a9dd01b048 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:17:37 -0500 Subject: [PATCH 07/43] spotless --- .../xpack/core/security/authz/RoleDescriptor.java | 1 - .../xpack/security/authz/RBACEngineTests.java | 11 ++++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 9771b750d4236..555b108412ba5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -45,7 +45,6 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 406651366990a..587ea4c2f1bd1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -1312,10 +1312,7 @@ public void testBuildUserPrivilegeResponse() { ) .addRemoteClusterPermissions( new RemoteClusterPermissions().addGroup( - new RemoteClusterPermissionGroup( - new String[] {"monitor_enrich"}, - new String[] { "remote-1" } - ) + new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "remote-1" }) ) .addGroup( new RemoteClusterPermissionGroup( @@ -1385,12 +1382,12 @@ public void testBuildUserPrivilegeResponse() { String[] allRemoteClusterPermissions = RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0]); assertThat(response.getRemoteClusterPermissions().groups(), iterableWithSize(2)); - //remote-1 has monitor_enrich permission - //remote-2 and remote-3 have all permissions + // remote-1 has monitor_enrich permission + // remote-2 and remote-3 have all permissions assertThat( response.getRemoteClusterPermissions().groups(), containsInAnyOrder( - new RemoteClusterPermissionGroup(new String[] {"monitor_enrich"}, new String[] { "remote-1" }), + new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "remote-1" }), new RemoteClusterPermissionGroup(allRemoteClusterPermissions, new String[] { "remote-2", "remote-3" }) ) ); From 8ce0e2b8c370a2cb8a2630aa56b1f7ba34dc7a91 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:28:48 -0500 Subject: [PATCH 08/43] fix forbidden api --- .../xpack/core/security/authz/RoleDescriptor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 555b108412ba5..66f44c27657ad 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -45,6 +45,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -830,7 +831,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName privileges = readStringArray(roleName, parser, false); if (Collections.disjoint( RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), - Arrays.stream(privileges).map(String::toLowerCase).collect(Collectors.toSet()) + Arrays.stream(privileges).map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toSet()) )) { throw new ElasticsearchParseException( "failed to parse remote_cluster for role [{}]. " From d8278493dff1d3bc1418eebeb203046a8b9708da Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:30:09 -0500 Subject: [PATCH 09/43] prepare for merge --- server/src/main/java/org/elasticsearch/TransportVersions.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 800e007a40091..01e1c2c998ff6 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -247,7 +247,8 @@ static TransportVersion def(int id) { public static final TransportVersion ML_INFERENCE_ATTACH_TO_EXISTSING_DEPLOYMENT = def(8_771_00_0); public static final TransportVersion CONVERT_FAILURE_STORE_OPTIONS_TO_SELECTOR_OPTIONS_INTERNALLY = def(8_772_00_0); public static final TransportVersion REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_773_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_774_00_0); + + public static final TransportVersion ROLE_MONITOR_STATS = def(8_777_00_0); /* * STOP! READ THIS FIRST! No, really, From fa10f8051700ffd01051e20b8345a9156868a8d4 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 16:40:36 -0500 Subject: [PATCH 10/43] spotless --- server/src/main/java/org/elasticsearch/TransportVersions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index cf863d10284c6..e8d2acba12a2f 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -178,7 +178,7 @@ static TransportVersion def(int id) { public static final TransportVersion REVERT_REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_774_00_0); public static final TransportVersion ESQL_FIELD_ATTRIBUTE_PARENT_SIMPLIFIED = def(8_775_00_0); public static final TransportVersion INFERENCE_DONT_PERSIST_ON_READ = def(8_776_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_777_00_0); + public static final TransportVersion ROLE_MONITOR_STATS = def(8_777_00_0); /* * STOP! READ THIS FIRST! No, really, From 4095ccaf1e427240373d0ed9cc47088e3a29a1df Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 21 Oct 2024 17:14:53 -0500 Subject: [PATCH 11/43] mo tests and todos --- ...usterApiKeyRoleDescriptorBuilderTests.java | 3 +++ .../RemoteClusterPermissionsTests.java | 23 ++++++++++++------- .../xpack/security/authc/ApiKeyService.java | 2 ++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java index 22590e155e642..1680a85500b48 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; +import org.junit.Ignore; import java.io.IOException; import java.util.List; @@ -355,9 +356,11 @@ public void testEmptyAccessIsNotAllowed() throws IOException { assertThat(e2.getMessage(), containsString("doesn't support values of type: VALUE_NULL")); } + @Ignore("TODO: create automaton and test that the permissions are supported instead of checking the names directly") public void testAPIKeyAllowsAllRemoteClusterPrivilegesForCCS() { // if users can add remote cluster permissions to a role, then the APIKey should also allow that for that permission // the inverse however, is not guaranteed. cross_cluster_search exists largely for internal use and is not exposed to the users role + // TODO: create automaton and test that the permissions are supported instead of checking the names directly. assertTrue(Set.of(CCS_CLUSTER_PRIVILEGE_NAMES).containsAll(RemoteClusterPermissions.getSupportedRemoteClusterPermissions())); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index fc4b6409f7f7d..b5292d3661675 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Set; +import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -126,7 +127,8 @@ public void testPrivilegeNames() { // create random groups with random privileges for random clusters List randomGroups = generateRandomGroups(true); // replace a random value with one that is allowed - groupPrivileges.get(0)[0] = "monitor_enrich"; + String singleValidPrivilege = randomFrom(RemoteClusterPermissions.allowedRemoteClusterPermissions.get(TransportVersion.current())); + groupPrivileges.get(0)[0] = singleValidPrivilege; for (int i = 0; i < randomGroups.size(); i++) { String[] privileges = groupPrivileges.get(i); @@ -149,7 +151,7 @@ public void testPrivilegeNames() { assertFalse(Arrays.equals(privileges, found)); if (i == 0) { // ensure that for the current version we only find the valid "monitor_enrich" - assertThat(Set.of(found), equalTo(Set.of("monitor_enrich"))); + assertThat(Set.of(found), equalTo(Set.of(singleValidPrivilege))); } else { // all other groups should be found to not have any privileges assertTrue(found.length == 0); @@ -160,20 +162,25 @@ public void testPrivilegeNames() { } public void testMonitorEnrichPerVersion() { - // test monitor_enrich before, after and on monitor enrich version - String[] privileges = randomBoolean() ? new String[] { "monitor_enrich" } : new String[] { "monitor_enrich", "foo", "bar" }; + testRemotePermissionPerVersion("monitor_enrich", ROLE_REMOTE_CLUSTER_PRIVS); + testRemotePermissionPerVersion("monitor_stats", ROLE_MONITOR_STATS); + } + + private void testRemotePermissionPerVersion(String permission, TransportVersion version) { + // test permission before, after and on the version + String[] privileges = randomBoolean() ? new String[] { permission } : new String[] { permission, "foo", "bar" }; String[] before = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) - .privilegeNames("*", TransportVersionUtils.getPreviousVersion(ROLE_REMOTE_CLUSTER_PRIVS)); + .privilegeNames("*", TransportVersionUtils.getPreviousVersion(version)); // empty set since monitor_enrich is not allowed in the before version assertThat(Set.of(before), equalTo(Collections.emptySet())); String[] on = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) - .privilegeNames("*", ROLE_REMOTE_CLUSTER_PRIVS); + .privilegeNames("*", version); // only monitor_enrich since the other values are not allowed - assertThat(Set.of(on), equalTo(Set.of("monitor_enrich"))); + assertThat(Set.of(on), equalTo(Set.of(permission))); String[] after = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) .privilegeNames("*", TransportVersion.current()); // only monitor_enrich since the other values are not allowed - assertThat(Set.of(after), equalTo(Set.of("monitor_enrich"))); + assertThat(Set.of(after), equalTo(Set.of(permission))); } public void testValidate() { 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 index 03558e72fdca3..4da5cb4b45f6b 100644 --- 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 @@ -831,6 +831,8 @@ static Set maybeRemoveRemotePrivileges( + ". Remote cluster privileges are not supported by all nodes in the cluster." ); } + // TODO: support the additional cases where we are trying to send to somethign like 8.16 that understands remote cluster, + // but does not support the new privilege } return result; } From aeb8ff10c3226e6e851ababe4f383969a58ca261 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 24 Oct 2024 16:03:46 -0500 Subject: [PATCH 12/43] give kibana monitor_stats priv --- .../store/KibanaOwnedReservedRoleDescriptors.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java index 0028508e87f32..e15e7bb9064ad 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java @@ -20,6 +20,9 @@ import org.elasticsearch.xpack.core.security.action.profile.SuggestProfilesAction; import org.elasticsearch.xpack.core.security.action.user.ProfileHasPrivilegesAction; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissionGroup; +import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges; import org.elasticsearch.xpack.core.security.support.MetadataUtils; @@ -492,7 +495,16 @@ static RoleDescriptor kibanaSystem(String name) { getRemoteIndicesReadPrivileges("metrics-apm.*"), getRemoteIndicesReadPrivileges("traces-apm.*"), getRemoteIndicesReadPrivileges("traces-apm-*") }, - null, + //TODO: test this! + new RemoteClusterPermissions().addGroup( + new RemoteClusterPermissionGroup( + RemoteClusterPermissions.getSupportedRemoteClusterPermissions() + .stream() + .filter(s -> s.equals(ClusterPrivilegeResolver.MONITOR_STATS.name())) + .toArray(String[]::new), + new String[] { "*" } + ) + ), null, "Grants access necessary for the Kibana system user to read from and write to the Kibana indices, " + "manage index templates and tokens, and check the availability of the Elasticsearch cluster. " From 86c03670b901d501e01b23e9ed43696793997450 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Fri, 25 Oct 2024 15:39:31 -0500 Subject: [PATCH 13/43] integration test --- .../KibanaOwnedReservedRoleDescriptors.java | 2 +- ...usterApiKeyRoleDescriptorBuilderTests.java | 4 +- .../RemoteClusterSecurityRestStatsIT.java | 266 ++++++++++++++++++ 3 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestStatsIT.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java index e15e7bb9064ad..2d2fdf2ca97c7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java @@ -495,7 +495,7 @@ static RoleDescriptor kibanaSystem(String name) { getRemoteIndicesReadPrivileges("metrics-apm.*"), getRemoteIndicesReadPrivileges("traces-apm.*"), getRemoteIndicesReadPrivileges("traces-apm-*") }, - //TODO: test this! + // TODO: test this! new RemoteClusterPermissions().addGroup( new RemoteClusterPermissionGroup( RemoteClusterPermissions.getSupportedRemoteClusterPermissions() diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java index 1680a85500b48..e4864291793fb 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java @@ -15,7 +15,6 @@ import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; -import org.junit.Ignore; import java.io.IOException; import java.util.List; @@ -356,7 +355,8 @@ public void testEmptyAccessIsNotAllowed() throws IOException { assertThat(e2.getMessage(), containsString("doesn't support values of type: VALUE_NULL")); } - @Ignore("TODO: create automaton and test that the permissions are supported instead of checking the names directly") + // TODO: create automaton and test that the permissions are supported instead of checking the names directly + @AwaitsFix(bugUrl = "http://example.com/do/not/merge/me") public void testAPIKeyAllowsAllRemoteClusterPrivilegesForCCS() { // if users can add remote cluster permissions to a role, then the APIKey should also allow that for that permission // the inverse however, is not guaranteed. cross_cluster_search exists largely for internal use and is not exposed to the users role diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestStatsIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestStatsIT.java new file mode 100644 index 0000000000000..e98fcf6f72881 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRestStatsIT.java @@ -0,0 +1,266 @@ +/* + * 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.remotecluster; + +import org.apache.http.util.EntityUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.Strings; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.SearchResponseUtils; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.elasticsearch.test.cluster.util.resource.Resource; +import org.elasticsearch.test.junit.RunnableTestRuleAdapter; +import org.elasticsearch.test.rest.ObjectPath; +import org.elasticsearch.xcontent.XContentType; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; + +public class RemoteClusterSecurityRestStatsIT extends AbstractRemoteClusterSecurityTestCase { + + private static final AtomicReference> API_KEY_MAP_REF = new AtomicReference<>(); + private static final AtomicReference> REST_API_KEY_MAP_REF = new AtomicReference<>(); + private static final AtomicBoolean SSL_ENABLED_REF = new AtomicBoolean(); + private static final AtomicBoolean NODE1_RCS_SERVER_ENABLED = new AtomicBoolean(); + private static final AtomicBoolean NODE2_RCS_SERVER_ENABLED = new AtomicBoolean(); + private static final int FULFILL_NODE_COUNT = 3; + private static final Logger logger = LogManager.getLogger(RemoteClusterSecurityRestStatsIT.class); + + static { + fulfillingCluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .name("fulfilling-cluster") + .nodes(FULFILL_NODE_COUNT) + .apply(commonClusterConfig) + .setting("remote_cluster.port", "0") + .setting("xpack.security.remote_cluster_server.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get())) + .setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key") + .setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("xpack.security.remote_cluster_server.ssl.secure_key_passphrase", "remote-cluster-password") + .node(0, spec -> spec.setting("remote_cluster_server.enabled", "true")) + .node(1, spec -> spec.setting("remote_cluster_server.enabled", () -> String.valueOf(NODE1_RCS_SERVER_ENABLED.get()))) + .node(2, spec -> spec.setting("remote_cluster_server.enabled", () -> String.valueOf(NODE2_RCS_SERVER_ENABLED.get()))) + .build(); + + queryCluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .name("query-cluster") + .apply(commonClusterConfig) + .setting("xpack.security.remote_cluster_client.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get())) + .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") + .setting("xpack.security.authc.token.enabled", "true") + .keystore("cluster.remote.my_remote_cluster.credentials", () -> { + if (API_KEY_MAP_REF.get() == null) { + final Map apiKeyMap = createCrossClusterAccessApiKey(""" + { + "search": [ + { + "names": ["*"] + } + ] + }"""); + API_KEY_MAP_REF.set(apiKeyMap); + } + return (String) API_KEY_MAP_REF.get().get("encoded"); + }) + // Define a bogus API key for another remote cluster + .keystore("cluster.remote.invalid_remote.credentials", randomEncodedApiKey()) + // Define remote with a REST API key to observe expected failure + .keystore("cluster.remote.wrong_api_key_type.credentials", () -> { + if (REST_API_KEY_MAP_REF.get() == null) { + initFulfillingClusterClient(); + final var createApiKeyRequest = new Request("POST", "/_security/api_key"); + createApiKeyRequest.setJsonEntity(""" + { + "name": "rest_api_key" + }"""); + try { + final Response createApiKeyResponse = performRequestWithAdminUser(fulfillingClusterClient, createApiKeyRequest); + assertOK(createApiKeyResponse); + REST_API_KEY_MAP_REF.set(responseAsMap(createApiKeyResponse)); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + return (String) REST_API_KEY_MAP_REF.get().get("encoded"); + }) + .rolesFile(Resource.fromClasspath("roles.yml")) + .user(REMOTE_METRIC_USER, PASS.toString(), "read_remote_shared_metrics", false) + .build(); + } + + @ClassRule + // Use a RuleChain to ensure that fulfilling cluster is started before query cluster + // `SSL_ENABLED_REF` is used to control the SSL-enabled setting on the test clusters + // We set it here, since randomization methods are not available in the static initialize context above + public static TestRule clusterRule = RuleChain.outerRule(new RunnableTestRuleAdapter(() -> { + SSL_ENABLED_REF.set(usually()); + NODE1_RCS_SERVER_ENABLED.set(randomBoolean()); + NODE2_RCS_SERVER_ENABLED.set(randomBoolean()); + })).around(fulfillingCluster).around(queryCluster); + + public void testCrossClusterStats() throws Exception { + configureRemoteCluster(); + setupRoleAndUserQueryCluster(); + addDocToIndexFulfillingCluster("index1"); + + // search #1 + searchFulfillingClusterFromQueryCluster("index1"); + Map statsResponseAsMap = getFulfillingClusterStatsFromQueryCluster(); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs.clusters.my_remote_cluster.nodes_count"), equalTo(FULFILL_NODE_COUNT)); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.clusters.my_remote_cluster.total"), equalTo(1)); + int initialIndexCount = ObjectPath.evaluate(statsResponseAsMap, "ccs.clusters.my_remote_cluster.indices_count"); + + // search #2 + searchFulfillingClusterFromQueryCluster("index1"); + statsResponseAsMap = getFulfillingClusterStatsFromQueryCluster(); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.total"), equalTo(2)); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.clusters.my_remote_cluster.total"), equalTo(2)); + + // search #3 + expectThrows(Exception.class, () -> searchFulfillingClusterFromQueryCluster("junk")); + statsResponseAsMap = getFulfillingClusterStatsFromQueryCluster(); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.total"), equalTo(3)); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.clusters.my_remote_cluster.total"), equalTo(2)); + + // search #4 + addDocToIndexFulfillingCluster("index2"); + searchFulfillingClusterFromQueryCluster("index2"); + statsResponseAsMap = getFulfillingClusterStatsFromQueryCluster(); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.total"), equalTo(4)); + assertThat(ObjectPath.evaluate(statsResponseAsMap, "ccs._search.clusters.my_remote_cluster.total"), equalTo(3)); + int updatedIndexCount = ObjectPath.evaluate(statsResponseAsMap, "ccs.clusters.my_remote_cluster.indices_count"); + assertThat(updatedIndexCount, equalTo(initialIndexCount + 1)); + } + + private Map getFulfillingClusterStatsFromQueryCluster() throws IOException { + return getFulfillingClusterStatsFromQueryCluster(false); + } + + private Map getFulfillingClusterStatsFromQueryCluster(boolean humanDebug) throws IOException { + Request stats = new Request("GET", "_cluster/stats?include_remotes=true&filter_path=ccs"); + Response statsResponse = performRequestWithRemoteSearchUser(stats); + if (humanDebug) { + debugResponse(statsResponse); + } + return entityAsMap(statsResponse.getEntity()); + } + + private void searchFulfillingClusterFromQueryCluster(String index, boolean humanDebug) throws IOException { + final var searchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s:%s/_search?ccs_minimize_roundtrips=%s", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + index, + randomBoolean() + ) + ); + Response response = performRequestWithRemoteSearchUser(searchRequest); + if (humanDebug) { + debugResponse(response); + } + assertOK(response); + final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response)); + try { + final List actualIndices = Arrays.stream(searchResponse.getHits().getHits()) + .map(SearchHit::getIndex) + .collect(Collectors.toList()); + assertThat(actualIndices, containsInAnyOrder(index)); + + } finally { + searchResponse.decRef(); + } + } + + private void searchFulfillingClusterFromQueryCluster(String index) throws IOException { + searchFulfillingClusterFromQueryCluster(index, false); + } + + private void addDocToIndexFulfillingCluster(String index) throws IOException { + // Index some documents, so we can attempt to search them from the querying cluster + final Request bulkRequest = new Request("POST", "/_bulk?refresh=true"); + bulkRequest.setJsonEntity(Strings.format(""" + { "index": { "_index": "%s" } } + { "foo": "bar" } + """, index)); + assertOK(performRequestAgainstFulfillingCluster(bulkRequest)); + } + + private void setupRoleAndUserQueryCluster() throws IOException { + final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleRequest.setJsonEntity(""" + { + "description": "Role with privileges for remote indices and stats.", + "cluster": ["monitor_stats"], + "remote_indices": [ + { + "names": ["*"], + "privileges": ["read", "read_cross_cluster"], + "clusters": ["*"] + } + ], + "remote_cluster": [ + { + "privileges": ["monitor_stats"], + "clusters": ["*"] + } + ] + }"""); + assertOK(adminClient().performRequest(putRoleRequest)); + final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(adminClient().performRequest(putUserRequest)); + } + + private Response performRequestWithRemoteSearchUser(final Request request) throws IOException { + request.setOptions( + RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(REMOTE_SEARCH_USER, PASS)) + ); + return client().performRequest(request); + } + + // helper method for humans see the responses for debug purposes, when used will always fail the test + private void debugResponse(Response response) throws IOException { + String jsonString = XContentHelper.convertToJson( + new BytesArray(EntityUtils.toString(response.getEntity())), + true, + true, + XContentType.JSON + ); + logger.error(jsonString); + assertFalse(true); // boom + } +} From c259d9dea97f4bdbc0f1338f615dfb2271b34afb Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 28 Oct 2024 14:09:22 -0500 Subject: [PATCH 14/43] test fixes --- .../security/get-builtin-privileges.asciidoc | 3 ++- .../store/KibanaOwnedReservedRoleDescriptors.java | 1 - .../xpack/security/authz/RBACEngineTests.java | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/reference/rest-api/security/get-builtin-privileges.asciidoc b/docs/reference/rest-api/security/get-builtin-privileges.asciidoc index 8755ce456195b..7f3d75b926780 100644 --- a/docs/reference/rest-api/security/get-builtin-privileges.asciidoc +++ b/docs/reference/rest-api/security/get-builtin-privileges.asciidoc @@ -153,7 +153,8 @@ A successful call returns an object with "cluster", "index", and "remote_cluster "write" ], "remote_cluster" : [ - "monitor_enrich" + "monitor_enrich", + "monitor_stats" ] } -------------------------------------------------- diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java index 2d2fdf2ca97c7..9fb43d3b450b8 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/KibanaOwnedReservedRoleDescriptors.java @@ -495,7 +495,6 @@ static RoleDescriptor kibanaSystem(String name) { getRemoteIndicesReadPrivileges("metrics-apm.*"), getRemoteIndicesReadPrivileges("traces-apm.*"), getRemoteIndicesReadPrivileges("traces-apm-*") }, - // TODO: test this! new RemoteClusterPermissions().addGroup( new RemoteClusterPermissionGroup( RemoteClusterPermissions.getSupportedRemoteClusterPermissions() diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 587ea4c2f1bd1..eced4af65d013 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -92,6 +92,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges.ManageApplicationPrivileges; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.Privilege; @@ -1792,6 +1793,18 @@ public void testGetRoleDescriptorsForRemoteClusterForReservedRoles() { null, null, null, + null, + null, + new RemoteClusterPermissions().addGroup( + new RemoteClusterPermissionGroup( + RemoteClusterPermissions.getSupportedRemoteClusterPermissions() + .stream() + .filter(s -> s.equals(ClusterPrivilegeResolver.MONITOR_STATS.name())) + .toArray(String[]::new), + new String[] { "*" } + ) + ), + null, null ) ) From 25ee3285bdd1d2947ca142e351cb60fa2ce08778 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 28 Oct 2024 14:29:09 -0500 Subject: [PATCH 15/43] remove tab --- server/src/main/java/org/elasticsearch/TransportVersions.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index bd7211ab4bef6..6f18f235ea136 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -183,8 +183,8 @@ static TransportVersion def(int id) { public static final TransportVersion INTRODUCE_ALL_APPLICABLE_SELECTOR = def(8_778_00_0); public static final TransportVersion INDEX_MODE_LOOKUP = def(8_779_00_0); public static final TransportVersion INDEX_REQUEST_REMOVE_METERING = def(8_780_00_0); - public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_782_00_0); + public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); + public static final TransportVersion ROLE_MONITOR_STATS = def(8_782_00_0); /* * STOP! READ THIS FIRST! No, really, From 49a4d1e1ef7fedc82bb2c8839d365ec6c3553f71 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 28 Oct 2024 15:21:24 -0500 Subject: [PATCH 16/43] fix test --- .../xpack/security/authz/RBACEngineTests.java | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index eced4af65d013..7404187c1bdbc 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -1781,7 +1781,10 @@ public void testGetRoleDescriptorsForRemoteClusterForReservedRoles() { new RoleDescriptorsIntersection( new RoleDescriptor( Role.REMOTE_USER_ROLE_NAME, - null, + RemoteClusterPermissions.getSupportedRemoteClusterPermissions() + .stream() + .filter(s -> s.equals(ClusterPrivilegeResolver.MONITOR_STATS.name())) + .toArray(String[]::new), new IndicesPrivileges[] { IndicesPrivileges.builder().indices(".monitoring-*").privileges("read", "read_cross_cluster").build(), IndicesPrivileges.builder().indices("apm-*").privileges("read", "read_cross_cluster").build(), @@ -1793,18 +1796,6 @@ public void testGetRoleDescriptorsForRemoteClusterForReservedRoles() { null, null, null, - null, - null, - new RemoteClusterPermissions().addGroup( - new RemoteClusterPermissionGroup( - RemoteClusterPermissions.getSupportedRemoteClusterPermissions() - .stream() - .filter(s -> s.equals(ClusterPrivilegeResolver.MONITOR_STATS.name())) - .toArray(String[]::new), - new String[] { "*" } - ) - ), - null, null ) ) From 7e4152f1156bb91a86649e96e2ce26adffcf9862 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 28 Oct 2024 16:46:36 -0500 Subject: [PATCH 17/43] adjustments for RemoteClusterPermissionsTests.java --- .../RemoteClusterPermissionsTests.java | 51 +++++++++++++++---- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index b5292d3661675..d841aec861c72 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -15,6 +15,8 @@ import org.elasticsearch.test.AbstractXContentSerializingTestCase; import org.elasticsearch.test.TransportVersionUtils; import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.xcontent.XContentUtils; import org.junit.Before; import java.io.IOException; @@ -27,6 +29,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; @@ -161,25 +164,25 @@ public void testPrivilegeNames() { } } - public void testMonitorEnrichPerVersion() { - testRemotePermissionPerVersion("monitor_enrich", ROLE_REMOTE_CLUSTER_PRIVS); - testRemotePermissionPerVersion("monitor_stats", ROLE_MONITOR_STATS); + public void testPermissionsPerVersion() { + testPermissionPerVersion("monitor_enrich", ROLE_REMOTE_CLUSTER_PRIVS); + testPermissionPerVersion("monitor_stats", ROLE_MONITOR_STATS); } - private void testRemotePermissionPerVersion(String permission, TransportVersion version) { + private void testPermissionPerVersion(String permission, TransportVersion version) { // test permission before, after and on the version String[] privileges = randomBoolean() ? new String[] { permission } : new String[] { permission, "foo", "bar" }; String[] before = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) .privilegeNames("*", TransportVersionUtils.getPreviousVersion(version)); - // empty set since monitor_enrich is not allowed in the before version + // empty set since permissions is not allowed in the before version assertThat(Set.of(before), equalTo(Collections.emptySet())); String[] on = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) .privilegeNames("*", version); - // only monitor_enrich since the other values are not allowed + // the permission is found on that provided version assertThat(Set.of(on), equalTo(Set.of(permission))); String[] after = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) .privilegeNames("*", TransportVersion.current()); - // only monitor_enrich since the other values are not allowed + // current version (after the version) has the permission assertThat(Set.of(after), equalTo(Set.of(permission))); } @@ -223,22 +226,48 @@ protected Writeable.Reader instanceReader() { @Override protected RemoteClusterPermissions createTestInstance() { + Set all = RemoteClusterPermissions.allowedRemoteClusterPermissions.values() + .stream() + .flatMap(Set::stream) + .collect(Collectors.toSet()); + List randomPermission = randomList(1, all.size(), () -> randomFrom(all)); return new RemoteClusterPermissions().addGroup( - new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" }) + new RemoteClusterPermissionGroup(randomPermission.toArray(new String[0]), new String[] { "*" }) ); } @Override protected RemoteClusterPermissions mutateInstance(RemoteClusterPermissions instance) throws IOException { return new RemoteClusterPermissions().addGroup( - new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" }) + new RemoteClusterPermissionGroup(new String[] { "monitor_enrich", "monitor_stats" }, new String[] { "*" }) ).addGroup(new RemoteClusterPermissionGroup(new String[] { "foobar" }, new String[] { "*" })); } @Override protected RemoteClusterPermissions doParseInstance(XContentParser parser) throws IOException { - // fromXContent/parsing isn't supported since we still do old school manual parsing of the role descriptor - return createTestInstance(); + // fromXContent/object parsing isn't supported since we still do old school manual parsing of the role descriptor + // so this test is silly because it only tests we know how to manually parse the test instance in this test + // this is needed since we want the other parts from the AbstractXContentSerializingTestCase suite + RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); + String[] privileges = null; + String[] clusters = null; + XContentParser.Token token; + String currentFieldName = null; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.START_OBJECT) { + continue; + } + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (RoleDescriptor.Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { + privileges = XContentUtils.readStringArray(parser, false); + + } else if (RoleDescriptor.Fields.CLUSTERS.match(currentFieldName, parser.getDeprecationHandler())) { + clusters = XContentUtils.readStringArray(parser, false); + } + } + remoteClusterPermissions.addGroup(new RemoteClusterPermissionGroup(privileges, clusters)); + return remoteClusterPermissions; } @Override From c3801124735ad17823f0fc84cd264cefa95750ef Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Tue, 29 Oct 2024 17:39:49 -0500 Subject: [PATCH 18/43] wip: properly gate remote cluster permission --- .../org/elasticsearch/TransportVersions.java | 6 +- .../core/security/authc/Authentication.java | 91 +++++++++++++++++++ .../RemoteClusterPermissionGroup.java | 24 ++++- .../permission/RemoteClusterPermissions.java | 36 +++++++- .../privilege/ClusterPrivilegeResolver.java | 2 +- .../store/RoleReferenceIntersection.java | 1 + .../RemoteClusterPermissionGroupTests.java | 14 +++ .../RemoteClusterPermissionsTests.java | 11 +++ .../security/qa/multi-cluster/build.gradle | 7 +- ...lusterSecurityBWCToRCS1ClusterRestIT.java} | 13 ++- .../authz/store/RoleDescriptorStore.java | 2 +- 11 files changed, 195 insertions(+), 12 deletions(-) rename x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/{RemoteClusterSecurityBwcRestIT.java => RemoteClusterSecurityBWCToRCS1ClusterRestIT.java} (92%) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 6f18f235ea136..7f737ea8c8709 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -184,7 +184,11 @@ static TransportVersion def(int id) { public static final TransportVersion INDEX_MODE_LOOKUP = def(8_779_00_0); public static final TransportVersion INDEX_REQUEST_REMOVE_METERING = def(8_780_00_0); public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_782_00_0); + + + + //FIXME: before merging this PR, make sure to update the transport version correctly + public static final TransportVersion ROLE_MONITOR_STATS = def(8_882_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 04dda75692208..66f29182ac4c2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -36,6 +36,7 @@ import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings; import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.InternalUser; import org.elasticsearch.xpack.core.security.user.InternalUsers; @@ -43,10 +44,12 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.util.ArrayList; import java.util.Base64; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; @@ -1296,6 +1299,7 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( TransportVersion streamVersion, Authentication authentication ) { + System.out.println("*************** maybeRewriteMetadataForApiKeyRoleDescriptors ***************"); Map metadata = authentication.getAuthenticatingSubject().getMetadata(); // If authentication user is an API key or a token created by an API key, // regardless whether it has run-as, the metadata must contain API key role descriptors @@ -1321,6 +1325,7 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( ); } + // removes the entire remote_cluster field from the role descriptors if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) && streamVersion.before(ROLE_REMOTE_CLUSTER_PRIVS)) { metadata = new HashMap<>(metadata); @@ -1338,6 +1343,28 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( ); } + // the current cluster understands remote_cluster field in role descriptors, so check each permission and remove as needed + if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)){ + + + RemoteClusterPermissions.getSupportedRemoteClusterPermissions(); + + metadata = new HashMap<>(metadata); + metadata.put( + AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, + maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( + (BytesReference) metadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY), streamVersion + ) + ); + metadata.put( + AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, + maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( + (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), streamVersion + ) + ); + + } + if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES) && streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) { metadata = new HashMap<>(metadata); @@ -1450,6 +1477,70 @@ static BytesReference maybeRemoveTopLevelFromRoleDescriptors(BytesReference role } } + /** + * Before we send the role descriptors to the remote cluster, we need to remove the remote cluster privileges that the other cluster + * will not understand. + * @param roleDescriptorsBytes The role descriptors to be sent to the remote cluster, represented as bytes. + * @return The role descriptors with the privileges that unsupported by version removed, represented as bytes. + */ + @SuppressWarnings("unchecked") + static BytesReference maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( + BytesReference roleDescriptorsBytes, + TransportVersion outboundVersion + ) { + if (roleDescriptorsBytes == null || roleDescriptorsBytes.length() == 0) { + return roleDescriptorsBytes; + } + + final Map roleDescriptorsMap = convertRoleDescriptorsBytesToMap(roleDescriptorsBytes); + final AtomicBoolean modified = new AtomicBoolean(false); + System.out.println("****************************** "); + System.out.println("roleDescriptorsMap: " + roleDescriptorsMap); + roleDescriptorsMap.forEach((key, value) -> { + if (value instanceof Map) { + @SuppressWarnings("unchecked") + Map roleDescriptor = (Map) value; + roleDescriptor.forEach((innerKey, innerValue) -> { + + // remote_cluster=[{privileges=[monitor_enrich, monitor_stats] + if ("remote_cluster".equals(innerKey)) { // todo: use constant + assert innerValue instanceof List; + RemoteClusterPermissions discoveredRemoteClusterPermission + = new RemoteClusterPermissions((List>>) innerValue); + + + RemoteClusterPermissions mutated = discoveredRemoteClusterPermission.removeUnsupportedPrivileges(outboundVersion); + System.out.println("********* mutated: " + mutated); + if(mutated.equals(discoveredRemoteClusterPermission) == false) { + System.out.println("********* modified: " + true); + modified.set(true); + } + + + } + }); + + + } + }); + Iterator> it = roleDescriptorsMap.entrySet().iterator(); + Map.Entry next = it.next(); + System.out.println("******** next: " + next); + + + if (modified.get()) { +// Iterator> it = roleDescriptorsMap.entrySet().iterator(); +// Map.Entry next = it.next(); +// System.out.println("******** next: " + next + + + return convertRoleDescriptorsMapToBytes(roleDescriptorsMap); + } else { + // No need to serialize if we did not remove anything. + return roleDescriptorsBytes; + } + } + static boolean equivalentRealms(String name1, String type1, String name2, String type2) { if (false == type1.equals(type2)) { return false; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java index 1c34a7829fcbb..a580e3ee02c6c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java @@ -18,6 +18,11 @@ import java.io.IOException; import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.xpack.core.security.authz.RoleDescriptor.Fields.CLUSTERS; +import static org.elasticsearch.xpack.core.security.authz.RoleDescriptor.Fields.PRIVILEGES; /** * Represents a group of permissions for a remote cluster. For example: @@ -41,6 +46,15 @@ public RemoteClusterPermissionGroup(StreamInput in) throws IOException { remoteClusterAliasMatcher = StringMatcher.of(remoteClusterAliases); } + + public RemoteClusterPermissionGroup(Map> remoteClusterGroup) { + assert remoteClusterGroup.get(PRIVILEGES.getPreferredName()) != null : "privileges must be non-null"; + assert remoteClusterGroup.get(CLUSTERS.getPreferredName()) != null : "clusters must be non-null"; + clusterPrivileges = remoteClusterGroup.get(PRIVILEGES.getPreferredName()).toArray(new String[0]); + remoteClusterAliases = remoteClusterGroup.get(CLUSTERS.getPreferredName()).toArray(new String[0]); + remoteClusterAliasMatcher = StringMatcher.of(remoteClusterAliases); + } + /** * @param clusterPrivileges The list of cluster privileges that are allowed for the remote cluster. must not be null or empty. * @param remoteClusterAliases The list of remote clusters that the privileges apply to. must not be null or empty. @@ -86,11 +100,17 @@ public String[] remoteClusterAliases() { return Arrays.copyOf(remoteClusterAliases, remoteClusterAliases.length); } + + public Map> toMap() { + return Map.of(PRIVILEGES.getPreferredName(), Arrays.asList(clusterPrivileges), + CLUSTERS.getPreferredName(), Arrays.asList(remoteClusterAliases)); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.array(RoleDescriptor.Fields.PRIVILEGES.getPreferredName(), clusterPrivileges); - builder.array(RoleDescriptor.Fields.CLUSTERS.getPreferredName(), remoteClusterAliases); + builder.array(PRIVILEGES.getPreferredName(), clusterPrivileges); + builder.array(CLUSTERS.getPreferredName(), remoteClusterAliases); builder.endObject(); return builder; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 64c3ac0fdbe5d..0a402e8a77a52 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -32,6 +32,8 @@ import java.util.TreeSet; import java.util.stream.Collectors; +import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; + /** * Represents the set of permissions for remote clusters. This is intended to be the model for both the {@link RoleDescriptor} * and {@link Role}. This model is not intended to be sent to a remote cluster, but can be (wire) serialized within a single cluster @@ -72,7 +74,7 @@ public class RemoteClusterPermissions implements NamedWriteable, ToXContentObjec static Map> allowedRemoteClusterPermissions = Map.of( ROLE_REMOTE_CLUSTER_PRIVS, Set.of(ClusterPrivilegeResolver.MONITOR_ENRICH.name()), - TransportVersions.ROLE_MONITOR_STATS, + ROLE_MONITOR_STATS, Set.of(ClusterPrivilegeResolver.MONITOR_STATS.name()) ); @@ -86,6 +88,14 @@ public RemoteClusterPermissions(StreamInput in) throws IOException { remoteClusterPermissionGroups = in.readNamedWriteableCollectionAsList(RemoteClusterPermissionGroup.class); } + public RemoteClusterPermissions(List>> remoteClusters){ + remoteClusterPermissionGroups = new ArrayList<>(); + for (Map> remoteCluster : remoteClusters) { + RemoteClusterPermissionGroup remoteClusterPermissionGroup = new RemoteClusterPermissionGroup(remoteCluster); + remoteClusterPermissionGroups.add(remoteClusterPermissionGroup); + } + } + public RemoteClusterPermissions() { remoteClusterPermissionGroups = new ArrayList<>(); } @@ -99,6 +109,24 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus return this; } + public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion outboundVersion) { + System.out.println("************ [DEBUG] RemoteClusterPermissions.removeUnsupportedPrivileges"); + RemoteClusterPermissions copyForOutBoundVersion = new RemoteClusterPermissions(); + for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) { + //TODO: actually implement this method, this is a hack to make the test pass for now + if(outboundVersion.onOrBefore(ROLE_MONITOR_STATS)){ + Set a = new HashSet<>(Arrays.asList(group.clusterPrivileges())); + a.remove("monitor_stats"); + copyForOutBoundVersion.addGroup( + new RemoteClusterPermissionGroup(a.toArray(new String[0]), group.remoteClusterAliases()) + ); + }else{ + copyForOutBoundVersion.addGroup(group); + } + } + return copyForOutBoundVersion; + } + /** * Gets the privilege names for the remote cluster. This method will collapse all groups to single String[] all lowercase * and will only return the appropriate privileges for the provided remote cluster version. @@ -140,6 +168,10 @@ public String[] privilegeNames(final String remoteClusterAlias, TransportVersion return allowedPrivileges.stream().sorted().toArray(String[]::new); } + public List>> toMap() { + return remoteClusterPermissionGroups.stream().map(RemoteClusterPermissionGroup::toMap).toList(); + } + /** * Validates the remote cluster permissions (regardless of remote cluster version). * This method will throw an {@link IllegalArgumentException} if the permissions are invalid. @@ -223,4 +255,6 @@ public String toString() { public String getWriteableName() { return NAME; } + + } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java index 80fc375d5a741..00d45fb135fb2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java @@ -507,7 +507,7 @@ public static NamedClusterPrivilege resolve(String name) { + Strings.collectionToCommaDelimitedString(VALUES.keySet()) + "] or a pattern over one of the available " + "cluster actions"; - logger.debug(errorMessage); + logger.warn(errorMessage); throw new IllegalArgumentException(errorMessage); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java index 86a56a30fc348..ec5e415d13497 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java @@ -36,6 +36,7 @@ public List getRoleReferences() { return roleReferences; } + public void buildRole(BiConsumer> singleRoleBuilder, ActionListener roleActionListener) { final GroupedActionListener roleGroupedActionListener = new GroupedActionListener<>( roleReferences.size(), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java index cd269bd1a97b3..5662158917ad5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Locale; +import java.util.Map; import static org.hamcrest.Matchers.containsString; @@ -103,6 +104,19 @@ public void testInvalidValues() { assertEquals("remote_cluster privileges must contain valid non-empty, non-null values", e2.getMessage()); } + public void testToMap(){ + String[] privileges = generateRandomStringArray(5, 5, false, false); + String[] clusters = generateRandomStringArray(5, 5, false, false); + RemoteClusterPermissionGroup remoteClusterPermissionGroup = new RemoteClusterPermissionGroup(privileges, clusters); + assertEquals( + Map.of( + "privileges", Arrays.asList(privileges), + "clusters", Arrays.asList(clusters) + ), + remoteClusterPermissionGroup.toMap() + ); + } + @Override protected Writeable.Reader instanceReader() { return RemoteClusterPermissionGroup::new; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index d841aec861c72..0296c804d41c2 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -197,6 +197,17 @@ public void testValidate() { .validate(); // no error } + public void testToMap(){ + RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); + List groups = generateRandomGroups(randomBoolean()); + for (int i = 0; i < groups.size(); i++) { + remoteClusterPermissions.addGroup(groups.get(i)); + } + List>> asAsMap = remoteClusterPermissions.toMap(); + RemoteClusterPermissions remoteClusterPermissionsAsMap = new RemoteClusterPermissions(asAsMap); + assertEquals(remoteClusterPermissions, remoteClusterPermissionsAsMap); + } + private List generateRandomGroups(boolean fuzzyCluster) { clean(); List groups = new ArrayList<>(); diff --git a/x-pack/plugin/security/qa/multi-cluster/build.gradle b/x-pack/plugin/security/qa/multi-cluster/build.gradle index c7b8f81bb7876..fbd1887da38c9 100644 --- a/x-pack/plugin/security/qa/multi-cluster/build.gradle +++ b/x-pack/plugin/security/qa/multi-cluster/build.gradle @@ -31,13 +31,14 @@ dependencies { tasks.named("javaRestTest") { enabled = true // This is tested explicitly in bwc test tasks. - exclude '**/RemoteClusterSecurityBwcRestIT.class' + exclude '**/RemoteClusterSecurityBWCToRCS1ClusterRestIT.class' } -BuildParams.bwcVersions.withWireCompatible(v -> v.before(BuildParams.isSnapshotBuild() ? '8.8.0' : '8.9.1')) { bwcVersion, baseName -> +//TODO: not sure if this works or not... can't run tests atm due to the issue with GH +BuildParams.bwcVersions.withWireCompatible() { bwcVersion, baseName -> tasks.register(bwcTaskName(bwcVersion), StandaloneRestIntegTestTask) { usesBwcDistribution(bwcVersion) systemProperty("tests.old_cluster_version", bwcVersion) - include '**/RemoteClusterSecurityBwcRestIT.class' + include '**/RemoteClusterSecurityBWCToRCS1ClusterRestIT.class' } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBwcRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java similarity index 92% rename from x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBwcRestIT.java rename to x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java index 17acd258ed34b..04d4627770bee 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBwcRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java @@ -38,9 +38,14 @@ import static org.hamcrest.Matchers.notNullValue; /** - * BWC test which ensures that users and API keys with defined {@code remote_indices} privileges can be used to query legacy remote clusters + * BWC test which ensures that users and API keys with defined {@code remote_indices}/{@code remote_cluster} privileges can be used + * to query legacy remote clusters when using RCS 1.0. We send the request the to an older fulfilling cluster using RCS 1.0 with a user/role + * and API key where the {@code remote_indices}/{@code remote_cluster} are defined in the newer query cluster. + * All RCS 2.0 config should be effectively ignored when using RCS 1 for CCS. We send to an elder fulfil cluster to help ensure that + * newly introduced RCS 2.0 artifacts are forward compatible from the perspective of the old cluster. For example, a new privilege + * sent to an old cluster should be ignored. */ -public class RemoteClusterSecurityBwcRestIT extends AbstractRemoteClusterSecurityTestCase { +public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteClusterSecurityTestCase { private static final Version OLD_CLUSTER_VERSION = Version.fromString(System.getProperty("tests.old_cluster_version")); @@ -51,6 +56,8 @@ public class RemoteClusterSecurityBwcRestIT extends AbstractRemoteClusterSecurit .name("fulfilling-cluster") .apply(commonClusterConfig) .setting("xpack.ml.enabled", "false") + // .setting("logger.org.elasticsearch.xpack.core", "trace") //useful for human debugging + // .setting("logger.org.elasticsearch.xpack.security", "trace") //useful for human debugging .build(); queryCluster = ElasticsearchCluster.local() @@ -166,7 +173,7 @@ public void testBwcWithLegacyCrossClusterSearch() throws Exception { ], "remote_cluster": [ { - "privileges": ["monitor_enrich"], + "privileges": ["monitor_enrich", "monitor_stats"], "clusters": ["*"] } ] diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/RoleDescriptorStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/RoleDescriptorStore.java index ac8d84d95fd1d..a64cef366926f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/RoleDescriptorStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/RoleDescriptorStore.java @@ -150,7 +150,7 @@ public void resolveCrossClusterAccessRoleReference( + "but other privileges found for subject [" + crossClusterAccessRoleReference.getUserPrincipal() + "]"; - logger.debug("{}. Invalid role descriptor: [{}]", message, roleDescriptor); + logger.warn("{}. Invalid role descriptor: [{}]", message, roleDescriptor); listener.onFailure(new IllegalArgumentException(message)); return; } From a34b620a24839acdfdfc00a40713dd063d4c568e Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Wed, 30 Oct 2024 10:35:50 -0500 Subject: [PATCH 19/43] passing test --- .../core/security/authc/Authentication.java | 50 +++++++------------ .../core/security/authz/RoleDescriptor.java | 19 ++++--- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 66f29182ac4c2..a1328dc6ca6f5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -25,9 +25,11 @@ import org.elasticsearch.core.Strings; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.security.action.profile.Profile; import org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo.RoleDescriptorsBytes; import org.elasticsearch.xpack.core.security.authc.RealmConfig.RealmIdentifier; @@ -79,6 +81,7 @@ import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_NAME; import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_TYPE; import static org.elasticsearch.xpack.core.security.authc.RealmDomain.REALM_DOMAIN_PARSER; +import static org.elasticsearch.xpack.core.security.authz.RoleDescriptor.Fields.REMOTE_CLUSTER; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; /** @@ -1343,6 +1346,7 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( ); } + //TODO: this needs to be in the else so we don't add back the remote_cluster field // the current cluster understands remote_cluster field in role descriptors, so check each permission and remove as needed if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)){ @@ -1444,7 +1448,7 @@ private static BytesReference convertRoleDescriptorsMapToBytes(Map roleDescriptorsMap = convertRoleDescriptorsBytesToMap(roleDescriptorsBytes); + final Map roleDescriptorsMapMutated = new HashMap<>(roleDescriptorsMap); final AtomicBoolean modified = new AtomicBoolean(false); - System.out.println("****************************** "); - System.out.println("roleDescriptorsMap: " + roleDescriptorsMap); roleDescriptorsMap.forEach((key, value) -> { if (value instanceof Map) { - @SuppressWarnings("unchecked") Map roleDescriptor = (Map) value; roleDescriptor.forEach((innerKey, innerValue) -> { - - // remote_cluster=[{privileges=[monitor_enrich, monitor_stats] - if ("remote_cluster".equals(innerKey)) { // todo: use constant - assert innerValue instanceof List; - RemoteClusterPermissions discoveredRemoteClusterPermission - = new RemoteClusterPermissions((List>>) innerValue); - - - RemoteClusterPermissions mutated = discoveredRemoteClusterPermission.removeUnsupportedPrivileges(outboundVersion); - System.out.println("********* mutated: " + mutated); - if(mutated.equals(discoveredRemoteClusterPermission) == false) { - System.out.println("********* modified: " + true); + // example: remote_cluster=[{privileges=[monitor_enrich, monitor_stats] + if (REMOTE_CLUSTER.getPreferredName().equals(innerKey)) { + assert innerValue instanceof List; + RemoteClusterPermissions discoveredRemoteClusterPermission = new RemoteClusterPermissions( + (List>>) innerValue + ); + RemoteClusterPermissions mutated = discoveredRemoteClusterPermission.removeUnsupportedPrivileges(outboundVersion); + if (mutated.equals(discoveredRemoteClusterPermission) == false) { + // swap out the old value with the new value modified.set(true); + Map remoteClusterMap = ((Map) roleDescriptorsMapMutated.get(key)); + remoteClusterMap.put(innerKey, mutated.toMap()); } - - } }); - - } }); - Iterator> it = roleDescriptorsMap.entrySet().iterator(); - Map.Entry next = it.next(); - System.out.println("******** next: " + next); - if (modified.get()) { -// Iterator> it = roleDescriptorsMap.entrySet().iterator(); -// Map.Entry next = it.next(); -// System.out.println("******** next: " + next - - - return convertRoleDescriptorsMapToBytes(roleDescriptorsMap); + return convertRoleDescriptorsMapToBytes(roleDescriptorsMapMutated); } else { - // No need to serialize if we did not remove anything. + // No need to serialize if we did not change anything. return roleDescriptorsBytes; } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 56c4552f57eaa..7ba42d30ea682 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -6,6 +6,8 @@ */ package org.elasticsearch.xpack.core.security.authz; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.TransportVersion; @@ -63,6 +65,7 @@ public class RoleDescriptor implements ToXContentObject, Writeable { public static final TransportVersion SECURITY_ROLE_DESCRIPTION = TransportVersions.V_8_15_0; public static final String ROLE_TYPE = "role"; + private static final Logger logger = LogManager.getLogger(RoleDescriptor.class); private final String name; private final String[] clusterPrivileges; @@ -835,22 +838,26 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), Arrays.stream(privileges).map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toSet()) )) { - throw new ElasticsearchParseException( - "failed to parse remote_cluster for role [{}]. " - + RemoteClusterPermissions.getSupportedRemoteClusterPermissions() - + " are the only values allowed for [{}] within [remote_cluster]", + final String message = String.format( + "failed to parse remote_cluster for role [%s]. " + + "[%s] are the only values allowed for [%s] within [remote_cluster]", roleName, + RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), currentFieldName ); + logger.warn(message); + throw new ElasticsearchParseException(message); } } else if (Fields.CLUSTERS.match(currentFieldName, parser.getDeprecationHandler())) { clusters = readStringArray(roleName, parser, false); } else { - throw new ElasticsearchParseException( - "failed to parse remote_cluster for role [{}]. unexpected field [{}]", + final String message = String.format( + "failed to parse remote_cluster for role [%s]. unexpected field [%s]", roleName, currentFieldName ); + logger.warn(message); + throw new ElasticsearchParseException(message); } } if (privileges != null && clusters == null) { From d711d1592195dfa4d7c31aa30b983c3a851db8c5 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Wed, 30 Oct 2024 12:34:09 -0500 Subject: [PATCH 20/43] clean up --- .../org/elasticsearch/TransportVersions.java | 4 +- .../core/security/authc/Authentication.java | 49 +++++------ .../core/security/authz/RoleDescriptor.java | 6 +- .../RemoteClusterPermissionGroup.java | 29 ++++--- .../permission/RemoteClusterPermissions.java | 87 +++++++++++++++---- .../store/RoleReferenceIntersection.java | 1 - .../RemoteClusterPermissionGroupTests.java | 7 +- .../RemoteClusterPermissionsTests.java | 2 +- ...ClusterSecurityBWCToRCS1ClusterRestIT.java | 4 +- 9 files changed, 121 insertions(+), 68 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 7f737ea8c8709..5686fb4bc4bef 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -185,9 +185,7 @@ static TransportVersion def(int id) { public static final TransportVersion INDEX_REQUEST_REMOVE_METERING = def(8_780_00_0); public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); - - - //FIXME: before merging this PR, make sure to update the transport version correctly + // FIXME: before merging this PR, make sure to update the transport version correctly public static final TransportVersion ROLE_MONITOR_STATS = def(8_882_00_0); /* diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index a1328dc6ca6f5..507b487eb4bf9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -25,11 +25,9 @@ import org.elasticsearch.core.Strings; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; -import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentType; -import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.security.action.profile.Profile; import org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo.RoleDescriptorsBytes; import org.elasticsearch.xpack.core.security.authc.RealmConfig.RealmIdentifier; @@ -46,12 +44,10 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.util.ArrayList; import java.util.Base64; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; @@ -239,8 +235,8 @@ public Authentication maybeRewriteForOlderVersion(TransportVersion olderVersion) + "]" ); } - final Map newMetadata = maybeRewriteMetadata(olderVersion, this); + final Authentication newAuthentication; if (isRunAs()) { // The lookup user for run-as currently doesn't have authentication metadata associated with them because @@ -278,12 +274,23 @@ public Authentication maybeRewriteForOlderVersion(TransportVersion olderVersion) } private static Map maybeRewriteMetadata(TransportVersion olderVersion, Authentication authentication) { - if (authentication.isAuthenticatedAsApiKey()) { - return maybeRewriteMetadataForApiKeyRoleDescriptors(olderVersion, authentication); - } else if (authentication.isCrossClusterAccess()) { - return maybeRewriteMetadataForCrossClusterAccessAuthentication(olderVersion, authentication); - } else { - return authentication.getAuthenticatingSubject().getMetadata(); + try { + if (authentication.isAuthenticatedAsApiKey()) { + return maybeRewriteMetadataForApiKeyRoleDescriptors(olderVersion, authentication); + } else if (authentication.isCrossClusterAccess()) { + return maybeRewriteMetadataForCrossClusterAccessAuthentication(olderVersion, authentication); + } else { + return authentication.getAuthenticatingSubject().getMetadata(); + } + } catch (Exception e) { + // CCS workflows may swallow the exception message making this difficult to troubleshoot, so we explicitly log and re-throw + // here. It may result in duplicate logs, so we only log the message at warn level. + if (logger.isDebugEnabled()) { + logger.debug("Un-expected exception thrown while rewriting metadata. This is likely a bug.", e); + } else { + logger.warn("Un-expected exception thrown while rewriting metadata. This is likely a bug [" + e.getMessage() + "]"); + } + throw e; } } @@ -1302,7 +1309,6 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( TransportVersion streamVersion, Authentication authentication ) { - System.out.println("*************** maybeRewriteMetadataForApiKeyRoleDescriptors ***************"); Map metadata = authentication.getAuthenticatingSubject().getMetadata(); // If authentication user is an API key or a token created by an API key, // regardless whether it has run-as, the metadata must contain API key role descriptors @@ -1344,29 +1350,23 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY) ) ); - } - - //TODO: this needs to be in the else so we don't add back the remote_cluster field - // the current cluster understands remote_cluster field in role descriptors, so check each permission and remove as needed - if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)){ - - - RemoteClusterPermissions.getSupportedRemoteClusterPermissions(); - + } else if (streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { + // the remote cluster understands remote_cluster field in role descriptors, so check each permission and remove as needed metadata = new HashMap<>(metadata); metadata.put( AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( - (BytesReference) metadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY), streamVersion + (BytesReference) metadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY), + streamVersion ) ); metadata.put( AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( - (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), streamVersion + (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), + streamVersion ) ); - } if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES) @@ -1495,7 +1495,6 @@ static BytesReference maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( if (roleDescriptorsBytes == null || roleDescriptorsBytes.length() == 0) { return roleDescriptorsBytes; } - final Map roleDescriptorsMap = convertRoleDescriptorsBytesToMap(roleDescriptorsBytes); final Map roleDescriptorsMapMutated = new HashMap<>(roleDescriptorsMap); final AtomicBoolean modified = new AtomicBoolean(false); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 7ba42d30ea682..e99068ca9be88 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -839,8 +839,9 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName Arrays.stream(privileges).map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toSet()) )) { final String message = String.format( - "failed to parse remote_cluster for role [%s]. " + - "[%s] are the only values allowed for [%s] within [remote_cluster]", + Locale.ROOT, + "failed to parse remote_cluster for role [%s]. " + + "[%s] are the only values allowed for [%s] within [remote_cluster]", roleName, RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), currentFieldName @@ -852,6 +853,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName clusters = readStringArray(roleName, parser, false); } else { final String message = String.format( + Locale.ROOT, "failed to parse remote_cluster for role [%s]. unexpected field [%s]", roleName, currentFieldName diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java index a580e3ee02c6c..ec245fae28612 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.support.StringMatcher; import java.io.IOException; @@ -46,12 +45,11 @@ public RemoteClusterPermissionGroup(StreamInput in) throws IOException { remoteClusterAliasMatcher = StringMatcher.of(remoteClusterAliases); } - - public RemoteClusterPermissionGroup(Map> remoteClusterGroup) { + public RemoteClusterPermissionGroup(Map> remoteClusterGroup) { assert remoteClusterGroup.get(PRIVILEGES.getPreferredName()) != null : "privileges must be non-null"; assert remoteClusterGroup.get(CLUSTERS.getPreferredName()) != null : "clusters must be non-null"; - clusterPrivileges = remoteClusterGroup.get(PRIVILEGES.getPreferredName()).toArray(new String[0]); - remoteClusterAliases = remoteClusterGroup.get(CLUSTERS.getPreferredName()).toArray(new String[0]); + clusterPrivileges = remoteClusterGroup.get(PRIVILEGES.getPreferredName()).toArray(new String[0]); + remoteClusterAliases = remoteClusterGroup.get(CLUSTERS.getPreferredName()).toArray(new String[0]); remoteClusterAliasMatcher = StringMatcher.of(remoteClusterAliases); } @@ -67,10 +65,14 @@ public RemoteClusterPermissionGroup(String[] clusterPrivileges, String[] remoteC throw new IllegalArgumentException("remote cluster groups must not be null or empty"); } if (Arrays.stream(clusterPrivileges).anyMatch(s -> Strings.hasText(s) == false)) { - throw new IllegalArgumentException("remote_cluster privileges must contain valid non-empty, non-null values"); + throw new IllegalArgumentException( + "remote_cluster privileges must contain valid non-empty, non-null values " + Arrays.toString(clusterPrivileges) + ); } if (Arrays.stream(remoteClusterAliases).anyMatch(s -> Strings.hasText(s) == false)) { - throw new IllegalArgumentException("remote_cluster clusters aliases must contain valid non-empty, non-null values"); + throw new IllegalArgumentException( + "remote_cluster clusters aliases must contain valid non-empty, non-null values " + Arrays.toString(remoteClusterAliases) + ); } this.clusterPrivileges = clusterPrivileges; @@ -100,10 +102,17 @@ public String[] remoteClusterAliases() { return Arrays.copyOf(remoteClusterAliases, remoteClusterAliases.length); } - + /** + * Converts the group to a map representation. + * @return A map representation of the group. + */ public Map> toMap() { - return Map.of(PRIVILEGES.getPreferredName(), Arrays.asList(clusterPrivileges), - CLUSTERS.getPreferredName(), Arrays.asList(remoteClusterAliases)); + return Map.of( + PRIVILEGES.getPreferredName(), + Arrays.asList(clusterPrivileges), + CLUSTERS.getPreferredName(), + Arrays.asList(remoteClusterAliases) + ); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 0a402e8a77a52..a29dfadda3fb7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -36,9 +36,10 @@ /** * Represents the set of permissions for remote clusters. This is intended to be the model for both the {@link RoleDescriptor} - * and {@link Role}. This model is not intended to be sent to a remote cluster, but can be (wire) serialized within a single cluster - * as well as the Xcontent serialization for the REST API and persistence of the role in the security index. The privileges modeled here - * will be converted to the appropriate cluster privileges when sent to a remote cluster. + * and {@link Role}. This model is not intended to support RCS 2.0 where these remote cluster permissions are converted to local cluster + * permissions {@link #privilegeNames(String, TransportVersion)} ) before sent to the remote cluster. This model also supports RCS 1.0 where + * these permissions are sent to the remote API keys {@link #toMap()}. In both cases the outbound transport version will be used to + * remove permissions that are not available to elder remote clusters. * For example, on the local/querying cluster this model represents the following: * * "remote_cluster" : [ @@ -52,15 +53,18 @@ * } * ] * - * when sent to the remote cluster "clusterA", the privileges will be converted to the appropriate cluster privileges. For example: + * (RCS 2.0) when sent to the remote cluster "clusterA", the privileges will be converted to the appropriate cluster privileges. + * For example: * * "cluster": ["foo"] * - * and when sent to the remote cluster "clusterB", the privileges will be converted to the appropriate cluster privileges. For example: + * and (RCS 2.0) when sent to the remote cluster "clusterB", the privileges will be converted to the appropriate cluster privileges. + * For example: * * "cluster": ["bar"] * - * If the remote cluster does not support the privilege, as determined by the remote cluster version, the privilege will be not be sent. + * (RCS 1.0 and RCS 2.0) If the remote cluster does not support the privilege, as determined by the remote cluster version, + * the privilege will be not be sent. Upstream code performs the removal, but this class owns the business logic for how to remove. */ public class RemoteClusterPermissions implements NamedWriteable, ToXContentObject { @@ -88,7 +92,7 @@ public RemoteClusterPermissions(StreamInput in) throws IOException { remoteClusterPermissionGroups = in.readNamedWriteableCollectionAsList(RemoteClusterPermissionGroup.class); } - public RemoteClusterPermissions(List>> remoteClusters){ + public RemoteClusterPermissions(List>> remoteClusters) { remoteClusterPermissionGroups = new ArrayList<>(); for (Map> remoteCluster : remoteClusters) { RemoteClusterPermissionGroup remoteClusterPermissionGroup = new RemoteClusterPermissionGroup(remoteCluster); @@ -109,27 +113,69 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus return this; } + /** + * Will remove any unsupported privileges for the provided outbound version. This method will not modify the current instance. + * This is useful for RCS 1.0 and API keys to help ensure that we don't send unsupported privileges to the remote cluster. + * @param outboundVersion The version by which to remove unsupported privileges, this is typically the version of the remote cluster + * @return a new instance of RemoteClusterPermissions with the unsupported privileges removed + */ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion outboundVersion) { - System.out.println("************ [DEBUG] RemoteClusterPermissions.removeUnsupportedPrivileges"); + RemoteClusterPermissions copyForOutBoundVersion = new RemoteClusterPermissions(); + + // TODO: centralize this and put in a poor mans cache + // find all the privileges that are allowed for the remote cluster version + Set allowedPermissionsPerVersion = allowedRemoteClusterPermissions.entrySet() + .stream() + .filter((entry) -> entry.getKey().onOrBefore(outboundVersion)) + .map(Map.Entry::getValue) + .flatMap(Set::stream) + .map(s -> s.toLowerCase(Locale.ROOT)) + .collect(Collectors.toSet()); + for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) { - //TODO: actually implement this method, this is a hack to make the test pass for now - if(outboundVersion.onOrBefore(ROLE_MONITOR_STATS)){ - Set a = new HashSet<>(Arrays.asList(group.clusterPrivileges())); - a.remove("monitor_stats"); - copyForOutBoundVersion.addGroup( - new RemoteClusterPermissionGroup(a.toArray(new String[0]), group.remoteClusterAliases()) + String[] privileges = group.clusterPrivileges(); + List privilegesMutated = new ArrayList<>(privileges.length); + for (String privilege : privileges) { + if (allowedPermissionsPerVersion.contains(privilege.toLowerCase(Locale.ROOT))) { + privilegesMutated.add(privilege); + } + } + if (privilegesMutated.isEmpty() == false) { + RemoteClusterPermissionGroup mutatedGroup = new RemoteClusterPermissionGroup( + privilegesMutated.toArray(new String[0]), + group.remoteClusterAliases() ); - }else{ - copyForOutBoundVersion.addGroup(group); + copyForOutBoundVersion.addGroup(mutatedGroup); + if (logger.isDebugEnabled()) { + if (group.equals(mutatedGroup) == false) { + logger.debug( + "Removed unsupported remote cluster permissions {} for remote cluster [{}]. " + + "Due to the remote cluster version, only the following permissions are allowed: {}", + privilegesMutated, + group.remoteClusterAliases(), + allowedPermissionsPerVersion + ); + } + } + } else { + if (logger.isDebugEnabled()) { + logger.debug( + "Removed all remote cluster permissions for remote cluster [{}]. " + + "Due to the remote cluster version, only the following permissions are allowed: {}", + group.remoteClusterAliases(), + allowedPermissionsPerVersion + ); + } } } return copyForOutBoundVersion; } /** - * Gets the privilege names for the remote cluster. This method will collapse all groups to single String[] all lowercase - * and will only return the appropriate privileges for the provided remote cluster version. + * Gets all the privilege names for the remote cluster. This method will collapse all groups to single String[] all lowercase + * and will only return the appropriate privileges for the provided remote cluster version. This is useful for RCS 2.0 to ensure + * that we properly convert all the remote_cluster -> cluster privileges per remote cluster. */ public String[] privilegeNames(final String remoteClusterAlias, TransportVersion remoteClusterVersion) { @@ -168,6 +214,10 @@ public String[] privilegeNames(final String remoteClusterAlias, TransportVersion return allowedPrivileges.stream().sorted().toArray(String[]::new); } + /** + * Converts this object to it's {@link Map} representation. + * @return a list of maps representing the remote cluster permissions + */ public List>> toMap() { return remoteClusterPermissionGroups.stream().map(RemoteClusterPermissionGroup::toMap).toList(); } @@ -256,5 +306,4 @@ public String getWriteableName() { return NAME; } - } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java index ec5e415d13497..86a56a30fc348 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleReferenceIntersection.java @@ -36,7 +36,6 @@ public List getRoleReferences() { return roleReferences; } - public void buildRole(BiConsumer> singleRoleBuilder, ActionListener roleActionListener) { final GroupedActionListener roleGroupedActionListener = new GroupedActionListener<>( roleReferences.size(), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java index 5662158917ad5..276d1e9e93657 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java @@ -104,15 +104,12 @@ public void testInvalidValues() { assertEquals("remote_cluster privileges must contain valid non-empty, non-null values", e2.getMessage()); } - public void testToMap(){ + public void testToMap() { String[] privileges = generateRandomStringArray(5, 5, false, false); String[] clusters = generateRandomStringArray(5, 5, false, false); RemoteClusterPermissionGroup remoteClusterPermissionGroup = new RemoteClusterPermissionGroup(privileges, clusters); assertEquals( - Map.of( - "privileges", Arrays.asList(privileges), - "clusters", Arrays.asList(clusters) - ), + Map.of("privileges", Arrays.asList(privileges), "clusters", Arrays.asList(clusters)), remoteClusterPermissionGroup.toMap() ); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index 0296c804d41c2..b1654a9c86777 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -197,7 +197,7 @@ public void testValidate() { .validate(); // no error } - public void testToMap(){ + public void testToMap() { RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); List groups = generateRandomGroups(randomBoolean()); for (int i = 0; i < groups.size(); i++) { diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java index 04d4627770bee..0e9a22299ed67 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java @@ -56,8 +56,8 @@ public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteC .name("fulfilling-cluster") .apply(commonClusterConfig) .setting("xpack.ml.enabled", "false") - // .setting("logger.org.elasticsearch.xpack.core", "trace") //useful for human debugging - // .setting("logger.org.elasticsearch.xpack.security", "trace") //useful for human debugging + // .setting("logger.org.elasticsearch.xpack.core", "trace") //useful for human debugging + // .setting("logger.org.elasticsearch.xpack.security", "trace") //useful for human debugging .build(); queryCluster = ElasticsearchCluster.local() From b4cce77ad4e830f80f951948fbcb149dc2e0b175 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 31 Oct 2024 12:02:52 -0500 Subject: [PATCH 21/43] fix tests --- .../org/elasticsearch/TransportVersions.java | 4 +- .../core/security/authc/Authentication.java | 40 ++++++++++--------- .../core/security/authz/RoleDescriptor.java | 2 +- .../RemoteClusterPermissionGroupTests.java | 4 +- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 3edcc73850b1b..c3026293f31b3 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -186,9 +186,7 @@ static TransportVersion def(int id) { public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); public static final TransportVersion QUERY_RULES_RETRIEVER = def(8_782_00_0); public static final TransportVersion ESQL_CCS_EXEC_INFO_WITH_FAILURES = def(8_783_00_0); - - // FIXME: before merging this PR, make sure to update the transport version correctly - public static final TransportVersion ROLE_MONITOR_STATS = def(8_882_00_0); + public static final TransportVersion ROLE_MONITOR_STATS = def(8_784_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 507b487eb4bf9..fa0c98cafa85b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -1350,24 +1350,28 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY) ) ); - } else if (streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { - // the remote cluster understands remote_cluster field in role descriptors, so check each permission and remove as needed - metadata = new HashMap<>(metadata); - metadata.put( - AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, - maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( - (BytesReference) metadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY), - streamVersion - ) - ); - metadata.put( - AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, - maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( - (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), - streamVersion - ) - ); - } + } else if (authentication.getEffectiveSubject() + .getTransportVersion() + .onOrAfter(TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY) + && streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { + // the remote cluster understands remote_cluster field in role descriptors, so check each permission and remove as + // needed + metadata = new HashMap<>(metadata); + metadata.put( + AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, + maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( + (BytesReference) metadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY), + streamVersion + ) + ); + metadata.put( + AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, + maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( + (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), + streamVersion + ) + ); + } if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES) && streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index e99068ca9be88..3f693d6492879 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -841,7 +841,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName final String message = String.format( Locale.ROOT, "failed to parse remote_cluster for role [%s]. " - + "[%s] are the only values allowed for [%s] within [remote_cluster]", + + "%s are the only values allowed for [%s] within [remote_cluster]", roleName, RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), currentFieldName diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java index 276d1e9e93657..0b99db826d540 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroupTests.java @@ -91,7 +91,7 @@ public void testInvalidValues() { ); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, invalidClusterAlias); - assertEquals("remote_cluster clusters aliases must contain valid non-empty, non-null values", e.getMessage()); + assertThat(e.getMessage(), containsString("remote_cluster clusters aliases must contain valid non-empty, non-null values")); final ThrowingRunnable invalidPermission = randomFrom( () -> new RemoteClusterPermissionGroup(new String[] { null }, new String[] { "bar" }), @@ -101,7 +101,7 @@ public void testInvalidValues() { ); IllegalArgumentException e2 = expectThrows(IllegalArgumentException.class, invalidPermission); - assertEquals("remote_cluster privileges must contain valid non-empty, non-null values", e2.getMessage()); + assertThat(e2.getMessage(), containsString("remote_cluster privileges must contain valid non-empty, non-null values")); } public void testToMap() { From 531fe9ce44c9e16da66018490a08d0719526552e Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 31 Oct 2024 13:42:36 -0500 Subject: [PATCH 22/43] better support to strip privs --- .../org/elasticsearch/TransportVersions.java | 2 +- .../core/security/authc/Authentication.java | 26 ++++-- .../security/authc/AuthenticationTests.java | 93 ++++++++++++++++++- .../RemoteClusterPermissionsTests.java | 26 ++++++ 4 files changed, 137 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index c3026293f31b3..53c7eb40569fd 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -186,7 +186,7 @@ static TransportVersion def(int id) { public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); public static final TransportVersion QUERY_RULES_RETRIEVER = def(8_782_00_0); public static final TransportVersion ESQL_CCS_EXEC_INFO_WITH_FAILURES = def(8_783_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_784_00_0); + public static final TransportVersion ROLE_MONITOR_STATS = def(8_884_00_0); //FIXME: before merge ensure this is correct /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index fa0c98cafa85b..1181ad3d9469c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -1334,9 +1334,9 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( ); } - // removes the entire remote_cluster field from the role descriptors if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) && streamVersion.before(ROLE_REMOTE_CLUSTER_PRIVS)) { + // if the remote does not understand the remote_cluster field remove it metadata = new HashMap<>(metadata); metadata.put( AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, @@ -1354,8 +1354,7 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( .getTransportVersion() .onOrAfter(TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY) && streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { - // the remote cluster understands remote_cluster field in role descriptors, so check each permission and remove as - // needed + // the remote does understand the remote_cluster field, so check each permission and remove permission as needed metadata = new HashMap<>(metadata); metadata.put( AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, @@ -1487,7 +1486,7 @@ static BytesReference maybeRemoveTopLevelFromRoleDescriptors(BytesReference role /** * Before we send the role descriptors to the remote cluster, we need to remove the remote cluster privileges that the other cluster - * will not understand. + * will not understand. If all privileges are removed, then the entire "remote_cluster" is removed to avoid sending empty privileges. * @param roleDescriptorsBytes The role descriptors to be sent to the remote cluster, represented as bytes. * @return The role descriptors with the privileges that unsupported by version removed, represented as bytes. */ @@ -1516,18 +1515,31 @@ static BytesReference maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( if (mutated.equals(discoveredRemoteClusterPermission) == false) { // swap out the old value with the new value modified.set(true); - Map remoteClusterMap = ((Map) roleDescriptorsMapMutated.get(key)); - remoteClusterMap.put(innerKey, mutated.toMap()); + Map remoteClusterMap = new HashMap<>((Map) roleDescriptorsMapMutated.get(key)); + if(mutated.hasPrivileges()){ + //has at least one group with privileges + remoteClusterMap.put(innerKey, mutated.toMap()); + }else { + //has no groups with privileges + remoteClusterMap.remove(innerKey); + } + roleDescriptorsMapMutated.put(key, remoteClusterMap); } } }); } }); - if (modified.get()) { + logger.debug( + "mutated role descriptors. Changed from {} to {} for outbound version {}", + roleDescriptorsMap, + roleDescriptorsMapMutated, + outboundVersion + ); return convertRoleDescriptorsMapToBytes(roleDescriptorsMapMutated); } else { // No need to serialize if we did not change anything. + logger.trace("no change to role descriptors {} for outbound version {}", roleDescriptorsMap, outboundVersion); return roleDescriptorsBytes; } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java index 66e246d1c8a50..085b7dc64635d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.core.security.authc; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.TransportVersion; import org.elasticsearch.TransportVersions; import org.elasticsearch.common.bytes.BytesArray; @@ -21,34 +23,43 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.TransportVersionUtils; import org.elasticsearch.transport.RemoteClusterPortSettings; +import org.elasticsearch.xcontent.ObjectPath; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef; import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings; import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings; import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings; import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptorsIntersection; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.User; +import org.hamcrest.Matchers; import java.io.IOException; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; import static java.util.Map.entry; +import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; +import static org.elasticsearch.xpack.core.security.authc.Authentication.VERSION_API_KEY_ROLES_AS_BYTES; import static org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper.randomCrossClusterAccessSubjectInfo; import static org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfoTests.randomRoleDescriptorsIntersection; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -1070,7 +1081,7 @@ public void testMaybeRewriteMetadataForApiKeyRoleDescriptorsWithRemoteIndices() // pick a version before that of the authentication instance to force a rewrite final TransportVersion olderVersion = TransportVersionUtils.randomVersionBetween( random(), - Authentication.VERSION_API_KEY_ROLES_AS_BYTES, + VERSION_API_KEY_ROLES_AS_BYTES, TransportVersionUtils.getPreviousVersion(original.getEffectiveSubject().getTransportVersion()) ); @@ -1115,7 +1126,7 @@ public void testMaybeRewriteMetadataForApiKeyRoleDescriptorsWithRemoteCluster() // pick a version before that of the authentication instance to force a rewrite final TransportVersion olderVersion = TransportVersionUtils.randomVersionBetween( random(), - Authentication.VERSION_API_KEY_ROLES_AS_BYTES, + VERSION_API_KEY_ROLES_AS_BYTES, TransportVersionUtils.getPreviousVersion(original.getEffectiveSubject().getTransportVersion()) ); @@ -1135,6 +1146,84 @@ public void testMaybeRewriteMetadataForApiKeyRoleDescriptorsWithRemoteCluster() ); } + public void testMaybeRewriteMetadataForApiKeyRoleDescriptorsWithRemoteClusterRemovePrivs() throws IOException { + final String apiKeyId = randomAlphaOfLengthBetween(1, 10); + final String apiKeyName = randomAlphaOfLengthBetween(1, 10); + Map metadata = Map.ofEntries( + entry(AuthenticationField.API_KEY_ID_KEY, apiKeyId), + entry(AuthenticationField.API_KEY_NAME_KEY, apiKeyName), + entry(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, new BytesArray(""" + {"base_role":{"cluster":["all"], + "remote_cluster":[{"privileges":["monitor_enrich", "monitor_stats"],"clusters":["*"]}] + }}""")), + entry(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray(""" + {"limited_by_role":{"cluster":["*"], + "remote_cluster":[{"privileges":["monitor_enrich", "monitor_stats"],"clusters":["*"]}] + }}""")) + ); + + final Authentication with2privs = AuthenticationTestHelper.builder() + .apiKey() + .metadata(metadata) + .transportVersion(TransportVersion.current()) + .build(); + + // pick a version that will only remove one of the two privileges + final TransportVersion olderVersion = TransportVersionUtils.randomVersionBetween( + random(), + ROLE_REMOTE_CLUSTER_PRIVS, + ROLE_MONITOR_STATS + ); + + Map rewrittenMetadata = with2privs.maybeRewriteForOlderVersion(olderVersion).getEffectiveSubject().getMetadata(); + assertThat(rewrittenMetadata.keySet(), equalTo(with2privs.getAuthenticatingSubject().getMetadata().keySet())); + + // only one of the two privileges are left after the rewrite + BytesReference baseRoleBytes = (BytesReference) rewrittenMetadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY); + Map baseRoleAsMap = XContentHelper.convertToMap(baseRoleBytes, false, XContentType.JSON).v2(); + assertThat(ObjectPath.eval("base_role.remote_cluster.0.privileges", baseRoleAsMap), Matchers.contains("monitor_enrich")); + assertThat(ObjectPath.eval("base_role.remote_cluster.0.clusters", baseRoleAsMap), notNullValue()); + BytesReference limitedByRoleBytes = (BytesReference) rewrittenMetadata.get( + AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY + ); + Map limitedByRoleAsMap = XContentHelper.convertToMap(limitedByRoleBytes, false, XContentType.JSON).v2(); + assertThat(ObjectPath.eval("limited_by_role.remote_cluster.0.privileges", limitedByRoleAsMap), Matchers.contains("monitor_enrich")); + assertThat(ObjectPath.eval("limited_by_role.remote_cluster.0.clusters", limitedByRoleAsMap), notNullValue()); + + // same version, but it removes the only defined privilege + metadata = Map.ofEntries( + entry(AuthenticationField.API_KEY_ID_KEY, apiKeyId), + entry(AuthenticationField.API_KEY_NAME_KEY, apiKeyName), + entry(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, new BytesArray(""" + {"base_role":{"cluster":["all"], + "remote_cluster":[{"privileges":["monitor_stats"],"clusters":["*"]}] + }}""")), + entry(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, new BytesArray(""" + {"limited_by_role":{"cluster":["*"], + "remote_cluster":[{"privileges":["monitor_stats"],"clusters":["*"]}] + }}""")) + ); + + final Authentication with1priv = AuthenticationTestHelper.builder() + .apiKey() + .metadata(metadata) + .transportVersion(TransportVersion.current()) + .build(); + + rewrittenMetadata = with1priv.maybeRewriteForOlderVersion(olderVersion).getEffectiveSubject().getMetadata(); + assertThat(rewrittenMetadata.keySet(), equalTo(with1priv.getAuthenticatingSubject().getMetadata().keySet())); + + // the one privileges is removed after the rewrite, which removes the full "remote_cluster" object + baseRoleBytes = (BytesReference) rewrittenMetadata.get(AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY); + baseRoleAsMap = XContentHelper.convertToMap(baseRoleBytes, false, XContentType.JSON).v2(); + assertThat(ObjectPath.eval("base_role.remote_cluster", baseRoleAsMap), nullValue()); + assertThat(ObjectPath.eval("base_role.cluster", baseRoleAsMap), notNullValue()); + limitedByRoleBytes = (BytesReference) rewrittenMetadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY); + limitedByRoleAsMap = XContentHelper.convertToMap(limitedByRoleBytes, false, XContentType.JSON).v2(); + assertThat(ObjectPath.eval("limited_by_role.remote_cluster", limitedByRoleAsMap), nullValue()); + assertThat(ObjectPath.eval("limited_by_role.cluster", limitedByRoleAsMap), notNullValue()); + } + public void testMaybeRemoveRemoteIndicesFromRoleDescriptors() { final boolean includeClusterPrivileges = randomBoolean(); final BytesReference roleWithoutRemoteIndices = new BytesArray(Strings.format(""" diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index b1654a9c86777..e5855e6f8a78b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -32,6 +32,7 @@ import java.util.stream.Collectors; import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; +import static org.elasticsearch.core.Booleans.isFalse; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -208,6 +209,31 @@ public void testToMap() { assertEquals(remoteClusterPermissions, remoteClusterPermissionsAsMap); } + public void testRemoveUnsupportedPrivilegesExplicit() { + +// RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); +// RemoteClusterPermissionGroup group = new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" }); +// remoteClusterPermissions.addGroup(group); +// assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); +// assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); +// +// remoteClusterPermissions = new RemoteClusterPermissions(); +// group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" }); +// remoteClusterPermissions.addGroup(group); +// assertNotEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); +// assertFalse(remoteClusterPermissions.hasPrivileges()); +// assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); +// +// +// +//// RemoteClusterPermissions allowed = new RemoteClusterPermissions().addGroup(group); +//// RemoteClusterPermissions removed = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" })); +//// assertEquals(allowed, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); +//// assertEquals(removed, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); +// +// +// //Randomized test for all privileges and versions + } private List generateRandomGroups(boolean fuzzyCluster) { clean(); List groups = new ArrayList<>(); From 2cde2f89b7b7d29e670d3736ac9f4e0312a66e70 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 31 Oct 2024 14:26:50 -0500 Subject: [PATCH 23/43] mo tests --- .../org/elasticsearch/TransportVersions.java | 2 +- .../core/security/authc/Authentication.java | 8 +-- .../security/authc/AuthenticationTests.java | 7 --- .../RemoteClusterPermissionsTests.java | 54 ++++++++++--------- 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 53c7eb40569fd..97d1535c0749c 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -186,7 +186,7 @@ static TransportVersion def(int id) { public static final TransportVersion CPU_STAT_STRING_PARSING = def(8_781_00_0); public static final TransportVersion QUERY_RULES_RETRIEVER = def(8_782_00_0); public static final TransportVersion ESQL_CCS_EXEC_INFO_WITH_FAILURES = def(8_783_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_884_00_0); //FIXME: before merge ensure this is correct + public static final TransportVersion ROLE_MONITOR_STATS = def(8_884_00_0); // FIXME: before merge ensure this is correct /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 1181ad3d9469c..0657d64e41f70 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -1516,11 +1516,11 @@ static BytesReference maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( // swap out the old value with the new value modified.set(true); Map remoteClusterMap = new HashMap<>((Map) roleDescriptorsMapMutated.get(key)); - if(mutated.hasPrivileges()){ - //has at least one group with privileges + if (mutated.hasPrivileges()) { + // has at least one group with privileges remoteClusterMap.put(innerKey, mutated.toMap()); - }else { - //has no groups with privileges + } else { + // has no groups with privileges remoteClusterMap.remove(innerKey); } roleDescriptorsMapMutated.put(key, remoteClusterMap); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java index 085b7dc64635d..cb08744773da1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java @@ -7,8 +7,6 @@ package org.elasticsearch.xpack.core.security.authc; -import org.apache.lucene.util.BytesRef; -import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.TransportVersion; import org.elasticsearch.TransportVersions; import org.elasticsearch.common.bytes.BytesArray; @@ -26,14 +24,12 @@ import org.elasticsearch.xcontent.ObjectPath; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; -import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef; import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings; import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings; import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings; import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptorsIntersection; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.User; @@ -42,7 +38,6 @@ import java.io.IOException; import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Consumer; @@ -54,12 +49,10 @@ import static org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper.randomCrossClusterAccessSubjectInfo; import static org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfoTests.randomRoleDescriptorsIntersection; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; -import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; -import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index e5855e6f8a78b..a1c17448b0ed8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -32,7 +32,6 @@ import java.util.stream.Collectors; import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; -import static org.elasticsearch.core.Booleans.isFalse; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -209,31 +208,38 @@ public void testToMap() { assertEquals(remoteClusterPermissions, remoteClusterPermissionsAsMap); } - public void testRemoveUnsupportedPrivilegesExplicit() { + public void testRemoveUnsupportedPrivileges() { + RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); + RemoteClusterPermissionGroup group = new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" }); + remoteClusterPermissions.addGroup(group); + // this privilege is allowed by versions, so nothing should be removed + assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); + assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); + + remoteClusterPermissions = new RemoteClusterPermissions(); + group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" }); + remoteClusterPermissions.addGroup(group); + // this single newer privilege is not allowed in the older version, so it should result in an object with no groups + assertNotEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); + assertFalse(remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS).hasPrivileges()); + assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); -// RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); -// RemoteClusterPermissionGroup group = new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" }); -// remoteClusterPermissions.addGroup(group); -// assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); -// assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); -// -// remoteClusterPermissions = new RemoteClusterPermissions(); -// group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" }); -// remoteClusterPermissions.addGroup(group); -// assertNotEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); -// assertFalse(remoteClusterPermissions.hasPrivileges()); -// assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); -// -// -// -//// RemoteClusterPermissions allowed = new RemoteClusterPermissions().addGroup(group); -//// RemoteClusterPermissions removed = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" })); -//// assertEquals(allowed, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); -//// assertEquals(removed, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); -// -// -// //Randomized test for all privileges and versions + int groupCount = randomIntBetween(1, 5); + remoteClusterPermissions = new RemoteClusterPermissions(); + group = new RemoteClusterPermissionGroup(new String[] { "monitor_enrich", "monitor_stats" }, new String[] { "*" }); + for (int i = 0; i < groupCount; i++) { + remoteClusterPermissions.addGroup(group); + } + // one of the newer privilege is not allowed in the older version, so it should result in as group with only the allowed privilege + RemoteClusterPermissions expected = new RemoteClusterPermissions(); + for (int i = 0; i < groupCount; i++) { + expected.addGroup(new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" })); + } + assertEquals(expected, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); + // both privileges allowed in the newer version, so it should not change the permission + assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); } + private List generateRandomGroups(boolean fuzzyCluster) { clean(); List groups = new ArrayList<>(); From 633aebe749fe00c34dd6da1de235b20678a967c9 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 31 Oct 2024 17:07:36 -0500 Subject: [PATCH 24/43] update test --- .../permission/RemoteClusterPermissions.java | 43 ++++++++---------- ...usterApiKeyRoleDescriptorBuilderTests.java | 45 ++++++++++++++++--- .../xpack/security/authc/ApiKeyService.java | 2 - 3 files changed, 57 insertions(+), 33 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index a29dfadda3fb7..807af32185815 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -123,16 +123,7 @@ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion out RemoteClusterPermissions copyForOutBoundVersion = new RemoteClusterPermissions(); - // TODO: centralize this and put in a poor mans cache - // find all the privileges that are allowed for the remote cluster version - Set allowedPermissionsPerVersion = allowedRemoteClusterPermissions.entrySet() - .stream() - .filter((entry) -> entry.getKey().onOrBefore(outboundVersion)) - .map(Map.Entry::getValue) - .flatMap(Set::stream) - .map(s -> s.toLowerCase(Locale.ROOT)) - .collect(Collectors.toSet()); - + Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(outboundVersion); for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) { String[] privileges = group.clusterPrivileges(); List privilegesMutated = new ArrayList<>(privileges.length); @@ -159,14 +150,12 @@ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion out } } } else { - if (logger.isDebugEnabled()) { - logger.debug( - "Removed all remote cluster permissions for remote cluster [{}]. " - + "Due to the remote cluster version, only the following permissions are allowed: {}", - group.remoteClusterAliases(), - allowedPermissionsPerVersion - ); - } + logger.debug( + "Removed all remote cluster permissions for remote cluster [{}]. " + + "Due to the remote cluster version, only the following permissions are allowed: {}", + group.remoteClusterAliases(), + allowedPermissionsPerVersion + ); } } return copyForOutBoundVersion; @@ -188,13 +177,7 @@ public String[] privilegeNames(final String remoteClusterAlias, TransportVersion .collect(Collectors.toSet()); // find all the privileges that are allowed for the remote cluster version - Set allowedPermissionsPerVersion = allowedRemoteClusterPermissions.entrySet() - .stream() - .filter((entry) -> entry.getKey().onOrBefore(remoteClusterVersion)) - .map(Map.Entry::getValue) - .flatMap(Set::stream) - .map(s -> s.toLowerCase(Locale.ROOT)) - .collect(Collectors.toSet()); + Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(remoteClusterVersion); // intersect the two sets to get the allowed privileges for the remote cluster version Set allowedPrivileges = new HashSet<>(groupPrivileges); @@ -270,6 +253,16 @@ public List groups() { return Collections.unmodifiableList(remoteClusterPermissionGroups); } + private Set getAllowedPermissionsPerVersion(TransportVersion outboundVersion) { + return allowedRemoteClusterPermissions.entrySet() + .stream() + .filter((entry) -> entry.getKey().onOrBefore(outboundVersion)) + .map(Map.Entry::getValue) + .flatMap(Set::stream) + .map(s -> s.toLowerCase(Locale.ROOT)) + .collect(Collectors.toSet()); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { for (RemoteClusterPermissionGroup remoteClusterPermissionGroup : remoteClusterPermissionGroups) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java index e4864291793fb..f0db44070d2c4 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java @@ -10,11 +10,16 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.core.Strings; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.json.JsonXContent; +import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import java.io.IOException; import java.util.List; @@ -27,6 +32,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mock; public class CrossClusterApiKeyRoleDescriptorBuilderTests extends ESTestCase { @@ -355,13 +361,40 @@ public void testEmptyAccessIsNotAllowed() throws IOException { assertThat(e2.getMessage(), containsString("doesn't support values of type: VALUE_NULL")); } - // TODO: create automaton and test that the permissions are supported instead of checking the names directly - @AwaitsFix(bugUrl = "http://example.com/do/not/merge/me") public void testAPIKeyAllowsAllRemoteClusterPrivilegesForCCS() { - // if users can add remote cluster permissions to a role, then the APIKey should also allow that for that permission - // the inverse however, is not guaranteed. cross_cluster_search exists largely for internal use and is not exposed to the users role - // TODO: create automaton and test that the permissions are supported instead of checking the names directly. - assertTrue(Set.of(CCS_CLUSTER_PRIVILEGE_NAMES).containsAll(RemoteClusterPermissions.getSupportedRemoteClusterPermissions())); + // test to help ensure that at least 1 action that is allowed by the remote cluster permissions are supported by CCS + List actionsToTest = List.of("cluster:monitor/xpack/enrich/esql/resolve_policy", "cluster:monitor/stats/remote"); + // if you add new remote cluster permissions, please define an action we can test to help ensure it is supported by RCS 2.0 + assertThat(actionsToTest.size(), equalTo(RemoteClusterPermissions.getSupportedRemoteClusterPermissions().size())); + + for (String privilege : RemoteClusterPermissions.getSupportedRemoteClusterPermissions()) { + boolean actionPassesRemoteClusterPermissionCheck = false; + ClusterPrivilege clusterPrivilege = ClusterPrivilegeResolver.resolve(privilege); + // each remote cluster privilege has an action to test + for (String action : actionsToTest) { + if (clusterPrivilege.buildPermission(ClusterPermission.builder()) + .build() + .check(action, mock(TransportRequest.class), AuthenticationTestHelper.builder().build())) { + actionPassesRemoteClusterPermissionCheck = true; + break; + } + } + assertTrue(actionPassesRemoteClusterPermissionCheck); + } + // test that the actions pass the privilege check for CCS + for (String privilege : Set.of(CCS_CLUSTER_PRIVILEGE_NAMES)) { + boolean actionPassesRemoteCCSCheck = false; + ClusterPrivilege clusterPrivilege = ClusterPrivilegeResolver.resolve(privilege); + for (String action : actionsToTest) { + if (clusterPrivilege.buildPermission(ClusterPermission.builder()) + .build() + .check(action, mock(TransportRequest.class), AuthenticationTestHelper.builder().build())) { + actionPassesRemoteCCSCheck = true; + break; + } + } + assertTrue(actionPassesRemoteCCSCheck); + } } private static void assertRoleDescriptor( 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 index 4da5cb4b45f6b..03558e72fdca3 100644 --- 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 @@ -831,8 +831,6 @@ static Set maybeRemoveRemotePrivileges( + ". Remote cluster privileges are not supported by all nodes in the cluster." ); } - // TODO: support the additional cases where we are trying to send to somethign like 8.16 that understands remote cluster, - // but does not support the new privilege } return result; } From abe85e0bcae155ab3b1b8e81299ad33d2a2cc3df Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 4 Nov 2024 09:58:47 -0600 Subject: [PATCH 25/43] fix test --- .../org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java index 667140b849951..8ce7fc77fe4f3 100644 --- a/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java +++ b/x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java @@ -828,7 +828,7 @@ public void testRemoteClusterSupportForApiKeys() throws IOException { assertOK(response); assertAPIKeyWithRemoteClusterPermissions(apiKeyId, includeRemoteCluster, false, null, new String[] { "foo", "bar" }); - // create API key as the remote user which does remote_cluster limited_by permissions + // create API key as the remote user which has all remote_cluster permissions via limited_by response = sendRequestAsRemoteUser(createApiKeyRequest); apiKeyId = ObjectPath.createFromResponse(response).evaluate("id"); assertThat(apiKeyId, notNullValue()); @@ -922,7 +922,7 @@ private void assertAPIKeyWithRemoteClusterPermissions( assertNotNull(limitedByRole); List>> remoteCluster = (List>>) limitedByRole.get("remote_cluster"); - assertThat(remoteCluster.get(0).get("privileges"), containsInAnyOrder("monitor_enrich")); + assertThat(remoteCluster.get(0).get("privileges"), containsInAnyOrder("monitor_stats", "monitor_enrich")); assertThat(remoteCluster.get(0).get("clusters"), containsInAnyOrder("remote")); } else { // no limited by permissions From 826427948b6c267211ea74d7c598421f48e998ee Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 4 Nov 2024 11:21:50 -0600 Subject: [PATCH 26/43] introduce RCS2 BWC test and DRY --- .../org/elasticsearch/TransportVersions.java | 2 +- .../security/qa/multi-cluster/build.gradle | 3 +- .../AbstractRemoteClusterSecurityBWCTest.java | 247 ++++++++++++++++++ ...ClusterSecurityBWCToRCS1ClusterRestIT.java | 219 +--------------- ...ClusterSecurityBWCToRCS2ClusterRestIT.java | 80 ++++++ 5 files changed, 333 insertions(+), 218 deletions(-) create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java create mode 100644 x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index ef745c8873ae2..e7a433cfee81f 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -187,7 +187,7 @@ static TransportVersion def(int id) { public static final TransportVersion QUERY_RULES_RETRIEVER = def(8_782_00_0); public static final TransportVersion ESQL_CCS_EXEC_INFO_WITH_FAILURES = def(8_783_00_0); public static final TransportVersion LOGSDB_TELEMETRY = def(8_784_00_0); - public static final TransportVersion ROLE_MONITOR_STATS = def(8_785_00_0); + public static final TransportVersion ROLE_MONITOR_STATS = def(8_785_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/x-pack/plugin/security/qa/multi-cluster/build.gradle b/x-pack/plugin/security/qa/multi-cluster/build.gradle index fbd1887da38c9..b8eccb14819a4 100644 --- a/x-pack/plugin/security/qa/multi-cluster/build.gradle +++ b/x-pack/plugin/security/qa/multi-cluster/build.gradle @@ -32,13 +32,14 @@ tasks.named("javaRestTest") { enabled = true // This is tested explicitly in bwc test tasks. exclude '**/RemoteClusterSecurityBWCToRCS1ClusterRestIT.class' + exclude '**/RemoteClusterSecurityBWCToRCS2ClusterRestIT.class' } -//TODO: not sure if this works or not... can't run tests atm due to the issue with GH BuildParams.bwcVersions.withWireCompatible() { bwcVersion, baseName -> tasks.register(bwcTaskName(bwcVersion), StandaloneRestIntegTestTask) { usesBwcDistribution(bwcVersion) systemProperty("tests.old_cluster_version", bwcVersion) include '**/RemoteClusterSecurityBWCToRCS1ClusterRestIT.class' + include '**/RemoteClusterSecurityBWCToRCS2ClusterRestIT.class' } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java new file mode 100644 index 0000000000000..ae4efc492113a --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java @@ -0,0 +1,247 @@ +/* + * 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.remotecluster; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.RequestOptions; +import org.elasticsearch.client.Response; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Strings; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.SearchResponseUtils; +import org.elasticsearch.test.rest.ObjectPath; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; + +/** + * A set of BWC tests that can be executed with either RCS 1 or RCS 2 against an older fulfilling cluster. + */ +public class AbstractRemoteClusterSecurityBWCTest extends AbstractRemoteClusterSecurityTestCase { + + protected void testBwcCCSViaRCS1orRCS2(boolean rcs2) throws Exception { + final boolean useProxyMode = randomBoolean(); + if (rcs2) { + configureRemoteCluster(useProxyMode); + } else { + setupQueryClusterRCS1(useProxyMode); + } + + ensureRemoteFulfillingClusterIsConnected(useProxyMode, rcs2); + + // Fulfilling cluster + { + // Index some documents, so we can attempt to search them from the querying cluster + final Request bulkRequest = new Request("POST", "/_bulk?refresh=true"); + bulkRequest.setJsonEntity(Strings.format(""" + { "index": { "_index": "remote_index1" } } + { "foo": "bar" } + { "index": { "_index": "remote_index2" } } + { "bar": "foo" } + """)); + assertOK(performRequestAgainstFulfillingCluster(bulkRequest)); + } + + // Query cluster + { + // Index some documents, to use them in a mixed-cluster search + final var indexDocRequest = new Request("POST", "/local_index/_doc?refresh=true"); + indexDocRequest.setJsonEntity("{\"local_foo\": \"local_bar\"}"); + assertOK(client().performRequest(indexDocRequest)); + + // Create user role with privileges for remote and local indices + final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleRequest.setJsonEntity(""" + { + "description": "This description should not be sent to remote clusters.", + "cluster": ["manage_own_api_key"], + "indices": [ + { + "names": ["local_index", "remote_index1"], + "privileges": ["read", "read_cross_cluster"] + } + ], + "remote_indices": [ + { + "names": ["remote_index1"], + "privileges": ["read", "read_cross_cluster"], + "clusters": ["my_remote_cluster"] + } + ], + "remote_cluster": [ + { + "privileges": ["monitor_enrich"], + "clusters": ["*"] + } + ] + }"""); + assertOK(adminClient().performRequest(putRoleRequest)); + if (rcs2 == false) { + // We need to define the same role on QC and FC in order for CCS to work. + final var putRoleRequestFulfilling = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); + putRoleRequestFulfilling.setJsonEntity(""" + { + "cluster": ["manage_own_api_key"], + "indices": [ + { + "names": ["remote_index1"], + "privileges": ["read", "read_cross_cluster"] + } + ] + }"""); + assertOK(performRequestAgainstFulfillingCluster(putRoleRequestFulfilling)); + } + + final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); + putUserRequest.setJsonEntity(""" + { + "password": "x-pack-test-password", + "roles" : ["remote_search"] + }"""); + assertOK(adminClient().performRequest(putUserRequest)); + + // Create API key (with REMOTE_SEARCH_USER as owner) which can be used for remote cluster search. + final var createApiKeyRequest = new Request("PUT", "/_security/api_key"); + // Note: remote_indices should be ignored when sending a request to FC which is on an unsupported version + createApiKeyRequest.setJsonEntity(randomBoolean() ? """ + { + "name": "qc_api_key_with_remote_access", + "role_descriptors": { + "my_remote_access_role": { + "indices": [ + { + "names": ["local_index", "remote_index1", "remote_index2"], + "privileges": ["read", "read_cross_cluster"] + } + ], + "remote_indices": [ + { + "names": ["remote_index1", "remote_index2"], + "privileges": ["read", "read_cross_cluster"], + "clusters": ["my_remote_*", "non_existing_remote_cluster"] + } + ], + "remote_cluster": [ + { + "privileges": ["monitor_enrich", "monitor_stats"], + "clusters": ["*"] + } + ] + } + } + }""" : """ + { + "name": "qc_api_key_with_remote_access", + "role_descriptors": {} + }"""); + final var createApiKeyResponse = performRequestWithRemoteAccessUser(createApiKeyRequest); + assertOK(createApiKeyResponse); + var createApiKeyResponsePath = ObjectPath.createFromResponse(createApiKeyResponse); + final String apiKeyEncoded = createApiKeyResponsePath.evaluate("encoded"); + final String apiKeyId = createApiKeyResponsePath.evaluate("id"); + assertThat(apiKeyEncoded, notNullValue()); + assertThat(apiKeyId, notNullValue()); + + // Check that we can search the fulfilling cluster from the querying cluster + final boolean alsoSearchLocally = randomBoolean(); + final var searchRequest = new Request( + "GET", + String.format( + Locale.ROOT, + "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", + alsoSearchLocally ? "local_index," : "", + randomFrom("my_remote_cluster", "*", "my_remote_*"), + randomFrom("remote_index1", "*"), + randomBoolean() + ) + ); + final String sendRequestWith = randomFrom("user", "apikey"); + final Response response = sendRequestWith.equals("user") + ? performRequestWithRemoteAccessUser(searchRequest) + : performRequestWithApiKey(searchRequest, apiKeyEncoded); + assertOK(response); + final SearchResponse searchResponse; + try (var parser = responseAsParser(response)) { + searchResponse = SearchResponseUtils.parseSearchResponse(parser); + } + try { + final List actualIndices = Arrays.stream(searchResponse.getHits().getHits()) + .map(SearchHit::getIndex) + .collect(Collectors.toList()); + if (alsoSearchLocally) { + assertThat(actualIndices, containsInAnyOrder("remote_index1", "local_index")); + } else { + assertThat(actualIndices, containsInAnyOrder("remote_index1")); + } + } finally { + searchResponse.decRef(); + } + } + } + + private void ensureRemoteFulfillingClusterIsConnected(boolean useProxyMode, boolean rcs2) throws Exception { + final int numberOfFcNodes = fulfillingCluster.getHttpAddresses().split(",").length; + final Request remoteInfoRequest = new Request("GET", "/_remote/info"); + assertBusy(() -> { + final Response remoteInfoResponse = adminClient().performRequest(remoteInfoRequest); + assertOK(remoteInfoResponse); + final Map remoteInfoMap = responseAsMap(remoteInfoResponse); + assertThat(remoteInfoMap, hasKey("my_remote_cluster")); + assertThat(org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.connected", remoteInfoMap), is(true)); + if (rcs2) { + assertThat( + org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.cluster_credentials", remoteInfoMap), + is("::es_redacted::") // RCS 2.0 + ); + } else { + assertThat(org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.cluster_credentials", remoteInfoMap), nullValue()); + } + if (false == useProxyMode) { + assertThat( + org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.num_nodes_connected", remoteInfoMap), + equalTo(numberOfFcNodes) + ); + } + }); + } + + private Response performRequestWithRemoteAccessUser(final Request request) throws IOException { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue(REMOTE_SEARCH_USER, PASS))); + return client().performRequest(request); + } + + private Response performRequestWithApiKey(final Request request, final String encoded) throws IOException { + request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + encoded)); + return client().performRequest(request); + } + + private void setupQueryClusterRCS1(boolean useProxyMode) throws IOException { + final Settings.Builder builder = Settings.builder(); + if (useProxyMode) { + builder.put("cluster.remote.my_remote_cluster.mode", "proxy") + .put("cluster.remote.my_remote_cluster.proxy_address", fulfillingCluster.getTransportEndpoint(0)); + } else { + builder.put("cluster.remote.my_remote_cluster.mode", "sniff") + .putList("cluster.remote.my_remote_cluster.seeds", fulfillingCluster.getTransportEndpoint(0)); + } + updateClusterSettings(builder.build()); + } + +} diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java index 0e9a22299ed67..e98ae941512e1 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java @@ -7,36 +7,14 @@ package org.elasticsearch.xpack.remotecluster; -import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.client.Request; -import org.elasticsearch.client.RequestOptions; -import org.elasticsearch.client.Response; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.core.Strings; -import org.elasticsearch.search.SearchHit; -import org.elasticsearch.search.SearchResponseUtils; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.test.cluster.util.Version; import org.elasticsearch.test.cluster.util.resource.Resource; -import org.elasticsearch.test.rest.ObjectPath; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; -import java.io.IOException; -import java.util.Arrays; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.stream.Collectors; - -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasKey; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; - /** * BWC test which ensures that users and API keys with defined {@code remote_indices}/{@code remote_cluster} privileges can be used * to query legacy remote clusters when using RCS 1.0. We send the request the to an older fulfilling cluster using RCS 1.0 with a user/role @@ -45,7 +23,7 @@ * newly introduced RCS 2.0 artifacts are forward compatible from the perspective of the old cluster. For example, a new privilege * sent to an old cluster should be ignored. */ -public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteClusterSecurityTestCase { +public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteClusterSecurityBWCTest { private static final Version OLD_CLUSTER_VERSION = Version.fromString(System.getProperty("tests.old_cluster_version")); @@ -75,198 +53,7 @@ public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteC // Use a RuleChain to ensure that fulfilling cluster is started before query cluster public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); - public void testBwcWithLegacyCrossClusterSearch() throws Exception { - final boolean useProxyMode = randomBoolean(); - // Update remote cluster settings on QC. - setupQueryClusterRemoteClusters(useProxyMode); - // Ensure remote cluster is connected - ensureRemoteFulfillingClusterIsConnected(useProxyMode); - - // Fulfilling cluster - { - // Index some documents, so we can attempt to search them from the querying cluster - final Request bulkRequest = new Request("POST", "/_bulk?refresh=true"); - bulkRequest.setJsonEntity(Strings.format(""" - { "index": { "_index": "remote_index1" } } - { "foo": "bar" } - { "index": { "_index": "remote_index2" } } - { "bar": "foo" } - """)); - assertOK(performRequestAgainstFulfillingCluster(bulkRequest)); - } - - // Query cluster - { - // Index some documents, to use them in a mixed-cluster search - final var indexDocRequest = new Request("POST", "/local_index/_doc?refresh=true"); - indexDocRequest.setJsonEntity("{\"local_foo\": \"local_bar\"}"); - assertOK(client().performRequest(indexDocRequest)); - - // Create user role with privileges for remote and local indices - final var putRoleRequest = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequest.setJsonEntity(""" - { - "description": "This description should not be sent to remote clusters.", - "cluster": ["manage_own_api_key"], - "indices": [ - { - "names": ["local_index", "remote_index1"], - "privileges": ["read", "read_cross_cluster"] - } - ], - "remote_indices": [ - { - "names": ["remote_index1"], - "privileges": ["read", "read_cross_cluster"], - "clusters": ["my_remote_cluster"] - } - ], - "remote_cluster": [ - { - "privileges": ["monitor_enrich"], - "clusters": ["*"] - } - ] - }"""); - assertOK(adminClient().performRequest(putRoleRequest)); - // We need to define the same role on QC and FC in order for CCS to work. - final var putRoleRequestFulfilling = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); - putRoleRequestFulfilling.setJsonEntity(""" - { - "cluster": ["manage_own_api_key"], - "indices": [ - { - "names": ["remote_index1"], - "privileges": ["read", "read_cross_cluster"] - } - ] - }"""); - assertOK(performRequestAgainstFulfillingCluster(putRoleRequestFulfilling)); - final var putUserRequest = new Request("PUT", "/_security/user/" + REMOTE_SEARCH_USER); - putUserRequest.setJsonEntity(""" - { - "password": "x-pack-test-password", - "roles" : ["remote_search"] - }"""); - assertOK(adminClient().performRequest(putUserRequest)); - - // Create API key (with REMOTE_SEARCH_USER as owner) which can be used for remote cluster search. - final var createApiKeyRequest = new Request("PUT", "/_security/api_key"); - // Note: remote_indices should be ignored when sending a request to FC which is on an unsupported version - createApiKeyRequest.setJsonEntity(randomBoolean() ? """ - { - "name": "qc_api_key_with_remote_access", - "role_descriptors": { - "my_remote_access_role": { - "indices": [ - { - "names": ["local_index", "remote_index1", "remote_index2"], - "privileges": ["read", "read_cross_cluster"] - } - ], - "remote_indices": [ - { - "names": ["remote_index1", "remote_index2"], - "privileges": ["read", "read_cross_cluster"], - "clusters": ["my_remote_*", "non_existing_remote_cluster"] - } - ], - "remote_cluster": [ - { - "privileges": ["monitor_enrich", "monitor_stats"], - "clusters": ["*"] - } - ] - } - } - }""" : """ - { - "name": "qc_api_key_with_remote_access", - "role_descriptors": {} - }"""); - final var createApiKeyResponse = performRequestWithRemoteAccessUser(createApiKeyRequest); - assertOK(createApiKeyResponse); - var createApiKeyResponsePath = ObjectPath.createFromResponse(createApiKeyResponse); - final String apiKeyEncoded = createApiKeyResponsePath.evaluate("encoded"); - final String apiKeyId = createApiKeyResponsePath.evaluate("id"); - assertThat(apiKeyEncoded, notNullValue()); - assertThat(apiKeyId, notNullValue()); - - // Check that we can search the fulfilling cluster from the querying cluster - final boolean alsoSearchLocally = randomBoolean(); - final var searchRequest = new Request( - "GET", - String.format( - Locale.ROOT, - "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", - alsoSearchLocally ? "local_index," : "", - randomFrom("my_remote_cluster", "*", "my_remote_*"), - randomFrom("remote_index1", "*"), - randomBoolean() - ) - ); - final String sendRequestWith = randomFrom("user", "apikey"); - final Response response = sendRequestWith.equals("user") - ? performRequestWithRemoteAccessUser(searchRequest) - : performRequestWithApiKey(searchRequest, apiKeyEncoded); - assertOK(response); - final SearchResponse searchResponse; - try (var parser = responseAsParser(response)) { - searchResponse = SearchResponseUtils.parseSearchResponse(parser); - } - try { - final List actualIndices = Arrays.stream(searchResponse.getHits().getHits()) - .map(SearchHit::getIndex) - .collect(Collectors.toList()); - if (alsoSearchLocally) { - assertThat(actualIndices, containsInAnyOrder("remote_index1", "local_index")); - } else { - assertThat(actualIndices, containsInAnyOrder("remote_index1")); - } - } finally { - searchResponse.decRef(); - } - } - } - - private void ensureRemoteFulfillingClusterIsConnected(boolean useProxyMode) throws Exception { - final int numberOfFcNodes = fulfillingCluster.getHttpAddresses().split(",").length; - final Request remoteInfoRequest = new Request("GET", "/_remote/info"); - assertBusy(() -> { - final Response remoteInfoResponse = adminClient().performRequest(remoteInfoRequest); - assertOK(remoteInfoResponse); - final Map remoteInfoMap = responseAsMap(remoteInfoResponse); - assertThat(remoteInfoMap, hasKey("my_remote_cluster")); - assertThat(org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.connected", remoteInfoMap), is(true)); - if (false == useProxyMode) { - assertThat( - org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.num_nodes_connected", remoteInfoMap), - equalTo(numberOfFcNodes) - ); - } - }); - } - - private void setupQueryClusterRemoteClusters(boolean useProxyMode) throws IOException { - final Settings.Builder builder = Settings.builder(); - if (useProxyMode) { - builder.put("cluster.remote.my_remote_cluster.mode", "proxy") - .put("cluster.remote.my_remote_cluster.proxy_address", fulfillingCluster.getTransportEndpoint(0)); - } else { - builder.put("cluster.remote.my_remote_cluster.mode", "sniff") - .putList("cluster.remote.my_remote_cluster.seeds", fulfillingCluster.getTransportEndpoint(0)); - } - updateClusterSettings(builder.build()); - } - - private Response performRequestWithRemoteAccessUser(final Request request) throws IOException { - request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue(REMOTE_SEARCH_USER, PASS))); - return client().performRequest(request); - } - - private Response performRequestWithApiKey(final Request request, final String encoded) throws IOException { - request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "ApiKey " + encoded)); - return client().performRequest(request); + public void testBwcCCSViaRCS1() throws Exception { + testBwcCCSViaRCS1orRCS2(false); } - } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java new file mode 100644 index 0000000000000..d70a3769437d9 --- /dev/null +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java @@ -0,0 +1,80 @@ +/* + * 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.remotecluster; + +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.elasticsearch.test.cluster.util.Version; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +/** + * BWC test which ensures that users and API keys with defined {@code remote_indices}/{@code remote_cluster} privileges can be used + * to query older remote clusters when using RCS 2.0. We send the request the to an older fulfilling cluster using RCS 2.0 with a user/role + * and API key where the {@code remote_indices}/{@code remote_cluster} are defined in the newer query cluster. + * All new RCS 2.0 config should be effectively ignored when sending to older RCS 2.0. For example, a new privilege + * sent to an old cluster should be ignored. + */ +public class RemoteClusterSecurityBWCToRCS2ClusterRestIT extends AbstractRemoteClusterSecurityBWCTest { + + private static final Version OLD_CLUSTER_VERSION = Version.fromString(System.getProperty("tests.old_cluster_version")); + private static final AtomicReference> API_KEY_MAP_REF = new AtomicReference<>(); + + static { + + fulfillingCluster = ElasticsearchCluster.local() + .name("fulfilling-cluster") + .version(OLD_CLUSTER_VERSION) + .distribution(DistributionType.DEFAULT) + .apply(commonClusterConfig) + .setting("xpack.ml.enabled", "false") + .setting("remote_cluster_server.enabled", "true") + .setting("remote_cluster.port", "0") + .setting("xpack.security.remote_cluster_server.ssl.enabled", "true") + .setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key") + .setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt") + .keystore("xpack.security.remote_cluster_server.ssl.secure_key_passphrase", "remote-cluster-password") + // .setting("logger.org.elasticsearch.xpack.core", "trace") //useful for human debugging + // .setting("logger.org.elasticsearch.xpack.security", "trace") //useful for human debugging + .build(); + + queryCluster = ElasticsearchCluster.local() + .name("query-cluster") + .apply(commonClusterConfig) + .setting("xpack.security.remote_cluster_client.ssl.enabled", "true") + .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") + .keystore("cluster.remote.my_remote_cluster.credentials", () -> { + if (API_KEY_MAP_REF.get() == null) { + final Map apiKeyMap = createCrossClusterAccessApiKey(""" + { + "search": [ + { + "names": ["*"] + } + ] + }"""); + API_KEY_MAP_REF.set(apiKeyMap); + } + return (String) API_KEY_MAP_REF.get().get("encoded"); + }) + .build(); + } + + @ClassRule + // Use a RuleChain to ensure that fulfilling cluster is started before query cluster + public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); + + public void testBwcCCSViaRCS2() throws Exception { + testBwcCCSViaRCS1orRCS2(true); + } + +} From 892ac085fc0f150b07ae259ea263f9d440ec5a3d Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 4 Nov 2024 11:44:21 -0600 Subject: [PATCH 27/43] clean up execution --- ...stractRemoteClusterSecurityBWCRestIT.java} | 22 ++++++++++++------- ...ClusterSecurityBWCToRCS1ClusterRestIT.java | 14 +++++++++--- ...ClusterSecurityBWCToRCS2ClusterRestIT.java | 13 ++++++++--- 3 files changed, 35 insertions(+), 14 deletions(-) rename x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/{AbstractRemoteClusterSecurityBWCTest.java => AbstractRemoteClusterSecurityBWCRestIT.java} (95%) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java similarity index 95% rename from x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java rename to x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java index ae4efc492113a..308701cab26ab 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCTest.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java @@ -16,6 +16,7 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchResponseUtils; import org.elasticsearch.test.rest.ObjectPath; +import org.junit.Before; import java.io.IOException; import java.util.Arrays; @@ -34,17 +35,22 @@ /** * A set of BWC tests that can be executed with either RCS 1 or RCS 2 against an older fulfilling cluster. */ -public class AbstractRemoteClusterSecurityBWCTest extends AbstractRemoteClusterSecurityTestCase { +public abstract class AbstractRemoteClusterSecurityBWCRestIT extends AbstractRemoteClusterSecurityTestCase { - protected void testBwcCCSViaRCS1orRCS2(boolean rcs2) throws Exception { + protected abstract boolean isRCS2(); + + @Before + public void setUp() throws Exception { final boolean useProxyMode = randomBoolean(); - if (rcs2) { + if (isRCS2()) { configureRemoteCluster(useProxyMode); } else { setupQueryClusterRCS1(useProxyMode); } - - ensureRemoteFulfillingClusterIsConnected(useProxyMode, rcs2); + ensureRemoteFulfillingClusterIsConnected(useProxyMode); + super.setUp(); + } + public void testBwcCCSViaRCS1orRCS2() throws Exception { // Fulfilling cluster { @@ -93,7 +99,7 @@ protected void testBwcCCSViaRCS1orRCS2(boolean rcs2) throws Exception { ] }"""); assertOK(adminClient().performRequest(putRoleRequest)); - if (rcs2 == false) { + if (isRCS2() == false) { // We need to define the same role on QC and FC in order for CCS to work. final var putRoleRequestFulfilling = new Request("PUT", "/_security/role/" + REMOTE_SEARCH_ROLE); putRoleRequestFulfilling.setJsonEntity(""" @@ -196,7 +202,7 @@ protected void testBwcCCSViaRCS1orRCS2(boolean rcs2) throws Exception { } } - private void ensureRemoteFulfillingClusterIsConnected(boolean useProxyMode, boolean rcs2) throws Exception { + private void ensureRemoteFulfillingClusterIsConnected(boolean useProxyMode) throws Exception { final int numberOfFcNodes = fulfillingCluster.getHttpAddresses().split(",").length; final Request remoteInfoRequest = new Request("GET", "/_remote/info"); assertBusy(() -> { @@ -205,7 +211,7 @@ private void ensureRemoteFulfillingClusterIsConnected(boolean useProxyMode, bool final Map remoteInfoMap = responseAsMap(remoteInfoResponse); assertThat(remoteInfoMap, hasKey("my_remote_cluster")); assertThat(org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.connected", remoteInfoMap), is(true)); - if (rcs2) { + if (isRCS2()) { assertThat( org.elasticsearch.xcontent.ObjectPath.eval("my_remote_cluster.cluster_credentials", remoteInfoMap), is("::es_redacted::") // RCS 2.0 diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java index e98ae941512e1..6520f5d9232f1 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java @@ -11,6 +11,7 @@ import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.test.cluster.util.Version; import org.elasticsearch.test.cluster.util.resource.Resource; +import org.junit.Before; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; @@ -23,7 +24,7 @@ * newly introduced RCS 2.0 artifacts are forward compatible from the perspective of the old cluster. For example, a new privilege * sent to an old cluster should be ignored. */ -public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteClusterSecurityBWCTest { +public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteClusterSecurityBWCRestIT { private static final Version OLD_CLUSTER_VERSION = Version.fromString(System.getProperty("tests.old_cluster_version")); @@ -53,7 +54,14 @@ public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteC // Use a RuleChain to ensure that fulfilling cluster is started before query cluster public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); - public void testBwcCCSViaRCS1() throws Exception { - testBwcCCSViaRCS1orRCS2(false); + @Override + protected boolean isRCS2() { + return false; + } + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java index d70a3769437d9..900e6534507c5 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.test.cluster.util.Version; +import org.junit.Before; import org.junit.ClassRule; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; @@ -24,7 +25,7 @@ * All new RCS 2.0 config should be effectively ignored when sending to older RCS 2.0. For example, a new privilege * sent to an old cluster should be ignored. */ -public class RemoteClusterSecurityBWCToRCS2ClusterRestIT extends AbstractRemoteClusterSecurityBWCTest { +public class RemoteClusterSecurityBWCToRCS2ClusterRestIT extends AbstractRemoteClusterSecurityBWCRestIT { private static final Version OLD_CLUSTER_VERSION = Version.fromString(System.getProperty("tests.old_cluster_version")); private static final AtomicReference> API_KEY_MAP_REF = new AtomicReference<>(); @@ -73,8 +74,14 @@ public class RemoteClusterSecurityBWCToRCS2ClusterRestIT extends AbstractRemoteC // Use a RuleChain to ensure that fulfilling cluster is started before query cluster public static TestRule clusterRule = RuleChain.outerRule(fulfillingCluster).around(queryCluster); - public void testBwcCCSViaRCS2() throws Exception { - testBwcCCSViaRCS1orRCS2(true); + @Override + protected boolean isRCS2() { + return true; } + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + } } From b4b85c8fb5d4194ca68e87d5be37aa2177acfd2b Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 4 Nov 2024 13:19:04 -0600 Subject: [PATCH 28/43] clean up bwc tests --- ...bstractRemoteClusterSecurityBWCRestIT.java | 73 ++++++++++++++----- ...ClusterSecurityBWCToRCS1ClusterRestIT.java | 3 +- ...ClusterSecurityBWCToRCS2ClusterRestIT.java | 2 + 3 files changed, 58 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java index 308701cab26ab..0275d67706ef4 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.remotecluster; +import org.apache.http.util.EntityUtils; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; @@ -16,6 +17,8 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchResponseUtils; import org.elasticsearch.test.rest.ObjectPath; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.json.JsonXContent; import org.junit.Before; import java.io.IOException; @@ -26,6 +29,7 @@ import java.util.stream.Collectors; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; @@ -50,6 +54,7 @@ public void setUp() throws Exception { ensureRemoteFulfillingClusterIsConnected(useProxyMode); super.setUp(); } + public void testBwcCCSViaRCS1orRCS2() throws Exception { // Fulfilling cluster @@ -167,38 +172,35 @@ public void testBwcCCSViaRCS1orRCS2() throws Exception { // Check that we can search the fulfilling cluster from the querying cluster final boolean alsoSearchLocally = randomBoolean(); + final String remoteClusterName = randomFrom("my_remote_cluster", "*", "my_remote_*"); + final String remoteIndexName = randomFrom("remote_index1", "*"); final var searchRequest = new Request( "GET", String.format( Locale.ROOT, "/%s%s:%s/_search?ccs_minimize_roundtrips=%s", alsoSearchLocally ? "local_index," : "", - randomFrom("my_remote_cluster", "*", "my_remote_*"), - randomFrom("remote_index1", "*"), + remoteClusterName, + remoteIndexName, randomBoolean() ) ); - final String sendRequestWith = randomFrom("user", "apikey"); - final Response response = sendRequestWith.equals("user") - ? performRequestWithRemoteAccessUser(searchRequest) - : performRequestWithApiKey(searchRequest, apiKeyEncoded); + String esqlCommand = String.format(Locale.ROOT, "FROM %s,%s:%s | LIMIT 10", "local_index", remoteClusterName, remoteIndexName); + // send request with user + Response response = performRequestWithRemoteAccessUser(searchRequest); assertOK(response); - final SearchResponse searchResponse; try (var parser = responseAsParser(response)) { - searchResponse = SearchResponseUtils.parseSearchResponse(parser); + assertSearchResponse(SearchResponseUtils.parseSearchResponse(parser), alsoSearchLocally); } - try { - final List actualIndices = Arrays.stream(searchResponse.getHits().getHits()) - .map(SearchHit::getIndex) - .collect(Collectors.toList()); - if (alsoSearchLocally) { - assertThat(actualIndices, containsInAnyOrder("remote_index1", "local_index")); - } else { - assertThat(actualIndices, containsInAnyOrder("remote_index1")); - } - } finally { - searchResponse.decRef(); + assertEsqlResponse(performRequestWithRemoteAccessUser(esqlRequest(esqlCommand))); + + // send request with apikey + response = performRequestWithApiKey(searchRequest, apiKeyEncoded); + assertOK(response); + try (var parser = responseAsParser(response)) { + assertSearchResponse(SearchResponseUtils.parseSearchResponse(parser), alsoSearchLocally); } + assertEsqlResponse(performRequestWithApiKey(esqlRequest(esqlCommand), apiKeyEncoded)); } } @@ -250,4 +252,37 @@ private void setupQueryClusterRCS1(boolean useProxyMode) throws IOException { updateClusterSettings(builder.build()); } + private Request esqlRequest(String command) throws IOException { + XContentBuilder body = JsonXContent.contentBuilder(); + body.startObject(); + body.field("query", command); + body.field("include_ccs_metadata", true); + body.endObject(); + Request request = new Request("POST", "_query"); + request.setJsonEntity(org.elasticsearch.common.Strings.toString(body)); + return request; + } + + private void assertSearchResponse(SearchResponse searchResponse, boolean alsoSearchLocally) { + try { + final List actualIndices = Arrays.stream(searchResponse.getHits().getHits()) + .map(SearchHit::getIndex) + .collect(Collectors.toList()); + if (alsoSearchLocally) { + assertThat(actualIndices, containsInAnyOrder("remote_index1", "local_index")); + } else { + assertThat(actualIndices, containsInAnyOrder("remote_index1")); + } + } finally { + searchResponse.decRef(); + } + } + + private void assertEsqlResponse(Response response) throws IOException { + assertOK(response); + String responseAsString = EntityUtils.toString(response.getEntity()); + assertThat(responseAsString, containsString("\"my_remote_cluster\":{\"status\":\"successful\"")); + assertThat(responseAsString, containsString("local_bar")); + assertThat(responseAsString, containsString("bar")); + } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java index 6520f5d9232f1..00848c94d9130 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java @@ -41,7 +41,8 @@ public class RemoteClusterSecurityBWCToRCS1ClusterRestIT extends AbstractRemoteC queryCluster = ElasticsearchCluster.local() .version(Version.CURRENT) - .distribution(DistributionType.INTEG_TEST) + .distribution(DistributionType.DEFAULT) + .setting("xpack.ml.enabled", "false") .name("query-cluster") .apply(commonClusterConfig) .setting("xpack.security.remote_cluster_client.ssl.enabled", "true") diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java index 900e6534507c5..fb92365ade0b8 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java @@ -50,6 +50,8 @@ public class RemoteClusterSecurityBWCToRCS2ClusterRestIT extends AbstractRemoteC queryCluster = ElasticsearchCluster.local() .name("query-cluster") + .distribution(DistributionType.DEFAULT) + .setting("xpack.ml.enabled", "false") .apply(commonClusterConfig) .setting("xpack.security.remote_cluster_client.ssl.enabled", "true") .setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt") From 7e9cb22af056618a6a75f3c345cea15c8624afb2 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 4 Nov 2024 15:28:59 -0600 Subject: [PATCH 29/43] fix set logic --- .../core/security/authz/RoleDescriptor.java | 13 ++++----- .../security/authz/RoleDescriptorTests.java | 28 +++++++++++++++++++ .../authz/store/FileRolesStoreTests.java | 2 +- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 3f693d6492879..a2463d724a805 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -50,7 +50,6 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; import static org.elasticsearch.common.xcontent.XContentHelper.createParserNotCompressed; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; @@ -834,17 +833,17 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName currentFieldName = parser.currentName(); } else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { privileges = readStringArray(roleName, parser, false); - if (Collections.disjoint( - RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), - Arrays.stream(privileges).map(s -> s.toLowerCase(Locale.ROOT)).collect(Collectors.toSet()) - )) { + if (Arrays.stream(privileges) + .map(s -> s.toLowerCase(Locale.ROOT)) + .allMatch(RemoteClusterPermissions.getSupportedRemoteClusterPermissions()::contains) == false) { final String message = String.format( Locale.ROOT, "failed to parse remote_cluster for role [%s]. " - + "%s are the only values allowed for [%s] within [remote_cluster]", + + "%s are the only values allowed for [%s] within [remote_cluster]. Found %s", roleName, RemoteClusterPermissions.getSupportedRemoteClusterPermissions(), - currentFieldName + currentFieldName, + Arrays.toString(privileges) ); logger.warn(message); throw new ElasticsearchParseException(message); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptorTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptorTests.java index 94430a4ed5bba..218876c7d40e8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptorTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptorTests.java @@ -542,6 +542,34 @@ public void testParseInvalidRemoteCluster() throws IOException { () -> RoleDescriptor.parserBuilder().build().parse("test", new BytesArray(q4), XContentType.JSON) ); assertThat(illegalArgumentException.getMessage(), containsString("remote cluster groups must not be null or empty")); + + // one invalid privilege + String q5 = """ + { + "remote_cluster": [ + { + "privileges": [ + "monitor_stats", "read_pipeline" + ], + "clusters": [ + "*" + ] + } + ] + }"""; + + ElasticsearchParseException parseException = expectThrows( + ElasticsearchParseException.class, + () -> RoleDescriptor.parserBuilder().build().parse("test", new BytesArray(q5), XContentType.JSON) + ); + assertThat( + parseException.getMessage(), + containsString( + "failed to parse remote_cluster for role [test]. " + + "[monitor_enrich, monitor_stats] are the only values allowed for [privileges] within [remote_cluster]. " + + "Found [monitor_stats, read_pipeline]" + ) + ); } public void testParsingFieldPermissionsUsesCache() throws IOException { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java index fb03f10ea2534..af5f44b5989fb 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java @@ -389,7 +389,7 @@ public void testParseFileWithRemoteIndicesAndCluster() throws IllegalAccessExcep startsWith( "failed to parse remote_cluster for role [invalid_role_bad_priv_remote_clusters]. " + "[monitor_enrich, monitor_stats] are the only values allowed for [privileges] within [remote_cluster]. " - + "skipping role..." + + "Found [junk]. skipping role..." ) ); } From d35dee8e006c2ab6a13fa3b0c912e9bcd21adcaf Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 4 Nov 2024 16:03:30 -0600 Subject: [PATCH 30/43] Update docs/changelog/114964.yaml --- docs/changelog/114964.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/114964.yaml diff --git a/docs/changelog/114964.yaml b/docs/changelog/114964.yaml new file mode 100644 index 0000000000000..8274aeb76a937 --- /dev/null +++ b/docs/changelog/114964.yaml @@ -0,0 +1,6 @@ +pr: 114964 +summary: Add a `monitor_stats` privilege and allow that privilege for remote cluster + privileges +area: Authorization +type: enhancement +issues: [] From 1c749bc305291ca6cbbc94597e0d0f32be153d9c Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Wed, 6 Nov 2024 14:40:48 -0600 Subject: [PATCH 31/43] update javadoc / minor rename --- .../core/security/authz/RoleDescriptor.java | 4 ++-- .../permission/RemoteClusterPermissions.java | 22 +++++++++++-------- .../security/authz/permission/SimpleRole.java | 2 +- .../RemoteClusterPermissionsTests.java | 12 +++++----- .../authz/store/ReservedRolesStoreTests.java | 2 +- .../xpack/security/authz/RBACEngineTests.java | 12 +++++++--- .../authz/store/CompositeRolesStoreTests.java | 4 ++-- 7 files changed, 34 insertions(+), 24 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index a2463d724a805..1ab6ca21c13c5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -845,7 +845,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName currentFieldName, Arrays.toString(privileges) ); - logger.warn(message); + logger.info(message); throw new ElasticsearchParseException(message); } } else if (Fields.CLUSTERS.match(currentFieldName, parser.getDeprecationHandler())) { @@ -857,7 +857,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName roleName, currentFieldName ); - logger.warn(message); + logger.info(message); throw new ElasticsearchParseException(message); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 807af32185815..1e82d256bbb60 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -36,10 +36,12 @@ /** * Represents the set of permissions for remote clusters. This is intended to be the model for both the {@link RoleDescriptor} - * and {@link Role}. This model is not intended to support RCS 2.0 where these remote cluster permissions are converted to local cluster - * permissions {@link #privilegeNames(String, TransportVersion)} ) before sent to the remote cluster. This model also supports RCS 1.0 where - * these permissions are sent to the remote API keys {@link #toMap()}. In both cases the outbound transport version will be used to - * remove permissions that are not available to elder remote clusters. + * and {@link Role}. This model is intended to be converted to local cluster permissions + * {@link #collapseAndRemoveUnsupportedPrivileges(String, TransportVersion)} before sent to the remote cluster. This model also be included + * in the role descriptors for (normal) API keys sent between nodes/clusters. In both cases the outbound transport version can be used to + * remove permissions that are not available to older nodes or clusters. The methods {@link #removeUnsupportedPrivileges(TransportVersion)} + * and {@link #collapseAndRemoveUnsupportedPrivileges(String, TransportVersion)} are used to aid in ensuring correct privileges per + * transport version. * For example, on the local/querying cluster this model represents the following: * * "remote_cluster" : [ @@ -63,8 +65,8 @@ * * "cluster": ["bar"] * - * (RCS 1.0 and RCS 2.0) If the remote cluster does not support the privilege, as determined by the remote cluster version, - * the privilege will be not be sent. Upstream code performs the removal, but this class owns the business logic for how to remove. + * For normal API keys and their role descriptors :If the remote cluster does not support the privilege, the privilege will be not be sent. + * Upstream code performs the removal, but this class owns the business logic for how to remove per outbound version. */ public class RemoteClusterPermissions implements NamedWriteable, ToXContentObject { @@ -115,7 +117,9 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus /** * Will remove any unsupported privileges for the provided outbound version. This method will not modify the current instance. - * This is useful for RCS 1.0 and API keys to help ensure that we don't send unsupported privileges to the remote cluster. + * This is useful for (normal) API keys role descriptors to help ensure that we don't send unsupported privileges. The result of + * this method may result in no groups if all privileges are removed. {@link #hasPrivileges()} can be used to check if there are + * any privileges left. * @param outboundVersion The version by which to remove unsupported privileges, this is typically the version of the remote cluster * @return a new instance of RemoteClusterPermissions with the unsupported privileges removed */ @@ -166,7 +170,7 @@ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion out * and will only return the appropriate privileges for the provided remote cluster version. This is useful for RCS 2.0 to ensure * that we properly convert all the remote_cluster -> cluster privileges per remote cluster. */ - public String[] privilegeNames(final String remoteClusterAlias, TransportVersion remoteClusterVersion) { + public String[] collapseAndRemoveUnsupportedPrivileges(final String remoteClusterAlias, TransportVersion outboundVersion) { // get all privileges for the remote cluster Set groupPrivileges = remoteClusterPermissionGroups.stream() @@ -177,7 +181,7 @@ public String[] privilegeNames(final String remoteClusterAlias, TransportVersion .collect(Collectors.toSet()); // find all the privileges that are allowed for the remote cluster version - Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(remoteClusterVersion); + Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(outboundVersion); // intersect the two sets to get the allowed privileges for the remote cluster version Set allowedPrivileges = new HashSet<>(groupPrivileges); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java index 08c86c5f71f4f..d9480d6490f07 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java @@ -224,7 +224,7 @@ public RoleDescriptorsIntersection getRoleDescriptorsIntersectionForRemoteCluste return new RoleDescriptorsIntersection( new RoleDescriptor( REMOTE_USER_ROLE_NAME, - remoteClusterPermissions.privilegeNames(remoteClusterAlias, remoteClusterVersion), + remoteClusterPermissions.collapseAndRemoveUnsupportedPrivileges(remoteClusterAlias, remoteClusterVersion), // The role descriptors constructed here may be cached in raw byte form, using a hash of their content as a // cache key; we therefore need deterministic order when constructing them here, to ensure cache hits for // equivalent role descriptors diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index a1c17448b0ed8..29f25eded98a5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -95,7 +95,7 @@ public void testMatcher() { } } - public void testPrivilegeNames() { + public void testCollapseAndRemoveUnsupportedPrivileges() { Map> original = RemoteClusterPermissions.allowedRemoteClusterPermissions; try { // create random groups with random privileges for random clusters @@ -112,7 +112,7 @@ public void testPrivilegeNames() { String[] privileges = groupPrivileges.get(i); String[] clusters = groupClusters.get(i); for (String cluster : clusters) { - String[] found = remoteClusterPermission.privilegeNames(cluster, TransportVersion.current()); + String[] found = remoteClusterPermission.collapseAndRemoveUnsupportedPrivileges(cluster, TransportVersion.current()); Arrays.sort(found); // ensure all lowercase since the privilege names are case insensitive and the method will result in lowercase for (int j = 0; j < privileges.length; j++) { @@ -137,7 +137,7 @@ public void testPrivilegeNames() { String[] privileges = groupPrivileges.get(i); String[] clusters = groupClusters.get(i); for (String cluster : clusters) { - String[] found = remoteClusterPermission.privilegeNames(cluster, TransportVersion.current()); + String[] found = remoteClusterPermission.collapseAndRemoveUnsupportedPrivileges(cluster, TransportVersion.current()); Arrays.sort(found); // ensure all lowercase since the privilege names are case insensitive and the method will result in lowercase for (int j = 0; j < privileges.length; j++) { @@ -173,15 +173,15 @@ private void testPermissionPerVersion(String permission, TransportVersion versio // test permission before, after and on the version String[] privileges = randomBoolean() ? new String[] { permission } : new String[] { permission, "foo", "bar" }; String[] before = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) - .privilegeNames("*", TransportVersionUtils.getPreviousVersion(version)); + .collapseAndRemoveUnsupportedPrivileges("*", TransportVersionUtils.getPreviousVersion(version)); // empty set since permissions is not allowed in the before version assertThat(Set.of(before), equalTo(Collections.emptySet())); String[] on = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) - .privilegeNames("*", version); + .collapseAndRemoveUnsupportedPrivileges("*", version); // the permission is found on that provided version assertThat(Set.of(on), equalTo(Set.of(permission))); String[] after = new RemoteClusterPermissions().addGroup(new RemoteClusterPermissionGroup(privileges, new String[] { "*" })) - .privilegeNames("*", TransportVersion.current()); + .collapseAndRemoveUnsupportedPrivileges("*", TransportVersion.current()); // current version (after the version) has the permission assertThat(Set.of(after), equalTo(Set.of(permission))); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java index a71ac6a9b51fd..fb4d822b7655c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java @@ -2833,7 +2833,7 @@ public void testSuperuserRole() { is(false) ); assertThat( - superuserRole.remoteCluster().privilegeNames("*", TransportVersion.current()), + superuserRole.remoteCluster().collapseAndRemoveUnsupportedPrivileges("*", TransportVersion.current()), equalTo(RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0])) ); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 7404187c1bdbc..a41c54ada781a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -1395,13 +1395,19 @@ public void testBuildUserPrivilegeResponse() { // ensure that all permissions are valid for the current transport version assertThat( - Arrays.asList(remoteClusterPermissions.privilegeNames("remote-1", TransportVersion.current())), + Arrays.asList(remoteClusterPermissions.collapseAndRemoveUnsupportedPrivileges("remote-1", TransportVersion.current())), hasItem("monitor_enrich") ); for (String permission : RemoteClusterPermissions.getSupportedRemoteClusterPermissions()) { - assertThat(Arrays.asList(remoteClusterPermissions.privilegeNames("remote-2", TransportVersion.current())), hasItem(permission)); - assertThat(Arrays.asList(remoteClusterPermissions.privilegeNames("remote-3", TransportVersion.current())), hasItem(permission)); + assertThat( + Arrays.asList(remoteClusterPermissions.collapseAndRemoveUnsupportedPrivileges("remote-2", TransportVersion.current())), + hasItem(permission) + ); + assertThat( + Arrays.asList(remoteClusterPermissions.collapseAndRemoveUnsupportedPrivileges("remote-3", TransportVersion.current())), + hasItem(permission) + ); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index da903ff7f7177..330a700e3c97f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -1158,7 +1158,7 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build assertHasRemoteIndexGroupsForClusters(forRemote, Set.of("*"), indexGroup("remote-idx-2-*")); assertValidRemoteClusterPermissions(role.remoteCluster(), new String[] { "remote-*" }); assertThat( - role.remoteCluster().privilegeNames("remote-foobar", TransportVersion.current()), + role.remoteCluster().collapseAndRemoveUnsupportedPrivileges("remote-foobar", TransportVersion.current()), equalTo(RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0])) ); } @@ -3327,7 +3327,7 @@ private void assertValidRemoteClusterPermissionsParent(RemoteClusterPermissions assertTrue(permissions.hasPrivileges(alias)); assertFalse(permissions.hasPrivileges(randomValueOtherThan(alias, () -> randomAlphaOfLength(5)))); assertThat( - permissions.privilegeNames(alias, TransportVersion.current()), + permissions.collapseAndRemoveUnsupportedPrivileges(alias, TransportVersion.current()), arrayContaining(RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0])) ); } From d19a07484a651d315ab5e7525dba0be0feb939c7 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Wed, 6 Nov 2024 15:14:59 -0600 Subject: [PATCH 32/43] test setup clean up --- .../AbstractRemoteClusterSecurityBWCRestIT.java | 13 ------------- ...RemoteClusterSecurityBWCToRCS1ClusterRestIT.java | 1 + ...RemoteClusterSecurityBWCToRCS2ClusterRestIT.java | 1 + 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java index 0275d67706ef4..20cdbb9f8b0df 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityBWCRestIT.java @@ -19,7 +19,6 @@ import org.elasticsearch.test.rest.ObjectPath; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.json.JsonXContent; -import org.junit.Before; import java.io.IOException; import java.util.Arrays; @@ -43,18 +42,6 @@ public abstract class AbstractRemoteClusterSecurityBWCRestIT extends AbstractRem protected abstract boolean isRCS2(); - @Before - public void setUp() throws Exception { - final boolean useProxyMode = randomBoolean(); - if (isRCS2()) { - configureRemoteCluster(useProxyMode); - } else { - setupQueryClusterRCS1(useProxyMode); - } - ensureRemoteFulfillingClusterIsConnected(useProxyMode); - super.setUp(); - } - public void testBwcCCSViaRCS1orRCS2() throws Exception { // Fulfilling cluster diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java index 00848c94d9130..73e0f096039f9 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS1ClusterRestIT.java @@ -63,6 +63,7 @@ protected boolean isRCS2() { @Before @Override public void setUp() throws Exception { + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, true, randomBoolean(), false); super.setUp(); } } diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java index fb92365ade0b8..5e173b72c66de 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityBWCToRCS2ClusterRestIT.java @@ -84,6 +84,7 @@ protected boolean isRCS2() { @Before @Override public void setUp() throws Exception { + configureRemoteCluster(REMOTE_CLUSTER_ALIAS, fulfillingCluster, false, randomBoolean(), false); super.setUp(); } } From 1ee1d438789f7c34a13137293a9498675355040f Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 15:05:45 -0600 Subject: [PATCH 33/43] clean up conditional striping of privs --- .../xpack/core/security/authc/Authentication.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 0657d64e41f70..ace7dcff69b05 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -1336,7 +1336,7 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) && streamVersion.before(ROLE_REMOTE_CLUSTER_PRIVS)) { - // if the remote does not understand the remote_cluster field remove it + // the authentication understands the remote_cluster field but the stream does not metadata = new HashMap<>(metadata); metadata.put( AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, @@ -1352,9 +1352,10 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( ); } else if (authentication.getEffectiveSubject() .getTransportVersion() - .onOrAfter(TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY) - && streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { - // the remote does understand the remote_cluster field, so check each permission and remove permission as needed + .onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) //ensure the authentication object understands remote_cluster + && streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { //ensure stream understands remote_cluster + // both the authentication object and the stream understand the remote_cluster field + // check each individual permission and remove as needed metadata = new HashMap<>(metadata); metadata.put( AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY, From 4b1dd9df8b02d8ebab6b33ddaf3e3d6ef6afb0db Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 15:58:40 -0600 Subject: [PATCH 34/43] skip test --- x-pack/plugin/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 2bced3c0a4ff6..193a82436f26a 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -88,6 +88,6 @@ tasks.named("yamlRestCompatTestTransform").configure({ task -> task.skipTest("esql/60_usage/Basic ESQL usage output (telemetry) non-snapshot version", "The number of functions is constantly increasing") task.skipTest("esql/80_text/reverse text", "The output type changed from TEXT to KEYWORD.") task.skipTest("esql/80_text/values function", "The output type changed from TEXT to KEYWORD.") - task.replaceValueInLength("cluster", 62, "Test get builtin privileges") // new builtin privileges have been added + task.skipTest("privileges/11_builtin/Test get builtin privileges" ,"unnecessary to test compatibility") }) From c734bbb4da8514abf69e2bee45b40703afbad707 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:01:45 -0600 Subject: [PATCH 35/43] add trim to string --- .../elasticsearch/xpack/core/security/authz/RoleDescriptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index 1ab6ca21c13c5..fadc15f38ea88 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -834,7 +834,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName } else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) { privileges = readStringArray(roleName, parser, false); if (Arrays.stream(privileges) - .map(s -> s.toLowerCase(Locale.ROOT)) + .map(s -> s.toLowerCase(Locale.ROOT).trim()) .allMatch(RemoteClusterPermissions.getSupportedRemoteClusterPermissions()::contains) == false) { final String message = String.format( Locale.ROOT, From 23279e52dc4bf6189c46876dd9c3e447f740fe4f Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:31:16 -0600 Subject: [PATCH 36/43] minor rename --- .../permission/RemoteClusterPermissions.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 1e82d256bbb60..33c0f05f6b3dd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -125,30 +125,31 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus */ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion outboundVersion) { - RemoteClusterPermissions copyForOutBoundVersion = new RemoteClusterPermissions(); + RemoteClusterPermissions copyForOutboundVersion = new RemoteClusterPermissions(); Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(outboundVersion); for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) { String[] privileges = group.clusterPrivileges(); - List privilegesMutated = new ArrayList<>(privileges.length); + List outboundPrivileges = new ArrayList<>(privileges.length); for (String privilege : privileges) { if (allowedPermissionsPerVersion.contains(privilege.toLowerCase(Locale.ROOT))) { - privilegesMutated.add(privilege); + outboundPrivileges.add(privilege); } } - if (privilegesMutated.isEmpty() == false) { - RemoteClusterPermissionGroup mutatedGroup = new RemoteClusterPermissionGroup( - privilegesMutated.toArray(new String[0]), + if (outboundPrivileges.isEmpty() == false) { + RemoteClusterPermissionGroup outboundGroup = new RemoteClusterPermissionGroup( + outboundPrivileges.toArray(new String[0]), group.remoteClusterAliases() ); - copyForOutBoundVersion.addGroup(mutatedGroup); + copyForOutboundVersion.addGroup(outboundGroup); if (logger.isDebugEnabled()) { - if (group.equals(mutatedGroup) == false) { + if (group.equals(outboundGroup) == false) { logger.debug( - "Removed unsupported remote cluster permissions {} for remote cluster [{}]. " + "Removed unsupported remote cluster permissions. Remaining {} for remote cluster [{}] for version [{}]." + "Due to the remote cluster version, only the following permissions are allowed: {}", - privilegesMutated, + outboundPrivileges, group.remoteClusterAliases(), + outboundVersion, allowedPermissionsPerVersion ); } @@ -162,7 +163,7 @@ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion out ); } } - return copyForOutBoundVersion; + return copyForOutboundVersion; } /** From 3182bcdeb6869ff606952a4729c09adb08ffeb09 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:36:02 -0600 Subject: [PATCH 37/43] s/hasPrivileges/hasAnyPrivileges --- .../security/action/user/GetUserPrivilegesResponse.java | 2 +- .../xpack/core/security/authc/Authentication.java | 2 +- .../xpack/core/security/authz/RoleDescriptor.java | 4 ++-- .../authz/permission/RemoteClusterPermissions.java | 8 ++++---- .../xpack/core/security/authz/permission/Role.java | 2 +- .../xpack/core/security/authz/permission/SimpleRole.java | 2 +- .../authz/permission/RemoteClusterPermissionsTests.java | 6 +++--- .../xpack/security/authz/store/CompositeRolesStore.java | 2 +- .../security/authz/store/CompositeRolesStoreTests.java | 6 +++--- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/GetUserPrivilegesResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/GetUserPrivilegesResponse.java index de351cd59c690..763ab6ccb9886 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/GetUserPrivilegesResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/user/GetUserPrivilegesResponse.java @@ -115,7 +115,7 @@ public boolean hasRemoteIndicesPrivileges() { } public boolean hasRemoteClusterPrivileges() { - return remoteClusterPermissions.hasPrivileges(); + return remoteClusterPermissions.hasAnyPrivileges(); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index ace7dcff69b05..db167fcd734ef 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -1517,7 +1517,7 @@ static BytesReference maybeRemoveRemoteClusterPrivilegesFromRoleDescriptors( // swap out the old value with the new value modified.set(true); Map remoteClusterMap = new HashMap<>((Map) roleDescriptorsMapMutated.get(key)); - if (mutated.hasPrivileges()) { + if (mutated.hasAnyPrivileges()) { // has at least one group with privileges remoteClusterMap.put(innerKey, mutated.toMap()); } else { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java index fadc15f38ea88..9f5aaa8562a88 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java @@ -194,7 +194,7 @@ public RoleDescriptor( ? Collections.unmodifiableMap(transientMetadata) : Collections.singletonMap("enabled", true); this.remoteIndicesPrivileges = remoteIndicesPrivileges != null ? remoteIndicesPrivileges : RemoteIndicesPrivileges.NONE; - this.remoteClusterPermissions = remoteClusterPermissions != null && remoteClusterPermissions.hasPrivileges() + this.remoteClusterPermissions = remoteClusterPermissions != null && remoteClusterPermissions.hasAnyPrivileges() ? remoteClusterPermissions : RemoteClusterPermissions.NONE; this.restriction = restriction != null ? restriction : Restriction.NONE; @@ -266,7 +266,7 @@ public boolean hasRemoteIndicesPrivileges() { } public boolean hasRemoteClusterPermissions() { - return remoteClusterPermissions.hasPrivileges(); + return remoteClusterPermissions.hasAnyPrivileges(); } public RemoteClusterPermissions getRemoteClusterPermissions() { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 33c0f05f6b3dd..0ea0eb0ba3a26 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -118,7 +118,7 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus /** * Will remove any unsupported privileges for the provided outbound version. This method will not modify the current instance. * This is useful for (normal) API keys role descriptors to help ensure that we don't send unsupported privileges. The result of - * this method may result in no groups if all privileges are removed. {@link #hasPrivileges()} can be used to check if there are + * this method may result in no groups if all privileges are removed. {@link #hasAnyPrivileges()} can be used to check if there are * any privileges left. * @param outboundVersion The version by which to remove unsupported privileges, this is typically the version of the remote cluster * @return a new instance of RemoteClusterPermissions with the unsupported privileges removed @@ -216,7 +216,7 @@ public List>> toMap() { * Generally, this method is just a safety check and validity should be checked before adding the permissions to this class. */ public void validate() { - assert hasPrivileges(); + assert hasAnyPrivileges(); Set invalid = getUnsupportedPrivileges(); if (invalid.isEmpty() == false) { throw new IllegalArgumentException( @@ -246,11 +246,11 @@ private Set getUnsupportedPrivileges() { return invalid; } - public boolean hasPrivileges(final String remoteClusterAlias) { + public boolean hasAnyPrivileges(final String remoteClusterAlias) { return remoteClusterPermissionGroups.stream().anyMatch(remoteIndicesGroup -> remoteIndicesGroup.hasPrivileges(remoteClusterAlias)); } - public boolean hasPrivileges() { + public boolean hasAnyPrivileges() { return remoteClusterPermissionGroups.isEmpty() == false; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java index d8d56a4fbb247..f52f8f85f006d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java @@ -283,7 +283,7 @@ public Builder addRemoteIndicesGroup( public Builder addRemoteClusterPermissions(RemoteClusterPermissions remoteClusterPermissions) { Objects.requireNonNull(remoteClusterPermissions, "remoteClusterPermissions must not be null"); assert this.remoteClusterPermissions == null : "addRemoteClusterPermissions should only be called once"; - if (remoteClusterPermissions.hasPrivileges()) { + if (remoteClusterPermissions.hasAnyPrivileges()) { remoteClusterPermissions.validate(); } this.remoteClusterPermissions = remoteClusterPermissions; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java index d9480d6490f07..0ec9d2a48316a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java @@ -210,7 +210,7 @@ public RoleDescriptorsIntersection getRoleDescriptorsIntersectionForRemoteCluste final RemoteIndicesPermission remoteIndicesPermission = this.remoteIndicesPermission.forCluster(remoteClusterAlias); if (remoteIndicesPermission.remoteIndicesGroups().isEmpty() - && remoteClusterPermissions.hasPrivileges(remoteClusterAlias) == false) { + && remoteClusterPermissions.hasAnyPrivileges(remoteClusterAlias) == false) { return RoleDescriptorsIntersection.EMPTY; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index 29f25eded98a5..fde26287b957b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -89,8 +89,8 @@ public void testMatcher() { for (int i = 0; i < generateRandomGroups(true).size(); i++) { String[] clusters = groupClusters.get(i); for (String cluster : clusters) { - assertTrue(remoteClusterPermission.hasPrivileges(cluster)); - assertFalse(remoteClusterPermission.hasPrivileges(randomAlphaOfLength(20))); + assertTrue(remoteClusterPermission.hasAnyPrivileges(cluster)); + assertFalse(remoteClusterPermission.hasAnyPrivileges(randomAlphaOfLength(20))); } } } @@ -221,7 +221,7 @@ public void testRemoveUnsupportedPrivileges() { remoteClusterPermissions.addGroup(group); // this single newer privilege is not allowed in the older version, so it should result in an object with no groups assertNotEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); - assertFalse(remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS).hasPrivileges()); + assertFalse(remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS).hasAnyPrivileges()); assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); int groupCount = randomIntBetween(1, 5); 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 index d79a3e31c1bc9..2e1a643bf4f4f 100644 --- 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 @@ -572,7 +572,7 @@ public static void buildRoleFromDescriptors( ); }); - if (remoteClusterPermissions.hasPrivileges()) { + if (remoteClusterPermissions.hasAnyPrivileges()) { builder.addRemoteClusterPermissions(remoteClusterPermissions); } else { builder.addRemoteClusterPermissions(RemoteClusterPermissions.NONE); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index 330a700e3c97f..cef3572ee3ac4 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -3322,10 +3322,10 @@ private void assertValidRemoteClusterPermissions(RemoteClusterPermissions permis } private void assertValidRemoteClusterPermissionsParent(RemoteClusterPermissions permissions, String[] aliases) { - assertTrue(permissions.hasPrivileges()); + assertTrue(permissions.hasAnyPrivileges()); for (String alias : aliases) { - assertTrue(permissions.hasPrivileges(alias)); - assertFalse(permissions.hasPrivileges(randomValueOtherThan(alias, () -> randomAlphaOfLength(5)))); + assertTrue(permissions.hasAnyPrivileges(alias)); + assertFalse(permissions.hasAnyPrivileges(randomValueOtherThan(alias, () -> randomAlphaOfLength(5)))); assertThat( permissions.collapseAndRemoveUnsupportedPrivileges(alias, TransportVersion.current()), arrayContaining(RemoteClusterPermissions.getSupportedRemoteClusterPermissions().toArray(new String[0])) From 7a66dcb357fec361034e5aff2d5bf2268e3c7fd4 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:38:18 -0600 Subject: [PATCH 38/43] better assert --- .../apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java index f0db44070d2c4..1dfd68ea95485 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java @@ -379,7 +379,10 @@ public void testAPIKeyAllowsAllRemoteClusterPrivilegesForCCS() { break; } } - assertTrue(actionPassesRemoteClusterPermissionCheck); + assertTrue( + "privilege [" + privilege + "] does not cover any actions among [" + actionsToTest + "]", + actionPassesRemoteClusterPermissionCheck + ); } // test that the actions pass the privilege check for CCS for (String privilege : Set.of(CCS_CLUSTER_PRIVILEGE_NAMES)) { From d1711c40439e727a9a552c344e4818e84d259137 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:47:16 -0600 Subject: [PATCH 39/43] fix off by one --- .../xpack/core/security/authc/AuthenticationTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java index cb08744773da1..c999c970a76da 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java @@ -1165,7 +1165,7 @@ public void testMaybeRewriteMetadataForApiKeyRoleDescriptorsWithRemoteClusterRem final TransportVersion olderVersion = TransportVersionUtils.randomVersionBetween( random(), ROLE_REMOTE_CLUSTER_PRIVS, - ROLE_MONITOR_STATS + TransportVersionUtils.getPreviousVersion(ROLE_MONITOR_STATS) ); Map rewrittenMetadata = with2privs.maybeRewriteForOlderVersion(olderVersion).getEffectiveSubject().getMetadata(); From fd2503ea185ad7b31228ae74137b5ec30aff63b9 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:54:20 -0600 Subject: [PATCH 40/43] ensure duplicate names don't cause issue with removal --- .../authz/permission/RemoteClusterPermissionsTests.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index fde26287b957b..4c21f07d2975d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -217,7 +217,12 @@ public void testRemoveUnsupportedPrivileges() { assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); remoteClusterPermissions = new RemoteClusterPermissions(); - group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" }); + if(randomBoolean()){ + group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" }); + } else { + //if somehow duplicates end up here, they should not influence removal + group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats", "monitor_stats" }, new String[] { "*" }); + } remoteClusterPermissions.addGroup(group); // this single newer privilege is not allowed in the older version, so it should result in an object with no groups assertNotEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_REMOTE_CLUSTER_PRIVS)); From b816117b52fc9ec5ddbf3cc994ae0fa82341fa2f Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 16:54:46 -0600 Subject: [PATCH 41/43] typo --- .../authz/permission/RemoteClusterPermissionsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index 4c21f07d2975d..214624ffd134b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -235,7 +235,7 @@ public void testRemoveUnsupportedPrivileges() { for (int i = 0; i < groupCount; i++) { remoteClusterPermissions.addGroup(group); } - // one of the newer privilege is not allowed in the older version, so it should result in as group with only the allowed privilege + // one of the newer privilege is not allowed in the older version, so it should result in a group with only the allowed privilege RemoteClusterPermissions expected = new RemoteClusterPermissions(); for (int i = 0; i < groupCount; i++) { expected.addGroup(new RemoteClusterPermissionGroup(new String[] { "monitor_enrich" }, new String[] { "*" })); From d1f4029baeaee822c3f5c4b098ccd875535bd642 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 17:30:29 -0600 Subject: [PATCH 42/43] short circuit removeUnsupportedPrivileges --- .../authz/permission/RemoteClusterPermissions.java | 10 ++++++++-- .../permission/RemoteClusterPermissionsTests.java | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 0ea0eb0ba3a26..204f2574c4678 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -83,6 +83,10 @@ public class RemoteClusterPermissions implements NamedWriteable, ToXContentObjec ROLE_MONITOR_STATS, Set.of(ClusterPrivilegeResolver.MONITOR_STATS.name()) ); + static final TransportVersion lastTransportVersionPermission = allowedRemoteClusterPermissions.keySet() + .stream() + .max(TransportVersion::compareTo) + .orElseThrow(); public static final RemoteClusterPermissions NONE = new RemoteClusterPermissions(); @@ -124,9 +128,11 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus * @return a new instance of RemoteClusterPermissions with the unsupported privileges removed */ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion outboundVersion) { - + Objects.requireNonNull(outboundVersion, "outboundVersion must not be null"); + if(outboundVersion.onOrAfter(lastTransportVersionPermission)) { + return this; + } RemoteClusterPermissions copyForOutboundVersion = new RemoteClusterPermissions(); - Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(outboundVersion); for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) { String[] privileges = group.clusterPrivileges(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index 214624ffd134b..ec43a92d34679 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -33,6 +33,7 @@ import static org.elasticsearch.TransportVersions.ROLE_MONITOR_STATS; import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.ROLE_REMOTE_CLUSTER_PRIVS; +import static org.elasticsearch.xpack.core.security.authz.permission.RemoteClusterPermissions.lastTransportVersionPermission; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -245,6 +246,15 @@ public void testRemoveUnsupportedPrivileges() { assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); } + public void testShortCircuitRemoveUnsupportedPrivileges(){ + RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); + assertSame(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(TransportVersion.current())); + assertSame(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(lastTransportVersionPermission)); + assertNotSame( + remoteClusterPermissions, + remoteClusterPermissions.removeUnsupportedPrivileges(TransportVersionUtils.getPreviousVersion(lastTransportVersionPermission))); + } + private List generateRandomGroups(boolean fuzzyCluster) { clean(); List groups = new ArrayList<>(); From 31096b71f632590ba6eeabe9ee1c89f3252b0740 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 7 Nov 2024 17:39:26 -0600 Subject: [PATCH 43/43] spotless --- .../xpack/core/security/authc/Authentication.java | 6 ++---- .../authz/permission/RemoteClusterPermissions.java | 2 +- .../authz/permission/RemoteClusterPermissionsTests.java | 9 +++++---- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index db167fcd734ef..c2f40a3e393b9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -1350,10 +1350,8 @@ private static Map maybeRewriteMetadataForApiKeyRoleDescriptors( (BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY) ) ); - } else if (authentication.getEffectiveSubject() - .getTransportVersion() - .onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) //ensure the authentication object understands remote_cluster - && streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { //ensure stream understands remote_cluster + } else if (authentication.getEffectiveSubject().getTransportVersion().onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS) + && streamVersion.onOrAfter(ROLE_REMOTE_CLUSTER_PRIVS)) { // both the authentication object and the stream understand the remote_cluster field // check each individual permission and remove as needed metadata = new HashMap<>(metadata); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index 204f2574c4678..1928cf117dde3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -129,7 +129,7 @@ public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClus */ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion outboundVersion) { Objects.requireNonNull(outboundVersion, "outboundVersion must not be null"); - if(outboundVersion.onOrAfter(lastTransportVersionPermission)) { + if (outboundVersion.onOrAfter(lastTransportVersionPermission)) { return this; } RemoteClusterPermissions copyForOutboundVersion = new RemoteClusterPermissions(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java index ec43a92d34679..2c31965009273 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionsTests.java @@ -218,10 +218,10 @@ public void testRemoveUnsupportedPrivileges() { assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); remoteClusterPermissions = new RemoteClusterPermissions(); - if(randomBoolean()){ + if (randomBoolean()) { group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats" }, new String[] { "*" }); } else { - //if somehow duplicates end up here, they should not influence removal + // if somehow duplicates end up here, they should not influence removal group = new RemoteClusterPermissionGroup(new String[] { "monitor_stats", "monitor_stats" }, new String[] { "*" }); } remoteClusterPermissions.addGroup(group); @@ -246,13 +246,14 @@ public void testRemoveUnsupportedPrivileges() { assertEquals(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(ROLE_MONITOR_STATS)); } - public void testShortCircuitRemoveUnsupportedPrivileges(){ + public void testShortCircuitRemoveUnsupportedPrivileges() { RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions(); assertSame(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(TransportVersion.current())); assertSame(remoteClusterPermissions, remoteClusterPermissions.removeUnsupportedPrivileges(lastTransportVersionPermission)); assertNotSame( remoteClusterPermissions, - remoteClusterPermissions.removeUnsupportedPrivileges(TransportVersionUtils.getPreviousVersion(lastTransportVersionPermission))); + remoteClusterPermissions.removeUnsupportedPrivileges(TransportVersionUtils.getPreviousVersion(lastTransportVersionPermission)) + ); } private List generateRandomGroups(boolean fuzzyCluster) {