[Home]

Summary:ASTERISK-06013: asterisk does not properly drop root privileges
Reporter:Kristof Köhler (kkoehler)Labels:
Date Opened:2006-01-07 12:18:41.000-0600Date Closed:2011-06-07 14:10:08
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:If told to run as non-root, asterisk does setgid/setuid, but not
initgroups. Consequently, asterisk still runs in group 0, i.e. with
root privileges.

Version: 1.0.10

****** ADDITIONAL INFORMATION ******

Patch:

--- asterisk.c.orig     Mon May 16 05:04:58 2005
+++ asterisk.c  Sat Nov 12 03:18:02 2005
@@ -1724,6 +1724,10 @@
                       ast_log(LOG_WARNING, "No such user '%s'!\n", runuser);
                       exit(1);
               }
+               if (initgroups(pw->pw_name,pw->pw_gid)) {
+                       ast_log(LOG_WARNING, "Unable to init groups for %s\n", r
unuser);
+                       exit(1);
+               }
               if (setuid(pw->pw_uid)) {
                       ast_log(LOG_WARNING, "Unable to setuid to %d (%s)\n", pw
->pw_uid, runuser);
                       exit(1);

I hereby disclaim all copyright interest in this patch.
Comments:By: Kristof Köhler (kkoehler) 2006-01-07 12:25:26.000-0600

This has been fixed in 1.2, but your page states that security fixes are still done for 1.0.
By: Mark Spencer (markster) 2006-01-07 12:42:32.000-0600

I didn't see it in SVN trunk so I went ahead and added it.  I don't know if it will make it back to 1.0 or not, but i'll leave that to the maintainers.
By: Kristof Köhler (kkoehler) 2006-01-07 12:57:36.000-0600

The SVN trunk version does setgroups in case rungroup is set, so that's ok, no problem there.

In SVN trunk this patch would add an initgroups call to the runuser case, however I feel you should not do group-related things there.

Sorry for the confusion.
By: Mark Spencer (markster) 2006-01-07 13:10:47.000-0600

I changed trunk to only run initgroups if the rungroup was not actually set.
By: Kristof Köhler (kkoehler) 2006-01-07 13:52:33.000-0600

Hmm, does that make sense? If i only changed the user, I would not expect changes in the supplementary groups. (Apart from the fact that it does not make *any* sense at all from a security point of view to drop uid-root, but not gid-root ;-)

And actually I would prefer initgroups over setgroups, to use the supplementary groups from /etc/groups. Some distributions organize device permissions with groups (e.g. /dev/dsp has group audio, oder /dev/ttyI* have group dialout), so I could simply add the asterisk user to these groups to enable access to these devices.
By: Mark Spencer (markster) 2006-01-07 13:57:32.000-0600

My feeling would be that in the absense of specifying a group, but when a user was specified, you would want Asterisk to run as though that user had executed it, meaning with all the groups that would be associated with that user.  If you prefer initgroups (as I implemented it) then you could simply setup a user with the user *and* group permissions you want and just use runuser and have *all* those group permissions not just the one that would be permitted by setgroup.
By: Tzafrir Cohen (tzafrir) 2006-01-07 17:40:14.000-0600

Hopefully I'm not totally misunderstanding something here:

Debian already has a similar patch. However initgroups only sets the suppllementary groups. Thus even with this patch, asterisk runs as group 0.

(Currently the Debian init.d script runs Asterisk with group ID asterisk by start-stop-daemon to work-around this).

I should also note that having several groups is sometimes required. Thus the option -G is not used on Debian, and instead the sysadmin should add the group to asterisk's suplementary groups.
By: Kristof Köhler (kkoehler) 2006-01-08 02:01:45.000-0600

I think that's the most logical solution. -U alone sets the groups from the login databases, i.e. call initgroups and setgid(pw->pw_gid). (As tzafrir said, the latter one needs to be added.) -G sets the default group and clears the supplementary groups.

So we are talking about this:

--- asterisk.c.orig     Sun Jan  8 09:39:08 2006
+++ asterisk.c  Sun Jan  8 09:48:06 2006
@@ -2171,7 +2171,11 @@
                       ast_log(LOG_WARNING, "No such user '%s'!\n", runuser);
                       exit(1);
               }
-               if (!ast_strlen_zero(rungroup)) {
+               if (!rungroup) {
+                       if (setgid(pw->pw_gid)) {
+                               ast_log(LOG_WARNING, "Unable to setgid to %d\n", pw->pw_gid);
+                               exit(1);
+                       }
                       if (initgroups(pw->pw_name, pw->pw_gid)) {
                               ast_log(LOG_WARNING, "Unable to init groups for '%s'\n", runuser);
                               exit(1);
--------------------------------------------------
BTW, i removed ast_strlen_zero(rungroup), because rungroup == NULL if not specified...

If we wanted to do The Most Generic Thing Ever (tm) ;-), we could let -G take a comma-separated list of groups. The first one becomes the default group, all others the supplementary groups. Too much time, anyone? ;-)
By: Russell Bryant (russell) 2006-01-10 23:03:42.000-0600

kpfleming has committed these changes to 1.2 ...