django-DefectDojo
django-DefectDojo copied to clipboard
passwords from secret management tool when installing via helm
Is your feature request related to a problem? Please describe security around kubernetes secrets is subject to debate, but a part of the community prefers to use external secret management tools (like hashicorp vault) to store secrets. The current helm chart supports passwords:
- defined directly in the values file (ends up being committed to git repo - insecure)
- load from kubernetes secrets (more secure, but still not as much as with external store)
Describe the solution you'd like Configuration of passwords should be possible via extra file. Something like:
postgresql:
loadFromFile: false | true
redis:
loadFromFile: false | true
extraConfigs:
DD_EXTRA_ENV: <path to env vars with passwords>
Additional context
I've attempted to do this and was able to disable the requirement of secrets by the chart, but I hit a bump when the initializer did not read vars from DD_EXTRA_ENV and so db password was wrong.
I've tried with DD_EXTRA_ENV=extra_settings/.env.prod (and others). I can probably get around this by providing a custom settings.py, but this seems unreasonable for such a little issue.
I've then changed line 235 of dojo/settings/settings.dist.py from:
if os.path.isfile(root('dojo/settings/.env.prod')) or 'DD_ENV_PATH' in os.environ:
env.read_env(root('dojo/settings/' + env.str('DD_ENV_PATH', '.env.prod')))
to
env.read_env(root('dojo/settings/' + env.str('DD_ENV_PATH', '.env.prod')), overwrite=True)
which caused the password to be correctly loaded, but initializer was still triggering a wrong password.
I would not mind to open an MR for this if you're at all interested, but I might need a bit of help debugging.
@tiagoposse did try with the local.settings.py ? It's a better solution when you want to override settings with a file.
@damiencarol I was able to get to this finally. I was able to inject the secrets into /app/docker/extra_settings/local_settings.py. Thanks for the tip! I'm preparing a PR to the chart.
But you have already option to inject extra secrets. That will end inside of secret resourse
extraSecrets:
Sensitive content you never keep in values, or you keep values file encrypted (ansible vault or vault), you can inject values using command line as well
@dsever correct if I'm wrong, but through extraSecrets, the data will end up in a kubernetes secret, which I'm trying to avoid. Or is the idea to pass something like:
extraSecrets:
DD_DATABASE_PASSWORD: 'xxx' # literal 'xxx', not a placeholder
and then somehow overwrite this via vault injection (I think this would cause a problem with the same mountPath for different secrets) or overwriting the command argument for a pod?
Ha look, if you really don't trust your k8s etcd (if is not encrypted at transit and rest), then you should use vault sidecar, that will inject secrets from the vault. Than you have bigger problem than security of DD.
First point you have storing directly in value file. Yes you can create additional file that will have extraSecrets and keep it encrypted then during the installation or upgrade just use -f to merge values. Or as I said use --set directive to set variable that will create value.
Where do you have in plan store .env or local_settings.py?
I'm trying to understand the problem
the idea here is really to use vault sidecar, I've been able to do this by injecting a local_settings.py with the following content:
DATABASES['default']['PASSWORD'] = '{{ .Data.password }}'
However, the current templating of the celery worker, celery beat, initializer and django deployments makes using kubernetes secret mandatory. While not exactly pretty, I can get around this by creating an empty secret and then injecting the correct value via vault anyways. So it seems that my use case is indeed supported via this "minor hack" of using an empty secret. My PR would mostly focus on eliminating this need. If you're interested in this I can go ahead and push the PR, otherwise I'm closing this issue as I can do what I need without much hassle
Definitely I would like to see the solution, if it doesn't have huge impact to current helm (like breaking many installation in the wild), why not to give a try...
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.