On Sun, 2003-07-13 at 00:58, Matthias Andree wrote:

...

...

That's the problem. The home dir is exported by nfs. Chdirs should be done as the user, not as root.

Right.. Well, I'll just ignore that error then:

What if the home directory is owned by another user rather than the real owner? You won't detect that condition any more with the patch.

Hmh.. The reason why I originally added the chdir() there was because unlink_directory() failed to save current working directory so it could be restored. I think it might just as well chdir() to /tmp, except it's also useful for setting directory where core dumps go.

Anyway, home directory isn't required information from auth process. I think I'll chdir to /tmp if it isn't given.

How about this scheme (pseudo code):

seteuid(uid); ret = chdir(targetdir); seteuid(0); if (ret == -1) { /* handle error */ }

Make sure you don't add ANY branches between the seteuid() calls, no mistakes must happen there.

Why? I don't see how dropped privileges can cause much problems. The error handling I would do is just to write to already opened log file and exit(). seteuid() calls also need handling.. How about this:

if (*home_dir != '\0') {
	full_home_dir = *chroot_dir == '\0' ? home_dir :
		t_strconcat(chroot_dir, "/", home_dir, NULL);
	/* NOTE: if home directory is NFS-mounted, we might not
	   have access to it as root. Change the effective UID
	   temporarily to make it work. */
	if (reply->uid != master_uid && seteuid(reply->uid) < 0)
		i_fatal("seteuid(%s) failed: %m", dec2str(reply->uid));
	ret = chdir(full_home_dir);
	if (reply->uid != master_uid && seteuid(master_uid) < 0)
		i_fatal("seteuid(%s) failed: %m", dec2str(master_uid));
	if (ret < 0) {
		i_fatal("chdir(%s) failed with uid %s: %m",
			full_home_dir, dec2str(reply->uid));
	}
} else {
	/* We still have to change to some directory where we have
	   rx-access. /tmp should exist everywhere. */
	if (chdir("/tmp") < 0)
		i_fatal("chdir(/tmp) failed: %m");
}

On Sun, 2003-07-13 at 04:16, Matthias Andree wrote:

...

Anyway, home directory isn't required information from auth process. I think I'll chdir to /tmp if it isn't given.

I wonder if core dumps (if any) of an authenticator process belong in /tmp -- /tmp is anarchy area and must be treated with care. umask(077) is the minimum to make sure no other user can harvest passwords from the core file.

No, I meant the auth process tells user's uid, gid, home directory and/or mail location to master process. So home directory isn't required information there. If it's not given, we should still chdir() to somewhere else than where master is running, $prefix/var/run/dovecot/ which is 0700 root owned.

...

...

Make sure you don't add ANY branches between the seteuid() calls, no mistakes must happen there.

Why? I don't see how dropped privileges can cause much problems.

seteuid doesn't drop privileges, but temporarily puts them aside, while the real and saved user ID remain zero. seteuid(0) restores root permissions if you had them, and is a standard procedure in escalating privileges after a break-in.

Yes, so why is it worse to add any branches running with temporarily dropped privileges than running with full privileges? Or are you thinking that some geteuid() call then doesn't return 0 and thinks it's not running as root?

I have anyway some checks that processes can't accidentally be started as root:

/* verify that we actually dropped the privileges */
if (uid != 0 || disallow_root) {
	if (setuid(0) == 0)
		i_fatal("We couldn't drop root privileges");
}

if ((gid != 0 && uid != 0) || disallow_root) {
	if (getgid() == 0 || getegid() == 0 || setgid(0) == 0)
		i_fatal("We couldn't drop root group privileges");
}

The part above looks OK provided that i_fatal is simple.

Currently it is, but I'm can't be sure if someone some day adds more complex logging functions.

...

} else { /* We still have to change to some directory where we have rx-access. /tmp should exist everywhere. */ if (chdir("/tmp") < 0) i_fatal("chdir(/tmp) failed: %m"); }

I don't like /tmp, see above. I'd feel more comfortable with a directory that only dovecot has access to, rather than /tmp.

Well, it has to be a directory where all users have at least +rx access. +w isn't really needed. I guess I could create /var/run/dovecot/home or something where they go, but I don't think it really matters.

Actually core dumps aren't either written by default since kernel thinks it's running setuid-binary. You'd have to set mail_drop_priv_before_exec = yes to allow that.