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

redact secrets in diagnostics collect command #241

Closed
michel-laterman opened this issue Mar 23, 2022 · 6 comments · Fixed by #566
Closed

redact secrets in diagnostics collect command #241

michel-laterman opened this issue Mar 23, 2022 · 6 comments · Fixed by #566
Assignees
Labels
debugging estimation:Day Task that represents a day of work. Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.4.0

Comments

@michel-laterman
Copy link
Contributor

Describe the enhancement:

The elastic-agent diagnostics collect command creates an archive that config files that have secret/sensitive data.
These fields should be redacted before being written to the archive.

api_key:  "<REDACTED>"
key_passphrase: "<REDACTED>"
access_api_key: "<REDACTED>"

Additionally the private key of a cert should be redacted if it's inlined in the config and not a path to a file.

@jlind23
Copy link
Contributor

jlind23 commented May 5, 2022

First step here, hide fields that follow some common patterns:

  • key
  • password
  • passphrase
  • token

@joshdover @ph could you please think about the long term solution and how we can mark fields as sensitive right from the integration definition?

@jlind23 jlind23 added v8.4.0 estimation:Day Task that represents a day of work. and removed 8.4-candidate v8.4.0 labels May 24, 2022
@jlind23
Copy link
Contributor

jlind23 commented May 25, 2022

@joshdover @ph as the package spec PR is now done, do we really need to do this intermediary step #241 (comment) Shouldn't we directly rely on the secret type?

@joshdover
Copy link
Contributor

We could still do the intermediate step as a stop-gap, but probably better to spend effort on the long-term solution.

@ph
Copy link
Contributor

ph commented May 25, 2022

I believe we should do this, the long-term solution would only be delivered on 8.x and won't be backported to 7.x. Since we will support 7.x for quite some time and use the diagnostic subcommand time we should have that stop gap in and backported to 7.x.

@AndersonQ
Copy link
Member

First step here, hide fields that follow some common patterns:

  • key
  • password
  • passphrase
  • token

Another pattern would be certificate or other terms we use for the ssl/certificates configuration

@ghost
Copy link

ghost commented Jul 19, 2022

Hi @AndersonQ,

We have verified below check-points for this feature on the latest 8.4.0-SNAPSHOT build.

Build details:

Version: 8.4.0 SNAPSHOT
Build: 54585
Commit: f9e2ed4d9f38424676a558c34b74f0a031746c9e

Observations

  • REDACTED value updated for below keys in the following files after using elastic-agent diagnostics collect command
Under Config folder:
1.  access_api_key is <REDACTED> in the elastic-agent-local.yaml file 
2.  api_key is <REDACTED> in the **elastic-agent-policy.yaml file , filebeat_default.yaml file , fleet_monitoring_default.yaml file and metricbeat_default.yaml file **
3. `access_api_key`, `service_token`, `certificate`, `key` are <REDACTED> in the elastic-agent-local.yaml file 

Further, for the ssl/cert configuration, we have following Queries:

Query 1: Could you please confirm if we need to validate any other check-points for the ssl/cert configuration other than elastic-agent-local.yaml file.

Query 2: Could you please confirm ca-trusted_fingerprint value is not displayed as REDACTED right now in following config files. Is it expected.

  • elastic-agent-local.yaml elastic-agent-policy.yaml filebeat_default.yaml fleet_monitoring_default.yaml & metricbeat_default.yaml .

image

image

image

Please let us know if we are missing anything while validating this feature.

Thanks!

CC: @michel-laterman

@ghost ghost added QA:Validated Validated by the QA Team and removed QA:Validated Validated by the QA Team labels Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugging estimation:Day Task that represents a day of work. Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v8.4.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants