Summary:	ASTERISK-13398: [patch] allow storage of vmsecret in users voicemail spool directory
Reporter:	klaus3000 (klaus3000)	Labels:
Date Opened:	2009-01-19 09:33:47.000-0600	Date Closed:	2009-10-22 14:14:34
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_voicemail/NewFeature
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_voicemail.c-svn-trunk-r214898.txt ( 1) patch_asterisk_voicemail_secretinspool_1.4.22.txt ( 2) patch_asterisk_voicemail_secretinspool_trunk.txt
Description:	When secretinspool=yes in voicemail.conf, voicemail reads and writes the secret into a file in the users voicemail spool directory. For example if voicemail is configured for user 1234 in voicemail context foobar, then the secret will be stored in /var/spool/asterisk/voicemail/foobar/1234/secret. When secretinspool=no, it uses the old behavior and stores secrets into the configuration files. Note: realtime voiceboxes are never influenced by this parameter ****** ADDITIONAL INFORMATION ****** Background: IMO it is a good practicse to differ between user parameters which are configured by the admin, and user parameters which are configured by the user itself. Thus, they should not be stored in the same place. IMO the users.conf and voicemail.conf are config files for the admin, and should not be changed when the user changes its configuration. As other user specific configuration is already stored in the spool directory (busy message, unavailable message ...) IMO it is a good idea to store the secret there too. Patches are supplied for trunk and for 1.4. 1.4 is tested, trunk compiles fine but is not tested well.
Comments:	By: klaus3000 (klaus3000) 2009-01-19 09:40:42.000-0600 Additional note: If secretinspool=yes and the secret file is not found, the secret specified in the configuration file is used (if present). This allows for example the admin to configure an initial vmsecret in voicemail.conf, which can be changed by the user. Once the secret file exists, it overrides the vmsecret in the configuration file. By: John Todd (jtodd) 2009-01-19 14:38:00.000-0600 I like it.  Is there any risk here without locking?  Imagine if the VM directory is shared across many servers.  Perhaps it's too fast to worry about locking. By: klaus3000 (klaus3000) 2009-01-19 15:07:15.000-0600 The only problem I can see is if a user calls into voicemail two times concurrently and changes the secret in both calls at the same time. But even here I think the race is handled by the OS - one of them is always faster and gets overwritten by the slower one, or in worst case the second fopen/fprintf causes an error in which case the secret is not written to file. By: James Golovich (jamesgolovich) 2009-01-19 15:23:04.000-0600 I like the idea of this.  I've reviewed the code and tested it out and it works as expected By: klaus3000 (klaus3000) 2009-02-16 08:19:43.000-0600 Any reasons why this is not accepted yet? By: Leif Madsen (lmadsen) 2009-05-20 09:00:54 Marking this as Ready for Testing so we can get some extra testing from one or two more people. After that then we can mark this as Ready for Review. By: Tilghman Lesher (tilghman) 2009-08-26 13:06:48 A couple of suggestions: 1) It would be better if this option were specified as something other than a boolean, because I can see the extension of storing the secret elsewhere, possibly in astdb or another database, independently of storing other voicemail configuration data.  passwordlocation=spooldir might be one way of doing this. 2) I'm uncomfortable with having yet another file format for user options.  I'd suggest that you create an ast_config structure and use the ast_config_load and ast_config_save functions, respectively, for loading and saving the password. By: klaus3000 (klaus3000) 2009-08-27 04:49:42 reg 1) good idea reg 2) I will try. But there is no ast_config_save - do you mean ast_config_text_file_save ? By: Tilghman Lesher (tilghman) 2009-08-27 08:01:27 Yes, ast_config_text_file_save, just as is done with the message information file (for each voicemail message). By: klaus3000 (klaus3000) 2009-09-01 09:09:17 1+2 is implemented in the new patch app_voicemail.c-svn-trunk-r214898.txt By: Digium Subversion (svnbot) 2009-10-22 14:14:32 Repository: asterisk Revision: 225406 U   trunk/CHANGES U   trunk/apps/app_voicemail.c U   trunk/configs/voicemail.conf.sample ------------------------------------------------------------------------ r225406 | tilghman | 2009-10-22 14:14:31 -0500 (Thu, 22 Oct 2009) | 7 lines Permit storage of voicemail secrets in a separate file, located within the spool directory. (closes issue ASTERISK-13398) Reported by: klaus3000 Patches:       app_voicemail.c-svn-trunk-r214898.txt uploaded by klaus3000 (license 65) Tested by: jamesgolovich ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=225406