[ postfixadmin-Patches-2607332 ] add dovecotpw encrypt support option for dovecot users
Brought to you by:
christian_boltz,
gingerdog
From: SourceForge.net <no...@so...> - 2009-03-13 23:36:21
|
Patches item #2607332, was opened at 2009-02-17 00:43 Message generated for change (Comment added) made by christian_boltz You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=937966&aid=2607332&group_id=191583 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Core Group: SVN (please specify revision!) Status: Open Resolution: None Priority: 5 Private: No Submitted By: cmuelle8 (trendypack) Assigned to: Nobody/Anonymous (nobody) Summary: add dovecotpw encrypt support option for dovecot users Initial Comment: Hi, please apply the following patch to functions.inc.php. Background: I don't want to store the pws plain. Auth mechanisms supported in dovecot: plain login cram-md5 digest-md5 (crypt-md5 is not supported as an auth mechanism, look at http://wiki.dovecot.org/Authentication/Mechanisms and ). I'm aware that using PLAIN or LOGIN over SSL is a viable option (in this case dovecot does PLAIN to MD5-CRYPT and compares). However, in a non-ssl scenario PLAIN and LOGIN are a bad option and disabled by default in dovecot. Using CRAM-MD5 or DIGEST-MD5 is possible, but then the passwords have to be in CRAM-MD5 format as well (since dovecot can't do CRAM-MD5 to MD5-CRYPT, obviously). The patch below makes this an option for dovecot users. A hint in config.inc.php will probably also be needed (along the comment lines for the other authentication methods). Greetings, cmuelle8 --- functions.inc.php.orig 2009-02-17 00:06:37.000000000 +0100 +++ functions.inc.php.cram-md5 2009-02-17 00:00:23.000000000 +0100 @@ -1126,6 +1126,11 @@ $password = md5($pw); } + if ($CONF['encrypt'] == 'cram-md5') { + $password = shell_exec("dovecotpw -s CRAM-MD5 -p $pw"); + $password = trim(str_replace('{CRAM-MD5}', '', $password)); + } + if ($CONF['encrypt'] == 'system') { if (ereg ("\$1\$", $pw_db)) { $split_salt = preg_split ('/\$/', $pw_db); ---------------------------------------------------------------------- >Comment By: Christian Boltz (christian_boltz) Date: 2009-03-14 00:35 Message: I just commited a modified version of your patch to SVN (r580), with most issues I listed solved. Changes compared to your patch: - fixed b) (error checking) by checking the encryption result against "^{$method}" - fixed c) by checking for ^dovecot: - Note: If someone sets $CONF['encrypt'] = "dovecot" instead of "dovecot:xy", it will result in the message "unknown/invalid $CONF[encrypt] setting", not "invalid dovecot encryption method" - fixed d) by checking that $method only contains [A-Z0-9-] - having a list of methods supported by dovecot would be better, but I don't have such a list ;-) - fixed e) - see $CONF['dovecotpw'] - $method is converted to uppercase since dovecotpw always returns the method uppercase - initializie $prefix for tempfile (was undefined) The only thing I did not (yet) change is a) (tempfile usage) for which I added a TODO comment. (@GingerDog: This part is not blocking the release.) One question came up while working on this: Why do you remove the {$method} part from the encrypted password? IMHO keeping it would be an advantage, because it would allow switching the encryption method without the need to change/reencrypt existing passwords. ---------------------------------------------------------------------- Comment By: GingerDog (gingerdog) Date: 2009-03-08 20:50 Message: Hi, Any chance of an updated patch then? ---------------------------------------------------------------------- Comment By: Christian Boltz (christian_boltz) Date: 2009-02-25 23:17 Message: Unfortunately "works for me" doesn't mean it is good - at least from my POV. But maybe I'm too strict ;-) The patch looks much better than the previous one, but I still have some things that should be improved: a) tempfile for the password Basically a good idea, but please switch to the tmpfile() PHP function - in comparison to tempnam() it has some advantages: - no need to specify the path - might be relevant on systems with open_basedir, where /tmp is outside the allowed path. (In this case $TMPDIR has to be set as environment variable, but this is easier to do (via apache config) than a hardcoded path.) - the tempfile is automatically deleted After re-reading the code and some PHP documentation, I think that proc_open (allows two-way communication with external processes) might be the best solution. You can even catch STDERR on a separate pipe to check for error messages. See http://www.php.net/manual/en/function.proc-open.php for description and an example. (With proc_open, no tempfile will be needed.) b) error checking The only error checking you do is not to use the pipe to dovecotpw if it can't be opened. If opening the pipe works, you blindly read the encrypted password from the temp file (or whatever the file contains ;-) Please check that the output looks like an encrypted password - shouldn't be too hard since it has to contain "{$method}" at the beginning. Untested: if ( !preg_match("/^{$method}/", $password) { die("can't encrypt password with dovecotpw") } I don't really like the die() method, but the pacrypt function doesn't offer a better way to error out currently :-( Additionally the calling functions expect pacrypt to "always work" - which was not a real problem up to now because it used only PHP-internal functions. c) if (strstr($CONF['encrypt'], 'dovecot:')) -> Please ensure that the $CONF parameter _begins_ with "dovecot:" - for example by using preg_match("/^dovecot:/", $CONF['encrypt']) Besides that, I like the idea of using "method:detail" - @GingerDog: This might also be an option for authlib / $CONF['authlib_default_flavour']. d) check/validate what becomes $method Even if $method comes from $CONF['encrypt'] , it may still contain an invalid value (like "dovecot:foobar") or evil characters ("dovecot:md5';rm -rf /"). There are several ways how this can be secured. I'll start with the best one. - check against an array of allowed password encryption methods. This is the most secure method, but has the disadvantage that it needs a code modification in case dovecot starts to support a new encryption method - check $method against a regular expression. preg_match("/[a-zA-Z0-9-]+/", $method) should do if I get the dovecot documentation right. - at least use escapeshellarg($method) if you really don't want to validate $method e) path to dovecotpw I'm quite sure "dovecotpw must be in $PATH" will cause some problems - for example the openSUSE package has dovecotpw in /usr/sbin - and therefore not in the webserver's $PATH. The only solution for this is a $CONF['dovecotpw'] = "/path/to/dovecotpw" parameter. (If this is not set or empty, you can still default to just "dovecotpw".) Maybe it can be included in $CONF['encrypt'] - something like "dovecot:MD5:/usr/sbin/dovecotpw" would work. However, things will become complicated when we add another 5 parameters to this string *g* BTW: Feel free to include the needed config.inc.php changes in your patch. The comment should mention the most common CRYPT-METHODs for dovecot. ---------------------------------------------------------------------- Comment By: cmuelle8 (trendypack) Date: 2009-02-25 17:07 Message: Absolutely, it works for me. Please make sure you add # dovecot:CRYPT-METHOD => use dovecotpw -s 'CRYPT-METHOD' (needs to be in PATH) or something more descriptive to config.inc.php though - the patch only modifies the functions file. Greetings, Christian ---------------------------------------------------------------------- Comment By: GingerDog (gingerdog) Date: 2009-02-24 21:33 Message: Christian - are you happy with this now? Can it be merged? ---------------------------------------------------------------------- Comment By: cmuelle8 (trendypack) Date: 2009-02-18 09:24 Message: File Added: postfixadmin.functions.patch ---------------------------------------------------------------------- Comment By: cmuelle8 (trendypack) Date: 2009-02-18 07:58 Message: alright, here's a revised patch.. hopefully this will do - as this is not the default in config.inc.php anyway, a note along the other encrypt methods will probably do: # dovecot:CRYPT-METHOD => use dovecotpw -s 'CRYPT-METHOD' (needs to be in PATH) best wishes, cmuelle8 (Christian Müller) File Added: postfixadmin.functions.patch ---------------------------------------------------------------------- Comment By: Christian Boltz (christian_boltz) Date: 2009-02-17 23:54 Message: Thanks for your patch! However, I see some problems with it: a) the password should be quoted/escaped/whatever before using it in the command line. You'll understand what I mean when somebody uses "topsecret ; rm -rf /" as his password *g* Hint: use escapeshellarg($pw) b) I don't really like that an external program is called with the password as parameter (which will be visilble for a short time in the process list). c) no error checking (what happens if dovecotpw is not installed or not in $PATH?) As you can see: things aren't that easy once you start calling external programs ;-) Unfortunately dovecotpw isn't easy to "emulate" in PHP, so calling dovecotpw might be the easiest option, even with the described disadvantages. For some information about "emulating" dovecotpw, see http://markmail.org/message/ug4mfvulgsajg3wk#query:dovecotpw%20php+page:1+mid:snbusk5sempgouyb+state:results http://www.scconsult.com/bill/crampass.pl http://www.dovecot.org/list/dovecot/2007-March/020889.html ---------------------------------------------------------------------- Comment By: cmuelle8 (trendypack) Date: 2009-02-17 02:02 Message: File Added: postfixadmin.functions.patch ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=937966&aid=2607332&group_id=191583 |