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

[Fleet] Exposed service to manipulate fleet-* should use internal ES client #116182

Closed
Tracked by #108252
nchaulet opened this issue Oct 25, 2021 · 10 comments · Fixed by #119017
Closed
Tracked by #108252

[Fleet] Exposed service to manipulate fleet-* should use internal ES client #116182

nchaulet opened this issue Oct 25, 2021 · 10 comments · Fixed by #119017
Assignees
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture v8.0.0

Comments

@nchaulet
Copy link
Member

nchaulet commented Oct 25, 2021

Description

Elasticsearch is restricting how an user can system indices by introducing allow_restricted_indices.

Fleet expose some services like the agent service that interact with .fleet-* indices (system indices. In order to avoid to give users read permissions to the fleet-* index and to avoid bugs we should probably change the signature of the services we expose from Fleet and provide ourself the elastic search client using the kibana system user.

Implementation details potential solutions

We have an internal user client available in our appContextService that we can use for that.

We should probably remove the esclient argument from all of the methods from the agent service see https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/services/index.ts/#L46-L74

We could also rely on having the request as an argument, I know this make this service a little more hard to consume outside of a request context (like in a background task for example)

@nchaulet nchaulet added technical debt Improvement of the software architecture and operational architecture Team:Fleet Team label for Observability Data Collection Fleet team labels Oct 25, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@paul-tavares
Copy link
Contributor

Thanks @nchaulet for creating this.
The agent service is not the only one that is exposed via the FleetstartContract to dependent plugins. There is also PackageService, PackagePolicyService and AgentPolicyServiceInterface

See here:

packageService: PackageService;
agentService: AgentService;
/**
* Services for Fleet's package policies
*/
packagePolicyService: typeof packagePolicyService;
agentPolicyService: AgentPolicyServiceInterface;

FYI: in the endpoint API, for package and agent policy services, we are using an internal SO client to get around the privileges issues, but if these could also abstract away the need to pass one in that would be awesome. See here:

async getFleetAgentPolicy(agentPolicyId: string): Promise<AgentPolicyWithPackagePolicies> {
const agentPolicy = await this.agentPolicyService
.get(this.DANGEROUS_INTERNAL_SO_CLIENT, agentPolicyId, true)
.catch(catchAndWrapError);
if (agentPolicy) {
return agentPolicy as AgentPolicyWithPackagePolicies;
}
throw new FleetAgentPolicyNotFoundError(
`Fleet agent policy with id ${agentPolicyId} not found`
);
}

(don't laugh at the class variable name 😆 )

@nchaulet nchaulet changed the title [Fleet] Exposed agent service should use internal ES client [Fleet] Exposed service to manipulate fleet-* should use internal ES client Oct 25, 2021
@joshdover
Copy link
Contributor

The agent service is not the only one that is exposed via the FleetstartContract to dependent plugins. There is also PackageService, PackagePolicyService and AgentPolicyServiceInterface

I think we should only change services that require access to .fleet-* indices to avoid using the privileges of the internal user where possible.

Though I think this will also depend on whether or not we end up using Kibana application privileges or Elasticsearch privileges to allow non-superusers to access Fleet. If we end up using Kibana privileges, we'll always need to use the internal user and should provide some utility functions for doing authz checks for the current request at the API level.

We could also rely on having the request as an argument, I know this make this service a little more hard to consume outside of a request context (like in a background task for example)

+1, let's avoid using the request where possible to keep this APIs flexible. This does mean that all consumers of our APIs need to be enforcing authz themselves (with the provided utility APIs)

@joshdover
Copy link
Contributor

I believe we only need to address this when we decide to tackle #108252. Otherwise, it's not a hard requirement for 8.0 since all (super)users of Fleet will already have this privilege so using the client configured with their credentials will continue to work.

@nchaulet
Copy link
Member Author

I believe we only need to address this when we decide to tackle #108252. Otherwise, it's not a hard requirement for 8.0 since all (super)users of Fleet will already have this privilege so using the client configured with their credentials will continue to work.

I think it's already a requirement for other plugins consuming Fleet API where the user is not required to be a superuser like endpoint.

For the fleet App your are right is not urgent as we require the superuser role.

@paul-tavares
Copy link
Contributor

💡 Maybe the Fleet exposed services could have a .asInternalUser() mechanism for cases where other plugins may need to retrieve data in cases where the user might not have privileges.

Btw - I assume we would allow this (the ability to pull data out of fleet using the internal kibana user) only for read. All creates/updates/deletes would still need to have proper auth - do you all agree?

In Endpoint security, we have one flow that updates Endpoint Integration Policy - my assumption is that for that flow (if they ever decided to apply fine-grained RBAC) is that a user would need to have Access in fleet to update that integration. I don't think we (endpoint) would create our own API for that to bypass Fleet privileges requirements

@joshdover
Copy link
Contributor

All creates/updates/deletes would still need to have proper auth - do you all agree?

Yes, but they won't have direct privileges to do things like install index templates, etc. Instead, users will be expected to have a fleet-specific privilege that we check for on Kibana's server-side and if they do, we use the underlying system user to execute the Elasticsearch API calls.

The recommended way I have of respecting these privilege requirements would be to call Fleet's HTTP API directly which will always do these authz checks. However in cases where Fleet operations may need to be executed as part of another server-side API, plugins will need to execute the authz checks directly. But like I said, we can provide some utilities to make this easier:

  1. Explicit authz APIs like hasAgentReadAccess(creds) that you must call directly before running internal operations.
  2. APIs that accept a KibanaRequest as a parameter and do the checks for you under the hood and then use the internal user for the actual requests to ES.

I think we will need (1) anyways for any background operations that may not have a request object, but (2) is likely to be more future-proof, since applications wouldn't need to make any changes as we add additional privilege levels in the future. I know this a bit contrary to what we were saying above, but now that I'm looking at our longer term plans to introduce finer-grained privileges to Fleet progressively, I think (2) is probably the safer option that also requires less cross-team coordination as the privilege model evolves.

I think it's already a requirement for other plugins consuming Fleet API where the user is not required to be a superuser like endpoint.

Good point 👍 Didn't realize apps were already doing this today.

@joshdover joshdover self-assigned this Nov 18, 2021
@joshdover
Copy link
Contributor

I've put up a POC here that implements both a scoped version of the agent service and a internal user version. PTAL: #119017

@joshdover
Copy link
Contributor

Note that we also need to do something similar for our access to .fleet-enrollment-api-keys in x-pack/plugins/fleet/server/services/api_keys/enrollment_api_key.ts. This is needed for the switch to system indices planned for 8.0.

@joshdover
Copy link
Contributor

Note that #119992 switched over to use the internal user for all accesses to .fleet-* indices within Fleet's HTTP APIs.

The only other service besides agentsService that requires an ES client is the createArtifactsClient and it is already using the internal user as well. This issue will close once #119017 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture v8.0.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants