Set file permissions for created files

[RealOrangeOne]

Fixes #1784

This isn't a full audit of all places files are created, but it covers most. Intentionally not set them on the image cache, as they're not sensitive.

It only works on unix, because that's the only place permissions like this work. It should work fine under docker even if docker isn't run on unix.

This also reuses the existing write_file util in a few places which custom implemented it.

[williamdes]

Does it run only when the file is created so it could not break read only file system mounts ?

[RealOrangeOne]

It always happens after files are written, so the write would fail before this would.

[jjlin]

This doesn't cover the SQLite DB file. It would probably be much simpler to just chmod 700 the data directory.

[BlackDex]

Maybe better to set an umask in the entry file? Since this is only a concern for docker run instances.

[RealOrangeOne]

I'd say this is still a concern to non docker users, it's general file permissions.

Setting a umask would probably work too, and default everything to 600, without having to remember to set it everywhere. Unfortunately there's nothing in the rust stdlib to set a umask, so it'd involve dropping down to a shell, which feels like a bit of a hack. Would also want to manually chmod everything on startup anyway, to make sure the permissions are updated for existing users.

What I don't know is whether umask works on macOS / windows, or if there are alternatives for those platforms we should be using.

[BlackDex]

It will only work within a docker layer.
I can also think that a group could maybe need read rights for say, backups or other stuff maybe.

Also, if someone runs it outside of docker, then the application would use the umask of the system or the init/systemd part, same goes for MacOS.

And, what if i do not want it to have 600, but 664 or 644 or whatever.
I'm not certain what to do here actually.

[BlackDex]

I'm not inclined to merge this PR. I still think adding a umask would be the best option here, and this will only add extra overhead.
It also is not touching every file created by Vaultwarden, and it would make maintaining this harder if someone is adding a new file to be saved somewhere. Therefor a umask would be better, because that would count for every file created.

[septatrix]

This approach is likely to miss some points in the code where files are created and the way the mode is set is inherently racy which is also very bad if this is really meant as a protection against attacks...

This doesn't cover the SQLite DB file. It would probably be much simpler to just chmod 700 the data directory.

Maybe better to set an umask in the entry file? Since this is only a concern for docker run instances.

Both of these feel like cleaner solutions and I think the former would be the "usual" approach to this and how most linux packages are set up when storing some secrets.

[BlackDex]

I'm going to close this stale PR.
There isn't a good clean solution for everything. Maybe adding the umask is a good one by default.
If so, please create a new PR for this.

Also, calling the the permissions setting during every storage is an other IO call, which only slows down, and i like to prevent that.