mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Harden cookies more appropriate 

This adds the __Host- prefix to the same-site cookies. This is a small but yet nice security hardening.

See https://googlechrome.github.io/samples/cookie-prefixes/ for the implications.

Fixes https://github.com/nextcloud/server/issues/1412

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
This commit is contained in:
Lukas Reschke 2016-11-23 12:53:44 +01:00
parent f692ea34f1
commit a05b8b7953
No known key found for this signature in database
GPG Key ID: B9F6980CF6E759B1
3 changed files with 110 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/base.php
View File

@ -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

33
lib/private/AppFramework/Http/Request.php
View File

@ -497,6 +497,31 @@ class Request implements \ArrayAccess, \Countable, IRequest {
return true;
}
/**
* 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;

70
tests/lib/AppFramework/Http/RequestTest.php
View File

@ -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')