#63 add dovecotpw encrypt support option for dovecot users

closed-accepted
nobody
Core (27)
5
2013-12-01
2009-02-16
cmuelle8
No

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);

Discussion

  • cmuelle8

    cmuelle8 - 2009-02-17

    File Added: postfixadmin.functions.patch

     
  • Christian Boltz

    Christian Boltz - 2009-02-17

    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

     
  • cmuelle8

    cmuelle8 - 2009-02-18

    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

     
  • cmuelle8

    cmuelle8 - 2009-02-18
    • summary: add CRAM-MD5 encrypt option for dovecot users --> add dovecotpw encrypt support option for dovecot users
     
  • cmuelle8

    cmuelle8 - 2009-02-18

    revised patch to deal with all issues except the PATH dependency of dovecotpw

     
  • cmuelle8

    cmuelle8 - 2009-02-18

    File Added: postfixadmin.functions.patch

     
  • GingerDog

    GingerDog - 2009-02-24

    Christian - are you happy with this now? Can it be merged?

     
  • cmuelle8

    cmuelle8 - 2009-02-25

    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

     
  • Christian Boltz

    Christian Boltz - 2009-02-25

    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.

     
  • GingerDog

    GingerDog - 2009-03-08

    Hi,

    Any chance of an updated patch then?

     
  • Christian Boltz

    Christian Boltz - 2009-03-13

    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.

     
  • cmuelle8

    cmuelle8 - 2009-03-14

    Not removing {$method} means that every app using this db field must understand this prefix - I removed it, cause I feared that it would get me into trouble setting up dovecot+postfix somewhere in the process.

    After all, what you suggest might work if you only use the dovecot auth stuff (and postfixadmin if we program it that way) to access the field, but I have not even checked, if all of dovecot understands the prefix added by dovecotpw (which you would think, but I'm just not sure).

    Personally I think it's a source of error, permiting multiple methods in that field + it restricts you to software that understands this prefix.

    Greetings and thx for fixing it up to your own suggestions..

    cmuelle8

     
  • GingerDog

    GingerDog - 2009-03-20
    • status: open --> closed-accepted
     
  • GingerDog

    GingerDog - 2009-03-20

    This has been merged; any future changes/issues should be in separate tickets... thanks

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks