-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
Store sensitive values in OS keyring #548
Comments
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 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). |
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:
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. |
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. 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 |
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:
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.
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. 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. powershell -ExecutionPolicy Bypass -File %APPDATA%\backrest\start-backrest.ps1 Go to properties and change Run to "minimized". Option 2 - Task Scheduler $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. |
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.
The text was updated successfully, but these errors were encountered: