Restrict APM Server user role #2777

barkbay · 2020-03-26T14:28:21Z

This PR:

Adds a role called default_apm_user_role (name can be discussed ofc) to allow APM Server instances to setup and manage apm-* indices:

default_apm_user_role:
      cluster:
      - monitor
      - manage_ilm
      indices:
      - names:
        - apm-*
        privileges:
        - manage
        - create_doc
        - create_index

Adds the APM users to the role described above and also to the built-in roles kibana_user and ingest_admin

I did some tests and it seems to work fine but I would be glad to have a quick review of the role by @simitt or @graphaelli. Thanks 🙇

pebrc

LGTM, but deferring to the APM folks for approval.

sebgl · 2020-03-27T10:29:04Z

@simitt @graphaelli should this role be builtin Elasticsearch, similar to the kibana user role?

simitt

Generally looks good, the monitoring privileges need to be applied to the .monitoring-beats-* index though, and I think we can be more restrictive wrt Kibana privileges.

Just as information: with 7.6 we released experimental support of API Key auth for communication between APM agents and APM Server. The APM Server provides a CLI tool to manage such API Keys. The required privileges for that are the manage_api_key on clusters, and adding specific APM application privileges.
Since this API Key tooling needs manual interaction anyways, I think it is fair to not add the privileges here, and assume the users either provide a superuser role when managing API Keys or add the privileges themselves.

UPDATE:
It is ok to not add the additional application privileges, but manage_api_key should be added to the cluster privileges, as it is required for the APM Server verifying API keys created by another user.

pkg/controller/elasticsearch/user/roles.go

simitt · 2020-03-27T09:38:37Z

...controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go

+	// apmDefaultRoles are the default roles used by APM Server instances
+	apmDefaultRoles = strings.Join([]string{
+		user.ApmDefaultUserRole, // Retrieve cluster details (e.g. version) and manage apm-* indices
+		"kibana_user",           // Load dependencies, such as example dashboards, if available, into Kibana


For fetching agent remote config from Kibana the APM Server needs to have read+write access to apm indices, for which we should grant the built in apm_user role rather than the more exhaustive kibana_user afaik (cc @elastic/apm-ui).

The privileges required for agent configuration management I was referring to above, need to be set for the user talking to Kibana (apm-server.kibana), but not for the elasticsearch user.
Based on the explanation in https://github.com/elastic/cloud-on-k8s/pull/2777/files#r400078619 the apmDefaultRoles will only be used for writing to ES. In this case no Kibana privileges are required for any APM Server >= 7.0, as dashboards are no longer shipped with the APM Server.

simitt · 2020-03-30T07:13:05Z

@elastic/apm-server for visibility

pkg/controller/elasticsearch/client/client.go

anyasabo · 2020-03-30T21:13:59Z

...controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go

+	apmDefaultRoles = strings.Join([]string{
+		user.ApmDefaultUserRole, // Retrieve cluster details (e.g. version) and manage apm-* indices
+		"kibana_user",           // Load dependencies, such as example dashboards, if available, into Kibana
+		"ingest_admin",          // Set up index templates


Can you help me understand the difference between ApmDefaultUserRole and this? It seems like we could put ingest_admin in the cluster role def there along with monitor and manage_ilm, right?

apmDefaultRoles is a collection of roles which contain the role ApmDefaultUserRole
monitor and manage_ilm are privileges, while ingest_admin is a built-in role.

I'm not sure we can mix a built-in role along with privileges ?

barkbay · 2020-03-31T06:41:23Z

Just as information: with 7.6 we released experimental support of API Key auth for communication between APM agents and APM Server. The APM Server provides a CLI tool to manage such API Keys. The required privileges for that are the manage_api_key on clusters, and adding specific APM application privileges.

manage_api_key seems to exist only in 7.5.x, I'm still a little bit new re. roles in Elasticsearch but I guess it means that we would have to manage 2 different roles: one for ES < 7.5 and an other for ES >= 7.5.

In this case no Kibana privileges are required for any APM Server >= 7.0, as dashboards are no longer shipped with the APM Server.

Same here, we still support 6.8. I would keep apm_user in the list of roles for backward compatibility.

barkbay · 2020-03-31T07:37:31Z

It is a little bit more complicated that what I thought :)

Any attempt to start the APM server with a privilege that does not exist makes the request fail with something like:

Failed to connect to backoff(elasticsearch(https://es-apm-sample-es-http.default.svc:9200)): 400 Bad Request: {"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"unknown index privilege [create_doc]. a privilege must be either one of the predefined fixed indices privileges ... or a pattern over one of the available cluster actions

If I understand correctly:

For 6.8 we would need this kind of role (doc ref):

{
  "cluster": ["manage_index_templates","monitor"],
  "indices": [
    {
      "names": [ "apm-*" ], 
      "privileges": ["write","create_index"]
    }
  ]
}

From 7.x to 7.4 (incl.) we can use the setup role described here (which seems to be the one described in this PR) but we can't use create_doc or manage_api_key which have been introduced in 7.5.
Starting from 7.5 we can introduce create_doc and manage_api_key

pkg/controller/elasticsearch/user/roles.go

simitt · 2020-03-31T14:32:01Z

Not sure if that helps, but in our integration testing where we can start different versions inkl. 6.8 and early 7.x versions we use a user that has following custom role:

apm_server:
  cluster: ['manage_ilm', 'manage_security', 'manage_api_key'] 
  indices:
    - names: ['apm-*']
      privileges: ['write', 'create_index', 'manage', 'manage_ilm']

and also built-in roles apm_server and apm_system
(see roles and users_roles)
Note: the manage_security role is only necessary when using the apm-server apikey CLI, which I don't think is necessary on ECK for now.

barkbay · 2020-03-31T14:40:45Z

Not sure if that helps, but in our integration testing where we can start different versions inkl. 6.8 and early 7.x versions we use a user that has following custom role:

This does not work for me. If I try to start an 6.8 APM Server with the "manage_api_key" privilege I have the following error:

2020-03-31T14:38:28.406Z	ERROR	pipeline/output.go:100	Failed to connect to backoff(elasticsearch(https://es-apm-sample-es-http.default.svc:9200)): 400 Bad Request:
{
	"error": {
		"root_cause": [{
			"type": "illegal_argument_exception",
			"reason": "unknown cluster privilege [[manage_index_templates, monitor, manage_api_key]]. a privilege must be either one of the predefined fixed cluster privileges [all=[all],monitor_ml=[monitor_ml],manage_ilm=[manage_ilm],manage_ingest_pipelines=[manage_ingest_pipelines],read_ccr=[read_ccr],monitor_watcher=[monitor_watcher],manage_index_templates=[manage_index_templates],manage_ccr=[manage_ccr],monitor=[monitor],manage_rollup=[manage_rollup],none=[none],manage_watcher=[manage_watcher],read_ilm=[read_ilm],manage=[manage],manage_token=[manage_token],manage_ml=[manage_ml],manage_pipeline=[manage_pipeline],monitor_rollup=[monitor_rollup],transport_client=[transport_client],manage_security=[manage_security],manage_saml=[manage_saml],create_snapshot=[create_snapshot]] or a pattern over one of the available cluster actions"
		}],
		"type": "illegal_argument_exception",
		"reason": "unknown cluster privilege [[manage_index_templates, monitor, manage_api_key]]. a privilege must be either one of the predefined fixed cluster privileges [all=[all],monitor_ml=[monitor_ml],manage_ilm=[manage_ilm],manage_ingest_pipelines=[manage_ingest_pipelines],read_ccr=[read_ccr],monitor_watcher=[monitor_watcher],manage_index_templates=[manage_index_templates],manage_ccr=[manage_ccr],monitor=[monitor],manage_rollup=[manage_rollup],none=[none],manage_watcher=[manage_watcher],read_ilm=[read_ilm],manage=[manage],manage_token=[manage_token],manage_ml=[manage_ml],manage_pipeline=[manage_pipeline],monitor_rollup=[monitor_rollup],transport_client=[transport_client],manage_security=[manage_security],manage_saml=[manage_saml],create_snapshot=[create_snapshot]] or a pattern over one of the available cluster actions"
	},
	"status": 400
}

simitt · 2020-04-01T09:20:33Z

@barkbay you are right, I only checked that the server can start up. Since API Key handling is only an experimental feature and it seems to be the last blocking bit, I suggest to go ahead without this privilege for now.
Is the APM Server the only component where different privileges and roles available and required for different versions becomes an issue? I could imagine that this is not an APM specific problem.

barkbay · 2020-04-01T10:01:12Z

My understanding is that we need at least 2 different roles: one for v6 and an other v7, because of the manage_ilm privilege for example which does not exist in v6 but is required in v7.

The benefit of a third one for > 7.5 would be to add the manage_api_key privilege but also replace write by create_doc which seems more restrictive from a security pov.

I have no strong opinion but I think that if we have to manage 2 roles we can add a third one.

pkg/controller/elasticsearch/user/roles.go

simitt

Thank you for all the effort you put into this. LGTM

pebrc

LGTM!

pebrc · 2020-04-02T13:21:07Z

pkg/controller/elasticsearch/user/roles.go

@@ -18,12 +18,46 @@ const (
 	SuperUserBuiltinRole = "superuser"
 	// ProbeUserRole is the name of the role used by the internal probe user.
 	ProbeUserRole = "elastic_internal_probe_user"
+
+	// ApmUserRoleV6 is the name of the role used by the association user for 6.8.x APMServer instances.


Why are we calling it the association user? Is it not just the user APM Server will use to connect to Elasticsearch?

barkbay · 2020-04-02T15:03:38Z

Thanks all for your help and the review 🙇 !

barkbay added 2 commits March 26, 2020 15:26

Restrict APM Server user role

f246f8b

FIx unit test

2e6976c

pebrc reviewed Mar 27, 2020

View reviewed changes

simitt requested changes Mar 27, 2020

View reviewed changes

simitt approved these changes Mar 30, 2020

View reviewed changes

anyasabo added >enhancement Enhancement of existing functionality v1.1.0 labels Mar 30, 2020

anyasabo reviewed Mar 30, 2020

View reviewed changes

pkg/controller/elasticsearch/client/client.go Outdated Show resolved Hide resolved

anyasabo reviewed Mar 30, 2020

View reviewed changes

barkbay added 2 commits March 31, 2020 08:04

Add manage_api_key privilege

8b2e680

Use apm_user instead of kibana_user

2c96194

barkbay mentioned this pull request Mar 31, 2020

Kibana resource fails to create in 1.0.1 #2693

Closed

barkbay added 2 commits March 31, 2020 13:41

Add dedicated roles

4c60664

update unit test

33a2dd1

barkbay requested a review from simitt March 31, 2020 11:54

Cleanup client code

4502325

barkbay commented Mar 31, 2020

View reviewed changes

pkg/controller/elasticsearch/user/roles.go Show resolved Hide resolved

anyasabo added v1.2.0 and removed v1.1.0 labels Mar 31, 2020

Add apm_system role + .monitoring-beats-* write access

bb4198b

simitt reviewed Apr 1, 2020

View reviewed changes

pkg/controller/elasticsearch/user/roles.go Show resolved Hide resolved

Remove .monitoring-beats-* from custom roles

4751e9f

simitt approved these changes Apr 2, 2020

View reviewed changes

barkbay requested a review from pebrc April 2, 2020 13:15

pebrc approved these changes Apr 2, 2020

View reviewed changes

update from code review

cff812c

barkbay merged commit c2a768d into elastic:master Apr 2, 2020

barkbay mentioned this pull request Apr 28, 2020

APM users should not use the built-in "superuser" role #2977

Closed

thbkrkr mentioned this pull request Jul 3, 2024

Add metadata field to ES role model #7927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restrict APM Server user role #2777

Restrict APM Server user role #2777

barkbay commented Mar 26, 2020

pebrc left a comment

sebgl commented Mar 27, 2020

simitt left a comment •

edited

Loading

simitt Mar 27, 2020

simitt Mar 30, 2020

simitt commented Mar 30, 2020

anyasabo Mar 30, 2020

barkbay Mar 31, 2020

barkbay commented Mar 31, 2020

barkbay commented Mar 31, 2020 •

edited

Loading

simitt commented Mar 31, 2020

barkbay commented Mar 31, 2020

simitt commented Apr 1, 2020

barkbay commented Apr 1, 2020

simitt left a comment

pebrc left a comment

pebrc Apr 2, 2020

barkbay commented Apr 2, 2020

Restrict APM Server user role #2777

Restrict APM Server user role #2777

Conversation

barkbay commented Mar 26, 2020

pebrc left a comment

Choose a reason for hiding this comment

sebgl commented Mar 27, 2020

simitt left a comment • edited Loading

Choose a reason for hiding this comment

simitt Mar 27, 2020

Choose a reason for hiding this comment

simitt Mar 30, 2020

Choose a reason for hiding this comment

simitt commented Mar 30, 2020

anyasabo Mar 30, 2020

Choose a reason for hiding this comment

barkbay Mar 31, 2020

Choose a reason for hiding this comment

barkbay commented Mar 31, 2020

barkbay commented Mar 31, 2020 • edited Loading

simitt commented Mar 31, 2020

barkbay commented Mar 31, 2020

simitt commented Apr 1, 2020

barkbay commented Apr 1, 2020

simitt left a comment

Choose a reason for hiding this comment

pebrc left a comment

Choose a reason for hiding this comment

pebrc Apr 2, 2020

Choose a reason for hiding this comment

barkbay commented Apr 2, 2020

simitt left a comment •

edited

Loading

barkbay commented Mar 31, 2020 •

edited

Loading