Skip to content

A sealed value is bound to its key but not to its place #159

Description

@vmillet-dev

Vault::seal_bytes calls encrypt(nonce, plaintext) with no associated data, so a sealed value is bound to the key and to nothing else. The GCM tag says "this was sealed by someone holding the key". It never says "this belongs in this row, in this column".

Anyone who can write the database file can therefore shuffle sealed values around, and the application opens every one of them and calls the result genuine.

Demonstrated

UPDATE notes SET content = title against a copy of the library, with no key at all:

RELOCATION 4/4 relocated ciphertexts still authenticate

The same shape covers the rest: swapping two notes' titles, copying one note's body over another's, moving an attachment's bytes onto a different attachment record, or rolling a value back to a ciphertext captured from an older backup. a_tampered_value_is_refused_rather_than_opened still passes throughout — it flips a byte, which is a different attack from moving an intact value.

Why it is worth fixing even though the threat model is "a file read at rest"

Confidentiality holds here; this is integrity. But the library is the place people keep the command that worked, and a note that silently becomes a different note is a nastier failure than one that refuses to open. The backups make the rollback variant realistic: an old devbox.sqlite3 sits right there, sealed under the same master key, so values from it paste cleanly into today's file.

Shape

  • Pass AAD binding each value to its identity: note_id || column for a note field, the space or folder id for a name, the attachment id for a file's bytes. seal/open grow a context argument, and the call sites are the only places that know what the context is.
  • ⚠️ This is a format change. Every value already on disk was sealed with empty AAD and will not open under a context. Decide between a migration that rewrites every sealed value — which is a full re-encrypt, with the atomicity problem Decide whether the database is encrypted at rest, and against what #26 avoided — and a version flag in vault.json that keeps reading old values without AAD while writing new ones with it. The second is cheaper and leaves the old rows attackable until they are next written.
  • Whatever is chosen, seal/open should stop being callable without a context, or the next feature will forget it the way this one could not have remembered it.

Done when

Moving an intact sealed value to another row, another column or another file is refused, and a test moves one to prove it.

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

    securitySecurity fix or hardening

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions