From 39baff40a80162d6f51e7558ecf3c2afe78cac56 Mon Sep 17 00:00:00 2001 From: Frederik Bosch Date: Fri, 7 Jul 2017 19:06:53 +0200 Subject: [PATCH] implement same site cookie see https://bugs.php.net/bug.php?id=72230 see https://tools.ietf.org/html/draft-west-first-party-cookies-07 see https://scotthelme.co.uk/csrf-is-dead/ --- ext/session/php_session.h | 1 + ext/session/session.c | 20 ++++++-- .../session_get_cookie_params_basic.phpt | 17 +++++-- .../session_get_cookie_params_variation1.phpt | 41 ++++++++++++--- .../session_set_cookie_params_variation3.phpt | 2 +- .../session_set_cookie_params_variation5.phpt | 2 +- .../session_set_cookie_params_variation6.phpt | 50 +++++++++++++++++++ ext/standard/basic_functions.c | 2 + ext/standard/head.c | 28 +++++++---- ext/standard/head.h | 3 +- php.ini-development | 5 ++ php.ini-production | 5 ++ 12 files changed, 149 insertions(+), 27 deletions(-) create mode 100644 ext/session/tests/session_set_cookie_params_variation6.phpt diff --git a/ext/session/php_session.h b/ext/session/php_session.h index 5e1e4f07736a7..26e0130e23691 100644 --- a/ext/session/php_session.h +++ b/ext/session/php_session.h @@ -157,6 +157,7 @@ typedef struct _php_ps_globals { char *cookie_domain; zend_bool cookie_secure; zend_bool cookie_httponly; + char *cookie_samesite; ps_module *mod; ps_module *default_mod; void *mod_data; diff --git a/ext/session/session.c b/ext/session/session.c index 70f56a3adeee9..f39b7359f7e42 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -799,6 +799,7 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("session.cookie_domain", "", PHP_INI_ALL, OnUpdateSessionString, cookie_domain, php_ps_globals, ps_globals) STD_PHP_INI_ENTRY("session.cookie_secure", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_secure, php_ps_globals, ps_globals) STD_PHP_INI_ENTRY("session.cookie_httponly", "0", PHP_INI_ALL, OnUpdateSessionBool, cookie_httponly, php_ps_globals, ps_globals) + STD_PHP_INI_ENTRY("session.cookie_samesite", "", PHP_INI_ALL, OnUpdateString, cookie_samesite, php_ps_globals, ps_globals) STD_PHP_INI_ENTRY("session.use_cookies", "1", PHP_INI_ALL, OnUpdateSessionBool, use_cookies, php_ps_globals, ps_globals) STD_PHP_INI_ENTRY("session.use_only_cookies", "1", PHP_INI_ALL, OnUpdateSessionBool, use_only_cookies, php_ps_globals, ps_globals) STD_PHP_INI_ENTRY("session.use_strict_mode", "0", PHP_INI_ALL, OnUpdateSessionBool, use_strict_mode, php_ps_globals, ps_globals) @@ -1357,6 +1358,11 @@ static int php_session_send_cookie(void) /* {{{ */ smart_str_appends(&ncookie, COOKIE_HTTPONLY); } + if (PS(cookie_samesite)[0]) { + smart_str_appends(&ncookie, COOKIE_SAMESITE); + smart_str_appends(&ncookie, PS(cookie_samesite)); + } + smart_str_0(&ncookie); php_session_remove_cookie(); /* remove already sent session ID cookie */ @@ -1655,18 +1661,18 @@ PHPAPI void session_adapt_url(const char *url, size_t urllen, char **new, size_t * Userspace exported functions * ******************************** */ -/* {{{ proto bool session_set_cookie_params(int lifetime [, string path [, string domain [, bool secure[, bool httponly]]]]) +/* {{{ proto void session_set_cookie_params(int lifetime [, string path [, string domain [, bool secure[, bool httponly[, string samesite]]]]]) Set session cookie parameters */ static PHP_FUNCTION(session_set_cookie_params) { zval *lifetime; - zend_string *path = NULL, *domain = NULL; + zend_string *path = NULL, *domain = NULL, *samesite = NULL; int argc = ZEND_NUM_ARGS(); zend_bool secure = 0, httponly = 0; zend_string *ini_name; if (!PS(use_cookies) || - zend_parse_parameters(argc, "z|SSbb", &lifetime, &path, &domain, &secure, &httponly) == FAILURE) { + zend_parse_parameters(argc, "z|SSbbS", &lifetime, &path, &domain, &secure, &httponly, &samesite) == FAILURE) { return; } @@ -1724,6 +1730,12 @@ static PHP_FUNCTION(session_set_cookie_params) zend_string_release(ini_name); } + if (argc > 5) { + ini_name = zend_string_init("session.cookie_samesite", sizeof("session.cookie_samesite") - 1, 0); + zend_alter_ini_entry(ini_name, samesite, PHP_INI_USER, PHP_INI_STAGE_RUNTIME); + zend_string_release(ini_name); + } + RETURN_TRUE; } /* }}} */ @@ -1743,6 +1755,7 @@ static PHP_FUNCTION(session_get_cookie_params) add_assoc_string(return_value, "domain", PS(cookie_domain)); add_assoc_bool(return_value, "secure", PS(cookie_secure)); add_assoc_bool(return_value, "httponly", PS(cookie_httponly)); + add_assoc_string(return_value, "samesite", PS(cookie_samesite)); } /* }}} */ @@ -2621,6 +2634,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_session_set_cookie_params, 0, 0, 1) ZEND_ARG_INFO(0, domain) ZEND_ARG_INFO(0, secure) ZEND_ARG_INFO(0, httponly) + ZEND_ARG_INFO(0, samesite) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO(arginfo_session_class_open, 0) diff --git a/ext/session/tests/session_get_cookie_params_basic.phpt b/ext/session/tests/session_get_cookie_params_basic.phpt index 5d328550eaef3..f966f110eed6f 100644 --- a/ext/session/tests/session_get_cookie_params_basic.phpt +++ b/ext/session/tests/session_get_cookie_params_basic.phpt @@ -8,6 +8,7 @@ session.cookie_path="/" session.cookie_domain="" session.cookie_secure=0 session.cookie_httponly=0 +session.cookie_samesite="" --FILE-- --EXPECTF-- *** Testing session_get_cookie_params() : basic functionality *** -array(5) { +array(6) { ["lifetime"]=> int(0) ["path"]=> @@ -43,9 +44,11 @@ array(5) { bool(false) ["httponly"]=> bool(false) + ["samesite"]=> + string(0) "" } bool(true) -array(5) { +array(6) { ["lifetime"]=> int(3600) ["path"]=> @@ -56,9 +59,11 @@ array(5) { bool(false) ["httponly"]=> bool(false) + ["samesite"]=> + string(3) "foo" } bool(true) -array(5) { +array(6) { ["lifetime"]=> int(1234567890) ["path"]=> @@ -69,5 +74,7 @@ array(5) { bool(true) ["httponly"]=> bool(true) + ["samesite"]=> + string(4) "blah" } Done diff --git a/ext/session/tests/session_get_cookie_params_variation1.phpt b/ext/session/tests/session_get_cookie_params_variation1.phpt index bb921a56b1733..07c74698bcd2e 100644 --- a/ext/session/tests/session_get_cookie_params_variation1.phpt +++ b/ext/session/tests/session_get_cookie_params_variation1.phpt @@ -8,6 +8,7 @@ session.cookie_path="/" session.cookie_domain="" session.cookie_secure=0 session.cookie_httponly=0 +session.cookie_samesite="" --FILE-- --EXPECTF-- *** Testing session_get_cookie_params() : variation *** -array(5) { +array(6) { ["lifetime"]=> int(0) ["path"]=> @@ -49,8 +52,10 @@ array(5) { bool(false) ["httponly"]=> bool(false) + ["samesite"]=> + string(0) "" } -array(5) { +array(6) { ["lifetime"]=> int(3600) ["path"]=> @@ -61,8 +66,10 @@ array(5) { bool(false) ["httponly"]=> bool(false) + ["samesite"]=> + string(0) "" } -array(5) { +array(6) { ["lifetime"]=> int(3600) ["path"]=> @@ -73,8 +80,10 @@ array(5) { bool(false) ["httponly"]=> bool(false) + ["samesite"]=> + string(0) "" } -array(5) { +array(6) { ["lifetime"]=> int(3600) ["path"]=> @@ -85,8 +94,10 @@ array(5) { bool(false) ["httponly"]=> bool(false) + ["samesite"]=> + string(0) "" } -array(5) { +array(6) { ["lifetime"]=> int(3600) ["path"]=> @@ -97,8 +108,10 @@ array(5) { bool(true) ["httponly"]=> bool(false) + ["samesite"]=> + string(0) "" } -array(5) { +array(6) { ["lifetime"]=> int(3600) ["path"]=> @@ -109,6 +122,22 @@ array(5) { bool(true) ["httponly"]=> bool(true) + ["samesite"]=> + string(0) "" +} +array(6) { + ["lifetime"]=> + int(3600) + ["path"]=> + string(5) "/path" + ["domain"]=> + string(3) "foo" + ["secure"]=> + bool(true) + ["httponly"]=> + bool(true) + ["samesite"]=> + string(3) "foo" } Done diff --git a/ext/session/tests/session_set_cookie_params_variation3.phpt b/ext/session/tests/session_set_cookie_params_variation3.phpt index 17d1e6a7713d0..e4e26b91b2ace 100644 --- a/ext/session/tests/session_set_cookie_params_variation3.phpt +++ b/ext/session/tests/session_set_cookie_params_variation3.phpt @@ -10,7 +10,7 @@ session.cookie_domain=foo ob_start(); /* - * Prototype : void session_set_cookie_params(int $lifetime [, string $path [, string $domain [, bool $secure [, bool $httponly]]]]) + * Prototype : void session_set_cookie_params(int $lifetime [, string $path [, string $domain [, bool $secure [, bool $httponly[, string $samesite]]]]]) * Description : Set the session cookie parameters * Source code : ext/session/session.c */ diff --git a/ext/session/tests/session_set_cookie_params_variation5.phpt b/ext/session/tests/session_set_cookie_params_variation5.phpt index ffdd29db2d48c..f3374d0daaad1 100644 --- a/ext/session/tests/session_set_cookie_params_variation5.phpt +++ b/ext/session/tests/session_set_cookie_params_variation5.phpt @@ -10,7 +10,7 @@ session.cookie_httponly=TRUE ob_start(); /* - * Prototype : void session_set_cookie_params(int $lifetime [, string $path [, string $domain [, bool $secure [, bool $httponly]]]]) + * Prototype : void session_set_cookie_params(int $lifetime [, string $path [, string $domain [, bool $secure [, bool $httponly[, string $samesite]]]]]) * Description : Set the session cookie parameters * Source code : ext/session/session.c */ diff --git a/ext/session/tests/session_set_cookie_params_variation6.phpt b/ext/session/tests/session_set_cookie_params_variation6.phpt new file mode 100644 index 0000000000000..7fe1e009eea6d --- /dev/null +++ b/ext/session/tests/session_set_cookie_params_variation6.phpt @@ -0,0 +1,50 @@ +--TEST-- +Test session_set_cookie_params() function : variation +--INI-- +session.cookie_samesite=test +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +*** Testing session_set_cookie_params() : variation *** +string(4) "test" +bool(true) +string(7) "nothing" +bool(true) +string(7) "nothing" + +Warning: session_set_cookie_params(): Cannot change session cookie parameters when session is active in %s on line 18 +bool(false) +string(7) "nothing" +bool(true) +string(7) "nothing" +bool(true) +string(5) "other" +Done diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 383e988090af1..9caec24c59cf6 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -1425,6 +1425,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_setcookie, 0, 0, 1) ZEND_ARG_INFO(0, domain) ZEND_ARG_INFO(0, secure) ZEND_ARG_INFO(0, httponly) + ZEND_ARG_INFO(0, samesite) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_setrawcookie, 0, 0, 1) @@ -1435,6 +1436,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_setrawcookie, 0, 0, 1) ZEND_ARG_INFO(0, domain) ZEND_ARG_INFO(0, secure) ZEND_ARG_INFO(0, httponly) + ZEND_ARG_INFO(0, samesite) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_headers_sent, 0, 0, 0) diff --git a/ext/standard/head.c b/ext/standard/head.c index 81d229d4c8307..506570901825a 100644 --- a/ext/standard/head.c +++ b/ext/standard/head.c @@ -80,8 +80,7 @@ PHPAPI int php_header(void) } } - -PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int url_encode, int httponly) +PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int url_encode, int httponly, zend_string *samesite) { char *cookie; size_t len = sizeof("Set-Cookie: "); @@ -121,6 +120,9 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, if (domain) { len += ZSTR_LEN(domain); } + if (samesite) { + len += ZSTR_LEN(samesite); + } cookie = emalloc(len + 100); @@ -182,6 +184,10 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, if (httponly) { strlcat(cookie, COOKIE_HTTPONLY, len + 100); } + if (samesite && ZSTR_LEN(samesite)) { + strlcat(cookie, COOKIE_SAMESITE, len + 100); + strlcat(cookie, ZSTR_VAL(samesite), len + 100); + } ctr.line = cookie; ctr.line_len = (uint32_t)strlen(cookie); @@ -193,15 +199,15 @@ PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, /* php_set_cookie(name, value, expires, path, domain, secure) */ -/* {{{ proto bool setcookie(string name [, string value [, int expires [, string path [, string domain [, bool secure[, bool httponly]]]]]]) +/* {{{ proto bool setcookie(string name [, string value [, int expires [, string path [, string domain [, bool secure[, bool httponly[, string samesite]]]]]]]) Send a cookie */ PHP_FUNCTION(setcookie) { - zend_string *name, *value = NULL, *path = NULL, *domain = NULL; + zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL; zend_long expires = 0; zend_bool secure = 0, httponly = 0; - ZEND_PARSE_PARAMETERS_START(1, 7) + ZEND_PARSE_PARAMETERS_START(1, 8) Z_PARAM_STR(name) Z_PARAM_OPTIONAL Z_PARAM_STR(value) @@ -210,9 +216,10 @@ PHP_FUNCTION(setcookie) Z_PARAM_STR(domain) Z_PARAM_BOOL(secure) Z_PARAM_BOOL(httponly) + Z_PARAM_STR(samesite) ZEND_PARSE_PARAMETERS_END(); - if (php_setcookie(name, value, expires, path, domain, secure, 1, httponly) == SUCCESS) { + if (php_setcookie(name, value, expires, path, domain, secure, 1, httponly, samesite) == SUCCESS) { RETVAL_TRUE; } else { RETVAL_FALSE; @@ -220,15 +227,15 @@ PHP_FUNCTION(setcookie) } /* }}} */ -/* {{{ proto bool setrawcookie(string name [, string value [, int expires [, string path [, string domain [, bool secure[, bool httponly]]]]]]) +/* {{{ proto bool setrawcookie(string name [, string value [, int expires [, string path [, string domain [, bool secure[, bool httponly[, string samesite]]]]]]]) Send a cookie with no url encoding of the value */ PHP_FUNCTION(setrawcookie) { - zend_string *name, *value = NULL, *path = NULL, *domain = NULL; + zend_string *name, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL; zend_long expires = 0; zend_bool secure = 0, httponly = 0; - ZEND_PARSE_PARAMETERS_START(1, 7) + ZEND_PARSE_PARAMETERS_START(1, 8) Z_PARAM_STR(name) Z_PARAM_OPTIONAL Z_PARAM_STR(value) @@ -237,9 +244,10 @@ PHP_FUNCTION(setrawcookie) Z_PARAM_STR(domain) Z_PARAM_BOOL(secure) Z_PARAM_BOOL(httponly) + Z_PARAM_STR(samesite) ZEND_PARSE_PARAMETERS_END(); - if (php_setcookie(name, value, expires, path, domain, secure, 0, httponly) == SUCCESS) { + if (php_setcookie(name, value, expires, path, domain, secure, 0, httponly, samesite) == SUCCESS) { RETVAL_TRUE; } else { RETVAL_FALSE; diff --git a/ext/standard/head.h b/ext/standard/head.h index 7a03e18d4f6ba..aef8e093ce056 100644 --- a/ext/standard/head.h +++ b/ext/standard/head.h @@ -27,6 +27,7 @@ #define COOKIE_PATH "; path=" #define COOKIE_SECURE "; secure" #define COOKIE_HTTPONLY "; HttpOnly" +#define COOKIE_SAMESITE "; SameSite=" extern PHP_RINIT_FUNCTION(head); PHP_FUNCTION(header); @@ -38,6 +39,6 @@ PHP_FUNCTION(headers_list); PHP_FUNCTION(http_response_code); PHPAPI int php_header(void); -PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int url_encode, int httponly); +PHPAPI int php_setcookie(zend_string *name, zend_string *value, time_t expires, zend_string *path, zend_string *domain, int secure, int url_encode, int httponly, zend_string *samesite); #endif diff --git a/php.ini-development b/php.ini-development index 8ba304b8c0228..412971ca99bdb 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1389,6 +1389,11 @@ session.cookie_domain = ; http://php.net/session.cookie-httponly session.cookie_httponly = +; Add SameSite attribute to cookie to help mitigate Cross-Site Request Forgery (CSRF/XSRF) +; Current valid values are "Lax" or "Strict" +; https://tools.ietf.org/html/draft-west-first-party-cookies-07 +session.cookie_samesite = + ; Handler used to serialize data. php is the standard serializer of PHP. ; http://php.net/session.serialize-handler session.serialize_handler = php diff --git a/php.ini-production b/php.ini-production index fb08287fa6e11..aca53c34cc896 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1396,6 +1396,11 @@ session.cookie_domain = ; http://php.net/session.cookie-httponly session.cookie_httponly = +; Add SameSite attribute to cookie to help mitigate Cross-Site Request Forgery (CSRF/XSRF) +; Current valid values are "Lax" or "Strict" +; https://tools.ietf.org/html/draft-west-first-party-cookies-07 +session.cookie_samesite = + ; Handler used to serialize data. php is the standard serializer of PHP. ; http://php.net/session.serialize-handler session.serialize_handler = php