vminitd: Scrub envvars printed in debug log#521
Conversation
We print the OCI spec at debug level, and this can contain sensitive info in envvars specifically. Lets redact the values in the envvars before we print. I went the route of just redacting everything as trying to check against some "possibly sensitive looking vars" seems rife for never getting it fully right. This is what the envvars look like after the change: env: ["PATH=<redacted>", "HOME=<redacted>"] This does make debugging worse, but that's the tradeoff.
|
|
||
| /// Redacts environment variable values from a list of env vars in "KEY=VALUE" format. | ||
| /// Returns the env vars with values replaced by "<redacted>". | ||
| func redactEnvValues(_ env: [String]) -> [String] { |
There was a problem hiding this comment.
Question: I was reading the Docker Compose secrets docs and wondered, should we adopt a policy of never logging any env/argv values (even at debug), that is always redact env + args/commandLine? Or is the intent to only redact env and leave argv visible for debugging?
There was a problem hiding this comment.
Good question. We should clearly understand where app secret values could be stored at rest:
.envfiles or anything similar - these are owned by the user socontainerhas no responsibility for them.- Container configuration files in the application data store. In Docker, it appears that these are stored as plaintext under /var/lib/docker.
- boot logs - perhaps redact entirely, or log them at trace level (which means we shouldn't see them until we add log level control, at which time we could emit a warning if trace is selected)
- application logs - the user's responsibility
There was a problem hiding this comment.
yeah, I agree w/ the boot logs part which are written today by default. As you said gate boot logs and if I'm thinking it right;
(1) dont set config.bootLog = BootLog.file(...) by default
(2) only write boot logs if the user explicitly asks?
(3) move sensitive guest logs to trace by making the spec/argv logs trace-level.
There was a problem hiding this comment.
We should emit boot logs always, it's too useful for diagnostics. I think John meant redact the container configs in them entirely possibly
There was a problem hiding this comment.
Ahh, I see what you mean. yeah, config only redaction will be a balanced approach.
jglogan
left a comment
jglogan
left a comment
There was a problem hiding this comment.
Perhaps we do a log level check and log only env variable names at all levels but trace, and log everything at trace.
Right now I don't think it's possible to set anything but info or debug in container. Should we allow trace logging there we can emit a warning on system start.
3 participants
Closes #518
We print the OCI spec at debug level, and this can contain sensitive info in envvars specifically. Lets redact the values in the envvars before we print. I went the route of just redacting everything as trying to check against some "possibly sensitive looking vars" seems rife for never getting it fully right.
This is what the envvars look like after the change:
env: ["PATH=<redacted>", "HOME=<redacted>"]This does make debugging worse, but that's the tradeoff.