elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.65k stars 8.23k forks source link

[Audit] CCS `:` index pattern delimiter parsing #193906

Closed thomasneirynck closed 1 month ago

thomasneirynck commented 1 month ago

To access failure store data, Elasticsearch is looking to introduce :: as a new delimiter (https://github.com/elastic/dev/issues/2698).

This introduction would break naive implementations of detecting whether an index pattern references a remote cluster, which uses a single colon (:).

A quick search (https://github.com/elastic/dev/issues/2698#issuecomment-2368745142) has shown that Kibana has many naive implementations. These just check for the presence of the :-character, rather than check that it is being used as a delimiter within a string.

e.g. String.prototype.includes(':') is often used. This is incorrect.

We need to do a full audit to get a better sense of what the blast radius of this change would be.


Where does Kibana detect if an index-pattern is a CCS-pattern?

Any other possible use-cases where the introduction of :: could cause failures?

Are there any other instances Kibana would be parsing an index-pattern string and where the introduction of :: could cause failures?


Introduce package with parsing functionality

There is an opportunity to align implementations here.

To fix this consistently across apps, Platform should provide some utility functions (e.g. along the lines of function isCCSPattern(pattern: string) : boolean) to perform this check.

Audit Partners

Please check the box when code under your control has been verified to be compatible.

Observability

Search Solution

jasonrhodes commented 1 month ago

In our @elastic/obs-ux-management-team areas, which include Alerting, SLO, Synthetics, and Uptime, we've found at least one instance so far, in the SLO plugin.

https://github.com/elastic/kibana/blob/ac96a4ca6d9168b9d22b2e92514f6761131e8fea/x-pack/plugins/observability_solution/slo/server/services/summary_search_client.ts#L228-L230

I'll update this ticket if we find more, and we'll create an issue for the above ^

gbamparop commented 1 month ago

Updated the description with some usages in the apm plugin.

thomasneirynck commented 1 month ago

Updated platform side.

fyi @jgowdyelastic : there are some ML-plugins in here @davismcphee @stratoula others are ES|QL / data-view related

stratoula commented 1 month ago

Yes we have a CCS check in ES|QL autocomplete but I am not sure if we need this tbh. I need to check

stratoula commented 1 month ago

I checked our usage and I think we should remove this check. I am doing this here https://github.com/elastic/kibana/pull/194903

thomasneirynck commented 1 month ago

@YulNaumenko Could you provide an update from the Security side? thx!

@joemcelroy After our off-thread discussion, I think there were no usages in Search? Do you mind confirming and updating the description? thx!

joemcelroy commented 1 month ago

Yep I confirmed in the enterprise-search plugin there is no usage o CCS : index pattern. updated the description under search solution.

thomasneirynck commented 1 month ago

short update:

I will add a centralized utility function to a package to parse these index-patterns, and which we then can use to replace these includes and indexOf-usages. (similar to how ES isolated this into a utility https://github.com/elastic/elasticsearch/pull/113501/files#diff-54b4d10d02244bb6e2b3413b315759f96cc44c6344c59358dabd304dd8def30aR61).

I will do a first pass at resolving these usages that are listed here, but there may be some follow-up/review from the solutions teams on this.

Will ping when there's progress. Thx all.

YulNaumenko commented 1 month ago

@thomasneirynck I've listed the single usage we have at Security Solution, which is a part of the Detections Engine.