fix: update encryption scheme for node migration files (#2539)

* fix: update encryption scheme for node migration files

Migration files are now encrypted with AES-CTR using a key derived via
Argon2 with a 32-byte salt, the same derivation used for encrypted
configuration values. Files created by earlier versions can still be
restored: the restore path detects the scheme by trial-decrypting the
archive header and checking for the ZIP file signature, which also
rejects an incorrect unlock password up front instead of extracting
garbage.

The migration screen now also tells users to never share their
migration file with anyone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: reword migration file warning

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: read full migration file header before detecting cipher scheme

io.ReadAtLeast can return once the smallest scheme's header is read,
which truncates the larger current-scheme header when the reader
delivers short reads (e.g. a network request body). Read the full
header and only tolerate a short read that still covers the smallest
scheme.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: extract migration files to a staging directory during restore

If extraction failed partway through, the partially populated restore
directory was left in the working directory, and the next startup would
apply the incomplete restore. Extract to a staging directory and only
move it into place after every entry has been extracted successfully.
Also reject archives that contain no files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: assert traversal-specific error in restore backup test

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

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)
+	// Read the largest possible header (salt, IV and the first bytes of the
-	if _, err := io.ReadFull(r, salt); err != nil {
+	// archive) upfront, then trial-decrypt with each supported cipher scheme
-		return nil, fmt.Errorf("failed to read salt: %w", err)
+	// 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)
+	// Read the full header with io.ReadFull rather than io.ReadAtLeast: the
-	if _, err := io.ReadFull(r, iv); err != nil {
+	// reader may deliver short reads (e.g. a network request body), and
-		return nil, fmt.Errorf("failed to read IV: %w", err)
+	// 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)
+	return nil, errors.New("invalid unlock password or backup file")
-	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
 }

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)
 }

frontend/src/screens/MigrateNode.tsx

@@ -109,6 +109,16 @@ export function MigrateNode() {
             another device and use the “Advanced” option during the onboarding.
           </p>
         </div>
+        <div className="flex flex-col gap-1">
+          <div className="flex gap-3 items-center">
+            <TriangleAlertIcon className="size-4" />
+            <h3>Never share your migration file</h3>
+          </div>
+          <p className="text-sm ml-7">
+            Anyone with this file and your unlock password can access your
+            funds. Never send it to anyone. Alby support will never ask for it.
+          </p>
+        </div>
         <div className="flex flex-col gap-1">
           <div className="flex gap-3 items-center">
             <InfoIcon className="size-4" />