wbrawner/server
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #13973 from owncloud/enhancement/security/13366

Browse source 
Respect `mod_unique_id` and refactor `OC_Request::getRequestId`
This commit is contained in:
Lukas Reschke 2015-02-09 17:35:19 +01:00
commit 47c7eb4e70
16 changed files with 248 additions and 125 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
lib/private/appframework/http/request.php
View file

@ -25,6 +25,7 @@
namespace OC\AppFramework\Http;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
/**
* Class for accessing variables in the request.
@ -48,24 +49,32 @@ class Request implements \ArrayAccess, \Countable, IRequest {
'method',
'requesttoken',
);
/** @var ISecureRandom */
protected $secureRandom;
/** @var string */
protected $requestId = '';
/**
* @param array $vars An associative array with the following optional values:
* @param array 'urlParams' the parameters which were matched from the URL
* @param array 'get' the $_GET array
* @param array|string 'post' the $_POST array or JSON string
* @param array 'files' the $_FILES array
* @param array 'server' the $_SERVER array
* @param array 'env' the $_ENV array
* @param array 'cookies' the $_COOKIE array
* @param string 'method' the request method (GET, POST etc)
* @param string|false 'requesttoken' the requesttoken or false when not available
* - array 'urlParams' the parameters which were matched from the URL
* - array 'get' the $_GET array
* - array|string 'post' the $_POST array or JSON string
* - array 'files' the $_FILES array
* - array 'server' the $_SERVER array
* - array 'env' the $_ENV array
* - array 'cookies' the $_COOKIE array
* - string 'method' the request method (GET, POST etc)
* - string|false 'requesttoken' the requesttoken or false when not available
* @param ISecureRandom $secureRandom
* @param string $stream
* @see http://www.php.net/manual/en/reserved.variables.php
*/
public function __construct(array $vars=array(), $stream='php://input') {
public function __construct(array $vars=array(),
ISecureRandom $secureRandom,
$stream='php://input') {
$this->inputStream = $stream;
$this->items['params'] = array();
$this->secureRandom = $secureRandom;
if(!array_key_exists('method', $vars)) {
$vars['method'] = 'GET';
@ -384,4 +393,23 @@ class Request implements \ArrayAccess, \Countable, IRequest {
// Valid token
return true;
}
}}
}
/**
* Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
* If `mod_unique_id` is installed this value will be taken.
* @return string
*/
public function getId() {
if(isset($this->server['UNIQUE_ID'])) {
return $this->server['UNIQUE_ID'];
}
if(empty($this->requestId)) {
$this->requestId = $this->secureRandom->getLowStrengthGenerator()->generate(20);
}
return $this->requestId;
}
}

2
lib/private/log/owncloud.php
View file

@ -68,7 +68,7 @@ class OC_Log_Owncloud {
$timezone = new DateTimeZone('UTC');
}
$time = new DateTime(null, $timezone);
$reqId = \OC_Request::getRequestID();
$reqId = \OC::$server->getRequest()->getId();
$remoteAddr = \OC_Request::getRemoteAddress();
// remove username/passwords from URLs before writing the to the log file
$time = $time->format($format);

12
lib/private/request.php
View file

@ -13,7 +13,6 @@ class OC_Request {
const USER_AGENT_ANDROID_MOBILE_CHROME = '#Android.*Chrome/[.0-9]*#';
const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#';
const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)$/';
static protected $reqId;
/**
* Returns the remote address, if the connection came from a trusted proxy and `forwarded_for_headers` has been configured
@ -43,17 +42,6 @@ class OC_Request {
return $remoteAddress;
}
/**
* Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
* @return string
*/
public static function getRequestID() {
if(self::$reqId === null) {
self::$reqId = hash('md5', microtime().\OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(20));
}
return self::$reqId;
}
/**
* Check overwrite condition
* @param string $type

7
lib/private/server.php
View file

@ -10,7 +10,6 @@ use OC\Cache\UserCache;
use OC\Diagnostics\NullQueryLogger;
use OC\Diagnostics\EventLogger;
use OC\Diagnostics\QueryLogger;
use OC\Files\Config\StorageManager;
use OC\Security\CertificateManager;
use OC\Files\Node\Root;
use OC\Files\View;
@ -64,7 +63,7 @@ class Server extends SimpleContainer implements IServerContainer {
}
return new Request(
array(
[
'get' => $_GET,
'post' => $_POST,
'files' => $_FILES,
@ -76,7 +75,9 @@ class Server extends SimpleContainer implements IServerContainer {
: null,
'urlParams' => $urlParams,
'requesttoken' => $requestToken,
), $stream
],
$this->getSecureRandom(),
$stream
);
});
$this->registerService('PreviewManager', function ($c) {

2
lib/private/template.php
View file

@ -223,7 +223,7 @@ class OC_Template extends \OC\Template\Base {
$content->assign('trace', $exception->getTraceAsString());
$content->assign('debugMode', defined('DEBUG') && DEBUG === true);
$content->assign('remoteAddr', OC_Request::getRemoteAddress());
$content->assign('requestID', OC_Request::getRequestID());
$content->assign('requestID', \OC::$server->getRequest()->getId());
$content->printPage();
die();
}

7
lib/public/irequest.php
View file

@ -127,4 +127,11 @@ interface IRequest {
* @return bool true if CSRF check passed
*/
public function passesCSRFCheck();
/**
* Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
* If `mod_unique_id` is installed this value will be taken.
* @return string
*/
public function getId();
}

7
tests/lib/appframework/controller/ApiControllerTest.php
View file

@ -25,18 +25,19 @@
namespace OCP\AppFramework;
use OC\AppFramework\Http\Request;
use OCP\AppFramework\Http\TemplateResponse;
class ChildApiController extends ApiController {};
class ApiControllerTest extends \Test\TestCase {
/** @var ChildApiController */
protected $controller;
public function testCors() {
$request = new Request(
array('server' => array('HTTP_ORIGIN' => 'test'))
['server' => ['HTTP_ORIGIN' => 'test']],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->controller = new ChildApiController('app', $request, 'verbs',
'headers', 100);

17
tests/lib/appframework/controller/ControllerTest.php
View file

@ -66,15 +66,16 @@ class ControllerTest extends \Test\TestCase {
parent::setUp();
$request = new Request(
array(
'get' => array('name' => 'John Q. Public', 'nickname' => 'Joey'),
'post' => array('name' => 'Jane Doe', 'nickname' => 'Janey'),
'urlParams' => array('name' => 'Johnny Weissmüller'),
'files' => array('file' => 'filevalue'),
'env' => array('PATH' => 'daheim'),
'session' => array('sezession' => 'kein'),
[
'get' => ['name' => 'John Q. Public', 'nickname' => 'Joey'],
'post' => ['name' => 'Jane Doe', 'nickname' => 'Janey'],
'urlParams' => ['name' => 'Johnny Weissmüller'],
'files' => ['file' => 'filevalue'],
'env' => ['PATH' => 'daheim'],
'session' => ['sezession' => 'kein'],
'method' => 'hi',
)
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->app = $this->getMock('OC\AppFramework\DependencyInjection\DIContainer',

5
tests/lib/appframework/dependencyinjection/DIContainerTest.php
View file

@ -71,7 +71,10 @@ class DIContainerTest extends \Test\TestCase {
public function testMiddlewareDispatcherIncludesSecurityMiddleware(){
$this->container['Request'] = new Request(array('method' => 'GET'));
$this->container['Request'] = new Request(
['method' => 'GET'],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$security = $this->container['SecurityMiddleware'];
$dispatcher = $this->container['MiddlewareDispatcher'];

132
tests/lib/appframework/http/DispatcherTest.php
View file

@ -276,13 +276,16 @@ class DispatcherTest extends \Test\TestCase {
public function testControllerParametersInjected() {
$this->request = new Request(array(
'post' => array(
$this->request = new Request(
[
'post' => [
'int' => '3',
'bool' => 'false'
),
'method' => 'POST'
));
],
'method' => 'POST'
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->dispatcher = new Dispatcher(
$this->http, $this->middlewareDispatcher, $this->reflector,
$this->request
@ -298,14 +301,17 @@ class DispatcherTest extends \Test\TestCase {
public function testControllerParametersInjectedDefaultOverwritten() {
$this->request = new Request(array(
'post' => array(
'int' => '3',
'bool' => 'false',
'test2' => 7
),
'method' => 'POST'
));
$this->request = new Request(
[
'post' => [
'int' => '3',
'bool' => 'false',
'test2' => 7
],
'method' => 'POST',
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->dispatcher = new Dispatcher(
$this->http, $this->middlewareDispatcher, $this->reflector,
$this->request
@ -322,16 +328,19 @@ class DispatcherTest extends \Test\TestCase {
public function testResponseTransformedByUrlFormat() {
$this->request = new Request(array(
'post' => array(
'int' => '3',
'bool' => 'false'
),
'urlParams' => array(
'format' => 'text'
),
'method' => 'GET'
));
$this->request = new Request(
[
'post' => [
'int' => '3',
'bool' => 'false'
],
'urlParams' => [
'format' => 'text'
],
'method' => 'GET'
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->dispatcher = new Dispatcher(
$this->http, $this->middlewareDispatcher, $this->reflector,
$this->request
@ -347,16 +356,19 @@ class DispatcherTest extends \Test\TestCase {
public function testResponseTransformsDataResponse() {
$this->request = new Request(array(
'post' => array(
'int' => '3',
'bool' => 'false'
),
'urlParams' => array(
'format' => 'json'
),
'method' => 'GET'
));
$this->request = new Request(
[
'post' => [
'int' => '3',
'bool' => 'false'
],
'urlParams' => [
'format' => 'json'
],
'method' => 'GET'
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->dispatcher = new Dispatcher(
$this->http, $this->middlewareDispatcher, $this->reflector,
$this->request
@ -372,17 +384,20 @@ class DispatcherTest extends \Test\TestCase {
public function testResponseTransformedByAcceptHeader() {
$this->request = new Request(array(
'post' => array(
'int' => '3',
'bool' => 'false'
),
'server' => array(
'HTTP_ACCEPT' => 'application/text, test',
'HTTP_CONTENT_TYPE' => 'application/x-www-form-urlencoded'
),
'method' => 'PUT'
));
$this->request = new Request(
[
'post' => [
'int' => '3',
'bool' => 'false'
],
'server' => [
'HTTP_ACCEPT' => 'application/text, test',
'HTTP_CONTENT_TYPE' => 'application/x-www-form-urlencoded'
],
'method' => 'PUT'
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->dispatcher = new Dispatcher(
$this->http, $this->middlewareDispatcher, $this->reflector,
$this->request
@ -398,19 +413,22 @@ class DispatcherTest extends \Test\TestCase {
public function testResponsePrimarilyTransformedByParameterFormat() {
$this->request = new Request(array(
'post' => array(
'int' => '3',
'bool' => 'false'
),
'get' => array(
'format' => 'text'
),
'server' => array(
'HTTP_ACCEPT' => 'application/json, test'
),
'method' => 'POST'
));
$this->request = new Request(
[
'post' => [
'int' => '3',
'bool' => 'false'
],
'get' => [
'format' => 'text'
],
'server' => [
'HTTP_ACCEPT' => 'application/json, test'
],
'method' => 'POST'
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->dispatcher = new Dispatcher(
$this->http, $this->middlewareDispatcher, $this->reflector,
$this->request

68
tests/lib/appframework/http/RequestTest.php
View file

@ -8,9 +8,13 @@
namespace OC\AppFramework\Http;
global $data;
use OCP\Security\ISecureRandom;
class RequestTest extends \Test\TestCase {
/** @var string */
protected $stream = 'fakeinput://data';
/** @var ISecureRandom */
protected $secureRandom;
protected function setUp() {
parent::setUp();
@ -20,7 +24,8 @@ class RequestTest extends \Test\TestCase {
stream_wrapper_unregister('fakeinput');
}
stream_wrapper_register('fakeinput', 'RequestStream');
$this->stream = 'fakeinput://data';
$this->secureRandom = $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock();
}
protected function tearDown() {
@ -34,7 +39,7 @@ class RequestTest extends \Test\TestCase {
'method' => 'GET',
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
// Countable
$this->assertEquals(2, count($request));
@ -61,7 +66,7 @@ class RequestTest extends \Test\TestCase {
'method' => 'GET'
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals(3, count($request));
$this->assertEquals('Janey', $request->{'nickname'});
@ -78,7 +83,7 @@ class RequestTest extends \Test\TestCase {
'method' => 'GET'
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$request['nickname'] = 'Janey';
}
@ -91,7 +96,7 @@ class RequestTest extends \Test\TestCase {
'method' => 'GET'
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$request->{'nickname'} = 'Janey';
}
@ -104,7 +109,7 @@ class RequestTest extends \Test\TestCase {
'method' => 'GET',
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$result = $request->post;
}
@ -114,7 +119,7 @@ class RequestTest extends \Test\TestCase {
'method' => 'GET',
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals('GET', $request->method);
$result = $request->get;
$this->assertEquals('John Q. Public', $result['name']);
@ -129,7 +134,7 @@ class RequestTest extends \Test\TestCase {
'server' => array('CONTENT_TYPE' => 'application/json; utf-8')
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals('POST', $request->method);
$result = $request->post;
$this->assertEquals('John Q. Public', $result['name']);
@ -147,7 +152,7 @@ class RequestTest extends \Test\TestCase {
'server' => array('CONTENT_TYPE' => 'application/x-www-form-urlencoded'),
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals('PATCH', $request->method);
$result = $request->patch;
@ -166,7 +171,7 @@ class RequestTest extends \Test\TestCase {
'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals('PUT', $request->method);
$result = $request->put;
@ -181,7 +186,7 @@ class RequestTest extends \Test\TestCase {
'server' => array('CONTENT_TYPE' => 'application/json; utf-8'),
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals('PATCH', $request->method);
$result = $request->patch;
@ -200,7 +205,7 @@ class RequestTest extends \Test\TestCase {
'server' => array('CONTENT_TYPE' => 'image/png'),
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertEquals('PUT', $request->method);
$resource = $request->put;
$contents = stream_get_contents($resource);
@ -223,7 +228,7 @@ class RequestTest extends \Test\TestCase {
'urlParams' => array('id' => '2'),
);
$request = new Request($vars, $this->stream);
$request = new Request($vars, $this->secureRandom, $this->stream);
$newParams = array('id' => '3', 'test' => 'test2');
$request->setUrlParameters($newParams);
@ -231,4 +236,39 @@ class RequestTest extends \Test\TestCase {
$this->assertEquals('3', $request->getParam('id'));
$this->assertEquals('3', $request->getParams()['id']);
}
public function testGetIdWithModUnique() {
$vars = [
'server' => [
'UNIQUE_ID' => 'GeneratedUniqueIdByModUnique'
],
];
$request = new Request($vars, $this->secureRandom, $this->stream);
$this->assertSame('GeneratedUniqueIdByModUnique', $request->getId());
}
public function testGetIdWithoutModUnique() {
$lowRandomSource = $this->getMockBuilder('\OCP\Security\ISecureRandom')
->disableOriginalConstructor()->getMock();
$lowRandomSource->expects($this->once())
->method('generate')
->with('20')
->will($this->returnValue('GeneratedByOwnCloudItself'));
$this->secureRandom
->expects($this->once())
->method('getLowStrengthGenerator')
->will($this->returnValue($lowRandomSource));
$request = new Request([], $this->secureRandom, $this->stream);
$this->assertSame('GeneratedByOwnCloudItself', $request->getId());
}
public function testGetIdWithoutModUniqueStable() {
$request = new Request([], \OC::$server->getSecureRandom(), $this->stream);
$firstId = $request->getId();
$secondId = $request->getId();
$this->assertSame($firstId, $secondId);
}
}

12
tests/lib/appframework/middleware/MiddlewareDispatcherTest.php
View file

@ -126,8 +126,16 @@ class MiddlewareDispatcherTest extends \Test\TestCase {
private function getControllerMock(){
return $this->getMock('OCP\AppFramework\Controller', array('method'),
array('app', new Request(array('method' => 'GET'))));
return $this->getMock(
'OCP\AppFramework\Controller',
['method'],
['app',
new Request(
['method' => 'GET'],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
)
]
);
}

10
tests/lib/appframework/middleware/MiddlewareTest.php
View file

@ -51,8 +51,14 @@ class MiddlewareTest extends \Test\TestCase {
->disableOriginalConstructor()
->getMock();
$this->controller = $this->getMock('OCP\AppFramework\Controller',
array(), array($this->api, new Request()));
$this->controller = $this->getMock(
'OCP\AppFramework\Controller',
[],
[
$this->api,
new Request([], $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock())
]
);
$this->exception = new \Exception();
$this->response = $this->getMock('OCP\AppFramework\Http\Response');
}

23
tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
View file

@ -32,7 +32,12 @@ class CORSMiddlewareTest extends \Test\TestCase {
*/
public function testSetCORSAPIHeader() {
$request = new Request(
array('server' => array('HTTP_ORIGIN' => 'test'))
[
'server' => [
'HTTP_ORIGIN' => 'test'
]
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->reflector->reflect($this, __FUNCTION__);
$middleware = new CORSMiddleware($request, $this->reflector);
@ -45,7 +50,12 @@ class CORSMiddlewareTest extends \Test\TestCase {
public function testNoAnnotationNoCORSHEADER() {
$request = new Request(
array('server' => array('HTTP_ORIGIN' => 'test'))
[
'server' => [
'HTTP_ORIGIN' => 'test'
]
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$middleware = new CORSMiddleware($request, $this->reflector);
@ -59,7 +69,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
* @CORS
*/
public function testNoOriginHeaderNoCORSHEADER() {
$request = new Request();
$request = new Request([], $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock());
$this->reflector->reflect($this, __FUNCTION__);
$middleware = new CORSMiddleware($request, $this->reflector);
@ -75,7 +85,12 @@ class CORSMiddlewareTest extends \Test\TestCase {
*/
public function testCorsIgnoredIfWithCredentialsHeaderPresent() {
$request = new Request(
array('server' => array('HTTP_ORIGIN' => 'test'))
[
'server' => [
'HTTP_ORIGIN' => 'test'
]
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->reflector->reflect($this, __FUNCTION__);
$middleware = new CORSMiddleware($request, $this->reflector);

12
tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
View file

@ -314,10 +314,14 @@ class SecurityMiddlewareTest extends \Test\TestCase {
public function testAfterExceptionReturnsRedirect(){
$this->request = new Request(
array('server' =>
array('HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
'REQUEST_URI' => 'owncloud/index.php/apps/specialapp')
)
[
'server' =>
[
'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
'REQUEST_URI' => 'owncloud/index.php/apps/specialapp'
]
],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->middleware = $this->getMiddleware(true, true);
$response = $this->middleware->afterException($this->controller, 'test',

5
tests/lib/appframework/middleware/sessionmiddlewaretest.php
View file

@ -33,7 +33,10 @@ class SessionMiddlewareTest extends \Test\TestCase {
protected function setUp() {
parent::setUp();
$this->request = new Request();
$this->request = new Request(
[],
$this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock()
);
$this->reflector = new ControllerMethodReflector();
}