Subject: Re: Addition to force open to open only regular files
To: Bill Studenmund , Warner Losh <imp@village.org>
From: Noriyuki Soda <soda@sra.co.jp>
List: tech-kern
Date: 11/29/2000 20:49:13
>>>>> On Tue, 28 Nov 2000 18:56:43 -0800 (PST),
	Bill Studenmund <wrstuden@zembu.com> said:

wrstuden> Why don't we just add a getsid(&uid_t, &gid_t) syscall?

Because that doesn't solve any problem.

For applications, saved-id can be retrieved quite easily by the
following code:
	main()
	{
		uid_t saved_uid = geteuid();

		:
	}

For libraries, it doens't resolve anything, see below.

wrstuden> Sounds a lot easier than re-writing the syscall interfaces....

We don't have to re-writing the syscall interfaces.
Because saved-id feature already provides the way to resolve
the $HOSTALIASES problem, if setre{u,g}id(2) is deprecated.
And because proposals in this thread (including half-open,
open-only-normal-files, fsetuid, open_as) doesn't provide anything
other than the features which saved-id already provides.

(The one who proposed open_as should think that his way of disabling
 seteuid breaks some critical features like "su" command. That is
 completely unacceptable, so the proposal should be avoided.)

>>>>> On Tue, 28 Nov 2000 23:35:44 -0700, Warner Losh <imp@village.org> said:

imp> Bill Studenmund writes:
imp> : One of the security models we impliment (mainly to be compatible with
imp> : other systems), setre[gu]id(), makes recovering this info hard as it
imp> : overwrites the variables we'd want to look at to see what that ID was.
imp> : 
imp> : So why not just make things easy and add a syscall to get it???
imp> : 
imp> : Sounds a lot easier than re-writing the syscall interfaces....

imp> [[ apart from the fact that getsid() is already a process session
imp>    thing, I like this idea.  getsuid() is what I've renamed it to ]]

Unfortunately, your proposal doesn't work.

imp> real uid is the original uid of the process.
imp> effective uid is the uid that is curently active.  For setuid
imp>   programs, the effective uid is set to the owner of the file on the
imp>   exec.
imp> saved uid is the effective uid at the start of the program after any
imp>   uid change has been made to it.

imp> Let us say that we have setuid 0 program run by user 100.  Let us
imp> ignore group things for the moment.

imp> Without any uid change at all, things work:
imp> // suid == 0, ruid == 100, euid == 0
imp> /* library call happens */
imp> 	// suid == 0, ruid == 100, euid == 0
imp> 	euid = geteuid();	// 0
imp> 	if (getsuid() == geteuid())
imp> 		seteuid(getuid());
imp> 	// suid == 0, ruid == 100, euid == 100
imp> 	/* do it */
imp> 	if (geteuid() != euid)
imp> 		seteuid(euid);
imp> 	// suid == 0, ruid == 100, euid == 0
imp> 	return;
imp> // suid == 0, ruid == 100, euid == 0

This can be done without the proposed getsuid() system call.
See below:
// suid == 0, ruid == 100, euid == 0
/* library call happens */
	// suid == 0, ruid == 100, euid == 0
	euid = geteuid();	// 0
	if (getuid() != euid)
		seteuid(getuid());
	// suid == 0, ruid == 100, euid == 100
	/* do it */
	if (geteuid() != euid)
		seteuid(euid);
	// suid == 0, ruid == 100, euid == 0
	return;
// suid == 0, ruid == 100, euid == 0

imp> Privs dropped with setreuid():
imp> // suid == 0, ruid == 100, euid == 0
imp> setreuid(geteuid(), getuid())
imp> // suid == 0, ruid == 0, euid == 100
imp> /* library call happens */
imp> 	// suid == 0, ruid == 0, euid == 100
imp> 	euid = geteuid();
imp> 	if (getsuid() == geteuid())
imp> 		seteuid(getuid());
imp> 	// suid == 0, ruid == 0, euid == 100
imp> 	/* do it */
imp> 	if (geteuid() != euid)
imp> 		seteuid(getuid());
imp> 	// suid == 0, ruid == 0, euid == 100
imp> 	return;
imp> // suid == 0, ruid == 0, euid == 100

The assumption of the above code is wrong.
After setreuid(geteuid(), getuid()), the "suid" doesn't remain 0,
but the "suid" becomes 100.
Please read NetBSD's kern_prot.c to confirm this.

And because the assumption is wrong, the above code doesn't work as
expected.

BTW, unlike mrg mentioned, setreuid() behaviour in NetBSD is different
from the BSD/OS behaviour which was mentioned by Chris Torek.

imp> Privs dropped with seteuid():
imp> // suid == 0, ruid == 100, euid == 0
imp> seteuid(getuid());
imp> // suid == 0, ruid == 100, euid == 100
imp> /* library call happens */
imp> 	// suid == 0, ruid == 100, euid == 100
imp> 	euid = geteuid();	// 100
imp> 	if (getsuid() == geteuid())
imp> 		seteuid(getuid());
imp> 	// suid == 0, ruid == 100, euid == 100
imp> 	/* do it */
imp> 	if (geteuid() != euid)
imp> 		seteuid(getuid());
imp> 	// suid == 0, ruid == 100, euid == 100
imp> 	return;
imp> // suid == 0, ruid == 100, euid == 100

This can be done without the proposed getsuid() system call.
See below:

// suid == 0, ruid == 100, euid == 0
seteuid(getuid());
// suid == 0, ruid == 100, euid == 100
/* library call happens */
	// Suid == 0, ruid == 100, euid == 100
	euid = geteuid();	// 100
	if (getuid() != euid)
		seteuid(getuid());
	// suid == 0, ruid == 100, euid == 100
	/* do it */
	if (geteuid() != euid)
		seteuid(getuid());
	// suid == 0, ruid == 100, euid == 100
	return;
// suid == 0, ruid == 100, euid == 100

imp> So unless I'm missing something, this looks like it is correct in all
imp> cases.

Unfortunetely, you missed the NetBSD's behaviour of setreuid(2).
--
soda