diff --git a/src/Frontend/Utils.php b/src/Frontend/Utils.php index be7380f8..59660614 100644 --- a/src/Frontend/Utils.php +++ b/src/Frontend/Utils.php @@ -108,7 +108,7 @@ public static function parseTimeParameter(string $timeStr): int * Converts a password to storage format according to the password storage scheme setting. * * @param string $clear The password in clear text. - * @return string The password in storage format (e.g. encrypted with user password as key) + * @return string The password in storage format (e.g., encrypted with user password as key) */ public static function encryptPassword(string $clear): string { @@ -131,7 +131,7 @@ public static function encryptPassword(string $clear): string $crypted = $rcube->encrypt($clear, 'carddav_des_key'); // there seems to be no way to unset a preference - $rcube->config->set('carddav_des_key', ''); + $rcube->config->set('carddav_des_key', null); if ($crypted === false) { throw new Exception("Password encryption with user password failed"); @@ -157,56 +157,72 @@ public static function encryptPassword(string $clear): string return '{DES_KEY}' . $crypted; } - // default: base64-coded password + // otherwise, it's BASE64 return '{BASE64}' . base64_encode($clear); } + /** + * Decrypts a password in the database according to the scheme used to encrypt or encode the password. + * + * In case of error, an error is logged and an empty string is returned. + */ public static function decryptPassword(string $crypt): string { $logger = Config::inst()->logger(); + $rcube = rcube::get_instance(); + $key = null; + $clear = ''; - if (strpos($crypt, '{ENCRYPTED}') === 0) { - try { + try { + if (strpos($crypt, '{ENCRYPTED}') === 0) { $crypt = substr($crypt, strlen('{ENCRYPTED}')); - $rcube = rcube::get_instance(); - $imap_password = self::getDesKey(); - $rcube->config->set('carddav_des_key', $imap_password); - $clear = $rcube->decrypt($crypt, 'carddav_des_key'); - // there seems to be no way to unset a preference - $rcube->config->set('carddav_des_key', ''); + + $key = 'carddav_des_key'; + $rcube->config->set($key, $imap_password); + } elseif (strpos($crypt, '{DES_KEY}') === 0) { + $crypt = substr($crypt, strlen('{DES_KEY}')); + $key = 'des_key'; + } elseif (strpos($crypt, '{BASE64}') === 0) { + $crypt = substr($crypt, strlen('{BASE64}')); + $clear = base64_decode($crypt, true); + if ($clear === false) { - $clear = ""; + throw new Exception('not a valid base64 string'); } - - return $clear; - } catch (Exception $e) { - $logger->warning("Cannot decrypt password: " . $e->getMessage()); - return ""; + } else { + // unknown scheme, assume cleartext + $clear = $crypt; } - } - if (strpos($crypt, '{DES_KEY}') === 0) { - $crypt = substr($crypt, strlen('{DES_KEY}')); - $rcube = rcube::get_instance(); - $clear = $rcube->decrypt($crypt); - if ($clear === false) { - $clear = ""; + if (isset($key)) { + $clear = $rcube->decrypt($crypt, $key); + if ($clear === false) { + throw new Exception("decryption with $key failed"); + } } - - return $clear; + } catch (Exception $e) { + $logger->warning("Cannot decrypt password: " . $e->getMessage()); + $clear = ''; } - if (strpos($crypt, '{BASE64}') === 0) { - $crypt = substr($crypt, strlen('{BASE64}')); - return base64_decode($crypt); + // not strictly needed, but lets not keep the cleartext IMAP password in rcube_config + if ($key === 'carddav_des_key') { + $rcube->config->set($key, null); } - // unknown scheme, assume cleartext - return $crypt; + return $clear; } - // password helpers + /** + * Returns the 24-byte decryption key derived from the user's password. + * + * The returned string is made exactly 24 bytes by repeating the IMAP password or, if it is longer, truncating it. + * This should not be required anymore, because openssl_encrypt() (and assumedly also openssl_decrypt()) pad the + * password themselves if needed, but this is kept for backwards compatibility with existing encrypted passwords. + * + * In case of error, this function throws an exception. + */ private static function getDesKey(): string { $rcube = rcube::get_instance(); @@ -218,16 +234,14 @@ private static function getDesKey(): string throw new Exception("No password available to use for encryption because user logged in via OAuth2"); } - $imap_password = $rcube->decrypt((string) $_SESSION['password']); + $imapPw = $rcube->decrypt((string) $_SESSION['password']); - if ($imap_password === false || strlen($imap_password) == 0) { + if ($imapPw === false || strlen($imapPw) == 0) { throw new Exception("No password available to use for encryption"); } - while (strlen($imap_password) < 24) { - $imap_password .= $imap_password; - } - return substr($imap_password, 0, 24); + $imapPw = str_pad($imapPw, 24, $imapPw); + return substr($imapPw, 0, 24); } } diff --git a/tests/Unit/FrontendUtilsTest.php b/tests/Unit/FrontendUtilsTest.php index 2a9c84c3..9761159a 100644 --- a/tests/Unit/FrontendUtilsTest.php +++ b/tests/Unit/FrontendUtilsTest.php @@ -26,11 +26,15 @@ namespace MStilkerich\Tests\RCMCardDAV\Unit; +use Exception; use MStilkerich\RCMCardDAV\Db\Database; use MStilkerich\Tests\RCMCardDAV\TestInfrastructure; use PHPUnit\Framework\TestCase; -use MStilkerich\RCMCardDAV\Frontend\Utils; +use MStilkerich\RCMCardDAV\Frontend\{AdminSettings,Utils}; +/** + * @psalm-import-type PasswordStoreScheme from AdminSettings + */ final class FrontendUtilsTest extends TestCase { public static function setUpBeforeClass(): void @@ -86,6 +90,224 @@ public function testTimeParameterParsedCorrectly(string $timestr, ?int $seconds) Utils::parseTimeParameter($timestr); } } + + /** + * @return array + */ + public function encryptedPasswordsProvider(): array + { + return [ + 'Cleartext password' => ['', '(l3art3x7!', '(l3art3x7!'], + 'Base64-encoded password' => ['', '{BASE64}YjRzZTY0IGVuYzBkZWQgUDQ1NXcwcmQh', 'b4se64 enc0ded P455w0rd!'], + 'DES-EDE3-CBC encrypted password (roundcube des_key)' => [ + 'DES-EDE3-CBC', + '{DES_KEY}iV+mfXdzDqpn/rjyah/i0u6xOOC7jBpf5DS39hVixAA=', + '3n(2ypted p4ssw0rd' + ], + 'DES-EDE3-CBC encrypted password (IMAP password)' => [ + 'DES-EDE3-CBC', + '{ENCRYPTED}Xfid9p4nu69npROnbKS4PrEUTkTPkKcG+UXecLAXjuA=', + 'imap 3n(2ypted p4ssw0rd', + ], + 'AES-256-CBC encrypted password (roundcube des_key)' => [ + 'AES-256-CBC', + '{DES_KEY}4hi+5PcK+EFetVqJOsfHLB0hic+hJ2lWTqbMaM1c6KZsKLfGNkYctcuCJQZaikXd', + '3n(Rypted P4s5w0rd' + ], + 'Decryption failure (wrong cipher method)' => [ + 'DES-EDE3-CBC', + '{DES_KEY}4hi+5PcK+EFetVqJOsfHLB0hic+hJ2lWTqbMaM1c6KZsKLfGNkYctcuCJQZaikXd', + '' // empty string expected on error + ], + 'Decryption failure (wrong password)' => [ + 'AES-256-CBC', + '{ENCRYPTED}4hi+5PcK+EFetVqJOsfHLB0hic+hJ2lWTqbMaM1c6KZsKLfGNkYctcuCJQZaikXd', + '' // empty string expected on error + ], + 'Decryption failure (invalid base64)' => [ + '', + '{BASE64}Not valid base64!', + '' // empty string expected on error + ], + ]; + } + + /** + * @dataProvider encryptedPasswordsProvider + */ + public function testPasswordCorrectlyDecrypted(string $cipherMethod, string $enc, string $expClear): void + { + $rcconfig = \rcube::get_instance()->config; + $rcconfig->set("cipher_method", $cipherMethod, false); + $rcconfig->set("des_key", 'ooceheiFeesah6PheeWaarae', false); + $_SESSION["password"] = \rcube::get_instance()->encrypt('iM4p P455w0rd'); + $clear = Utils::decryptPassword($enc); + $this->assertSame($expClear, $clear); + + // carddav_des_key must be cleared if set + $this->assertNull($rcconfig->get("carddav_des_key")); + + if ($expClear === '') { + // empty password means expected error in our tests + TestInfrastructure::logger()->expectMessage('warning', 'Cannot decrypt password'); + } + } + + /** + * @return array + */ + public function passwordsProvider(): array + { + return [ + 'Cleartext password' => [ + '', + 'plain', + '(l3art3x7!', + '(l3art3x7!' + ], + 'Cleartext password - %p shall not be replaced' => [ + '', + 'plain', + '%p', + '%p' + ], + 'Cleartext password - %b shall not be replaced' => [ + '', + 'plain', + '%b', + '%b' + ], + 'Base64-encoded password' => [ + '', + 'base64', + 'b4se64 enc0ded P455w0rd!', + '{BASE64}YjRzZTY0IGVuYzBkZWQgUDQ1NXcwcmQh' + ], + 'DES-EDE3-CBC encrypted password (roundcube des_key)' => [ + 'DES-EDE3-CBC', + 'des_key', + '3n(2ypted p4ssw0rd', + null + ], + 'DES-EDE3-CBC encrypted password (IMAP password)' => [ + 'DES-EDE3-CBC', + 'encrypted', + 'imap 3n(2ypted p4ssw0rd', + null + ], + 'AES-256-CBC encrypted password (roundcube des_key)' => [ + 'AES-256-CBC', + 'des_key', + '3n(Rypted P4s5w0rd', + null + ], + ]; + } + + /** + * @param PasswordStoreScheme $scheme + * @dataProvider passwordsProvider + */ + public function testPasswordCorrectlyEncrypted( + string $cipherMethod, + string $scheme, + string $clear, + ?string $expCipher + ): void { + if ($clear === '%b' || $clear === '%p' || $scheme === 'plain') { + // placeholders always expected to be stored as plaintext, independent of configured pwStoreScheme + $prefix = ''; + } else { + $prefix = "{" . strtoupper($scheme) . "}"; + } + + $admPrefs = TestInfrastructure::$infra->admPrefs(); + + /** @psalm-suppress InaccessibleProperty For test purposes, we override the scheme */ + $admPrefs->pwStoreScheme = $scheme; + + $rcube = \rcube::get_instance(); + $rcconfig = $rcube->config; + $rcconfig->set("cipher_method", $cipherMethod, false); + $rcconfig->set("des_key", 'ooceheiFeesah6PheeWaarae', false); + $rcconfig->set("rcmcarddav_test_des_key", 'iM4p P455w0rdiM4p P455w0', false); + $_SESSION["password"] = $rcube->encrypt('iM4p P455w0rd'); + + $cipher = Utils::encryptPassword($clear); + + // carddav_des_key must be cleared if set + $this->assertNull($rcconfig->get("carddav_des_key")); + + if (strlen($prefix) > 0) { + $this->assertStringStartsWith($prefix, $cipher); + } + + if (is_null($expCipher)) { + $cipher = substr($cipher, strlen($prefix)); + $key = (($scheme === 'des_key') ? 'des_key' : 'rcmcarddav_test_des_key'); + $clearDec = $rcube->decrypt($cipher, $key); + $this->assertSame($clear, $clearDec); + } else { + $this->assertSame($expCipher, $cipher); + } + } + + /** + * When encryption of a password with the encrypted method fails, RCMCardDAV shall fall back to DES_KEY encryption. + * This is tested here. + */ + public function testPasswordEncryptionFallsBackToDesKeyOnError(): void + { + $admPrefs = TestInfrastructure::$infra->admPrefs(); + + /** @psalm-suppress InaccessibleProperty For test purposes, we override the scheme */ + $admPrefs->pwStoreScheme = 'encrypted'; + + $rcube = \rcube::get_instance(); + $rcconfig = $rcube->config; + $rcconfig->set("des_key", 'ooceheiFeesah6PheeWaarae', false); + $_SESSION["password"] = 'not an encrypted imap password'; + + $clear = 't3st p4ssw0rd'; + $cipher = Utils::encryptPassword($clear); + $this->assertStringStartsWith('{DES_KEY}', $cipher); + + $cipher = substr($cipher, strlen('{DES_KEY}')); + $clearDec = $rcube->decrypt($cipher); + $this->assertSame($clear, $clearDec); + TestInfrastructure::logger()->expectMessage('warning', "Could not encrypt password with 'encrypted' method"); + } + + /** + * When OAuth authentication is used, the SESSION password contains a volatile token that must not be used for + * encryption, as at a later point in time, the token will change and we would not be able to decrypt the password + * anymore. Fallback to DES_KEY expected in this case on encryption. + */ + public function testPasswordEncryptionFallsBackToDesKeyOnOauth(): void + { + $admPrefs = TestInfrastructure::$infra->admPrefs(); + + /** @psalm-suppress InaccessibleProperty For test purposes, we override the scheme */ + $admPrefs->pwStoreScheme = 'encrypted'; + + $rcube = \rcube::get_instance(); + $rcconfig = $rcube->config; + $rcconfig->set("des_key", 'ooceheiFeesah6PheeWaarae', false); + $_SESSION["password"] = $rcube->encrypt('iM4p P455w0rd'); + $_SESSION["oauth_token"] = 'I am a token!'; + + $clear = 'oauth t3st p4ssw0rd'; + $cipher = Utils::encryptPassword($clear); + $this->assertStringStartsWith('{DES_KEY}', $cipher); + + $cipher = substr($cipher, strlen('{DES_KEY}')); + $clearDec = $rcube->decrypt($cipher); + $this->assertSame($clear, $clearDec); + TestInfrastructure::logger()->expectMessage( + 'warning', + 'No password available to use for encryption because user logged in via OAuth2' + ); + } } // vim: ts=4:sw=4:expandtab:fenc=utf8:ff=unix:tw=120