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

Restricted Elasticsearch Clients #81237

Open
legrego opened this issue Oct 20, 2020 · 9 comments
Open

Restricted Elasticsearch Clients #81237

legrego opened this issue Oct 20, 2020 · 9 comments
Labels
discuss enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! unified-security-painpoint Highlights issues that are painpoints as a result of the lack of a unified security model

Comments

@legrego
Copy link
Member

legrego commented Oct 20, 2020

Background

Kibana's authorization model relies on making certain requests to Elasticsearch as the kibana_system user, on behalf of the end user.

For example, accessing a dashboard roughly looks like:

image

Securing access to saved objects happens mostly transparently to engineers. They'd have to go out of their way to access saved objects in a way which bypasses security controls.

Problem

Say for example that I want to retrieve a list of all users within the native realm via the /_security/users ES API. If I were to grant the kibana_system user access to this endpoint, then I could make this information available to Kibana users, even if they themselves didn't have the proper ES privileges. We can instead leverage the kibana privilege model to authorize the end user according to our own rules before making the request as kibana_system on their behalf.

This is all possible today, but it doesn't prevent engineers from mistakenly bypassing Kibana's authorization checks. It's entirely possible for another plugin to get an instance of the ElasticsearchClient scoped to the internal user, and then make any request they'd like. In doing so, they would bypass our authorization checks, audit logging, etc. The new client has great TS support, but I fear that makes the available options even more discoverable, and engineers might not know that they shouldn't be interacting with some of these endpoints directly.

This isn't a problem yet, but I think it would improve the dx and help keep Kibana secure if we could restrict which endpoints plugins could call from the ElasticsearchClient exposed by core. In this example above, I wouldn't want other plugins to be able to call client.security.getUser() directly. I would instead want to force them to consume the security plugin's public contract in order to call our version, where we can intercept the call, perform the necessary authorization & audit logging, and only then return the response.

@legrego legrego added discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! enhancement New value added to drive a business result labels Oct 20, 2020
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet
Copy link
Contributor

pgayvallet commented Oct 21, 2020

This isn't a problem yet, but I think it would improve the dx and help keep Kibana secure if we could restrict which endpoints plugins could call from the ElasticsearchClient exposed by core. In this example above, I wouldn't want other plugins to be able to call client.security.getUser() directly.

We could eventually restrict some client API calls to be restricted to be only performed by core (even if it would be a mess honestly, as we decided to expose a plain/vanilla client instead of a wrapper).

However here, you would need to basically only have the security plugin have access to such endpoints. Do you have any proposal on a technical design that would allow core to restrict access ONLY for part of the plugins?

@legrego
Copy link
Member Author

legrego commented Oct 22, 2020

Do you have any proposal on a technical design that would allow core to restrict access ONLY for part of the plugins?

I was intentionally not proposing solutions just yet, because I didn't want to pigeonhole the discussion. I do have one idea, but I'm not sure how feasible it is:

We could eventually restrict some client API calls to be restricted to be only performed by core (even if it would be a mess honestly, as we decided to expose a plain/vanilla client instead of a wrapper).

Yeah I'm not too keen on a wrapper either. I really like the idea of exposing the plain client. Could we create the plain client without specific endpoints? If so, then each plugin could get a restricted version of the client by default, unless they've indicated that they need access to some privileged endpoints.

We could add an optional entitlements property to the kibana.json spec to indicate that a plugin should be entitled to access specific resources. Or this could be done during the setup phase since the new client is only available during start, but I worry that will make this hard to audit.

{
  "id": "security",
  "version": "8.0.0",
  "kibanaVersion": "kibana",
  "configPath": ["xpack", "security"],
  "requiredPlugins": ["data", "features", "licensing", "taskManager", "securityOss"],
  "optionalPlugins": ["home", "management", "usageCollection"],
  "server": true,
  "ui": true,
  "requiredBundles": [],
  "entitlements": [{
    "elasticsearchClient": {
      "privilegedEndpoints": ["security.getUser"] 
    }
  }]
}

@azasypkin
Copy link
Member

azasypkin commented Oct 26, 2020

We could add an optional entitlements property to the kibana.json spec to indicate that a plugin should be entitled to access specific resources. Or this could be done during the setup phase since the new client is only available during start, but I worry that will make this hard to audit.

💯 I really hope we can go with a declarative approach using kibana.json for things like this. Similar use cases (fs access, native controllers, etc.) have come up multiple times when we were designing kibana.json in the past. The we don't necessarily have to restrict plugins in what they can do, if we can easily audit them sounds as both scalable and secure enough approach to me.

@mshustov
Copy link
Contributor

mshustov commented Oct 26, 2020

We could add an optional entitlements property to the kibana.json spec to indicate that a plugin should be entitled to access specific resources. Or this could be done during the setup phase since the new client is only available during start, but I worry that will make this hard to audit.

It makes an audit harder, but it gives plugins flexibility to implement an additional logic (such as rely on config values, for example) and adds type safety for further changes.

If so, then each plugin could get a restricted version of the client by default

a restricted version is a client without any privileged endpoints? What are those privileged endpoints?

Yeah I'm not too keen on a wrapper either. I really like the idea of exposing the plain client. Could we create the plain client without specific endpoints?

I'm not sure how we can implement it without a wrapper because the client must not have access to privileged endpoints types & runtime API.

@legrego
Copy link
Member Author

legrego commented Oct 26, 2020

a restricted version is a client without any privileged endpoints? What are those privileged endpoints?

Those endpoints are still TBD. In general we wouldn't expect other plugins to use a vast majority of the client.security.* endpoints. A POC I'm working on would need to exclude client.security.getUser(), and I expect that we would want to prevent plugins from accessing the ML endpoints outside of the ML plugin (They rely on the kibana_system user to delegate access to ML jobs, much like we do for the kibana index today).

I'm not sure how we can implement it without a wrapper because the client must not have access to privileged endpoints types & runtime API.

I don't have a great solution for this just yet, unless we could upstream a change to allow "partial" clients to be created.

@rudolf
Copy link
Contributor

rudolf commented Nov 12, 2020

Because the system indices Elasticsearch plugin will only expose a subset of API's it would be nice if the dedicated system indices client only had the types for those API's #82716

Perhaps we can just exclude the sensitive APIs from the internal user client? Relying on Typescript is not a strong control, but if a developer tries to make an API call which results in a TS error it's a very strong signal that they are doing something wrong.

@pgayvallet
Copy link
Contributor

pgayvallet commented Nov 13, 2020

For system indices, the subset of ES APIs we want to use/allow is known and static, so we could use the same approach we did for the saved object 'retry' client, using composition on the base ElasticsearchClient type and extract the methods we need for the implementation. Note that this represents some additional work/load every time a client is created, not sure of the performance impact.

const methods = [
'bulk',
'create',
'delete',
'get',
'index',
'mget',
'search',
'update',
'updateByQuery',
] as const;
type MethodName = typeof methods[number];
export type RepositoryEsClient = Pick<ElasticsearchClient, MethodName>;
export function createRepositoryEsClient(client: ElasticsearchClient): RepositoryEsClient {
return methods.reduce((acc: RepositoryEsClient, key: MethodName) => {
Object.defineProperty(acc, key, {
value: async (params?: unknown, options?: TransportRequestOptions) => {
try {
return await retryCallCluster(() =>
(client[key] as Function)(params, { maxRetries: 0, ...options })
);
} catch (e) {
throw decorateEsError(e);
}
},
});
return acc;
}, {} as RepositoryEsClient);
}

We could also, as suggested by @rudolf, just expose a limited ts type, while still providing a 'full' client as the implementation. As the ES call would fails anyway, only having TS as guard control seems good enough to me.

For restricted client, I'm not sure of the feasibility, as using the same approach would constraint us to only have statically defined lists of allowed endpoint, which is not what

"entitlements": [{
    "elasticsearchClient": {
      "privilegedEndpoints": ["security.getUser"] 
    }
  }]

is suggesting. In the case we would really want that kind of manifest-based permissions, the best we could do would be to expose the whole client type, and throw errors when invoking forbidden apis. This still requires a wrapper of some kind around the elasticsearch Client, which we don't have atm.

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Aug 5, 2021
@legrego legrego removed EnableJiraSync loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Aug 18, 2022
@legrego legrego added the unified-security-painpoint Highlights issues that are painpoints as a result of the lack of a unified security model label Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New value added to drive a business result Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! unified-security-painpoint Highlights issues that are painpoints as a result of the lack of a unified security model
Projects
None yet
Development

No branches or pull requests

6 participants