Re: [Phpgroupware-developers] Re: [ phpgroupware-Bugs-445721 ] email password not saved.

[Del]

Tony (Angles) Puglisi wrote:
> 
> The 2 changed files are:
> mail/inc/class.mail_msg.inc.php
> phpgwapi/inc/class.crypto.inc.php
> 
> You'll notice I had to create 2 new functions in class crypto that are 90% 
> the same
> as their namesakes, these are:
> encrypt_mail_pass
> decrypt_mail_pass
> 
> Because the changes to the original en/decrypt functions broke some other 
> behaviour
> elsewhere in phpgw, namely users withOUT custom email passwords did not get a 
> passwd
> at all in their pgpgw_info array.

OK, I tried to reproduce this and failed, but never mind.  For me it seemed to 
work
fine because the imap functions just used the login password.

> I assume this is related to one thing we are trying to fix here, that 
> installations
> without mcrypt should have data returned unmolested from the crypto class 
> since no
> crypto is going to take place. Additionally, if crypto (mcrypt) does take 
> place, the
> pre and post encryption data manipulation must be "mirror imaged" in the 
> en/decrypt
> functions.

Yes, agreed.

You have a typo on line 229 of class.crypto.inc, it should read:

  return $encrypteddata

not

  return $data

Personally, I think there should just be two functions, encrypt() and decrypt()
and that they should always serialize/unserialize and addslashes/stripslashes
so that the cleartext data can always be an object, and the ciphertext can 
always
be put into a cookie or database field.

Line 224: your question:  Yes it IS necessary to serialize and addslashes.
PHP will only encrypt strings.  You need to turn all objects / arrays into
strings before encryption.

I recommend leaving the serialize/addslashes code there even when mcrypt
is turned off.  It ensures that all encrypted data can be fed into a cookie
or database field without doing any extra work.  Also, I should be able to
implement some simple (albeit breakable) crypto code in PHP for those without
mcrypt.

That means that the encrypt() / decrypt() code will always mangle data, however.
Not sure how much of a religious / design issue that is for you.  I'd hate to
see code like this:

if (mcrypt_enabled) {
  // encrypt does the serialization for us
  encrypt_data
  store_in_cookie
} else {
  // we have to do the addslashes and serialize ourselves here
  serialize
  addslashes
  store_in_cookie
}

i.e. the API should be identical whether mcrypt is turned on or off.

> This provides us with a seperate code path to fix and/or improve upon the 
> crypto,
> then later have the rest of the api migrate over to it.

OK, that sounds like a good idea.

We should move off of the old encrypt() / decrypt() functions there because they
are broken.  encrypt(decrypt(X)) never returns X whereas it should always return
X.  The squirrelmail code needs to be moved over because it's broken in the 
current
implementation (I had it working against the new functions but don't have commit
access).

Lastly, some cipher issues:

-  Line 59, I recommend changing MCRYPT_TRIPLEDES to MCRYPT_RIJNDAEL_128, gives
   better performance and security as I mentioned in a previous e-mail.  Only
   works for mcrypt >= 2.4

-  Lines 48, 59, and all places where mcrypt_cbc is used:  Replace CBC/cbc
   with ECB/ecb.  ECB is really only suitable for encrypting multiple blocks
   (where 1 block = 8 bytes) of data at a time.  Below about 5 blocks it adds
   a performance overhead and doesn't gain you any security over ECB mode.
   Also it makes the encrypted data bigger.

> >
> >Bugs item #445721, was opened at 2001-07-29 10:40
> >You can respond by visiting:
> >http://sourceforge.net/tracker/?func=detail&atid=107305&aid=445721&group_id=7305
> >
> >Category: Email
> >Group: Stable Code
> >Status: Open
> >Resolution: Fixed
> >Priority: 5
> >Submitted By: Micahel Roark (rhuian)
> >Assigned to: Mark A Peters (skeeter)
> >Summary: email password not saved.
> >
> _SNIP_
> --
> that's "angle" as in geometry
> 
> _______________________________________________
> Phpgroupware-developers mailing list
> address@hidden
> http://mail.gnu.org/mailman/listinfo/phpgroupware-developers

--
Del