diff --git a/api/backup.go b/api/backup.go index 9d2d7622..32e5aa03 100644 --- a/api/backup.go +++ b/api/backup.go @@ -1,9 +1,11 @@ package api import ( + "bytes" "errors" "fmt" "io" + "math" "strings" "time" @@ -16,12 +18,48 @@ import ( "crypto/rand" "crypto/sha256" + "github.com/getAlby/hub/config" "github.com/getAlby/hub/db" "github.com/getAlby/hub/logger" "github.com/getAlby/hub/utils" "golang.org/x/crypto/pbkdf2" ) +// zipMagic is the ZIP local file header signature "PK\x03\x04" — the first +// four bytes of every ZIP file, and therefore of every archive produced by +// CreateBackup. decryptingReader uses it to detect which cipher scheme the +// backup file was created with. +var zipMagic = []byte{'P', 'K', 0x03, 0x04} + +// backupCipher describes one of the cipher schemes used for backup files, +// which are laid out as salt || iv || encrypted zip archive. +type backupCipher struct { + saltSize int + deriveKey func(password string, salt []byte) ([]byte, error) + newStream func(block cipher.Block, iv []byte) cipher.Stream +} + +var backupCiphers = []backupCipher{ + // current scheme, used for all new backup files + { + saltSize: 32, + deriveKey: func(password string, salt []byte) ([]byte, error) { + key, _, err := config.DeriveKey(password, salt) + return key, err + }, + newStream: cipher.NewCTR, + }, + // legacy scheme, kept to restore backup files created by older versions + { + saltSize: 8, + deriveKey: func(password string, salt []byte) ([]byte, error) { + return pbkdf2.Key([]byte(password), salt, 4096, 32, sha256.New), nil + }, + //nolint:staticcheck // OFB is required to read files created by older versions + newStream: cipher.NewOFB, + }, +} + func (api *api) CreateBackup(unlockPassword string, w io.Writer) error { logger.Logger.Info("Creating backup to migrate Alby Hub to another device") var err error @@ -257,8 +295,22 @@ func (api *api) RestoreBackup(unlockPassword string, r io.Reader) error { return fmt.Errorf("failed to create zip reader: %w", err) } + if len(zr.File) == 0 { + return errors.New("backup file contains no files") + } + restoreDir := filepath.Join(workDir, "restore") + // Extract into a staging directory and only move it to the restore + // directory once every entry has been extracted, so that a failed + // extraction cannot leave a partial restore directory behind, which + // would be applied on the next startup. + stagingDir, err := os.MkdirTemp(workDir, "albyhub-restore-") + if err != nil { + return fmt.Errorf("failed to create staging directory: %w", err) + } + defer os.RemoveAll(stagingDir) + extractZipEntry := func(zipFile *zip.File) error { // Entry names come from the archive and must not be trusted. Reject any // name that is absolute or points outside the restore directory via @@ -268,11 +320,11 @@ func (api *api) RestoreBackup(unlockPassword string, r io.Reader) error { return fmt.Errorf("refusing to extract zip entry outside restore directory: %q", zipFile.Name) } - fsFilePath := filepath.Join(restoreDir, entryName) + fsFilePath := filepath.Join(stagingDir, entryName) - // Confirm the cleaned path is still contained within the restore + // Confirm the cleaned path is still contained within the staging // directory. - if fsFilePath != restoreDir && !strings.HasPrefix(fsFilePath, restoreDir+string(os.PathSeparator)) { + if fsFilePath != stagingDir && !strings.HasPrefix(fsFilePath, stagingDir+string(os.PathSeparator)) { return fmt.Errorf("refusing to extract zip entry outside restore directory: %q", zipFile.Name) } @@ -308,6 +360,13 @@ func (api *api) RestoreBackup(unlockPassword string, r io.Reader) error { } logger.Logger.WithField("count", len(zr.File)).Info("Extracted files") + if err = os.RemoveAll(restoreDir); err != nil { + return fmt.Errorf("failed to remove existing restore directory: %w", err) + } + if err = os.Rename(stagingDir, restoreDir); err != nil { + return fmt.Errorf("failed to move extracted files to restore directory: %w", err) + } + go func() { logger.Logger.Info("Backup restored. Shutting down Alby Hub...") api.svc.Shutdown() @@ -328,12 +387,17 @@ func (api *api) RestoreBackup(unlockPassword string, r io.Reader) error { } func encryptingWriter(w io.Writer, password string) (io.Writer, error) { - salt := make([]byte, 8) + scheme := backupCiphers[0] + + salt := make([]byte, scheme.saltSize) if _, err := rand.Read(salt); err != nil { return nil, fmt.Errorf("failed to generate salt: %w", err) } - encKey := pbkdf2.Key([]byte(password), salt, 4096, 32, sha256.New) + encKey, err := scheme.deriveKey(password, salt) + if err != nil { + return nil, fmt.Errorf("failed to derive encryption key: %w", err) + } block, err := aes.NewCipher(encKey) if err != nil { return nil, fmt.Errorf("failed to create AES cipher: %w", err) @@ -354,9 +418,8 @@ func encryptingWriter(w io.Writer, password string) (io.Writer, error) { return nil, fmt.Errorf("failed to write IV: %w", err) } - stream := cipher.NewOFB(block, iv) cw := &cipher.StreamWriter{ - S: stream, + S: scheme.newStream(block, iv), W: w, } @@ -364,27 +427,61 @@ func encryptingWriter(w io.Writer, password string) (io.Writer, error) { } func decryptingReader(r io.Reader, password string) (io.Reader, error) { - salt := make([]byte, 8) - if _, err := io.ReadFull(r, salt); err != nil { - return nil, fmt.Errorf("failed to read salt: %w", err) + // Read the largest possible header (salt, IV and the first bytes of the + // archive) upfront, then trial-decrypt with each supported cipher scheme + // and pick the one that produces the ZIP signature. + maxHeaderSize := 0 + minHeaderSize := math.MaxInt + for _, scheme := range backupCiphers { + headerSize := scheme.saltSize + aes.BlockSize + len(zipMagic) + maxHeaderSize = max(maxHeaderSize, headerSize) + minHeaderSize = min(minHeaderSize, headerSize) } - iv := make([]byte, aes.BlockSize) - if _, err := io.ReadFull(r, iv); err != nil { - return nil, fmt.Errorf("failed to read IV: %w", err) + // Read the full header with io.ReadFull rather than io.ReadAtLeast: the + // reader may deliver short reads (e.g. a network request body), and + // stopping early could truncate the header of a scheme with a larger + // salt. A short file is only acceptable if it still covers the smallest + // scheme header. + header := make([]byte, maxHeaderSize) + n, err := io.ReadFull(r, header) + if err != nil && !(errors.Is(err, io.ErrUnexpectedEOF) && n >= minHeaderSize) { + return nil, fmt.Errorf("failed to read backup header: %w", err) + } + header = header[:n] + + for _, scheme := range backupCiphers { + if len(header) < scheme.saltSize+aes.BlockSize+len(zipMagic) { + continue + } + salt := header[:scheme.saltSize] + iv := header[scheme.saltSize : scheme.saltSize+aes.BlockSize] + encrypted := header[scheme.saltSize+aes.BlockSize:] + + encKey, err := scheme.deriveKey(password, salt) + if err != nil { + return nil, fmt.Errorf("failed to derive encryption key: %w", err) + } + + block, err := aes.NewCipher(encKey) + if err != nil { + return nil, fmt.Errorf("failed to create AES cipher: %w", err) + } + + stream := scheme.newStream(block, iv) + decrypted := make([]byte, len(encrypted)) + stream.XORKeyStream(decrypted, encrypted) + if !bytes.Equal(decrypted[:len(zipMagic)], zipMagic) { + continue + } + + cr := &cipher.StreamReader{ + S: stream, + R: r, + } + + return io.MultiReader(bytes.NewReader(decrypted), cr), nil } - encKey := pbkdf2.Key([]byte(password), salt, 4096, 32, sha256.New) - block, err := aes.NewCipher(encKey) - if err != nil { - return nil, fmt.Errorf("failed to create AES cipher: %w", err) - } - - stream := cipher.NewOFB(block, iv) - cr := &cipher.StreamReader{ - S: stream, - R: r, - } - - return cr, nil + return nil, errors.New("invalid unlock password or backup file") } diff --git a/api/backup_test.go b/api/backup_test.go index 27bc112a..9ddfcc5a 100644 --- a/api/backup_test.go +++ b/api/backup_test.go @@ -3,11 +3,14 @@ package api import ( "archive/zip" "bytes" + "encoding/hex" "io" "os" "path/filepath" "strconv" + "strings" "testing" + "testing/iotest" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -148,15 +151,111 @@ func TestRestoreBackupRejectsPathTraversal(t *testing.T) { cw, err := encryptingWriter(&buf, unlockPassword) require.NoError(t, err) zw := zip.NewWriter(cw) - entryWriter, err := zw.Create(escapeEntryName) + // A valid entry before the malicious one, to verify that a partially + // extracted archive is not left behind when a later entry fails. + entryWriter, err := zw.Create("nwc.db") + require.NoError(t, err) + _, err = entryWriter.Write([]byte("backup contents")) + require.NoError(t, err) + entryWriter, err = zw.Create(escapeEntryName) require.NoError(t, err) _, err = entryWriter.Write([]byte("pwned")) require.NoError(t, err) require.NoError(t, zw.Close()) err = theAPI.RestoreBackup(unlockPassword, &buf) - require.Error(t, err) + require.ErrorContains(t, err, "refusing to extract zip entry outside restore directory") _, statErr := os.Stat(escapeTarget) require.True(t, os.IsNotExist(statErr), "traversal entry must not be written outside the restore directory") + + // The failed restore must not leave a restore directory (which would be + // applied on the next startup) or any staging leftovers. + _, statErr = os.Stat(filepath.Join(workDir, "restore")) + require.True(t, os.IsNotExist(statErr), "failed restore must not leave a restore directory") + + entries, err := os.ReadDir(workDir) + require.NoError(t, err) + for _, entry := range entries { + require.False(t, strings.HasPrefix(entry.Name(), "albyhub-restore-"), "failed restore must not leave a staging directory") + } +} + +// legacyBackupFixture is a backup file created with the encryption scheme +// used by older versions (PBKDF2 key derivation), encrypted with the +// password "test-unlock-password". Its archive contains a single "nwc.db" +// entry with the contents "legacy backup contents". +const legacyBackupFixture = "0102030405060708101112131415161718191a1b1c1d1e1f8eca79631915f679a00cdd95d3f20d8d169eb9aa5d52642ca13b93886c3c7d7ba4b759462bc9dd8deccf638edcc9b5b9fda3d23dcd904cf6e99bc57ac59c4df6be5aa676542b7cbc9998029420c0ae5a6986c735150ababde5b382560acaebd5894aa4420924f1ced63fde570adc60c43b32e9e14a0ef60c379da5cac1be0000845992ea072ead036e336c7b859e8d018c4ef61667e3f520fe01" + +// TestDecryptingReaderLegacyBackup verifies that backup files created by +// older versions can still be decrypted. +func TestDecryptingReaderLegacyBackup(t *testing.T) { + encrypted, err := hex.DecodeString(legacyBackupFixture) + require.NoError(t, err) + + cr, err := decryptingReader(bytes.NewReader(encrypted), "test-unlock-password") + require.NoError(t, err) + + decrypted, err := io.ReadAll(cr) + require.NoError(t, err) + + zr, err := zip.NewReader(bytes.NewReader(decrypted), int64(len(decrypted))) + require.NoError(t, err) + + dbFile, err := zr.Open("nwc.db") + require.NoError(t, err) + dbContents, err := io.ReadAll(dbFile) + require.NoError(t, err) + require.NoError(t, dbFile.Close()) + require.Equal(t, "legacy backup contents", string(dbContents)) +} + +// TestDecryptingReaderFragmentedReader verifies that a backup file is +// decrypted correctly even when the reader delivers one byte at a time, +// which would truncate the header if it were not read in full. +func TestDecryptingReaderFragmentedReader(t *testing.T) { + var buf bytes.Buffer + cw, err := encryptingWriter(&buf, "test-unlock-password") + require.NoError(t, err) + + zw := zip.NewWriter(cw) + entryWriter, err := zw.Create("nwc.db") + require.NoError(t, err) + _, err = entryWriter.Write([]byte("backup contents")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + + cr, err := decryptingReader(iotest.OneByteReader(bytes.NewReader(buf.Bytes())), "test-unlock-password") + require.NoError(t, err) + + decrypted, err := io.ReadAll(cr) + require.NoError(t, err) + + zr, err := zip.NewReader(bytes.NewReader(decrypted), int64(len(decrypted))) + require.NoError(t, err) + + dbFile, err := zr.Open("nwc.db") + require.NoError(t, err) + dbContents, err := io.ReadAll(dbFile) + require.NoError(t, err) + require.NoError(t, dbFile.Close()) + require.Equal(t, "backup contents", string(dbContents)) +} + +// TestDecryptingReaderWrongPassword verifies that decryption fails upfront +// when the password does not match the backup file. +func TestDecryptingReaderWrongPassword(t *testing.T) { + var buf bytes.Buffer + cw, err := encryptingWriter(&buf, "test-unlock-password") + require.NoError(t, err) + + zw := zip.NewWriter(cw) + entryWriter, err := zw.Create("nwc.db") + require.NoError(t, err) + _, err = entryWriter.Write([]byte("backup contents")) + require.NoError(t, err) + require.NoError(t, zw.Close()) + + _, err = decryptingReader(bytes.NewReader(buf.Bytes()), "wrong-password") + require.Error(t, err) } diff --git a/frontend/src/screens/MigrateNode.tsx b/frontend/src/screens/MigrateNode.tsx index 0c97b578..55bbb021 100644 --- a/frontend/src/screens/MigrateNode.tsx +++ b/frontend/src/screens/MigrateNode.tsx @@ -109,6 +109,16 @@ export function MigrateNode() { another device and use the “Advanced” option during the onboarding.
++ Anyone with this file and your unlock password can access your + funds. Never send it to anyone. Alby support will never ask for it. +
+