SEC2-032: Backup restore treats an archive whose signature was removed as "written before signing" and restores its database and media #974

New issue

Closed

opened 2026-10-17 10:38:52 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-17 10:38:52 +00:00

Owner

Found on 2026-10-16 in a read-only security review of postulo at 56fc9d531, checked by a second, independent pass.

Severity: Medium · Category: Cryptography · Reference: CWE-345 (Insufficient Verification of Data Authenticity); CWE-757 (Selection of Less-Secure Algorithm During Negotiation); OWASP A08:2021 (Software and Data Integrity Failures)

Location:

src/postulo/core/backup.py:713-722 (authenticity: a manifest with no authentication key is UNSIGNED)
src/postulo/core/backup.py:800-810 (restore_backup refuses only UNVERIFIED)
src/postulo/core/backup.py:851-855 (an unsigned archive loses only plugins/)
src/postulo/core/management/commands/restore.py:65-80 (a warning line, exit status 0)
tests/test_backup.py:547-563 and tests/test_hardening_772.py:138-162 (both pin the permissive behaviour)
docs/THREAT-MODEL.md (row "Someone who obtains a backup", the sentence "Someone who can alter a copy of a backup is stopped by its signature (#772)")

Issue

The signature added for #772 is only consulted when the manifest claims one. authenticity() returns UNSIGNED the moment the key is absent:

claimed = (manifest.get("authentication") or {}).get("mac")
if not claimed:
    return UNSIGNED, {}

and restore_backup refuses only the UNVERIFIED verdict; for anything else that is not AUTHENTIC it withholds plugins/ and nothing more:

if report.authenticity == UNVERIFIED and not unverified:
    raise BackupError(...)
...
if report.authenticity != AUTHENTIC and not unverified:
    kept = [item for item in files if item[1] != PLUGINS_PREFIX]

The database member and every media/ file are then written. verify_backup compares the database member only with the sha256 in the same manifest, which whoever alters the archive also writes. So altering a copy of a signed archive does not require forging the MAC: an archive without its authentication object is accepted with a warning saying it "was written before archives were signed". Nothing distinguishes a genuine pre-#772 archive from one whose signature was removed, because BACKUP_FORMAT stayed at 2 when signing arrived. tests/test_backup.py:547-563 (test_an_archive_from_before_the_plugins_went_in_still_restores) and tests/test_hardening_772.py:138-162 both restore such an archive without unverified=True.

The threat model promises more than the code does ("a restore refuses an archive that does not check out ... unless --unverified"), while the wiki page Backups and your data (line 317) already says an archive taken before signing needs --unverified: the documentation is ahead of the code. The Restore page (BackupRestoreView) shows the checksum result as "verified" without computing authenticity(), and suggests a restore command without --unverified.

Impact

Someone with write access to where backups are copied (an off-site share, a cloud bucket, a workstation the archive passes through) who waits for the operator to run manage.py restore gets their altered database restored: an added administrator row or a replaced password hash is a full takeover of the restored instance and every account's documents on it; replaced media files are served to their owners as their own CVs and letters. The restore prints a warning but exits successfully, so a scripted disaster-recovery run never stops. Plugins (code run at start-up) are withheld, which is the one half of #772 that does hold.

Fix

Treat an unsigned archive like an unverified one: a restore requires the operator's explicit --unverified for both, so an archive from before signing is restored once, knowingly, and an altered one is never restored by accident.

# backup.py, restore_backup
if report.authenticity != AUTHENTIC and not unverified:
    raise BackupError(
        "This archive carries no signature this instance can check ..."
        if report.authenticity == UNSIGNED
        else "This archive was not signed by this instance's key ...",
    )

On the Restore page, have BackupRestoreView compute authenticity() (it already opens the archive to read the manifest), show the verdict (authentic / unsigned / unverified) beside the checksum result, and print --unverified in the suggested commands when the verdict is not authentic. Update the threat-model sentence to match. As defence in depth, and only together with the first change, bump BACKUP_FORMAT to 3 and refuse a format-3 manifest without authentication outright.

Done when: tests/test_backup.py takes a fresh archive, rewrites the manifest without authentication, and asserts restore_backup(old) raises BackupError while restore_backup(old, unverified=True) restores and reports authenticity == UNSIGNED; the existing tests at tests/test_backup.py:547 and tests/test_hardening_772.py:138 pass unverified=True.

#772 (closed) - the signature was added for it, but its text and the threat model describe the unsigned path as a pre-signing archive restored without plugins/; neither names an archive whose signature was removed, so the fix is incomplete on this point. #811 (closed) - the key mark, unrelated. #234 (closed) - the backup documentation.

Found on 2026-10-16 in a read-only security review of `postulo` at `56fc9d531`, checked by a second, independent pass. **Severity:** Medium · **Category:** Cryptography · **Reference:** CWE-345 (Insufficient Verification of Data Authenticity); CWE-757 (Selection of Less-Secure Algorithm During Negotiation); OWASP A08:2021 (Software and Data Integrity Failures) **Location:** - `src/postulo/core/backup.py:713-722` (`authenticity`: a manifest with no `authentication` key is `UNSIGNED`) - `src/postulo/core/backup.py:800-810` (`restore_backup` refuses only `UNVERIFIED`) - `src/postulo/core/backup.py:851-855` (an unsigned archive loses only `plugins/`) - `src/postulo/core/management/commands/restore.py:65-80` (a warning line, exit status 0) - `tests/test_backup.py:547-563` and `tests/test_hardening_772.py:138-162` (both pin the permissive behaviour) - `docs/THREAT-MODEL.md` (row "Someone who obtains a backup", the sentence "Someone who can alter a copy of a backup is stopped by its signature (#772)") ## Issue The signature added for #772 is only consulted when the manifest claims one. `authenticity()` returns `UNSIGNED` the moment the key is absent: ```python claimed = (manifest.get("authentication") or {}).get("mac") if not claimed: return UNSIGNED, {} ``` and `restore_backup` refuses only the `UNVERIFIED` verdict; for anything else that is not `AUTHENTIC` it withholds `plugins/` and nothing more: ```python if report.authenticity == UNVERIFIED and not unverified: raise BackupError(...) ... if report.authenticity != AUTHENTIC and not unverified: kept = [item for item in files if item[1] != PLUGINS_PREFIX] ``` The database member and every `media/` file are then written. `verify_backup` compares the database member only with the `sha256` in the same manifest, which whoever alters the archive also writes. So altering a copy of a signed archive does not require forging the MAC: an archive without its `authentication` object is accepted with a warning saying it "was written before archives were signed". Nothing distinguishes a genuine pre-#772 archive from one whose signature was removed, because `BACKUP_FORMAT` stayed at 2 when signing arrived. `tests/test_backup.py:547-563` (`test_an_archive_from_before_the_plugins_went_in_still_restores`) and `tests/test_hardening_772.py:138-162` both restore such an archive without `unverified=True`. The threat model promises more than the code does ("a restore refuses an archive that does not check out ... unless `--unverified`"), while the wiki page *Backups and your data* (line 317) already says an archive taken before signing needs `--unverified`: the documentation is ahead of the code. The Restore page (`BackupRestoreView`) shows the checksum result as "verified" without computing `authenticity()`, and suggests a restore command without `--unverified`. ## Impact Someone with write access to where backups are copied (an off-site share, a cloud bucket, a workstation the archive passes through) who waits for the operator to run `manage.py restore` gets their altered database restored: an added administrator row or a replaced password hash is a full takeover of the restored instance and every account's documents on it; replaced media files are served to their owners as their own CVs and letters. The restore prints a warning but exits successfully, so a scripted disaster-recovery run never stops. Plugins (code run at start-up) are withheld, which is the one half of #772 that does hold. ## Fix Treat an unsigned archive like an unverified one: a restore requires the operator's explicit `--unverified` for both, so an archive from before signing is restored once, knowingly, and an altered one is never restored by accident. ```python # backup.py, restore_backup if report.authenticity != AUTHENTIC and not unverified: raise BackupError( "This archive carries no signature this instance can check ..." if report.authenticity == UNSIGNED else "This archive was not signed by this instance's key ...", ) ``` On the Restore page, have `BackupRestoreView` compute `authenticity()` (it already opens the archive to read the manifest), show the verdict (`authentic` / `unsigned` / `unverified`) beside the checksum result, and print `--unverified` in the suggested commands when the verdict is not `authentic`. Update the threat-model sentence to match. As defence in depth, and only together with the first change, bump `BACKUP_FORMAT` to 3 and refuse a format-3 manifest without `authentication` outright. Done when: `tests/test_backup.py` takes a fresh archive, rewrites the manifest without `authentication`, and asserts `restore_backup(old)` raises `BackupError` while `restore_backup(old, unverified=True)` restores and reports `authenticity == UNSIGNED`; the existing tests at `tests/test_backup.py:547` and `tests/test_hardening_772.py:138` pass `unverified=True`. ## Related #772 (closed) - the signature was added for it, but its text and the threat model describe the unsigned path as a pre-signing archive restored without `plugins/`; neither names an archive whose signature was removed, so the fix is incomplete on this point. #811 (closed) - the key mark, unrelated. #234 (closed) - the backup documentation.

tiagoagueda added the

bug

security

labels

2026-10-17 10:38:53 +00:00

tiagoagueda added this to the 0.5.0 milestone

2026-10-17 10:38:54 +00:00

tiagoagueda referenced this issue

2026-10-17 10:41:22 +00:00

SEC2 index: security review of 2026-10-16 (86 issues across the core, plugins and extensions) #992

tiagoagueda referenced this issue from a commit

2026-10-17 14:17:26 +00:00