Weak Authentication in Session Handling in typo3/cms-core

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Http;
+
+final class CookieScope
+{
+    public function __construct(
+        public readonly string $domain,
+        public readonly bool $hostOnly,
+        public readonly string $path,
+    ) {}
+}

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Http;
+
+trait CookieScopeTrait
+{
+    /**
+     * Returns the domain and path to be used for setting cookies.
+     * The information is taken from the value in $GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieDomain'] if set,
+     * otherwise the normalized request params are used.
+     */
+    private function getCookieScope(NormalizedParams $normalizedParams): CookieScope
+    {
+        $cookieDomain = $GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieDomain'] ?? '';
+        // If a specific cookie domain is defined for a given application type, use that domain
+        if (!empty($GLOBALS['TYPO3_CONF_VARS'][$this->loginType]['cookieDomain'])) {
+            $cookieDomain = $GLOBALS['TYPO3_CONF_VARS'][$this->loginType]['cookieDomain'];
+        }
+        if (!$cookieDomain) {
+            return new CookieScope(
+                domain: $normalizedParams->getRequestHostOnly(),
+                hostOnly: true,
+                // If no cookie domain is set, use the base path
+                path: $normalizedParams->getSitePath(),
+            );
+        }
+        if ($cookieDomain[0] === '/') {
+            $match = [];
+            $matchCount = @preg_match($cookieDomain, $normalizedParams->getRequestHostOnly(), $match);
+            if ($matchCount === false) {
+                $this->logger->critical(
+                    'The regular expression for the cookie domain ({domain}) contains errors. The session is not shared across sub-domains.',
+                    ['domain' => $cookieDomain]
+                );
+            }
+            if ($matchCount === false || $matchCount === 0) {
+                return new CookieScope(
+                    domain: $normalizedParams->getRequestHostOnly(),
+                    hostOnly: true,
+                    // If no cookie domain could be matched, use the base path
+                    path: $normalizedParams->getSitePath(),
+                );
+            }
+            $cookieDomain = $match[0];
+        }
+
+        return new CookieScope(
+            // Normalize cookie domain by removing leading and trailing dots,
+            // see https://www.rfc-editor.org/rfc/rfc6265#section-4.1.2.3
+            // > Note that a leading %x2E ("."), if present, is ignored even though that character is not permitted,
+            // > but a trailing %x2E ("."), if present, will cause the user agent to ignore the attribute.
+            domain: trim($cookieDomain, '.'),
+            hostOnly: false,
+            path: '/',
+        );
+    }
+}

@@ -32,6 +32,7 @@
 class SetCookieService
 {
     use CookieHeaderTrait;
+    use CookieScopeTrait;
 
     protected readonly LoggerInterface $logger;
 
@@ -65,9 +66,7 @@ public function setSessionCookie(UserSession $userSession, NormalizedParams $nor
         $isRefreshTimeBasedCookie = $this->isRefreshTimeBasedCookie($userSession);
         if ($this->isSetSessionCookie($userSession) || $isRefreshTimeBasedCookie) {
             // Get the domain to be used for the cookie (if any):
-            $cookieDomain = $this->getCookieDomain($normalizedParams);
-            // If no cookie domain is set, use the base path:
-            $cookiePath = $cookieDomain ? '/' : $normalizedParams->getSitePath();
+            $cookieScope = $this->getCookieScope($normalizedParams);
             // If the cookie lifetime is set, use it:
             $cookieExpire = $isRefreshTimeBasedCookie ? $GLOBALS['EXEC_TIME'] + $this->lifetime : 0;
             // Valid options are "strict", "lax" or "none", whereas "none" only works in HTTPS requests (default & fallback is "strict")
@@ -78,13 +77,19 @@ public function setSessionCookie(UserSession $userSession, NormalizedParams $nor
             // SameSite "none" needs the secure option (only allowed on HTTPS)
             $isSecure = $cookieSameSite === Cookie::SAMESITE_NONE || $normalizedParams->isHttps();
             $sessionId = $userSession->getIdentifier();
-            $cookieValue = $userSession->getJwt();
+            $cookieValue = $userSession->getJwt($cookieScope);
             $setCookie = new Cookie(
                 $this->name,
                 $cookieValue,
                 $cookieExpire,
-                $cookiePath,
-                $cookieDomain,
+                $cookieScope->path,
+                // Host-Only cookies need to be provided without an explicit domain,
+                // see https://datatracker.ietf.org/doc/html/rfc6265#section-4.1.2.3
+                // and https://datatracker.ietf.org/doc/html/rfc6265#section-5.3
+                // | * If the value of the Domain attribute is "example.com", the user agent will include the cookie
+                // |   in the Cookie header when making HTTP requests to example.com, www.example.com, and www.corp.example.com
+                // | * If the server omits the Domain attribute, the user agent will return the cookie only to the origin server.
+                $cookieScope->hostOnly ? null : $cookieScope->domain,
                 $isSecure,
                 true,
                 false,
@@ -93,45 +98,12 @@ public function setSessionCookie(UserSession $userSession, NormalizedParams $nor
             $message = $isRefreshTimeBasedCookie ? 'Updated Cookie: {session}, {domain}' : 'Set Cookie: {session}, {domain}';
             $this->logger->debug($message, [
                 'session' => sha1($sessionId),
-                'domain' => $cookieDomain,
+                'domain' => $cookieScope->domain,
             ]);
         }
         return $setCookie;
     }
 
-    /**
-     * Gets the domain to be used on setting cookies.
-     * The information is taken from the value in $GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieDomain'].
-     *
-     * @return string The domain to be used on setting cookies
-     */
-    protected function getCookieDomain(NormalizedParams $normalizedParams): string
-    {
-        $result = '';
-        $cookieDomain = $GLOBALS['TYPO3_CONF_VARS']['SYS']['cookieDomain'] ?? '';
-        // If a specific cookie domain is defined for a given application type, use that domain
-        if (!empty($GLOBALS['TYPO3_CONF_VARS'][$this->loginType]['cookieDomain'])) {
-            $cookieDomain = $GLOBALS['TYPO3_CONF_VARS'][$this->loginType]['cookieDomain'];
-        }
-        if ($cookieDomain) {
-            if ($cookieDomain[0] === '/') {
-                $match = [];
-                $matchCnt = @preg_match($cookieDomain, $normalizedParams->getRequestHostOnly(), $match);
-                if ($matchCnt === false) {
-                    $this->logger->critical(
-                        'The regular expression for the cookie domain ({domain}) contains errors. The session is not shared across sub-domains.',
-                        ['domain' => $cookieDomain]
-                    );
-                } elseif ($matchCnt) {
-                    $result = $match[0];
-                }
-            } else {
-                $result = $cookieDomain;
-            }
-        }
-        return $result;
-    }
-
     /**
      * Determine whether a session cookie needs to be set (lifetime=0)
      */
@@ -180,15 +152,13 @@ public function isCookieSet(?ServerRequestInterface $request, ?UserSession $user
      */
     public function removeCookie(NormalizedParams $normalizedParams): Cookie
     {
-        $cookieDomain = $this->getCookieDomain($normalizedParams);
-        // If no cookie domain is set, use the base path
-        $cookiePath = $cookieDomain ? '/' : $normalizedParams->getSitePath();
+        $scope = $this->getCookieScope($normalizedParams);
         return new Cookie(
             $this->name,
             '',
             -1,
-            $cookiePath,
-            $cookieDomain
+            $scope->path,
+            $scope->domain
         );
     }
 }

@@ -22,6 +22,7 @@
 use Psr\Log\LoggerAwareTrait;
 use TYPO3\CMS\Core\Authentication\IpLocker;
 use TYPO3\CMS\Core\Crypto\Random;
+use TYPO3\CMS\Core\Http\CookieScopeTrait;
 use TYPO3\CMS\Core\Session\Backend\Exception\SessionNotFoundException;
 use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -43,6 +44,7 @@
 class UserSessionManager implements LoggerAwareInterface
 {
     use LoggerAwareTrait;
+    use CookieScopeTrait;
 
     protected const SESSION_ID_LENGTH = 32;
     protected const GARBAGE_COLLECTION_LIFETIME = 86400;
@@ -59,17 +61,19 @@ class UserSessionManager implements LoggerAwareInterface
     protected int $garbageCollectionForAnonymousSessions = self::LIFETIME_OF_ANONYMOUS_SESSION_DATA;
     protected SessionBackendInterface $sessionBackend;
     protected IpLocker $ipLocker;
+    protected string $loginType;
 
     /**
      * Constructor. Marked as internal, as it is recommended to use the factory method "create"
      *
      * @internal it is recommended to use the factory method "create"
      */
-    public function __construct(SessionBackendInterface $sessionBackend, int $sessionLifetime, IpLocker $ipLocker)
+    public function __construct(SessionBackendInterface $sessionBackend, int $sessionLifetime, IpLocker $ipLocker, string $loginType)
     {
         $this->sessionBackend = $sessionBackend;
         $this->sessionLifetime = $sessionLifetime;
         $this->ipLocker = $ipLocker;
+        $this->loginType = $loginType;
     }
 
     protected function setGarbageCollectionTimeoutForAnonymousSessions(int $garbageCollectionForAnonymousSessions = 0): void
@@ -91,7 +95,8 @@ public function createFromRequestOrAnonymous(ServerRequestInterface $request, st
     {
         try {
             $cookieValue = (string)($request->getCookieParams()[$cookieName] ?? '');
-            $sessionId = UserSession::resolveIdentifierFromJwt($cookieValue);
+            $scope = $this->getCookieScope($request->getAttribute('normalizedParams'));
+            $sessionId = UserSession::resolveIdentifierFromJwt($cookieValue, $scope);
         } catch (\Exception $exception) {
             $this->logger->debug('Could not resolve session identifier from JWT', ['exception' => $exception]);
         }
@@ -354,7 +359,8 @@ public static function create(string $loginType, int $sessionLifetime = null, Se
             self::class,
             $sessionManager->getSessionBackend($loginType),
             $sessionLifetime,
-            $ipLocker
+            $ipLocker,
+            $loginType
         );
         if ($loginType === 'FE') {
             $object->setGarbageCollectionTimeoutForAnonymousSessions((int)($GLOBALS['TYPO3_CONF_VARS']['FE']['sessionDataLifetime'] ?? 0));

@@ -17,7 +17,10 @@
 
 namespace TYPO3\CMS\Core\Session;
 
+use TYPO3\CMS\Core\Http\CookieScope;
+use TYPO3\CMS\Core\Log\LogManager;
 use TYPO3\CMS\Core\Security\JwtTrait;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
  * Represents all information about a user's session.
@@ -195,17 +198,19 @@ public function needsUpdate(): bool
 
     /**
      * Gets session ID wrapped in JWT to be used for emitting a new cookie.
-     * `Cookie: <JWT(HS256, [identifier => <session-id>], <signature>)>`
+     * `Cookie: <JWT(HS256, [identifier => <session-id>], <signature(encryption-key, cookie-domain)>)>`
      *
+     * @param ?CookieScope $scope
      * @return string the session ID wrapped in JWT to be used for emitting a new cookie
      */
-    public function getJwt(): string
+    public function getJwt(?CookieScope $scope = null): string
     {
         // @todo payload could be organized in a new `SessionToken` object
         return self::encodeHashSignedJwt(
             [
                 'identifier' => $this->identifier,
                 'time' => (new \DateTimeImmutable())->format(\DateTimeImmutable::RFC3339),
+                'scope' => $scope,
             ],
             self::createSigningKeyFromEncryptionKey(UserSession::class)
         );
@@ -246,20 +251,40 @@ public static function createNonFixated(string $identifier): self
 
     /**
      * Verifies and resolves the session ID from a submitted cookie value:
-     * `Cookie: <JWT(HS256, [identifier => <session-id>], <signature>)>`
+     * `Cookie: <JWT(HS256, [identifier => <session-id>], <signature(encryption-key, cookie-domain)>)>`
      *
      * @param string $cookieValue submitted cookie value
+     * @param CookieScope $scope
      * @return non-empty-string|null session ID, null in case verification failed
      * @throws \Exception
      * @see getJwt()
      */
-    public static function resolveIdentifierFromJwt(string $cookieValue): ?string
+    public static function resolveIdentifierFromJwt(string $cookieValue, CookieScope $scope): ?string
     {
         if ($cookieValue === '') {
             return null;
         }
+
         $payload = self::decodeJwt($cookieValue, self::createSigningKeyFromEncryptionKey(UserSession::class));
-        return !empty($payload->identifier) && is_string($payload->identifier) ? $payload->identifier : null;
+
+        $identifier = !empty($payload->identifier) && is_string($payload->identifier) ? $payload->identifier : null;
+        if ($identifier === null) {
+            return null;
+        }
+
+        $domainScope = (string)($payload->scope->domain ?? '');
+        $pathScope = (string)($payload->scope->path ?? '');
+        if ($domainScope === '' || $pathScope === '') {
+            $logger = GeneralUtility::makeInstance(LogManager::class)->getLogger(self::class);
+            $logger->notice('A session cookie with out a domain scope has been used', ['cookieHash' => substr(sha1($cookieValue), 0, 12)]);
+            return $identifier;
+        }
+        if ($domainScope !== $scope->domain || $pathScope !== $scope->path) {
+            // invalid scope, the cookie jwt has been used on a wrong path or domain
+            return null;
+        }
+
+        return $identifier;
     }
 
     /**

@@ -80,7 +80,8 @@ public function logoffCleansFormProtectionIfBackendUserIsLoggedIn(): void
         $userSessionManager = new UserSessionManager(
             $sessionBackendMock,
             86400,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'BE'
         );
 
         $GLOBALS['BE_USER'] = $this->getMockBuilder(BackendUserAuthentication::class)->getMock();

@@ -20,6 +20,7 @@
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Log\NullLogger;
 use TYPO3\CMS\Core\Authentication\IpLocker;
+use TYPO3\CMS\Core\Http\NormalizedParams;
 use TYPO3\CMS\Core\Security\JwtTrait;
 use TYPO3\CMS\Core\Session\Backend\Exception\SessionNotFoundException;
 use TYPO3\CMS\Core\Session\Backend\SessionBackendInterface;
@@ -62,7 +63,8 @@ public function willExpireWillExpire(int $sessionLifetime, int $gracePeriod, boo
         $subject = new UserSessionManager(
             $sessionBackendMock,
             $sessionLifetime,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'FE'
         );
         $session = $subject->createAnonymousSession();
         self::assertEquals($expectedResult, $subject->willExpire($session, $gracePeriod));
@@ -75,7 +77,8 @@ public function hasExpiredIsCalculatedCorrectly(): void
         $subject = new UserSessionManager(
             $sessionBackendMock,
             60,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'FE'
         );
         $expiredSession = UserSession::createFromRecord('random-string', ['ses_tstamp' => time() - 500]);
         self::assertTrue($subject->hasExpired($expiredSession));
@@ -100,17 +103,31 @@ public function createFromRequestOrAnonymousCreatesProperSessionObjectForValidSe
         $subject = new UserSessionManager(
             $sessionBackendMock,
             50,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'FE'
         );
         $subject->setLogger(new NullLogger());
+        $cookieDomain = 'example.org';
         $validSessionJwt = self::encodeHashSignedJwt(
             [
                 'identifier' => 'valid-session',
                 'time' => (new \DateTimeImmutable())->format(\DateTimeImmutable::RFC3339),
+                'scope' => [
+                    'domain' => $cookieDomain,
+                    'path' => '/',
+                ],
             ],
             self::createSigningKeyFromEncryptionKey(UserSession::class)
         );
+
+        $normalizedParams = $this->createMock(NormalizedParams::class);
+        $normalizedParams->method('getRequestHostOnly')->willReturn($cookieDomain);
+        $normalizedParams->method('getSitePath')->willReturn('/');
         $request = $this->createMock(ServerRequestInterface::class);
+        $request->method('getAttribute')->willReturnCallback(static fn(string $name): mixed => match ($name) {
+            'normalizedParams' => $normalizedParams,
+            default => null,
+        });
         $request->method('getCookieParams')->willReturn(['bar' => $validSessionJwt]);
         $persistedSession = $subject->createFromRequestOrAnonymous($request, 'bar');
         self::assertEquals(13, $persistedSession->getUserId());
@@ -134,11 +151,19 @@ public function createFromRequestOrAnonymousCreatesProperSessionObjectForInvalid
         $subject = new UserSessionManager(
             $sessionBackendMock,
             50,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'FE'
         );
         $subject->setLogger(new NullLogger());
 
+        $cookieDomain = 'example.org';
+        $normalizedParams = $this->createMock(NormalizedParams::class);
+        $normalizedParams->method('getRequestHostOnly')->willReturn($cookieDomain);
         $request = $this->createMock(ServerRequestInterface::class);
+        $request->method('getAttribute')->willReturnCallback(static fn(string $name): mixed => match ($name) {
+            'normalizedParams' => $normalizedParams,
+            default => null,
+        });
         $request->method('getCookieParams')->willReturnOnConsecutiveCalls([], ['foo' => 'invalid-session']);
         $anonymousSession = $subject->createFromRequestOrAnonymous($request, 'foo');
         self::assertTrue($anonymousSession->isNew());
@@ -165,7 +190,8 @@ public function updateSessionWillSetLastUpdated(): void
         $subject = new UserSessionManager(
             $sessionBackendMock,
             60,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'FE'
         );
         $session = UserSession::createFromRecord('random-string', ['ses_tstamp' => time() - 500]);
         $session = $subject->updateSession($session);
@@ -188,7 +214,8 @@ public function fixateAnonymousSessionWillUpdateSessionObject(): void
         $subject = new UserSessionManager(
             $sessionBackendMock,
             60,
-            new IpLocker(0, 0)
+            new IpLocker(0, 0),
+            'FE'
         );
         $session = UserSession::createFromRecord('random-string', ['ses_tstamp' => time() - 500]);
         $session = $subject->fixateAnonymousSession($session);

@@ -17,6 +17,7 @@
 
 namespace TYPO3\CMS\Core\Tests\Unit\Session;
 
+use TYPO3\CMS\Core\Http\CookieScope;
 use TYPO3\CMS\Core\Security\JwtTrait;
 use TYPO3\CMS\Core\Session\UserSession;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
@@ -38,6 +39,7 @@ public function createFromRecordTest(): void
             'ses_tstamp' => 1607041477,
             'ses_permanent' => 1,
         ];
+        $scope = new CookieScope(domain: 'example.com', hostOnly: true, path: '/');
 
         $session = UserSession::createFromRecord($record['ses_id'], $record, true);
 
@@ -61,7 +63,7 @@ public function createFromRecordTest(): void
 
         self::assertTrue($session->dataWasUpdated());
         self::assertEquals(['override' => 'data'], $session->getData());
-        self::assertSame($record['ses_id'], UserSession::resolveIdentifierFromJwt($session->getJwt()));
+        self::assertSame($record['ses_id'], UserSession::resolveIdentifierFromJwt($session->getJwt($scope), $scope) ?? '');
     }
 
     /**

@@ -19,6 +19,9 @@
 
 use GuzzleHttp\Cookie\SetCookie;
 use Psr\Log\NullLogger;
+use TYPO3\CMS\Core\Core\Environment;
+use TYPO3\CMS\Core\Http\NormalizedParams;
+use TYPO3\CMS\Core\Http\ServerRequest;
 use TYPO3\CMS\Core\Security\Nonce;
 use TYPO3\CMS\Core\Security\RequestToken;
 use TYPO3\CMS\Core\Tests\Functional\SiteHandling\SiteBasedTestTrait;
@@ -64,6 +67,19 @@ public function canCreateNewAndExistingSessionWithValidRequestToken(): void
     {
         $this->importCSVDataSet(__DIR__ . '/Fixtures/fe_users.csv');
 
+        $normalizedParams = new NormalizedParams(
+            [
+                'REQUEST_URI' => '/',
+                'HTTP_HOST' => 'localhost',
+                'DOCUMENT_ROOT' => Environment::getPublicPath(),
+                'SCRIPT_FILENAME' => Environment::getPublicPath() . '/index.php',
+                'SCRIPT_NAME' => '/index.php',
+            ],
+            $GLOBALS['TYPO3_CONF_VARS']['SYS'],
+            Environment::getPublicPath() . '/index.php',
+            Environment::getPublicPath()
+        );
+
         $nonce = Nonce::create();
         $requestToken = RequestToken::create('core/user-auth/fe')->toHashSignedJwt($nonce);
         $request = (new InternalRequest())
@@ -77,6 +93,7 @@ public function canCreateNewAndExistingSessionWithValidRequestToken(): void
                     '__RequestToken' => $requestToken,
                 ]
             )
+            ->withAttribute('normalizedParams', $normalizedParams)
             ->withCookieParams([123 => 'bogus', 'typo3nonce_' . $nonce->getSigningIdentifier()->name => $nonce->toHashSignedJwt()]);
 
         $response = $this->executeFrontendSubRequest($request);
@@ -86,8 +103,8 @@ public function canCreateNewAndExistingSessionWithValidRequestToken(): void
 
         // Now check whether the existing session is retrieved by providing the retrieved JWT token in the cookie params.
         $cookie = SetCookie::fromString($response->getHeaderLine('Set-Cookie'));
-        $request = (new InternalRequest())
-            ->withPageId(self::ROOT_PAGE_ID)
+        $request = (new ServerRequest('http://localhost/'))
+            ->withAttribute('normalizedParams', $normalizedParams)
             ->withCookieParams([$cookie->getName() => $cookie->getValue()]);
 
         $frontendUserAuthentication = new FrontendUserAuthentication();