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

Docker: Support Secrets #1987

Open
jandaa opened this issue Jan 28, 2022 · 31 comments · May be fixed by #2302
Open

Docker: Support Secrets #1987

jandaa opened this issue Jan 28, 2022 · 31 comments · May be fixed by #2302
Labels
idea Feedback wanted / feature request

Comments

@jandaa
Copy link

jandaa commented Jan 28, 2022

Is your feature request related to a problem? Please describe.

I would like to have a way to pass the admin password to Photoprism as a docker secret. The only way to set the password right now is to explicitly write out the password in the docker-compose file which is not very secure.

Describe the solution you'd like

Support passing secrets using the standard convention of passing in the secret to an environment variable with _FILE appended to it. It would look like this

photoprism:
    image: photoprism/photoprism:20220121
    secrets:
      - photoprism_admin_password
   ...
    environment:
      PHOTOPRISM_ADMIN_PASSWORD_FILE: /run/secrets/photoprism_admin_password

This can be done by modifying the entrypoint.sh file to check if its a secret or not and if it is, to cat the file to get its contents

Describe alternatives you've considered

Running from command line but this is not very maintainable or scalable.

Additional context

@jandaa jandaa added the idea Feedback wanted / feature request label Jan 28, 2022
@mbelangergit
Copy link

I would love to see this request get traction too. Most (if not all) of the other containers I used are using the proposed _FILE ending or similar. PHOTOPRISM_DATABASE_USER and PHOTOPRISM_DATABASE_PASSWORD are also good candidates for this feature.

@petertrr petertrr linked a pull request May 3, 2022 that will close this issue
3 tasks
@StudioLE
Copy link

StudioLE commented Oct 21, 2022

Instead of writing the passwords directly in the docker-compose.yml you can use a .env file:
https://docs.docker.com/compose/environment-variables/#the-env-file

It will still be loaded as an environment variable in the container but this way means you can safely commit the docker-compose.yml to version control while ignoring the .env

@lastzero
Copy link
Member

Also, you can encrypt the password with blowfish. That way, you can't just read it. While that is not possible for MariaDB for the obvious reason, you can use an env file or apply the env value from the global environment without setting an explicit password in the docker-compose.yml file.

@Stunkymonkey
Copy link

@lastzero I am currently working in providing this as NixOS-Module. I considered all the suggestions. In the end a passed env-var can be read by any other user. Placing tha password into a file is the easiest.
Could you please consider the PR linked to this Issue.

@dynamicat
Copy link

Any chance of pushing this request forward?

@lastzero
Copy link
Member

Since we are currently busy with a very long list of other tasks, you can either use a .env file or provide the password as a hash as a workaround. Also, we recommend changing the password after the first login so that you can use a password that is secure enough for the initial setup instead of choosing your actual password, which may be very long and complex.

Helping us with issues labeled "help wanted" will allow us to ship feature requests more quickly, for example:

@inthreedee
Copy link

I understand that the project has other new feature priorities at the moment, but I'd like to politely suggest that this security improvement be given a higher priority than it currently has, especially since someone has already provided help by implementing it in a PR.

Using an env file does not improve security in the way docker secret files do since the password still gets preserved in the container's perpetual environment. The point of docker secrets is to remove the environment variable attack vector. The hash workaround does not work for mariadb as mentioned.

New features are nice, but security of publicly exposed services is important.

@lastzero
Copy link
Member

Docker Secrets are only available in swarm mode, not stand alone based on what I know. I also wouldn't know how we could improve the security of the mariadb service other than proving a more complex default configuration consisting of multiple files, updating all our documentation and asking all users to change their existing setup to use swarm mode or an env file?

@Stunkymonkey
Copy link

this is also relevant for non docker environments. e.g: NixOS

@inthreedee
Copy link

inthreedee commented Apr 12, 2023

Docker secret files don't require swarm mode. Docker compose mounts the files provided as files within the container, which the services then access on the filesystem rather than relying on environment variables. It's not as secure as docker secrets used through swarm mode, but it's an improvement over using persistent environment variables and doesn't require anyone to make any changes to their existing setups unless they want to tighten their security a bit. This is meant to work for any secrets anyone wants to provide to the compose file.

For example, a compose file could be modified like so:

secrets:
  # Secrets are single-line text files where the sole content is the secret
  # Paths in this example assume that secrets are kept in local folder called ".secrets"
  DB_ROOT_PWD:
    file: .secrets/db_root_pwd.txt
  DB_PWD:
    file: .secrets/db_pwd.txt

services:
  mariadb:
    environment:
      MARIADB_PASSWORD_FILE: /run/secrets/DB_PWD
      MARIADB_ROOT_PASSWORD_FILE: /run/secrets/DB_ROOT_PWD
    secrets:
      - DB_ROOT_PWD
      - DB_PWD

I'll be honest though, I don't know what Photoprism needs to do on its end to support this or if it's already baked in as a feature of docker-compose?

@lastzero
Copy link
Member

Right, there are more complicated and potentially more secure ways to use Docker Compose. We already provide ways to share examples and tutorials with the community though. At the same time, we don't want to exclude users who don't have the need / skills for this and we don't have time to update all our getting started docs to use these features and make it look like this is mandatory.

@lastzero
Copy link
Member

For users that cannot or don't want to use environment variables, we have recently added these docs which hopefully are enough to get started? Is there anything missing?

👉 https://docs.photoprism.app/getting-started/config-files/

@Stunkymonkey
Copy link

For users that cannot or don't want to use environment variables, we have recently added these docs which hopefully are enough to get started? Is there anything missing?

point_right https://docs.photoprism.app/getting-started/config-files/

--admin-password-file would be nice. this way the secrets are never stored in memory and only available if the file permissions are right:
#2302

@lastzero
Copy link
Member

What about contributing a Docker Security Guide for our docs similar to this?

https://docs.photoprism.app/getting-started/advanced/volumes/

That could provide a lot of value to the community without reworking all the existing docs and examples.

@lastzero
Copy link
Member

For users that cannot or don't want to use environment variables, we have recently added these docs which hopefully are enough to get started? Is there anything missing?
point_right https://docs.photoprism.app/getting-started/config-files/

--admin-password-file would be nice. this way the secrets are never stored in memory and only available if the file permissions are right: #2302

I can see how another way to set a password in a file would be more convenient for a small number of users, but there are already several ways to do this - including using the configuration files documented at the link above. The options.yml would look like this:

AdminPassword: insecure

Alternatively:

AdminPassword: $$bcrypthash

If for some reason the bcrypt hash method does not work, that would be a bug, and we are happy to quickly merge a fix or provide a fix ourselves if no one can submit a PR.

@lastzero
Copy link
Member

It's true that the config values are stored (cached) in memory even when you read them from a file. Then again, you can encrypt it and you should change it anyway. In the same way, you could read it from the database result cache or directly from the database. I fail to understand how that differs?

@lastzero
Copy link
Member

On a last note, TLS certs and keys can also be provided as files. So I also don't see the need to implement yet another mechanism for that.

@Stunkymonkey
Copy link

from NixOS side the options.yml is a bad idea, because that would expose the password to every user on the machine. That is due to the nature of NixOS-modules. Of course this i not your fault, but also something we can not change.

The current work around is cat when starting the application the password is read to the environment variable: https://github.com/NixOS/nixpkgs/blob/b3f466c32a2fbd34566d72fa71df8beca049e9a1/nixos/modules/services/web-apps/photoprism.nix#L148
in comparison to other options can simply be passed here: https://github.com/NixOS/nixpkgs/blob/b3f466c32a2fbd34566d72fa71df8beca049e9a1/nixos/modules/services/web-apps/photoprism.nix#L6
I am fine with it, but it would be much nicer to simply specify the file and be done.
With this workaround the password is visible in /proc/<pid>/environ.

Maybe an article like this convinces you: https://www.netmeister.org/blog/passing-passwords.html
Sure we can discuss about the usage of photoprism without docker, ... but I am not sure if we get to a conclusion...

@lastzero
Copy link
Member

This has absolutely nothing to do with Docker. More like you can't work with our YAML config files and want us to support yet another format that is compatible with NixOS? Do you agree it's not worth delaying our release even further for this and there is no security vulnerability related to the current implementation other than your tools require a plain password file instead of the config files documented?

@lastzero
Copy link
Member

lastzero commented Apr 12, 2023

Also, why must private app config files be WORLD READABLE with NixOS? This doesn't make any sense to me and should not be implemented that way.

@lastzero
Copy link
Member

You could, by the way, also run the passwd command to set the password. So there is a absolutely no requirement to use environment variables.

@inthreedee
Copy link

inthreedee commented Apr 12, 2023

For users that cannot or don't want to use environment variables, we have recently added these docs which hopefully are enough to get started? Is there anything missing?

point_right https://docs.photoprism.app/getting-started/config-files/

Oh, I wasn't aware this existed, thanks for pointing that out! This works for me. I configured it and the passwords are no longer stored in the container's environment 👍

Edited to add:
For anyone else looking to also do this, don't forget your db container. Mariadb's official images support the _file variables and the example I gave above will work.

@Stunkymonkey
Copy link

NixOS could use the "private app"-file yes; and without being world read-able, ... but then all the configuration has to be done by the user itself. But lets not have a discussion about NixOS here, since this will not help our discussion here. NixOS is simply different.

One thing we both might agree is: interactive password entry is not nice for automation. For me this is more like a backup-/recovery-solution.

IMHO: the applications-settings and passwords/secrets/token/... do not belong in the exact same "location".

E.g: people might want to have their application settings stored in a version-control.
It could be that users might accidentally commit the password stored in the app-config-file/environment-variables and therefore a reference/split is a better option.

I already see as you say that there are several ways to pass the parameters. So your suggestion will be to pass the settings via "app-config" and the password via "environment-file" (or the other way around).

Yes this works.
The tiny difference: the secret is stored perfectly safe 👍,
but the "settings-key" (AdminPassword/PHOTOPRISM_ADMIN_PASSWORD) is not stored anywhere. I think it would be nice to know if something should be set or not.

Many programs use exactly this kind of "secret-passing" e.g: docker-secrets, systemd-service-credentials, ssh-keys, ... They all work exactly the same and have a setting for the "password"/"key"/... stored in plain-text and pass the file to the program.
Surely there are other examples like .my.cnf (mysql), ... that store the password as you do it.

I totally agree that all of this is a personal preference, but this practice can be seen in multiple programs.

Hashing the password is a nice option. Maybe this is an option for me. But I am too tired now.

Have a great night. Thanks for this awesome tool ❤️

@lastzero
Copy link
Member

Providing too many ways to do the same thing can be counter productive for security and confuses users on top of it. So I must make sure you have read our complete documentation and there really is a need to implement yet another way to do the same thing. Note we must also document it and maintain it for the years to come.

@lastzero
Copy link
Member

lastzero commented Apr 12, 2023

If you read our complete docs, you will find that there is a defaults.yml in addition to the options.yml. This is so you can separate both, for example for security reasons but also to keep your config in a version management system. Either the defaults or the current values changed through the user interface. That's why it's hard to imagine this still isn't enough!?

PS: User Settings are also separated from the system config in a settings.yml file.

@inthreedee
Copy link

I've started a Docker Security Guide in photoprism/photoprism-docs#152 as requested above. If anyone else has other recommendations, please feel free to add to it.

lastzero added a commit that referenced this issue Apr 13, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Apr 13, 2023

These changes enforce the (previously implicit) 72-character length limit for bcrypt and allow you to use a valid 60-character bcrypt hash with a minimum difficulty of 4 when setting the initial super admin password. If you think anything is wrong or insecure with this approach, now would be a good time to let us know!

Edit: Our tests with this were successful. Note that you need to escape $ with $$ in the docker-compose.yml file, while this is not required in an options.yml or defaults.yml file.

There are many commands available to generate standardized bcrypt password hashes, see for example:

https://unix.stackexchange.com/questions/307994/compute-bcrypt-hash-from-command-line

lastzero added a commit that referenced this issue Apr 13, 2023
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

Help with testing is much appreciated so we can make sure this works for everyone!

@kkovaletp
Copy link

kkovaletp commented Jun 13, 2023

I might be late here with my proposal, but it is always better to say, right?

In general, I'd agree that storing secrets and passing them to apps in plain text is a bad idea. Mapping them to env vars is even worse because once something wrong gets into the environment, it dumps all the env vars as a 1st step. Then it could easily search for shell history files and go through open files to search for plain text secrets.
So, all discussed variants of storing secrets in different configs, other text files, and vars - are vulnerable at the pretty same level.
Using passwd and bcrypt is much better, but might be tricky for some users, so they skip that and keep using plain-text solutions.

I think that security should be the default and should not be difficult to use. It should be as much difficult as possible to break, but simple to use. So, I propose using the self-hosted secret manager as part of the docker-compose service. In this case, the user will have a convenient UI to set and maintain the secrets, while all software components will retrieve them in the most secure way.
There are a lot of them, I guess, but after some fast googling, I found this one https://infisical.com/, which is open-source, has a free plan, and can be self-hosted in docker-compose.

UPD: I found a much better secret manager, which is 1 of the options to use in Kubernetes, and also supports Docker-compose and other orchestration types: HashiCorp Vault.
Here are a few guides for Docker-compose:

What do you think?

@Stunkymonkey
Copy link

@kkovaletp I fully support your statement that this should be supported.
But it should be supported in a layer before.
This is not something that should be included in photoprism.

It would be much work to adjust (and hard to update) and to include this functionality in every program and every provider.
A better solution would be to provide this via docker itself so each application can continue to work without changes.

@kkovaletp
Copy link

@kkovaletp I fully support your statement that this should be supported. But it should be supported in a layer before. This is not something that should be included in photoprism.

It would be much work to adjust (and hard to update) and to include this functionality in every program and every provider. A better solution would be to provide this via docker itself so each application can continue to work without changes.

Sorry, I didn't get what you really mean by "supported in a layer before" and "to provide this via docker itself" - could you please clarify that a bit with maybe some links to guides and/or examples?

As far as I see from the Vault's documentation, there is native support for DBs (including MariaDB). To add support to the Photoprism app, you need to add just ability to do simple HTTP requests to configured URL of Vault Agent, which will respond with the secret's value - that should be a very easy task. To simplify the initial config of the service, you could ship the Vault's init script, config, and policy, like in examples from my previous post. You could even enhance the script with some secrets creation, so a user won't even need to open Vault UI and set them manually.

If I miss something, please point me there and provide your point of view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Feedback wanted / feature request
Projects
Status: Ideas 💭
Development

Successfully merging a pull request may close this issue.

8 participants