Skip to content

feat(backup): encrypt archives at rest (+ fix latent phpbu env-adapter bug) - #9

Merged
CybotTM merged 2 commits into
mainfrom
fix/backup-at-rest-encryption
Jun 28, 2026
Merged

feat(backup): encrypt archives at rest (+ fix latent phpbu env-adapter bug)#9
CybotTM merged 2 commits into
mainfrom
fix/backup-at-rest-encryption

Conversation

@CybotTM

@CybotTM CybotTM commented Jun 28, 2026

Copy link
Copy Markdown
Member

Implements the one substantive deferred review finding ([7]) — now unblocked since phpbu-docker was un-bricked (phpbu#162).

Problem

phpbu wrote the DB dump, files, and the config archive — which contains glpicrypt.key (the AES key for every encrypted DB field) — unencrypted to the backups volume, colocating the key with the ciphertext it decrypts. Anyone with the volume had both.

Verifying the fix also surfaced two latent bugs that meant the backup never actually worked (it had never run — the image was broken and the backup is opt-in):

  • backup.json used adapter:env:* with **no adapters block registered→ phpbu errorsno adapter registered with name: env` (would have broken even the DB-password lookup).
  • the config archive failed with tar: oauth.pem: Permission denied — phpbu (uid 1000) can't read GLPI's 0660 oauth keys (owned by www-data).

Fix

  • Encrypt every archive — phpbu openssl crypt (aes-256-cbc + pbkdf2), keyed by BACKUP_CRYPT_PASSWORD; plaintext is deleted post-encryption.
  • Register the env adapter (fixes the latent resolve bug).
  • Run backup as root so cap_add: DAC_OVERRIDE actually applies and phpbu can read the protected config (caps are inert for a non-root uid). Opt-in + socket-less + :ro sources bound the risk.
  • make init generates a random BACKUP_CRYPT_PASSWORD; .env.example documents it with a store-it-separately warning.
  • Restore runbook updated — every step decrypts first; file table shows .enc.

Verified end-to-end

OK (3 backups, 3 crypts), zero plaintext on the volume, and the exact documented restore decrypts to valid SQL (DROP TABLE IF EXISTS glpi_agents…) and recovers glpicrypt.key + oauth keys from the config archive. Default stack unaffected; hardening-check + yamllint + shellcheck + JSON-validate all clean.

Addresses the review finding that phpbu wrote the DB dump, files, and the config
archive — which holds glpicrypt.key (the AES key for every encrypted DB field) —
UNENCRYPTED to the backups volume, colocating the key with the ciphertext it
decrypts.

- backup.json: openssl crypt step (aes-256-cbc + pbkdf2) on all three backups,
  keyed by BACKUP_CRYPT_PASSWORD; the plaintext archive is deleted after
  encryption so the volume never holds cleartext. Also register the `env`
  adapter — it was missing, so every `adapter:env:*` reference (incl. the DB
  password) would have failed to resolve once the backup actually ran.
- compose.yml: forward BACKUP_CRYPT_PASSWORD to the backup container; run it as
  root so cap_add DAC_OVERRIDE actually lets phpbu read GLPI's 0660 oauth keys
  (the config backup failed on them otherwise — caps are inert for a non-root uid).
- init.sh generates a random BACKUP_CRYPT_PASSWORD; .env.example documents it
  with a loud 'store it separately' warning.
- runbook-restore.md: every restore step now decrypts first; file table shows .enc.

Verified end-to-end: OK (3 backups, 3 crypts), zero plaintext on the volume, and
the documented restore decrypts to valid SQL + recovers glpicrypt.key.

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
github-actions[bot]
github-actions Bot previously approved these changes Jun 28, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces at-rest encryption for GLPI backups using phpbu's openssl adapter with a new BACKUP_CRYPT_PASSWORD environment variable. Feedback focuses on correcting the documented decryption commands by removing the incorrect -a (base64) flag, which would cause decryption to fail. Additionally, it is recommended to optimize the restore runbook by piping decryption output from the existing backup container rather than installing openssl at runtime in a temporary container, ensuring offline compatibility and better security.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread .env.example
Comment thread config/phpbu/backup.json
Comment thread docs/runbook-restore.md
Comment thread docs/runbook-restore.md
Comment thread docs/runbook-restore.md Outdated
Comment thread docs/runbook-restore.md Outdated
Addresses review feedback: instead of `apk add openssl` in a throwaway alpine
container (needs internet, fails air-gapped) with BACKUP_CRYPT_PASSWORD in its
env, decrypt in the existing backup container (already has openssl + the
passphrase) and pipe the plaintext tar into the extraction container. Keeps the
'-a' flag — phpbu's openssl crypter base64-encodes its output (verified via
phpbu --debug + round-trip), so decryption requires it.

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM
CybotTM merged commit f46347e into main Jun 28, 2026
21 checks passed
@CybotTM
CybotTM deleted the fix/backup-at-rest-encryption branch June 28, 2026 21:15
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant