diff --git a/docs/OPENBSD-REFERENCE.md b/docs/OPENBSD-REFERENCE.md new file mode 100644 index 0000000..6c55e2e --- /dev/null +++ b/docs/OPENBSD-REFERENCE.md @@ -0,0 +1,91 @@ +# OpenBSD Security Reference for shadow-rs + +Reference notes from OpenBSD's passwd implementation (ISC license). +These are design patterns and hardening techniques to adopt. + +## Key OpenBSD Security Patterns + +### 1. pledge(2) — Syscall Restriction + +OpenBSD's passwd calls `pledge("stdio rpath wpath cpath flock proc exec getpw id tty", NULL)` +immediately after startup, restricting the process to only the syscalls it needs. + +**Linux equivalent**: `seccomp-bpf` or `landlock`. We should investigate adding +a seccomp filter after initialization to restrict syscalls. + +**Status**: Not implemented. Future work. + +### 2. unveil(2) — Filesystem Restriction + +OpenBSD restricts file access to only: +- `/etc/` (read/write for shadow files) +- `/dev/tty` (read/write for password prompts) + +**Linux equivalent**: `landlock` (kernel 5.13+). Could restrict filesystem +access to only `/etc/passwd`, `/etc/shadow`, `/dev/tty`. + +**Status**: Not implemented. Future work. + +### 3. Privilege Separation + +OpenBSD drops privileges as early as possible. The passwd binary: +1. Reads files as root +2. Drops to the target user's UID for PAM interaction +3. Re-elevates only for the final file write + +**Our approach**: We use `caller_is_root()` (getuid) for authorization but +run the entire operation with full privileges. Could improve by dropping +euid to caller's uid during PAM conversation. + +### 4. Signal Handling + +OpenBSD blocks `SIGINT`, `SIGQUIT`, `SIGHUP`, `SIGTSTP` during critical +sections (file writes) to prevent partial updates, then restores them. + +**Our approach**: We rely on RAII (lock drop, echo guard drop) but don't +block signals during the file write itself. A signal between the rename +and the lock release is harmless, but a signal during the write closure +could leave a partial temp file (mitigated by TmpGuard). + +### 5. Memory Zeroing + +OpenBSD uses `explicit_bzero()` on all password buffers — this cannot be +optimized away by the compiler (unlike `memset`). + +**Our approach**: We use the `zeroize` crate which uses volatile writes +to prevent compiler optimization. Equivalent security. + +### 6. File Locking + +OpenBSD uses `flock(2)` (advisory locks) instead of `.lock` files. +The `.lock` file approach (used by GNU shadow-utils and us) has the +TOCTOU race we mitigated with hard-link pattern. + +`flock(2)` is cleaner but: +- Not compatible with GNU shadow-utils convention +- Doesn't work across NFS (neither do .lock files) + +**Our approach**: Hard-link pattern is correct for GNU compatibility. + +### 7. Atomic File Replacement + +OpenBSD's `pw_mkdb` creates the file with restrictive permissions from +the start (like our fix in #19), fsyncs, then renames. + +**Our approach**: Same pattern. Already implemented correctly. + +## Recommendations for shadow-rs + +| Priority | What | OpenBSD Pattern | Effort | +|----------|------|-----------------|--------| +| High | Drop privileges during PAM conversation | `seteuid(caller_uid)` | Medium | +| High | Block signals during file write | `sigprocmask` | Low | +| Medium | Add landlock filesystem restriction (Linux 5.13+) | Like `unveil` | Medium | +| Medium | Add seccomp filter after init | Like `pledge` | High | +| Low | Environment sanitization | Clear env except essentials | Low | + +## File References + +- OpenBSD passwd.c: https://cvsweb.openbsd.org/src/usr.bin/passwd/ +- OpenBSD pw_dup.c: https://cvsweb.openbsd.org/src/lib/libc/gen/pw_dup.c +- sudo-rs privilege handling: https://github.com/trifectatechfoundation/sudo-rs diff --git a/docs/SECURITY-HARDENING.md b/docs/SECURITY-HARDENING.md new file mode 100644 index 0000000..be85bab --- /dev/null +++ b/docs/SECURITY-HARDENING.md @@ -0,0 +1,99 @@ +# Security Hardening Roadmap + +Techniques to adopt from OpenBSD and best practices for setuid-root tools. + +## Already Implemented + +- [x] `caller_is_root()` uses `getuid()` not `geteuid()` for authorization +- [x] Atomic file writes with `fsync` + `rename` +- [x] Temp files created with `0o600` (no world-readable window) +- [x] Lock-via-hard-link (TOCTOU-resistant) +- [x] Stale lock detection only on `ESRCH` (not `EPERM`) +- [x] Password strings zeroed via `zeroize` crate +- [x] Absolute paths for subprocess execution (`/usr/sbin/nscd`) +- [x] PAM delegation (no custom password hashing) +- [x] `TmpGuard` drop pattern (no leaked temp files) + +## Phase 1: Quick Wins + +### Signal Blocking During File Writes +Block `SIGINT`/`SIGTERM`/`SIGHUP` during the critical section between +lock acquisition and lock release. Prevents partial shadow file updates. + +```rust +use nix::sys::signal::{SigSet, SigmaskHow, sigprocmask}; + +let mut oldset = SigSet::empty(); +let mut blockset = SigSet::empty(); +blockset.add(Signal::SIGINT); +blockset.add(Signal::SIGTERM); +blockset.add(Signal::SIGHUP); +sigprocmask(SigmaskHow::SIG_BLOCK, Some(&blockset), Some(&mut oldset))?; + +// ... critical section: lock, write, rename, unlock ... + +sigprocmask(SigmaskHow::SIG_SETMASK, Some(&oldset), None)?; +``` + +### Environment Sanitization +Clear the environment on startup for setuid binaries, keeping only: +- `PATH=/usr/bin:/bin` +- `TERM` +- `LANG`/`LC_*` + +```rust +fn sanitize_env() { + let keep = ["TERM", "LANG", "LC_ALL", "LC_MESSAGES"]; + let saved: Vec<_> = keep.iter() + .filter_map(|k| std::env::var(k).ok().map(|v| (*k, v))) + .collect(); + // Clear everything + for (key, _) in std::env::vars() { + std::env::remove_var(&key); + } + // Restore kept vars + safe PATH + std::env::set_var("PATH", "/usr/bin:/bin:/usr/sbin:/sbin"); + for (key, val) in saved { + std::env::set_var(key, val); + } +} +``` + +### Privilege Drop During PAM Conversation +Drop effective UID to caller's real UID during the PAM conversation, +re-elevate only for file writes: + +```rust +let caller_uid = nix::unistd::getuid(); +nix::unistd::seteuid(caller_uid)?; // drop privs +pam.authenticate(0)?; +pam.chauthtok(0)?; +nix::unistd::seteuid(Uid::from_raw(0))?; // re-elevate for file write +``` + +## Phase 2: Linux-Specific Hardening + +### Landlock (Linux 5.13+) +Restrict filesystem access to only the files we need: + +```rust +// Only allow: /etc/passwd, /etc/shadow, /etc/shadow.lock, /dev/tty +let ruleset = Ruleset::new() + .handle_access(AccessFs::ReadFile | AccessFs::WriteFile)? + .create()?; +ruleset.add_rule(PathBeneath::new(PathFd::new("/etc/")?, AccessFs::all()))?; +ruleset.add_rule(PathBeneath::new(PathFd::new("/dev/tty")?, AccessFs::all()))?; +ruleset.restrict_self()?; +``` + +### Seccomp-BPF +Restrict syscalls to only what passwd needs after initialization. +Complex but effective — sudo-rs uses this approach. + +## References + +- OpenBSD pledge(2): https://man.openbsd.org/pledge.2 +- OpenBSD unveil(2): https://man.openbsd.org/unveil.2 +- Linux landlock: https://docs.kernel.org/userspace-api/landlock.html +- Linux seccomp: https://man7.org/linux/man-pages/man2/seccomp.2.html +- sudo-rs security: https://github.com/trifectatechfoundation/sudo-rs