Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add kibana-system service account #76449

Merged
merged 5 commits into from
Aug 13, 2021

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Aug 12, 2021

This change introduces a Service Account for Kibana to use when
authenticating to Elasticsearch. The Service Account with
kibana-system service name under the elastic namespace,
uses the same RoleDescriptor as the existing kibana_system
built-in user and is its functional equivalent when it comes to
AuthZ.

deprecates: #73738

This change introduces a Service Account for Kibana to use when
authenticating to Elasticsearch. The Service Account with
kibana-system service name under the elastic namespace,
uses the same RoleDescriptor as the existing kibana_system
built-in user and is its functional equivalent when it comes to
AuthZ.
@jkakavas jkakavas added >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 v7.15.0 labels Aug 12, 2021
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Aug 12, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

There is also a JavaRest test class for service accounts in case you haven't looked at it. Because most of its tests are more for testing the service account infrastructure, you may find it not necessary to touch it anyway. One thing that could have some minor value is asserting the role descriptor return from the get-service-account call (testGetServiceAccount) because it is not tested in yaml tests. We can choose to not add a literal string of the role descriptor, but instead rely on RoleDescriptor.toXContent to reduce maintanence cost.

},
null,
new ConfigurableClusterPrivilege[] { new ManageApplicationPrivileges(Collections.singleton("kibana-*")) },
null, MetadataUtils.DEFAULT_RESERVED_METADATA, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reserved metadata is not really needed for service account role descriptor. But it should not cause any issue either.

@@ -434,6 +363,81 @@ private static RoleDescriptor kibanaAdminUser(String name, Map<String, Object> m
null, null, metadata, null);
}

public static RoleDescriptor kibanaSystemUserRole(String name) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
public static RoleDescriptor kibanaSystemUserRole(String name) {
public static RoleDescriptor kibanaSystemRoleDescriptor(String name) {

@@ -46,6 +145,280 @@

public class ElasticServiceAccountsTests extends ESTestCase {

public void testKibanaSystemPrivileges() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but might not be necessary and also add to ongoing maintanence since the same is tested in ReservedRoleStoreTests. I'd prefer to just assert the service account role descriptor is equivalent to the one used for the reserved role. Something like:

var serviceRoleDescriptor = ElasticServiceAccounts.ACCOUNTS.get("elastic/kibana-system").roleDescriptor();
var reservedRoleDescriptor = ReservedRoleStore.roleDescriptor(KibanaSystemUser.ROLE_NAME);
assertThat(serviceRoleDescriptor.getClusterPrivileges(), equalTo(reservedRoleDescriptor.getClusterPrivileges()));
...

One caveat of the above is that the method ReservedRoleStore.roleDescriptor(...) is currently not static. But I think it should be OK to change it to static or alternatively we can use new ReservedRolesStore().roleDescriptor(...) similar to how it is done in ReservedRoleStoreTests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I was also not particularly fond of this, your idea makes sense, thanks!

@@ -43,8 +44,10 @@
null,
null
));
private static final ServiceAccount KIBANA_SYSTEM_ACCOUNT =
new ElasticServiceAccount("kibana-system", ReservedRolesStore.kibanaSystemUserRole(NAMESPACE + "/kibana-system"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if it was already discussed elsewhere, but I'm wondering if the name of the service account should be kibana instead of kibana-system. Given this is a service account, I don't think there may be confusion with a kibana user, and the name sounds clearer to me and more consistent with fleet-server.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have an opinion tbh, happy bothways. I'll leave this up to you and Kibana folks to decide

Copy link
Member

@azasypkin azasypkin Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was initially wondering why not kibana_system (with the underscore) to be consistent with the existing built-in user (I guess Fleet doesn't have any "system" built-in users yet, so it's not a problem for them). At the same time, I see the name format for service accounts is <namespace>/<service> where service already implies it's not a user, and elastic namespace would imply that this service is a reserved/"system" one. With this, elastic/kibana sounds good to me.

@jportner @kobelb any concerns regarding elastic/kibana service account name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The elastic/fleet-server account was initially named elastic/fleet. It got renamed because the Fleet team wanted to differentiate between the Fleet Server and other fleet services, e.g. there could be another elastic/fleet service account that is used to setup Fleet in Kibana. Do we foresee other service accounts needed for Kibana? If not, elastic/kibana might be a better choice.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jportner @kobelb any concerns regarding elastic/kibana service account name?

Fine by me

Do we foresee other service accounts needed for Kibana? If not, elastic/kibana might be a better choice.

Looking through the open issues, I don't think there is a desire to make additional service accounts for Kibana. But Perhaps @kobelb knows something I don't, so let's see what he says.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jportner @kobelb any concerns regarding elastic/kibana service account name?

Fine by me.

The elastic/fleet-server account was initially named elastic/fleet. It got renamed because the Fleet team wanted to differentiate between the Fleet Server and other fleet services, e.g. there could be another elastic/fleet service account that is used to setup Fleet in Kibana

This makes sense to me because Fleet as a product is partially implemented in Kibana and Fleet Server. However, at the moment, we don't have the same complexity with Kibana, Kibana is just Kibana at the moment.

Looking through the open issues, I don't think there is a desire to make additional service accounts for Kibana. But Perhaps @kobelb knows something I don't, so let's see what he says.

TL;DR: I'm not aware of any concrete plans to introduce additional service accounts.

There have been some super preliminary discussions about how we could in the future allow Kibana to run certain setup steps with different privileges than during normal runtime. For example, when Kibana is first starting up, it needs privileges to provision and migrate data in its own system indices; however, these privileges are no longer required after Kibana has started up. If Kibana could drop privileges after performing setup, it could allow Kibana to run normal operations with fewer privileges, mitigating some attack vectors. However, we'd almost certainly want to find a way for end-users to only have to configure a single reserved user or service account in the kibana.yml, or else we'd make configuring Kibana harder.

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally with Kibana, worked as expected, thanks! Just want to get an additional input for the service account name before we merge: https://github.com/elastic/elasticsearch/pull/76449/files#r688364607

@jkakavas
Copy link
Member Author

It seems like there is agreement to call the account elastic/kibana so I went ahead and made the change already. I will refrain from merging until @jportner @kobelb have time to comment

@jkakavas jkakavas added the auto-backport Automatically create backport pull requests when merged label Aug 13, 2021
@jkakavas jkakavas merged commit 59d75f2 into elastic:master Aug 13, 2021
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run backport --upstream elastic/elasticsearch --pr 76449

jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Aug 13, 2021
This change introduces a Service Account for Kibana to use when
authenticating to Elasticsearch. The Service Account with
kibana service name under the elastic namespace,
uses the same RoleDescriptor as the existing kibana_system
built-in user and is its functional equivalent when it comes to
AuthZ.
# Conflicts:
#	rest-api-spec/build.gradle
#	x-pack/plugin/build.gradle
#	x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java
#	x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ServiceAccountServiceTests.java
@jkakavas jkakavas added this to Implementation Milestone 4 (August 13th) in Security On by Default Implementation Tracking Aug 13, 2021
@jkakavas jkakavas moved this from Implementation Milestone 4 (August 13th) to Completed in Security On by Default Implementation Tracking Aug 13, 2021
jkakavas added a commit that referenced this pull request Aug 13, 2021
This change introduces a Service Account for Kibana to use when
authenticating to Elasticsearch. The Service Account with
kibana service name under the elastic namespace,
uses the same RoleDescriptor as the existing kibana_system
built-in user and is its functional equivalent when it comes to
AuthZ.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v7.15.0 v8.0.0-alpha2
Development

Successfully merging this pull request may close these issues.

None yet

9 participants