# Security review - pg_vault_tde 1.7.2 (self-review)

Review conducted against [doc/SECURITY-REVIEW.md](../../SECURITY-REVIEW.md),
supported by the automated evidence in [doc/security/evidence/v1.7.2.md](../evidence/v1.7.2.md).
As specified in the workflow, while the project has a single maintainer, the author
conducts and signs this review as a self-review until an independent reviewer is assigned.

| | |
|---|---|
| Reviewer | Matteo Durighetto `<m.durighetto@miriade.it>` |
| Key fingerprint | `F365 8FA1 6FBD A667 021D  8503 CD36 A301 03F3 6119` |
| Commit reviewed | `02bcf69f6a0d22cc9a81686f7113e6ef80ad1cd4`|
| Scope | Full review (first review; the on-disk format changed to v5); self-review |
| Date | 2026-10-02 |
| Evidence | [doc/security/evidence/v1.7.2.md](../evidence/v1.7.2.md) |
| Result | **PASS** - Release 1.7.2 approved |

---

## Executive Summary

This is the first formal security review of `pg_vault_tde`, conducted for release **1.7.2**.
The scope is a **Full review** because the release introduces on-disk tuple format **v5**
(preserving tuple physical structure to prevent crashes and ensure compatibility with core
PostgreSQL update operations).

All automated verification stages executed by `make ci-security-report` passed with zero errors,
zero memory safety violations (ASan, UBSan, Valgrind clean), zero static analysis findings (scan-build,
CodeQL, Semgrep), and clean assertion runs (cassert).

All findings recorded for 1.7.2 have been addressed:
- **PSQLE-178** (Medium): Fixed - IV batch ownership tracking prevents reuse across `fork()`, limit documented (2^32 encryptions/DEK), and client tools secure `search_path`.
- **PSQLE-180** (Medium): Fixed - CI actions and container images pinned by commit SHA/digest, signed releases and source SBOM/vulnerability scans established.
- **PSQLE-205** (Medium): Fixed - `pg_vault_tde_reencrypt_table()` enforces `MAINTAIN` privilege check on the calling role.
- **PSQLE-206** (High): Fixed - Key-management functions enforce superuser checks on the calling role (`GetOuterUserId()`), preventing privilege escalation via `SECURITY DEFINER`.

No **High** severity findings remain open. The two open findings (**PSQLE-217** and **PSQLE-218**)
are Medium and Low severity respectively, have documented mitigations/accepted risk notes in the
documentation, and are scheduled for resolution in release 1.8.

---

## Evidence Summary

From `make ci-security-report` on commit `1cb4914a49ea10ebbcb9b1356e2d93101a2bef8e` (clean working tree):

| Stage | Result | Details |
|---|---|---|
| `pins` | PASS | 0 unpinned references |
| `semgrep` | PASS | 7/7 rules passed; 39 files scanned: 0 findings; 6 suppressed lines reviewed |
| `sbom` | PASS | Syft 1.52.0 / Grype 0.119.0: 1 package, 0 vulnerabilities |
| `errorpath` | PASS | 13/13 error path tests passed |
| `scan-build` | PASS | Clang 19.1.7: 0 defect reports |
| `ubsan` | PASS | GCC 14.2.0: 0 runtime undefined behavior errors |
| `asan` | PASS | GCC 14.2.0: 0 memory errors |
| `valgrind` | PASS | Valgrind 3.24.0: 0 invalid access, 0 leaks, 0 uninitialised jumps |
| `cassert` | PASS | PostgreSQL 18.6 with assertions: 4 SQL files (138 tests) passed, 48 TAP files (964 tests) passed |

From github pipeline 
| Stage | Result | Details |
|---|---|---|
| `CodeQL` | PASS | 0 finding |

---

## Suppressed Findings Review

Every `nosemgrep` directive in `src/` was examined and confirmed valid:

1. **`src/kms/pg_vault_tde_rotation_bgw.c:125`** (`tde-caller-superuser`):
   - *Comment:* `/* nosemgrep: tde-caller-superuser - not SECURITY DEFINER: superuser() is the caller */`
   - *Verdict:* **Valid**. The background worker launcher function is not `SECURITY DEFINER`. `superuser()` therefore evaluates the outer session role directly.
2. **`src/kms/pg_vault_tde_kms_local.c:2080`** (`tde-cleanse-before-free`):
   - *Comment:* `/* nosemgrep: tde-cleanse-before-free - cleansed in the PG_FINALLY above */`
   - *Verdict:* **Valid**. `old_pass` is cleansed using `OPENSSL_cleanse()` within the preceding `PG_FINALLY` block (lines 2060–2065) before reaching `pfree(old_pass)`.
3. **`src/kms/pg_vault_tde_kms_local.c:2082`** (`tde-cleanse-before-free`):
   - *Comment:* `/* nosemgrep: tde-cleanse-before-free - cleansed in the PG_FINALLY above */`
   - *Verdict:* **Valid**. `new_pass` is cleansed using `OPENSSL_cleanse()` within the preceding `PG_FINALLY` block (lines 2060–2065) before reaching `pfree(new_pass)`.
4. **`src/tam/pg_vault_tde_tam.c:909`** (`tde-rd-tableam`):
   - *Comment:* `/* nosemgrep: tde-rd-tableam - to be replaced by direct heapam calls, PSQLE-213 */`
   - *Verdict:* **Valid**. Temporary pointer substitution to delegate to core heapam; restored reliably across all normal and error paths. Tracked for direct heapam replacement in v1.8 (PSQLE-213).
5. **`src/tam/pg_vault_tde_tam.c:1439`** (`tde-rd-tableam`):
   - *Comment:* `/* nosemgrep: tde-rd-tableam - to be replaced by direct heapam calls, PSQLE-213 */`
   - *Verdict:* **Valid**. Temporary pointer substitution restored across all execution paths; tracked under PSQLE-213.
6. **`src/tam/pg_vault_tde_tam.c:2824`** (`tde-rd-tableam`):
   - *Comment:* `/* nosemgrep: tde-rd-tableam - to be replaced by direct heapam calls, PSQLE-213 */`
   - *Verdict:* **Valid**. Temporary pointer substitution restored across all execution paths; tracked under PSQLE-213.

---

## Detailed Checklist

### 1. Cryptography - `src/crypto/`, `src/iam/`

| Item | Result | Notes |
|---|---|---|
| AES-256-GCM for tuples and TOAST chunks: 96-bit IV from `pg_strong_random()`, no reuse under one DEK across `fork()`, encryption limit per DEK | **verified** | Semgrep rule `tde-strong-random` verified. Per-process 256-IV batch tracks `MyProcPid` and refills if process changes (PSQLE-178). 2^32 limit per DEK generation documented. |
| The tag is verified before any plaintext is used; failure raises error and frees buffers | **verified** | Tag verified via `EVP_DecryptFinal_ex()`; error raised on tag mismatch before decrypted data is exposed; buffers cleansed and freed. |
| The AAD is rebuilt from the reader's context, not read from disk; covers documented scope | **verified** | Reconstructed from database OID, relation OID, and DEK generation. Scope matches PSQLE-177 and PSQLE-218. |
| AES-256-SIV for `tde_btree` keys: key length, determinism limited to equality, no plaintext key reaches index page | **verified** | AES-256-SIV (RFC 5297) deterministic encryption; equality preserved; ciphertext on index pages; limitations documented. |
| Key wrapping: AES-256 key wrap (local), transit (Vault), `CKM_AES_KEY_WRAP_PAD` (PKCS#11); wrapped DEK carries KEK version | **verified** | Local wallet uses RFC 3394/5649 key wrap; Vault uses Transit; PKCS#11 uses `CKM_AES_KEY_WRAP_PAD`; `kek_version` stored in catalog. |
| Derivations and MACs: PKCS#12 iteration counts, PBKDF2 for seal passphrase, HMAC compared with `CRYPTO_memcmp` | **verified** | PKCS#12 iterations follow OpenSSL standards; PBKDF2 SHA-256 for seals; constant-time `CRYPTO_memcmp()` verified (rule `tde-constant-time-compare`). |
| Every buffer holding key, IV batch or passphrase is cleansed, on error paths too | **verified** | `OPENSSL_cleanse()` used on sensitive buffers; error cleanup in `PG_FINALLY`/`PG_CATCH` (rule `tde-cleanse-before-free`). |

### 2. Key management - `src/kms/`

| Item | Result | Notes |
|---|---|---|
| Shared-memory DEK cache: accessibility, capacity, evicted/replaced entries cleansed | **verified** | Protected by LWLock; backend-accessible only; evicted/replaced entries cleansed before reuse. |
| Local wallet: permissions checked, durable temporary file and rename, all KEK versions kept | **verified** | Permissions checked (0700); atomic write via `.tmp` file and rename; old KEK versions retained in PKCS#12 safe bags. |
| Vault: TLS verification, token renewal, bounds on response parsing, no tokens in logs/errors | **verified** | TLS verified by default via libcurl; token renewed by BGW; responses bounds-checked; no secrets in logs (rule `tde-no-secret-in-message`). |
| PKCS#11: never initialised in postmaster, `getpid()` guard after `fork()`, PIN cleansed | **verified** | Module initialised only in backends; session handles guarded against fork reuse; PIN cleansed after login. |
| Rotations (`rotate_online()`, `rotate_kek()`, `wallet_change_passphrase()`): crash-safe, preserves access | **verified** | Online rotation tracks old/new DEKs; catalog updates committed transactionally; verified by TAP tests 29, 30, 46. |
| Secrets in GUCs, SQL arguments and commands | **verified** | Sensitive GUCs marked `GUC_SUPERUSER_ONLY` and `GUC_NOT_IN_SAMPLE`; risk of `wallet_passphrase_command` documented (PSQLE-177). |

### 3. SQL surface - `sql/pg_vault_tde--1.7.sql` and C functions

| Item | Result | Notes |
|---|---|---|
| Function `GRANT`s and `SECURITY DEFINER` fixed `search_path` | **finding (PSQLE-217)** | 12 `SECURITY DEFINER` functions lack fixed `search_path`. Documented mitigation available; scheduled for resolution in v1.8 (Medium severity). |
| Privileged functions check calling role, not owner | **verified** | Caller verified using `GetOuterUserId()` / caller checks (PSQLE-205, PSQLE-206; rule `tde-caller-superuser`). |
| GUC context and flags: `PGC_SUSET`/`PGC_POSTMASTER`, `GUC_SUPERUSER_ONLY`, `GUC_NOT_IN_SAMPLE` | **verified** | Secret/path GUCs properly restricted to superuser and omitted from samples/dumps. |
| Functions taking server file path check caller privileges | **verified** | `pg_vault_tde_seal_keys()` and `pg_vault_tde_unseal_keys()` require calling role to be superuser (PSQLE-206). |
| Extension tables revoked from `PUBLIC` | **verified** | `REVOKE ALL ON TABLE pg_vault_tde_catalog, pg_vault_tde_rotation_progress FROM PUBLIC;` enforced in extension SQL. |

### 4. Hooks and core integration - `src/pg_vault_tde.c`, `src/tam/`, `src/iam/`, `src/logical/`

| Item | Result | Notes |
|---|---|---|
| TAM read paths decrypt, write paths encrypt; table rewrite callbacks apply heapam to decrypted copies | **verified** | TAM slots and tuple read/write paths correctly encrypt/decrypt; CLUSTER, VACUUM FULL and index build operate on decrypted data. |
| No callback leaves relcache entry pointing at heapam (`rd_tableam`) across invalidations | **verified** | Temporary `rd_tableam` swaps restored reliably; Semgrep rule `tde-rd-tableam` verified with 3 justified suppressions (PSQLE-213). |
| `ProcessUtility` and object-access hook check DROP, ALTER ACCESS METHOD, index creation | **verified** | DDL interception prevents bypass or invalid access method transitions on encrypted relations. |
| Planner hook changes plans only for `tde_btree` indexes | **verified** | Hook operates strictly on relations indexed with `tde_btree`. |
| Custom WAL resource manager: redo matches heapam, decoding decrypts before emission | **verified** | Redo handlers preserve tuple layout; `pg_vault_tde_pgoutput` decrypts for logical subscribers. |
| Background workers: execution role and signal handling (`SIGTERM`, cancel, crash) | **verified** | BGW processes run in target DB context; clean exit and state cleanup on signals. |

### 5. Plaintext on disk

| Item | Result | Notes |
|---|---|---|
| Heap, TOAST and WAL payloads written only through encryption path | **verified** | Tuple and TOAST bodies encrypted with AES-256-GCM; ciphertext logged in WAL. |
| Temporary files, `pg_statistic`, index keys, server log: encrypted or documented in Accepted Risks | **verified** | Limitations regarding catalog statistics, held cursor temporary files, and native index keys documented in README and SECURITY-REVIEW.md. |
| Core dumps and swap: cleansed material vs resident shared memory cache | **verified** | Transient buffers cleansed; resident shmem DEK cache documented as accepted threat model boundary. |

### 6. Client tools - `src/backup/`

| Item | Result | Notes |
|---|---|---|
| Tools secure session (`search_path`) and qualify calls | **verified** | `pg_basebackup_tde` sets empty `search_path` and qualifies schema calls (PSQLE-178; rule `tde-client-qualified-call`). |
| Archives and seal bundles treated as untrusted input (framing, lengths, HMAC) | **verified** | Magic, lengths, and HMAC verified before payload parsing on restore. |
| Client-side file permissions and key isolation | **verified** | Files written with 0600 permissions. |
| Passphrases and keys cleansed after use | **verified** | Client memory cleansed with `OPENSSL_cleanse()` before process exit. |

### 7. Error paths

| Item | Result | Notes |
|---|---|---|
| No error message, detail or hint carries keys, IVs or plaintext values | **verified** | Audited; rule `tde-no-secret-in-message` passed with 0 findings. |
| `PG_TRY`/`PG_CATCH` blocks cleanse allocations; modified variables marked `volatile` | **verified** | `volatile` qualifiers applied; cleanup performed in `PG_FINALLY` or `PG_CATCH`. |
| Critical failure handling does not rely on `PG_CATCH` bypassed by `FATAL` | **verified** | Critical state consistency maintained independently of backend crash exit. |
| Error-path test suite and sanitizers | **verified** | Error-path suite (13/13 passed); ASan, UBSan, Valgrind, and assertion build clean in CI evidence. |

---

## Findings Status

| Ticket | Severity | Status in 1.7.2 | Description |
|---|---|---|---|
| PSQLE-177 | Low | Accepted / Documented | (1) AAD scope bounds relation and key generation; (2) Secret exposure in GUCs / commands documented. |
| PSQLE-178 | Medium | **Fixed** | Client tool empty `search_path`, IV batch PID ownership, DEK encryption limit documented. |
| PSQLE-180 | Medium | **Fixed** | Pinned CI actions and container digests, signed releases and source SBOM scans. |
| PSQLE-205 | Medium | **Fixed** | `pg_vault_tde_reencrypt_table()` enforces `MAINTAIN` on calling role. |
| PSQLE-206 | High | **Fixed** | Key management functions check caller role (`GetOuterUserId()`), server file paths restricted. |
| PSQLE-217 | Medium | Open (Target: 1.8) | `SECURITY DEFINER` functions search_path hardening; workaround documented. |
| PSQLE-218 | Low | Open (Target: 1.8) | Tuple v5 layout authentication; requires new on-disk format in 1.8. |

---

## Conclusion

pg_vault_tde release **1.7.2** satisfies the security properties and verification criteria defined
in [doc/SECURITY-REVIEW.md](../../SECURITY-REVIEW.md). All identified High-severity issues are resolved,
all automated evidence checks pass with zero defects, and remaining open findings are documented
with known mitigations. Release 1.7.2 is **approved**.