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

[Telemetry] Pull Kibana usage stats on-demand if Monitoring is not enabled. #21241

Closed
tsullivan opened this issue Jul 25, 2018 · 9 comments
Closed
Assignees
Labels
enhancement New value added to drive a business result Feature:Telemetry Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@tsullivan
Copy link
Member

tsullivan commented Jul 25, 2018

Historically, the Telemetry feature was a part of monitoring and it had to be opted-into through code that existed in the monitoring plugin. That coupling has been separated and Telemetry is now a feature of the xpack_main "meta" plugin. Telemetry even works if Monitoring is disabled - instead of querying for usage data in .monitoring- indices, there are modules in x-pack/plugins/xpack_main/server/lib/telemetry/local that pull the usage data "live" from Elasticsearch. Live pulling is limited to Elasticsearch because at the time it was originally implemented, there was no way to pull usage data from other parts of the stack like Kibana, Beats or Logstash.

Now with the usage service in Kibana under src/server/usage, a live-puller does have an interface to call and get the registered Kibana usage collectors to fetch data: server.usage.collectorSet.bulkFetchUsage(). That call can be integrated in some module in x-pack/plugins/xpack_main/server/lib/telemetry/local and telemetry payload would be able to include Kibana stats when Monitoring is disabled.

cc @jinmu03

@tsullivan tsullivan added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc enhancement New value added to drive a business result Feature:Telemetry labels Jul 25, 2018
@tsullivan
Copy link
Member Author

tsullivan commented Aug 8, 2018

This will require include moving the Kibana usage collector out of monitoring and into src/server/usage/collectors. That way local telemetry pulling will include it

@tsullivan
Copy link
Member Author

This is blocked on Security team creating a "Summary Client" that can return a callCluster function that is authenticated for querying anonymous, summary-only usage data from all the Spaces in .kibana.

How it will work securely: the live stats can be pulled through the server.collectorSet.bulkFetchUsage function, which requires a callCluster function parameter. The Kibana Security team will build a "Summary Client" to return a callCluster function to use. It will manage the concerns about users requesting stats from all the Spaces in Kibana and make sure they can only request anonymous summarized usage data from across Spaces.

@tsullivan
Copy link
Member Author

tsullivan commented Nov 29, 2018

@kobelb and I chatted and we've come to a 2-part solution

Collection

When Monitoring is disabled:

  • User loads a page in Kibana
  • Telemetry timer determines it's time to get a payload to sent to Elastic servers.
  • Browser makes an automatic request to the Kibana server to get stats
  • Server finds Monitoring is disabled and will pull stats live (instead of from monitoring data)
  • Server checks user privileges to see if user could potentially read .monitoring* indices. [*] They will be able to if:
    • Security is turned off
    • They are a superuser
    • An admin gave them a monitoring_user role for some reason; maybe Monitoring used to be enabled.
  • If their privileges pass, then we use callWithInternalUser to build a callCluster function that gets passed to server.collectorSet.bulkFetchUsage. That gets the same scope of data as internal collection would get.

[*] Why we check the user potentially could read monitoring data: If the user is someone who should not be able to personally view Kibana-wide stats, then they should not be able to pull the stats live. Essentially, the solution is to simulate a callWithRequest call that reads Kibana-wide stats, which they would be able to see if they were a Monitoring user.

Transmission

We're putting reliance on a superuser being logged in so they'll be able to collect the Kibana stats. We will remove the 5-minute timer that delays transmission of the telemetry data to Elastic servers. The 5-minute timer was added to give a user time to read the "opt out" message when you had to opt-out to keep stats from sending. The behavior is opt-in now, so if telemetry is enabled in the browser, we know the user is already opted in. If a superuser is logged in, they might not stay on the page for very long, so there shouldn't be a delay on collecting and transmitting stats to Elastic servers.

@skearns64
Copy link

Server checks user privileges to see if user could potentially read .monitoring* indices. [*]

I'm not sure we need this security check, tbh - when we originally implemented the ES side of "live" telemetry collection, we agonized over the same thing, and chose to collect the monitoring data from ES with the kibana system user. There were a few reasons, but my thinking was that we have a clear and transparent opt-in process, this isn't sensitive data, and if someone does consider it to be sensitive data, they are going to disable telemetry entirely, so this won't be an issue.

@tsullivan
Copy link
Member Author

I'm taking assignment of this issue to get the change in as an enhancement in 6.6

@pickypg would you be available to review the changes?

@pickypg
Copy link
Member

pickypg commented Nov 30, 2018

@tsullivan Yep!

@kobelb
Copy link
Contributor

kobelb commented Dec 3, 2018

I'm not sure we need this security check, tbh - when we originally implemented the ES side of "live" telemetry collection, we agonized over the same thing, and chose to collect the monitoring data from ES with the kibana system user. There were a few reasons, but my thinking was that we have a clear and transparent opt-in process, this isn't sensitive data, and if someone does consider it to be sensitive data, they are going to disable telemetry entirely, so this won't be an issue.

I don't feel comfortable removing the authorization entirely; however, I do feel comfortable with ensuring that the user has access to read all saved objects across all spaces via the Kibana application privileges or through having read-only access to the .kibana index. This will hopefully get more users authorized to report telemetry without violating the fundamental concern that we have of disclosing the existence of objects which you don't have access to.

When users opted into telemetry, they opted into reporting the anonymous statistic to Elastic itself, and I don't feel that the disclosure of this information to end-users was explicit.

There are a few things that we talked about during our Security sync today that a few members of the @elastic/kibana-platform team also attended that can improve this situation. We can explore using asymmetric encryption to report these statistics so that end-users aren't able to view these statistics themselves. Additionally, we can also explore having the Kibana server itself report this telemetry when it's able to. I'm sure there were others ideas floated that I'm missing as well, and elaboration upon these is likely required; however, it feels out of scope for the current issue at hand.

@jinmu03
Copy link
Contributor

jinmu03 commented Dec 4, 2018

Meeting summary:

In the short-term we're alright continuing the model that we've implemented for the Elasticsearch stats collection and allow any authenticated user to live-pull this information. In the not-long-term but not-blocking-this-effort-term, we'd like to prioritize sending all telemetry stats using public/private key encryption to prevent the relaying user from being able to sniff or augment the payload (edited)

It's worth having a discussion regarding which team will be implementing the asymmetric encryption solution, since platform will technically be owning telemetry long-term.

@tsullivan
Copy link
Member Author

PR for fix is ready for review: #26496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Telemetry Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

5 participants