diff --git a/lib/base.php b/lib/base.php index 010fed1ea9f..7723468826c 100644 --- a/lib/base.php +++ b/lib/base.php @@ -7,6 +7,7 @@ declare(strict_types=1); * SPDX-License-Identifier: AGPL-3.0-only */ use OC\Encryption\HookManager; +use OC\Session\CryptoSessionHandler; use OC\Share20\Hooks; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Events\UserRemovedEvent; @@ -361,6 +362,9 @@ class OC { public static function initSession(): void { $request = Server::get(IRequest::class); + $cryptoHandler = Server::get(CryptoSessionHandler::class); + session_set_save_handler($cryptoHandler, true); + // TODO: Temporary disabled again to solve issues with CalDAV/CardDAV clients like DAVx5 that use cookies // TODO: See https://github.com/nextcloud/server/issues/37277#issuecomment-1476366147 and the other comments // TODO: for further information. @@ -658,7 +662,7 @@ class OC { if (!function_exists('simplexml_load_file')) { throw new \OCP\HintException('The PHP SimpleXML/PHP-XML extension is not installed.', 'Install the extension or make sure it is enabled.'); } - + OC_App::loadApps(['session']); if (!self::$CLI) { self::initSession(); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1a3a86fd207..459bb482f7f 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1871,9 +1871,10 @@ return array( 'OC\\ServerContainer' => $baseDir . '/lib/private/ServerContainer.php', 'OC\\ServerNotAvailableException' => $baseDir . '/lib/private/ServerNotAvailableException.php', 'OC\\ServiceUnavailableException' => $baseDir . '/lib/private/ServiceUnavailableException.php', - 'OC\\Session\\CryptoSessionData' => $baseDir . '/lib/private/Session/CryptoSessionData.php', + 'OC\\Session\\CryptoSessionHandler' => $baseDir . '/lib/private/Session/CryptoSessionHandler.php', 'OC\\Session\\CryptoWrapper' => $baseDir . '/lib/private/Session/CryptoWrapper.php', 'OC\\Session\\Internal' => $baseDir . '/lib/private/Session/Internal.php', + 'OC\\Session\\LegacyCryptoSessionData' => $baseDir . '/lib/private/Session/LegacyCryptoSessionData.php', 'OC\\Session\\Memory' => $baseDir . '/lib/private/Session/Memory.php', 'OC\\Session\\Session' => $baseDir . '/lib/private/Session/Session.php', 'OC\\Settings\\AuthorizedGroup' => $baseDir . '/lib/private/Settings/AuthorizedGroup.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c636ba769f9..df38c56cae5 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1904,9 +1904,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\ServerContainer' => __DIR__ . '/../../..' . '/lib/private/ServerContainer.php', 'OC\\ServerNotAvailableException' => __DIR__ . '/../../..' . '/lib/private/ServerNotAvailableException.php', 'OC\\ServiceUnavailableException' => __DIR__ . '/../../..' . '/lib/private/ServiceUnavailableException.php', - 'OC\\Session\\CryptoSessionData' => __DIR__ . '/../../..' . '/lib/private/Session/CryptoSessionData.php', + 'OC\\Session\\CryptoSessionHandler' => __DIR__ . '/../../..' . '/lib/private/Session/CryptoSessionHandler.php', 'OC\\Session\\CryptoWrapper' => __DIR__ . '/../../..' . '/lib/private/Session/CryptoWrapper.php', 'OC\\Session\\Internal' => __DIR__ . '/../../..' . '/lib/private/Session/Internal.php', + 'OC\\Session\\LegacyCryptoSessionData' => __DIR__ . '/../../..' . '/lib/private/Session/LegacyCryptoSessionData.php', 'OC\\Session\\Memory' => __DIR__ . '/../../..' . '/lib/private/Session/Memory.php', 'OC\\Session\\Session' => __DIR__ . '/../../..' . '/lib/private/Session/Session.php', 'OC\\Settings\\AuthorizedGroup' => __DIR__ . '/../../..' . '/lib/private/Settings/AuthorizedGroup.php', diff --git a/lib/private/Session/CryptoSessionHandler.php b/lib/private/Session/CryptoSessionHandler.php new file mode 100644 index 00000000000..594a9ca35ad --- /dev/null +++ b/lib/private/Session/CryptoSessionHandler.php @@ -0,0 +1,144 @@ +secureRandom->generate(128); + return implode('|', [$id, $passphrase]); + } + + /** + * Read and decrypt session data + * + * The decryption passphrase is encoded in the id since Nextcloud 31. For + * backwards compatibility after upgrade we also read the pass phrase from + * the old cookie and try to restore session from the legacy format. + * + * @param string $id + * + * @return false|string + */ + public function read(string $id): false|string { + [$sessionId, $passphrase] = $this->parseId($id); + + /** + * Legacy handling + * + * @TODO remove in Nextcloud 32 + */ + if ($passphrase === null) { + if (($passphrase = $this->request->getCookie(CryptoWrapper::COOKIE_NAME)) === false) { + // No passphrase in the ID or an old cookie. Time to give up. + return false; + } + + // Read the encrypted and encoded data + $serializedData = parent::read($sessionId); + if ($serializedData === '') { + // Nothing to decode or decrypt + return ''; + } + // Back up the superglobal before we decode/restore the legacy session data + // This is necessary because session_decode populates the superglobals + // We restore the old state end the end (the final block, also run in + // case of an error) + $globalBackup = $_SESSION; + try { + if (!session_decode($serializedData)) { + // Bail if we can't decode the data + return false; + } + $encryptedData = $_SESSION['encrypted_session_data']; + try { + $sessionValues = json_decode( + $this->crypto->decrypt($encryptedData, $passphrase), + true, + 512, + JSON_THROW_ON_ERROR, + ); + foreach ($sessionValues as $key => $value) { + $_SESSION[$key] = $value; + } + } catch (Exception $e) { + logger('core')->critical('Could not decrypt or decode encrypted legacy session data', [ + 'exception' => $e, + 'sessionId' => $sessionId, + ]); + } + return session_encode(); + } finally { + foreach (array_keys($_SESSION) as $key) { + unset($_SESSION[$key]); + } + foreach ($globalBackup as $key => $value) { + $_SESSION[$key] = $value; + } + $_SESSION = $globalBackup; + } + } + + $read = parent::read($sessionId); + return $read; + } + + /** + * Encrypt and write session data + * + * @param string $id + * @param string $data + * + * @return bool + */ + public function write(string $id, string $data): bool { + [$sessionId, $passphrase] = $this->parseId($id); + + if ($passphrase === null) { + $passphrase = $this->request->getCookie(CryptoWrapper::COOKIE_NAME); + // return false; + } + + return parent::write($sessionId, $data); + } + + public function close(): bool { + parent::close(); + } + +} diff --git a/lib/private/Session/CryptoWrapper.php b/lib/private/Session/CryptoWrapper.php index aceb387ea74..3d538a8dc6a 100644 --- a/lib/private/Session/CryptoWrapper.php +++ b/lib/private/Session/CryptoWrapper.php @@ -90,8 +90,8 @@ class CryptoWrapper { * @return ISession */ public function wrapSession(ISession $session) { - if (!($session instanceof CryptoSessionData)) { - return new CryptoSessionData($session, $this->crypto, $this->passphrase); + if (!($session instanceof LegacyCryptoSessionData)) { + return new LegacyCryptoSessionData($session, $this->crypto, $this->passphrase); } return $session; diff --git a/lib/private/Session/CryptoSessionData.php b/lib/private/Session/LegacyCryptoSessionData.php similarity index 70% rename from lib/private/Session/CryptoSessionData.php rename to lib/private/Session/LegacyCryptoSessionData.php index 323253af534..9cf115f79e4 100644 --- a/lib/private/Session/CryptoSessionData.php +++ b/lib/private/Session/LegacyCryptoSessionData.php @@ -15,28 +15,24 @@ use function json_decode; use function OCP\Log\logger; /** - * Class CryptoSessionData - * * @package OC\Session * @template-implements \ArrayAccess + * @deprecated 31.0.0 */ -class CryptoSessionData implements \ArrayAccess, ISession { +class LegacyCryptoSessionData implements \ArrayAccess, ISession { /** @var ISession */ protected $session; - /** @var \OCP\Security\ICrypto */ + /** @var ICrypto */ protected $crypto; /** @var string */ protected $passphrase; - /** @var array */ - protected $sessionValues; - /** @var bool */ - protected $isModified = false; public const encryptedSessionName = 'encrypted_session_data'; /** * @param ISession $session * @param ICrypto $crypto * @param string $passphrase + * @deprecated 31.0.0 */ public function __construct(ISession $session, ICrypto $crypto, @@ -49,6 +45,7 @@ class CryptoSessionData implements \ArrayAccess, ISession { /** * Close session if class gets destructed + * @deprecated 31.0.0 */ public function __destruct() { try { @@ -59,26 +56,31 @@ class CryptoSessionData implements \ArrayAccess, ISession { } } + /** + * @deprecated 31.0.0 + */ protected function initializeSession() { $encryptedSessionData = $this->session->get(self::encryptedSessionName) ?: ''; if ($encryptedSessionData === '') { // Nothing to decrypt - $this->sessionValues = []; - } else { - try { - $this->sessionValues = json_decode( - $this->crypto->decrypt($encryptedSessionData, $this->passphrase), - true, - 512, - JSON_THROW_ON_ERROR, - ); - } catch (\Exception $e) { - logger('core')->critical('Could not decrypt or decode encrypted session data', [ - 'exception' => $e, - ]); - $this->sessionValues = []; - $this->regenerateId(true, false); + return; + } + try { + $sessionValues = json_decode( + $this->crypto->decrypt($encryptedSessionData, $this->passphrase), + true, + 512, + JSON_THROW_ON_ERROR, + ); + foreach ($sessionValues as $key => $value) { + $this->session->set($key, $value); } + $this->session->remove(self::encryptedSessionName); + } catch (\Exception $e) { + logger('core')->critical('Could not decrypt or decode encrypted session data', [ + 'exception' => $e, + ]); + $this->regenerateId(true, false); } } @@ -87,19 +89,10 @@ class CryptoSessionData implements \ArrayAccess, ISession { * * @param string $key * @param mixed $value + * @deprecated 31.0.0 */ public function set(string $key, $value) { - if ($this->get($key) === $value) { - // Do not write the session if the value hasn't changed to avoid reopening - return; - } - - $reopened = $this->reopen(); - $this->sessionValues[$key] = $value; - $this->isModified = true; - if ($reopened) { - $this->close(); - } + $this->session->set($key, $value); } /** @@ -107,13 +100,10 @@ class CryptoSessionData implements \ArrayAccess, ISession { * * @param string $key * @return string|null Either the value or null + * @deprecated 31.0.0 */ public function get(string $key) { - if (isset($this->sessionValues[$key])) { - return $this->sessionValues[$key]; - } - - return null; + return $this->session->get($key); } /** @@ -121,42 +111,37 @@ class CryptoSessionData implements \ArrayAccess, ISession { * * @param string $key * @return bool + * @deprecated 31.0.0 */ public function exists(string $key): bool { - return isset($this->sessionValues[$key]); + return $this->session->exists($key); } /** * Remove a $key/$value pair from the session * * @param string $key + * @deprecated 31.0.0 */ public function remove(string $key) { - $reopened = $this->reopen(); - $this->isModified = true; - unset($this->sessionValues[$key]); - if ($reopened) { - $this->close(); - } + $this->session->remove($key); } /** * Reset and recreate the session + * @deprecated 31.0.0 */ public function clear() { - $reopened = $this->reopen(); $requesttoken = $this->get('requesttoken'); - $this->sessionValues = []; + $this->session->clear(); if ($requesttoken !== null) { $this->set('requesttoken', $requesttoken); } - $this->isModified = true; - $this->session->clear(); - if ($reopened) { - $this->close(); - } } + /** + * @deprecated 31.0.0 + */ public function reopen(): bool { $reopened = $this->session->reopen(); if ($reopened) { @@ -171,6 +156,7 @@ class CryptoSessionData implements \ArrayAccess, ISession { * @param bool $deleteOldSession Whether to delete the old associated session file or not. * @param bool $updateToken Wheater to update the associated auth token * @return void + * @deprecated 31.0.0 */ public function regenerateId(bool $deleteOldSession = true, bool $updateToken = false) { $this->session->regenerateId($deleteOldSession, $updateToken); @@ -182,6 +168,7 @@ class CryptoSessionData implements \ArrayAccess, ISession { * @return string * @throws SessionNotAvailableException * @since 9.1.0 + * @deprecated 31.0.0 */ public function getId(): string { return $this->session->getId(); @@ -189,19 +176,16 @@ class CryptoSessionData implements \ArrayAccess, ISession { /** * Close the session and release the lock, also writes all changed data in batch + * @deprecated 31.0.0 */ public function close() { - if ($this->isModified) { - $encryptedValue = $this->crypto->encrypt(json_encode($this->sessionValues), $this->passphrase); - $this->session->set(self::encryptedSessionName, $encryptedValue); - $this->isModified = false; - } $this->session->close(); } /** * @param mixed $offset * @return bool + * @deprecated 31.0.0 */ public function offsetExists($offset): bool { return $this->exists($offset); @@ -210,6 +194,7 @@ class CryptoSessionData implements \ArrayAccess, ISession { /** * @param mixed $offset * @return mixed + * @deprecated 31.0.0 */ #[\ReturnTypeWillChange] public function offsetGet($offset) { @@ -219,6 +204,7 @@ class CryptoSessionData implements \ArrayAccess, ISession { /** * @param mixed $offset * @param mixed $value + * @deprecated 31.0.0 */ public function offsetSet($offset, $value): void { $this->set($offset, $value); @@ -226,6 +212,7 @@ class CryptoSessionData implements \ArrayAccess, ISession { /** * @param mixed $offset + * @deprecated 31.0.0 */ public function offsetUnset($offset): void { $this->remove($offset); diff --git a/tests/lib/Session/CryptoSessionDataTest.php b/tests/lib/Session/CryptoSessionDataTest.php index 6c1536f4769..4d95ffe2f02 100644 --- a/tests/lib/Session/CryptoSessionDataTest.php +++ b/tests/lib/Session/CryptoSessionDataTest.php @@ -7,7 +7,7 @@ namespace Test\Session; -use OC\Session\CryptoSessionData; +use OC\Session\LegacyCryptoSessionData; use OCP\Security\ICrypto; class CryptoSessionDataTest extends Session { @@ -36,6 +36,6 @@ class CryptoSessionDataTest extends Session { return substr($input, 1, -1); }); - $this->instance = new CryptoSessionData($this->wrappedSession, $this->crypto, 'PASS'); + $this->instance = new LegacyCryptoSessionData($this->wrappedSession, $this->crypto, 'PASS'); } } diff --git a/tests/lib/Session/CryptoWrappingTest.php b/tests/lib/Session/CryptoWrappingTest.php index 093f2214929..9217eb464a7 100644 --- a/tests/lib/Session/CryptoWrappingTest.php +++ b/tests/lib/Session/CryptoWrappingTest.php @@ -7,7 +7,7 @@ namespace Test\Session; -use OC\Session\CryptoSessionData; +use OC\Session\LegacyCryptoSessionData; use OCP\ISession; use Test\TestCase; @@ -18,7 +18,7 @@ class CryptoWrappingTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject|\OCP\ISession */ protected $wrappedSession; - /** @var \OC\Session\CryptoSessionData */ + /** @var \OC\Session\LegacyCryptoSessionData */ protected $instance; protected function setUp(): void { @@ -44,7 +44,7 @@ class CryptoWrappingTest extends TestCase { return substr($input, 1, -1); }); - $this->instance = new CryptoSessionData($this->wrappedSession, $this->crypto, 'PASS'); + $this->instance = new LegacyCryptoSessionData($this->wrappedSession, $this->crypto, 'PASS'); } public function testUnwrappingGet() {