From 638b8387c36e6edafbde258595c74d011a680c13 Mon Sep 17 00:00:00 2001 From: Iain Kay Date: Thu, 11 Jul 2013 19:17:49 +0000 Subject: [PATCH 1/6] Updated global.inc.php to reflect the new values required for cookie configuration and documented each of the options. --- public/include/config/global.inc.dist.php | 37 ++++++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/public/include/config/global.inc.dist.php b/public/include/config/global.inc.dist.php index 1cf894bf..33aa1151 100644 --- a/public/include/config/global.inc.dist.php +++ b/public/include/config/global.inc.dist.php @@ -349,16 +349,43 @@ $config['memcache']['splay'] = 15; /** * Cookie configiration * - * For multiple installations of this cookie change the cookie name + * You can configure the cookie behaviour to secure your cookies more than the PHP defaults + * + * For multiple installations of mmcfe-ng on the same domain you must change the cookie + * path or change the cookie name to avoid conflicts. + * + * Description + * duration: the amount of time, in seconds, that a cookie should persist in the users + * browser. 0 = until closed; 1440 = 24 minutes. + * + * domain: the only domain name that may access this cookie in the browser + * + * path: the highest path on the domain that can access this cookie; i.e. if running + * two pools from a single domain you might set the path /ltc/ and /ftc/ to + * separate user session cookies between the two. + * + * httponly: marks the cookie as accessible only through the HTTP protocol. The cookie + * can't be accessed by scripting languages, such as JavaScript. This can + * help to reduce identity theft through XSS attacks in most browsers. + * + * secure: marks the cookie as accessible only through the HTTPS protocol. If you're + * using SSL this will stop a user accidently accessing the site without SSL + * and exposing their session cookie. * * Default: - * path = '/' - * name = 'POOLERCOOKIE' - * domain = '' + * duration = '1440' + * domain = '' + * path = '/' + * name = 'POOLERCOOKIE' + * httponly = true + * secure = false **/ +$config['cookie']['duration'] = '1440'; +$config['cookie']['domain'] = ''; $config['cookie']['path'] = '/'; $config['cookie']['name'] = 'POOLERCOOKIE'; -$config['cookie']['domain'] = ''; +$config['cookie']['httponly'] = true; +$config['cookie']['secure'] = false; /** * Enable or disable the Smarty cache From d2bbc366d16b83baa9c9f713355bdc1a32c27451 Mon Sep 17 00:00:00 2001 From: Iain Kay Date: Thu, 11 Jul 2013 19:26:09 +0000 Subject: [PATCH 2/6] Changed the Cookie Explanation in global.inc.php to be more in line with the rest of the structure. --- public/include/config/global.inc.dist.php | 31 +++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/public/include/config/global.inc.dist.php b/public/include/config/global.inc.dist.php index 33aa1151..a83f47be 100644 --- a/public/include/config/global.inc.dist.php +++ b/public/include/config/global.inc.dist.php @@ -354,23 +354,28 @@ $config['memcache']['splay'] = 15; * For multiple installations of mmcfe-ng on the same domain you must change the cookie * path or change the cookie name to avoid conflicts. * - * Description - * duration: the amount of time, in seconds, that a cookie should persist in the users - * browser. 0 = until closed; 1440 = 24 minutes. + * Explanation: + * duration: + * the amount of time, in seconds, that a cookie should persist in the users browser. + * 0 = until closed; 1440 = 24 minutes. * - * domain: the only domain name that may access this cookie in the browser + * domain: + * the only domain name that may access this cookie in the browser * - * path: the highest path on the domain that can access this cookie; i.e. if running - * two pools from a single domain you might set the path /ltc/ and /ftc/ to - * separate user session cookies between the two. + * path: + * the highest path on the domain that can access this cookie; i.e. if running two pools + * from a single domain you might set the path /ltc/ and /ftc/ to separate user session + * cookies between the two. * - * httponly: marks the cookie as accessible only through the HTTP protocol. The cookie - * can't be accessed by scripting languages, such as JavaScript. This can - * help to reduce identity theft through XSS attacks in most browsers. + * httponly: + * marks the cookie as accessible only through the HTTP protocol. The cookie can't be + * accessed by scripting languages, such as JavaScript. This can help to reduce identity + * theft through XSS attacks in most browsers. * - * secure: marks the cookie as accessible only through the HTTPS protocol. If you're - * using SSL this will stop a user accidently accessing the site without SSL - * and exposing their session cookie. + * secure: + * marks the cookie as accessible only through the HTTPS protocol. If you have a SSL + * certificate installed on your domain name then this will stop a user accidently + * accessing the site over a HTTP connection, without SSL, exposing their session cookie. * * Default: * duration = '1440' From 9f4789c707591f21e09c5b9e51aec93ee7d0c986 Mon Sep 17 00:00:00 2001 From: Iain Kay Date: Thu, 11 Jul 2013 19:29:24 +0000 Subject: [PATCH 3/6] In order to read the cookie configuration from include/config/globa.inc.php the session must begin after this has been included. --- public/index.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/public/index.php b/public/index.php index dbb28f9a..ad58b9e0 100644 --- a/public/index.php +++ b/public/index.php @@ -24,13 +24,13 @@ define("BASEPATH", "./"); // Our security check define("SECURITY", 1); +// Include our configuration (holding defines for the requires) +if (!include_once(BASEPATH . 'include/config/global.inc.php')) die('Unable to load site configuration'); + // Start a session session_start(); $session_id = session_id(); -// Include our configuration (holding defines for the requires) -if (!include_once(BASEPATH . 'include/config/global.inc.php')) die('Unable to load site configuration'); - // Load Classes, they name defines the $ variable used // We include all needed files here, even though our templates could load them themself require_once(INCLUDE_DIR . '/autoloader.inc.php'); From aac202da2b353eec3b302901ad8e464a7c86ada0 Mon Sep 17 00:00:00 2001 From: Iain Kay Date: Thu, 11 Jul 2013 19:34:58 +0000 Subject: [PATCH 4/6] Pull cookie session params from include/config/global.inc.php before session_start() --- public/index.php | 1 + 1 file changed, 1 insertion(+) diff --git a/public/index.php b/public/index.php index ad58b9e0..2f51e67f 100644 --- a/public/index.php +++ b/public/index.php @@ -28,6 +28,7 @@ define("SECURITY", 1); if (!include_once(BASEPATH . 'include/config/global.inc.php')) die('Unable to load site configuration'); // Start a session +session_set_cookie_params($config['cookie']['duration'], $config['cookie']['path'], $config['cookie']['domain'], $config['cookie']['secure'], $config['cookie']['httponly']); session_start(); $session_id = session_id(); From dfbaf621de80c4715ce020e6c85a240166f0e418 Mon Sep 17 00:00:00 2001 From: Iain Kay Date: Thu, 11 Jul 2013 19:41:50 +0000 Subject: [PATCH 5/6] When destroying a users session on the server we now also remove all session data immediately, rather than relying on garbage collection, and we destroy the cookie on the users browser. --- public/include/classes/user.class.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/public/include/classes/user.class.php b/public/include/classes/user.class.php index 68616e3e..86250001 100644 --- a/public/include/classes/user.class.php +++ b/public/include/classes/user.class.php @@ -387,7 +387,16 @@ class User { **/ public function logoutUser($redirect="index.php") { $this->debug->append("STA " . __METHOD__, 4); + // Unset all of the session variables + $_SESSION = array(); + // As we're killing the sesison, also kill the cookie! + if (ini_get("session.use_cookies")) { + $params = session_get_cookie_params(); + setcookie(session_name(), '', time() - 42000, $params["path"], $params["domain"], $params["secure"], $params["httponly"]); + } + // Destroy the session. session_destroy(); + // Enforce generation of a new Session ID and delete the old session_regenerate_id(true); // Enforce a page reload header("Location: $redirect"); From a635d2163c82301f0b473245ba6e69b61474ba8f Mon Sep 17 00:00:00 2001 From: Iain Kay Date: Thu, 11 Jul 2013 19:56:10 +0000 Subject: [PATCH 6/6] Added note about php.ini session.gc_maxlifetime value - Important to stop garbage collection removing cookies that should be valid. --- public/include/config/global.inc.dist.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/public/include/config/global.inc.dist.php b/public/include/config/global.inc.dist.php index a83f47be..28a2cd8c 100644 --- a/public/include/config/global.inc.dist.php +++ b/public/include/config/global.inc.dist.php @@ -357,7 +357,8 @@ $config['memcache']['splay'] = 15; * Explanation: * duration: * the amount of time, in seconds, that a cookie should persist in the users browser. - * 0 = until closed; 1440 = 24 minutes. + * 0 = until closed; 1440 = 24 minutes. Check your php.ini 'session.gc_maxlifetime' value + * and ensure that it is at least the duration specified here. * * domain: * the only domain name that may access this cookie in the browser