wolfsshd: open PID file with O_NOFOLLOW and a fixed mode

[yosuke-wolfssl]

wolfsshd: harden PID-file write against symlink, hard-link, and FIFO attacks

Summary

wolfSSHD_ConfigSavePID wrote the PidFile with WFOPEN(..., "wb") — plain
fopen() on POSIX: no O_NOFOLLOW, no explicit mode. The daemon sets
umask(0) during daemonization and calls this as root, before privilege
drop. A local user with write access to the PidFile directory could:

• pre-place a symlink at the PID path → fopen follows it and truncates
  an arbitrary root-owned file, writing the daemon PID into it; and
• under umask(0), have the PID file created mode 0666 (world-writable).

Exploitability is conditional (the default config leaves PidFile unset and
/var/run is root-owned), so impact is limited to non-default PidFile
locations, container/embedded deployments with a non-root-writable PID
directory, or first-install windows.

Addressed by f_5853.

Fix — apps/wolfsshd/configuration.c

The POSIX path now opens without O_TRUNC, validates the descriptor, then
truncates and writes:

fd = open(conf->pidFile, O_WRONLY | O_CREAT | O_NONBLOCK | O_NOFOLLOW, 0644);
/* refuse unless a single-link regular file, then ftruncate + fdopen + write */

Defenses, each mapping to a concrete attack:

Measure	Stops
O_NOFOLLOW (+ lstat fallback where unavailable)	symlink redirect / truncation
explicit 0644 mode	world-writable PID file under umask(0)
fstat → S_ISREG && st_nlink == 1	hard link onto a root-owned file (which O_NOFOLLOW does not cover)
O_NONBLOCK on open	a planted FIFO stalling sshd startup before the S_ISREG check
no O_TRUNC; ftruncate only after validation	clobbering a refused target
WFWRITE/WFCLOSE return checked; unlink on failure	leaving a truncated / zero-length PID file

Other notes:

• Truncation now happens (ftruncate) only after the descriptor is proven a
  single-link regular file, so nothing is clobbered when a target is refused.
• _XOPEN_SOURCE/_GNU_SOURCE are defined before the system headers (as
  auth.c does) so fdopen/ftruncate/lstat/S_ISLNK are declared under
  strict -std= builds.
• The Windows branch keeps the portable WFOPEN path, with the same
  WFWRITE/WFCLOSE checks and WREMOVE-on-failure.

Supporting change — wolfssh/port.h

The WOLFSSH_O_NOFOLLOW portability macro (and its <fcntl.h> include) was
gated behind (WOLFSSH_SFTP || WOLFSSH_SCP), so it was undefined in an
--enable-sshd-only build. It is moved into a WOLFSSH_HAVE_SYMLINK block with
a header-wide 0 catch-all, so SFTP, SCP, and SSHD share one definition.

Tests — apps/wolfsshd/test/test_configuration.c

New test_ConfigSavePID (POSIX) drives the real config loader +
wolfSSHD_ConfigSavePID:

• normal PidFile written with the daemon PID, not group/world writable
  even under umask(0);
• a symlinked PID path is refused, link target intact;
• a hard-linked PID path is refused, link target intact;
• a FIFO at the PID path is refused (fast-fail) and left in place.

The symlink-refusal assertion is gated on WOLFSSH_HAVE_SYMLINK so the test is
also correct under a WOLFSSH_NO_SYMLINK_CHECK build.

Scope note

This intentionally does not alter the daemon-wide umask. The PID file is
protected by its explicit 0644 mode, so changing the process umask (which
would also affect SFTP/SCP client-requested upload modes) was left out to keep
the change scoped to the finding. This matches OpenSSH, which relies on explicit
modes plus a root-owned PidFile directory and does no symlink/hard-link checks
itself; operators should still place PidFile in a root-owned directory.

Verification

• Builds clean (0 compiler warnings) across --enable-all, --enable-scp,
  --enable-sftp, --enable-scp --disable-sftp, --enable-sftp --disable-scp,
  and --enable-sshd (no SFTP/SCP).
• Negative controls: built against the pre-fix source and confirmed the symlink,
  mode, and hard-link assertions all fail without the fix; a standalone
  check confirms O_NONBLOCK is what prevents the FIFO open from blocking.
• codespell clean.

Files

apps/wolfsshd/configuration.c           | O_NOFOLLOW + 0644, st_nlink/S_ISREG, O_NONBLOCK, ftruncate-after-check, WFWRITE/WFCLOSE checks, feature-test macros
apps/wolfsshd/test/test_configuration.c | test_ConfigSavePID (normal / mode / symlink / hard-link / FIFO)
wolfssh/port.h                          | centralize WOLFSSH_O_NOFOLLOW for SSHD

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1074

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1074

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 1 total — 1 posted, 0 skipped

Posted findings

• [High] Existing regular PID files keep unsafe ownership or permissions — apps/wolfsshd/configuration.c:1828-1845

Review generated by Skoll.

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 1 total — 1 posted, 0 skipped

Posted findings

• [High] Existing regular PID files keep unsafe ownership or permissions — apps/wolfsshd/configuration.c:1828-1845

Review generated by Skoll.

[yosuke-wolfssl]

Hello @aidangarske ,
Thanks for reviewing. I fixed the issues.
Could you please review this again ?

[aidangarske]

@ejohnstown can you look at this one please