Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CodeIgniter cookie optimization for samesite and httponly (PHP7.2+) #1508

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions application/config/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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';

/*
|--------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion system/core/CodeIgniter.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
* @var string
*
*/
const CI_VERSION = '3.2.0-dev';
const CI_VERSION = '3.2.1-dev';

/*
* ------------------------------------------------------
Expand Down
33 changes: 30 additions & 3 deletions system/core/Input.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
{
Expand Down Expand Up @@ -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);

}

// --------------------------------------------------------------------
Expand Down
13 changes: 8 additions & 5 deletions system/core/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
4 changes: 2 additions & 2 deletions system/helpers/cookie_helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
44 changes: 32 additions & 12 deletions system/libraries/Session/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']
]
);
}

Expand Down Expand Up @@ -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))
{
Expand Down
13 changes: 8 additions & 5 deletions system/libraries/Session/Session_driver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']
]
);
}

Expand Down