diff --git a/Cake/Controller/Component/SecurityComponent.php b/Cake/Controller/Component/SecurityComponent.php index 35305da0d41..0edac0a9834 100644 --- a/Cake/Controller/Component/SecurityComponent.php +++ b/Cake/Controller/Component/SecurityComponent.php @@ -29,7 +29,6 @@ * your application. It provides methods for various tasks like: * * - Restricting which HTTP methods your application accepts. - * - CSRF protection. * - Form tampering protection * - Requiring that SSL be used. * - Limiting cross controller communication. @@ -90,7 +89,7 @@ class SecurityComponent extends Component { public $unlockedFields = array(); /** - * Actions to exclude from CSRF and POST validation checks. + * Actions to exclude from POST validation checks. * Other checks like requireAuth(), requireSecure(), * requirePost(), requireGet() etc. will still be applied. * @@ -106,47 +105,6 @@ class SecurityComponent extends Component { */ public $validatePost = true; -/** - * Whether to use CSRF protected forms. Set to false to disable CSRF protection on forms. - * - * @var boolean - * @see http://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF) - * @see SecurityComponent::$csrfExpires - */ - public $csrfCheck = true; - -/** - * The duration from when a CSRF token is created that it will expire on. - * Each form/page request will generate a new token that can only be submitted once unless - * it expires. Can be any value compatible with strtotime() - * - * @var string - */ - public $csrfExpires = '+30 minutes'; - -/** - * Controls whether or not CSRF tokens are use and burn. Set to false to not generate - * new tokens on each request. One token will be reused until it expires. This reduces - * the chances of users getting invalid requests because of token consumption. - * It has the side effect of making CSRF less secure, as tokens are reusable. - * - * @var boolean - */ - public $csrfUseOnce = true; - -/** - * Control the number of tokens a user can keep open. - * This is most useful with one-time use tokens. Since new tokens - * are created on each request, having a hard limit on the number of open tokens - * can be useful in controlling the size of the session file. - * - * When tokens are evicted, the oldest ones will be removed, as they are the most likely - * to be dead/expired. - * - * @var integer - */ - public $csrfLimit = 100; - /** * Other components used by the Security component * @@ -195,9 +153,6 @@ public function startup(Event $event) { if ($this->validatePost && $this->_validatePost($controller) === false) { return $this->blackHole($controller, 'auth'); } - if ($this->csrfCheck && $this->_validateCsrf($controller) === false) { - return $this->blackHole($controller, 'csrf'); - } } $this->generateToken($controller->request); if ($isPost && is_array($controller->request->data)) { @@ -422,22 +377,11 @@ public function generateToken(Request $request) { 'allowedControllers' => $this->allowedControllers, 'allowedActions' => $this->allowedActions, 'unlockedFields' => $this->unlockedFields, - 'csrfTokens' => array() ); $tokenData = array(); if ($this->Session->check('_Token')) { $tokenData = $this->Session->read('_Token'); - if (!empty($tokenData['csrfTokens']) && is_array($tokenData['csrfTokens'])) { - $token['csrfTokens'] = $this->_expireTokens($tokenData['csrfTokens']); - } - } - if ($this->csrfUseOnce || empty($token['csrfTokens'])) { - $token['csrfTokens'][$authKey] = strtotime($this->csrfExpires); - } - if (!$this->csrfUseOnce) { - $csrfTokens = array_keys($token['csrfTokens']); - $token['key'] = $csrfTokens[0]; } $this->Session->write('_Token', $token); $request->params['_Token'] = array( @@ -447,47 +391,6 @@ public function generateToken(Request $request) { return true; } -/** - * Validate that the controller has a CSRF token in the POST data - * and that the token is legit/not expired. If the token is valid - * it will be removed from the list of valid tokens. - * - * @param Controller $controller A controller to check - * @return boolean Valid csrf token. - */ - protected function _validateCsrf(Controller $controller) { - $token = $this->Session->read('_Token'); - $requestToken = $controller->request->data('_Token.key'); - if (isset($token['csrfTokens'][$requestToken]) && $token['csrfTokens'][$requestToken] >= time()) { - if ($this->csrfUseOnce) { - $this->Session->delete('_Token.csrfTokens.' . $requestToken); - } - return true; - } - return false; - } - -/** - * Expire CSRF nonces and remove them from the valid tokens. - * Uses a simple timeout to expire the tokens. - * - * @param array $tokens An array of nonce => expires. - * @return array An array of nonce => expires. - */ - protected function _expireTokens($tokens) { - $now = time(); - foreach ($tokens as $nonce => $expires) { - if ($expires < $now) { - unset($tokens[$nonce]); - } - } - $overflow = count($tokens) - $this->csrfLimit; - if ($overflow > 0) { - $tokens = array_slice($tokens, $overflow + 1, null, true); - } - return $tokens; - } - /** * Calls a controller callback method * diff --git a/Cake/Test/TestCase/Controller/Component/SecurityComponentTest.php b/Cake/Test/TestCase/Controller/Component/SecurityComponentTest.php index 5bfb3da009b..0b0fdc3cb27 100644 --- a/Cake/Test/TestCase/Controller/Component/SecurityComponentTest.php +++ b/Cake/Test/TestCase/Controller/Component/SecurityComponentTest.php @@ -138,7 +138,6 @@ public function setUp() { $this->Controller->Security = $this->Controller->TestSecurity; $this->Controller->Security->blackHoleCallback = 'fail'; $this->Security = $this->Controller->Security; - $this->Security->csrfCheck = false; Configure::write('Session', [ 'defaults' => 'php' ]); @@ -978,242 +977,6 @@ public function testBlackHoleNotDeletingSessionInformation() { $this->Controller->Security->blackHole($this->Controller, 'auth'); $this->assertTrue($this->Controller->Security->Session->check('_Token'), '_Token was deleted by blackHole %s'); } - -/** - * test that csrf checks are skipped for request action. - * - * @return void - */ - public function testCsrfSkipRequestAction() { - $_SERVER['REQUEST_METHOD'] = 'POST'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $this->Controller->request->params['requested'] = 1; - $this->Security->startup($event); - - $this->assertFalse($this->Controller->failed, 'fail() was called.'); - } - -/** - * test setting - * - * @return void - */ - public function testCsrfSettings() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->startup($event); - - $token = $this->Security->Session->read('_Token'); - $this->assertEquals(1, count($token['csrfTokens']), 'Missing the csrf token.'); - $this->assertEquals(strtotime('+10 minutes'), current($token['csrfTokens']), 'Token expiry does not match'); - $this->assertEquals(array('key', 'unlockedFields'), array_keys($this->Controller->request->params['_Token']), 'Keys don not match'); - } - -/** - * Test setting multiple nonces, when startup() is called more than once, (ie more than one request.) - * - * @return void - */ - public function testCsrfSettingMultipleNonces() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $csrfExpires = strtotime('+10 minutes'); - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->startup($event); - $this->Security->startup($event); - - $token = $this->Security->Session->read('_Token'); - $this->assertEquals(2, count($token['csrfTokens']), 'Missing the csrf token.'); - foreach ($token['csrfTokens'] as $expires) { - $diff = $csrfExpires - $expires; - $this->assertTrue($diff === 0 || $diff === 1, 'Token expiry does not match'); - } - } - -/** - * test that nonces are consumed by form submits. - * - * @return void - */ - public function testCsrfNonceConsumption() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('+10 minutes'))); - - $this->Controller->request = $this->getMock('Cake\Network\Request', array('is')); - $this->Controller->request->expects($this->once())->method('is') - ->with(array('post', 'put')) - ->will($this->returnValue(true)); - - $this->Controller->request->params['action'] = 'index'; - $this->Controller->request->data = array( - '_Token' => array( - 'key' => 'nonce1' - ), - 'Post' => array( - 'title' => 'Woot' - ) - ); - $this->Security->startup($event); - $token = $this->Security->Session->read('_Token'); - $this->assertFalse(isset($token['csrfTokens']['nonce1']), 'Token was not consumed'); - } - -/** - * test that expired values in the csrfTokens are cleaned up. - * - * @return void - */ - public function testCsrfNonceVacuum() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->Session->write('_Token.csrfTokens', array( - 'valid' => strtotime('+30 minutes'), - 'poof' => strtotime('-11 minutes'), - 'dust' => strtotime('-20 minutes') - )); - $this->Security->startup($event); - $tokens = $this->Security->Session->read('_Token.csrfTokens'); - $this->assertEquals(2, count($tokens), 'Too many tokens left behind'); - $this->assertNotEmpty('valid', $tokens, 'Valid token was removed.'); - } - -/** - * test that when the key is missing the request is blackHoled - * - * @return void - */ - public function testCsrfBlackHoleOnKeyMismatch() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('+10 minutes'))); - - $this->Controller->request = $this->getMock('Cake\Network\Request', array('is')); - $this->Controller->request->expects($this->once())->method('is') - ->with(array('post', 'put')) - ->will($this->returnValue(true)); - - $this->Controller->request->params['action'] = 'index'; - $this->Controller->request->data = array( - '_Token' => array( - 'key' => 'not the right value' - ), - 'Post' => array( - 'title' => 'Woot' - ) - ); - $this->Security->startup($event); - $this->assertTrue($this->Controller->failed, 'fail() was not called.'); - } - -/** - * test that when the key is missing the request is blackHoled - * - * @return void - */ - public function testCsrfBlackHoleOnExpiredKey() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('-5 minutes'))); - - $this->Controller->request = $this->getMock('Cake\Network\Request', array('is')); - $this->Controller->request->expects($this->once())->method('is') - ->with(array('post', 'put')) - ->will($this->returnValue(true)); - - $this->Controller->request->params['action'] = 'index'; - $this->Controller->request->data = array( - '_Token' => array( - 'key' => 'nonce1' - ), - 'Post' => array( - 'title' => 'Woot' - ) - ); - $this->Security->startup($event); - $this->assertTrue($this->Controller->failed, 'fail() was not called.'); - } - -/** - * test that csrfUseOnce = false works. - * - * @return void - */ - public function testCsrfNotUseOnce() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfUseOnce = false; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - // Generate one token - $this->Security->startup($event); - $token = $this->Security->Session->read('_Token.csrfTokens'); - $this->assertEquals(1, count($token), 'Should only be one token.'); - - $this->Security->startup($event); - $tokenTwo = $this->Security->Session->read('_Token.csrfTokens'); - $this->assertEquals(1, count($tokenTwo), 'Should only be one token.'); - $this->assertEquals($token, $tokenTwo, 'Tokens should not be different.'); - - $key = $this->Controller->request->params['_Token']['key']; - $this->assertEquals(array($key), array_keys($token), '_Token.key and csrfToken do not match request will blackhole.'); - } - -/** - * ensure that longer session tokens are not consumed - * - * @return void - */ - public function testCsrfNotUseOnceValidationLeavingToken() { - $this->Security->validatePost = false; - $this->Security->csrfCheck = true; - $this->Security->csrfUseOnce = false; - $this->Security->csrfExpires = '+10 minutes'; - $event = new Event('Controller.startup', $this->Controller); - - $this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('+10 minutes'))); - - $this->Controller->request = $this->getMock('Cake\Network\Request', array('is')); - $this->Controller->request->expects($this->once())->method('is') - ->with(array('post', 'put')) - ->will($this->returnValue(true)); - - $this->Controller->request->params['action'] = 'index'; - $this->Controller->request->data = array( - '_Token' => array( - 'key' => 'nonce1' - ), - 'Post' => array( - 'title' => 'Woot' - ) - ); - $this->Security->startup($event); - $token = $this->Security->Session->read('_Token'); - $this->assertTrue(isset($token['csrfTokens']['nonce1']), 'Token was consumed'); - } - /** * Test generateToken() * @@ -1228,32 +991,6 @@ public function testGenerateToken() { $this->assertTrue(isset($request->params['_Token']['key'])); } -/** - * Test the limiting of CSRF tokens. - * - * @return void - */ - public function testCsrfLimit() { - $this->Security->csrfLimit = 3; - $time = strtotime('+10 minutes'); - $tokens = array( - '1' => $time, - '2' => $time, - '3' => $time, - '4' => $time, - '5' => $time, - ); - $this->Security->Session->write('_Token', array('csrfTokens' => $tokens)); - $this->Security->generateToken($this->Controller->request); - $result = $this->Security->Session->read('_Token.csrfTokens'); - - $this->assertFalse(isset($result['1'])); - $this->assertFalse(isset($result['2'])); - $this->assertFalse(isset($result['3'])); - $this->assertTrue(isset($result['4'])); - $this->assertTrue(isset($result['5'])); - } - /** * Test unlocked actions * @@ -1268,4 +1005,5 @@ public function testUnlockedActions() { $result = $this->Controller->Security->startup($event); $this->assertNull($result); } + }