aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--lib/base.php10
-rw-r--r--lib/private/AppFramework/Http/Request.php33
-rw-r--r--tests/lib/AppFramework/Http/RequestTest.php70
3 files changed, 110 insertions, 3 deletions
diff --git a/lib/base.php b/lib/base.php
index d6c6e17eff9..2f5517f4614 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -493,10 +493,18 @@ class OC {
'lax',
'strict',
];
+
+ // Append __Host to the cookie if it meets the requirements
+ $cookiePrefix = '';
+ if($cookieParams['secure'] === true && $cookieParams['path'] === '/') {
+ $cookiePrefix = '__Host-';
+ }
+
foreach($policies as $policy) {
header(
sprintf(
- 'Set-Cookie: nc_sameSiteCookie%s=true; path=%s; httponly;' . $secureCookie . 'expires=Fri, 31-Dec-2100 23:59:59 GMT; SameSite=%s',
+ 'Set-Cookie: %snc_sameSiteCookie%s=true; path=%s; httponly;' . $secureCookie . 'expires=Fri, 31-Dec-2100 23:59:59 GMT; SameSite=%s',
+ $cookiePrefix,
$policy,
$cookieParams['path'],
$policy
diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php
index c7a3be163fe..62d7fc7ed30 100644
--- a/lib/private/AppFramework/Http/Request.php
+++ b/lib/private/AppFramework/Http/Request.php
@@ -498,6 +498,31 @@ class Request implements \ArrayAccess, \Countable, IRequest {
}
/**
+ * Wrapper around session_get_cookie_params
+ *
+ * @return array
+ */
+ protected function getCookieParams() {
+ return session_get_cookie_params();
+ }
+
+ /**
+ * Appends the __Host- prefix to the cookie if applicable
+ *
+ * @param string $name
+ * @return string
+ */
+ protected function getProtectedCookieName($name) {
+ $cookieParams = $this->getCookieParams();
+ $prefix = '';
+ if($cookieParams['secure'] === true && $cookieParams['path'] === '/') {
+ $prefix = '__Host-';
+ }
+
+ return $prefix.$name;
+ }
+
+ /**
* Checks if the strict cookie has been sent with the request if the request
* is including any cookies.
*
@@ -508,7 +533,9 @@ class Request implements \ArrayAccess, \Countable, IRequest {
if(!$this->cookieCheckRequired()) {
return true;
}
- if($this->getCookie('nc_sameSiteCookiestrict') === 'true'
+
+ $cookieName = $this->getProtectedCookieName('nc_sameSiteCookiestrict');
+ if($this->getCookie($cookieName) === 'true'
&& $this->passesLaxCookieCheck()) {
return true;
}
@@ -526,7 +553,9 @@ class Request implements \ArrayAccess, \Countable, IRequest {
if(!$this->cookieCheckRequired()) {
return true;
}
- if($this->getCookie('nc_sameSiteCookielax') === 'true') {
+
+ $cookieName = $this->getProtectedCookieName('nc_sameSiteCookielax');
+ if($this->getCookie($cookieName) === 'true') {
return true;
}
return false;
diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php
index 1ba20869439..b1515b0efb5 100644
--- a/tests/lib/AppFramework/Http/RequestTest.php
+++ b/tests/lib/AppFramework/Http/RequestTest.php
@@ -1500,6 +1500,76 @@ class RequestTest extends \Test\TestCase {
$this->assertFalse($request->passesCSRFCheck());
}
+ public function testPassesStrictCookieCheckWithAllCookiesAndStrict() {
+ /** @var Request $request */
+ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+ ->setMethods(['getScriptName', 'getCookieParams'])
+ ->setConstructorArgs([
+ [
+ 'server' => [
+ 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+ ],
+ 'cookies' => [
+ session_name() => 'asdf',
+ '__Host-nc_sameSiteCookiestrict' => 'true',
+ '__Host-nc_sameSiteCookielax' => 'true',
+ ],
+ ],
+ $this->secureRandom,
+ $this->config,
+ $this->csrfTokenManager,
+ $this->stream
+ ])
+ ->getMock();
+ $request
+ ->expects($this->any())
+ ->method('getCookieParams')
+ ->willReturn([
+ 'secure' => true,
+ 'path' => '/',
+ ]);
+
+ $this->assertTrue($request->passesStrictCookieCheck());
+ }
+
+ public function testFailsStrictCookieCheckWithAllCookiesAndMissingStrict() {
+ /** @var Request $request */
+ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request')
+ ->setMethods(['getScriptName', 'getCookieParams'])
+ ->setConstructorArgs([
+ [
+ 'server' => [
+ 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds',
+ ],
+ 'cookies' => [
+ session_name() => 'asdf',
+ 'nc_sameSiteCookiestrict' => 'true',
+ 'nc_sameSiteCookielax' => 'true',
+ ],
+ ],
+ $this->secureRandom,
+ $this->config,
+ $this->csrfTokenManager,
+ $this->stream
+ ])
+ ->getMock();
+ $request
+ ->expects($this->any())
+ ->method('getCookieParams')
+ ->willReturn([
+ 'secure' => true,
+ 'path' => '/',
+ ]);
+
+ $this->assertFalse($request->passesStrictCookieCheck());
+ }
+
+ public function testGetCookieParams() {
+ $request = $this->createMock(Request::class);
+ $actual = $this->invokePrivate($request, 'getCookieParams');
+ $this->assertSame(session_get_cookie_params(), $actual);
+ }
+
public function testPassesStrictCookieCheckWithAllCookies() {
/** @var Request $request */
$request = $this->getMockBuilder('\OC\AppFramework\Http\Request')