Correct the security claims in the docs and comments, and write down the accepted risks #171

Open
opened 2026-09-22 00:55:14 +02:00 by clawbot · 0 comments
Collaborator

This is definition-of-done item 4 of the security review #73, plus the false statements the review found. Docs and comments only; no behaviour change. Line numbers are as of next at 6fcd8e1. Severity: medium for part A items 1 and 2 (a document promises a property the design does not deliver), low for the rest.

A. Statements to correct

  1. Snapshot names are not hidden from someone who can guess them. The remote key is SHA256(SHA256("vaultik|" + id)) with no secret, the id is {hostname}_{name}_{UTC second}, and the manifest beside it publishes a timestamp one backup-duration later. An observer with a list of candidate hostnames and snapshot names hashes each over the preceding seconds and gets a yes or no. Overclaiming: docs/REPOSTRUCTURE.md:62 ("no structure a remote observer can reverse"), :69, :131; README.md:363-364; docs/DATAMODEL.md:220; the comment at internal/snapshot/snapshot.go:479-481 ("leaks no host", "no scheduling information"). Say instead: the remote key keeps names out of a plain listing; the backup time is public; a guessed hostname and snapshot name can be confirmed.
  2. Blob names. Nine places say a blob is named by the SHA-256 of its encrypted bytes: docs/REPOSTRUCTURE.md:38; ARCHITECTURE.md:77, :86; docs/DATAMODEL.md:93; internal/blob/packer.go:4, :10; internal/database/models.go:55, :61; internal/types/types.go:149. The name is hex(SHA256(SHA256(uncompressed blob contents))), computed before compression and encryption. State it once and link to it. Consequence to state: only the key holder can check a stored blob's integrity (restore, verify --deep).
  3. Double hashing. internal/blobgen/writer.go:127-129 says it stops an attacker confirming known content. It does not: anyone who can reproduce a blob's whole plaintext can compute the name. That is realistic for a snapshot made wholly of known content or a known file much larger than blob_size_limit, not for one ordinary file.
  4. Cipher. README.md:41 and :411 say XChaCha20-Poly1305; age uses ChaCha20-Poly1305.
  5. Encryption described as optional: internal/snapshot/scanner.go:120 ("empty means no encryption"), internal/snapshot/snapshot.go:27, :240, internal/vaultik/vaultik.go:92. Recipients are required and output is always encrypted.
  6. Consistency guarantee 4 (docs/REPOSTRUCTURE.md:143) says a snapshot is marked complete only after metadata upload. The code marks it complete first (internal/vaultik/snapshot.go:350, then :355), as ARCHITECTURE.md:339-341 says. State the real order and that a crash in between leaves a local-only record which vaultik prune removes. Do not swap the calls.
  7. Export comment (internal/snapshot/snapshot.go:243-247) says the caller closes the index before the copy; the only caller leaves it open. Correct the comment to what happens.
  8. "cryptographically verifies every blob" (README.md:77): say it detects corruption, not authorship.
  9. Recipients (internal/cli/config.go:48-49): "Backups are encrypted to ALL listed recipients" is false for a recipient added after data exists; see B6.
  10. Restore example (README.md:81) uses /tmp/restored, a predictable name in a shared directory. Use a path the user owns and say the target should be new or writable only by the user.

B. Accepted risks to write down

One section in docs/REPOSTRUCTURE.md under "Security Considerations", cross-referenced from the README threat-model text:

  1. No proof of authorship. Restore and verify --deep prove that data decrypts with the private key and matches unkeyed hashes. Anyone who knows a recipient string and can replace objects can substitute a snapshot they built. The recipient is not stored at the destination; a compromised backed-up host has it. Mitigations: bucket versioning or object lock, source-host credentials that cannot delete versions, prune from a trusted host. Do not write "write-only credentials": S3 PutObject overwrites. (Pending the owner's answer on #73.)
  2. Compression and sizes. Blobs and db.zst.age are zstd-compressed then age-encrypted; the manifest is compressed only. age does not pad, so object size gives the compressed length exactly. All new chunks packed into one blob share one zstd stream (8 MiB window, 4 MiB at levels 1-2); a blob closes at blob_size_limit and at the end of each configured path. Someone who can write into a backed-up file and watch blob sizes learns something only when their data and the secret sit in the same chunk of a file that keeps changing, because stored chunks are never packed again. Advice: back up outsider-writable directories as their own snapshot.
  3. Chunking uses no secret. FastCDC parameters are fixed and public; default 10MB average gives chunks of 2.5MB to 40MB, and any file of 2.5MB or less is one chunk. At the default 10GB blob_size_limit a blob holds hundreds of chunks, so chunk lengths are not visible; lowering the limit toward the chunk size exposes them. Say the same in one line next to blob_size_limit in config.example.yml.
  4. Decrypted data on local disk. snapshot restore stages decrypted blobs under $TMPDIR/vaultik-blobcache-* (no size cap) and the decrypted metadata database at $TMPDIR/vaultik-restore-*.db; verify --deep uses $TMPDIR/vaultik-verify-*.db; snapshot create keeps a plaintext index copy in $TMPDIR/vaultik-snapshot-*. All 0600. kill -9 or power loss leaves them; say what to delete and that TMPDIR should be as trusted as the restore target.
  5. Store permissions per command. README.md:5-7 says the host needs only what is required to PUT, while the recommended cron line (README.md:84) adds --prune, which lists, reads and deletes. List what each command needs and say --prune on the backed-up host means that host can delete its backups.
  6. Changing recipients does not re-encrypt. Deduplicated chunks and same-named blobs stay encrypted to the earlier recipients; new snapshots that reuse them cannot be restored with the new key alone. The remedy is vaultik database delete plus a full backup to a fresh destination or prefix.
  7. X25519 only. ssh and plugin recipients are rejected; long-lived ciphertext held by a third party has no protection if X25519 is ever broken.

Definition of done

  1. Every item in A is corrected at every listed location; every item in B is in the new section.
  2. make fmt run over the changed markdown; make check green.
  3. "Pruning Safety" is handled by #157, and age_secret_key documentation by #74 item 7; do not duplicate either.

model: fable-5-1

This is definition-of-done item 4 of the security review https://git.eeqj.de/sneak/vaultik/issues/73, plus the false statements the review found. Docs and comments only; no behaviour change. Line numbers are as of `next` at `6fcd8e1`. Severity: **medium** for part A items 1 and 2 (a document promises a property the design does not deliver), low for the rest. ## A. Statements to correct 1. **Snapshot names are not hidden from someone who can guess them.** The remote key is `SHA256(SHA256("vaultik|" + id))` with no secret, the id is `{hostname}_{name}_{UTC second}`, and the manifest beside it publishes a timestamp one backup-duration later. An observer with a list of candidate hostnames and snapshot names hashes each over the preceding seconds and gets a yes or no. Overclaiming: `docs/REPOSTRUCTURE.md:62` ("no structure a remote observer can reverse"), `:69`, `:131`; `README.md:363-364`; `docs/DATAMODEL.md:220`; the comment at `internal/snapshot/snapshot.go:479-481` ("leaks no host", "no scheduling information"). Say instead: the remote key keeps names out of a plain listing; the backup time is public; a guessed hostname and snapshot name can be confirmed. 2. **Blob names.** Nine places say a blob is named by the SHA-256 of its encrypted bytes: `docs/REPOSTRUCTURE.md:38`; `ARCHITECTURE.md:77`, `:86`; `docs/DATAMODEL.md:93`; `internal/blob/packer.go:4`, `:10`; `internal/database/models.go:55`, `:61`; `internal/types/types.go:149`. The name is `hex(SHA256(SHA256(uncompressed blob contents)))`, computed before compression and encryption. State it once and link to it. Consequence to state: only the key holder can check a stored blob's integrity (restore, `verify --deep`). 3. **Double hashing.** `internal/blobgen/writer.go:127-129` says it stops an attacker confirming known content. It does not: anyone who can reproduce a blob's whole plaintext can compute the name. That is realistic for a snapshot made wholly of known content or a known file much larger than `blob_size_limit`, not for one ordinary file. 4. **Cipher.** `README.md:41` and `:411` say XChaCha20-Poly1305; age uses ChaCha20-Poly1305. 5. **Encryption described as optional:** `internal/snapshot/scanner.go:120` ("empty means no encryption"), `internal/snapshot/snapshot.go:27`, `:240`, `internal/vaultik/vaultik.go:92`. Recipients are required and output is always encrypted. 6. **Consistency guarantee 4** (`docs/REPOSTRUCTURE.md:143`) says a snapshot is marked complete only after metadata upload. The code marks it complete first (`internal/vaultik/snapshot.go:350`, then `:355`), as `ARCHITECTURE.md:339-341` says. State the real order and that a crash in between leaves a local-only record which `vaultik prune` removes. Do not swap the calls. 7. **Export comment** (`internal/snapshot/snapshot.go:243-247`) says the caller closes the index before the copy; the only caller leaves it open. Correct the comment to what happens. 8. **"cryptographically verifies every blob"** (`README.md:77`): say it detects corruption, not authorship. 9. **Recipients** (`internal/cli/config.go:48-49`): "Backups are encrypted to ALL listed recipients" is false for a recipient added after data exists; see B6. 10. **Restore example** (`README.md:81`) uses `/tmp/restored`, a predictable name in a shared directory. Use a path the user owns and say the target should be new or writable only by the user. ## B. Accepted risks to write down One section in `docs/REPOSTRUCTURE.md` under "Security Considerations", cross-referenced from the README threat-model text: 1. **No proof of authorship.** Restore and `verify --deep` prove that data decrypts with the private key and matches unkeyed hashes. Anyone who knows a recipient string and can replace objects can substitute a snapshot they built. The recipient is not stored at the destination; a compromised backed-up host has it. Mitigations: bucket versioning or object lock, source-host credentials that cannot delete versions, prune from a trusted host. Do not write "write-only credentials": S3 `PutObject` overwrites. (Pending the owner's answer on https://git.eeqj.de/sneak/vaultik/issues/73.) 2. **Compression and sizes.** Blobs and `db.zst.age` are zstd-compressed then age-encrypted; the manifest is compressed only. age does not pad, so object size gives the compressed length exactly. All new chunks packed into one blob share one zstd stream (8 MiB window, 4 MiB at levels 1-2); a blob closes at `blob_size_limit` and at the end of each configured path. Someone who can write into a backed-up file and watch blob sizes learns something only when their data and the secret sit in the same chunk of a file that keeps changing, because stored chunks are never packed again. Advice: back up outsider-writable directories as their own snapshot. 3. **Chunking uses no secret.** FastCDC parameters are fixed and public; default 10MB average gives chunks of 2.5MB to 40MB, and any file of 2.5MB or less is one chunk. At the default 10GB `blob_size_limit` a blob holds hundreds of chunks, so chunk lengths are not visible; lowering the limit toward the chunk size exposes them. Say the same in one line next to `blob_size_limit` in `config.example.yml`. 4. **Decrypted data on local disk.** `snapshot restore` stages decrypted blobs under `$TMPDIR/vaultik-blobcache-*` (no size cap) and the decrypted metadata database at `$TMPDIR/vaultik-restore-*.db`; `verify --deep` uses `$TMPDIR/vaultik-verify-*.db`; `snapshot create` keeps a plaintext index copy in `$TMPDIR/vaultik-snapshot-*`. All 0600. `kill -9` or power loss leaves them; say what to delete and that `TMPDIR` should be as trusted as the restore target. 5. **Store permissions per command.** `README.md:5-7` says the host needs only what is required to PUT, while the recommended cron line (`README.md:84`) adds `--prune`, which lists, reads and deletes. List what each command needs and say `--prune` on the backed-up host means that host can delete its backups. 6. **Changing recipients does not re-encrypt.** Deduplicated chunks and same-named blobs stay encrypted to the earlier recipients; new snapshots that reuse them cannot be restored with the new key alone. The remedy is `vaultik database delete` plus a full backup to a fresh destination or prefix. 7. **X25519 only.** ssh and plugin recipients are rejected; long-lived ciphertext held by a third party has no protection if X25519 is ever broken. ## Definition of done 1. Every item in A is corrected at every listed location; every item in B is in the new section. 2. `make fmt` run over the changed markdown; `make check` green. 3. "Pruning Safety" is handled by https://git.eeqj.de/sneak/vaultik/issues/157, and `age_secret_key` documentation by https://git.eeqj.de/sneak/vaultik/issues/74 item 7; do not duplicate either. model: fable-5-1
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#171