From 1a2651040ef701e750b1c13cd69cc70814b079d0 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 5 Jan 2022 18:52:24 +0200 Subject: Add SameSite cookie support to Session library --- application/config/config.php | 5 ++ system/libraries/Session/Session.php | 83 ++++++++++++++++++++++------ system/libraries/Session/Session_driver.php | 24 ++++++-- user_guide_src/source/changelog.rst | 1 + user_guide_src/source/libraries/sessions.rst | 2 + 5 files changed, 94 insertions(+), 21 deletions(-) diff --git a/application/config/config.php b/application/config/config.php index 161b95699..ee599c7aa 100644 --- a/application/config/config.php +++ b/application/config/config.php @@ -341,6 +341,10 @@ $config['encryption_key'] = ''; | | The session cookie name, must contain only [0-9a-z_-] characters | +| 'sess_samesite' +| +| Session cookie SameSite attribute: Lax (default), Strict or None +| | 'sess_expiration' | | The number of SECONDS you want the session to last. @@ -381,6 +385,7 @@ $config['encryption_key'] = ''; */ $config['sess_driver'] = 'files'; $config['sess_cookie_name'] = 'ci_session'; +$config['sess_samesite'] = 'Lax'; $config['sess_expiration'] = 7200; $config['sess_save_path'] = NULL; $config['sess_match_ip'] = FALSE; diff --git a/system/libraries/Session/Session.php b/system/libraries/Session/Session.php index ed379146d..1c5c980ae 100644 --- a/system/libraries/Session/Session.php +++ b/system/libraries/Session/Session.php @@ -154,15 +154,36 @@ class CI_Session { // unless it is being currently created or regenerated elseif (isset($_COOKIE[$this->_config['cookie_name']]) && $_COOKIE[$this->_config['cookie_name']] === session_id()) { - 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']; + if (is_php('7.3')) + { + setcookie( + $this->_config['cookie_name'], + session_id(), + array( + 'expires' => $expires, + 'path' => $this->_config['cookie_path'], + 'domain' => $this->_config['cookie_domain'], + 'secure' => $this->_config['cookie_secure'], + 'httponly' => TRUE, + 'samesite' => $this->_config['cookie_samesite'] + ) + ); + } + else + { + $header = 'Set-Cookie: '.$this->_config['cookie_name'].'='.session_id(); + $header .= empty($expires) ? '' : '; Expires='.gmdate('D, d-M-Y H:i:s T', $expires).'; Max-Age='.$this->_config['cookie_lifetime']; + $header .= '; Path='.$this->_config['cookie_path']; + $header .= ($this->_config['cookie_domain'] !== '' ? '; Domain='.$this->_config['cookie_domain'] : ''); + $header .= ($this->_config['cookie_secure'] ? '; Secure' : '').'; HttpOnly; SameSite='.$this->_config['cookie_samesite']; + header($header); + } + + if ( ! $this->_config['cookie_secure'] && $this->_config['cookie_samesite'] === 'None') + { + log_message('error', 'Session:', $this->_config['cookie_name'].' cookie sent with SameSite=None, but without Secure attribute.'); + } } $this->_ci_init_vars(); @@ -284,13 +305,43 @@ class CI_Session { 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']) && is_php('7.3')) + { + $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'], array('Lax', 'Strict', 'None'), TRUE) OR $params['cookie_samesite'] = 'Lax'; + } + else + { + $params['cookie_samesite'] = 'Lax'; + } + + if (is_php('7.3')) + { + session_set_cookie_params(array( + 'lifetime' => $params['cookie_lifetime'], + 'path' => $params['cookie_path'], + 'domain' => $params['cookie_domain'], + 'secure' => $params['cookie_secure'], + 'httponly' => $params['cookie_httponly'], + 'samesite' => $params['cookie_samesite'] + )); + } + else + { + 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 + ); + } if (empty($expiration)) { diff --git a/system/libraries/Session/Session_driver.php b/system/libraries/Session/Session_driver.php index d78492b5e..b1b1b073e 100644 --- a/system/libraries/Session/Session_driver.php +++ b/system/libraries/Session/Session_driver.php @@ -140,14 +140,28 @@ abstract class CI_Session_driver { */ protected function _cookie_destroy() { + if ( ! is_php('7.3')) + { + $header = 'Set-Cookie: '.$this->_config['cookie_name'].'='; + $header .= '; Expires='.gmdate('D, d-M-Y H:i:s T', 1).'; Max-Age=-1'; + $header .= '; Path='.$this->_config['cookie_path']; + $header .= ($this->_config['cookie_domain'] !== '' ? '; Domain='.$this->_config['cookie_domain'] : ''); + $header .= ($this->_config['cookie_secure'] ? '; Secure' : '').'; HttpOnly; SameSite='.$this->_config['cookie_samesite']; + header($header); + return; + } + return setcookie( $this->_config['cookie_name'], NULL, - 1, - $this->_config['cookie_path'], - $this->_config['cookie_domain'], - $this->_config['cookie_secure'], - TRUE + array( + 'expires' => 1, + 'path' => $this->_config['cookie_path'], + 'domain' => $this->_config['cookie_domain'], + 'secure' => $this->_config['cookie_secure'], + 'httponly' => TRUE, + 'samesite' => $this->_config['cookie_samesite'] + ) ); } diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 0c61136c3..0e347f891 100644 --- a/user_guide_src/source/changelog.rst +++ b/user_guide_src/source/changelog.rst @@ -17,6 +17,7 @@ Release Date: Not Released - Updated :doc:`Database Library ` 'pdo' driver to attempt to free resources in order to allow connections to be closed. - Added ``SameSite=Strict`` attribute to the CSRF cookie sent by the :doc:`Security Class `. - Added ``$config['cookie_samesite']`` option and ``$samesite`` parameter to :doc:`Input Library ` method ``set_cookie()``. + - Added ``SameSite`` support through ``$config['sess_samesite']`` option to the :doc:`Session Library `. - Added a wrapper class around :doc:`Session ` drivers to deal with compatibility between PHP 8.1 and older versions. - Updated a lot of code for PHP 8.0 and 8.1 compatibility. diff --git a/user_guide_src/source/libraries/sessions.rst b/user_guide_src/source/libraries/sessions.rst index 994dc2e08..ced4463d0 100644 --- a/user_guide_src/source/libraries/sessions.rst +++ b/user_guide_src/source/libraries/sessions.rst @@ -438,6 +438,8 @@ Preference Default Options ============================ =============== ======================================== ============================================================================================ **sess_driver** files files/database/redis/memcached/*custom* The session storage driver to use. **sess_cookie_name** ci_session [A-Za-z\_-] characters only The name used for the session cookie. +**sess_samesite** ci_session 'Lax', 'Strict' or 'None' SameSite attribute value for session cookies. + Defaults to ``session.cookie_samesite`` on PHP 7.3+ or 'Lax' if not present at all. **sess_expiration** 7200 (2 hours) Time in seconds (integer) The number of seconds you would like the session to last. If you would like a non-expiring session (until browser is closed) set the value to zero: 0 **sess_save_path** NULL None Specifies the storage location, depends on the driver being used. -- cgit v1.2.3-24-g4f1b