Re: [Postfixadmin-devel] SF.net SVN: postfixadmin:[560] trunk

[Christian B. <pos...@cb...>]

Hello,

I have some questions and remarks about your commit.

Minor issue first: Postfixadmin uses 3 space characters per indention 
level for historical reasons. No tabs please.
(I did the whitespace fixes of your commit in SVN r561.)

If you use vim as editor, the vim: line in each file should setup this 
automatically (except if you have disabled modelines - which is the 
case on most newer installations because modelines can be security 
relevant somehow [don't ask for the details]).

Am Dienstag, 3. Februar 2009 schrieb roe...@us...:
> Revision: 560
> Author:   roehrijn

> function.inc.php:
> - changed pa_crypt to make it handle courier authlib authentication
> flavors

> --- trunk/functions.inc.php	2009-02-02 22:14:23 UTC (rev 559)
> +++ trunk/functions.inc.php	2009-02-03 17:50:13 UTC (rev 560)
> @@ -1160,6 +1160,27 @@

> +    if ($CONF['encrypt'] == 'authlib') {
> +    	$flavor = $CONF['authlib_default_flavor'];
> +    	$salt = '  ';

Is there a special reason to always use the same static salt? Otherwise, 
I'd prefer a random salt for security reasons.

Also the PHP documentation about crypt() says that in some cases 
(depending on the encryption method) the salt should be longer than two 
characters.

> +    	if(ereg('^{.*}', $pw_db)) {

preg_match() might be faster - at least that's what the documentation 
says ;-)

> +      	if(stripos($flavor, 'md5raw') === 0) {
> +         	$password = '{' . $flavor . '}' . md5($pw);
> +      	} else if(stripos($flavor, 'md5') === 0) {

Why "else if" instead of "elseif"?

> +      		$password = '{' . $flavor . '}' . base64_encode(md5($pw,
>                                                         TRUE)); 
> +      	} else if(stripos($flavor, 'crypt') === 0) { 

Same question about "else if" again ;-)

Oh, and what happens if someone sets something like
    $CONF['authlib_default_flavour'] = 'this-is-not-supported';
(or just a typo like 'md6')?

I'd expect an "else" with a clear error message for this case...
(Feel free to use die() since this is a configuration error.)


Regards,

Christian Boltz
-- 
         .:  Schneewittchen & die Pfälzer Waldconnection  :.
        Ein polit-kabarettistisches Märchenstück mit viel Musik
  gesungen & gespielt von Mitgliedern der Landjugend RheinhessenPfalz
18.1.2009 Berlin - 6.2.2009 Neustadt - Infos: www.LJ-RheinhessenPfalz.de