1.) sshd should warn user when cannot create pid file. 2.) sshd -t should warn user when dont have write permition to pid_file_dir. One possible reason is missing directory.
> 1.) sshd should warn user when cannot create pid file. This is difficult - see the comment in sshd.c where we write the pidfile > 2.) sshd -t should warn user when dont have write permition to > pid_file_dir. One possible reason is missing directory. IMO that is just bloat.
Created attachment 338 [details] Check directory and file perms for PidFile Also reports error if there's an error writing the PidFile (this could still happen, either due to a race or things like ENOSPC). This part based on Roumen Petrov's patch. Patch against OpenBSD -current.
OK - (1) isn't difficult unless one wants the error written to stderr (the pidfile writing happens after the fork). I'd say that the patch confirms that (2) is bloat
Don't know.. Other projects tend to provide notifications on testing configurations (apache does), but Darren's patch is wrong. + /* Determine directory path to PidFile */ + if (strrchr(options.pid_file, '/') != NULL) { + path = xstrdup(options.pid_file); + *(strrchr(path, '/')+1) = '\0'; + } else + path = xstrdup("."); Fails when the admin is insane and does: "/my/loc/my\/myfile.pid" Since it will not decide if the / has been escaped or not. I don't have a strong feeling either way (For the record, I don't use the pid file for tracking the sshd).
Might is better configure to warn for missing piddir and sshd only when cannot create pidfile.
Created attachment 339 [details] Use realpath to test existence of PidFile directory One person's "bloat" is another's "valuable diagnostic" :-) If you cut down the things that are checked for and can live with less descriptive errors you can just use realpath(). It still performs a useful subset of tests (and handles Ben's evilness too) with less code. # sshd/sshd -t -o 'PidFile /no/such' Invalid PidFile specification: /no/such (No such file or directory) # sshd/sshd -t -o 'PidFile /etc/passwd/pidfile' Invalid PidFile specification: /etc/passwd/pidfile (Not a directory) # sshd/sshd -t -o 'PidFile /tmp\/myfile.pid' Invalid PidFile specification: /tmp\\/myfile.pid (No such file or directory)
I have committed a similar patch which reports the error() on failure to create the pidfile, but not the "sshd -t" tests. There are just too many ways for an admin to break a system to test for them all. Besides, pidfile creation failure is non-fatal.
Mass change of RESOLVED bugs to CLOSED