django-DefectDojo icon indicating copy to clipboard operation
django-DefectDojo copied to clipboard

passwords from secret management tool when installing via helm

Open tiagoposse opened this issue 4 years ago • 8 comments

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 avatar Dec 02 '21 11:12 tiagoposse

@tiagoposse did try with the local.settings.py ? It's a better solution when you want to override settings with a file.

damiencarol avatar Dec 10 '21 06:12 damiencarol

@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.

tiagoposse avatar Jan 28 '22 18:01 tiagoposse

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 avatar Jan 28 '22 18:01 dsever

@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?

tiagoposse avatar Jan 28 '22 18:01 tiagoposse

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

dsever avatar Jan 28 '22 19:01 dsever

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

tiagoposse avatar Jan 28 '22 19:01 tiagoposse

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...

dsever avatar Jan 28 '22 19:01 dsever

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.

stale[bot] avatar Apr 28 '22 22:04 stale[bot]