Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Vanilla

Reply
 
Thread Tools Display Modes
Old June 15, 2007, 19:13   #1
CJNyfalt
Swordsman
 
Join Date: May 2007
Posts: 289
CJNyfalt is on a distinguished road
The safe_setuid code

The safe_setuid_drop and safe_setuid_grab functions have a ridiculous number of ifdefs.

I suggests that at least SAFE_SETUID and SAFE_SETUID_POSIX is turned on permanently. For the first one, I see no reason to turn it off and for the second one, we assume in all other places that all Unix systems are Posix compatible by now.

Finally there is setegid vs setgid. Reading the manpages I can't figure out the difference. Anyone got a clue?
CJNyfalt is offline   Reply With Quote
Old June 15, 2007, 20:56   #2
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,828
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by CJNyfalt View Post
The safe_setuid_drop and safe_setuid_grab functions have a ridiculous number of ifdefs.

I suggests that at least SAFE_SETUID and SAFE_SETUID_POSIX is turned on permanently. For the first one, I see no reason to turn it off and for the second one, we assume in all other places that all Unix systems are Posix compatible by now.

Finally there is setegid vs setgid. Reading the manpages I can't figure out the difference. Anyone got a clue?
I'll have a look into these; for now, I googled up the following which look useful:

BSD man page for setegid
Linux man page for setegid

The Rationale section of the Open Group's setuid man page seems to explain it, but I haven't got the time right now to read it properly

This slide references a paper (PDF) which explains everything in a much lengthier fashion, which I might print out tonight and read.
takkaria is offline   Reply With Quote
Old June 20, 2007, 15:43   #3
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,828
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by takkaria View Post
This slide references a paper (PDF) which explains everything in a much lengthier fashion, which I might print out tonight and read.
After reading the above paper and coming out a little confused still:

SAFE_SETUID_POSIX means "comply with the POSIX standard", but it's actually less secure than not having SAFE_SETUID_POSIX defined -- setregid has the better semantics over setgid, but it's not POSIX. This is therefore a better (I think) formulation of safe_setuid_grab():

EDIT: simplify code

Code:
void safe_setuid_grab(void)
{
#ifdef SET_UID
# if defined(HAVE_SETREGID)

	if (setregid(getegid(), getgid()) != 0)
		quit("setregid(): cannot set permissions correctly!");

# elif defined(HAVE_SETEGID)

	if (setegid(player_egid) != 0)
		quit("setegid(): cannot set permissions correctly!");

# else

	if (setgid(player_egid) != 0)
		quit("setgid(): cannot set permissions correctly!");

# endif /* HAVE_SETEGID */
#endif /* SET_UID */
}

Last edited by takkaria; June 20, 2007 at 19:04.
takkaria is offline   Reply With Quote
Old June 20, 2007, 19:52   #4
CJNyfalt
Swordsman
 
Join Date: May 2007
Posts: 289
CJNyfalt is on a distinguished road
Quote:
Originally Posted by takkaria View Post

This slide references a paper (PDF) which explains everything in a much lengthier fashion, which I might print out tonight and read.
You have to take a second look. The paper recommends using setresuid, which is not setreuid. See section 4.4.
In order of preference, see the slide and section 8.1.1 of the pdf:
1. setresgid (Which we doesn't have code for. Note the s.)
2. setegid (Recommends setregid for cases that we doesn't use.)
3. setgid

"Since setresuid has a clear semantics and is able to set
each user ID individually, it should always be used if
available. Otherwise, to set only the effective uid, se-
teuid(new euid) should be used; ....

setuid should be avoided ...."
CJNyfalt is offline   Reply With Quote
Old June 20, 2007, 19:55   #5
CJNyfalt
Swordsman
 
Join Date: May 2007
Posts: 289
CJNyfalt is on a distinguished road
The code should look like this, note the changes in the first part in bold:

Code:
void safe_setuid_grab(void)
{
#ifdef SET_UID
# if defined(HAVE_SETRESGID)

	if (setresgid(-1, player_egid, -1) != 0)
		quit("setresgid(): cannot set permissions correctly!");

# elif defined(HAVE_SETEGID)

	if (setegid(player_egid) != 0)
		quit("setegid(): cannot set permissions correctly!");

# else

	if (setgid(player_egid) != 0)
		quit("setgid(): cannot set permissions correctly!");

# endif /* HAVE_SETEGID */
#endif /* SET_UID */
}
CJNyfalt is offline   Reply With Quote
Old June 20, 2007, 23:48   #6
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,828
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by CJNyfalt View Post
The code should look like this, note the changes in the first part in bold:

(snip)
Got it, thanks. As I said, I came out confused.
takkaria is offline   Reply With Quote
Old June 21, 2007, 06:08   #7
Neil Stevens
Scout
 
Neil Stevens's Avatar
 
Join Date: May 2007
Location: California
Posts: 26
Neil Stevens is on a distinguished road
Beware what you do with the setuid stuff if you intend on supporting multiuser Angband on Darwin. Apple's apparently does something odd/buggy/whatever. I'm sorry that I can't tell you exactly what's wrong; we don't use setuid on ToME anymore so the issue became moot.
Neil Stevens is offline   Reply With Quote
Old June 21, 2007, 07:46   #8
CJNyfalt
Swordsman
 
Join Date: May 2007
Posts: 289
CJNyfalt is on a distinguished road
Quote:
Originally Posted by Neil Stevens View Post
Beware what you do with the setuid stuff if you intend on supporting multiuser Angband on Darwin. Apple's apparently does something odd/buggy/whatever. I'm sorry that I can't tell you exactly what's wrong; we don't use setuid on ToME anymore so the issue became moot.
What do you then do with high-scores (and player ghosts if you have them)? Private for each user or none at all?
CJNyfalt is offline   Reply With Quote
Old June 21, 2007, 12:13   #9
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,828
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by Neil Stevens View Post
Beware what you do with the setuid stuff if you intend on supporting multiuser Angband on Darwin. Apple's apparently does something odd/buggy/whatever. I'm sorry that I can't tell you exactly what's wrong; we don't use setuid on ToME anymore so the issue became moot.
I don't intend to support multiuser Angband on Darwin, so it's all good.
takkaria is offline   Reply With Quote
Old June 23, 2007, 05:41   #10
Neil Stevens
Scout
 
Neil Stevens's Avatar
 
Join Date: May 2007
Location: California
Posts: 26
Neil Stevens is on a distinguished road
Quote:
Originally Posted by takkaria View Post
I don't intend to support multiuser Angband on Darwin, so it's all good.
People with X servers will be sad, heh.
Neil Stevens is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT +1. The time now is 19:11.


Powered by vBulletin® Version 3.8.7
Copyright ©2000 - 2017, vBulletin Solutions, Inc.