Summary:	ASTERISK-02800: [patch] allow setting permissions so vmail.cgi can read vmailbox
Reporter:	fineberg (fineberg)	Labels:
Date Opened:	2004-11-12 20:40:20.000-0600	Date Closed:	2011-06-07 14:05:30
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_voicemail
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) vm.patch
Description:	This patch sets the owner/group and permissions on created directories and voicemail messages.  This allows you to run apache as a user/group other than root as well as NOT have vmail.cgi be setuid.  The uname and gname as well as the permissions are set at the top of the patch.
Comments:	By: khb (khb) 2004-11-12 22:28:23.000-0600 I still think this is incomplete. Changing file ownership and modes requires root priviledges, but there is no provision for setting euid to root during these operations, so this forces one to run Asterisk as root. Also, if you start playing with permissions, the username and group at a minimum,  but the file modes probably as well, must be configurable in the configuration file and not static. Changing filemodes, I believe, also requires changes to file.c:ast_filehelper(). I would like to see this done by proper use of group memberships. placing the Asterisk user and http server (or other application, like imapd) user in a common group and setting group file permissions appropriately.  Done half of this once for somebody with a special case (setting and configuring the modes). Would be nice to do it properly finally. edited on: 11-12-04 22:35 edited on: 11-12-04 22:47 By: fineberg (fineberg) 2004-11-13 19:41:58.000-0600 Thanks for the comments.  I've updated the patch and will submit a new one after testing. By: fineberg (fineberg) 2004-11-16 10:48:42.000-0600 Here is a new patch that allows you to set the owner/group & perms from the config file and does a seteuid before/after the operations.  This has not yet been significantly tested as I don't at the moment have a system where I can test this.  I hope to test this week. By: twisted (twisted) 2004-12-01 01:00:41.000-0600 the guid shouldn't be hardcoded to apache - not all systems use apache as the group.  Instead, we need to make these configurable via voicemail.conf or another location.  Perhaps: cgiuser=apache cgigroup=www-data options in the conf file to determine the ownership... By: khb (khb) 2004-12-01 02:04:03.000-0600 CGI or web apps aren't the only programs that may want to access these files, so the the names should be generic. The default user/group should be the user/group asterisk was started as, not other defaults in the code. As for option names, how about labeling them as a group since they target the same issue. mailboxowner= mailboxgroup= mailboxdirmode= mailboxfilemode= this relates their function directly to chown, chgrp, chmod edited on: 12-01-04 02:07 By: twisted (twisted) 2004-12-15 20:43:52.000-0600 Reminder sent to fineberg Have you seen the questions/comments/etc that have been posted/asked of you?  We need a response ASAP to avoid closing this due to lack of interest. By: fineberg (fineberg) 2004-12-15 21:05:49.000-0600 The user and group are already configurable via voicemail.conf in the patch from 11/16/04.  I'll attach a new patch to fixup the option names as well as set the defaults to those that asterisk was started with. By: twisted (twisted) 2004-12-28 10:58:15.000-0600 So can all go except 12-15-04's patch? By: fineberg (fineberg) 2004-12-28 11:39:56.000-0600 If you mean can the first 2 patches be deleted, then certainly yes. By: khb (khb) 2004-12-28 14:41:52.000-0600 You have this code in your patch: + +static void set_owner_and_group_all(const char* dir, int msgnum) +{ + DIR *vmdir = NULL; + struct dirent *vment = NULL; +        char fn[32]; + char pn[1024]; + snprintf(fn, sizeof(fn), "msg%04d", msgnum); + + if (sizeof(dir) + 11 >= sizeof(pn)) { +        ast_log(LOG_WARNING, "directory name too long to set owner and group, skipping\n"); and you are using sizeof(dir)  to get what? sizeof is not a function. sizeof(dir) will always be 4, is this what you intended or did you mean strlen(dir) ? By: fineberg (fineberg) 2004-12-28 18:07:56.000-0600 Definitely meant strlen, not sizeof.  The intent is just to make sure the pathname will not be too long for the reserved space. By: Mark Spencer (markster) 2005-01-09 03:44:06.000-0600 Why not just use setgid folders? By: Mark Spencer (markster) 2005-01-16 02:09:00.000-0600 Hello, one more time, need some feedback...  How does this improve upon using setgid folders (I believe app_voicemail has special support for handling setuid or setgid voicemail folders) By: fineberg (fineberg) 2005-01-16 17:38:28.000-0600 I'm sorry I can't respond as quickly as you'd like but this isn't my first priority in life.  If my responsiveness is insufficient, then just close this bug. I see nothing in app_voicemail.c that does anything to support setgid folders.  I'd be happy to review if you could point it out. By: nick (nick) 2005-02-15 20:49:26.000-0600 setgid directories really shouldn't need any special code... let's test to confirm this works so that we can either close or move ahead. Nick edited on: 02-15-05 20:49 By: fineberg (fineberg) 2005-02-16 03:41:55.000-0600 I'm not sure what you are suggesting to test here. By: nick (nick) 2005-02-16 14:13:33.000-0600 To break it down, my questions are: A) Can we use setgid directories to allow other users (nobody/apache/httpd/etc.) to read it. B) If so, does that functionality abrogate the need for this patch? Nick By: Olle Johansson (oej) 2005-03-17 08:04:04.000-0600 No activity for over a month, closing this. Please re-open if you have an update or are still interested in moving this patch forward. /Housekeeping