Harden temp file creation for cert writes #42

Manually merged

heiko merged 4 commits from fix-35-oexcl into master

2026-05-24 19:59:29 +02:00

heiko commented

2026-05-24 18:51:29 +02:00

Owner

Hardens cert-proxy-client temp file handling against symlink injection, predictability-based DoS, and umask exposure.

Fixes #35 (symlink-following and concurrent-run collision).

Core Fix

Use O_EXCL instead of O_TRUNC when creating infixed temp files
Prevents symlink-following attacks and O_TRUNC data corruption
Atomic rejection of pre-existing files (explicit test coverage)

Security Hardening (from multi-reviewer security audit)

Umask Protection

Actively hardens process umask to 0077 at startup (no group/other read)
Integrated with systemd-aware verbose logging for operational visibility
Defense-in-depth even with explicit 0600 permission bits

NFS Incompatibility

Documented that O_EXCL is unreliable on NFS (causes race conditions)
Clarified cert-proxy-client is not suitable for NFS-backed storage
Recommends local filesystems (ext4, btrfs, ZFS)

Error Recovery

Best-effort cleanup of orphaned infixed temp files on write failure
Prevents accumulation of partial/incomplete writes
Added test coverage verifying cleanup occurs on error

Deployment Documentation

Updated README with security & hardening section
Includes umask explanation, NFS caveat, and manual cleanup procedures
Covers Windows behavior (.pfx symlink disabled)

Test Coverage Improvements

TestWriteFile_ExistingFile — O_EXCL rejects pre-existing regular files
TestWriteFile_ExistingSymlink — O_EXCL rejects symlinks with explicit readlink assertion
TestExecute_CleanupOnWriteFileError — Orphaned files cleaned on write failure
TestExecute_RenameExistingFile — os.Rename correctly replaces existing files with correct perms

Quality Assurance

✅ All tests pass (37+ test cases)
✅ Linting clean (golangci-lint, gofmt)
✅ Security audit passed (symlink injection fixed, no new vulnerabilities)
✅ Code quality approved (comprehensive tests, idiomatic error handling)
✅ Architecture sound (backward compatible intentional security changes, extensible)
✅ Documentation complete (README section, code comments, deployment guidance)

Known Residual Risks (separate follow-up issues)

Predictable infix (second granularity): local attacker with certbase write access can pre-plant files to DoS rotation
Predictable .tmp in replaceSymlink: similar race window (requires timing attack)
Optional future: migrate to os.CreateTemp() for cryptographic unpredictability

Commits

3fe5bba: Core O_EXCL fix + initial regression tests
528f1bf: NFS warning, orphaned cleanup, umask check, enhanced symlink test
e5a58b8: Active umask hardening, deployment docs, cleanup test, Rename behavior test
340f6dc: Fix wsl_v5 whitespace violations (linting clean)

Hardens cert-proxy-client temp file handling against symlink injection, predictability-based DoS, and umask exposure. Fixes #35 (symlink-following and concurrent-run collision). ## Core Fix - Use O_EXCL instead of O_TRUNC when creating infixed temp files - Prevents symlink-following attacks and O_TRUNC data corruption - Atomic rejection of pre-existing files (explicit test coverage) ## Security Hardening (from multi-reviewer security audit) **Umask Protection** - Actively hardens process umask to 0077 at startup (no group/other read) - Integrated with systemd-aware verbose logging for operational visibility - Defense-in-depth even with explicit 0600 permission bits **NFS Incompatibility** - Documented that O_EXCL is unreliable on NFS (causes race conditions) - Clarified cert-proxy-client is not suitable for NFS-backed storage - Recommends local filesystems (ext4, btrfs, ZFS) **Error Recovery** - Best-effort cleanup of orphaned infixed temp files on write failure - Prevents accumulation of partial/incomplete writes - Added test coverage verifying cleanup occurs on error **Deployment Documentation** - Updated README with security & hardening section - Includes umask explanation, NFS caveat, and manual cleanup procedures - Covers Windows behavior (.pfx symlink disabled) ## Test Coverage Improvements - TestWriteFile_ExistingFile — O_EXCL rejects pre-existing regular files - TestWriteFile_ExistingSymlink — O_EXCL rejects symlinks with explicit readlink assertion - TestExecute_CleanupOnWriteFileError — Orphaned files cleaned on write failure - TestExecute_RenameExistingFile — os.Rename correctly replaces existing files with correct perms ## Quality Assurance ✅ All tests pass (37+ test cases) ✅ Linting clean (golangci-lint, gofmt) ✅ Security audit passed (symlink injection fixed, no new vulnerabilities) ✅ Code quality approved (comprehensive tests, idiomatic error handling) ✅ Architecture sound (backward compatible intentional security changes, extensible) ✅ Documentation complete (README section, code comments, deployment guidance) ## Known Residual Risks (separate follow-up issues) - **Predictable infix** (second granularity): local attacker with certbase write access can pre-plant files to DoS rotation - **Predictable .tmp in replaceSymlink**: similar race window (requires timing attack) - Optional future: migrate to os.CreateTemp() for cryptographic unpredictability ## Commits - 3fe5bba: Core O_EXCL fix + initial regression tests - 528f1bf: NFS warning, orphaned cleanup, umask check, enhanced symlink test - e5a58b8: Active umask hardening, deployment docs, cleanup test, Rename behavior test - 340f6dc: Fix wsl_v5 whitespace violations (linting clean)

heiko added 1 commit

2026-05-24 18:51:29 +02:00

cert: harden temp file creation 3fe5bbab3c

heiko added 1 commit

2026-05-24 19:06:48 +02:00

cert: add hardening — NFS warning, orphaned file cleanup, umask startup check, enhanced symlink test 528f1bf906

heiko added 1 commit

2026-05-24 19:29:03 +02:00

cert: address security review findings — active umask hardening, deployment docs, cleanup test, Rename behavior e5a58b8370

heiko added 1 commit

2026-05-24 19:53:54 +02:00

fix: wsl_v5 whitespace violations (add blank lines before statements) 340f6dc694