Bug 606

> 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