backrest icon indicating copy to clipboard operation
backrest copied to clipboard

Store sensitive values in OS keyring

Open homandr opened this issue 1 year ago • 7 comments

First of all, thank you for this awesome piece of software. Backrest is lightweight, has clear and intuitive interface, and has pretty much all the features I ever needed in a backup tool. Restic is awesome on its own, but an average Windows home user really needs a GUI. I can now recommend Backrest+restic to all my non-technical friends.

Currently Backrest stores repository password and extra environment variables (often used to store credentials for cloud storage) in plain text in config.json. This is less than ideal. The standard place for sensitive information is OS keyring. It appears there are several Go-based libraries that can help with that: https://github.com/99designs/keyring https://github.com/zalando/go-keyring https://github.com/tmc/keyring

Admittedly, I don't know how much true security this would add since presumably if the host is compromised under user's account, that account's keyring is also compromised. However, I think it's still much better than just keeping credentials in plain text, which doesn't require any special skills to read. I'm not a security expert, although I'm sure OS keyring has its own weaknesses. Nevertheless, it's a standard place for these things and would move this responsibility from Backrest to OS implementation.

There are probably other ways to store passwords securely even outside of keyring. For example, on Windows in PowerShell I can use ConvertTo-SecureString and ConvertFrom-SecureString to produce an encrypted string that can be decrypted only under the same user account. This string could be stored in a plain text file.

For the time being at the very least I would recommend Windows users to encrypt their %appdata%\backrest folder (not available in Home Edition): https://support.microsoft.com/en-us/windows/how-to-encrypt-a-file-1131805c-47b8-2e3e-a705-807e13c10da7 This way, at least if user's laptop is lost or stolen, their backup should be reasonably safe.

homandr avatar Nov 11 '24 02:11 homandr

Agree with all of your recommendations on the disk encryption front, this is a good best practice. I'd also like to aim to get a security article written for backrest's docs.

I'd add to this with the recommendation that users enable a 30 day object retention on their backup server (e.g. s3, b2) to protect against a compromised key being used to delete backups (i.e. ransomware attacks).

On the keyring front I think a decent implementation direction would be an implementation of ConfigStore interface https://github.com/garethgeorge/backrest/blob/main/internal/config/config.go#L15-L18 to store the whole config in the keyring -- looks like most keyrings support large entry sizes, large enough for the whole thing as a JSON string.

This would probably make most sense as an opt in feature controlled by either an env var or a cli flag (e.g. it wouldn't make sense as a default for docker).

garethgeorge avatar Nov 11 '24 09:11 garethgeorge

Thank you for looking into this! I'm not sure it would be a good idea to store the whole config there. A few concerns come to mind:

  1. I did a bit of searching, and it appears that Windows credentials store is quite limited. See https://github.com/docker/docker-credential-helpers/issues/190 and https://github.com/danieljoos/wincred/issues/18#issuecomment-752719647. I quickly tested from GUI side by trying to add a very long password to a new Windows credential, it didn't take it.
  2. I see you have a neat feature of backing up config.json. This functionality would be broken.
  3. Sometimes it's nice to see the actual configuration JSON, say, when troubleshooting.

I discovered restic already has --password-command option that could be used to obtain the repo password securely. I'm going to look deeper into that, not sure if it would take precedence over RESTIC_PASSWORD that Backrest sets up. However, the issue of storage backend credentials remains. Arguably, this is even more important than the repo password.

I'm thinking now that maybe an easier interim solution to implement would be to have Backrest run a pre-restic command hook for setting up the variables. And then leave it up to the user what script and tool they want to use. Some might not want to use OS keyring but something like gpg or a password manager. Technically, it's doable already if launching Backrest from a script, but now we are talking about setting up shortcuts, a way to hide the shell window (on Windows) and so on. Not something an average desktop user knows how to deal with.

Sure, having native keyring support would be a nice thing out of the box, I don't know what would be easier to implement. Ideally, both! But we can't have everything 😄 , Backrest already does a lot to make users' life easier.

homandr avatar Nov 12 '24 18:11 homandr

Thanks for calling out that windows limit -- thinking a bit more about this over the last few days I think it would also be fine for backrest to simply store an encryption key in the secret store and to store an encrypted config on disk. E.g. ~/.local/share/backrest/config.json.aes-enc or such. This approach would also be compatible with the config backups as you point out -- but would be more painful re: an opaque encrypted file is essentially impossible to edit outside of backrest's UI.

I think that could be mitigated by adding a JSON preview of the config or such in Backrest's UI possibly.

On the password command side of things, if you provide RESTIC_PASSWORD_COMMAND as an env var you are able to leave the password field of the repo empty.

garethgeorge avatar Nov 14 '24 21:11 garethgeorge

I agree it's the best approach. I was just going to suggest the same idea after pondering over it. It could work like so:

  1. Upon starting Backrest checks OS keyring for a predefined credential name.
  2. If it doesn't exist, proceed as is today.
  3. If it exists, obtain the password/key.
  4. Use this password/key for encrypting/decrypting config.json for any write/read disk operations with it.

This encryption password should be an optional feature, maybe even autogenerated by WebUI when enabled, with the ability for user to override to a custom password value. Sorry I can't provide code contributions, I know zero about Go.

but would be more painful re: an opaque encrypted file is essentially impossible to edit outside of backrest's UI. I think that could be mitigated by adding a JSON preview of the config or such in Backrest's UI possibly.

That's a good point, it's inevitable with full encryption approach. Maybe even allow edits in JSON preview in the UI? Or allow to export/import the config.

For now I implemented a workaround for Windows. Sharing it here in case anyone might find it useful. This approach uses Windows Data Protection API to stored encrypted values on disk (see https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/export-clixml?view=powershell-5.1). The sensitive variables are then decrypted and available as plain text in the session environment for Backrest process. It's not the highest security but good enough to prevent plain text on disk.

Open PowerShell and run this snippet to save encrypted password to a file. You may use anything for username, it won't be used.

Get-Credential | Export-Clixml $env:APPDATA\backrest\repo_cred.xml

In Backrest UI configuration add this backup flag to pass to restic:

--password-command "powershell (Import-Clixml $env:APPDATA/backrest/repo_cred.xml).GetNetworkCredential().Password"

Add this flag to both repo configuration and the backup plan. Note the use of forward slashes; this is intentional. Restic doesn't like backslashes with this flag, and PowerShell is smart enough to understand either one. Restic itself can take backslashes with proper quoting, but adding all the extra quotes breaks Backrest. Just use the approach above, it's easier anyway.

That's it for the repo password. Encrypting variables for cloud storage provider is trickier because right now the only way to supply environment variables to Backrest is with a wrapper script. This script will decrypt the values and create the variables, then launch Backrest.

Open PowerShell and run this snippet to save encrypted S3 access credentials.

$vars = @(  
  "AWS_ACCESS_KEY_ID"
  "AWS_SECRET_ACCESS_KEY"
)
$credentials = foreach ($var in $vars) {
  Get-Credential -UserName "$var" -Message "Enter your credentials"
}
$credentials | Export-Clixml $env:APPDATA\backrest\s3_cred.xml

Next, you need to create a startup script. Create start-backrest.ps1 file in %APPDATA%\backrest with this contents:

$appdir = "$env:APPDATA\backrest"
$creds = Import-CliXml "$appdir\s3_cred.xml"
foreach ($c in $creds) {
  [Environment]::SetEnvironmentVariable($c.UserName, $c.GetNetworkCredential().Password)
}
Start-Process "C:\Program Files\Backrest\backrest-windows-tray.exe" -WorkingDirectory "$appdir"

Next, you need to remove the original startup Backrest shortcut. Go to Start - Run, type shell:startup. Remove the shortcut.

Next, there are two options to start this script upon logon. Option 1 - startup folder In the same shell:startup folder right-click - New - Shortcut. Copy and paste this:

powershell -ExecutionPolicy Bypass -File %APPDATA%\backrest\start-backrest.ps1

Go to properties and change Run to "minimized".

Option 2 - Task Scheduler In PowerShell window run this snippet to create a scheduled task (you could also do it manually in GUI):

$trigger = New-ScheduledTaskTrigger -AtLogOn -User $env:USERNAME
$action = New-ScheduledTaskAction -Execute "powershell.exe" -Argument "-ExecutionPolicy Bypass -File %APPDATA%\backrest\start-backrest.ps1" -WorkingDirectory "%APPDATA%\backrest"
$settings = New-ScheduledTaskSettingsSet -AllowStartIfOnBatteries -ExecutionTimeLimit 0 -DontStopIfGoingOnBatteries -Hidden
Register-ScheduledTask -TaskName "Backrest" -Trigger $trigger -Action $action -Settings $settings
Start-ScheduledTask "Backrest"

If you use more than one bucket with the same provider, change the vars array above with some unique variable names like AWS_ACCESS_KEY_ID_2. You will then need to reference it in Backrest UI like so: AWS_ACCESS_KEY_ID=${AWS_ACCESS_KEY_ID_2} in order for this repo to use each own variables. That's because the variables above are defined within Backrest process environment and will be inherited by all restic instances.

homandr avatar Nov 16 '24 03:11 homandr

@garethgeorge any idea if this is something you'll be considering going forward with? As I might be able to help with introducing this.

Maybe there's a less intrusive approach than encrypting the whole file? You could modify config.json file to have these additional fields:

  1. hashedPassword
  2. privateKeyLocation

The hashed password would be for each repository, the private key could be on an application config level.

Both of these allow for

  • External programs to modify the config.json file, but keep the password secure.
  • Export of config data (you could provide an export functionality of the private key, and provide a way to update the secret key for the application)

Nathan-Nesbitt avatar Feb 17 '25 00:02 Nathan-Nesbitt

@homandr recently bumped this issue, appreciate raising it. @Nathan-Nesbitt I think using login keychain to encrypt configuration is still something I'm interested in, the way I imagine this working is a an implementation of the ConfigStore interface https://github.com/garethgeorge/backrest/blob/main/internal/config/config.go#L37-L40 which blocks on Get() until the login keychain is unlocked and the configuration can be loaded.

It's a good point that preserving the ability to edit the config externally is a nice feature. An alternative might be just encrypting portions of the config, i.e. repos?

Then configuration about what encryption is in use, and where to load keys can simply be stored elsewhere in unencrypted parts of the config.

garethgeorge avatar Jul 31 '25 07:07 garethgeorge

Not exactly sure it would help but this could be an example at least. The keychain has the passphrase stored as vorta-repo for Vorta.

amarendra avatar Aug 12 '25 09:08 amarendra