ttionya / vaultwarden-backup

Backup vaultwarden (formerly known as bitwarden_rs) SQLite3/PostgreSQL/MySQL/MariaDB database by rclone. (Docker)
MIT License
956 stars 113 forks source link

GPG encryption #135

Closed kongwy closed 2 months ago

kongwy commented 9 months ago

Add the ability to encrypt file(s) with GPG before uploading.

Change Details

Added two environment variables: * NEW: `GPG_ENABLE` * Description: Toggle the GPG feature. * Default: `FALSE` * NEW: `GPG_PUBKEY` * Description: Path to the public key for HPH encryption. * CHANGE: `ZIP_PASSWORD` * Default: `"WHEREISMYPASSWORD?"` -> `""` (empty str) * Reason: It is no longer necessary to be compulsory as GPG encryption is an alternative.

P.S. I'm not quite familiar with shell script. Please feel free to make any further changes.

ttionya commented 9 months ago

Thank you for your pull request.

There's another matter I believe is worth discussing, regarding the environment variable GPG_PUBKEY.

The environment variable GPG_PUBKEY specifies the path to the GPG public key, and users are required to mount the GPG public key file into the container when using it. However, I think it might be preferable to have the public key path determined internally rather than by the user.

Additionally, the /config/ path is defined as the configuration folder for rclone. While placing the GPG public key inside it is feasible, the provided docker-compose.yml file already directs /config/ to the vaultwarden-rclone-data volume, which could lead to confusion. Therefore, specifying it as /gpg/key.pub might be a better choice.

Furthermore, since containers can be launched using non-root users, mounting a file could present permission issues for these users.

So, I'm wondering if it's possible to have GPG_PUBKEY filled with the content of the public key rather than the file, and then use a script to write this content into the container. This approach could potentially avoid permission issues. Users who prefer to mount the file directly would need to handle any permission problems themselves. I'm not familiar with the gpg command, so I don't know whether it's possible to use the public key content directly instead of a public key file.

We can discuss this matter first and come to a solution. I would like to handle the implementation, and then you can assist with the review afterward.

kongwy commented 9 months ago

@ttionya Fair enough. Filling GPG_PUBKEY with the public key directly sounds good. Although it seems gpg doesn't accept inline key content directly, I reckon it wouldn't be difficult to save the key as a file with cat in the script.

Again, I'm not really familiar with all these either. I just feel it would be great if this project could have GPG encryption, as all contents in a password manager backend should be considered sensitive data, while some of them (e.g. attachments) are not internally encrypted by Vaultwarden.

ttionya commented 9 months ago

Alright, I will modify GPG_PUBKEY to accept a public key string instead of a file path.

Once you've made the changes based on the review, I'll go ahead and merge this.

ttionya commented 8 months ago

@kongwy ,

I noticed that this pull request has had no new commits over 10 days. Are you willing to make code changes based on the review feedback?

AtomAsking commented 2 months ago

@ttionya Is there a plan to advance this PR? GPG encryption is very useful for me, I am looking forward to it. Thank you for everything you've done. Thanks.

ttionya commented 2 months ago

@AtomAsking ,

The development has been completed, but it lacks testing. I will check the existing code over the weekend and release a beta version for testing.

ttionya commented 2 months ago

Moved to PR #152 .