search

argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.39k stars 5.28k forks source link

We need a way to load the redis password as a file mount instead of always using env vars #3914

Open BenManifold opened 4 years ago

BenManifold commented 4 years ago

Summary

server, controller, and repoServer should all be able to access the redis password as a mounted secrets file, or some other secure alternative to plaintext environment variables.

Motivation

Many organizations have compliance rules prohibiting non-encrypted secrets in environment variables.

Proposal

A secrets file mount or some other secure alternative.

FrediWeber commented 4 years ago

Can you explain what the security benefit would be? The same user that sees the env variables would be able to read the file. Imo this adds complexity because it is not obvious but no real security benefit in this specific context. I do understand that it absolutely makes sense in a classic environment but I don‘t see the benefit in a container.

ghostsquad commented 4 years ago

I've heard this request before, and the benefit is that sometimes debug/stack trace dumps print the environment variables. Using a file prevents secrets from being exposed in indirect and subtle ways.

FrediWeber commented 4 years ago

Okay I see your point but should we not address this issue directly by making sure that sensitive env variables are not printed in stack traces?

BenManifold commented 4 years ago

We could, but that still ends up as a larger 'surface area' to maintain. The solution as a whole becomes more expensive.

On Sat, Jul 11, 2020, 4:52 PM Frederik Weber notifications@github.com wrote:

Okay I see your point but should we not address this issue directly by making sure that sensitive env variables are not printed in stack traces?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/argoproj/argo-cd/issues/3914#issuecomment-657127949, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHA4KLPYQK7P2LOKRS6VTDR3DGKRANCNFSM4OS4TFRA .

ghostsquad commented 4 years ago

@benmanifold I'm still curious about your data point that many companies disallow secrets as environment variables. I've never heard that be an issue. There are usually no technical restrictions in these policies. They boil down to access controls. At the end of the day, a secret must be in plain text somewhere in order to use it.

BenManifold commented 4 years ago

I can only really speak to my own constraints, this gets flagged by our runtime scans as an issue we need to track and mitigate, alongside a number of vulnerable image dependencies.

It's true that most attackers than could see env vars could also read files in containers, but the edge case is regarded as significant.

On Sat, Jul 11, 2020, 6:41 PM Wes McNamee notifications@github.com wrote:

@BenManifold https://github.com/BenManifold I'm still curious about your data point that many companies disallow secrets as environment variables. I've never heard that be an issue. There are usually no technical restrictions in these policies. They boil down to access controls. At the end of the day, a secret must be in plain text somewhere in order to use it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/argoproj/argo-cd/issues/3914#issuecomment-657141295, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHA4KMB45PUSDTS5QGXFUTR3DS77ANCNFSM4OS4TFRA .

ghostsquad commented 4 years ago

@benmanifold what are you using for runtime scanning?

BenManifold commented 4 years ago

twistlock, for now

On Sat, Jul 11, 2020, 7:45 PM Wes McNamee notifications@github.com wrote:

@BenManifold https://github.com/BenManifold what are you using for runtime scanning?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/argoproj/argo-cd/issues/3914#issuecomment-657148515, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHA4KPQC754PLY7MVBOYT3R3D2Q5ANCNFSM4OS4TFRA .

ghostsquad commented 4 years ago

I did a bit of research, and twistlock's enforcement of unencrypted secrets as env vars is just an opinion of theirs. It states they follow NIST SP 800-190 in this regard, but in fact.. the NIST guidelines refers only to secrets embedded into the image (via file or env var) as being insecure, which is very true.

https://www.twistlock.com/wp-content/uploads/2018/07/Executing-on-NIST-800-190.pdf

https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-190.pdf

Feel free to bring that up to your security team. Cause the threat doesn't really exist. If an attacker has access to your container at runtime, they don't need any more permissions than the application itself to gain access.

Using a file instead of env var can definitely be beneficial. But printing debug/dump information could easily lead to secret leakage in other situations too, so one should be very careful regardless.

ghostsquad commented 4 years ago

Security through obscurity is not secure

ghostsquad commented 4 years ago

For reference, it's in section 4.1.4 in both documents

ghostsquad commented 4 years ago

If you really want security, the solution is to use short lived tokens (30 minutes or less) that is requested directly by the application itself.

Anything else is just obscurity.

BenManifold commented 4 years ago

I am inclined to agree with you about security by obscurity, though I don't think this is that.

I think the additional exposure potential of env vars over file mounts is still worth addressing, file mounts aren't just an extra step an attacker with access must go through, for reasons discussed above.

On Sat, Jul 11, 2020, 8:36 PM Wes McNamee notifications@github.com wrote:

Security through obscurity is not secure

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/argoproj/argo-cd/issues/3914#issuecomment-657154329, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHA4KNZVIAEEU3XXWJ5LCDR3EAPBANCNFSM4OS4TFRA .

BenManifold commented 4 years ago

That would be even better, honestly. The goal is a reduced potential for leakage, and additional options for injection methods just gives us more ways to achieve that.

On Sat, Jul 11, 2020, 8:42 PM Wes McNamee notifications@github.com wrote:

If you really want security, the solution is to use short lived tokens (30 minutes or less) that is requested directly by the application itself.

Anything else is just obscurity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/argoproj/argo-cd/issues/3914#issuecomment-657154965, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHA4KOGPI57S4S7S7OFSPLR3EBGFANCNFSM4OS4TFRA .

ghostsquad commented 4 years ago

file mounts aren't just an extra step an attacker with access must go through, for reasons discussed above

I'm not sure where "above" you are referring to here.

So, what if you wrote your own wrapper image with an entrypoint that read a file and converted it to a env var prior to executing the original binary?

ghostsquad commented 4 years ago

If files were supported, folks could use things like the vault agent, which reads from vault and writes to a file. Leaving the application in charge of rereading the file at an interval. I think this is reason alone to support reading from files.