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

WIP Experiment in handling Sensitive keys #6595

Closed
wants to merge 5 commits into from
Closed

Conversation

notnoop
Copy link
Contributor

@notnoop notnoop commented Oct 30, 2019

Nomad has sensitive keys (e.g. ACL/Vault/Consul tokens) that Nomad ought not to log or expose in HTTP. One approach is to wrap the values in types such that they get redacted when logged.

Here, I attempt this approach to report early feedback and finding.

In this implementation, I introduce Sensitive string wrapper that is automatically redacted when logged and serialized in json but not when persisted in client state or RPC responses. To access the value, one must use the Plaintext() method to access value.

My experience has been relatively mixed:

  • We have many sensitive values that get persisted and returned in APIs (e.g. ACL Tokens is the clear example)
    • Some sensitive keys are never persisted or passed around (e.g. replication/consul/telemetry tokens).
  • Redaction on logging but not on API response is quite accidental.
    • HCLLog uses encoding/json while agent http handler and persistence uses ugorji/go for serialization.
    • Use use serialization to pass values to auxiliary processes (e.g. external drivers) and we may need to pass some sensitive tokens
    • when using the same library, it's not trivial to redacted based on context.

Though the approach is neat, given that we pass secrets around in RPC, I am not fully convinced this is useful as it is.

Some possible next steps:

  • introduce a Secret level for values that should only be stored in memory and never persisted or returned in API/RPC.
  • Change app such that we only expose a secret id once and never again.

@notnoop notnoop closed this Nov 19, 2019
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants