Make backups password-encrypted and self-contained#10
Conversation
The v1 backup left account keys and API-key secrets encrypted with two
keys that lived only in deploy/.env - losing the server meant the backup
was ciphertext that could never be opened. Restore refused it by design
(encryption_key_mismatch), so disaster recovery was impossible.
Backups now embed both keys (ACCOUNT_KEY_ENCRYPTION_KEY,
API_HMAC_MASTER_KEY) and are safe to do so because the whole file is
sealed under an admin-chosen password: argon2id (same cost posture as
authcrypto's password hashing, params travel in the envelope) deriving
an AES-256-GCM key - new internal/backupcrypto package.
Restore takes file + password. When the target server's .env keys differ
from the embedded ones (fresh install after losing the old server),
every accounts.private_key_encrypted and api_keys.secret_encrypted /
previous_secret_encrypted is re-encrypted to the current keys before
insert - the restored panel works with the new .env as-is. All
validation (password, schema match, re-encryption) happens before any
state is touched.
- Download is now POST /api/v1/backup (password rides in the body);
restore takes {password, backup}. wrong_password replaces the v1
key-canary check, which is no longer needed.
- Open() bounds KDF params from the file so a hostile envelope can't
demand unbounded memory before GCM auth rejects it.
- UI: password+confirm dialog on download, password prompt on restore,
both stating the password is unrecoverable.
- Replaces the hour-old v1 format outright - it was never in a release.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces password-encrypted backups and restores using Argon2id and AES-256-GCM, allowing full panel recovery on a fresh server by embedding the necessary deployment encryption keys and automatically re-encrypting sensitive columns during restore. The review feedback highlights critical security and stability issues where untrusted input in the backup envelope could trigger server panics. Specifically, the reviewer pointed out that argon2.IDKey can panic on invalid cost parameters, gcm.Open can panic on incorrect nonce lengths, and the restore handler can panic if the tables map is null. Actionable suggestions and test cases were provided to robustly guard against these vectors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Guard the KDF cost against a hostile file that would have us allocate | ||
| // unbounded memory before authentication can reject it. | ||
| if env.KDF.MemoryKB > 1024*1024 || env.KDF.TimeCost > 16 || env.KDF.Threads == 0 { | ||
| return nil, fmt.Errorf("unreasonable KDF parameters in backup file") | ||
| } |
There was a problem hiding this comment.
The argon2.IDKey function in golang.org/x/crypto/argon2 will panic if time < 1 (i.e., TimeCost == 0) or if memory < 8 * threads (i.e., MemoryKB < 8 * Threads). Since these parameters are read directly from the untrusted backup file envelope, a malicious or malformed backup file can easily trigger a server panic and cause a Denial of Service (DoS).
We should explicitly validate these bounds before calling argon2.IDKey.
| // Guard the KDF cost against a hostile file that would have us allocate | |
| // unbounded memory before authentication can reject it. | |
| if env.KDF.MemoryKB > 1024*1024 || env.KDF.TimeCost > 16 || env.KDF.Threads == 0 { | |
| return nil, fmt.Errorf("unreasonable KDF parameters in backup file") | |
| } | |
| // Guard the KDF cost against a hostile file that would have us allocate | |
| // unbounded memory or trigger a panic in argon2 before authentication can reject it. | |
| if env.KDF.MemoryKB > 1024*1024 || env.KDF.TimeCost > 16 || env.KDF.TimeCost == 0 || env.KDF.Threads == 0 || env.KDF.MemoryKB < 8*uint32(env.KDF.Threads) { | |
| return nil, fmt.Errorf("unreasonable KDF parameters in backup file") | |
| } |
| gcm, err := cipher.NewGCM(block) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| plaintext, err := gcm.Open(nil, nonce, data, nil) |
There was a problem hiding this comment.
The gcm.Open function will panic if the provided nonce slice length is not exactly equal to gcm.NonceSize() (which is 12 bytes for AES-GCM). Since the nonce is decoded from the untrusted backup envelope, an invalid nonce length will crash the server.
We should validate the nonce length before calling gcm.Open.
| gcm, err := cipher.NewGCM(block) | |
| if err != nil { | |
| return nil, err | |
| } | |
| plaintext, err := gcm.Open(nil, nonce, data, nil) | |
| gcm, err := cipher.NewGCM(block) | |
| if err != nil { | |
| return nil, err | |
| } | |
| if len(nonce) != gcm.NonceSize() { | |
| return nil, ErrWrongPassword | |
| } | |
| plaintext, err := gcm.Open(nil, nonce, data, nil) |
| var backup backupPayload | ||
| if err := json.Unmarshal(plaintext, &backup); err != nil { | ||
| writeJSONError(w, http.StatusBadRequest, "invalid_request", "backup contents are not valid JSON") | ||
| return | ||
| } | ||
| if backup.Secrets.AccountKeyEncryptionKey == "" || backup.Secrets.APIHMACMasterKey == "" { | ||
| writeJSONError(w, http.StatusBadRequest, "invalid_request", "backup is missing its embedded encryption keys") | ||
| return | ||
| } |
There was a problem hiding this comment.
If the uploaded backup JSON does not contain the "tables" key (or if it is null), backup.Tables will be nil. In Go, assigning to a key of a nil map (e.g., backup.Tables["accounts"] = reencrypted) will trigger a runtime panic.
We should validate that backup.Tables is not nil before proceeding with the restore process.
var backup backupPayload
if err := json.Unmarshal(plaintext, &backup); err != nil {
writeJSONError(w, http.StatusBadRequest, "invalid_request", "backup contents are not valid JSON")
return
}
if backup.Secrets.AccountKeyEncryptionKey == "" || backup.Secrets.APIHMACMasterKey == "" {
writeJSONError(w, http.StatusBadRequest, "invalid_request", "backup is missing its embedded encryption keys")
return
}
if backup.Tables == nil {
writeJSONError(w, http.StatusBadRequest, "invalid_request", "backup is missing its tables data")
return
}| func TestOpenRejectsHostileKDFParams(t *testing.T) { | ||
| env, err := Seal("pw", []byte("data"), "2026-07-18T00:00:00Z") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| env.KDF.MemoryKB = 64 * 1024 * 1024 // 64GB - a memory-exhaustion attempt | ||
| if _, err := Open("pw", env); err == nil || errors.Is(err, ErrWrongPassword) { | ||
| t.Fatalf("expected a KDF-parameter rejection, got %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
To ensure the new panic guards are robust and prevent regressions, we should add test cases verifying that Open gracefully rejects invalid KDF parameters (like TimeCost = 0 or insufficient MemoryKB) and invalid nonce lengths without panicking.
func TestOpenRejectsHostileKDFParams(t *testing.T) {
env, err := Seal("pw", []byte("data"), "2026-07-18T00:00:00Z")
if err != nil {
t.Fatal(err)
}
env.KDF.MemoryKB = 64 * 1024 * 1024 // 64GB - a memory-exhaustion attempt
if _, err := Open("pw", env); err == nil || errors.Is(err, ErrWrongPassword) {
t.Fatalf("expected a KDF-parameter rejection, got %v", err)
}
// Test TimeCost = 0 (panic vector in argon2)
env2, _ := Seal("pw", []byte("data"), "2026-07-18T00:00:00Z")
env2.KDF.TimeCost = 0
if _, err := Open("pw", env2); err == nil || errors.Is(err, ErrWrongPassword) {
t.Fatalf("expected rejection for TimeCost=0, got %v", err)
}
// Test MemoryKB too low for threads (panic vector in argon2)
env3, _ := Seal("pw", []byte("data"), "2026-07-18T00:00:00Z")
env3.KDF.MemoryKB = 1
env3.KDF.Threads = 4
if _, err := Open("pw", env3); err == nil || errors.Is(err, ErrWrongPassword) {
t.Fatalf("expected rejection for low MemoryKB, got %v", err)
}
// Test invalid nonce length (panic vector in gcm.Open)
env4, _ := Seal("pw", []byte("data"), "2026-07-18T00:00:00Z")
env4.Nonce = ""
if _, err := Open("pw", env4); !errors.Is(err, ErrWrongPassword) {
t.Fatalf("expected ErrWrongPassword for invalid nonce length, got %v", err)
}
}1 participant
Problem (caught by @iamfarhad right after #9 merged)
Account WireGuard keys and bot API-key secrets are stored encrypted with
ACCOUNT_KEY_ENCRYPTION_KEY/API_HMAC_MASTER_KEY, which live only indeploy/.env. The v1 backup deliberately excluded them - so losing the server meant losing the ability to ever open the backup. Restore even refused withencryption_key_mismatch. A backup that can't survive server loss isn't a backup.Fix
.envkeys and are safe to do so because the entire file is sealed under an admin-chosen password: argon2id → AES-256-GCM (newinternal/backupcryptopackage; KDF params travel in the envelope so they can be tuned without breaking old files)..env), everyprivate_key_encrypted/secret_encrypted/previous_secret_encryptedis re-encrypted to the current keys before insert. The restored panel works with the new.envas-is - nothing from the old server is needed beyond the file and its password.install.sh→ log in with bootstrap admin → Settings → Restore →wgpanel restart(flagged by the UI when the CA changed) → repoint DNS.POST /api/v1/backup(password in body); restore takes{password, backup}. The v1 key-canary check is gone -wrong_passwordcovers it.Open()bounds the envelope's KDF parameters so a hostile file can't demand unbounded memory before GCM authentication rejects it.Replaces the v1 format outright - it merged an hour ago and was never in a release, so no compatibility shim.
Verification
backupcryptotests: seal/open round-trip through JSON serialization, plaintext-leak check, wrong password →ErrWrongPassword, tampered ciphertext rejected, hostile KDF params rejected.reencryptRowstests: decryptable under new key after transform, nullable columns pass through, wrong source key fails loudly, empty blobs pass through.gofmt/go vet/go test ./...clean; frontendtsc+ build clean; OpenAPI updated (both copies).🤖 Generated with Claude Code