RainLoop / rainloop-webmail

Simple, modern & fast web-based email client
http://rainloop.net
MIT License
4.11k stars 892 forks source link

Owncloud integration: User passwords insecure #1082

Open pfy opened 8 years ago

pfy commented 8 years ago

If you do a select * from oc_preferences where configkey = 'rainloop-autologin-password'; and call the decodePassword function from apps/rainloop/lib/RainLoopHelper.php on it with the $sSalt = md5(username) and $sPassword = configvalue you get the cleartext email and login password of this user if "Automatically login with ownCloud user credentials" is enabled.

Proposed fix: Use an IV and store it in the user session. Like this the database can never be used to get cleartext passwords. (sorry, i don't know how to use the owncloud session system. This would be better than use $_SESSION).

apps/rainloop/lib/RainLoopHelper.php

public static function decodePassword($sPassword, $sSalt)
{
        if (function_exists('mcrypt_encrypt') && function_exists('mcrypt_create_iv') && function_exists('mcrypt_get_iv_size') &&
                defined('MCRYPT_RIJNDAEL_256') && defined('MCRYPT_MODE_ECB') && defined('MCRYPT_RAND'))
        {
                return @base64_decode(trim(@mcrypt_decrypt(MCRYPT_RIJNDAEL_256, md5($sSalt.$_SESSION['rainloop_pw_iv']), base64_decode(trim($sPassword)),
                        MCRYPT_MODE_ECB, mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_ECB), MCRYPT_RAND))));
        }
        return @base64_decode(trim($sPassword));
}

public static function encodePassword($sPassword, $sSalt)
{
  $_SESSION['rainloop_pw_iv'] = openssl_random_pseudo_bytes(1024);
  if (function_exists('mcrypt_encrypt') && function_exists('mcrypt_create_iv') && function_exists('mcrypt_get_iv_size') &&
    defined('MCRYPT_RIJNDAEL_256') && defined('MCRYPT_MODE_ECB') && defined('MCRYPT_RAND'))
  {
    return @trim(base64_encode(mcrypt_encrypt(MCRYPT_RIJNDAEL_256, md5($sSalt.$_SESSION['rainloop_pw_iv']), base64_encode($sPassword),
      MCRYPT_MODE_ECB, mcrypt_create_iv(mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256, MCRYPT_MODE_ECB), MCRYPT_RAND))));
  }
  return @trim(base64_encode($sPassword));
}
e-alfred commented 7 years ago

Hello,

mcrypt is deprecated for a decade now and will be moved from official PHP to PECL with PHP 7.2.

The mcrypt extension has been abandonware for nearly a decade now, and was also fairly complex to use. It has therefore been deprecated in favour of OpenSSL, where it will be removed from the core and into PECL in PHP 7.2.

Link here: http://php.net/manual/en/migration71.deprecated.php

Please provide a secure way to store the account passwords of users accessing their mail accounts through Rainloop. A decade old extension definitely isn't one.

pierre-alain-b commented 7 years ago

I totally agree with this. If someone has a working implementation, I will be ready to merge into the nextcloud plugin to protect rainloop@nextcloud users!

muppeth commented 7 years ago

It's surprising and worrying that this issue is over 1 year old. Is rainloop team responsible for nextcloud/owncloud integration?

realitygaps commented 7 years ago

It does seem to be maintained by the rainloop team. I'm also awaiting a fix and have disabled this in our nextcloud for now. Perhaps it should be mentioned in the README?

RainLoop commented 7 years ago

Proposed fix: Use an IV and store it in the user session. Like this the database can never be used to get cleartext passwords. (sorry, i don't know how to use the owncloud session system. This would be better than use $_SESSION).

Unfortunately I cannot do that since I need cleartext password to login IMAP server.

P.S.: this commit adding support openssl encrypt/decrypt functions, but it's a breaking change that requires re-save users passwords.

muppeth commented 7 years ago

I wonder, could an attacker without access to the database server (so not knowing $sPassword's configvalue) could remotely use this bug to get any password and email of any user on the instance?

brianWreaves commented 6 years ago

Any potential solutions on this one?

Shen commented 4 years ago

This security problem has now been unresolved for almost two years. Is there any progress here? Only solution is not to save login-data in the app-settings?

jolly-jump commented 4 years ago

Proposed fix: Use an IV and store it in the user session. Like this the database can never be used to get cleartext passwords. (sorry, i don't know how to use the owncloud session system. This would be better than use $_SESSION).

Unfortunately I cannot do that since I need cleartext password to login IMAP server.

@pfy : Could you reply, if this is correct in your opinion? From what I understand: Your proposed fix stores an IV in the session and with that you can encrypt and decrypt the password from and to cleartext. But with ending the session (and deleting all cookies, say), the IV gets probably removed and you can't decrypt the next time you log in, am I right?

As far as I can see, there is no way one can protect decrypting a password with the database and the config.php in your hand. Am I right?

elhananjair commented 3 years ago

Hello guys, if this issue still persist isn't it good to remove this option from Nextcloud settings?

Evengard commented 2 years ago

Just derive the encryption key from the user's own Owncloud/Nextcloud password. This password is used anyway by the "autologin with the current user credentials" admin setting, right? Sure, if the user changes his Owncloud/Nextcloud password, the saved password for the personal autologin would be undecryptable, but isn't that kind of acceptable?

elhananjair commented 2 years ago

@pierre-alain-b any updates on this?

pierre-alain-b commented 2 years ago

This will not be fixed. We will shortly recommend to migrate to Snappymail that is maintained, unlike rainloop. See here: https://github.com/the-djmaze/snappymail/issues/96