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 support for secondary auth to Core's ES client #179458

Closed
angorayc opened this issue Mar 26, 2024 · 12 comments · Fixed by #184901
Closed

Add support for secondary auth to Core's ES client #179458

angorayc opened this issue Mar 26, 2024 · 12 comments · Fixed by #184901
Assignees
Labels
discuss Feature:elasticsearch Feature:Security/Authentication Platform Security - Authentication NeededFor:Fleet Needed for Fleet Team NeededFor:ML NeededFor:Threat Hunting 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!

Comments

@angorayc
Copy link
Contributor

angorayc commented Mar 26, 2024

Describe the feature:

We need a way to access an ES API with asInternalUser and pass the current user's credentials at the same time via es-secondary-authorization.
This could be a third “scoping” method to the Kibana Elasticsearch client, such as asXXXSecondary

Our current workaround is: https://elastic.slack.com/archives/C06L0LC2LA0/p1711443697698389?thread_ts=1710963106.929479&cid=C06L0LC2LA0

Describe a specific use case for the feature:
In this case the API can remain internal but it still knows the current user's access right.

@angorayc angorayc added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Mar 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@azasypkin azasypkin added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Mar 26, 2024
@elasticmachine
Copy link
Contributor

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

@pgayvallet
Copy link
Contributor

pgayvallet commented Mar 26, 2024

This could be a third “scoping” method to the Kibana Elasticsearch client, such as asXXXSecondary

Yeah... tbh, I'm not sure how I feel about changing our IClusterClient to add something I've never heard being used until today.

Would someone (@azasypkin maybe?) mind providing some context on what that es-secondary-authorization thingy is? Is that something new?

@angorayc
Copy link
Contributor Author

angorayc commented Mar 27, 2024

The ES api (get index count and size in serverless) we are going to access leverages secondary authentication to allow work to be done as a normal user (secondary auth) but passes the operator only REST API check (primary auth). This would allow the API to remain internal but the data/permissions to be representative of the non-operator user's sets of permission. This approach requires Kibana to make a call using 2 valid credentials (one of which is an operator, the other is users token credential.) More details about the API Here

ML & Fleet has a similar use case:
#154665
x-pack/plugins/ml/server/lib/request_authorization.ts
x-pack/plugins/fleet/common/types/models/transform_api_key.ts

Currently, the way they implement this is via asInternalUser + custom es-secondary-authorization header populated with request.headers.authorization.

Downside of this workaround:
(Below is a summary of this conversation)
The ability to access request.headers.authorization is a side-effect of the BWC code we have in place, mostly to support PDF Reporting, which needs to capture user credentials from the request. We're not particularly happy about exposing raw credentials like that and would like to eventually remove/replace this hack/workaround.

Using user credentials in this manner also has some drawbacks. For instance, if the bearer token expires after you've extracted it from the request object, Kibana won't attempt to transparently refresh it and retry the request, as it does when you use asCurrentUser. If you forward such a response directly to the "interactive" user (browser), they will be logged out. If not, you might have to retry the request manually somehow. It's not a very common scenario, but it does occur with SAML from time to time.

Given that this method will be used for more and more use cases in the future, it would make sense to add a third "scoping" method to the Kibana Elasticsearch client, such as asXXXSecondary. So that you don't have to handle the raw request and can benefit from automatic refresh/retry logic and any other logic we'll introduce in the future. We can discuss potential gotchas regarding serverless vs non-serverless usage that Luke mentioned there as well.

@angorayc
Copy link
Contributor Author

This could be a third “scoping” method to the Kibana Elasticsearch client, such as asXXXSecondary

Yeah... tbh, I'm not sure how I feel about changing our IClusterClient to add something I've never heard being used until today.

Would someone (@azasypkin maybe?) mind providing some context on what that es-secondary-authorization thingy is? Is that something new?

@pgayvallet
https://www.elastic.co/guide/en/elasticsearch/reference/current/http-clients.html#http-clients-secondary-authorization

This existing functionality allows authentication against 2 valid credentials and optional use the second set of credentials for custom work. Secondary authentication can be used such that the operator user’s credentials (e.g. Kibana user) is used to call the operator only REST APIs, but the normal user can be used to perform the transport action behind the REST API. So we can pass the operator only REST check, but swap out the user to do the actual work.

@azasypkin
Copy link
Member

Thanks for filing the issue and adding all the details @angorayc!

Would someone (@azasypkin maybe?) mind providing some context on what that es-secondary-authorization thingy is? Is that something new?

@angorayc covered it well in the previous comments, I just wanted to add that it's not really a new thing, since both Fleet and ML have already been leveraging secondary authentication for some time (I've learnt about it not so long ago though). And since we're going to have a third consumer that will rely on request.headers.authorization (which ideally shouldn't be available to anyone outside of the Core), we thought it might be a good idea to introduce a dedicated API. This way, we can remove request.headers.authorization more easily once we're ready (Reporting still needs it).

Happy to explain further if anything isn't clear, but essentially, we need a hybrid of asCurrentUser and asInternalUser where the credentials of the internal Kibana user are considered as main credentials and set via Authorization HTTP header, while the credentials of the current user are moved to the ES-Secondary-Authorization HTTP header.

@pgayvallet
Copy link
Contributor

So, I just checked with the ES team, and it appears that:

  1. this secondary authentication method is only supported by a few ES APIs (which is not surprising)
  2. this ES-Secondary-Authorization should only be used against APIs that are supporting secondary authentication (atm the header is ignored against other APIs, but they are thinking of rejecting the request altogether)

This second point makes is kinda awkward to expose a way to retrieve a full "secondary-auth-scoped" client that would send the header against all ES APIs, given we're not supposed to do so, which makes me wonder if that "third scoping method to client provider" is really the right approach.

E.g. my fear is that it would be very easy to do things like:

core.elasticsearch.asInternalUserWithSecondaryAuth(request).ping(); // ping doesn't support secondary auth

Although I can't really think of any better quick wins enforcing better guard rails right now (as we really don't want to manually maintain an interface with the list of APIs that support secondary auth, right?), and in a way, that approach would probably still be better than having API consumers manually inject the headers as done (especially given direct access to authz headers is supposed to be something we've being trying to suppress since... well at least since I joined..)

 headers: { 'es-secondary-authorization': request.headers.authorization }

@pgayvallet pgayvallet changed the title Add a third "scoping" method to the Kibana Elasticsearch client Add support for secondary auth to Core's ES client May 21, 2024
@azasypkin
Copy link
Member

E.g. my fear is that it would be very easy to do things like:
core.elasticsearch.asInternalUserWithSecondaryAuth(request).ping(); // ping doesn't support secondary auth

But is it that bad? I mean, we already have a similar behavior for asInternalUser — consumers can easily call it against an API the Kibana service account will never have access to, and hence the request will fail. I'd expect consumers to know when to use which scope, similar to knowing what parameters they send to the endpoint.

as we really don't want to manually maintain an interface with the list of APIs that support secondary auth, right?

Yeah, we definitely don't want to do that.

@pgayvallet
Copy link
Contributor

pgayvallet commented May 22, 2024

we already have a similar behavior for asInternalUser — consumers can easily call it against an API the Kibana service account will never have access to, and hence the request will fail

For me, it's not really the same behavior.

You can call any API with any user, and it may be rejected depending of the user's permissions
You can call an API not supporting secondary authentication with the secondary auth header, and it will be rejected because of invalid parameters / headers (regardless of the secondary user's permission or even identity)

First case is rejected because of authz, second case is rejected because of a bad usage of the API. it's not the same to me, is it?

Now regarding

But is it that bad?

It would be fine. I just wanted to take a bit of time thinking about it, because changing the IClusterClient interface is not a 3-line change, so I'd like to make sure we're not going in a direction we'll eventually regret later.

But I still don't see any better option (all "more DX friendly" alternative require to change the base Client interface, which is out of the question there), so unless someone wants to voice a proposal, I guess we're going with that IClusterClient. asInternalUserWithSecondaryAuth (real name TBD depending on my inspiration) API.

@azasypkin
Copy link
Member

azasypkin commented May 22, 2024

First case is rejected because of authz, second case is rejected because of a bad usage of the API. it's not the same to me, is it?

I see where you're coming from, and, yes, the underlying reason for failure when either of these APIs is misused isn't the same. What I meant is that conceptually, the reason we have asCurrentUser, asInternalUser, and might have asXXXSecondaryUser is very similar to me - we'd like to make it explicit and convenient for a consumer to call certain APIs with a specific set of credentials (credentials of the current user, credentials of the Kibana service account, or both).

Ideally, for both asInternalUser and asXXXSecondaryUser, we want to limit which ES APIs can be called. We don't do that for asInternalUser just because it's not worth it, which leads to the same behavior for the APIs we expose - we let consumers call any available API with any set of credentials, assuming they know what they are doing and defer "validation" to Elasticsearch, be it privilege checks for the service account (in both cases, and privileges are hard-coded directly in Elasticsearch) or something else.

I just wanted to take a bit of time thinking about it, because changing the IClusterClient interface is not a 3-line change, so I'd like to make sure we're not going in a direction we'll eventually regret later.

Sure, we're aligned here, and there certainly might be better options. It's just that I didn't manage to come up with anything more ergonomic, easily auditable, and still familiar to the existing consumers of IClusterClient.

@pgayvallet pgayvallet self-assigned this Jun 5, 2024
@pgayvallet
Copy link
Contributor

pgayvallet commented Jun 5, 2024

@azasypkin I took a shot at trying to implement this, and I'm facing an issue regarding retrieving the right value to set to the es-secondary-authorization header.

I wanted to naively re-use the existing mechanism that retrieves the user's credential when creating the primary-scoped client:

private getScopedHeaders(request: ScopeableRequest): Headers {
let scopedHeaders: Headers;
if (isRealRequest(request)) {
const requestHeaders = ensureRawRequest(request).headers ?? {};
const requestIdHeaders = isKibanaRequest(request) ? { 'x-opaque-id': request.id } : {};
const authHeaders = this.authHeaders ? this.authHeaders.get(request) : {};
scopedHeaders = {
...filterHeaders(requestHeaders, this.config.requestHeadersWhitelist),
...requestIdHeaders,
...authHeaders,
};

However I forgot that this.authHeaders.get returns a record of headers we're merging, not a single value.

So my question is, from this this.authHeaders.get, how should I do to retrieve the value to set to the es-secondary-authorization header?

Should I just naively assume there's only one entry in the record returned, and do something like Object.values(authHeaders)[0] 🙈? Or is there something better to do?

@azasypkin
Copy link
Member

Should I just naively assume there's only one entry in the record returned, and do something like Object.values(authHeaders)[0] 🙈? Or is there something better to do?

I checked all our providers that create these authHeaders, and it's always a single header, so this approach would technically be correct.

However, if we want to make it less ugly, we can change our internal API: make Core expect just one header (or rather one header value, assuming it's always for the Authorization HTTP header) and update the Security plugin to return just one header as well. I cannot think of any use case where a provider would need to return multiple headers. I assume we designed this API to be more generic back then since we didn't know what authentication mechanisms we'd have to support in the coming years. We also shouldn't worry much about backward compatibility, as it's highly unlikely anyone is replacing the security plugin. So, whatever approach works better for you.

pgayvallet added a commit that referenced this issue Jun 13, 2024
## Summary

Fix #179458

Add a third method to `IScopedClusterClient`, `asSecondaryAuth` which
allow performing requests on behalf of the kibana system users with the
current user as secondary authentication (via the
`es-secondary-authorization` header)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:elasticsearch Feature:Security/Authentication Platform Security - Authentication NeededFor:Fleet Needed for Fleet Team NeededFor:ML NeededFor:Threat Hunting 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!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants