From 8133d46620efa39b74dbb216acbed82efad8c4d2 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 21 Oct 2015 17:07:23 +0200 Subject: [PATCH 1/2] Remove dependency on ICrypto + use XOR --- lib/private/appframework/http/request.php | 15 +--- lib/private/server.php | 2 - lib/private/util.php | 19 +++-- .../controller/ApiControllerTest.php | 1 - .../controller/ControllerTest.php | 1 - .../controller/OCSControllerTest.php | 4 - .../dependencyinjection/DIContainerTest.php | 1 - .../lib/appframework/http/DispatcherTest.php | 6 -- tests/lib/appframework/http/RequestTest.php | 84 ++----------------- .../middleware/MiddlewareDispatcherTest.php | 1 - .../middleware/MiddlewareTest.php | 1 - .../security/CORSMiddlewareTest.php | 10 --- .../security/SecurityMiddlewareTest.php | 1 - .../middleware/sessionmiddlewaretest.php | 1 - tests/lib/util.php | 2 +- 15 files changed, 22 insertions(+), 127 deletions(-) diff --git a/lib/private/appframework/http/request.php b/lib/private/appframework/http/request.php index 7778513516..96620838df 100644 --- a/lib/private/appframework/http/request.php +++ b/lib/private/appframework/http/request.php @@ -88,20 +88,17 @@ class Request implements \ArrayAccess, \Countable, IRequest { * - string 'method' the request method (GET, POST etc) * - string|false 'requesttoken' the requesttoken or false when not available * @param ISecureRandom $secureRandom - * @param ICrypto $crypto * @param IConfig $config * @param string $stream * @see http://www.php.net/manual/en/reserved.variables.php */ public function __construct(array $vars=array(), ISecureRandom $secureRandom = null, - ICrypto $crypto, IConfig $config, $stream='php://input') { $this->inputStream = $stream; $this->items['params'] = array(); $this->secureRandom = $secureRandom; - $this->crypto = $crypto; $this->config = $config; if(!array_key_exists('method', $vars)) { @@ -439,22 +436,18 @@ class Request implements \ArrayAccess, \Countable, IRequest { return false; } - // Decrypt token to prevent BREACH like attacks + // Deobfuscate token to prevent BREACH like attacks $token = explode(':', $token); if (count($token) !== 2) { return false; } - $encryptedToken = $token[0]; + $obfuscatedToken = $token[0]; $secret = $token[1]; - try { - $decryptedToken = $this->crypto->decrypt($encryptedToken, $secret); - } catch (\Exception $e) { - return false; - } + $deobfuscatedToken = base64_decode($obfuscatedToken) ^ $secret; // Check if the token is valid - if(\OCP\Security\StringUtils::equals($decryptedToken, $this->items['requesttoken'])) { + if(\OCP\Security\StringUtils::equals($deobfuscatedToken, $this->items['requesttoken'])) { return true; } else { return false; diff --git a/lib/private/server.php b/lib/private/server.php index 14fa323f74..15ee3454d8 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -438,7 +438,6 @@ class Server extends SimpleContainer implements IServerContainer { 'requesttoken' => $requestToken, ], $this->getSecureRandom(), - $this->getCrypto(), $this->getConfig(), $stream ); @@ -512,7 +511,6 @@ class Server extends SimpleContainer implements IServerContainer { : null, ], new SecureRandom(), - $c->getCrypto(), $c->getConfig() ); diff --git a/lib/private/util.php b/lib/private/util.php index 05f10aef1e..e51edaf0ee 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -1093,7 +1093,7 @@ class OC_Util { return $id; } - protected static $encryptedToken; + protected static $obfuscatedToken; /** * Register an get/post call. Important to prevent CSRF attacks. * @@ -1107,24 +1107,27 @@ class OC_Util { */ public static function callRegister() { // Use existing token if function has already been called - if(isset(self::$encryptedToken)) { - return self::$encryptedToken; + if(isset(self::$obfuscatedToken)) { + return self::$obfuscatedToken; } + $tokenLength = 30; + // Check if a token exists if (!\OC::$server->getSession()->exists('requesttoken')) { // No valid token found, generate a new one. - $requestToken = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(30); + $requestToken = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate($tokenLength); \OC::$server->getSession()->set('requesttoken', $requestToken); } else { // Valid token already exists, send it $requestToken = \OC::$server->getSession()->get('requesttoken'); } - // Encrypt the token to mitigate breach-like attacks - $sharedSecret = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate(10); - self::$encryptedToken = \OC::$server->getCrypto()->encrypt($requestToken, $sharedSecret) . ':' . $sharedSecret; - return self::$encryptedToken; + // XOR the token to mitigate breach-like attacks + $sharedSecret = \OC::$server->getSecureRandom()->getMediumStrengthGenerator()->generate($tokenLength); + self::$obfuscatedToken = base64_encode($requestToken ^ $sharedSecret) .':'.$sharedSecret; + + return self::$obfuscatedToken; } /** diff --git a/tests/lib/appframework/controller/ApiControllerTest.php b/tests/lib/appframework/controller/ApiControllerTest.php index 573fe7f3ba..137e5950f6 100644 --- a/tests/lib/appframework/controller/ApiControllerTest.php +++ b/tests/lib/appframework/controller/ApiControllerTest.php @@ -38,7 +38,6 @@ class ApiControllerTest extends \Test\TestCase { $request = new Request( ['server' => ['HTTP_ORIGIN' => 'test']], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->controller = new ChildApiController('app', $request, 'verbs', diff --git a/tests/lib/appframework/controller/ControllerTest.php b/tests/lib/appframework/controller/ControllerTest.php index c847525c26..1493c0c317 100644 --- a/tests/lib/appframework/controller/ControllerTest.php +++ b/tests/lib/appframework/controller/ControllerTest.php @@ -76,7 +76,6 @@ class ControllerTest extends \Test\TestCase { 'method' => 'hi', ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); diff --git a/tests/lib/appframework/controller/OCSControllerTest.php b/tests/lib/appframework/controller/OCSControllerTest.php index 292a56e3ca..92b092cf0e 100644 --- a/tests/lib/appframework/controller/OCSControllerTest.php +++ b/tests/lib/appframework/controller/OCSControllerTest.php @@ -43,7 +43,6 @@ class OCSControllerTest extends \Test\TestCase { ], ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $controller = new ChildOCSController('app', $request, 'verbs', @@ -65,7 +64,6 @@ class OCSControllerTest extends \Test\TestCase { $controller = new ChildOCSController('app', new Request( [], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') )); $expected = "\n" . @@ -98,7 +96,6 @@ class OCSControllerTest extends \Test\TestCase { $controller = new ChildOCSController('app', new Request( [], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') )); $expected = "\n" . @@ -131,7 +128,6 @@ class OCSControllerTest extends \Test\TestCase { $controller = new ChildOCSController('app', new Request( [], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') )); $expected = '{"ocs":{"meta":{"status":"failure","statuscode":400,"message":"OK",' . diff --git a/tests/lib/appframework/dependencyinjection/DIContainerTest.php b/tests/lib/appframework/dependencyinjection/DIContainerTest.php index 598e70beff..0cbdddbb20 100644 --- a/tests/lib/appframework/dependencyinjection/DIContainerTest.php +++ b/tests/lib/appframework/dependencyinjection/DIContainerTest.php @@ -74,7 +74,6 @@ class DIContainerTest extends \Test\TestCase { $this->container['Request'] = new Request( ['method' => 'GET'], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $security = $this->container['SecurityMiddleware']; diff --git a/tests/lib/appframework/http/DispatcherTest.php b/tests/lib/appframework/http/DispatcherTest.php index c25fd7b6f8..02c86df8e7 100644 --- a/tests/lib/appframework/http/DispatcherTest.php +++ b/tests/lib/appframework/http/DispatcherTest.php @@ -295,7 +295,6 @@ class DispatcherTest extends \Test\TestCase { 'method' => 'POST' ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->dispatcher = new Dispatcher( @@ -323,7 +322,6 @@ class DispatcherTest extends \Test\TestCase { 'method' => 'POST', ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->dispatcher = new Dispatcher( @@ -354,7 +352,6 @@ class DispatcherTest extends \Test\TestCase { 'method' => 'GET' ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->dispatcher = new Dispatcher( @@ -384,7 +381,6 @@ class DispatcherTest extends \Test\TestCase { 'method' => 'GET' ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->dispatcher = new Dispatcher( @@ -415,7 +411,6 @@ class DispatcherTest extends \Test\TestCase { 'method' => 'PUT' ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->dispatcher = new Dispatcher( @@ -448,7 +443,6 @@ class DispatcherTest extends \Test\TestCase { 'method' => 'POST' ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->dispatcher = new Dispatcher( diff --git a/tests/lib/appframework/http/RequestTest.php b/tests/lib/appframework/http/RequestTest.php index bb9910b6a4..f628a30f1d 100644 --- a/tests/lib/appframework/http/RequestTest.php +++ b/tests/lib/appframework/http/RequestTest.php @@ -54,7 +54,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -87,7 +86,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -110,7 +108,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -130,7 +127,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -150,7 +146,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -167,7 +162,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -189,7 +183,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -213,7 +206,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -235,7 +227,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -260,7 +251,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -281,7 +271,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -306,7 +295,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -336,7 +324,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -358,7 +345,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( $vars, $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -382,7 +368,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -394,7 +379,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], \OC::$server->getSecureRandom(), - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -419,7 +403,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -448,7 +431,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -477,7 +459,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -510,7 +491,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -561,7 +541,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -589,7 +568,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -611,7 +589,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -622,7 +599,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -646,7 +622,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -667,7 +642,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -684,7 +658,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -705,7 +678,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -727,7 +699,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -816,7 +787,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -833,7 +803,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -851,7 +820,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -869,7 +837,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -897,7 +864,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -919,7 +885,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -946,7 +911,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -973,7 +937,6 @@ class RequestTest extends \Test\TestCase { ], ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -990,7 +953,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1018,7 +980,6 @@ class RequestTest extends \Test\TestCase { $request = new Request( [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1034,7 +995,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1055,7 +1015,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1076,7 +1035,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1099,7 +1057,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1122,7 +1079,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1145,7 +1101,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1168,7 +1123,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1223,7 +1177,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ); @@ -1263,7 +1216,6 @@ class RequestTest extends \Test\TestCase { ] ], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ]) @@ -1277,25 +1229,17 @@ class RequestTest extends \Test\TestCase { } public function testPassesCSRFCheckWithGet() { - $crypto = $this->getMock('\OCP\Security\ICrypto'); - $crypto - ->expects($this->once()) - ->method('decrypt') - ->with('1c637c4147e40a8a8f09428ec2059cebea3480c27b402b4e793c69710a731513|wlXxNUaFqHuQnZr5|e6ab49c9e0e20c8d3607e02f1d8e6ec17ad6020ae10b7d64ab4b0a6318c0875940943a6aa303dc090fea0b4cd5b9fb8bcbecac4308a2bd15d9f369cdc22121a4', 'secret') - ->will($this->returnValue('MyStoredRequestToken')); - /** @var Request $request */ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') ->setMethods(['getScriptName']) ->setConstructorArgs([ [ 'get' => [ - 'requesttoken' => '1c637c4147e40a8a8f09428ec2059cebea3480c27b402b4e793c69710a731513|wlXxNUaFqHuQnZr5|e6ab49c9e0e20c8d3607e02f1d8e6ec17ad6020ae10b7d64ab4b0a6318c0875940943a6aa303dc090fea0b4cd5b9fb8bcbecac4308a2bd15d9f369cdc22121a4:secret', + 'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'requesttoken' => 'MyStoredRequestToken', ], $this->secureRandom, - $crypto, $this->config, $this->stream ]) @@ -1305,25 +1249,17 @@ class RequestTest extends \Test\TestCase { } public function testPassesCSRFCheckWithPost() { - $crypto = $this->getMock('\OCP\Security\ICrypto'); - $crypto - ->expects($this->once()) - ->method('decrypt') - ->with('1c637c4147e40a8a8f09428ec2059cebea3480c27b402b4e793c69710a731513|wlXxNUaFqHuQnZr5|e6ab49c9e0e20c8d3607e02f1d8e6ec17ad6020ae10b7d64ab4b0a6318c0875940943a6aa303dc090fea0b4cd5b9fb8bcbecac4308a2bd15d9f369cdc22121a4', 'secret') - ->will($this->returnValue('MyStoredRequestToken')); - /** @var Request $request */ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') ->setMethods(['getScriptName']) ->setConstructorArgs([ [ 'post' => [ - 'requesttoken' => '1c637c4147e40a8a8f09428ec2059cebea3480c27b402b4e793c69710a731513|wlXxNUaFqHuQnZr5|e6ab49c9e0e20c8d3607e02f1d8e6ec17ad6020ae10b7d64ab4b0a6318c0875940943a6aa303dc090fea0b4cd5b9fb8bcbecac4308a2bd15d9f369cdc22121a4:secret', + 'requesttoken' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'requesttoken' => 'MyStoredRequestToken', ], $this->secureRandom, - $crypto, $this->config, $this->stream ]) @@ -1333,24 +1269,17 @@ class RequestTest extends \Test\TestCase { } public function testPassesCSRFCheckWithHeader() { - $crypto = $this->getMock('\OCP\Security\ICrypto'); - $crypto - ->expects($this->once()) - ->method('decrypt') - ->with('1c637c4147e40a8a8f09428ec2059cebea3480c27b402b4e793c69710a731513|wlXxNUaFqHuQnZr5|e6ab49c9e0e20c8d3607e02f1d8e6ec17ad6020ae10b7d64ab4b0a6318c0875940943a6aa303dc090fea0b4cd5b9fb8bcbecac4308a2bd15d9f369cdc22121a4', 'secret') - ->will($this->returnValue('MyStoredRequestToken')); /** @var Request $request */ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') ->setMethods(['getScriptName']) ->setConstructorArgs([ [ 'server' => [ - 'HTTP_REQUESTTOKEN' => '1c637c4147e40a8a8f09428ec2059cebea3480c27b402b4e793c69710a731513|wlXxNUaFqHuQnZr5|e6ab49c9e0e20c8d3607e02f1d8e6ec17ad6020ae10b7d64ab4b0a6318c0875940943a6aa303dc090fea0b4cd5b9fb8bcbecac4308a2bd15d9f369cdc22121a4:secret', + 'HTTP_REQUESTTOKEN' => 'AAAHGxsTCTc3BgMQESAcNR0OAR0=:MyTotalSecretShareds', ], 'requesttoken' => 'MyStoredRequestToken', ], $this->secureRandom, - $crypto, $this->config, $this->stream ]) @@ -1359,6 +1288,9 @@ class RequestTest extends \Test\TestCase { $this->assertTrue($request->passesCSRFCheck()); } + /** + * @return array + */ public function invalidTokenDataProvider() { return [ ['InvalidSentToken'], @@ -1373,8 +1305,6 @@ class RequestTest extends \Test\TestCase { * @param string $invalidToken */ public function testPassesCSRFCheckWithInvalidToken($invalidToken) { - $crypto = new Crypto($this->config, $this->secureRandom); - /** @var Request $request */ $request = $this->getMockBuilder('\OC\AppFramework\Http\Request') ->setMethods(['getScriptName']) @@ -1386,7 +1316,6 @@ class RequestTest extends \Test\TestCase { 'requesttoken' => 'MyStoredRequestToken', ], $this->secureRandom, - $crypto, $this->config, $this->stream ]) @@ -1402,7 +1331,6 @@ class RequestTest extends \Test\TestCase { ->setConstructorArgs([ [], $this->secureRandom, - $this->getMock('\OCP\Security\ICrypto'), $this->config, $this->stream ]) diff --git a/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php b/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php index eab45b03c9..a873152579 100644 --- a/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php +++ b/tests/lib/appframework/middleware/MiddlewareDispatcherTest.php @@ -133,7 +133,6 @@ class MiddlewareDispatcherTest extends \Test\TestCase { new Request( ['method' => 'GET'], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ) ] diff --git a/tests/lib/appframework/middleware/MiddlewareTest.php b/tests/lib/appframework/middleware/MiddlewareTest.php index 8e077b37e2..33f04e1383 100644 --- a/tests/lib/appframework/middleware/MiddlewareTest.php +++ b/tests/lib/appframework/middleware/MiddlewareTest.php @@ -61,7 +61,6 @@ class MiddlewareTest extends \Test\TestCase { new Request( [], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ) ] diff --git a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php index f5e6106dc3..ca526fb859 100644 --- a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php +++ b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php @@ -42,7 +42,6 @@ class CORSMiddlewareTest extends \Test\TestCase { ] ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->reflector->reflect($this, __FUNCTION__); @@ -62,7 +61,6 @@ class CORSMiddlewareTest extends \Test\TestCase { ] ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $middleware = new CORSMiddleware($request, $this->reflector, $this->session); @@ -80,7 +78,6 @@ class CORSMiddlewareTest extends \Test\TestCase { $request = new Request( [], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->reflector->reflect($this, __FUNCTION__); @@ -104,7 +101,6 @@ class CORSMiddlewareTest extends \Test\TestCase { ] ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->reflector->reflect($this, __FUNCTION__); @@ -123,7 +119,6 @@ class CORSMiddlewareTest extends \Test\TestCase { $request = new Request( [], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->reflector->reflect($this, __FUNCTION__); @@ -149,7 +144,6 @@ class CORSMiddlewareTest extends \Test\TestCase { 'PHP_AUTH_PW' => 'pass' ]], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->session->expects($this->once()) @@ -175,7 +169,6 @@ class CORSMiddlewareTest extends \Test\TestCase { 'PHP_AUTH_PW' => 'pass' ]], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->session->expects($this->once()) @@ -197,7 +190,6 @@ class CORSMiddlewareTest extends \Test\TestCase { 'PHP_AUTH_PW' => 'pass' ]], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $middleware = new CORSMiddleware($request, $this->reflector, $this->session); @@ -214,7 +206,6 @@ class CORSMiddlewareTest extends \Test\TestCase { 'PHP_AUTH_PW' => 'pass' ]], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $middleware = new CORSMiddleware($request, $this->reflector, $this->session); @@ -235,7 +226,6 @@ class CORSMiddlewareTest extends \Test\TestCase { 'PHP_AUTH_PW' => 'pass' ]], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $middleware = new CORSMiddleware($request, $this->reflector, $this->session); diff --git a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php index 3b4d7987e9..347a0423ea 100644 --- a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php +++ b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php @@ -322,7 +322,6 @@ class SecurityMiddlewareTest extends \Test\TestCase { ] ], $this->getMock('\OCP\Security\ISecureRandom'), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->middleware = $this->getMiddleware(true, true); diff --git a/tests/lib/appframework/middleware/sessionmiddlewaretest.php b/tests/lib/appframework/middleware/sessionmiddlewaretest.php index 06390e96d4..11c1600f51 100644 --- a/tests/lib/appframework/middleware/sessionmiddlewaretest.php +++ b/tests/lib/appframework/middleware/sessionmiddlewaretest.php @@ -36,7 +36,6 @@ class SessionMiddlewareTest extends \Test\TestCase { $this->request = new Request( [], $this->getMockBuilder('\OCP\Security\ISecureRandom')->getMock(), - $this->getMock('\OCP\Security\ICrypto'), $this->getMock('\OCP\IConfig') ); $this->reflector = new ControllerMethodReflector(); diff --git a/tests/lib/util.php b/tests/lib/util.php index eaa3d0795e..9974e799d0 100644 --- a/tests/lib/util.php +++ b/tests/lib/util.php @@ -91,7 +91,7 @@ class Test_Util extends \Test\TestCase { function testCallRegister() { $result = strlen(OC_Util::callRegister()); - $this->assertEquals(221, $result); + $this->assertEquals(71, $result); } function testSanitizeHTML() { From 23e22c52b07b963e022a17c7ee8cff19f1223484 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Wed, 21 Oct 2015 17:33:49 +0200 Subject: [PATCH 2/2] Use IRequest's `getScriptName` functionality instead of $_SERVER['SCRIPT_NAME'] --- lib/base.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/base.php b/lib/base.php index 09e1d0aea4..95a47dec1e 100644 --- a/lib/base.php +++ b/lib/base.php @@ -133,7 +133,18 @@ class OC { OC_Config::$object = new \OC\Config(self::$configDir); OC::$SUBURI = str_replace("\\", "/", substr(realpath($_SERVER["SCRIPT_FILENAME"]), strlen(OC::$SERVERROOT))); - $scriptName = $_SERVER['SCRIPT_NAME']; + /** + * FIXME: The following lines are required because we can't yet instantiiate + * \OC::$server->getRequest() since \OC::$server does not yet exist. + */ + $params = [ + 'server' => [ + 'SCRIPT_NAME' => $_SERVER['SCRIPT_NAME'], + 'SCRIPT_FILENAME' => $_SERVER['SCRIPT_FILENAME'], + ], + ]; + $fakeRequest = new \OC\AppFramework\Http\Request($params, null, new \OC\AllConfig(new \OC\SystemConfig())); + $scriptName = $fakeRequest->getScriptName(); if (substr($scriptName, -1) == '/') { $scriptName .= 'index.php'; //make sure suburi follows the same rules as scriptName @@ -145,6 +156,7 @@ class OC { } } + if (OC::$CLI) { OC::$WEBROOT = OC_Config::getValue('overwritewebroot', ''); } else {