Skip to content

Vault key metadata sits outside the AEAD, so a swapped public key still unlocks the vault #57

Description

@Sadykhzadeh

Tested against main (4abcf33).

What happens

vault.json has two parts. One is authenticated, the other is not, and the key list lives in the wrong one.

// core/src/vault.rs:18-23
#[derive(Serialize, Deserialize)]
pub struct VaultFile {
    salt: String,
    ciphertext: String,
    pub keys_meta: Vec<KeyMeta>,
}

KeyMeta (keys.rs:11-18) carries fingerprint and public_key_base64 — the two values a user reads out of the app and pastes into a server's authorized_keys. Both sit outside the ciphertext in cleartext.

Nothing binds them to it. seal passes the plaintext straight to AES-GCM with no associated data (vault.rs:208-209), and open decrypts the same way (vault.rs:225-226). The AEAD tag covers the ciphertext and nothing else. There is also no version field, no counter, and no MAC over the file as a whole.

So the header can be rewritten at will and the vault still unlocks.

Attack 1 — metadata substitution

The webview already has arbitrary local read and arbitrary local write, both filed as #12:

  • commands.rs:732read_key_file(path), any path, 256 KB cap. A vault with a handful of keys is a few KB.
  • sftp_transfer.rs:15sftp_download_to_local, which writes attacker-served bytes to any local path.

Chain them: read vault.json, hand back the same JSON with one entry's public_key_base64 and fingerprint swapped for the attacker's Ed25519 key, write it back over the original. The ciphertext is untouched, so the tag still verifies and the vault unlocks normally. list_keys (commands.rs:146-149) then hands the UI the attacker's public key under the user's own label, and the user copies "their" key into authorized_keys on a production host. Nothing anywhere notices.

The private half of that entry is still the user's, so signing with it also keeps working — the vault behaves normally in every way the user can observe.

Confirmed in a local test: after swapping only public_key_base64 and fingerprint, verify_passphrase succeeds and keys_meta() returns the attacker's key with the label prod-bastion intact.

Attack 2 — silent rollback

The same write primitive, a restored backup, or a roaming profile that syncs the app data directory reverts vault.json wholesale. A key deliberately deleted after a compromise comes back and get_key returns its private material again. There is no epoch or generation counter anywhere, so nothing distinguishes an old file from the current one.

Also confirmed locally: back up vault.json, remove_key, restore the backup, and the removed key is present and readable again.

Why it matters, and what the actual barrier is

This is post-XSS impact, not a standalone remote break. Attack 1 needs the #12 write primitive, which in turn needs code running in the webview, and #30 covers why that is currently hard to get: the CSP has no script-src override and no 'unsafe-eval', and there is no v-html/innerHTML/eval sink in desktop/src/. The strongest local-write path also requires the attacker to stand up an SFTP server and drive a connection to it.

What makes it worth fixing on its own terms is that the vault is supposed to be the thing that survives that. It is the only persisted form of secrets in the app, and the whole point of authenticated encryption is that a modified file is refused rather than trusted. Right now the AEAD protects the private keys and leaves the values the user actually acts on — the fingerprint and the public key — unprotected next to them. A user who is handed an attacker's public key under a trusted label installs a backdoor on a production host with their own hands, and neither the app nor the server logs will show anything unusual.

Attack 2 is also reachable without any attacker at all: a routine restore from backup silently resurrects a revoked key.

Related: #12 (the read/write primitives), #19 (passphrase handling in the same area), #51 (KDF parameters, separately).

Suggested fix

Version the file and bind the header to the ciphertext as AEAD associated data:

{ "version": 1, "salt": "<b64>", "nonce": "<b64>", "epoch": <n>,
  "keys_meta": [...], "ciphertext": "<b64>" }

with (version, salt, epoch, keys_meta) passed as aes_gcm::aead::Payload { msg, aad }. Editing any of them then fails the tag check and the vault refuses to open.

keys_meta should stay in the header rather than moving inside the ciphertext. The key list is rendered before the vault is unlocked — keys.load() runs unconditionally at App.vue:67 and again on mount in KeyManager.vue and IdentityManager.vue — and list_keys does not require an unlocked vault. Moving the metadata inside the ciphertext would leave the Keys view empty on a locked cold start, next to a reset button, which is its own kind of hazard.

Migration has to read the current format and rewrite it on first unlock, when the master key is available, and a failed migration must leave the old file intact and usable. The atomic-write helper in fs_util.rs already gives that.

The epoch is only half a fix on its own: an attacker rewriting the file rewrites the epoch too, so recognizing a rollback needs a high-water mark held somewhere the file cannot reach. Every candidate location in the app data directory is writable by the same primitive, so that half likely needs the OS keychain and should be tracked separately.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    SecurityWhen there are security issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions