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

App Pool Identity credentials show in clear text in INFO log #32

Closed
torrick opened this issue Dec 11, 2020 · 6 comments · Fixed by #35
Closed

App Pool Identity credentials show in clear text in INFO log #32

torrick opened this issue Dec 11, 2020 · 6 comments · Fixed by #35
Assignees
Labels
bug Something isn't working

Comments

@torrick
Copy link

torrick commented Dec 11, 2020

When setting an app pool identity in the driver config, the entire driver config is written to the nomad log with INFO and the app pool identity username and password are visible in clear text. Is there a way we can redact that in the log?

@Vulfox Vulfox self-assigned this Dec 11, 2020
@Vulfox
Copy link
Contributor

Vulfox commented Dec 11, 2020

Thank you for bringing this up @torrick! I totally agree to redact that or at the least just log that there are credentials being passed but not THE credentials themselves. Working on this ASAP!

@Vulfox Vulfox added the bug Something isn't working label Dec 11, 2020
@Vulfox
Copy link
Contributor

Vulfox commented Jan 5, 2021

So looking at how other nomad drivers log their inputs, I see where identity data became an info level log. This log is essentially to inform that the driver is starting with the set values, which seems to be a staple with other nomad drivers (log the driver configs, not secrets as none of them have secret level configs).

Nomad assumes that anything set on the nomad job spec is not a secret with their workflows and rely on secrets being transferred via the template stanza to env vars or a file in the secrets dir. I am thinking, to help facilitate a properly kept secret for user+pass for the driver, we have env vars be set and the driver will pull the values from the env vars from the nomad job execution and remove it from the list to be sent to the IIS process.

Maybe something along the lines of NOMAD_APPPOOL_USERNAME and NOMAD_APPPOOL_PASSWORD are to be set via a template stanza.

Nomad does have an open issue with credentials being used within the nomad spec itself, but it seems to be in the discussion phase still.
hashicorp/nomad#3854

@shishir-a412ed and @torrick thoughts?

@shishir-a412ed
Copy link
Contributor

shishir-a412ed commented Jan 6, 2021

@Vulfox Do we really need to print the entire driver config every time a task is started?

If this information is for debugging purposes, then we can change d.logger.Info to d.logger.Debug, so atleast the password is only logged (exposed) when the driver is logging at DEBUG level.

Another option is to just make a separate copy of the struct and redact the password, and use the copy for logging purposes.

var driverConfigWithPasswordRedacted TaskConfig
driverConfigWithPasswordRedacted = driverConfig
driverConfigWithPasswordRedacted.AppPoolIdentity.Password = "REDACTED"

d.logger.Info("starting iis task", "driver_cfg", hclog.Fmt("%+v", driverConfigWithPasswordRedacted))

@Vulfox
Copy link
Contributor

Vulfox commented Jan 6, 2021

@shishir-a412ed it is not needed to log at info level for this part. I only set it at this level and with the whole driver config to match what hashicorp has done with their other drivers for consistency.

I think we should lean to remove the app pool identity configs from the driver config or allow for it to be set elsewhere to provide a more secure solution for folks to use until hashicorp has the credentials stanza implemented. Even if we change the logging level for the given log or redact the value from log, the username and password will be in clear text from nomad itself. Anyone with permissions to view nomad jobs will be able to get the secrets, where as with the env var population from the template stanza, it leaves that up to users to leave it in clear text or use something like consul or vault to store the secrets and not be exposed.

@shishir-a412ed
Copy link
Contributor

@Vulfox I agree, ideally, the secrets should be stored in vault and injected at runtime.

I just suggested this as a stop-gap solution, since @torrick original concern was the password being exposed in the log.

@torrick
Copy link
Author

torrick commented Jan 6, 2021

I'm currently using @Vulfox template -> env trick to populate the app pool identity secret bits so my main concern is just that they're written to the info log in clear text. Now the driver config written to the log has been super helpful in debugging some weird nomad string encoding issues so I'd keep it in but @shishir-a412ed's idea about copying the driver config struct would solve the immediate issue for me. Provided the additional struct doesn't doesn't make the codebase more confusing.

@Vulfox Vulfox linked a pull request Jan 12, 2021 that will close this issue
@Vulfox Vulfox closed this as completed Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants