From dba0c4cedae25c82cd9f255df540bd3d664afa9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D?= Date: Sun, 21 May 2023 17:09:09 +0200 Subject: [PATCH] chore(tests): Add SecureSessionService tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rubén D --- .phpstorm.meta.php | 2 + app/modules/web/Init.php | 4 +- lib/SP/Core/Crypt/Cookie.php | 26 +-- lib/SP/Core/Crypt/RequestBasedPassword.php | 61 ++++++ .../Crypt/RequestBasedPasswordInterface.php | 33 +++ lib/SP/Core/Crypt/SecureKeyCookie.php | 144 ------------ lib/SP/Core/Crypt/UUIDCookie.php | 9 +- lib/SP/Core/Definitions/CoreDefinitions.php | 67 +++--- lib/SP/Core/Definitions/DomainDefinitions.php | 29 ++- .../Ports/SecureSessionServiceInterface.php | 15 +- .../Crypt/Services/SecureSessionService.php | 136 +++++------- lib/SP/Infrastructure/File/FileCache.php | 24 +- .../File/FileCacheInterface.php | 11 +- tests/SP/Core/Crypt/SecureKeyCookieTest.php | 101 --------- .../Services/SecureSessionServiceTest.php | 206 ++++++++++++++++++ .../Crypt/SecureSessionServiceTest.php | 63 ------ 16 files changed, 472 insertions(+), 459 deletions(-) create mode 100644 lib/SP/Core/Crypt/RequestBasedPassword.php create mode 100644 lib/SP/Core/Crypt/RequestBasedPasswordInterface.php delete mode 100644 lib/SP/Core/Crypt/SecureKeyCookie.php delete mode 100644 tests/SP/Core/Crypt/SecureKeyCookieTest.php create mode 100644 tests/SP/Domain/Crypt/Services/SecureSessionServiceTest.php delete mode 100644 tests/SP/Services/Crypt/SecureSessionServiceTest.php diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index 9129aeb4..d7c2e727 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -23,7 +23,9 @@ */ namespace PHPSTORM_META { + override(\Psr\Container\ContainerInterface::get(0), type(0)); override(\SP\Infrastructure\Database\QueryResult::getData(0), type(0)); override(\SP\Util\Util::unserialize(0), type(0)); + override(\SP\Infrastructure\File\FileCacheInterface::load(0, 1), type(1)); } diff --git a/app/modules/web/Init.php b/app/modules/web/Init.php index ea2560ab..192baae8 100644 --- a/app/modules/web/Init.php +++ b/app/modules/web/Init.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -311,7 +311,7 @@ final class Init extends HttpModuleBase { if ($encrypt === true && BootstrapBase::$checkPhpVersion - && ($key = $this->secureSessionService->getKey(UUIDCookie::factory($this->request))) !== false) { + && ($key = $this->secureSessionService->getKey()) !== false) { session_set_save_handler(new CryptSessionHandler($key), true); } diff --git a/lib/SP/Core/Crypt/Cookie.php b/lib/SP/Core/Crypt/Cookie.php index 81b16e0b..88bc960b 100644 --- a/lib/SP/Core/Crypt/Cookie.php +++ b/lib/SP/Core/Crypt/Cookie.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2021, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -25,8 +25,8 @@ namespace SP\Core\Crypt; use SP\Core\Bootstrap\BootstrapBase; -use SP\Http\Request; use SP\Http\RequestInterface; +use function SP\logger; /** * Class Cookie @@ -35,20 +35,13 @@ use SP\Http\RequestInterface; */ abstract class Cookie { - protected Request $request; - private string $cookieName; - /** * Cookie constructor. * * @param string $cookieName * @param RequestInterface $request */ - protected function __construct(string $cookieName, RequestInterface $request) - { - $this->cookieName = $cookieName; - $this->request = $request; - } + protected function __construct(private readonly string $cookieName, protected readonly RequestInterface $request) {} /** * Firmar la cookie para autentificación @@ -57,17 +50,20 @@ abstract class Cookie { $data = base64_encode($data); - return Hash::signMessage($data, $cypher) . ';' . $data; + return Hash::signMessage($data, $cypher).';'.$data; } /** * Comprobar la firma de la cookie y devolver los datos * + * @param string $data + * @param string $cypher + * * @return bool|string */ - final public function getCookieData(string $data, string $cypher) + final public function getCookieData(string $data, string $cypher): bool|string { - if (strpos($data, ';') === false) { + if (!str_contains($data, ';')) { return false; } @@ -83,7 +79,7 @@ abstract class Cookie * * @return bool|string */ - protected function getCookie() + protected function getCookie(): bool|string { return $this->request ->getRequest() @@ -109,4 +105,4 @@ abstract class Cookie return setcookie($this->cookieName, $data, 0, BootstrapBase::$WEBROOT); } -} \ No newline at end of file +} diff --git a/lib/SP/Core/Crypt/RequestBasedPassword.php b/lib/SP/Core/Crypt/RequestBasedPassword.php new file mode 100644 index 00000000..ce4c7bc2 --- /dev/null +++ b/lib/SP/Core/Crypt/RequestBasedPassword.php @@ -0,0 +1,61 @@ +. + */ + +namespace SP\Core\Crypt; + +use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Http\RequestInterface; + +/** + * Class RequestBasedPassword + */ +final class RequestBasedPassword implements RequestBasedPasswordInterface +{ + public function __construct( + private readonly RequestInterface $request, + private readonly ConfigDataInterface $configData + ) {} + + public function build(): string + { + return hash_pbkdf2( + 'sha1', + $this->getWellKnownData(), + $this->configData->getPasswordSalt(), + 5000, + 32 + ); + } + + /** + * @return string + */ + private function getWellKnownData(): string + { + return sha1( + $this->request->getHeader('User-Agent'). + $this->request->getClientAddress() + ); + } +} diff --git a/lib/SP/Core/Crypt/RequestBasedPasswordInterface.php b/lib/SP/Core/Crypt/RequestBasedPasswordInterface.php new file mode 100644 index 00000000..3c5f6c42 --- /dev/null +++ b/lib/SP/Core/Crypt/RequestBasedPasswordInterface.php @@ -0,0 +1,33 @@ +. + */ + +namespace SP\Core\Crypt; + +/** + * Class RequestBasedPassword + */ +interface RequestBasedPasswordInterface +{ + public function build(): string; +} diff --git a/lib/SP/Core/Crypt/SecureKeyCookie.php b/lib/SP/Core/Crypt/SecureKeyCookie.php deleted file mode 100644 index 143cb3b5..00000000 --- a/lib/SP/Core/Crypt/SecureKeyCookie.php +++ /dev/null @@ -1,144 +0,0 @@ -. - */ - -namespace SP\Core\Crypt; - -use Defuse\Crypto\Exception\CryptoException; -use Defuse\Crypto\Exception\EnvironmentIsBrokenException; -use Defuse\Crypto\Key; -use SP\Http\Request; -use SP\Http\RequestInterface; - -/** - * Class SecureKeyCookie - * - * @package SP\Core\Crypt - */ -class SecureKeyCookie extends Cookie -{ - /** - * Nombre de la cookie - */ - public const COOKIE_NAME = 'SYSPASS_SK'; - /** - * Llave usada para encriptar los datos - */ - private ?Key $securedKey = null; - private ?string $cypher = null; - - public static function factory(RequestInterface $request): SecureKeyCookie - { - $self = new self(self::COOKIE_NAME, $request); - $self->cypher = $self->getCypher(); - - return $self; - } - - /** - * Devolver la llave de cifrado para los datos de la cookie - */ - public function getCypher(): string - { - return sha1($this->request->getHeader('User-Agent') . $this->request->getClientAddress()); - } - - /** - * Obtener una llave de encriptación - * - * @return false|Key - */ - public function getKey() - { - $cookie = $this->getCookie(); - - if ($cookie !== false) { - $data = $this->getCookieData($cookie, $this->cypher); - - if ($data !== false) { - /** @var Vault $vault */ - $vault = unserialize($data, ['allowed_classes' => Vault::class]); - - if (($vault instanceof Vault) === true - ) { - try { - $this->securedKey = Key::loadFromAsciiSafeString($vault->getData($this->cypher)); - - return $this->securedKey; - } catch (CryptoException $e) { - logger($e->getMessage(), 'EXCEPTION'); - } - - return false; - } - } else { - logger('Cookie verification error', 'ERROR'); - } - } elseif (($this->securedKey instanceof Key) === true) { - return $this->securedKey; - } - - return $this->saveKey() ? $this->securedKey : false; - } - - /** - * Guardar una llave de encriptación - */ - public function saveKey(): bool - { - try { - if ($this->setCookie($this->sign($this->generateSecuredData()->getSerialized(), $this->cypher)) === false) { - logger('Could not generate session\'s key cookie', 'ERROR'); - - unset($this->securedKey); - - return false; - } - - logger('Generating a new session\'s key cookie'); - - return true; - } catch (CryptoException $e) { - logger($e->getMessage(), 'EXCEPTION'); - } - - return false; - } - - /** - * @throws CryptoException - * @throws EnvironmentIsBrokenException - */ - public function generateSecuredData(): Vault - { - $this->securedKey = Key::createNewRandomKey(); - - return (new Vault()) - ->saveData($this->securedKey->saveToAsciiSafeString(), $this->cypher); - } - - public function getSecuredKey(): Key - { - return $this->securedKey; - } -} \ No newline at end of file diff --git a/lib/SP/Core/Crypt/UUIDCookie.php b/lib/SP/Core/Crypt/UUIDCookie.php index 518f2cbb..5e780cf5 100644 --- a/lib/SP/Core/Crypt/UUIDCookie.php +++ b/lib/SP/Core/Crypt/UUIDCookie.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2021, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -24,7 +24,6 @@ namespace SP\Core\Crypt; -use SP\Http\Request; use SP\Http\RequestInterface; /** @@ -49,7 +48,7 @@ class UUIDCookie extends Cookie * * @return string|false */ - public function createCookie(string $signKey) + public function create(string $signKey): bool|string { $uuid = uniqid('', true); @@ -65,7 +64,7 @@ class UUIDCookie extends Cookie * * @return false|string */ - public function loadCookie(string $signKey) + public function load(string $signKey): bool|string { $data = $this->getCookie(); @@ -73,4 +72,4 @@ class UUIDCookie extends Cookie ? $this->getCookieData($data, $signKey) : false; } -} \ No newline at end of file +} diff --git a/lib/SP/Core/Definitions/CoreDefinitions.php b/lib/SP/Core/Definitions/CoreDefinitions.php index 19e54911..d07f0671 100644 --- a/lib/SP/Core/Definitions/CoreDefinitions.php +++ b/lib/SP/Core/Definitions/CoreDefinitions.php @@ -30,6 +30,7 @@ use PHPMailer\PHPMailer\PHPMailer; use Psr\Container\ContainerInterface; use SP\Core\Acl\Acl; use SP\Core\Acl\Actions; +use SP\Core\Application; use SP\Core\Context\ContextFactory; use SP\Core\Context\ContextInterface; use SP\Core\Crypt\Crypt; @@ -37,6 +38,9 @@ use SP\Core\Crypt\CryptInterface; use SP\Core\Crypt\CryptPKI; use SP\Core\Crypt\CryptPKIInterface; use SP\Core\Crypt\CSRF; +use SP\Core\Crypt\RequestBasedPassword; +use SP\Core\Crypt\RequestBasedPasswordInterface; +use SP\Core\Crypt\UUIDCookie; use SP\Core\Exceptions\SPException; use SP\Core\Language; use SP\Core\LanguageInterface; @@ -101,11 +105,11 @@ final class CoreDefinitions public static function getDefinitions(): array { return [ - RequestInterface::class => create(Request::class) + RequestInterface::class => create(Request::class) ->constructor(\Klein\Request::createFromGlobals(), autowire(CryptPKI::class)), - ContextInterface::class => + ContextInterface::class => static fn() => ContextFactory::getForModule(APP_MODULE), - ConfigInterface::class => create(ConfigFileService::class) + ConfigInterface::class => create(ConfigFileService::class) ->constructor( create(XmlHandler::class) ->constructor(create(FileHandler::class)->constructor(CONFIG_FILE)), @@ -113,37 +117,37 @@ final class CoreDefinitions get(ContextInterface::class), autowire(ConfigBackupService::class) ), - ConfigDataInterface::class => + ConfigDataInterface::class => static fn(ConfigInterface $config) => $config->getConfigData(), - DatabaseConnectionData::class => factory([DatabaseConnectionData::class, 'getFromConfig']), - DbStorageInterface::class => autowire(MysqlHandler::class), - Actions::class => + DatabaseConnectionData::class => factory([DatabaseConnectionData::class, 'getFromConfig']), + DbStorageInterface::class => autowire(MysqlHandler::class), + Actions::class => static fn() => new Actions( new FileCache(Actions::ACTIONS_CACHE_FILE), new XmlHandler(new FileHandler(ACTIONS_FILE)) ), - MimeTypesInterface::class => + MimeTypesInterface::class => static fn() => new MimeTypes( new FileCache(MimeTypes::MIME_CACHE_FILE), new XmlHandler(new FileHandler(MIMETYPES_FILE)) ), - Acl::class => autowire(Acl::class) + Acl::class => autowire(Acl::class) ->constructorParameter('actions', get(Actions::class)), - ThemeInterface::class => autowire(Theme::class) + ThemeInterface::class => autowire(Theme::class) ->constructorParameter('module', APP_MODULE) ->constructorParameter( 'fileCache', create(FileCache::class)->constructor(Theme::ICONS_CACHE_FILE) ), - TemplateInterface::class => autowire(Template::class), - DatabaseAuthInterface::class => autowire(DatabaseAuth::class), - BrowserAuthInterface::class => autowire(BrowserAuth::class), - LdapAuthInterface::class => autowire(LdapAuth::class) + TemplateInterface::class => autowire(Template::class), + DatabaseAuthInterface::class => autowire(DatabaseAuth::class), + BrowserAuthInterface::class => autowire(BrowserAuth::class), + LdapAuthInterface::class => autowire(LdapAuth::class) ->constructorParameter( 'ldap', factory([Ldap::class, 'factory'])->parameter('ldapParams', factory([LdapParams::class, 'getFrom'])) ), - AuthProviderInterface::class => + AuthProviderInterface::class => static function (ContainerInterface $c, ConfigDataInterface $configData) { $provider = $c->get(AuthProvider::class); @@ -157,18 +161,18 @@ final class CoreDefinitions return $provider; }, - Logger::class => create(Logger::class) + Logger::class => create(Logger::class) ->constructor('syspass'), - \GuzzleHttp\Client::class => create(\GuzzleHttp\Client::class) + \GuzzleHttp\Client::class => create(\GuzzleHttp\Client::class) ->constructor(factory([Client::class, 'getOptions'])), - CSRF::class => autowire(CSRF::class), - LanguageInterface::class => autowire(Language::class), - DatabaseInterface::class => autowire(Database::class), - MailProviderInterface::class => autowire(MailProvider::class), - MailerInterface::class => autowire(PhpMailerWrapper::class)->constructor( + CSRF::class => autowire(CSRF::class), + LanguageInterface::class => autowire(Language::class), + DatabaseInterface::class => autowire(Database::class), + MailProviderInterface::class => autowire(MailProvider::class), + MailerInterface::class => autowire(PhpMailerWrapper::class)->constructor( create(PHPMailer::class)->constructor(true) ), - DatabaseSetupInterface::class => static function (RequestInterface $request) { + DatabaseSetupInterface::class => static function (RequestInterface $request) { $installData = InstallDataFactory::buildFromRequest($request); if ($installData->getBackendType() === 'mysql') { @@ -177,7 +181,7 @@ final class CoreDefinitions throw new SPException(__u('Unimplemented'), SPException::ERROR, __u('Wrong backend type')); }, - ProvidersHelper::class => factory(static function (ContainerInterface $c) { + ProvidersHelper::class => factory(static function (ContainerInterface $c) { $configData = $c->get(ConfigDataInterface::class); if (!$configData->isInstalled()) { @@ -194,13 +198,20 @@ final class CoreDefinitions $c->get(NotificationHandler::class) ); }), - QueryFactory::class => create(QueryFactory::class) + QueryFactory::class => create(QueryFactory::class) ->constructor('mysql', QueryFactory::COMMON), - CryptInterface::class => create(Crypt::class), - CryptPKIInterface::class => autowire(CryptPKI::class) + CryptInterface::class => create(Crypt::class), + CryptPKIInterface::class => autowire(CryptPKI::class) ->constructorParameter('publicKeyFile', new FileHandler(CryptPKI::PUBLIC_KEY_FILE)) ->constructorParameter('privateKeyFile', new FileHandler(CryptPKI::PRIVATE_KEY_FILE)), - FileCacheInterface::class => create(FileCache::class), + FileCacheInterface::class => create(FileCache::class), + Application::class => autowire(Application::class), + UUIDCookie::class => factory([UUIDCookie::class, 'factory']) + ->parameter( + 'request', + get(RequestInterface::class) + ), + RequestBasedPasswordInterface::class => autowire(RequestBasedPassword::class), ]; } } diff --git a/lib/SP/Core/Definitions/DomainDefinitions.php b/lib/SP/Core/Definitions/DomainDefinitions.php index 73d48592..0269ca53 100644 --- a/lib/SP/Core/Definitions/DomainDefinitions.php +++ b/lib/SP/Core/Definitions/DomainDefinitions.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -24,9 +24,19 @@ namespace SP\Core\Definitions; +use Psr\Container\ContainerInterface; +use SP\Core\Application; +use SP\Core\Crypt\CryptInterface; +use SP\Core\Crypt\RequestBasedPassword; +use SP\Core\Crypt\UUIDCookie; use SP\Domain\Account\Ports\AccountSearchDataBuilderInterface; use SP\Domain\Account\Search\AccountSearchDataBuilder; +use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Domain\Crypt\Ports\SecureSessionServiceInterface; +use SP\Domain\Crypt\Services\SecureSessionService; +use SP\Infrastructure\File\FileCache; use function DI\autowire; +use function DI\factory; /** * Class DomainDefinitions @@ -92,6 +102,23 @@ final class DomainDefinitions 'SP\Domain\Plugin\Ports\*RepositoryInterface' => autowire( 'SP\Infrastructure\Plugin\Repositories\*Repository' ), + SecureSessionServiceInterface::class => factory( + static function (ContainerInterface $c) { + $fileCache = new FileCache( + SecureSessionService::getFileNameFrom( + $c->get(UUIDCookie::class), + $c->get(ConfigDataInterface::class)->getPasswordSalt() + ) + ); + + return new SecureSessionService( + $c->get(Application::class), + $c->get(CryptInterface::class), + $fileCache, + $c->get(RequestBasedPassword::class) + ); + } + ), ]; } } diff --git a/lib/SP/Domain/Crypt/Ports/SecureSessionServiceInterface.php b/lib/SP/Domain/Crypt/Ports/SecureSessionServiceInterface.php index 174290f1..e77430f8 100644 --- a/lib/SP/Domain/Crypt/Ports/SecureSessionServiceInterface.php +++ b/lib/SP/Domain/Crypt/Ports/SecureSessionServiceInterface.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -26,6 +26,7 @@ namespace SP\Domain\Crypt\Ports; use Defuse\Crypto\Key; use SP\Core\Crypt\UUIDCookie; +use SP\Domain\Common\Services\ServiceException; /** * Class SecureSessionService @@ -34,14 +35,18 @@ use SP\Core\Crypt\UUIDCookie; */ interface SecureSessionServiceInterface { + /** + * Returns an unique filename from a browser cookie + * + * @throws ServiceException + */ + public static function getFileNameFrom(UUIDCookie $cookie, string $seed): string; + /** * Returns the encryption key * - * @param UUIDCookie $cookie * * @return Key|false */ - public function getKey(UUIDCookie $cookie): Key|bool; - - public function getFilename(): string; + public function getKey(): Key|bool; } diff --git a/lib/SP/Domain/Crypt/Services/SecureSessionService.php b/lib/SP/Domain/Crypt/Services/SecureSessionService.php index 3039affe..159e8f3c 100644 --- a/lib/SP/Domain/Crypt/Services/SecureSessionService.php +++ b/lib/SP/Domain/Crypt/Services/SecureSessionService.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -27,13 +27,15 @@ namespace SP\Domain\Crypt\Services; use Defuse\Crypto\Key; use Exception; use SP\Core\Application; +use SP\Core\Crypt\CryptInterface; +use SP\Core\Crypt\RequestBasedPassword; +use SP\Core\Crypt\RequestBasedPasswordInterface; use SP\Core\Crypt\UUIDCookie; use SP\Core\Crypt\Vault; use SP\Domain\Common\Services\Service; use SP\Domain\Common\Services\ServiceException; use SP\Domain\Crypt\Ports\SecureSessionServiceInterface; -use SP\Http\RequestInterface; -use SP\Infrastructure\File\FileCache; +use SP\Infrastructure\File\FileCacheInterface; use SP\Infrastructure\File\FileException; use function SP\logger; use function SP\processException; @@ -48,50 +50,13 @@ final class SecureSessionService extends Service implements SecureSessionService private const CACHE_EXPIRE_TIME = 86400; private const CACHE_PATH = CACHE_PATH.DIRECTORY_SEPARATOR.'secure_session'; - private RequestInterface $request; - private string $seed; - private ?UUIDCookie $cookie = null; - private ?string $filename = null; - - public function __construct(Application $application, RequestInterface $request) - { + public function __construct( + Application $application, + private readonly CryptInterface $crypt, + private readonly FileCacheInterface $fileCache, + private readonly RequestBasedPasswordInterface $requestBasedPassword + ) { parent::__construct($application); - - $this->request = $request; - $this->seed = $this->config->getConfigData()->getPasswordSalt(); - } - - - /** - * Returns the encryption key - * - * @param UUIDCookie $cookie - * - * @return Key|false - */ - public function getKey(UUIDCookie $cookie): Key|bool - { - $this->cookie = $cookie; - - try { - $cache = FileCache::factory($this->getFileNameFromCookie()); - - if ($cache->isExpired(self::CACHE_EXPIRE_TIME)) { - logger('Session key expired or does not exist', 'ERROR'); - - return $this->saveKey(); - } - - if (($vault = $cache->load()) instanceof Vault) { - return Key::loadFromAsciiSafeString($vault->getData($this->getCypher())); - } - } catch (FileException $e) { - return $this->saveKey(); - } catch (Exception $e) { - processException($e); - } - - return false; } /** @@ -99,19 +64,42 @@ final class SecureSessionService extends Service implements SecureSessionService * * @throws ServiceException */ - private function getFileNameFromCookie(): string + public static function getFileNameFrom(UUIDCookie $cookie, string $seed): string { - if (empty($this->filename)) { - if (($uuid = $this->cookie->loadCookie($this->seed)) === false - && ($uuid = $this->cookie->createCookie($this->seed)) === false - ) { - throw new ServiceException('Unable to get UUID for filename'); - } - - $this->filename = self::CACHE_PATH.DIRECTORY_SEPARATOR.$uuid; + if (($uuid = $cookie->load($seed)) === false + && ($uuid = $cookie->create($seed)) === false + ) { + throw new ServiceException('Unable to get UUID for filename'); } - return $this->filename; + return self::CACHE_PATH.DIRECTORY_SEPARATOR.$uuid; + } + + /** + * Returns the encryption key + * + * + * @return Key|false + */ + public function getKey(): Key|bool + { + try { + if ($this->fileCache->isExpired(self::CACHE_EXPIRE_TIME)) { + logger('Session key expired or does not exist', 'ERROR'); + + return $this->saveKey(); + } + + $vault = $this->fileCache->load(null, Vault::class); + + return Key::loadFromAsciiSafeString($vault->getData($this->requestBasedPassword->build())); + } catch (FileException) { + return $this->saveKey(); + } catch (Exception $e) { + processException($e); + } + + return false; } /** @@ -124,13 +112,13 @@ final class SecureSessionService extends Service implements SecureSessionService try { $securedKey = Key::createNewRandomKey(); - FileCache::factory($this->getFileNameFromCookie()) - ->save( - (new Vault())->saveData( - $securedKey->saveToAsciiSafeString(), - $this->getCypher() - ) - ); + $data = Vault::factory($this->crypt) + ->saveData( + $securedKey->saveToAsciiSafeString(), + $this->requestBasedPassword->build() + ); + + $this->fileCache->save($data); logger('Saved session key'); @@ -141,26 +129,4 @@ final class SecureSessionService extends Service implements SecureSessionService return false; } - - /** - * Returns the key to be used for encrypting the session data - */ - private function getCypher(): string - { - return hash_pbkdf2( - 'sha1', - sha1( - $this->request->getHeader('User-Agent'). - $this->request->getClientAddress() - ), - $this->seed, - 500, - 32 - ); - } - - public function getFilename(): string - { - return $this->filename; - } } diff --git a/lib/SP/Infrastructure/File/FileCache.php b/lib/SP/Infrastructure/File/FileCache.php index 3ac3fc55..e6c0b00c 100644 --- a/lib/SP/Infrastructure/File/FileCache.php +++ b/lib/SP/Infrastructure/File/FileCache.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -24,6 +24,9 @@ namespace SP\Infrastructure\File; +use SP\Core\Exceptions\InvalidClassException; +use function SP\__u; + /** * Class FileCache * @@ -33,13 +36,22 @@ class FileCache extends FileCacheBase { /** * @throws FileException + * @throws \SP\Core\Exceptions\InvalidClassException */ - public function load(?string $path = null): mixed + public function load(?string $path = null, ?string $class = null): mixed { $this->checkOrInitializePath($path); /** @noinspection UnserializeExploitsInspection */ - return unserialize($this->path->checkIsReadable()->readToString()); + $data = unserialize($this->path->checkIsReadable()->readToString()); + + if ($class && (!class_exists($class) || !($data instanceof $class))) { + throw new InvalidClassException( + sprintf(__u('Either class does not exist or file data cannot unserialized into: %s'), $class) + ); + } + + return $data; } /** @@ -50,10 +62,8 @@ class FileCache extends FileCacheBase $this->checkOrInitializePath($path); $this->createPath(); - $this->path->checkIsWritable(); - $this->path->open('wb', true); - $this->path->write(serialize($data)); - $this->path->close(); + $this->path->checkIsWritable()->open('wb', true); + $this->path->write(serialize($data))->close(); return $this; } diff --git a/lib/SP/Infrastructure/File/FileCacheInterface.php b/lib/SP/Infrastructure/File/FileCacheInterface.php index 7f925308..539f340e 100644 --- a/lib/SP/Infrastructure/File/FileCacheInterface.php +++ b/lib/SP/Infrastructure/File/FileCacheInterface.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -32,14 +32,19 @@ namespace SP\Infrastructure\File; interface FileCacheInterface { /** - * @param string|null $path + * Load file data unserializing the data + * + * @param string|null $path The path to the file + * @param string|null $class The class to unserialize the data * * @return mixed * @throws \SP\Infrastructure\File\FileException */ - public function load(?string $path = null): mixed; + public function load(?string $path = null, ?string $class = null): mixed; /** + * Save file data serializing the data + * * @throws FileException */ public function save(mixed $data, ?string $path = null): FileCacheInterface; diff --git a/tests/SP/Core/Crypt/SecureKeyCookieTest.php b/tests/SP/Core/Crypt/SecureKeyCookieTest.php deleted file mode 100644 index d6c352ae..00000000 --- a/tests/SP/Core/Crypt/SecureKeyCookieTest.php +++ /dev/null @@ -1,101 +0,0 @@ -. - */ - -namespace SP\Tests\Core\Crypt; - -use Defuse\Crypto\Exception\CryptoException; -use Defuse\Crypto\Exception\EnvironmentIsBrokenException; -use Defuse\Crypto\Key; -use Faker\Factory; -use PHPUnit\Framework\TestCase; -use SP\Core\Crypt\SecureKeyCookie; -use SP\Http\Request; - -/** - * Class SecureKeyCookieTest - * - * @package SP\Tests\SP\Core\Crypt - */ -class SecureKeyCookieTest extends TestCase -{ - const FAKE_PROVIDERS = ['text', 'email', 'password', 'url', 'ipv4', 'ipv6', 'creditCardDetails']; - - /** - * @var SecureKeyCookie - */ - protected $cookie; - - public function testGetCookieData() - { - $faker = Factory::create(); - - $_SERVER['HTTP_USER_AGENT'] = $faker->userAgent; - - $cypher = $this->cookie->getCypher(); - - foreach (self::FAKE_PROVIDERS as $provider) { - $text = $faker->$provider; - - if (!is_scalar($text)) { - $text = serialize($text); - } - - $data = $this->cookie->sign($text, $cypher); - - $this->assertNotEmpty($data); - $this->assertStringContainsString(';', $data); - $this->assertEquals($text, $this->cookie->getCookieData($data, $cypher)); - } - } - - /** - * @throws CryptoException - * @throws EnvironmentIsBrokenException - */ - public function testGetKey() - { - $_COOKIE[SecureKeyCookie::COOKIE_NAME] = $this->cookie->sign($this->cookie->generateSecuredData()->getSerialized(), $this->cookie->getCypher()); - - $this->assertNotEmpty($this->cookie->getKey()); - $this->assertInstanceOf(Key::class, $this->cookie->getSecuredKey()); - } - - /** - * testSaveKey - */ - public function testSaveKey() - { - $this->assertTrue($this->cookie->saveKey()); - $this->assertInstanceOf(Key::class, $this->cookie->getSecuredKey()); - } - - /** - * Sets up the fixture, for example, open a network connection. - * This method is called before a test is executed. - */ - protected function setUp(): void - { - $this->cookie = SecureKeyCookie::factory(new Request(\Klein\Request::createFromGlobals())); - } -} diff --git a/tests/SP/Domain/Crypt/Services/SecureSessionServiceTest.php b/tests/SP/Domain/Crypt/Services/SecureSessionServiceTest.php new file mode 100644 index 00000000..31399283 --- /dev/null +++ b/tests/SP/Domain/Crypt/Services/SecureSessionServiceTest.php @@ -0,0 +1,206 @@ +. + */ + +namespace SP\Tests\Domain\Crypt\Services; + +use Defuse\Crypto\Key; +use Exception; +use PHPUnit\Framework\MockObject\MockObject; +use SP\Core\Crypt\Crypt; +use SP\Core\Crypt\CryptInterface; +use SP\Core\Crypt\RequestBasedPasswordInterface; +use SP\Core\Crypt\UUIDCookie; +use SP\Core\Crypt\Vault; +use SP\Domain\Common\Services\ServiceException; +use SP\Domain\Crypt\Services\SecureSessionService; +use SP\Infrastructure\File\FileCacheInterface; +use SP\Infrastructure\File\FileException; +use SP\Tests\UnitaryTestCase; + +/** + * Class SecureSessionServiceTest + * + * @group unitary + */ +class SecureSessionServiceTest extends UnitaryTestCase +{ + private SecureSessionService $secureSessionService; + private RequestBasedPasswordInterface|MockObject $requestBasedPassword; + private CryptInterface|MockObject $crypt; + private FileCacheInterface|MockObject $fileCache; + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException + * @throws \SP\Core\Exceptions\CryptException + */ + public function testGetKey() + { + $securedKey = Key::createNewRandomKey(); + $key = self::$faker->password; + $vault = Vault::factory($this->crypt)->saveData($securedKey->saveToAsciiSafeString(), $key); + + $this->fileCache->expects(self::once())->method('isExpired')->willReturn(false); + $this->fileCache->expects(self::once())->method('load')->willReturn($vault); + $this->requestBasedPassword->expects(self::once())->method('build')->willReturn($key); + + $this->assertInstanceOf(Key::class, $this->secureSessionService->getKey()); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + */ + public function testGetKeyCacheExpired() + { + $this->fileCache->expects(self::once())->method('isExpired')->willReturn(true); + $this->fileCache->expects(self::once())->method('save'); + $this->requestBasedPassword->expects(self::once())->method('build')->willReturn(self::$faker->password); + + $this->assertInstanceOf(Key::class, $this->secureSessionService->getKey()); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + */ + public function testGetKeyFileErrorCheckingExpire() + { + $this->fileCache->expects(self::once())->method('isExpired')->willThrowException(new FileException('test')); + $this->fileCache->expects(self::once())->method('save'); + $this->requestBasedPassword->expects(self::once())->method('build')->willReturn(self::$faker->password); + + $this->assertInstanceOf(Key::class, $this->secureSessionService->getKey()); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + */ + public function testGetKeyFileErrorLoading() + { + $this->fileCache->expects(self::once())->method('isExpired')->willReturn(false); + $this->fileCache->expects(self::once())->method('load')->willThrowException(new FileException('test')); + $this->fileCache->expects(self::once())->method('save'); + $this->requestBasedPassword->expects(self::once())->method('build')->willReturn(self::$faker->password); + + $this->assertInstanceOf(Key::class, $this->secureSessionService->getKey()); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + */ + public function testGetKeyFileErrorSaving() + { + $this->fileCache->expects(self::once())->method('isExpired')->willReturn(true); + $this->fileCache->expects(self::once())->method('save')->willThrowException(new FileException('test')); + $this->requestBasedPassword->expects(self::once())->method('build')->willReturn(self::$faker->password); + + $this->assertFalse($this->secureSessionService->getKey()); + } + + /** + * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException + * @throws \SP\Core\Exceptions\CryptException + */ + public function testGetKeyBuildPasswordException() + { + $securedKey = Key::createNewRandomKey(); + $key = self::$faker->password; + $vault = Vault::factory($this->crypt)->saveData($securedKey->saveToAsciiSafeString(), $key); + + $this->fileCache->expects(self::once())->method('isExpired')->willReturn(false); + $this->fileCache->expects(self::once())->method('load')->willReturn($vault); + $this->requestBasedPassword->expects(self::once())->method('build')->willThrowException(new Exception()); + + $this->assertFalse($this->secureSessionService->getKey()); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + * @throws \SP\Domain\Common\Services\ServiceException + */ + public function testGetFileNameFrom() + { + $uuidCookie = $this->createMock(UUIDCookie::class); + + $uuidCookie->method('load') + ->willReturn(uniqid('', true)); + $uuidCookie->method('create') + ->willReturn(uniqid('', true)); + + $this->assertNotEmpty(SecureSessionService::getFileNameFrom($uuidCookie, self::$faker->password)); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + * @throws \SP\Domain\Common\Services\ServiceException + */ + public function testGetFileNameFromErrorLoadingCookie() + { + $uuidCookie = $this->createMock(UUIDCookie::class); + + $uuidCookie->method('load')->willReturn(false); + + $this->expectException(ServiceException::class); + $this->expectExceptionMessage('Unable to get UUID for filename'); + + SecureSessionService::getFileNameFrom($uuidCookie, self::$faker->password); + } + + /** + * @throws \PHPUnit\Framework\MockObject\Exception + * @throws \SP\Domain\Common\Services\ServiceException + */ + public function testGetFileNameFromErrorCreatingCookie() + { + $uuidCookie = $this->createMock(UUIDCookie::class); + + $uuidCookie->method('create')->willReturn(false); + + $this->expectException(ServiceException::class); + $this->expectExceptionMessage('Unable to get UUID for filename'); + + SecureSessionService::getFileNameFrom($uuidCookie, self::$faker->password); + } + + /** + * @return void + * @throws \PHPUnit\Framework\MockObject\Exception + * @throws \SP\Core\Context\ContextException + */ + protected function setUp(): void + { + parent::setUp(); + + $this->crypt = new Crypt(); + $this->requestBasedPassword = $this->createMock(RequestBasedPasswordInterface::class); + $this->fileCache = $this->createMock(FileCacheInterface::class); + + $this->secureSessionService = new SecureSessionService( + $this->application, + $this->crypt, + $this->fileCache, + $this->requestBasedPassword + ); + } + +} diff --git a/tests/SP/Services/Crypt/SecureSessionServiceTest.php b/tests/SP/Services/Crypt/SecureSessionServiceTest.php deleted file mode 100644 index 8b7c7a98..00000000 --- a/tests/SP/Services/Crypt/SecureSessionServiceTest.php +++ /dev/null @@ -1,63 +0,0 @@ -. - */ - -namespace SP\Tests\Services\Crypt; - -use Defuse\Crypto\Key; -use DI\DependencyException; -use DI\NotFoundException; -use PHPUnit\Framework\TestCase; -use SP\Core\Context\ContextException; -use SP\Core\Crypt\UUIDCookie; -use SP\Domain\Crypt\Services\SecureSessionService; -use function SP\Tests\setupContext; - -/** - * Class SecureSessionServiceTest - * - * @package SP\Tests\Services\Crypt - */ -class SecureSessionServiceTest extends TestCase -{ - /** - * @throws DependencyException - * @throws NotFoundException - * @throws ContextException - */ - public function testGetKey() - { - $dic = setupContext(); - $service = $dic->get(SecureSessionService::class); - - $stub = $this->createMock(UUIDCookie::class); - - $stub->method('loadCookie') - ->willReturn(uniqid('', true)); - $stub->method('createCookie') - ->willReturn(uniqid('', true)); - - $this->assertInstanceOf(Key::class, $service->getKey($stub)); - $this->assertFileExists($service->getFilename()); - } -}