review: address 5 Copilot review comments

- Fix function name: sanitize_env() → sanitized_env() / harden_process()
- Fix SIGINT handler: was removed, not implemented
- Fix user enumeration: early permission check, not constant-time PAM
- Fix Rust module paths: shadow-core → shadow_core (crate vs module)
This commit is contained in:
Pierre Warnier
2026-04-03 15:27:58 +02:00
parent 4259dc7cd0
commit 0a33f2186d
3 changed files with 7 additions and 7 deletions
+1 -1
View File
@@ -92,7 +92,7 @@ Utilities must be embeddable. Return `UResult<()>` from `uumain`. The
### `unsafe`
Denied at the workspace level (`unsafe_code = "deny"`). Only two FFI boundary
modules are exempted: `shadow-core::pam` (PAM C library) and `shadow-core::crypt`
modules are exempted: `shadow_core::pam` (PAM C library) and `shadow_core::crypt`
(POSIX crypt(3)). Every `unsafe` block must have a `// SAFETY:` comment.
### `str`, `OsStr` & `Path`
+3 -3
View File
@@ -11,7 +11,7 @@ Source: `cvsweb.openbsd.org/src/usr.bin/passwd/` and `src/lib/libutil/passwd.c`.
|---|---------|--------|
| Signal blocking during file writes | #38`SignalBlocker` RAII |
| Privilege drop during PAM conversation | #39`PrivDrop` RAII |
| Environment sanitization | #40`sanitize_env()` |
| Environment sanitization | #40`sanitized_env()` / `harden_process()` |
| Landlock filesystem restriction | #41`apply_landlock()` in passwd |
| Absolute paths for subprocesses | #20`/usr/sbin/nscd` |
| Password zeroing | #7`zeroize` crate |
@@ -21,8 +21,8 @@ Source: `cvsweb.openbsd.org/src/usr.bin/passwd/` and `src/lib/libutil/passwd.c`.
| Resource limit hardening | #44`raise_file_size_limit()` in hardening.rs |
| Zero-length output guard | #45 — in `atomic_write` |
| setuid(0) consolidation | #47 — before file operations in passwd |
| User enumeration prevention | #49constant-time PAM in passwd |
| SIGINT handler during password input | #48 — "Password unchanged" + terminal restore |
| User enumeration prevention | #49early permission check in passwd |
| Password input interrupt handling | #48 — custom SIGINT handler removed; signal blocking covers critical sections |
| Umask reset | #51`UmaskGuard` RAII in lock/tmp creation |
### Not Yet Implemented
+3 -3
View File
@@ -14,14 +14,14 @@ Techniques adopted from OpenBSD and best practices for setuid-root tools.
- [x] PAM delegation (no custom password hashing)
- [x] `TmpGuard` drop pattern (no leaked temp files)
- [x] Signal blocking during file writes (#38`SignalBlocker` RAII)
- [x] Environment sanitization (#40`sanitize_env()`)
- [x] Environment sanitization (#40`sanitized_env()` / `harden_process()`)
- [x] Privilege drop during PAM conversation (#39`PrivDrop` RAII)
- [x] Core dump suppression (#43`suppress_core_dumps()`)
- [x] Resource limit hardening (#44`raise_file_size_limit()`)
- [x] Zero-length output guard (#45 — in `atomic_write`)
- [x] setuid(0) consolidation (#47 — before file operations)
- [x] SIGINT handler during password input (#48 — "Password unchanged")
- [x] User enumeration prevention (#49constant-time PAM)
- [x] Password input interrupt handling (#48 — custom SIGINT handler removed; signal blocking during file writes covers critical sections)
- [x] User enumeration prevention (#49early permission check for non-root callers)
- [x] O_CLOEXEC on file descriptors (#50)
- [x] Umask reset (#51`UmaskGuard` RAII)
- [x] Landlock filesystem restriction (#41`apply_landlock()` in passwd)