Config: Read secrets from file

[petertrr]

This PR adds support to read admin password and database password from a file. After all configuration options are loaded from environment, CLI and yaml configs, if admin-password-file and/or database-password-file are set, these settings are read from the files.
Closes #1987

Acceptance Criteria:

• Features and improvements are fully implemented so that they can be released at any time without additional work
• Automated unit and/or acceptance tests have been added to ensure the changes work as expected and to reduce repetitive manual work
• User interface changes are fully responsive and have been tested on all major browsers and various devices No UI changes
• Database-related changes are compatible with SQLite and MariaDB No changes in DB
• Translations have been / will be updated (specify if needed) No changes in texts
• Documentation on Docker deployment should be updated later
• Contributor License Agreement (CLA) has been signed

[CLAassistant]

All committers have signed the CLA.

[Stunkymonkey]

This would be very helpful for NixOS. The package does already exist, but no module has been created yet.

[lastzero]

Note that PhotoPrism already supports reading pretty much all config values from files, which is now also well documented in our official Getting Started guide:

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

[lastzero]

From my understanding this isn't needed as you can use an options.yml file to specify most config options, including the admin password. As we haven't received any feedback on the same comment months ago, I'll move ahead and close this (for now). Thank you for your contribution and sorry for the wait!

[kkovaletp]

I assume that this PR should provide the ability to map secrets from the docker host as files in a container and in this case it should be 1 plain text file per secret, which is not the case with configs.
Mapping secrets as files, in general, is a bit better from a security perspective, because an attacker will need to search for the file in the FS instead of simply dumping all the environment variables, but to make it work, Photoprism has to close the secret files right after reading so that when attacker will check the list of opened files, secrets are not there.

[lastzero]

@kkovaletp Thanks for your remarks! If this is something that a few of our users need, and @petertrr still wants us to merge it, I'm happy to reopen it.

However, I'm still not sure what the impact of this is e.g. what additional security this actually provides, as a complex web application like ours offers many more potential attack vectors, some of which might be much easier to exploit... Having an overly long list of config options and too many different ways to set a password can be a security issue in itself? 🤔

[kkovaletp]

@lastzero As I mentioned in the discussion of #1987, I think that security and privacy should be the default, but also I assume that most users do not open the service for Internet access (maybe I'm wrong) and for that users, it makes sense to provide simple and easy setup with secrets in env. vars.

If choosing between different secure setups, I'd vote for the secrets manager, as it provides the best possible level of secrets protection, but it is up to you to decide.

Yes, adding a new method of providing secrets will increase complexity. But from my perspective, it is not a reason to not have any relatively secure one. What could be done here, is making a review of all available methods to make sure you don't have any obsolete and not actively used (or not adding any value to the product, so that could be safely replaced by other existing ones). Then, it is a key point to provide clear documentation for all supported use cases:

• for a simple setup in a private local network go to this page with a simple setup guide,
• for case A go to this page with the Yaml config setup guide,
• for internet-accessible service go to this page with secrets management, reverse proxy and SSL, firewall configuration guide...
  This approach will simplify the setup for end users and decrease the probability of incorrect configuration or mixing different config options together.

But again, this is my opinion and it is up to you to decide.

[lastzero]

Since this is part of the core functionality, our biggest concern is that we have to test this several times a month before each release when only a handful of users actually use it. How about we merge and add it as a hidden flag (so as not to confuse users who follow our simple examples) when this PR gets 20 thumbs up? It currently has 4 👍

[kkovaletp]

I have just 1 question, as I'm not an expert in Go:
Does the app close secret files right after reading them? I didn't see that in the code explicitly.
As I mentioned before, it is crucial for security, as getting the list of opened files for the process and then iterating through them is easy, so there is no any security improvement, and even worse: we simulate it making a false impression of being protected...

[lastzero]

I have just 1 question, as I'm not an expert in Go: Does the app close secret files right after reading them?

It is a best practice to automatically close files when the function that reads them returns:

func () {
   f.Open()
   defer f.Close()
   doSomething()
   return
}

Of course, there are exceptions, such as when you keep log files open for writing.

However, a common attack vector is reading sensitive information from memory. Direct access may seem difficult, but most servers have a swap file configured, so you could read passwords from there as well. Thus, if you focus on just one aspect, the overall security might not be increased significantly.

I didn't see that in the code explicitly.

Should we proceed with merging, there will be a security review that takes additional time. That's why I want to make sure we really need this.

we simulate it making a false impression of being protected...

I agree, this is a common problem and should not be underestimated.

[clitters]

Perhaps as an additional explanation: When running the app in a Kubernetes or Docker Swarm cluster, secrets are the only/best way to maintain and manage such configurations securely in a centralized location.
It might be not a big improvement from the applications point of view in terms of security, but from the infrastructure point of view this would be a big improvement.

[bVdCreations]

I think another benefit of this is that you can store the configuration in git without exposing any passwords.