Path: utzoo!utgpu!news-server.csri.toronto.edu!bonnie.concordia.ca!uunet!wuarchive!julius.cs.uiuc.edu!apple!veritas!amdcad!mozart.amd.com!proton!tim
From: tim@proton.amd.com (Tim Olson)
Newsgroups: comp.os.minix
Subject: Re: Minix 1.5.10 Kernel hiccup.
Message-ID: <1991Jan15.162345.23304@mozart.amd.com>
Date: 15 Jan 91 16:23:45 GMT
References: <1983@proxima.UUCP>
Sender: usenet@mozart.amd.com (Usenet News)
Reply-To: tim@amd.com (Tim Olson)
Organization: Advanced Micro Devices, Austin, TX
Lines: 76

In article <1983@proxima.UUCP> lucio@proxima.UUCP (Lucio de Re) writes:
| I hope this has not been addressed before I obtained a comp.os.minix
| feed, if it has, please excuse me, I begged for months for the feed
| and only recently has an alternative source of news arisen.
| 
| The problem? Being somewhat security conscious, I removed group and
| other read permissions from the binaries in /bin and /usr/bin with the
| result that only root can now execute programs. Shame! I took a look
| at the EXEC code in the kernel and reached the following conclusions:
|
| (a) EXEC attempts to establish whether the code is executable by opening
|     the file (O_RONLY) with the caller's UID and GID. Take away
|     read-permissions, and the call fails; hence problem.

This has been noted before, but I don't remember what the
previously-posted fixes were.  However, I have a suggestion that looks
feasible...

| (b) One solution seems to be to change the effective ID at that
|     point, but my feeble attempt at doing so failed, probably just
|     as well, as it looked somewhat insane. One must keep in mind
|     that EXEC needs to _read_ the input file to load it into memory
|     for execution.
|
| (c) I also noticed that permissions are checked in two distinct
|     places, in FS and MM, very similar code being employed. Now, the
|     OPEN being executed by MM (in mm/utility.c) uses the FS
|     permission check (which we would probably prefer to bypass,
|     actually), the shortcut being that the file is conveniently left
|     open for EXEC to load it into memory. I wonder whether ideally
|     one should not create a new OPEN mode (O_XONLY), which (a) would
|     not be available at user level (shudder!) and (b) would include
|     read permission; the offshoot would be that the execute
|     permission would be checked by FS (and only once), the file
|     would remain open and the user would be none the wiser.

I think the problem is that the tell_fs(CHDIR...) call in MM is trying
to do two different things:

	1) temporarily change MM's workdir to the original caller's
	   workdir

	2) change MM's effuid to the original caller's effuid

The second action is really only required to creat() a core file with
the correct ownership, since permissions are already checked in MM.
[By the way, shouldn't the effgid also be changed?  I believe that
core files should have both effuid *and* effgid ownerships]

The fix I propose is to split these actions by slightly modifying the
do_chdir code in FS pertaining to a call by MM:

	 cd_flag == 0:	change MM's workdir to the original caller's
	 		workdir, set effuid to SUPER_USER

	 cd_flag == 1:	change MM's workdir back to MM's rootdir, set
	 		effuid to SUPER_USER

	 cd_flag == 2:  change MM's effuid and effgid to the original
	 		caller's effuid and effgid.

With these changes in FS, the only change to MM needs to be in
"signal.c", where the core file is creat()'ed -- just before the
creat() call, we can add:

	tell_fs(CHDIR, slot, 2, 0);

to set the user and group ownerships correctly.

I haven't given this potential fix much rigorous thought; it just
seemed to be correct to me.  Anyone find any nasty holes in this?

--
	-- Tim Olson
	Advanced Micro Devices
	(tim@amd.com)