From 4dde2c49dbcbc537ba74282a1e684ecb52713b2e Mon Sep 17 00:00:00 2001 From: Thomas Ingles Date: Wed, 24 Apr 2024 00:25:21 +0200 Subject: [PATCH] CI 3.2.1-dev cookie SameSite & httponly + config (php>7.2) Inspired by : https://github.com/bcit-ci/CodeIgniter/blob/develop/ + +https://stackoverflow.com/questions/60634341/codeigniter-3-samesite-attribute-for-csrf-protection#63010341 + + See : at end of https://github.com/bcit-ci/CodeIgniter/issues/5791 + + https://github.com/bcit-ci/CodeIgniter/commit/1a2651040ef701e750b1c13cd69cc70814b079d0 + + https://github.com/bcit-ci/CodeIgniter/commit/0286ab3513ade8681a7172c78440a81059435e22 + + https://github.com/bcit-ci/CodeIgniter/pull/6025/files --- application/config/config.php | 9 +++++ system/core/CodeIgniter.php | 2 +- system/core/Input.php | 33 ++++++++++++++-- system/core/Security.php | 13 +++--- system/helpers/cookie_helper.php | 4 +- system/libraries/Session/Session.php | 44 +++++++++++++++------ system/libraries/Session/Session_driver.php | 13 +++--- 7 files changed, 90 insertions(+), 28 deletions(-) diff --git a/application/config/config.php b/application/config/config.php index 4ca2c8022d..7ae16ff71d 100644 --- a/application/config/config.php +++ b/application/config/config.php @@ -345,10 +345,12 @@ | 'sess_match_ip' = Whether to match the user's IP address when reading the session data | 'sess_match_useragent' = Whether to match the User Agent when reading the session data | 'sess_time_to_update' = how many seconds between CI refreshing Session Information +| 'sess_samesite' = Session cookie SameSite attribute: Lax (default), Strict or None | */ $config['sess_driver'] = 'files'; $config['sess_cookie_name'] = 'ea_session'; +$config['sess_samesite'] = 'Strict'; $config['sess_expiration'] = 7200; $config['sess_save_path'] = __DIR__ . '/../../storage/sessions'; $config['sess_match_ip'] = false; @@ -364,12 +366,19 @@ | 'cookie_domain' = Set to .your-domain.com for site-wide cookies | 'cookie_path' = Typically will be a forward slash | 'cookie_secure' = Cookies will only be set if a secure HTTPS connection exists. +| 'cookie_httponly' = Cookie will only be accessible via HTTP(S) (no javascript) +| 'cookie_samesite' = Cookie's samesite attribute (Lax, Strict or None) +| +| Note: These settings (with the exception of 'cookie_prefix' and +| 'cookie_httponly') will also affect sessions. | */ $config['cookie_prefix'] = ''; $config['cookie_domain'] = ''; $config['cookie_path'] = '/'; $config['cookie_secure'] = strpos($config['base_url'], 'https') !== false; +$config['cookie_httponly'] = true; +$config['cookie_samesite'] = 'Lax'; /* |-------------------------------------------------------------------------- diff --git a/system/core/CodeIgniter.php b/system/core/CodeIgniter.php index 704539ef48..17bdd98151 100644 --- a/system/core/CodeIgniter.php +++ b/system/core/CodeIgniter.php @@ -55,7 +55,7 @@ * @var string * */ - const CI_VERSION = '3.2.0-dev'; + const CI_VERSION = '3.2.1-dev'; /* * ------------------------------------------------------ diff --git a/system/core/Input.php b/system/core/Input.php index 3681f499ea..3f3c885454 100644 --- a/system/core/Input.php +++ b/system/core/Input.php @@ -300,14 +300,15 @@ public function input_stream($index = NULL, $xss_clean = FALSE) * @param string $prefix Cookie name prefix * @param bool $secure Whether to only transfer cookies via SSL * @param bool $httponly Whether to only makes the cookie accessible via HTTP (no javascript) + * @param string $samesite SameSite attribute * @return void */ - public function set_cookie($name, $value = '', $expire = 0, $domain = '', $path = '/', $prefix = '', $secure = NULL, $httponly = NULL) + public function set_cookie($name, $value = '', $expire = 0, $domain = '', $path = '/', $prefix = '', $secure = NULL, $httponly = NULL, $samesite = NULL) { if (is_array($name)) { // always leave 'name' in last place, as the loop will break otherwise, due to $$item - foreach (array('value', 'expire', 'domain', 'path', 'prefix', 'secure', 'httponly', 'name') as $item) + foreach (array('value', 'expire', 'domain', 'path', 'prefix', 'secure', 'httponly', 'name', 'samesite') as $item) { if (isset($name[$item])) { @@ -348,7 +349,33 @@ public function set_cookie($name, $value = '', $expire = 0, $domain = '', $path $expire = ($expire > 0) ? time() + $expire : 0; } - setcookie($prefix.$name, $value, $expire, $path, $domain, $secure, $httponly); + + isset($samesite) OR $samesite = config_item('cookie_samesite'); + if (isset($samesite)) + { + $samesite = ucfirst(strtolower($samesite)); + in_array($samesite, ['Lax', 'Strict', 'None'], TRUE) OR $samesite = 'Lax'; + } + else + { + $samesite = 'Lax'; + } + + if ($samesite === 'None' && ! $secure) + { + log_message('error', $name.' cookie sent with SameSite=None, but without Secure attribute.'); + } + + $setcookie_options = [ + 'expires' => $expire, + 'path' => $path, + 'domain' => $domain, + 'secure' => $secure, + 'httponly' => $httponly, + 'samesite' => $samesite, + ]; + setcookie($prefix.$name, $value, $setcookie_options); + } // -------------------------------------------------------------------- diff --git a/system/core/Security.php b/system/core/Security.php index 126fe444c6..2eada439c9 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -276,11 +276,14 @@ public function csrf_set_cookie() setcookie( $this->_csrf_cookie_name, $this->_csrf_hash, - $expire, - config_item('cookie_path'), - config_item('cookie_domain'), - $secure_cookie, - (bool)config_item('cookie_httponly') + [ + 'expires' => $expire, + 'path' => config_item('cookie_path'), + 'domain' => config_item('cookie_domain'), + 'secure' => $secure_cookie, + 'httponly' => (bool)config_item('cookie_httponly'), + 'samesite' => 'Strict' + ] ); log_message('info', 'CSRF cookie sent'); diff --git a/system/helpers/cookie_helper.php b/system/helpers/cookie_helper.php index 8183e05410..c798ed0f06 100644 --- a/system/helpers/cookie_helper.php +++ b/system/helpers/cookie_helper.php @@ -67,10 +67,10 @@ * @param bool true makes the cookie accessible via http(s) only (no javascript) * @return void */ - function set_cookie($name, $value = '', $expire = 0, $domain = '', $path = '/', $prefix = '', $secure = NULL, $httponly = NULL) + function set_cookie($name, $value = '', $expire = 0, $domain = '', $path = '/', $prefix = '', $secure = NULL, $httponly = NULL, $samesite = NULL) { // Set the config file options - get_instance()->input->set_cookie($name, $value, $expire, $domain, $path, $prefix, $secure, $httponly); + get_instance()->input->set_cookie($name, $value, $expire, $domain, $path, $prefix, $secure, $httponly, $samesite); } } diff --git a/system/libraries/Session/Session.php b/system/libraries/Session/Session.php index 6daf8dae8a..d99c9230f8 100644 --- a/system/libraries/Session/Session.php +++ b/system/libraries/Session/Session.php @@ -147,11 +147,14 @@ public function __construct(array $params = array()) setcookie( $this->_config['cookie_name'], session_id(), - (empty($this->_config['cookie_lifetime']) ? 0 : time() + $this->_config['cookie_lifetime']), - $this->_config['cookie_path'], - $this->_config['cookie_domain'], - $this->_config['cookie_secure'], - TRUE + [ + 'expires' => (empty($this->_config['cookie_lifetime']) ? 0 : time() + $this->_config['cookie_lifetime']), + 'path' => $this->_config['cookie_path'], + 'domain' => $this->_config['cookie_domain'], + 'secure' => $this->_config['cookie_secure'], + 'httponly' => TRUE, + 'samesite' => $this->_config['cookie_samesite'] + ] ); } @@ -267,13 +270,30 @@ protected function _configure(&$params) isset($params['cookie_domain']) OR $params['cookie_domain'] = config_item('cookie_domain'); isset($params['cookie_secure']) OR $params['cookie_secure'] = (bool) config_item('cookie_secure'); - session_set_cookie_params( - $params['cookie_lifetime'], - $params['cookie_path'], - $params['cookie_domain'], - $params['cookie_secure'], - TRUE // HttpOnly; Yes, this is intentional and not configurable for security reasons - ); + isset($params['cookie_samesite']) OR $params['cookie_samesite'] = config_item('sess_samesite'); + if ( ! isset($params['cookie_samesite'])) + { + $params['cookie_samesite'] = ini_get('session.cookie_samesite'); + } + + if (isset($params['cookie_samesite'])) + { + $params['cookie_samesite'] = ucfirst(strtolower($params['cookie_samesite'])); + in_array($params['cookie_samesite'], ['Lax', 'Strict', 'None'], TRUE) OR $params['cookie_samesite'] = 'Lax'; + } + else + { + $params['cookie_samesite'] = 'Lax'; + } + + session_set_cookie_params([ + 'lifetime' => $params['cookie_lifetime'], + 'path' => $params['cookie_path'], + 'domain' => $params['cookie_domain'], + 'secure' => $params['cookie_secure'], + 'httponly' => TRUE,// Yes, this is intentional and not configurable for security reasons + 'samesite' => $params['cookie_samesite'] + ]); if (empty($expiration)) { diff --git a/system/libraries/Session/Session_driver.php b/system/libraries/Session/Session_driver.php index 7bf56e9f71..1a0216b4ad 100644 --- a/system/libraries/Session/Session_driver.php +++ b/system/libraries/Session/Session_driver.php @@ -142,11 +142,14 @@ protected function _cookie_destroy() return @setcookie( $this->_config['cookie_name'], '', - 1, - $this->_config['cookie_path'], - $this->_config['cookie_domain'], - $this->_config['cookie_secure'], - TRUE + [ + 'expires' => 1, + 'path' => $this->_config['cookie_path'], + 'domain' => $this->_config['cookie_domain'], + 'secure' => $this->_config['cookie_secure'], + 'httponly' => TRUE, + 'samesite' => $this->_config['cookie_samesite'] + ] ); }