summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorxXx-caillou-xXx2017-11-24 14:03:37 +0100
committerjvoisin2017-11-24 14:03:37 +0100
commit5a224ee0c92d1639395d6a0c629316ae64226125 (patch)
tree8925d27e2bbfa877e9fb1fc20868fbef3d009b04
parent79304a29661476dc75bba07c5a83133122bbcb5c (diff)
Implement anti csrf measures
This is done by using the "samesite" cookie attribute.
-rw-r--r--doc/source/config.rst24
-rw-r--r--doc/source/features.rst14
-rw-r--r--src/snuffleupagus.c15
-rw-r--r--src/sp_config.c2
-rw-r--r--src/sp_config.h23
-rw-r--r--src/sp_config_keywords.c53
-rw-r--r--src/sp_config_keywords.h4
-rw-r--r--src/sp_cookie_encryption.c39
-rw-r--r--src/sp_cookie_encryption.h2
-rw-r--r--src/tests/broken_conf_no_cookie_action.phpt9
-rw-r--r--src/tests/broken_conf_no_cookie_name.phpt2
-rw-r--r--src/tests/broken_conf_samesite.phpt9
-rw-r--r--src/tests/broken_conf_weird_keyword.phpt2
-rw-r--r--src/tests/config/broken_conf_cookie_action.ini1
-rw-r--r--src/tests/config/broken_conf_cookie_samesite.ini1
-rw-r--r--src/tests/config/broken_conf_line_empty_string.ini2
-rw-r--r--src/tests/config/broken_conf_line_no_closing.ini2
-rw-r--r--src/tests/config/broken_conf_lots_of_quotes.ini2
-rw-r--r--src/tests/config/broken_conf_wrong_quotes.ini2
-rw-r--r--src/tests/config/config_encrypted_cookies.ini2
-rw-r--r--src/tests/config/config_encrypted_cookies_empty_env.ini2
-rw-r--r--src/tests/config/config_encrypted_cookies_noname.ini2
-rw-r--r--src/tests/config/config_samesite_cookies.ini5
-rw-r--r--src/tests/config/encrypt_cookies_no_env.ini2
-rw-r--r--src/tests/config/encrypt_cookies_no_key.ini2
-rw-r--r--src/tests/samesite_cookies.phpt61
26 files changed, 229 insertions, 55 deletions
diff --git a/doc/source/config.rst b/doc/source/config.rst
index fc0df2d..c271403 100644
--- a/doc/source/config.rst
+++ b/doc/source/config.rst
@@ -115,6 +115,26 @@ It can either be ``enabled`` or ``disabled``.
115 sp.auto_cookie_secure.enable(); 115 sp.auto_cookie_secure.enable();
116 sp.auto_cookie_secure.disable(); 116 sp.auto_cookie_secure.disable();
117 117
118cookie_samesite
119^^^^^^^^^^^^^^^^
120 * `default: disabled`
121
122``samesite`` will add the `samesite <https://tools.ietf.org/html/draft-west-first-party-cookies-07>`_
123attribute to cookies. It `prevents CSRF <https://www.owasp.org/index.php/SameSite>`_
124but is not implemented by `all web browsers <https://caniuse.com/#search=samesite>`_ yet.
125
126It can either be set to ``strict`` or ``lax``:
127
128- The ``lax`` attribute prevents cookies from being sent cross-domain for
129 "dangerous" methods, like ``POST``, ``PUT`` or ``DELETE``.
130
131- The ``strict`` one prevents any cookies from beind sent cross-domain.
132
133::
134
135 sp.cookie.name("cookie1").samesite("lax");
136 sp.cookie.name("cookie2").samesite("strict");;
137
118.. _cookie-encryption_config: 138.. _cookie-encryption_config:
119 139
120cookie_encryption 140cookie_encryption
@@ -137,8 +157,8 @@ It can either be ``enabled`` or ``disabled`` and can be used in ``simulation`` m
137 157
138:: 158::
139 159
140 sp.cookie_encryption.cookie("my_cookie_name"); 160 sp.cookie.name("my_cookie_name").encrypt();
141 sp.cookie_encryption.cookie("another_cookie_name"); 161 sp.cookie.name("another_cookie_name").encrypt();
142 162
143Choosing the proper environment variable 163Choosing the proper environment variable
144"""""""""""""""""""""""""""""""""""""""" 164""""""""""""""""""""""""""""""""""""""""
diff --git a/doc/source/features.rst b/doc/source/features.rst
index 3643326..c4239e9 100644
--- a/doc/source/features.rst
+++ b/doc/source/features.rst
@@ -321,6 +321,20 @@ would be to use a different user to run PHP than for administrating the website,
321and using this feature to lock this up. 321and using this feature to lock this up.
322 322
323 323
324Protection against cross site request forgery
325^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
326
327Cross-site request forgery, sometimes abbreviated as *CSRF*,
328is when unauthorised commands are issued from a user that the application trusts.
329For example, if a user is authenticated on a banking website,
330an other site might present something like
331``<img src="http://mybank.com/transfer?from=user&to=attack&amount=1337EUR">``,
332effectivement transfering money from the user's account to the attacker one.
333
334Snuffleupagus can prevent this (in `supported browsers <https://caniuse.com/#search=samesite>`__)
335by setting the `samesite <https://tools.ietf.org/html/draft-west-first-party-cookies-07>`__
336attribute on cookies.
337
324 338
325Dumping capabilities 339Dumping capabilities
326^^^^^^^^^^^^^^^^^^^^ 340^^^^^^^^^^^^^^^^^^^^
diff --git a/src/snuffleupagus.c b/src/snuffleupagus.c
index e453587..9467a5d 100644
--- a/src/snuffleupagus.c
+++ b/src/snuffleupagus.c
@@ -71,14 +71,13 @@ PHP_GINIT_FUNCTION(snuffleupagus) {
71 SP_INIT(snuffleupagus_globals->config.config_upload_validation); 71 SP_INIT(snuffleupagus_globals->config.config_upload_validation);
72 SP_INIT(snuffleupagus_globals->config.config_disabled_functions); 72 SP_INIT(snuffleupagus_globals->config.config_disabled_functions);
73 SP_INIT(snuffleupagus_globals->config.config_disabled_functions_ret); 73 SP_INIT(snuffleupagus_globals->config.config_disabled_functions_ret);
74 SP_INIT(snuffleupagus_globals->config.config_cookie_encryption); 74 SP_INIT(snuffleupagus_globals->config.config_cookie);
75 SP_INIT(snuffleupagus_globals->config.config_disabled_constructs); 75 SP_INIT(snuffleupagus_globals->config.config_disabled_constructs);
76 76
77 snuffleupagus_globals->config.config_disabled_constructs->construct_include = sp_list_new(); 77 snuffleupagus_globals->config.config_disabled_constructs->construct_include = sp_list_new();
78 snuffleupagus_globals->config.config_disabled_functions->disabled_functions = sp_list_new(); 78 snuffleupagus_globals->config.config_disabled_functions->disabled_functions = sp_list_new();
79 snuffleupagus_globals->config.config_disabled_functions_ret->disabled_functions = sp_list_new(); 79 snuffleupagus_globals->config.config_disabled_functions_ret->disabled_functions = sp_list_new();
80 80 SP_INIT_HT(snuffleupagus_globals->config.config_cookie->cookies);
81 SP_INIT_HT(snuffleupagus_globals->config.config_cookie_encryption->names);
82 81
83#undef SP_INIT 82#undef SP_INIT
84#undef SP_INIT_HT 83#undef SP_INIT_HT
@@ -96,7 +95,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
96 pefree(SNUFFLEUPAGUS_G(F), 1); 95 pefree(SNUFFLEUPAGUS_G(F), 1);
97 96
98 FREE_HT(disabled_functions_hook); 97 FREE_HT(disabled_functions_hook);
99 FREE_HT(config.config_cookie_encryption->names); 98 FREE_HT(config.config_cookie->cookies);
100 99
101#undef FREE_HT 100#undef FREE_HT
102 101
@@ -108,7 +107,6 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
108 pefree(SNUFFLEUPAGUS_G(config.config_snuffleupagus), 1); 107 pefree(SNUFFLEUPAGUS_G(config.config_snuffleupagus), 1);
109 pefree(SNUFFLEUPAGUS_G(config.config_disable_xxe), 1); 108 pefree(SNUFFLEUPAGUS_G(config.config_disable_xxe), 1);
110 pefree(SNUFFLEUPAGUS_G(config.config_upload_validation), 1); 109 pefree(SNUFFLEUPAGUS_G(config.config_upload_validation), 1);
111 pefree(SNUFFLEUPAGUS_G(config.config_cookie_encryption), 1);
112 110
113#define FREE_LST(L) \ 111#define FREE_LST(L) \
114 do { \ 112 do { \
@@ -126,6 +124,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
126 pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions), 1); 124 pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions), 1);
127 pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions_ret), 1); 125 pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions_ret), 1);
128 pefree(SNUFFLEUPAGUS_G(config.config_disabled_constructs), 1); 126 pefree(SNUFFLEUPAGUS_G(config.config_disabled_constructs), 1);
127 pefree(SNUFFLEUPAGUS_G(config.config_cookie), 1);
129 128
130 UNREGISTER_INI_ENTRIES(); 129 UNREGISTER_INI_ENTRIES();
131 130
@@ -137,9 +136,9 @@ PHP_RINIT_FUNCTION(snuffleupagus) {
137 ZEND_TSRMLS_CACHE_UPDATE(); 136 ZEND_TSRMLS_CACHE_UPDATE();
138#endif 137#endif
139 if (NULL != SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key) { 138 if (NULL != SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key) {
140 if (NULL != SNUFFLEUPAGUS_G(config).config_cookie_encryption->names) { 139 if (NULL != SNUFFLEUPAGUS_G(config).config_cookie->cookies) {
141 zend_hash_apply_with_arguments( 140 zend_hash_apply_with_arguments(
142 Z_ARRVAL(PG(http_globals)[TRACK_VARS_COOKIE]), decrypt_cookie, 0); 141 Z_ARRVAL(PG(http_globals)[TRACK_VARS_COOKIE]), decrypt_cookie, 0);
143 } 142 }
144 } 143 }
145 return SUCCESS; 144 return SUCCESS;
@@ -190,8 +189,8 @@ static PHP_INI_MH(OnUpdateConfiguration) {
190 if (SNUFFLEUPAGUS_G(config).config_unserialize->enable) { 189 if (SNUFFLEUPAGUS_G(config).config_unserialize->enable) {
191 hook_serialize(); 190 hook_serialize();
192 } 191 }
193 hook_cookies();
194 } 192 }
193 hook_cookies();
195 194
196 if (true == SNUFFLEUPAGUS_G(config).config_global_strict->enable) { 195 if (true == SNUFFLEUPAGUS_G(config).config_global_strict->enable) {
197 if (!zend_get_extension(PHP_SNUFFLEUPAGUS_EXTNAME)) { 196 if (!zend_get_extension(PHP_SNUFFLEUPAGUS_EXTNAME)) {
diff --git a/src/sp_config.c b/src/sp_config.c
index 13002cc..2432cc4 100644
--- a/src/sp_config.c
+++ b/src/sp_config.c
@@ -15,7 +15,7 @@ sp_config_tokens const sp_func[] = {
15 {.func = parse_readonly_exec, .token = SP_TOKEN_READONLY_EXEC}, 15 {.func = parse_readonly_exec, .token = SP_TOKEN_READONLY_EXEC},
16 {.func = parse_global_strict, .token = SP_TOKEN_GLOBAL_STRICT}, 16 {.func = parse_global_strict, .token = SP_TOKEN_GLOBAL_STRICT},
17 {.func = parse_upload_validation, .token = SP_TOKEN_UPLOAD_VALIDATION}, 17 {.func = parse_upload_validation, .token = SP_TOKEN_UPLOAD_VALIDATION},
18 {.func = parse_cookie_encryption, .token = SP_TOKEN_COOKIE_ENCRYPTION}, 18 {.func = parse_cookie, .token = SP_TOKEN_COOKIE_ENCRYPTION},
19 {.func = parse_global, .token = SP_TOKEN_GLOBAL}, 19 {.func = parse_global, .token = SP_TOKEN_GLOBAL},
20 {.func = parse_auto_cookie_secure, .token = SP_TOKEN_AUTO_COOKIE_SECURE}, 20 {.func = parse_auto_cookie_secure, .token = SP_TOKEN_AUTO_COOKIE_SECURE},
21 {.func = parse_disable_xxe, .token = SP_TOKEN_DISABLE_XXE}, 21 {.func = parse_disable_xxe, .token = SP_TOKEN_DISABLE_XXE},
diff --git a/src/sp_config.h b/src/sp_config.h
index e14e30b..12f12e8 100644
--- a/src/sp_config.h
+++ b/src/sp_config.h
@@ -55,7 +55,12 @@ typedef struct { bool enable; } sp_config_auto_cookie_secure;
55 55
56typedef struct { bool enable; } sp_config_disable_xxe; 56typedef struct { bool enable; } sp_config_disable_xxe;
57 57
58typedef struct { HashTable *names; } sp_config_cookie_encryption; 58enum samesite_type {strict=1, lax=2};
59
60typedef struct {
61 enum samesite_type samesite;
62 bool encrypt;
63} sp_cookie;
59 64
60typedef struct { 65typedef struct {
61 bool enable; 66 bool enable;
@@ -105,6 +110,10 @@ typedef struct {
105} sp_config_disabled_functions; 110} sp_config_disabled_functions;
106 111
107typedef struct { 112typedef struct {
113 HashTable *cookies; // HashTable of sp_cookie
114} sp_config_cookie;
115
116typedef struct {
108 sp_node_t *construct_include; // list of rules for `(include|require)_(once)?` 117 sp_node_t *construct_include; // list of rules for `(include|require)_(once)?`
109 sp_node_t *construct_echo; 118 sp_node_t *construct_echo;
110} sp_config_disabled_constructs; 119} sp_config_disabled_constructs;
@@ -122,7 +131,7 @@ typedef struct {
122 sp_config_disabled_functions *config_disabled_functions_ret; 131 sp_config_disabled_functions *config_disabled_functions_ret;
123 sp_config_readonly_exec *config_readonly_exec; 132 sp_config_readonly_exec *config_readonly_exec;
124 sp_config_upload_validation *config_upload_validation; 133 sp_config_upload_validation *config_upload_validation;
125 sp_config_cookie_encryption *config_cookie_encryption; 134 sp_config_cookie *config_cookie;
126 sp_config_global *config_snuffleupagus; 135 sp_config_global *config_snuffleupagus;
127 sp_config_auto_cookie_secure *config_auto_cookie_secure; 136 sp_config_auto_cookie_secure *config_auto_cookie_secure;
128 sp_config_global_strict *config_global_strict; 137 sp_config_global_strict *config_global_strict;
@@ -144,7 +153,7 @@ typedef struct {
144#define SP_TOKEN_BASE "sp" 153#define SP_TOKEN_BASE "sp"
145 154
146#define SP_TOKEN_AUTO_COOKIE_SECURE ".auto_cookie_secure" 155#define SP_TOKEN_AUTO_COOKIE_SECURE ".auto_cookie_secure"
147#define SP_TOKEN_COOKIE_ENCRYPTION ".cookie_encryption" 156#define SP_TOKEN_COOKIE_ENCRYPTION ".cookie"
148#define SP_TOKEN_DISABLE_FUNC ".disable_function" 157#define SP_TOKEN_DISABLE_FUNC ".disable_function"
149#define SP_TOKEN_GLOBAL ".global" 158#define SP_TOKEN_GLOBAL ".global"
150#define SP_TOKEN_GLOBAL_STRICT ".global_strict" 159#define SP_TOKEN_GLOBAL_STRICT ".global_strict"
@@ -187,7 +196,13 @@ typedef struct {
187#define SP_TOKEN_LINE_NUMBER ".line(" 196#define SP_TOKEN_LINE_NUMBER ".line("
188 197
189// cookies encryption 198// cookies encryption
190#define SP_TOKEN_NAME ".cookie(" 199#define SP_TOKEN_NAME ".name("
200
201// cookies samesite
202#define SP_TOKEN_SAMESITE ".samesite("
203#define SP_TOKEN_ENCRYPT ".encrypt("
204#define SP_TOKEN_SAMESITE_LAX "Lax"
205#define SP_TOKEN_SAMESITE_STRICT "Strict"
191 206
192// Global configuration options 207// Global configuration options
193#define SP_TOKEN_ENCRYPTION_KEY ".secret_key(" 208#define SP_TOKEN_ENCRYPTION_KEY ".secret_key("
diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c
index 34b855a..077d78f 100644
--- a/src/sp_config_keywords.c
+++ b/src/sp_config_keywords.c
@@ -105,12 +105,16 @@ int parse_global(char *line) {
105 return parse_keywords(sp_config_funcs_global, line); 105 return parse_keywords(sp_config_funcs_global, line);
106} 106}
107 107
108int parse_cookie_encryption(char *line) { 108int parse_cookie(char *line) {
109 int ret = 0; 109 int ret = 0;
110 char *name = NULL; 110 char *samesite = NULL, *name = NULL;
111 sp_cookie *cookie = pecalloc(sizeof(sp_cookie), 1, 1);
112 zend_string *zend_name;
111 113
112 sp_config_functions sp_config_funcs_cookie_encryption[] = { 114 sp_config_functions sp_config_funcs_cookie_encryption[] = {
113 {parse_str, SP_TOKEN_NAME, &name}, 115 {parse_str, SP_TOKEN_NAME, &name},
116 {parse_str, SP_TOKEN_SAMESITE, &samesite},
117 {parse_empty, SP_TOKEN_ENCRYPT, &cookie->encrypt},
114 {0}}; 118 {0}};
115 119
116 ret = parse_keywords(sp_config_funcs_cookie_encryption, line); 120 ret = parse_keywords(sp_config_funcs_cookie_encryption, line);
@@ -118,25 +122,42 @@ int parse_cookie_encryption(char *line) {
118 return ret; 122 return ret;
119 } 123 }
120 124
121 if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)) { 125 if (cookie->encrypt) {
122 sp_log_err("config", "You're trying to use the cookie encryption feature" 126 if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)) {
123 "on line %zu without having set the `.cookie_env_var` option in" 127 sp_log_err("config", "You're trying to use the cookie encryption feature"
124 "`sp.global`: please set it first.", sp_line_no); 128 "on line %zu without having set the `.cookie_env_var` option in"
125 return -1; 129 "`sp.global`: please set it first.", sp_line_no);
126 } else if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key)) { 130 return -1;
127 sp_log_err("config", "You're trying to use the cookie encryption feature" 131 } else if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key)) {
128 "on line %zu without having set the `.encryption_key` option in" 132 sp_log_err("config", "You're trying to use the cookie encryption feature"
129 "`sp.global`: please set it first.", sp_line_no); 133 "on line %zu without having set the `.encryption_key` option in"
134 "`sp.global`: please set it first.", sp_line_no);
135 return -1;
136 }
137 } else if (!samesite) {
138 sp_log_err("config", "You must specify a at least one action to a cookie on line "
139 "%zu.", sp_line_no);
130 return -1; 140 return -1;
131 } else if (0 == strlen(name)) { 141 }
132 sp_log_err("config", "You must specify a cookie name to encrypt on line " 142 if (0 == strlen(name)) {
143 sp_log_err("config", "You must specify a cookie name on line "
133 "%zu.", sp_line_no); 144 "%zu.", sp_line_no);
134 return -1; 145 return -1;
135 } 146 }
147 if (samesite) {
148 if (0 == strcasecmp(samesite, SP_TOKEN_SAMESITE_LAX)) {
149 cookie->samesite = lax;
150 } else if (0 == strcasecmp(samesite, SP_TOKEN_SAMESITE_STRICT)) {
151 cookie->samesite = strict;
152 } else {
153 sp_log_err("config", "%s is an invalid value to samesite (expected %s or %s) on line "
154 "%zu.", samesite, SP_TOKEN_SAMESITE_LAX, SP_TOKEN_SAMESITE_STRICT, sp_line_no);
155 return -1;
156 }
157 }
136 158
137 zend_hash_str_add_empty_element( 159 zend_name = zend_string_init(name, strlen(name), 1);
138 SNUFFLEUPAGUS_G(config).config_cookie_encryption->names, name, 160 zend_hash_add_ptr(SNUFFLEUPAGUS_G(config).config_cookie->cookies, zend_name, cookie);
139 strlen(name));
140 161
141 return SUCCESS; 162 return SUCCESS;
142} 163}
diff --git a/src/sp_config_keywords.h b/src/sp_config_keywords.h
index 40fac47..fdea1c5 100644
--- a/src/sp_config_keywords.h
+++ b/src/sp_config_keywords.h
@@ -7,10 +7,10 @@ int parse_disable_xxe(char *line);
7int parse_auto_cookie_secure(char *line); 7int parse_auto_cookie_secure(char *line);
8int parse_global_strict(char *line); 8int parse_global_strict(char *line);
9int parse_global(char *line) ; 9int parse_global(char *line) ;
10int parse_cookie_encryption(char *line); 10int parse_cookie(char *line);
11int parse_unserialize(char *line) ; 11int parse_unserialize(char *line) ;
12int parse_readonly_exec(char *line); 12int parse_readonly_exec(char *line);
13int parse_disabled_functions(char *line) ; 13int parse_disabled_functions(char *line) ;
14int parse_upload_validation(char *line); 14int parse_upload_validation(char *line);
15 15
16#endif // __SP_CONFIG_KEYWORDS_H \ No newline at end of file 16#endif // __SP_CONFIG_KEYWORDS_H
diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c
index 2ebcc96..eb20c52 100644
--- a/src/sp_cookie_encryption.c
+++ b/src/sp_cookie_encryption.c
@@ -45,12 +45,12 @@ int decrypt_cookie(zval *pDest, int num_args, va_list args,
45 size_t value_len; 45 size_t value_len;
46 zend_string *debase64; 46 zend_string *debase64;
47 unsigned char *decrypted; 47 unsigned char *decrypted;
48 sp_cookie *cookie = zend_hash_find_ptr(SNUFFLEUPAGUS_G(config).config_cookie->cookies,
49 hash_key->key);
48 int ret = 0; 50 int ret = 0;
49 51
50 /* If the cookie isn't in the conf, it shouldn't be encrypted. */ 52 /* If the cookie isn't in the conf, it shouldn't be encrypted. */
51 if (0 == 53 if (!cookie || !cookie->encrypt) {
52 zend_hash_exists(SNUFFLEUPAGUS_G(config).config_cookie_encryption->names,
53 hash_key->key)) {
54 return ZEND_HASH_APPLY_KEEP; 54 return ZEND_HASH_APPLY_KEEP;
55 } 55 }
56 56
@@ -135,11 +135,13 @@ static zend_string *encrypt_data(char *data, unsigned long long data_len) {
135 135
136PHP_FUNCTION(sp_setcookie) { 136PHP_FUNCTION(sp_setcookie) {
137 zval params[7] = { 0 }; 137 zval params[7] = { 0 };
138 zend_string *name = NULL, *value = NULL, *path = NULL, *domain = NULL; 138 zend_string *name = NULL, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
139 zend_long expires = 0; 139 zend_long expires = 0;
140 zend_bool secure = 0, httponly = 0; 140 zend_bool secure = 0, httponly = 0;
141 zval ret_val; 141 zval ret_val;
142 const sp_cookie *cookie_node = NULL;
142 zval func_name; 143 zval func_name;
144 char *cookie_samesite;
143 145
144 146
145 // LCOV_EXCL_BR_START 147 // LCOV_EXCL_BR_START
@@ -167,17 +169,18 @@ PHP_FUNCTION(sp_setcookie) {
167 } 169 }
168 } 170 }
169 171
172 cookie_node =
173 zend_hash_find_ptr(SNUFFLEUPAGUS_G(config).config_cookie->cookies, name);
174
170 /* If the cookie's value is encrypted, it won't be usable by 175 /* If the cookie's value is encrypted, it won't be usable by
171 * javascript anyway. 176 * javascript anyway.
172 */ 177 */
173 if (zend_hash_exists(SNUFFLEUPAGUS_G(config).config_cookie_encryption->names, 178 if (cookie_node && cookie_node->encrypt) {
174 name) > 0) {
175 httponly = 1; 179 httponly = 1;
176 } 180 }
177 181
178 /* Shall we encrypt the cookie's value? */ 182 /* Shall we encrypt the cookie's value? */
179 if (zend_hash_exists(SNUFFLEUPAGUS_G(config).config_cookie_encryption->names, 183 if (httponly && value) {
180 name) > 0 && value) {
181 zend_string *encrypted_data = encrypt_data(value->val, value->len); 184 zend_string *encrypted_data = encrypt_data(value->val, value->len);
182 ZVAL_STR_COPY(&params[1], encrypted_data); 185 ZVAL_STR_COPY(&params[1], encrypted_data);
183 zend_string_release(encrypted_data); 186 zend_string_release(encrypted_data);
@@ -188,9 +191,6 @@ PHP_FUNCTION(sp_setcookie) {
188 ZVAL_STRING(&func_name, "setcookie"); 191 ZVAL_STRING(&func_name, "setcookie");
189 ZVAL_STR_COPY(&params[0], name); 192 ZVAL_STR_COPY(&params[0], name);
190 ZVAL_LONG(&params[2], expires); 193 ZVAL_LONG(&params[2], expires);
191 if (path) {
192 ZVAL_STR_COPY(&params[3], path);
193 }
194 if (domain) { 194 if (domain) {
195 ZVAL_STR_COPY(&params[4], domain); 195 ZVAL_STR_COPY(&params[4], domain);
196 } 196 }
@@ -201,6 +201,23 @@ PHP_FUNCTION(sp_setcookie) {
201 ZVAL_LONG(&params[6], httponly); 201 ZVAL_LONG(&params[6], httponly);
202 } 202 }
203 203
204 /* param[3](path) is concatenated to path= and is not filtered, we can inject
205 the samesite parameter here */
206 if (cookie_node && cookie_node->samesite) {
207 if (!path) {
208 path = zend_string_init("", 0, 0);
209 }
210 cookie_samesite = (cookie_node->samesite == lax) ? SAMESITE_COOKIE_FORMAT SP_TOKEN_SAMESITE_LAX
211 : SAMESITE_COOKIE_FORMAT SP_TOKEN_SAMESITE_STRICT;
212 /* Concatenating everything, as is in PHP internals */
213 samesite = zend_string_extend(path, ZSTR_LEN(path) + strlen(cookie_samesite) + 1, 0);
214 memcpy(ZSTR_VAL(samesite) + ZSTR_LEN(path), cookie_samesite, strlen(cookie_samesite) + 1);
215 ZVAL_STR_COPY(&params[3], samesite);
216 zend_string_release(path);
217 } else if (path) {
218 ZVAL_STR_COPY(&params[3], path);
219 }
220
204 /* This is the _fun_ part: because PHP is utterly idiotic and nonsensical, 221 /* This is the _fun_ part: because PHP is utterly idiotic and nonsensical,
205 the `call_user_function` macro will __discard__ (yes) its first argument 222 the `call_user_function` macro will __discard__ (yes) its first argument
206 (the hashtable), effectively calling functions from `CG(function_table)`. 223 (the hashtable), effectively calling functions from `CG(function_table)`.
diff --git a/src/sp_cookie_encryption.h b/src/sp_cookie_encryption.h
index 9904738..889a89c 100644
--- a/src/sp_cookie_encryption.h
+++ b/src/sp_cookie_encryption.h
@@ -11,6 +11,8 @@
11#include "ext/hash/php_hash_sha.h" 11#include "ext/hash/php_hash_sha.h"
12#include "ext/standard/base64.h" 12#include "ext/standard/base64.h"
13 13
14#define SAMESITE_COOKIE_FORMAT "; samesite="
15
14int hook_cookies(); 16int hook_cookies();
15int decrypt_cookie(zval *pDest, int num_args, va_list args, zend_hash_key *hash_key); 17int decrypt_cookie(zval *pDest, int num_args, va_list args, zend_hash_key *hash_key);
16 18
diff --git a/src/tests/broken_conf_no_cookie_action.phpt b/src/tests/broken_conf_no_cookie_action.phpt
new file mode 100644
index 0000000..49be31e
--- /dev/null
+++ b/src/tests/broken_conf_no_cookie_action.phpt
@@ -0,0 +1,9 @@
1--TEST--
2Bad config, invalid action.
3--SKIPIF--
4<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
5--INI--
6sp.configuration_file={PWD}/config/broken_conf_cookie_action.ini
7--FILE--
8--EXPECT--
9[snuffleupagus][0.0.0.0][config][error] You must specify a at least one action to a cookie on line 1.
diff --git a/src/tests/broken_conf_no_cookie_name.phpt b/src/tests/broken_conf_no_cookie_name.phpt
index feaf6ca..4616f12 100644
--- a/src/tests/broken_conf_no_cookie_name.phpt
+++ b/src/tests/broken_conf_no_cookie_name.phpt
@@ -6,4 +6,4 @@ Borken configuration - encrypted cookie with no name
6sp.configuration_file={PWD}/config/config_encrypted_cookies_noname.ini 6sp.configuration_file={PWD}/config/config_encrypted_cookies_noname.ini
7--FILE-- 7--FILE--
8--EXPECT-- 8--EXPECT--
9[snuffleupagus][0.0.0.0][config][error] You must specify a cookie name to encrypt on line 2. 9[snuffleupagus][0.0.0.0][config][error] You must specify a cookie name on line 2.
diff --git a/src/tests/broken_conf_samesite.phpt b/src/tests/broken_conf_samesite.phpt
new file mode 100644
index 0000000..26e525c
--- /dev/null
+++ b/src/tests/broken_conf_samesite.phpt
@@ -0,0 +1,9 @@
1--TEST--
2Bad config, invalid samesite type.
3--SKIPIF--
4<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
5--INI--
6sp.configuration_file={PWD}/config/broken_conf_cookie_samesite.ini
7--FILE--
8--EXPECT--
9[snuffleupagus][0.0.0.0][config][error] nop is an invalid value to samesite (expected Lax or Strict) on line 1.
diff --git a/src/tests/broken_conf_weird_keyword.phpt b/src/tests/broken_conf_weird_keyword.phpt
index 17de7fe..464800a 100644
--- a/src/tests/broken_conf_weird_keyword.phpt
+++ b/src/tests/broken_conf_weird_keyword.phpt
@@ -6,4 +6,4 @@ Bad config, unknown keyword
6sp.configuration_file={PWD}/config/broken_conf_weird_keyword.ini 6sp.configuration_file={PWD}/config/broken_conf_weird_keyword.ini
7--FILE-- 7--FILE--
8--EXPECT-- 8--EXPECT--
9[snuffleupagus][0.0.0.0][config][error] Trailing chars '.not_a_valid_keyword("test");' at the end of '.enable().not_a_valid_keyword("test");' on line 1. \ No newline at end of file 9[snuffleupagus][0.0.0.0][config][error] Trailing chars '.not_a_valid_keyword("test");' at the end of '.enable().not_a_valid_keyword("test");' on line 1.
diff --git a/src/tests/config/broken_conf_cookie_action.ini b/src/tests/config/broken_conf_cookie_action.ini
new file mode 100644
index 0000000..5f07c28
--- /dev/null
+++ b/src/tests/config/broken_conf_cookie_action.ini
@@ -0,0 +1 @@
sp.cookie.name("my_cookie_name");
diff --git a/src/tests/config/broken_conf_cookie_samesite.ini b/src/tests/config/broken_conf_cookie_samesite.ini
new file mode 100644
index 0000000..acc4aa0
--- /dev/null
+++ b/src/tests/config/broken_conf_cookie_samesite.ini
@@ -0,0 +1 @@
sp.cookie.name("my_cookie_name").samesite("nop");
diff --git a/src/tests/config/broken_conf_line_empty_string.ini b/src/tests/config/broken_conf_line_empty_string.ini
index c130384..dfa5520 100644
--- a/src/tests/config/broken_conf_line_empty_string.ini
+++ b/src/tests/config/broken_conf_line_empty_string.ini
@@ -1 +1 @@
sp.cookie_encryption.cookie( sp.cookie.name(
diff --git a/src/tests/config/broken_conf_line_no_closing.ini b/src/tests/config/broken_conf_line_no_closing.ini
index 24dc3f0..6a8c922 100644
--- a/src/tests/config/broken_conf_line_no_closing.ini
+++ b/src/tests/config/broken_conf_line_no_closing.ini
@@ -1 +1 @@
sp.cookie_encryption.cookie("123" sp.cookie.name("123"
diff --git a/src/tests/config/broken_conf_lots_of_quotes.ini b/src/tests/config/broken_conf_lots_of_quotes.ini
index 310bce5..189a10d 100644
--- a/src/tests/config/broken_conf_lots_of_quotes.ini
+++ b/src/tests/config/broken_conf_lots_of_quotes.ini
@@ -1 +1 @@
sp.cookie_encryption.cookie("this\"is a weird\"\"\"cookie\"name""); sp.cookie.name("this\"is a weird\"\"\"cookie\"name"");
diff --git a/src/tests/config/broken_conf_wrong_quotes.ini b/src/tests/config/broken_conf_wrong_quotes.ini
index 1c13e96..ff41f93 100644
--- a/src/tests/config/broken_conf_wrong_quotes.ini
+++ b/src/tests/config/broken_conf_wrong_quotes.ini
@@ -1 +1 @@
sp.cookie_encryption.cookie("\) sp.cookie.name("\)
diff --git a/src/tests/config/config_encrypted_cookies.ini b/src/tests/config/config_encrypted_cookies.ini
index 977d27f..4b50440 100644
--- a/src/tests/config/config_encrypted_cookies.ini
+++ b/src/tests/config/config_encrypted_cookies.ini
@@ -1,3 +1,3 @@
1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR"); 1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR");
2sp.cookie_encryption.cookie("super_cookie"); 2sp.cookie.name("super_cookie").encrypt();
3sp.auto_cookie_secure.enable(); 3sp.auto_cookie_secure.enable();
diff --git a/src/tests/config/config_encrypted_cookies_empty_env.ini b/src/tests/config/config_encrypted_cookies_empty_env.ini
index ac1f840..8c7c779 100644
--- a/src/tests/config/config_encrypted_cookies_empty_env.ini
+++ b/src/tests/config/config_encrypted_cookies_empty_env.ini
@@ -1,2 +1,2 @@
1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR"); 1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR");
2sp.cookie_encryption.cookie("super_cookie"); 2sp.cookie.name("super_cookie").encrypt();
diff --git a/src/tests/config/config_encrypted_cookies_noname.ini b/src/tests/config/config_encrypted_cookies_noname.ini
index 27773e3..048e404 100644
--- a/src/tests/config/config_encrypted_cookies_noname.ini
+++ b/src/tests/config/config_encrypted_cookies_noname.ini
@@ -1,3 +1,3 @@
1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR"); 1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR");
2sp.cookie_encryption.cookie(""); 2sp.cookie.name("").encrypt();
3sp.auto_cookie_secure.enable(); 3sp.auto_cookie_secure.enable();
diff --git a/src/tests/config/config_samesite_cookies.ini b/src/tests/config/config_samesite_cookies.ini
new file mode 100644
index 0000000..9fb5f25
--- /dev/null
+++ b/src/tests/config/config_samesite_cookies.ini
@@ -0,0 +1,5 @@
1sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR");
2sp.cookie.name("super_cookie").samesite("Lax");
3sp.cookie.name("awful_cookie").samesite("strict").encrypt();
4sp.cookie.name("nice_cookie").samesite("STRICT");
5sp.auto_cookie_secure.enable();
diff --git a/src/tests/config/encrypt_cookies_no_env.ini b/src/tests/config/encrypt_cookies_no_env.ini
index 9e1c025..845bd02 100644
--- a/src/tests/config/encrypt_cookies_no_env.ini
+++ b/src/tests/config/encrypt_cookies_no_env.ini
@@ -1,2 +1,2 @@
1sp.global.secret_key("abcdef"); 1sp.global.secret_key("abcdef");
2sp.cookie_encryption.cookie("super_cookie"); 2sp.cookie.name("super_cookie").encrypt();
diff --git a/src/tests/config/encrypt_cookies_no_key.ini b/src/tests/config/encrypt_cookies_no_key.ini
index 1b5cf83..a585e12 100644
--- a/src/tests/config/encrypt_cookies_no_key.ini
+++ b/src/tests/config/encrypt_cookies_no_key.ini
@@ -1,2 +1,2 @@
1sp.global.cookie_env_var("TEST"); 1sp.global.cookie_env_var("TEST");
2sp.cookie_encryption.cookie("super_cookie"); 2sp.cookie.name("super_cookie").encrypt();
diff --git a/src/tests/samesite_cookies.phpt b/src/tests/samesite_cookies.phpt
new file mode 100644
index 0000000..70fe10c
--- /dev/null
+++ b/src/tests/samesite_cookies.phpt
@@ -0,0 +1,61 @@
1--TEST--
2Cookie samesite
3--SKIPIF--
4<?php if (!extension_loaded("snuffleupagus")) die "skip"; ?>
5--INI--
6sp.configuration_file={PWD}/config/config_samesite_cookies.ini
7--COOKIE--
8super_cookie=if_there_is_no_cookie_here_there_is_no_header_list
9--ENV--
10return <<<EOF
11REMOTE_ADDR=2001:0db8:0000:0000:0000:fe00:0042:8329
12HTTP_USER_AGENT=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/59.0.3071.109 Chrome/59.0.3071.109 Safari/537.36
13HTTPS=1
14EOF;
15--FILE--
16<?php
17setcookie("super_cookie", "super_value");
18setcookie("awful_cookie", "awful_value");
19setcookie("nice_cookie", "nice_value", 1, "1", "1", true, true);
20
21$expected = array(
22 'Set-Cookie: super_cookie=super_value; path=; samesite=Lax',
23 'Set-Cookie: awful_cookie=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFyZcYjfEskB0AU0V3%2BvwazcRuU%2Ft6KpcUahvxw%3D; path=; samesite=Strict; HttpOnly',
24 'Set-Cookie: nice_cookie=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAJ8ko%2ByA4y%2Bmw5MGBx8fgc3TWOAvhIu%2BfF%2Bx2g%3D%3D; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=1; samesite=Strict; domain=1; secure; HttpOnly',
25 );
26
27$headers = headers_list();
28if (($i = count($expected)) > count($headers))
29{
30 echo "Fewer headers are being sent than expected - aborting";
31 return;
32}
33
34do
35{
36 if (strncmp(current($headers), 'Set-Cookie:', 11) !== 0)
37 {
38 continue;
39 }
40
41 if (current($headers) === current($expected))
42 {
43 $i--;
44 }
45 else
46 {
47 echo "Header mismatch:\n\tExpected: "
48 .current($expected)
49 ."\n\tReceived: ".current($headers)."\n";
50 }
51
52 next($expected);
53}
54while (next($headers) !== FALSE);
55
56echo ($i === 0)
57 ? 'OK'
58 : 'A total of '.$i.' errors found.';
59?>
60--EXPECT--
61OK