Skip to content

Commit

Permalink
Tests for password encryption and decryption
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstilkerich
mstilkerich committed Jan 19, 2023
1 parent dd319f3 commit 78c2a1c
Show file tree
Hide file tree
Showing 2 changed files with 275 additions and 39 deletions.
90 changes: 52 additions & 38 deletions src/Frontend/Utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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");
Expand All @@ -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();
Expand All @@ -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);
}
}

Expand Down
224 changes: 223 additions & 1 deletion tests/Unit/FrontendUtilsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -86,6 +90,224 @@ public function testTimeParameterParsedCorrectly(string $timestr, ?int $seconds)
Utils::parseTimeParameter($timestr);
}
}

/**
* @return array<string, list{string,string,string}>
*/
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<string, list{string,PasswordStoreScheme,string,?string}>
*/
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

0 comments on commit 78c2a1c

Please sign in to comment.