From a7c25f9fff8a07d99a9a94c29256ecab8a3deccb Mon Sep 17 00:00:00 2001 From: Michael Berkowski Date: Tue, 11 Oct 2016 18:10:12 -0500 Subject: [PATCH 1/4] Adds secureCookie configurable option --- libs/config.sample.php | 1 + libs/csrf/csrfprotector.php | 5 ++++- test/csrfprotector_test.php | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/libs/config.sample.php b/libs/config.sample.php index e21ca93..5c2907f 100755 --- a/libs/config.sample.php +++ b/libs/config.sample.php @@ -19,6 +19,7 @@ "jsPath" => "../js/csrfprotector.js", "jsUrl" => "", "tokenLength" => 10, + "secureCookie" => false, "disabledJavascriptMessage" => "This site attempts to protect users against Cross-Site Request Forgeries attacks. In order to do so, you must have JavaScript enabled in your web browser otherwise this site will fail to work correctly for you. See details of your web browser for how to enable JavaScript.", diff --git a/libs/csrf/csrfprotector.php b/libs/csrf/csrfprotector.php index c21509a..a26726a 100755 --- a/libs/csrf/csrfprotector.php +++ b/libs/csrf/csrfprotector.php @@ -325,7 +325,10 @@ public static function refreshToken() //set token to cookie for client side processing setcookie(self::$config['CSRFP_TOKEN'], $token, - time() + self::$cookieExpiryTime); + time() + self::$cookieExpiryTime, + '', + '', + (array_key_exists('secureCookie', self::$config) ? (bool)self::$config['secureCookie'] : false)); } /* diff --git a/test/csrfprotector_test.php b/test/csrfprotector_test.php index 8db2894..0c8d2a4 100644 --- a/test/csrfprotector_test.php +++ b/test/csrfprotector_test.php @@ -24,6 +24,19 @@ public static function checkHeader($needle) } return false; } + + public static function getHeaderValue($needle) + { + $haystack = xdebug_get_headers(); + foreach ($haystack as $key => $value) { + if (strpos($value, $needle) === 0) { + // Deliberately overwrite to accept the last rather than first match + // as xdebug_get_headers() will accumulate all set headers + list(,$hvalue) = explode(':', $value, 2); + } + } + return $hvalue; + } } @@ -44,6 +57,7 @@ public function setUp() { csrfprotector::$config['jsPath'] = '../js/csrfprotector.js'; csrfprotector::$config['CSRFP_TOKEN'] = 'csrfp_token'; + csrfprotector::$config['secureCookie'] = false; @@ -54,6 +68,7 @@ public function setUp() $_POST[csrfprotector::$config['CSRFP_TOKEN']] = $_GET[csrfprotector::$config['CSRFP_TOKEN']] = '123'; $_SESSION[csrfprotector::$config['CSRFP_TOKEN']] = array('abc'); //token mismatch - leading to failed validation $_SERVER['SERVER_PROTOCOL'] = 'HTTP/1.1'; + $_SERVER['HTTPS'] = null; $this->config = include(__DIR__ .'/../libs/config.sample.php'); @@ -90,6 +105,15 @@ public function testRefreshToken() $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][1])); } + public function testSecureCookie() + { + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SESSION[csrfprotector::$config['CSRFP_TOKEN']] = array('123abcd'); + + csrfprotector::$config['secureCookie'] = true; + csrfprotector::refreshToken(); //will create new session and cookies + $this->assertRegExp('/; secure/', csrfp_wrapper::getHeaderValue('Set-Cookie')); + } /** * test authorise post -> log directory exception From 004f4a8c2de445a2571874d31c5175efc8744783 Mon Sep 17 00:00:00 2001 From: Michael Berkowski Date: Tue, 11 Oct 2016 18:16:59 -0500 Subject: [PATCH 2/4] Adds negative test for secure cookie flag --- test/csrfprotector_test.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/csrfprotector_test.php b/test/csrfprotector_test.php index 0c8d2a4..7a94989 100644 --- a/test/csrfprotector_test.php +++ b/test/csrfprotector_test.php @@ -105,13 +105,20 @@ public function testRefreshToken() $this->assertTrue(csrfp_wrapper::checkHeader($_SESSION[csrfprotector::$config['CSRFP_TOKEN']][1])); } + /** + * test secure flag is set in the token cookie when requested + */ public function testSecureCookie() { $_SERVER['REQUEST_METHOD'] = 'POST'; $_SESSION[csrfprotector::$config['CSRFP_TOKEN']] = array('123abcd'); + csrfprotector::$config['secureCookie'] = false; + csrfprotector::refreshToken(); + $this->assertNotRegExp('/; secure/', csrfp_wrapper::getHeaderValue('Set-Cookie')); + csrfprotector::$config['secureCookie'] = true; - csrfprotector::refreshToken(); //will create new session and cookies + csrfprotector::refreshToken(); $this->assertRegExp('/; secure/', csrfp_wrapper::getHeaderValue('Set-Cookie')); } From c5039101f194eda4a33a4bf344ffc5e1667dbd03 Mon Sep 17 00:00:00 2001 From: Michael Berkowski Date: Fri, 4 Nov 2016 07:59:11 -0500 Subject: [PATCH 3/4] secureCookie config docs --- libs/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/README.md b/libs/README.md index a988b97..6562a9d 100644 --- a/libs/README.md +++ b/libs/README.md @@ -16,5 +16,6 @@ CSRFProtector configuration - `jsPath`: location of the js file **relative** to `config.php`.
**Default:** `../js/csrfprotector.js` - `jsUrl`: **Absolute url** of the js file. (See [Setting up](https://github.com/mebjas/CSRF-Protector-PHP/wiki/Setting-up-CSRF-Protector-PHP-in-your-web-application) for more information) - `tokenLength`: length of csrfp token, Default `10` + - `secureCookie`: sets the "secure" HTTPS flag on the cookie.
**Default: `false`** - `disabledJavascriptMessage`: messaged to be shown if js is disabled (string) - `verifyGetFor`: regex rules for those urls for which csrfp validation should be enabled for `GET` requests also. (View [verifyGetFor rules](https://github.com/mebjas/CSRF-Protector-PHP/wiki/verifyGetFor-rules) for more information) From 5007eb1597993abbf882fa8d1ac9b56078a053a4 Mon Sep 17 00:00:00 2001 From: Michael Berkowski Date: Fri, 4 Nov 2016 08:09:08 -0500 Subject: [PATCH 4/4] Test function documentation for HTTP header helpers --- test/csrfprotector_test.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/csrfprotector_test.php b/test/csrfprotector_test.php index 7a94989..0d6d9cb 100644 --- a/test/csrfprotector_test.php +++ b/test/csrfprotector_test.php @@ -15,6 +15,10 @@ public static function changeRequestType($type) self::$requestType = $type; } + /** + * Function to check for a string value anywhere within HTTP response headers + * Returns true on first match of $needle in header names or values + */ public static function checkHeader($needle) { $haystack = xdebug_get_headers(); @@ -25,6 +29,10 @@ public static function checkHeader($needle) return false; } + /** + * Function to return the string value of the last response header + * identified by name $needle + */ public static function getHeaderValue($needle) { $haystack = xdebug_get_headers();