diff --git a/3rdparty b/3rdparty index 23b76aa5251ad..5935defbd4b61 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit 23b76aa5251ada94e04b47ef2513b9f1a7c8e8ce +Subproject commit 5935defbd4b614d76f84ef357f28810957d1ee11 diff --git a/apps/encryption/lib/Crypto/Crypt.php b/apps/encryption/lib/Crypto/Crypt.php index 806b6b954b723..238db34b37967 100644 --- a/apps/encryption/lib/Crypto/Crypt.php +++ b/apps/encryption/lib/Crypto/Crypt.php @@ -17,7 +17,7 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IUserSession; -use phpseclib\Crypt\RC4; +use phpseclib3\Crypt\RC4; use Psr\Log\LoggerInterface; /** @@ -722,7 +722,6 @@ public function useLegacyBase64Encoding(): bool { */ private function rc4Decrypt(string $data, string $secret): string { $rc4 = new RC4(); - /** @psalm-suppress InternalMethod */ $rc4->setKey($secret); return $rc4->decrypt($data); @@ -733,7 +732,6 @@ private function rc4Decrypt(string $data, string $secret): string { */ private function rc4Encrypt(string $data, string $secret): string { $rc4 = new RC4(); - /** @psalm-suppress InternalMethod */ $rc4->setKey($secret); return $rc4->encrypt($data); diff --git a/apps/files_external/composer/composer/autoload_classmap.php b/apps/files_external/composer/composer/autoload_classmap.php index e8bf64ac1688d..6cb11dc66fe85 100644 --- a/apps/files_external/composer/composer/autoload_classmap.php +++ b/apps/files_external/composer/composer/autoload_classmap.php @@ -96,6 +96,7 @@ 'OCA\\Files_External\\Lib\\Storage\\OwnCloud' => $baseDir . '/../lib/Lib/Storage/OwnCloud.php', 'OCA\\Files_External\\Lib\\Storage\\SFTP' => $baseDir . '/../lib/Lib/Storage/SFTP.php', 'OCA\\Files_External\\Lib\\Storage\\SFTPReadStream' => $baseDir . '/../lib/Lib/Storage/SFTPReadStream.php', + 'OCA\\Files_External\\Lib\\Storage\\SFTPReflection' => $baseDir . '/../lib/Lib/Storage/SFTPReflection.php', 'OCA\\Files_External\\Lib\\Storage\\SFTPWriteStream' => $baseDir . '/../lib/Lib/Storage/SFTPWriteStream.php', 'OCA\\Files_External\\Lib\\Storage\\SMB' => $baseDir . '/../lib/Lib/Storage/SMB.php', 'OCA\\Files_External\\Lib\\Storage\\StreamWrapper' => $baseDir . '/../lib/Lib/Storage/StreamWrapper.php', diff --git a/apps/files_external/composer/composer/autoload_static.php b/apps/files_external/composer/composer/autoload_static.php index 9b0a1df75b051..132da2a2eff10 100644 --- a/apps/files_external/composer/composer/autoload_static.php +++ b/apps/files_external/composer/composer/autoload_static.php @@ -111,6 +111,7 @@ class ComposerStaticInitFiles_External 'OCA\\Files_External\\Lib\\Storage\\OwnCloud' => __DIR__ . '/..' . '/../lib/Lib/Storage/OwnCloud.php', 'OCA\\Files_External\\Lib\\Storage\\SFTP' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTP.php', 'OCA\\Files_External\\Lib\\Storage\\SFTPReadStream' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTPReadStream.php', + 'OCA\\Files_External\\Lib\\Storage\\SFTPReflection' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTPReflection.php', 'OCA\\Files_External\\Lib\\Storage\\SFTPWriteStream' => __DIR__ . '/..' . '/../lib/Lib/Storage/SFTPWriteStream.php', 'OCA\\Files_External\\Lib\\Storage\\SMB' => __DIR__ . '/..' . '/../lib/Lib/Storage/SMB.php', 'OCA\\Files_External\\Lib\\Storage\\StreamWrapper' => __DIR__ . '/..' . '/../lib/Lib/Storage/StreamWrapper.php', diff --git a/apps/files_external/lib/Controller/AjaxController.php b/apps/files_external/lib/Controller/AjaxController.php index 5e3ceb91842d8..64a9fe141d0e1 100644 --- a/apps/files_external/lib/Controller/AjaxController.php +++ b/apps/files_external/lib/Controller/AjaxController.php @@ -78,10 +78,15 @@ public function getApplicableEntities(string $pattern = '', ?int $limit = null, */ private function generateSshKeys($keyLength) { $key = $this->rsaMechanism->createKey($keyLength); - // Replace the placeholder label with a more meaningful one - $key['publickey'] = str_replace('phpseclib-generated-key', gethostname(), $key['publickey']); - - return $key; + return [ + 'private_key' => $key->toString('PKCS1'), + // Replace the placeholder label with a more meaningful one + 'public_key' => str_replace( + 'phpseclib-generated-key', + gethostname(), + $key->getPublicKey()->toString('OpenSSH'), + ), + ]; } /** @@ -93,10 +98,7 @@ private function generateSshKeys($keyLength) { public function getSshKeys($keyLength = 1024) { $key = $this->generateSshKeys($keyLength); return new JSONResponse([ - 'data' => [ - 'private_key' => $key['privatekey'], - 'public_key' => $key['publickey'] - ], + 'data' => $key, 'status' => 'success', ]); } diff --git a/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php b/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php index 807445f0e27d2..c55bb5c4ec3c7 100644 --- a/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php +++ b/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php @@ -14,7 +14,7 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IUser; -use phpseclib\Crypt\RSA as RSACrypt; +use phpseclib3\Crypt\RSA as RSACrypt; /** * RSA public key authentication @@ -45,15 +45,16 @@ public function __construct( */ #[\Override] public function manipulateStorageConfig(StorageConfig &$storage, ?IUser $user = null) { - $auth = new RSACrypt(); - $auth->setPassword($this->config->getSystemValue('secret', '')); - if (!$auth->loadKey($storage->getBackendOption('private_key'))) { + try { + $auth = RSACrypt::loadPrivateKey( + $storage->getBackendOption('private_key'), + $this->config->getSystemValue('secret', '') + ); + } catch (\Throwable) { // Add fallback routine for a time where secret was not enforced to be exists - $auth->setPassword(''); - if (!$auth->loadKey($storage->getBackendOption('private_key'))) { - throw new \RuntimeException('unable to load private key'); - } + $auth = RSACrypt::loadPrivateKey($storage->getBackendOption('private_key')); } + $storage->setBackendOption('public_key_auth', $auth); } @@ -61,17 +62,14 @@ public function manipulateStorageConfig(StorageConfig &$storage, ?IUser $user = * Generate a keypair * * @param int $keyLenth - * @return array ['privatekey' => $privateKey, 'publickey' => $publicKey] */ - public function createKey($keyLength) { - $rsa = new RSACrypt(); - $rsa->setPublicKeyFormat(RSACrypt::PUBLIC_FORMAT_OPENSSH); - $rsa->setPassword($this->config->getSystemValue('secret', '')); - + public function createKey($keyLength): RSACrypt\PrivateKey { if ($keyLength !== 1024 && $keyLength !== 2048 && $keyLength !== 4096) { $keyLength = 1024; } - return $rsa->createKey($keyLength); + $secret = $this->config->getSystemValue('secret', ''); + $rsa = RSACrypt::createKey($keyLength); + return $rsa->withPassword($secret); } } diff --git a/apps/files_external/lib/Lib/Auth/PublicKey/RSAPrivateKey.php b/apps/files_external/lib/Lib/Auth/PublicKey/RSAPrivateKey.php index a954467ebe96c..16260c0d074f2 100644 --- a/apps/files_external/lib/Lib/Auth/PublicKey/RSAPrivateKey.php +++ b/apps/files_external/lib/Lib/Auth/PublicKey/RSAPrivateKey.php @@ -13,7 +13,8 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IUser; -use phpseclib\Crypt\RSA as RSACrypt; +use phpseclib3\Crypt\RSA; +use phpseclib3\Exception\NoKeyLoadedException; /** * RSA public key authentication @@ -42,14 +43,18 @@ public function __construct( */ #[\Override] public function manipulateStorageConfig(StorageConfig &$storage, ?IUser $user = null) { - $auth = new RSACrypt(); - $auth->setPassword($this->config->getSystemValue('secret', '')); - if (!$auth->loadKey($storage->getBackendOption('private_key'))) { + + try { + $auth = RSA\PrivateKey::loadPrivateKey( + $storage->getBackendOption('private_key'), + $this->config->getSystemValue('secret', ''), + ); + } catch (NoKeyLoadedException) { // Add fallback routine for a time where secret was not enforced to be exists - $auth->setPassword(''); - if (!$auth->loadKey($storage->getBackendOption('private_key'))) { - throw new \RuntimeException('unable to load private key'); - } + $auth = RSA\PrivateKey::loadPrivateKey( + $storage->getBackendOption('private_key'), + '', + ); } $storage->setBackendOption('public_key_auth', $auth); } diff --git a/apps/files_external/lib/Lib/Storage/SFTP.php b/apps/files_external/lib/Lib/Storage/SFTP.php index f2b8924673a61..67a38f7019fd3 100644 --- a/apps/files_external/lib/Lib/Storage/SFTP.php +++ b/apps/files_external/lib/Lib/Storage/SFTP.php @@ -19,7 +19,7 @@ use OCP\Files\FileInfo; use OCP\Files\IMimeTypeDetector; use OCP\Server; -use phpseclib\Net\SFTP\Stream; +use phpseclib3\Net\SFTP\Stream; /** * Uses phpseclib's Net\SFTP class and the Net\SFTP\Stream stream wrapper to @@ -34,7 +34,7 @@ class SFTP extends Common { private $auth = []; /** - * @var \phpseclib\Net\SFTP + * @var \phpseclib3\Net\SFTP */ protected $client; private CappedMemoryCache $knownMTimes; @@ -106,16 +106,16 @@ public function __construct(array $parameters) { /** * Returns the connection. * - * @return \phpseclib\Net\SFTP connected client instance + * @return \phpseclib3\Net\SFTP connected client instance * @throws \Exception when the connection failed */ - public function getConnection(): \phpseclib\Net\SFTP { + public function getConnection(): \phpseclib3\Net\SFTP { if (!is_null($this->client)) { return $this->client; } $hostKeys = $this->readHostKeys(); - $this->client = new \phpseclib\Net\SFTP($this->host, $this->port); + $this->client = new \phpseclib3\Net\SFTP($this->host, $this->port); // The SSH Host Key MUST be verified before login(). $currentHostKey = $this->client->getServerPublicHostKey(); @@ -130,7 +130,6 @@ public function getConnection(): \phpseclib\Net\SFTP { $login = false; foreach ($this->auth as $auth) { - /** @psalm-suppress TooManyArguments */ $login = $this->client->login($this->user, $auth); if ($login === true) { break; @@ -338,7 +337,7 @@ public function fopen(string $path, string $mode) { case 'wb': SFTPWriteStream::register(); // the SFTPWriteStream doesn't go through the "normal" methods so it doesn't clear the stat cache. - $connection->_remove_from_stat_cache($absPath); + $connection->clearStatCache(); $context = stream_context_create(['sftp' => ['session' => $connection]]); $fh = fopen('sftpwrite://' . trim($absPath, '/'), 'w', false, $context); if ($fh) { @@ -487,7 +486,7 @@ public function copy(string $source, string $target): bool { $absTarget = $this->absPath($target); $connection = $this->getConnection(); - $size = $connection->size($absSource); + $size = $connection->filesize($absSource); if ($size === false) { return false; } @@ -498,7 +497,7 @@ public function copy(string $source, string $target): bool { return false; } /** @psalm-suppress InternalMethod */ - if (!$connection->put($absTarget, $chunk, \phpseclib\Net\SFTP::SOURCE_STRING, $i)) { + if (!$connection->put($absTarget, $chunk, \phpseclib3\Net\SFTP::SOURCE_STRING, $i)) { return false; } } diff --git a/apps/files_external/lib/Lib/Storage/SFTPReadStream.php b/apps/files_external/lib/Lib/Storage/SFTPReadStream.php index 5007c251a47eb..0547bf25c59a2 100644 --- a/apps/files_external/lib/Lib/Storage/SFTPReadStream.php +++ b/apps/files_external/lib/Lib/Storage/SFTPReadStream.php @@ -10,13 +10,15 @@ namespace OCA\Files_External\Lib\Storage; use Icewind\Streams\File; -use phpseclib\Net\SSH2; +use phpseclib3\Net\SSH2; class SFTPReadStream implements File { + use SFTPReflection; + /** @var resource */ public $context; - /** @var \phpseclib\Net\SFTP */ + /** @var \phpseclib3\Net\SFTP */ private $sftp; /** @var string */ @@ -54,7 +56,7 @@ protected function loadContext(string $name) { } else { throw new \BadMethodCallException('Invalid context, "' . $name . '" options not set'); } - if (isset($context['session']) && $context['session'] instanceof \phpseclib\Net\SFTP) { + if (isset($context['session']) && $context['session'] instanceof \phpseclib3\Net\SFTP) { $this->sftp = $context['session']; } else { throw new \BadMethodCallException('Invalid context, session not set'); @@ -73,27 +75,25 @@ public function stream_open($path, $mode, $options, &$opened_path) { $this->loadContext('sftp'); - if (!($this->sftp->bitmap & SSH2::MASK_LOGIN)) { + if (!($this->getSftpProperty($this->sftp, 'bitmap') & SSH2::MASK_LOGIN)) { return false; } - $remote_file = $this->sftp->_realpath($path); + $remote_file = $this->sftp->realpath($path); if ($remote_file === false) { return false; } $packet = pack('Na*N2', strlen($remote_file), $remote_file, NET_SFTP_OPEN_READ, 0); - if (!$this->sftp->_send_sftp_packet(NET_SFTP_OPEN, $packet)) { - return false; - } + $this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_OPEN, $packet]); - $response = $this->sftp->_get_sftp_packet(); - switch ($this->sftp->packet_type) { + $response = $this->invokeSftp($this->sftp, 'get_sftp_packet'); + switch ($this->getSftpProperty($this->sftp, 'packet_type')) { case NET_SFTP_HANDLE: $this->handle = substr($response, 4); break; case NET_SFTP_STATUS: // presumably SSH_FX_NO_SUCH_FILE or SSH_FX_PERMISSION_DENIED - $this->sftp->_logError($response); + $this->invokeSftp($this->sftp, 'logError', [$response]); return false; default: user_error('Expected SSH_FXP_HANDLE or SSH_FXP_STATUS'); @@ -152,19 +152,24 @@ public function stream_read($count) { private function request_chunk(int $size) { if ($this->pendingRead) { - $this->sftp->_get_sftp_packet(); + $this->invokeSftp($this->sftp, 'get_sftp_packet'); } $packet = pack('Na*N3', strlen($this->handle), $this->handle, $this->internalPosition / 4294967296, $this->internalPosition, $size); $this->pendingRead = true; - return $this->sftp->_send_sftp_packet(NET_SFTP_READ, $packet); + try { + $this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_READ, $packet]); + return true; + } catch (\Throwable) { + return false; + } } private function read_chunk() { $this->pendingRead = false; - $response = $this->sftp->_get_sftp_packet(); + $response = $this->invokeSftp($this->sftp, 'get_sftp_packet'); - switch ($this->sftp->packet_type) { + switch ($this->getSftpProperty($this->sftp, 'packet_type')) { case NET_SFTP_DATA: $temp = substr($response, 4); $len = strlen($temp); @@ -220,9 +225,9 @@ public function stream_eof() { public function stream_close() { // we still have a read request incoming that needs to be handled before we can close if ($this->pendingRead) { - $this->sftp->_get_sftp_packet(); + $this->invokeSftp($this->sftp, 'get_sftp_packet'); } - if (!$this->sftp->_close_handle($this->handle)) { + if (!$this->invokeSftp($this->sftp, 'close_handle', [$this->handle])) { return false; } return true; diff --git a/apps/files_external/lib/Lib/Storage/SFTPReflection.php b/apps/files_external/lib/Lib/Storage/SFTPReflection.php new file mode 100644 index 0000000000000..830d28351dd24 --- /dev/null +++ b/apps/files_external/lib/Lib/Storage/SFTPReflection.php @@ -0,0 +1,47 @@ + */ + private static array $sftpReflectionMethods = []; + /** @var array */ + private static array $sftpReflectionProperties = []; + + /** + * Invoke a method that is private in phpseclib v3. + */ + private function invokeSftp(SFTP $sftp, string $method, array $arguments = []): mixed { + self::$sftpReflectionMethods[$method] ??= new \ReflectionMethod(SFTP::class, $method); + return self::$sftpReflectionMethods[$method]->invokeArgs($sftp, $arguments); + } + + /** + * Read a property that is private/protected in phpseclib v3. + */ + private function getSftpProperty(SFTP $sftp, string $property): mixed { + self::$sftpReflectionProperties[$property] ??= new \ReflectionProperty(SFTP::class, $property); + return self::$sftpReflectionProperties[$property]->getValue($sftp); + } +} diff --git a/apps/files_external/lib/Lib/Storage/SFTPWriteStream.php b/apps/files_external/lib/Lib/Storage/SFTPWriteStream.php index 4673573589047..d9e63650b2ab4 100644 --- a/apps/files_external/lib/Lib/Storage/SFTPWriteStream.php +++ b/apps/files_external/lib/Lib/Storage/SFTPWriteStream.php @@ -10,13 +10,15 @@ namespace OCA\Files_External\Lib\Storage; use Icewind\Streams\File; -use phpseclib\Net\SSH2; +use phpseclib3\Net\SSH2; class SFTPWriteStream implements File { + use SFTPReflection; + /** @var resource */ public $context; - /** @var \phpseclib\Net\SFTP */ + /** @var \phpseclib3\Net\SFTP */ private $sftp; /** @var string */ @@ -54,7 +56,7 @@ protected function loadContext(string $name) { } else { throw new \BadMethodCallException('Invalid context, "' . $name . '" options not set'); } - if (isset($context['session']) && $context['session'] instanceof \phpseclib\Net\SFTP) { + if (isset($context['session']) && $context['session'] instanceof \phpseclib3\Net\SFTP) { $this->sftp = $context['session']; } else { throw new \BadMethodCallException('Invalid context, session not set'); @@ -70,11 +72,11 @@ public function stream_open($path, $mode, $options, &$opened_path) { $this->loadContext('sftp'); - if (!($this->sftp->bitmap & SSH2::MASK_LOGIN)) { + if (!($this->getSftpProperty($this->sftp, 'bitmap') & SSH2::MASK_LOGIN)) { return false; } - $remote_file = $this->sftp->_realpath($path); + $remote_file = $this->sftp->realpath($path); $this->path = $remote_file; if ($remote_file === false) { @@ -82,17 +84,19 @@ public function stream_open($path, $mode, $options, &$opened_path) { } $packet = pack('Na*N2', strlen($remote_file), $remote_file, NET_SFTP_OPEN_WRITE | NET_SFTP_OPEN_CREATE | NET_SFTP_OPEN_TRUNCATE, 0); - if (!$this->sftp->_send_sftp_packet(NET_SFTP_OPEN, $packet)) { + try { + $this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_OPEN, $packet]); + } catch (\Throwable) { return false; } - $response = $this->sftp->_get_sftp_packet(); - switch ($this->sftp->packet_type) { + $response = $this->invokeSftp($this->sftp, 'get_sftp_packet'); + switch ($this->getSftpProperty($this->sftp, 'packet_type')) { case NET_SFTP_HANDLE: $this->handle = substr($response, 4); break; case NET_SFTP_STATUS: // presumably SSH_FX_NO_SUCH_FILE or SSH_FX_PERMISSION_DENIED - $this->sftp->_logError($response); + $this->invokeSftp($this->sftp, 'logError', [$response]); return false; default: user_error('Expected SSH_FXP_HANDLE or SSH_FXP_STATUS'); @@ -157,13 +161,15 @@ public function stream_lock($operation) { public function stream_flush() { $size = strlen($this->buffer); $packet = pack('Na*N3a*', strlen($this->handle), $this->handle, $this->internalPosition / 4294967296, $this->internalPosition, $size, $this->buffer); - if (!$this->sftp->_send_sftp_packet(NET_SFTP_WRITE, $packet)) { + try { + $this->invokeSftp($this->sftp, 'send_sftp_packet', [NET_SFTP_WRITE, $packet]); + } catch (\Throwable) { return false; } $this->internalPosition += $size; $this->buffer = ''; - return $this->sftp->_read_put_responses(1); + return $this->invokeSftp($this->sftp, 'read_put_responses', [1]); } #[\Override] @@ -174,7 +180,7 @@ public function stream_eof() { #[\Override] public function stream_close() { $this->stream_flush(); - if (!$this->sftp->_close_handle($this->handle)) { + if (!$this->invokeSftp($this->sftp, 'close_handle', [$this->handle])) { return false; } $this->sftp->touch($this->path, time(), time()); diff --git a/apps/files_external/lib/Service/EncryptionService.php b/apps/files_external/lib/Service/EncryptionService.php index c36f6d0613269..6a36cdad4e231 100644 --- a/apps/files_external/lib/Service/EncryptionService.php +++ b/apps/files_external/lib/Service/EncryptionService.php @@ -12,7 +12,7 @@ use OCP\IConfig; use OCP\Security\ISecureRandom; -use phpseclib\Crypt\AES; +use phpseclib3\Crypt\AES; class EncryptionService { public function __construct( @@ -78,8 +78,28 @@ private function decryptPassword(string $encryptedPassword): string { * Returns the encryption cipher */ private function getCipher(): AES { - $cipher = new AES(AES::MODE_CBC); - $cipher->setKey($this->config->getSystemValue('passwordsalt', null)); + $cipher = new AES('cbc'); + $cipher->setKey($this->normalizeKey((string)$this->config->getSystemValue('passwordsalt', ''))); return $cipher; } + + /** + * Normalize the configured `passwordsalt` into a valid AES key. + * + * Note: phpseclib v2 accepted keys of any length and silently normalized them: + * the key length was rounded up to the next valid AES size (16, 24 or 32 + * bytes), the key was read in whole 4-byte words (trailing bytes that did + * not form a full word were dropped) and any missing high words were + * treated as zero. phpseclib v3 rejects keys that are not exactly 16, 24 or + * 32 bytes, so we reproduce the v2 behaviour here to keep previously stored + * passwords decryptable. + */ + private function normalizeKey(string $key): string { + $length = strlen($key); + $keyLength = $length <= 16 ? 16 : ($length <= 24 ? 24 : 32); + // Drop trailing bytes that do not form a full 4-byte word (phpseclib v2 used unpack('N*')) + $key = substr($key, 0, intdiv($length, 4) * 4); + // Zero-pad missing high words and truncate to the target key length + return substr(str_pad($key, $keyLength, "\0"), 0, $keyLength); + } } diff --git a/apps/files_external/tests/Controller/AjaxControllerTest.php b/apps/files_external/tests/Controller/AjaxControllerTest.php index b186c48b29ced..510fd886c751c 100644 --- a/apps/files_external/tests/Controller/AjaxControllerTest.php +++ b/apps/files_external/tests/Controller/AjaxControllerTest.php @@ -21,6 +21,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; +use phpseclib3\Crypt\RSA as CryptRSA; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -37,8 +38,12 @@ class AjaxControllerTest extends TestCase { protected function setUp(): void { $this->request = $this->createMock(IRequest::class); - $this->rsa = $this->createMock(RSA::class); - $this->globalAuth = $this->createMock(GlobalAuth::class); + $this->rsa = $this->getMockBuilder(RSA::class) + ->disableOriginalConstructor() + ->getMock(); + $this->globalAuth = $this->getMockBuilder(GlobalAuth::class) + ->disableOriginalConstructor() + ->getMock(); $this->userSession = $this->createMock(IUserSession::class); $this->groupManager = $this->createMock(IGroupManager::class); $this->userManager = $this->createMock(IUserManager::class); @@ -114,24 +119,32 @@ public function testGetApplicableEntitiesWithNoResults(): void { } public function testGetSshKeys(): void { + // phpseclib3's PrivateKey/PublicKey are final and cannot be mocked, so + // generate a real key pair and assert the controller serialises it. + $privateKey = CryptRSA::createKey(1024); + $this->rsa ->expects($this->once()) ->method('createKey') - ->willReturn([ - 'privatekey' => 'MyPrivateKey', - 'publickey' => 'MyPublicKey', - ]); - - $expected = new JSONResponse( - [ - 'data' => [ - 'private_key' => 'MyPrivateKey', - 'public_key' => 'MyPublicKey', - ], - 'status' => 'success', - ] + ->willReturn($privateKey); + + $response = $this->ajaxController->getSshKeys(); + + $this->assertInstanceOf(JSONResponse::class, $response); + $data = $response->getData(); + $this->assertSame('success', $data['status']); + $this->assertSame( + $privateKey->toString('PKCS1'), + $data['data']['private_key'], + ); + $this->assertSame( + str_replace( + 'phpseclib-generated-key', + gethostname(), + $privateKey->getPublicKey()->toString('OpenSSH'), + ), + $data['data']['public_key'], ); - $this->assertEquals($expected, $this->ajaxController->getSshKeys()); } public function testSaveGlobalCredentialsAsAdminForAnotherUser(): void { diff --git a/apps/files_external/tests/Service/EncryptionServiceTest.php b/apps/files_external/tests/Service/EncryptionServiceTest.php new file mode 100644 index 0000000000000..a9bffb539c3f0 --- /dev/null +++ b/apps/files_external/tests/Service/EncryptionServiceTest.php @@ -0,0 +1,82 @@ +config = $this->createMock(IConfig::class); + $this->secureRandom = $this->createMock(ISecureRandom::class); + } + + private function service(string $passwordSalt): EncryptionService { + $this->config->method('getSystemValue') + ->with('passwordsalt', '') + ->willReturn($passwordSalt); + return new EncryptionService($this->config, $this->secureRandom); + } + + public static function saltProvider(): array { + return [ + 'typical 30-char salt' => [str_repeat('a', 30)], + '16-char salt' => [str_repeat('b', 16)], + '24-char salt' => [str_repeat('c', 24)], + '32-char salt' => [str_repeat('d', 32)], + 'long 40-char salt' => [str_repeat('e', 40)], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('saltProvider')] + public function testEncryptDecryptRoundTrip(string $passwordSalt): void { + $this->secureRandom->method('generate')->willReturn(str_repeat("\x11", 16)); + $service = $this->service($passwordSalt); + + $options = $service->encryptPasswords(['password' => 'my-secret-password']); + $this->assertArrayHasKey('password_encrypted', $options); + $this->assertSame('', $options['password']); + + $decrypted = $service->decryptPasswords(['password_encrypted' => $options['password_encrypted']]); + $this->assertSame('my-secret-password', $decrypted['password']); + $this->assertArrayNotHasKey('password_encrypted', $decrypted); + } + + /** + * Ciphertexts written with phpseclib v2 must remain decryptable after the + * upgrade to phpseclib v3. v2 accepted arbitrary-length keys and normalized + * them (round up to 16/24/32 bytes, read whole 4-byte words only, zero-pad + * missing high words). Here we recreate such a ciphertext with OpenSSL — an + * independent AES implementation — using that exact effective key. + */ + #[\PHPUnit\Framework\Attributes\DataProvider('saltProvider')] + public function testDecryptsLegacyPhpseclibV2Ciphertext(string $passwordSalt): void { + $plaintext = 'legacy-external-storage-password'; + + // Reproduce the phpseclib v2 effective AES key + $length = strlen($passwordSalt); + $keyLength = $length <= 16 ? 16 : ($length <= 24 ? 24 : 32); + $effectiveKey = substr(str_pad(substr($passwordSalt, 0, intdiv($length, 4) * 4), $keyLength, "\0"), 0, $keyLength); + + $iv = str_repeat("\x22", 16); + $rawCiphertext = openssl_encrypt($plaintext, 'aes-' . ($keyLength * 8) . '-cbc', $effectiveKey, OPENSSL_RAW_DATA, $iv); + $stored = base64_encode($iv . $rawCiphertext); + + $service = $this->service($passwordSalt); + $decrypted = $service->decryptPasswords(['password_encrypted' => $stored]); + $this->assertSame($plaintext, $decrypted['password']); + } +} diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index dbd7cb5ac8be2..7e59e0d212c8e 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1603,12 +1603,6 @@ - - - - - - @@ -3939,11 +3933,6 @@ - - getDN(X509::DN_OPENSSL)['CN']]]> - getDN(X509::DN_OPENSSL)['CN']]]> - getDN(true)['CN']]]> - @@ -4087,16 +4076,6 @@ getDefaultCertificatesBundlePath())]]> - - - - - - - - - - diff --git a/core/Command/Integrity/SignApp.php b/core/Command/Integrity/SignApp.php index e4d61e8cc5b08..1c0aa18a41ad4 100644 --- a/core/Command/Integrity/SignApp.php +++ b/core/Command/Integrity/SignApp.php @@ -11,8 +11,8 @@ use OC\IntegrityCheck\Checker; use OC\IntegrityCheck\Helpers\FileAccessHelper; use OCP\IURLGenerator; -use phpseclib\Crypt\RSA; -use phpseclib\File\X509; +use phpseclib3\Crypt\RSA; +use phpseclib3\File\X509; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -71,8 +71,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - $rsa = new RSA(); - $rsa->loadKey($privateKey); + $rsa = RSA::loadPrivateKey($privateKey); $x509 = new X509(); $x509->loadX509($keyBundle); $x509->setPrivateKey($rsa); diff --git a/core/Command/Integrity/SignCore.php b/core/Command/Integrity/SignCore.php index 73b7e76b4ba18..b718b6c3a6f37 100644 --- a/core/Command/Integrity/SignCore.php +++ b/core/Command/Integrity/SignCore.php @@ -10,8 +10,8 @@ use OC\IntegrityCheck\Checker; use OC\IntegrityCheck\Helpers\FileAccessHelper; -use phpseclib\Crypt\RSA; -use phpseclib\File\X509; +use phpseclib3\Crypt\RSA; +use phpseclib3\File\X509; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -66,8 +66,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - $rsa = new RSA(); - $rsa->loadKey($privateKey); + $rsa = RSA::loadPrivateKey($privateKey); $x509 = new X509(); $x509->loadX509($keyBundle); $x509->setPrivateKey($rsa); diff --git a/lib/private/Installer.php b/lib/private/Installer.php index 5990e93277da3..b1e444f6b984a 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -31,7 +31,7 @@ use OCP\Migration\IOutput; use OCP\Server; use OCP\Util; -use phpseclib\File\X509; +use phpseclib3\File\X509; use Psr\Log\LoggerInterface; /** diff --git a/lib/private/IntegrityCheck/Checker.php b/lib/private/IntegrityCheck/Checker.php index 8425a96bff30a..3f3d135b90226 100644 --- a/lib/private/IntegrityCheck/Checker.php +++ b/lib/private/IntegrityCheck/Checker.php @@ -22,8 +22,8 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\ServerVersion; -use phpseclib\Crypt\RSA; -use phpseclib\File\X509; +use phpseclib3\Crypt\RSA; +use phpseclib3\File\X509; /** * Class Checker handles the code signing using X.509 and RSA. ownCloud ships with @@ -166,24 +166,27 @@ private function generateHashes(\RecursiveIteratorIterator $iterator, * * @param array $hashes * @param X509 $certificate - * @param RSA $privateKey + * @param RSA\PrivateKey $privateKey * @return array */ - private function createSignatureData(array $hashes, + private function createSignatureData( + array $hashes, X509 $certificate, - RSA $privateKey): array { + RSA\PrivateKey $privateKey, + ): array { ksort($hashes); - $privateKey->setSignatureMode(RSA::SIGNATURE_PSS); - $privateKey->setMGFHash('sha512'); - // See https://tools.ietf.org/html/rfc3447#page-38 - $privateKey->setSaltLength(0); - $signature = $privateKey->sign(json_encode($hashes)); + $signature = $privateKey + ->withPadding(RSA::SIGNATURE_PSS) + ->withHash('sha1') + ->withMGFHash('sha512') + ->withSaltLength(0) + ->sign(json_encode($hashes)); return [ 'hashes' => $hashes, 'signature' => base64_encode($signature), - 'certificate' => $certificate->saveX509($certificate->currentCert), + 'certificate' => $certificate->saveX509($certificate->getCurrentCert()), ]; } @@ -192,12 +195,12 @@ private function createSignatureData(array $hashes, * * @param string $path * @param X509 $certificate - * @param RSA $privateKey + * @param RSA\PrivateKey $privateKey * @throws \Exception */ public function writeAppSignature($path, X509 $certificate, - RSA $privateKey) { + RSA\PrivateKey $privateKey) { $appInfoDir = $path . '/appinfo'; try { $this->fileAccessHelper->assertDirectoryExists($appInfoDir); @@ -210,7 +213,7 @@ public function writeAppSignature($path, json_encode($signature, JSON_PRETTY_PRINT) ); } catch (\Exception $e) { - if (!$this->fileAccessHelper->is_writable($appInfoDir)) { + if ($this->fileAccessHelper->is_writable($appInfoDir) === false) { throw new \Exception($appInfoDir . ' is not writable'); } throw $e; @@ -221,12 +224,12 @@ public function writeAppSignature($path, * Write the signature of core * * @param X509 $certificate - * @param RSA $rsa + * @param RSA\PrivateKey $rsa * @param string $path * @throws \Exception */ public function writeCoreSignature(X509 $certificate, - RSA $rsa, + RSA\PrivateKey $rsa, $path) { $coreDir = $path . '/core'; try { @@ -290,15 +293,14 @@ private function verify(string $signaturePath, string $basePath, string $certifi $certificate = $signatureData['certificate']; // Check if certificate is signed by Nextcloud Root Authority - $x509 = new \phpseclib\File\X509(); + $x509 = new X509(); $rootCertificatePublicKey = $this->fileAccessHelper->file_get_contents($this->environmentHelper->getServerRoot() . '/resources/codesigning/root.crt'); $rootCerts = $this->splitCerts($rootCertificatePublicKey); foreach ($rootCerts as $rootCert) { $x509->loadCA($rootCert); } - $x509->loadX509($certificate); - if (!$x509->validateSignature()) { + if ($x509->loadX509($certificate) === false || !$x509->validateSignature()) { throw new InvalidSignatureException('Certificate is not valid.'); } // Verify if certificate has proper CN. "core" CN is always trusted. @@ -309,13 +311,19 @@ private function verify(string $signaturePath, string $basePath, string $certifi } // Check if the signature of the files is valid - $rsa = new \phpseclib\Crypt\RSA(); - $rsa->loadKey($x509->currentCert['tbsCertificate']['subjectPublicKeyInfo']['subjectPublicKey']); - $rsa->setSignatureMode(RSA::SIGNATURE_PSS); - $rsa->setMGFHash('sha512'); - // See https://tools.ietf.org/html/rfc3447#page-38 - $rsa->setSaltLength(0); - if (!$rsa->verify(json_encode($expectedHashes), $signature)) { + /** @var RSA\PublicKey|false */ + $rsa = $x509->getPublicKey(); + if ($rsa === false) { + throw new InvalidSignatureException('Certificate does not provide valid public RSA key.'); + } + + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withHash('sha1') + ->withMGFHash('sha512') + ->withSaltLength(0); + + if (!$rsa->verify(json_encode($expectedHashes), (string)$signature)) { throw new InvalidSignatureException('Signature could not get verified.'); } diff --git a/lib/private/Repair/NC25/AddMissingSecretJob.php b/lib/private/Repair/NC25/AddMissingSecretJob.php index 5dabe2577369a..eb9302a3bab4b 100644 --- a/lib/private/Repair/NC25/AddMissingSecretJob.php +++ b/lib/private/Repair/NC25/AddMissingSecretJob.php @@ -32,7 +32,7 @@ public function run(IOutput $output): void { $passwordSalt = $this->config->getSystemValueString('passwordsalt', ''); if ($passwordSalt === '') { try { - $this->config->setSystemValue('passwordsalt', $this->random->generate(30)); + $this->config->setSystemValue('passwordsalt', $this->random->generate(32)); } catch (HintException $e) { $output->warning('passwordsalt is missing from your config.php and your config.php is read only. Please fix it manually.'); } diff --git a/lib/private/Security/Crypto.php b/lib/private/Security/Crypto.php index 711e9f98e9ca1..8db44ecee00b6 100644 --- a/lib/private/Security/Crypto.php +++ b/lib/private/Security/Crypto.php @@ -12,8 +12,8 @@ use Exception; use OCP\IConfig; use OCP\Security\ICrypto; -use phpseclib\Crypt\AES; -use phpseclib\Crypt\Hash; +use phpseclib3\Crypt\AES; +use phpseclib3\Crypt\Hash; use SensitiveParameter; /** @@ -33,7 +33,7 @@ class Crypto implements ICrypto { public function __construct( private IConfig $config, ) { - $this->cipher = new AES(); + $this->cipher = new AES('cbc'); } /** @@ -77,7 +77,11 @@ public function encrypt( $password = $this->config->getSystemValueString('secret'); } $keyMaterial = hash_hkdf('sha512', $password); - $this->cipher->setPassword(substr($keyMaterial, 0, 32)); + // Pin the PBKDF2 parameters to phpseclib v2 defaults ('sha1' hash, 'phpseclib' salt). + // phpseclib v3 changed the default salt to 'phpseclib/salt', which would derive a + // different AES key and break decryption of previously stored ciphertexts. + // TODO: We should put our own salt into the HKDF derivation to avoid this dependency on phpseclib's defaults! + $this->cipher->setPassword(substr($keyMaterial, 0, 32), 'pbkdf2', 'sha1', 'phpseclib'); $iv = \random_bytes($this->ivLength); $this->cipher->setIV($iv); @@ -172,7 +176,9 @@ private function decryptWithoutSecret(string $authenticatedCiphertext, string $p $hmacKey = substr($keyMaterial, 32); } } - $this->cipher->setPassword($encryptionKey); + // Match the v2-compatible PBKDF2 parameters used in encrypt(), see the note there. + // TODO: We should put our own salt into the HKDF derivation to avoid this dependency on phpseclib's defaults! + $this->cipher->setPassword($encryptionKey, 'pbkdf2', 'sha1', 'phpseclib'); $this->cipher->setIV($iv); if ($isOwnCloudV2Migration) { diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 05a55a149fa2e..7f598f7abc843 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -57,7 +57,7 @@ use Psr\Log\LoggerInterface; class Setup { - public const MIN_PASSWORD_SALT_LENGTH = 30; + public const MIN_PASSWORD_SALT_LENGTH = 32; public const MIN_SECRET_LENGTH = 48; protected IL10N $l10n; diff --git a/tests/lib/IntegrityCheck/CheckerTest.php b/tests/lib/IntegrityCheck/CheckerTest.php index 6579c34c9b9e2..f86f2acc4eaac 100644 --- a/tests/lib/IntegrityCheck/CheckerTest.php +++ b/tests/lib/IntegrityCheck/CheckerTest.php @@ -19,26 +19,27 @@ use OCP\ICacheFactory; use OCP\IConfig; use OCP\ServerVersion; -use phpseclib\Crypt\RSA; -use phpseclib\File\X509; +use phpseclib3\Crypt\RSA; +use phpseclib3\File\X509; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class CheckerTest extends TestCase { - /** @var ServerVersion|\PHPUnit\Framework\MockObject\MockObject */ + /** @var ServerVersion&MockObject */ private $serverVersion; - /** @var EnvironmentHelper|\PHPUnit\Framework\MockObject\MockObject */ + /** @var EnvironmentHelper&MockObject */ private $environmentHelper; - /** @var FileAccessHelper|\PHPUnit\Framework\MockObject\MockObject */ + /** @var FileAccessHelper&MockObject */ private $fileAccessHelper; - /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IConfig&MockObject */ private $config; - /** @var IAppConfig|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IAppConfig&MockObject */ private $appConfig; - /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ + /** @var ICacheFactory&MockObject */ private $cacheFactory; - /** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IAppManager&MockObject */ private $appManager; - /** @var Detection|\PHPUnit\Framework\MockObject\MockObject */ + /** @var Detection&MockObject */ private $mimeTypeDetector; private Checker $checker; @@ -93,8 +94,12 @@ public function testWriteAppSignatureOfNotExistingApp(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeAppSignature('NotExistingApp', $x509, $rsa); @@ -111,8 +116,13 @@ public function testWriteAppSignatureWrongPermissions(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.key'); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeAppSignature(\OC::$SERVERROOT . '/tests/data/integritycheck/app/', $x509, $rsa); @@ -127,6 +137,14 @@ public function testWriteAppSignature(): void { "signature": "Y5yvXvcGHVPuRRatKVDUONWq1FpLXugZd6Km\/+aEHsQj7coVl9FeMj9OsWamBf7yRIw3dtNLguTLlAA9QAv\/b0uHN3JnbNZN+dwFOve4NMtqXfSDlWftqKN00VS+RJXpG1S2IIx9Poyp2NoghL\/5AuTv4GHiNb7zU\/DT\/kt71pUGPgPR6IIFaE+zHOD96vjYkrH+GfWZzKR0FCdLib9yyNvk+EGrcjKM6qjs2GKfS\/XFjj\/\/neDnh\/0kcPuKE3ZbofnI4TIDTv0CGqvOp7PtqVNc3Vy\/UKa7uF1PT0MAUKMww6EiMUSFZdUVP4WWF0Y72W53Qdtf1hrAZa2kfKyoK5kd7sQmCSKUPSU8978AUVZlBtTRlyT803IKwMV0iHMkw+xYB1sN2FlHup\/DESADqxhdgYuK35bCPvgkb4SBe4B8Voz\/izTvcP7VT5UvkYdAO+05\/jzdaHEmzmsD92CFfvX0q8O\/Y\/29ubftUJsqcHeMDKgcR4eZOE8+\/QVc\/89QO6WnKNuNuV+5bybO6g6PAdC9ZPsCvnihS61O2mwRXHLR3jv2UleFWm+lZEquPKtkhi6SLtDiijA4GV6dmS+dzujSLb7hGeD5o1plZcZ94uhWljl+QIp82+zU\/lYB1Zfr4Mb4e+V7r2gv7Fbv7y6YtjE2GIQwRhC5jq56bD0ZB+I=", "certificate": "-----BEGIN CERTIFICATE-----\r\nMIIEwTCCAqmgAwIBAgIUWv0iujufs5lUr0svCf\/qTQvoyKAwDQYJKoZIhvcNAQEF\r\nBQAwIzEhMB8GA1UECgwYb3duQ2xvdWQgQ29kZSBTaWduaW5nIENBMB4XDTE1MTEw\r\nMzIyNDk1M1oXDTE2MTEwMzIyNDk1M1owEjEQMA4GA1UEAwwHU29tZUFwcDCCAiIw\r\nDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBAK8q0x62agGSRBqeWsaeEwFfepMk\r\nF8cAobMMi50qHCv9IrOn\/ZH9l52xBrbIkErVmRjmly0d4JhD8Ymhidsh9ONKYl\/j\r\n+ishsZDM8eNNdp3Ew+fEYVvY1W7mR1qU24NWj0bzVsClI7hvPVIuw7AjfBDq1C5+\r\nA+ZSLSXYvOK2cEWjdxQfuNZwEZSjmA63DUllBIrm35IaTvfuyhU6BW9yHZxmb8+M\r\nw0xDv30D5UkE\/2N7Pa\/HQJLxCR+3zKibRK3nUyRDLSXxMkU9PnFNaPNX59VPgyj4\r\nGB1CFSToldJVPF4pzh7p36uGXZVxs8m3LFD4Ol8mhi7jkxDZjqFN46gzR0r23Py6\r\ndol9vfawGIoUwp9LvL0S7MvdRY0oazLXwClLP4OQ17zpSMAiCj7fgNT661JamPGj\r\nt5O7Zn2wA7I4ddDS\/HDTWCu98Zwc9fHIpsJPgCZ9awoqxi4Mnf7Pk9g5nnXhszGC\r\ncxxIASQKM+GhdzoRxKknax2RzUCwCzcPRtCj8AQT\/x\/mqN3PfRmlnFBNACUw9bpZ\r\nSOoNq2pCF9igftDWpSIXQ38pVpKLWowjjg3DVRmVKBgivHnUnVLyzYBahHPj0vaz\r\ntFtUFRaqXDnt+4qyUGyrT5h5pjZaTcHIcSB4PiarYwdVvgslgwnQzOUcGAzRWBD4\r\n6jV2brP5vFY3g6iPAgMBAAEwDQYJKoZIhvcNAQEFBQADggIBACTY3CCHC+Z28gCf\r\nFWGKQ3wAKs+k4+0yoti0qm2EKX7rSGQ0PHSas6uW79WstC4Rj+DYkDtIhGMSg8FS\r\nHVGZHGBCc0HwdX+BOAt3zi4p7Sf3oQef70\/4imPoKxbAVCpd\/cveVcFyDC19j1yB\r\nBapwu87oh+muoeaZxOlqQI4UxjBlR\/uRSMhOn2UGauIr3dWJgAF4pGt7TtIzt+1v\r\n0uA6FtN1Y4R5O8AaJPh1bIG0CVvFBE58esGzjEYLhOydgKFnEP94kVPgJD5ds9C3\r\npPhEpo1dRpiXaF7WGIV1X6DI\/ipWvfrF7CEy6I\/kP1InY\/vMDjQjeDnJ\/VrXIWXO\r\nyZvHXVaN\/m+1RlETsH7YO\/QmxRue9ZHN3gvvWtmpCeA95sfpepOk7UcHxHZYyQbF\r\n49\/au8j+5tsr4A83xzsT1JbcKRxkAaQ7WDJpOnE5O1+H0fB+BaLakTg6XX9d4Fo7\r\n7Gin7hVWX7pL+JIyxMzME3LhfI61+CRcqZQIrpyaafUziPQbWIPfEs7h8tCOWyvW\r\nUO8ZLervYCB3j44ivkrxPlcBklDCqqKKBzDP9dYOtS\/P4RB1NkHA9+NTvmBpTonS\r\nSFXdg9fFMD7VfjDE3Vnk+8DWkVH5wBYowTAD7w9Wuzr7DumiAULexnP\/Y7xwxLv7\r\n4B+pXTAcRK0zECDEaX3npS8xWzrB\r\n-----END CERTIFICATE-----" }'; + $this->fileAccessHelper + ->expects(self::any()) + ->method('is_writable') + ->with( + $this->equalTo(\OC::$SERVERROOT . '/tests/data/integritycheck/app//appinfo'), + ) + ->willReturn(true); + $this->fileAccessHelper ->expects($this->once()) ->method('file_put_contents') @@ -142,8 +160,12 @@ public function testWriteAppSignature(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeAppSignature(\OC::$SERVERROOT . '/tests/data/integritycheck/app/', $x509, $rsa); @@ -446,8 +468,12 @@ public function testWriteCoreSignatureWithException(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeCoreSignature($x509, $rsa, __DIR__); @@ -469,8 +495,12 @@ public function testWriteCoreSignatureWrongPermissions(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/SomeApp.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeCoreSignature($x509, $rsa, __DIR__); @@ -504,8 +534,12 @@ public function testWriteCoreSignature(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/core.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/core.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeCoreSignature($x509, $rsa, \OC::$SERVERROOT . '/tests/data/integritycheck/app/'); @@ -539,8 +573,12 @@ public function testWriteCoreSignatureWithUnmodifiedHtaccess(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/core.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/core.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeCoreSignature($x509, $rsa, \OC::$SERVERROOT . '/tests/data/integritycheck/htaccessUnmodified/'); @@ -569,8 +607,12 @@ public function testWriteCoreSignatureWithInvalidModifiedHtaccess(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/core.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/core.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeCoreSignature($x509, $rsa, \OC::$SERVERROOT . '/tests/data/integritycheck/htaccessWithInvalidModifiedContent/'); @@ -605,8 +647,12 @@ public function testWriteCoreSignatureWithValidModifiedHtaccess(): void { $keyBundle = file_get_contents(__DIR__ . '/../../data/integritycheck/core.crt'); $rsaPrivateKey = file_get_contents(__DIR__ . '/../../data/integritycheck/core.key'); - $rsa = new RSA(); - $rsa->loadKey($rsaPrivateKey); + $rsa = RSA::loadPrivateKey($rsaPrivateKey); + // After loading the key, always set the PSS padding and options: + $rsa = $rsa + ->withPadding(RSA::SIGNATURE_PSS) + ->withMGFHash('sha512') + ->withSaltLength(0); $x509 = new X509(); $x509->loadX509($keyBundle); $this->checker->writeCoreSignature($x509, $rsa, \OC::$SERVERROOT . '/tests/data/integritycheck/htaccessWithValidModifiedContent'); @@ -976,7 +1022,8 @@ public function testVerifyCoreWithDifferentScope(): void { } public function testRunInstanceVerification(): void { - $this->checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') + /** @var Checker&MockObject */ + $checker = $this->getMockBuilder('\OC\IntegrityCheck\Checker') ->setConstructorArgs([ $this->serverVersion, $this->environmentHelper, @@ -993,7 +1040,7 @@ public function testRunInstanceVerification(): void { ]) ->getMock(); - $this->checker + $checker ->expects($this->once()) ->method('verifyCoreSignature'); $this->appManager @@ -1020,7 +1067,7 @@ public function testRunInstanceVerification(): void { 'calendar', 'dav', ]; - $this->checker + $checker ->expects($this->exactly(3)) ->method('verifyAppSignature') ->willReturnCallback(function ($app) use (&$calls) { @@ -1047,7 +1094,7 @@ public function testRunInstanceVerification(): void { ->method('deleteKey') ->with('core', 'oc.integritycheck.checker'); - $this->checker->runInstanceVerification(); + $checker->runInstanceVerification(); } public function testVerifyAppSignatureWithoutSignatureDataAndCodeCheckerDisabled(): void {