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

Pipe X-Opaque-Id header to AuditTrail logs and Elasticsearch API calls #62018

Closed
joshdover opened this issue Mar 31, 2020 · 7 comments · Fixed by #71019
Closed

Pipe X-Opaque-Id header to AuditTrail logs and Elasticsearch API calls #62018

joshdover opened this issue Mar 31, 2020 · 7 comments · Fixed by #71019
Assignees
Labels
enhancement New value added to drive a business result Feature:New Platform NeededFor:Security Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@joshdover
Copy link
Contributor

Supports #52125
Related #60119

We need to be able to correlate different logs in the regular system logger, Kibana's audit logs, and Elasticsearch's audit logs to the same user request.

When the X-Opaque-Id has been set on the request (for example, by a reverse proxy), we should:

  • Forward this header to Elasticsearch
    • This may already be solved by the elasticsearch.requestHeadersWhitelist configuration option. This should be verified that this works.
  • Include the identifier on any AuditTrail logs emitted during an HTTP request. This should automatically be bound via the request context.
@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform enhancement New value added to drive a business result labels Mar 31, 2020
@elasticmachine
Copy link
Contributor

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

@joshdover
Copy link
Contributor Author

@jportner Previously we mentioned adding a config that allows the operator to specify which IP addresses this header should be accepted from. Is that a requirement for the MVP?

@jportner
Copy link
Contributor

jportner commented Jul 1, 2020

@jportner Previously we mentioned adding a config that allows the operator to specify which IP addresses this header should be accepted from. Is that a requirement for the MVP?

No, but we should provide a config option to disable this behavior.

@mshustov
Copy link
Contributor

mshustov commented Jul 7, 2020

@jportner What are the requirements regarding X-Opaque-Id for FakeRequest? I suspect they shouldn't re-use one from the "parent" request, otherwise, several operations would have the same id. Should it be generated for every "task"? Then we might need to introduce an explicit notion of execution context for such cases. @dover is this covered by #60122?

@jportner
Copy link
Contributor

jportner commented Jul 7, 2020

@jportner What are the requirements regarding X-Opaque-Id for FakeRequest? I suspect they shouldn't re-use one from the "parent" request, otherwise, several operations would have the same id. Should it be generated for every "task"?

I would expect that any Elasticsearch operation resulting from a single inbound HTTP request would have the same X-Opaque-Id header. So, in that sense, yes several operations would have the same X-Opaque-Id header in the Elasticsearch audit records.

I don't necessarily think background "tasks" should be treated the same way, though. Perhaps @thomheymann and/or @arisonl can weigh in.

@thomheymann
Copy link
Contributor

@jportner What are the requirements regarding X-Opaque-Id for FakeRequest? I suspect they shouldn't re-use one from the "parent" request, otherwise, several operations would have the same id. Should it be generated for every "task"?

I would expect that any Elasticsearch operation resulting from a single inbound HTTP request would have the same X-Opaque-Id header. So, in that sense, yes several operations would have the same X-Opaque-Id header in the Elasticsearch audit records.

Agreed 👍

I don't necessarily think background "tasks" should be treated the same way, though. Perhaps @thomheymann and/or @arisonl can weigh in.

Do you have an example of such a background task? The way I understand the aim of the X-Opaque-Id header is to be able to correlate log lines in Kibana with those in subsequent systems (e.g. ElasticSearch). In systems I worked on previously these were exclusively triggered by incoming HTTP requests but if Kibana itself is the initiator maybe Kibana should generate an id in those cases too if we need to correlate such activities?

@joshdover
Copy link
Contributor Author

Do you have an example of such a background task? The way I understand the aim of the X-Opaque-Id header is to be able to correlate log lines in Kibana with those in subsequent systems (e.g. ElasticSearch). In systems I worked on previously these were exclusively triggered by incoming HTTP requests but if Kibana itself is the initiator maybe Kibana should generate an id in those cases too if we need to correlate such activities?

Some examples of background tasks:

  • Executing an alert
  • Rendering a report PDF
  • Processing a reindex operation for the Upgrade Assistant

I generally agree that it makes sense to use a unique identifier per invocation of a given task or job. The tricky part is where in the actual code do we generate this and how do we ensure that all audittrail events are bound to that ID.

My plan is to leverage the AuditTrail.asScoped(request) function for this and use the same logic we use for sourcing the identifier as we would normal request.

  • If request includes the X-Opaque-Id header from a trusted remote IP, use this header's value
  • Otherwise, generate a new UUIDv4

This assumes that the AuditTrail.asScoped function will eventually support FakeRequest for the background task use case (#39430). Briefly looking at the current implementation, I believe there will need to be some changes to ensure that a new identifier is created for each separate invocation of a task and not just once per set of authentication credentials. This may require that plugins that have background tasks call an AuditTrail API on each task execution. Later this could be integrated into the future Background Task service (#18854) to be handled by Core automatically.

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:New Platform NeededFor:Security Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants