diff --git a/app/modules/web/Controllers/Bootstrap/GetEnvironmentController.php b/app/modules/web/Controllers/Bootstrap/GetEnvironmentController.php index dd168b40..64c2341b 100644 --- a/app/modules/web/Controllers/Bootstrap/GetEnvironmentController.php +++ b/app/modules/web/Controllers/Bootstrap/GetEnvironmentController.php @@ -28,6 +28,7 @@ use Exception; use SP\Core\Application; use SP\Core\Bootstrap\BootstrapBase; use SP\Core\Crypt\CryptPKI; +use SP\Core\Crypt\CryptPKIInterface; use SP\Domain\Import\Services\ImportService; use SP\Infrastructure\File\FileException; use SP\Modules\Web\Controllers\SimpleControllerBase; @@ -45,14 +46,14 @@ final class GetEnvironmentController extends SimpleControllerBase { use JsonTrait; - private CryptPKI $cryptPKI; + private CryptPKIInterface $cryptPKI; private PluginManager $pluginManager; private BrowserAuthInterface $browser; public function __construct( Application $application, SimpleControllerHelper $simpleControllerHelper, - CryptPKI $cryptPKI, + CryptPKIInterface $cryptPKI, PluginManager $pluginManager, BrowserAuthInterface $browser ) { @@ -178,4 +179,4 @@ final class GetEnvironmentController extends SimpleControllerBase return $this->session->getCSRF(); } -} \ No newline at end of file +} diff --git a/app/modules/web/Controllers/Helpers/LayoutHelper.php b/app/modules/web/Controllers/Helpers/LayoutHelper.php index fca5963c..1e02ec88 100644 --- a/app/modules/web/Controllers/Helpers/LayoutHelper.php +++ b/app/modules/web/Controllers/Helpers/LayoutHelper.php @@ -29,7 +29,7 @@ use SP\Core\Acl\ActionsInterface; use SP\Core\AppInfoInterface; use SP\Core\Application; use SP\Core\Bootstrap\BootstrapBase; -use SP\Core\Crypt\CryptPKI; +use SP\Core\Crypt\CryptPKIInterface; use SP\Core\Exceptions\SPException; use SP\Core\Language; use SP\Core\UI\ThemeInterface; @@ -48,17 +48,17 @@ use SP\Util\VersionUtil; */ final class LayoutHelper extends HelperBase { - private ThemeInterface $theme; - private CryptPKI $cryptPKI; - private PluginManager $pluginManager; - private bool $loggedIn; + private ThemeInterface $theme; + private CryptPKIInterface $cryptPKI; + private PluginManager $pluginManager; + private bool $loggedIn; public function __construct( Application $application, TemplateInterface $template, RequestInterface $request, ThemeInterface $theme, - CryptPKI $cryptPKI, + CryptPKIInterface $cryptPKI, PluginManager $pluginManager ) { parent::__construct($application, $template, $request); @@ -71,7 +71,6 @@ final class LayoutHelper extends HelperBase $this->view->assign('loggedIn', $this->loggedIn); } - /** * Sets a full layout page * @@ -132,7 +131,6 @@ final class LayoutHelper extends HelperBase $this->view->assign('lang', $this->loggedIn ? Language::$userLang : substr(Language::$globalLang, 0, 2)); $this->view->assign('loadApp', $this->context->getAuthCompleted()); - try { // Cargar la clave pública en la sesión $this->context->setPublicKey($this->cryptPKI->getPublicKey()); @@ -416,4 +414,4 @@ final class LayoutHelper extends HelperBase return $this; } -} \ No newline at end of file +} diff --git a/lib/SP/Core/Crypt/CryptPKI.php b/lib/SP/Core/Crypt/CryptPKI.php index 01de6175..f5d9edd5 100644 --- a/lib/SP/Core/Crypt/CryptPKI.php +++ b/lib/SP/Core/Crypt/CryptPKI.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2021, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -24,35 +24,31 @@ namespace SP\Core\Crypt; -defined('APP_ROOT') || die(); - use phpseclib\Crypt\RSA; use SP\Core\Exceptions\SPException; use SP\Infrastructure\File\FileException; -use SP\Infrastructure\File\FileHandler; +use SP\Infrastructure\File\FileHandlerInterface; +use function SP\processException; /** * Class CryptPKI para el manejo de las funciones para PKI * * @package SP */ -class CryptPKI +final class CryptPKI implements CryptPKIInterface { - public const KEY_SIZE = 1024; - public const PUBLIC_KEY_FILE = CONFIG_PATH . DIRECTORY_SEPARATOR . 'pubkey.pem'; - public const PRIVATE_KEY_FILE = CONFIG_PATH . DIRECTORY_SEPARATOR . 'key.pem'; - - protected RSA $rsa; - private ?FileHandler $publicKeyFile = null; - private ?FileHandler $privateKeyFile = null; + public const KEY_SIZE = 1024; + public const PUBLIC_KEY_FILE = CONFIG_PATH.DIRECTORY_SEPARATOR.'pubkey.pem'; + public const PRIVATE_KEY_FILE = CONFIG_PATH.DIRECTORY_SEPARATOR.'key.pem'; /** * @throws SPException */ - public function __construct(RSA $rsa) - { - $this->rsa = $rsa; - + public function __construct( + private RSA $rsa, + private FileHandlerInterface $publicKeyFile, + private FileHandlerInterface $privateKeyFile + ) { $this->setUp(); } @@ -63,9 +59,6 @@ class CryptPKI */ private function setUp(): void { - $this->publicKeyFile = new FileHandler(self::PUBLIC_KEY_FILE); - $this->privateKeyFile = new FileHandler(self::PRIVATE_KEY_FILE); - try { $this->publicKeyFile->checkFileExists(); $this->privateKeyFile->checkFileExists(); @@ -86,9 +79,7 @@ class CryptPKI $keys = $this->rsa->createKey(self::KEY_SIZE); $this->publicKeyFile->save($keys['publickey']); - $this->privateKeyFile->save($keys['privatekey']); - - chmod(self::PRIVATE_KEY_FILE, 0600); + $this->privateKeyFile->save($keys['privatekey'])->chmod(0600); } public static function getMaxDataSize(): int @@ -96,19 +87,6 @@ class CryptPKI return (self::KEY_SIZE / 8) - 11; } - /** - * Encriptar datos con la clave pública - * - * @throws FileException - */ - public function encryptRSA(string $data): string - { - $this->rsa->setEncryptionMode(RSA::ENCRYPTION_PKCS1); - $this->rsa->loadKey($this->getPublicKey(), RSA::PUBLIC_FORMAT_PKCS1); - - return $this->rsa->encrypt($data); - } - /** * Devuelve la clave pública desde el archivo * @@ -116,9 +94,7 @@ class CryptPKI */ public function getPublicKey(): string { - return $this->publicKeyFile - ->checkFileExists() - ->readToString(); + return $this->publicKeyFile->checkFileExists()->readToString(); } /** @@ -135,25 +111,10 @@ class CryptPKI } /** - * Devuelve la clave privada desde el archivo - * - * @throws FileException + * @throws \SP\Infrastructure\File\FileException */ - public function getPrivateKey(): string + private function getPrivateKey(): string { - return $this->privateKeyFile - ->checkFileExists() - ->readToString(); + return $this->privateKeyFile->checkFileExists()->readToString(); } - - /** - * @throws FileException - */ - public function getKeySize(): int - { - $this->rsa->setEncryptionMode(RSA::ENCRYPTION_PKCS1); - $this->rsa->loadKey($this->getPrivateKey(), RSA::PRIVATE_FORMAT_PKCS1); - - return $this->rsa->getSize(); - } -} \ No newline at end of file +} diff --git a/lib/SP/Core/Crypt/CryptPKIInterface.php b/lib/SP/Core/Crypt/CryptPKIInterface.php new file mode 100644 index 00000000..e23aa103 --- /dev/null +++ b/lib/SP/Core/Crypt/CryptPKIInterface.php @@ -0,0 +1,56 @@ +. + */ + +namespace SP\Core\Crypt; + +use SP\Infrastructure\File\FileException; + +/** + * Class CryptPKI para el manejo de las funciones para PKI + * + * @package SP + */ +interface CryptPKIInterface +{ + /** + * Crea el par de claves pública y privada + * + * @throws FileException + */ + public function createKeys(): void; + + /** + * Devuelve la clave pública desde el archivo + * + * @throws FileException + */ + public function getPublicKey(): string; + + /** + * Desencriptar datos cifrados con la clave pública + * + * @throws FileException + */ + public function decryptRSA(string $data): ?string; +} diff --git a/lib/SP/Http/Request.php b/lib/SP/Http/Request.php index 1b94e562..35da4f27 100644 --- a/lib/SP/Http/Request.php +++ b/lib/SP/Http/Request.php @@ -27,7 +27,7 @@ namespace SP\Http; use Exception; use Klein\DataCollection\DataCollection; use Klein\DataCollection\HeaderDataCollection; -use SP\Core\Crypt\CryptPKI; +use SP\Core\Crypt\CryptPKIInterface; use SP\Core\Crypt\Hash; use SP\Core\Exceptions\SPException; use SP\Util\Filter; @@ -46,7 +46,7 @@ class Request implements RequestInterface public const SECURE_DIRS = ['css', 'js']; private \Klein\Request $request; - private CryptPKI $cryptPKI; + private CryptPKIInterface $cryptPKI; private HeaderDataCollection $headers; private DataCollection $params; private ?string $method = null; @@ -55,7 +55,7 @@ class Request implements RequestInterface /** * Request constructor. */ - public function __construct(\Klein\Request $request, CryptPKI $cryptPKI) + public function __construct(\Klein\Request $request, CryptPKIInterface $cryptPKI) { $this->request = $request; $this->cryptPKI = $cryptPKI; @@ -423,9 +423,9 @@ class Request implements RequestInterface // Check (deprecated) de facto standard if (!empty($forwardedHost) && !empty($forwardedProto)) { $data = [ - 'host' => trim(str_replace('"', '', $forwardedHost)), + 'host' => trim(str_replace('"', '', $forwardedHost)), 'proto' => trim(str_replace('"', '', $forwardedProto)), - 'for' => $this->getForwardedFor(), + 'for' => $this->getForwardedFor(), ]; // Check if protocol and host are not empty @@ -461,4 +461,4 @@ class Request implements RequestInterface { return (string)$this->request->server()->get($key, ''); } -} \ No newline at end of file +} diff --git a/lib/SP/Infrastructure/File/FileHandler.php b/lib/SP/Infrastructure/File/FileHandler.php index a400da8b..05cedf0d 100644 --- a/lib/SP/Infrastructure/File/FileHandler.php +++ b/lib/SP/Infrastructure/File/FileHandler.php @@ -25,6 +25,7 @@ namespace SP\Infrastructure\File; use SP\Util\Util; +use function SP\__; use function SP\logger; /** @@ -340,4 +341,19 @@ final class FileHandler implements FileHandlerInterface return filemtime($this->file) ?: 0; } + + /** + * @param int $permissions Octal permissions + * + * @return \SP\Infrastructure\File\FileHandlerInterface + * @throws \SP\Infrastructure\File\FileException + */ + public function chmod(int $permissions): FileHandlerInterface + { + if (chmod($this->file, $permissions) === false) { + throw new FileException(sprintf(__('Unable to set permissions for file (%s)'), $this->file)); + } + + return $this; + } } diff --git a/lib/SP/Infrastructure/File/FileHandlerInterface.php b/lib/SP/Infrastructure/File/FileHandlerInterface.php index 023c3331..fdc8f36c 100644 --- a/lib/SP/Infrastructure/File/FileHandlerInterface.php +++ b/lib/SP/Infrastructure/File/FileHandlerInterface.php @@ -24,7 +24,6 @@ namespace SP\Infrastructure\File; - /** * Class FileHandler * @@ -141,4 +140,14 @@ interface FileHandlerInterface * @throws FileException */ public function getFileTime(): int; -} \ No newline at end of file + + /** + * Changes file permissions + * + * @param int $permissions Octal permissions + * + * @return \SP\Infrastructure\File\FileHandlerInterface + * @throws \SP\Infrastructure\File\FileException + */ + public function chmod(int $permissions): FileHandlerInterface; +} diff --git a/tests/SP/Core/Crypt/CryptPKITest.php b/tests/SP/Core/Crypt/CryptPKITest.php index d4eb9fa2..5eb217a9 100644 --- a/tests/SP/Core/Crypt/CryptPKITest.php +++ b/tests/SP/Core/Crypt/CryptPKITest.php @@ -1,10 +1,10 @@ . + * along with sysPass. If not, see . */ namespace SP\Tests\Core\Crypt; -use Defuse\Crypto\Exception\EnvironmentIsBrokenException; use phpseclib\Crypt\RSA; -use PHPUnit\Framework\TestCase; +use PHPUnit\Framework\MockObject\MockObject; use SP\Core\Crypt\CryptPKI; use SP\Core\Exceptions\SPException; use SP\Infrastructure\File\FileException; -use SP\Util\PasswordUtil; +use SP\Infrastructure\File\FileHandlerInterface; +use SP\Tests\UnitaryTestCase; +use function PHPUnit\Framework\once; /** * Class CryptPKITest * * @package SP\Tests\SP\Core\Crypt */ -class CryptPKITest extends TestCase +class CryptPKITest extends UnitaryTestCase { - /** - * @var CryptPKI - */ - private $cryptPki; + private CryptPKI $cryptPki; + private RSA|MockObject $rsa; + private FileHandlerInterface|MockObject $privateKey; + private FileHandlerInterface|MockObject $publicKey; /** - * @throws EnvironmentIsBrokenException * @throws FileException */ public function testDecryptRSA() { - $length = (CryptPKI::KEY_SIZE / 8) - 11; + $data = self::$faker->sha1; + $privateKey = self::$faker->sha1; - $random = PasswordUtil::generateRandomBytes($length); + $this->privateKey->expects(once())->method('checkFileExists')->willReturnSelf(); + $this->privateKey->expects(once())->method('readToString')->willReturn($privateKey); + $this->rsa->expects(once())->method('setEncryptionMode')->with(RSA::ENCRYPTION_PKCS1); + $this->rsa->expects(once())->method('loadKey')->with($privateKey, RSA::PRIVATE_FORMAT_PKCS1); + $this->rsa->expects(once())->method('decrypt')->with('test')->willReturn($data); - $data = $this->cryptPki->encryptRSA($random); + $out = $this->cryptPki->decryptRSA('test'); - $this->assertNotEmpty($data); - - $this->assertEquals($random, $this->cryptPki->decryptRSA($data)); - - $this->assertNull($this->cryptPki->decryptRSA('test123')); - } - - /** - * @throws FileException - */ - public function testDecryptRSAPassword() - { - $length = (CryptPKI::KEY_SIZE / 8) - 11; - - $random = PasswordUtil::randomPassword($length); - - $data = $this->cryptPki->encryptRSA($random); - - $this->assertNotEmpty($data); - - $this->assertEquals($random, $this->cryptPki->decryptRSA($data)); - } - - /** - * @throws EnvironmentIsBrokenException - * @throws FileException - */ - public function testDecryptRSAWrongLength() - { - $length = ((CryptPKI::KEY_SIZE / 8) - 11) + 1; - - $random = PasswordUtil::generateRandomBytes($length); - - $data = $this->cryptPki->encryptRSA($random); - - $this->assertEquals($random, $this->cryptPki->decryptRSA($data)); + $this->assertEquals($data, $out); } /** @@ -99,68 +69,38 @@ class CryptPKITest extends TestCase */ public function testGetPublicKey() { - $key = $this->cryptPki->getPublicKey(); + $this->publicKey->expects(once())->method('checkFileExists')->willReturnSelf(); + $this->publicKey->expects(once())->method('readToString')->willReturn('test'); - $this->assertNotEmpty($key); + $out = $this->cryptPki->getPublicKey(); - $this->assertMatchesRegularExpression('/^-----BEGIN PUBLIC KEY-----.*/', $key); + $this->assertEquals('test', $out); } /** - * @throws FileException - */ - public function testGetPrivateKey() - { - $key = $this->cryptPki->getPrivateKey(); - - $this->assertNotEmpty($key); - - $this->assertMatchesRegularExpression('/^-----BEGIN RSA PRIVATE KEY-----.*/', $key); - } - - /** - * @throws EnvironmentIsBrokenException - * @throws FileException - */ - public function testEncryptRSA() - { - $length = (CryptPKI::KEY_SIZE / 8) - 11; - - $random = PasswordUtil::generateRandomBytes($length); - - $data = $this->cryptPki->encryptRSA($random); - - $this->assertNotEmpty($data); - - $this->assertEquals($random, $this->cryptPki->decryptRSA($data)); - - // Encrypt a long message - $random = PasswordUtil::generateRandomBytes(128); - - $data = $this->cryptPki->encryptRSA($random); - - $this->assertNotEmpty($data); - - $this->assertEquals($random, $this->cryptPki->decryptRSA($data)); - } - - /** - * @throws SPException + * @throws \SP\Core\Exceptions\SPException */ public function testCreateKeys() { - $this->cryptPki->createKeys(); + $this->publicKey->expects(once())->method('checkFileExists')->willReturnSelf(); + $this->privateKey->expects(once()) + ->method('checkFileExists') + ->willThrowException(new FileException('test')); - $this->assertFileExists(CryptPKI::PUBLIC_KEY_FILE); - $this->assertFileExists(CryptPKI::PRIVATE_KEY_FILE); + $keys = ['publickey' => self::$faker->sha1, 'privatekey' => self::$faker->sha1]; + + $this->rsa->expects(once())->method('createKey')->with(CryptPKI::KEY_SIZE)->willReturn($keys); + + $this->privateKey->expects(once())->method('save')->with($keys['privatekey'])->willReturnSelf(); + $this->privateKey->expects(once())->method('chmod')->with(0600); + $this->publicKey->expects(once())->method('save')->with($keys['publickey']); + + new CryptPKI($this->rsa, $this->publicKey, $this->privateKey); } - /** - * @throws FileException - */ - public function testGetKeySize() + public function testGetMaxDataSize() { - $this->assertEquals(CryptPKI::KEY_SIZE, $this->cryptPki->getKeySize()); + $this->assertEquals(117, CryptPKI::getMaxDataSize()); } /** @@ -171,18 +111,10 @@ class CryptPKITest extends TestCase */ protected function setUp(): void { - $this->cryptPki = new CryptPKI(new RSA()); + $this->rsa = $this->createMock(RSA::class); + $this->privateKey = $this->createMock(FileHandlerInterface::class); + $this->publicKey = $this->createMock(FileHandlerInterface::class); + + $this->cryptPki = new CryptPKI($this->rsa, $this->publicKey, $this->privateKey); } - - /** - * Tears down the fixture, for example, close a network connection. - * This method is called after a test is executed. - */ - protected function tearDown(): void - { - unlink(CryptPKI::PUBLIC_KEY_FILE); - unlink(CryptPKI::PRIVATE_KEY_FILE); - } - - }