Explore entrypoint binary redacting secrets from logs

[imjasonh]

Stemmed from #3365 though there have been other cases it's come up.

Today, if the user specifies a script: without shebang, we assume #!/bin/bash and set -euxo pipefail -- we consider this a reasonable default script preamble, but set -x especially can surprise some users that might be using secrets in their script, and don't expect those to be emitted to output logs. One solution (as described in #3365) is to set your own preamble or to otherwise disable -x so commands aren't logged.

But we might be able to do better. The entrypoint binary we inject to run the user's command (including scripts), today passes the sub-command's stdout directly to its own stdout (same with stderr). It could consider doing something smarter in this case, and take a set of byte sequences to redact from stdout/stderr, which could be passed as secret references into the Pod.

The entrypoint binary could then place this filter around stdout and stderr to replace detected secrets with some descriptive placeholder like [REDACTED], instead of passing through potentially sensitive valeus to logs.

Considerations:

• Performance overhead passing all user logs through our filter
• Limitations we should impose on filtering -- the target is presumably short-ish byte sequences (passwords and keys), not gigabytes of sensitive text
• Use cases where a user might want some secret value to be emitted through stdout/stderr, or would otherwise be surprised to find it being redacted
• User configuration to disable filtering, including rollout strategy to make it opt-in (and eventually opt-out?) -- should it be a per-run configuration, or per-installation feature flag?
• e2e tests we could write to check that filtering works

/kind feature

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[vdemeester]

/remove-lifecycle stale

This still needs investigation

[renannprado]

@imjasonh I'm not sure where this discussion would belong, but would be nice if Tekton also considers "secret parameters".

e.g. so that if I have a task whose output is a token (secret) I can pass to the next step seamlessly without having to create a k8s secret in order to have it [REDACTED] later.

[imjasonh]

@renannprado I think "secret params" (or results) would be a separate issue, though it might end up sharing the same redaction code. Results today are very much not secret, and shouldn't be treated as such. Anybody who can view a TaskRun's underlying Pod status can see results data, whether or not they have access to the TaskRun, etc. Redacting from logs could be useful, but we'd need to close that hole too before we recommend people put sensitive data in results.

[renannprado]

@imjasonh do you have interest in developing this feature? it doesn't seem very complicated to develop (I guess it depends on how hard is it to get the metadata to the pipeline inside of this method hehe), maybe I could develop it if some guidance/mentoring is provided. Of course, after developing it, I would love to contribute to a feature like the "secret parameters" too, but that's another story :)

[prashant-juluri]

A small suggestion but it might go a long way in enterprise adoption. Rather than show a static "redacted" message, why not show the key of the secret itself. That way, the users have a way of identifying which secret it is.

Secrets can differ across environments. Typically you may have different secrets driven by DBs for specific environment such as Dev, staging prod.

Would love to hear more on this!

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[renannprado]

IMHO redacting secrets is important and has to be implemented.

/reopen

[tekton-robot]

@renannprado: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

IMHO redacting secrets is important and has to be implemented.

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[afrittoli]

/remove-lifecycle rotten
/lifecycle frozen
/reopen

[tekton-robot]

@afrittoli: Reopened this issue.

In response to this:

/remove-lifecycle rotten
/lifecycle frozen
/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.