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.

Sign up for GitHub

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

[Fleet] Allow Kibana system user to read and write to Fleet server indices #67726

Merged
merged 5 commits into from
Jan 21, 2021
Merged

[Fleet] Allow Kibana system user to read and write to Fleet server indices #67726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7719096
[Fleet] Allow Kibana system user to read and write to Fleet server in…
nchaulet Jan 19, 2021
a9e1003
Remove write permissions for .fleet-servers and .fleet-actions-results
nchaulet Jan 20, 2021
1483236
Better comment
nchaulet Jan 20, 2021
f6450fa
Fix linting error
nchaulet Jan 20, 2021
5aaef0c
more detailed comment
nchaulet Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ...e/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,30 @@ private static Map<String, RoleDescriptor> initializeReservedRoles() {
RoleDescriptor.IndicesPrivileges.builder()
.indices(".logs-endpoint.diagnostic.collection-*")
.privileges("read").build(),
// Fleet Server indices. Kibana read and write from these indices to manage Elastic Agents.
// Kibana write to this indice to reassign agent policy or perform force unenroll
RoleDescriptor.IndicesPrivileges.builder()
.indices(".fleet-agents")
.privileges("read", "write").build(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does Kibana need write permissions for the agents?

Copy link
Member Author

@nchaulet nchaulet Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yes right now if we want to reassign an agent to a new policy, force unenroll, we do it by writing to .fleet-agents

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to use the doc line 158 to also mention why a specific access is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was hoping for a bit more details and have comments on all the lines. For example that write permissions are need to force unenroll etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just updated with more a per indice comment 👍

// Kibana write to this indice to add action to an agent, upgrade, unenroll, ...
RoleDescriptor.IndicesPrivileges.builder()
.indices(".fleet-actions")
.privileges("read", "write").build(),
// Kibana write to this indice new enrollment api key
RoleDescriptor.IndicesPrivileges.builder()
.indices(".fleet-enrollment-api-keys")
Copy link
Contributor

Choose a reason for hiding this comment

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

TLDR: If there are Kibana authz checks that can be bypassed by directly reading the enrollment api keys index, I suggest we make it a restricted index. This might be the case for the other fleet indices as well.

Users that have read access to all indices, eg:

POST /_security/role/read_all_indices
{
  "indices": [
    {
      "names": [ "*" ],
      "privileges": ["all"]
    }
  ]
}

can access dot-named indices (which are implicitly hidden) using the index name or, in the case of wildcards, with a request option.

For indices that contain sensitive data (more than just configuration data or raw-metrics, which is a nuisance to expose but is not equivalent to bypassing any other checks we have in place) we have an extra protection level, named restricted indices. The restricted indices concept is most likely to become superseded by system indices, but this is what we have and use currently.

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 was not aware of restricted indices @ruflin @blakerouse @aleksmaus do you think this will work for us. The mapping of this indices will be created with a regular user using an integration package.

Copy link
Contributor

Choose a reason for hiding this comment

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

Was also not aware of restricted indices concept. @scunningham @jaymode One more option ... ?

Copy link
Contributor

Choose a reason for hiding this comment

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

To maybe make it more clear what I mean, restricted indices confer addition protection by requiring explicit toggling of a flag when defining index permissions in a role, and relies on the expectation that roles in general don't have this flag toggled. Importantly, .kibana* is not a restricted index (but .security* is) which means that roles don't require the "hidden" flag toggled to grant access to it. Besides kibana there are other indices (think ML) that maybe should deserve more protection.

The fact that this is a new index and that it contains credentials prompted my suggestion.

But restricted indices is no silver bullet, we can just simply document non-fleet-admin users should not be granted privileges over these indices.

Copy link
Member

Choose a reason for hiding this comment

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

@ruflin sorry I have not had a chance to communicate this, but restricted indices will be used for external system indices. @tvernum and I discussed this on the external system indices doc and then again last week. I'll begin working on implementing external system indices soon :)

.privileges("read", "write").build(),
// Kibana write to this indice every policy change
RoleDescriptor.IndicesPrivileges.builder()
.indices(".fleet-policies")
.privileges("read", "write").build(),
// Fleet Server indices. Kibana read from these indices to manage Elastic Agents
RoleDescriptor.IndicesPrivileges.builder()
.indices(".fleet-servers")
.privileges("read").build(),
RoleDescriptor.IndicesPrivileges.builder()
.indices(".fleet-actions-results")
.privileges("read").build(),
},
null,
new ConfigurableClusterPrivilege[] { new ManageApplicationPrivileges(Collections.singleton("kibana-*")) },
Expand Down
38 changes: 38 additions & 0 deletions .../test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,44 @@ public void testKibanaSystemRole() {
assertThat(kibanaRole.indices().allowedIndicesMatcher(READ_CROSS_CLUSTER_NAME).test(mockIndexAbstraction(index)), is(false));
});

// read and write indices for Fleet Server
Arrays.asList(
".fleet-agents",
".fleet-actions",
".fleet-enrollment-api-keys",
".fleet-policies"
).forEach((index) -> {
assertThat(kibanaRole.indices().allowedIndicesMatcher("indices:foo").test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher("indices:bar").test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(DeleteIndexAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(GetIndexAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(CreateIndexAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(IndexAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(DeleteAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(UpdateSettingsAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(SearchAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(MultiSearchAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(GetAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(READ_CROSS_CLUSTER_NAME).test(mockIndexAbstraction(index)), is(false));
});
// readonly indices for Fleet Server
Arrays.asList(
".fleet-actions-results",
".fleet-servers"
).forEach((index) -> {
assertThat(kibanaRole.indices().allowedIndicesMatcher("indices:foo").test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher("indices:bar").test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(DeleteIndexAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(GetIndexAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(CreateIndexAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(IndexAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(DeleteAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(UpdateSettingsAction.NAME).test(mockIndexAbstraction(index)), is(false));
assertThat(kibanaRole.indices().allowedIndicesMatcher(SearchAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(MultiSearchAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(GetAction.NAME).test(mockIndexAbstraction(index)), is(true));
assertThat(kibanaRole.indices().allowedIndicesMatcher(READ_CROSS_CLUSTER_NAME).test(mockIndexAbstraction(index)), is(false));
});

// Data telemetry reads mappings, metadata and stats of indices
Arrays.asList(randomAlphaOfLengthBetween(8, 24), "packetbeat-*", "logs-*").forEach((index) -> {
Expand Down