From 36c06637ad262f0e5fc0c8e70f4c1fc6a565f056 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 1 Oct 2017 20:54:03 +0200 Subject: First pass for #9 --- src/sp_config.c | 13 ------ src/sp_config.h | 39 ++++++++--------- src/sp_config_keywords.c | 35 ++++++++------- src/sp_config_utils.c | 50 ++-------------------- src/sp_cookie_encryption.c | 14 +++--- src/sp_network_utils.c | 31 -------------- src/sp_network_utils.h | 1 - src/tests/broken_conf_expecting_int.phpt | 9 ---- src/tests/broken_conf_line_too_long.phpt | 10 ----- src/tests/broken_conf_no_closing_misc.phpt | 10 ----- src/tests/config/broken_conf_expecting_int.ini | 2 - src/tests/config/broken_conf_line_empty_string.ini | 2 +- src/tests/config/broken_conf_line_no_closing.ini | 2 +- src/tests/config/broken_conf_line_too_long.ini | 1 - src/tests/config/broken_conf_lots_of_quotes.ini | 2 +- src/tests/config/broken_conf_no_closing_misc.ini | 1 - src/tests/config/broken_conf_wrong_quotes.ini | 2 +- src/tests/config/config_encrypted_cookies.ini | 4 +- src/tests/config/encrypt_cookies_no_env.ini | 2 + src/tests/config/encrypt_cookies_no_key.ini | 2 + src/tests/encrypt_cookies.phpt | 2 +- src/tests/encrypt_cookies3.phpt | 2 +- src/tests/encrypt_cookies_no_env.phpt | 19 ++++++++ src/tests/encrypt_cookies_no_key.phpt | 19 ++++++++ 24 files changed, 98 insertions(+), 176 deletions(-) delete mode 100644 src/tests/broken_conf_expecting_int.phpt delete mode 100644 src/tests/broken_conf_line_too_long.phpt delete mode 100644 src/tests/broken_conf_no_closing_misc.phpt delete mode 100644 src/tests/config/broken_conf_expecting_int.ini delete mode 100644 src/tests/config/broken_conf_line_too_long.ini delete mode 100644 src/tests/config/broken_conf_no_closing_misc.ini create mode 100644 src/tests/config/encrypt_cookies_no_env.ini create mode 100644 src/tests/config/encrypt_cookies_no_key.ini create mode 100644 src/tests/encrypt_cookies_no_env.phpt create mode 100644 src/tests/encrypt_cookies_no_key.phpt diff --git a/src/sp_config.c b/src/sp_config.c index 3da027c..c41b8f7 100644 --- a/src/sp_config.c +++ b/src/sp_config.c @@ -55,19 +55,6 @@ int parse_empty(char *restrict line, char *restrict keyword, void *retval) { return 0; } -int parse_int(char *restrict line, char *restrict keyword, void *retval) { - size_t consumed = 0; - char *value = get_param(&consumed, line, SP_TYPE_INT, keyword); - if (value) { - sscanf(value, "%ud", (uint32_t *)retval); - pefree(value, 1); - return consumed; - } else { - sp_log_err("error", "%s) is expecting a valid integer on line %zu.", keyword, sp_line_no); - return -1; - } -} - int parse_php_type(char *restrict line, char *restrict keyword, void *retval) { size_t consumed = 0; char *value = get_param(&consumed, line, SP_TYPE_STR, keyword); diff --git a/src/sp_config.h b/src/sp_config.h index 8d41333..34096ce 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -15,16 +15,16 @@ typedef enum { } sp_type; typedef enum { - SP_PHP_TYPE_UNDEF = IS_UNDEF, - SP_PHP_TYPE_NULL = IS_NULL, - SP_PHP_TYPE_FALSE = IS_FALSE, - SP_PHP_TYPE_TRUE = IS_TRUE, - SP_PHP_TYPE_LONG = IS_LONG, - SP_PHP_TYPE_DOUBLE = IS_DOUBLE, - SP_PHP_TYPE_STRING = IS_STRING, - SP_PHP_TYPE_ARRAY = IS_ARRAY, - SP_PHP_TYPE_OBJECT = IS_OBJECT, - SP_PHP_TYPE_RESOURCE = IS_RESOURCE, + SP_PHP_TYPE_UNDEF = IS_UNDEF, + SP_PHP_TYPE_NULL = IS_NULL, + SP_PHP_TYPE_FALSE = IS_FALSE, + SP_PHP_TYPE_TRUE = IS_TRUE, + SP_PHP_TYPE_LONG = IS_LONG, + SP_PHP_TYPE_DOUBLE = IS_DOUBLE, + SP_PHP_TYPE_STRING = IS_STRING, + SP_PHP_TYPE_ARRAY = IS_ARRAY, + SP_PHP_TYPE_OBJECT = IS_OBJECT, + SP_PHP_TYPE_RESOURCE = IS_RESOURCE, SP_PHP_TYPE_REFERENCE = IS_REFERENCE } sp_php_type; @@ -37,7 +37,10 @@ typedef struct { uint8_t mask; } sp_cidr; -typedef struct { char *encryption_key; } sp_config_encryption_key; +typedef struct { + char *encryption_key; + char *cookies_env_var; +} sp_config_global; typedef struct { bool enable; @@ -52,11 +55,7 @@ typedef struct { bool enable; } sp_config_auto_cookie_secure; typedef struct { bool enable; } sp_config_disable_xxe; -typedef struct { - HashTable *names; - uint32_t mask_ipv4; - uint32_t mask_ipv6; -} sp_config_cookie_encryption; +typedef struct { HashTable *names; } sp_config_cookie_encryption; typedef struct { bool enable; @@ -81,7 +80,7 @@ typedef struct { char *ret; pcre *r_ret; sp_php_type ret_type; - + pcre *regexp; char *value; @@ -122,7 +121,7 @@ typedef struct { sp_config_readonly_exec *config_readonly_exec; sp_config_upload_validation *config_upload_validation; sp_config_cookie_encryption *config_cookie_encryption; - sp_config_encryption_key *config_snuffleupagus; + sp_config_global *config_snuffleupagus; sp_config_auto_cookie_secure *config_auto_cookie_secure; sp_config_global_strict *config_global_strict; sp_config_disable_xxe *config_disable_xxe; @@ -185,11 +184,10 @@ typedef struct { // cookies encryption #define SP_TOKEN_NAME ".cookie(" -#define SP_TOKEN_MASK_IPV4 ".mask_ipv4(" -#define SP_TOKEN_MASK_IPV6 ".mask_ipv6(" // Global configuration options #define SP_TOKEN_ENCRYPTION_KEY ".secret_key(" +#define SP_TOKEN_ENV_VAR ".cookie_env_var(" // upload_validator #define SP_TOKEN_UPLOAD_SCRIPT ".script(" @@ -200,7 +198,6 @@ int parse_array(sp_disabled_function *); int parse_str(char *restrict, char *restrict, void *); int parse_regexp(char *restrict, char *restrict, void *); int parse_empty(char *restrict, char *restrict, void *); -int parse_int(char *restrict, char *restrict, void *); int parse_cidr(char *restrict, char *restrict, void *); int parse_php_type(char *restrict, char *restrict, void *); diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index b068bec..cf52ae3 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -19,10 +19,10 @@ static int parse_enable(char *line, bool * restrict retval, bool * restrict simu if (!(enable ^ disable)) { sp_log_err("config", "A rule can't be enabled and disabled on line %zu.", sp_line_no); return -1; - } + } *retval = enable; - + return ret; } @@ -51,11 +51,13 @@ int parse_readonly_exec(char *line) { } int parse_global(char *line) { - sp_config_functions sp_config_funcs_encryption_key[] = { + sp_config_functions sp_config_global[] = { {parse_str, SP_TOKEN_ENCRYPTION_KEY, &(SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key)}, + {parse_str, SP_TOKEN_ENV_VAR, + &(SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)}, {0}}; - return parse_keywords(sp_config_funcs_encryption_key, line); + return parse_keywords(sp_config_global, line); } int parse_cookie_encryption(char *line) { @@ -64,10 +66,6 @@ int parse_cookie_encryption(char *line) { sp_config_functions sp_config_funcs_cookie_encryption[] = { {parse_str, SP_TOKEN_NAME, &name}, - {parse_int, SP_TOKEN_MASK_IPV4, - &(SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv4)}, - {parse_int, SP_TOKEN_MASK_IPV6, - &(SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv6)}, {0}}; ret = parse_keywords(sp_config_funcs_cookie_encryption, line); @@ -75,11 +73,16 @@ int parse_cookie_encryption(char *line) { return ret; } - if (32 < SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv4) { - SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv4 = 32; - } - if (128 < SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv6) { - SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv6 = 128; + if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)) { + sp_log_err("config", "You're trying to use the cookie encryption feature" + "on line %zu without having set the `.cookie_env_var` option in" + "`sp.global`: please set it first.", sp_line_no); + return -1; + } else if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key)) { + sp_log_err("config", "You're trying to use the cookie encryption feature" + "on line %zu without having set the `.encryption_key` option in" + "`sp.global`: please set it first.", sp_line_no); + return -1; } if (name) { @@ -190,7 +193,7 @@ int parse_disabled_functions(char *line) { } df->param_is_array = 1; } - + if (df->var && strchr(df->var, '[')) { // assume that this is an array df->var_array_keys = sp_new_list(); if (0 != array_to_list(&df->var, &df->var_array_keys)) { @@ -247,7 +250,7 @@ int parse_upload_validation(char *line) { if (!(enable ^ disable)) { sp_log_err("config", "A rule can't be enabled and disabled on line %zu.", sp_line_no); return -1; - } + } SNUFFLEUPAGUS_G(config).config_upload_validation->enable = enable; char const *script = SNUFFLEUPAGUS_G(config).config_upload_validation->script; @@ -263,6 +266,6 @@ int parse_upload_validation(char *line) { sp_log_err("config", "The `script` (%s) isn't executable on line %zu.", script, sp_line_no); return -1; } - + return ret; } diff --git a/src/sp_config_utils.c b/src/sp_config_utils.c index 39951cc..3ea82d0 100644 --- a/src/sp_config_utils.c +++ b/src/sp_config_utils.c @@ -2,18 +2,8 @@ size_t sp_line_no; -static int validate_int(const char *value); static int validate_str(const char *value); -static sp_pure int validate_int(const char *value) { - for (size_t i = 0; i < strlen(value); i++) { - if (!isdigit(value[i])) { - return -1; - } - } - return 0; -} - static sp_pure int validate_str(const char *value) { int balance = 0; // ghetto [] validation @@ -124,48 +114,14 @@ err: return NULL; } -static char *get_misc(char *restrict line, const char *restrict keyword) { - size_t i = 0; - char *ret = pecalloc(sizeof(char), 1024, 1); - - while (i < 1024 - 1 && line[i] && line[i] != SP_TOKEN_END_PARAM) { - ret[i] = line[i]; - i++; - } - - if (line[i] != SP_TOKEN_END_PARAM) { - if (i >= 1024 - 1) { - sp_log_err("config", "The following line is too long: %s on line %zu.", line, sp_line_no); - } else { - sp_log_err("config", "Missing closing %c in line %s on line %zu.", SP_TOKEN_END_PARAM, - line, sp_line_no); - } - return NULL; - } else if (i == 0) { - sp_log_err("config", "The keyword %s%c is expecting a parameter on line %zu.", - keyword, SP_TOKEN_END_PARAM, sp_line_no); - return NULL; - } - return ret; -} - char *get_param(size_t *consumed, char *restrict line, sp_type type, const char *restrict keyword) { - char *retval = NULL; - if (type == SP_TYPE_STR) { - retval = get_string(consumed, line, keyword); - } else { - retval = get_misc(line, keyword); - *consumed = retval ? strlen(retval) : 0; - } + char *retval = get_string(consumed, line, keyword); - if (retval) { - if (type == SP_TYPE_STR && 0 == validate_str(retval)) { - return retval; - } else if (type == SP_TYPE_INT && 0 == validate_int(retval)) { + if (retval && 0 == validate_str(retval)) { return retval; - } } + return NULL; } diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c index a47f6e1..69c438d 100644 --- a/src/sp_cookie_encryption.c +++ b/src/sp_cookie_encryption.c @@ -9,7 +9,8 @@ static unsigned int nonce_d = 0; static inline void generate_key(unsigned char *key) { PHP_SHA256_CTX ctx; const char *user_agent = sp_getenv("HTTP_USER_AGENT"); - const char *remote_addr = sp_getenv("REMOTE_ADDR"); + const char *env_var = + sp_getenv(SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var); const char *encryption_key = SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key; @@ -22,10 +23,8 @@ static inline void generate_key(unsigned char *key) { PHP_SHA256Update(&ctx, (unsigned char *)user_agent, strlen(user_agent)); } - if (remote_addr) { - char out[128]; - apply_mask_on_ip(out, remote_addr); - PHP_SHA256Update(&ctx, (unsigned char*)out, sizeof(out)); + if (env_var) { + PHP_SHA256Update(&ctx, (unsigned char*)env_var, strlen(env_var)); } if (encryption_key) { @@ -115,8 +114,11 @@ static zend_string *encrypt_data(char *data, unsigned long long data_len) { assert(sizeof(size_t) <= crypto_secretbox_NONCEBYTES); + if (0 == nonce_d) { + nonce_d = getpid(); + } nonce_d++; - sscanf((char*)nonce, "%ud", &nonce_d); + sscanf((char*)nonce, "%ud", &nonce_d); memcpy(encrypted_data, nonce, crypto_secretbox_NONCEBYTES); crypto_secretbox(encrypted_data + crypto_secretbox_NONCEBYTES, diff --git a/src/sp_network_utils.c b/src/sp_network_utils.c index 28bc324..4b78ca5 100644 --- a/src/sp_network_utils.c +++ b/src/sp_network_utils.c @@ -12,37 +12,6 @@ static inline bool cidr6_match(const struct in6_addr address, const struct in6_addr network, uint8_t bits); static inline int get_ip_version(const char *ip); -void apply_mask_on_ip(char *out, const char *const remote_addr) { - uint8_t mask4 = SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv4; - uint8_t mask6 = SNUFFLEUPAGUS_G(config).config_cookie_encryption->mask_ipv6; - const int ip_version = get_ip_version(remote_addr); - - memset(out, 0, 128); - - if (ip_version == AF_INET) { - struct in_addr out4; - inet_pton(AF_INET, remote_addr, &out4); - const long n = out4.s_addr & htonl(0xFFFFFFFFu << (32 - mask4)); - out[0] = (n >> 24) & 0xFF; - out[1] = (n >> 16) & 0xFF; - out[2] = (n >> 8) & 0xFF; - out[3] = (n >> 0) & 0xFF; - } else if (ip_version == AF_INET6) { - inet_pton(AF_INET6, remote_addr, out); - uint32_t *p_ip = (uint32_t *)out; - while (32 < mask6) { - *p_ip = 0xFFFFFFFFu; - p_ip++; - mask6 -= 32; - } - if (0 != mask6) { - *p_ip = htonl(0xFFFFFFFFu << (32 - mask6)); - } - } else { - sp_log_err("ip_mask", "It seems that %s isn't a valid ip.", remote_addr); - } -} - /* http://fxr.watson.org/fxr/source/include/net/xfrm.h?v=linux-2.6#L840 */ static inline bool cidr4_match(const struct in_addr addr, const struct in_addr net, uint8_t bits) { diff --git a/src/sp_network_utils.h b/src/sp_network_utils.h index 6b6ce92..2c1062a 100644 --- a/src/sp_network_utils.h +++ b/src/sp_network_utils.h @@ -3,6 +3,5 @@ int get_ip_and_cidr(char *, sp_cidr *); bool cidr_match(const char *, const sp_cidr *); -void apply_mask_on_ip(char *, const char * const); #endif /*SP_NETWORK_UTILS_H*/ diff --git a/src/tests/broken_conf_expecting_int.phpt b/src/tests/broken_conf_expecting_int.phpt deleted file mode 100644 index 207e21a..0000000 --- a/src/tests/broken_conf_expecting_int.phpt +++ /dev/null @@ -1,9 +0,0 @@ ---TEST-- -Bad integer value in configuration ---SKIPIF-- - ---INI-- -sp.configuration_file={PWD}/config/broken_conf_expecting_int.ini ---FILE-- ---EXPECT-- -[snuffleupagus][0.0.0.0][error][error] .mask_ipv4() is expecting a valid integer on line 2. diff --git a/src/tests/broken_conf_line_too_long.phpt b/src/tests/broken_conf_line_too_long.phpt deleted file mode 100644 index c4e3938..0000000 --- a/src/tests/broken_conf_line_too_long.phpt +++ /dev/null @@ -1,10 +0,0 @@ ---TEST-- -Line too long in configuration ---SKIPIF-- - ---INI-- -sp.configuration_file={PWD}/config/broken_conf_line_too_long.ini ---FILE-- ---EXPECT-- -[snuffleupagus][0.0.0.0][config][error] The following line is too long: 1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111); on line 1. -[snuffleupagus][0.0.0.0][error][error] .mask_ipv4() is expecting a valid integer on line 1. diff --git a/src/tests/broken_conf_no_closing_misc.phpt b/src/tests/broken_conf_no_closing_misc.phpt deleted file mode 100644 index 933f290..0000000 --- a/src/tests/broken_conf_no_closing_misc.phpt +++ /dev/null @@ -1,10 +0,0 @@ ---TEST-- -Configuration line without closing parenthese, misc ---SKIPIF-- - ---INI-- -sp.configuration_file={PWD}/config/broken_conf_no_closing_misc.ini ---FILE-- ---EXPECT-- -[snuffleupagus][0.0.0.0][config][error] Missing closing ) in line 123 on line 1. -[snuffleupagus][0.0.0.0][error][error] .mask_ipv4() is expecting a valid integer on line 1. diff --git a/src/tests/config/broken_conf_expecting_int.ini b/src/tests/config/broken_conf_expecting_int.ini deleted file mode 100644 index 8e2efea..0000000 --- a/src/tests/config/broken_conf_expecting_int.ini +++ /dev/null @@ -1,2 +0,0 @@ -sp.global.secret_key("abcdef"); -sp.cookie_encryption.cookie("super_cookie").mask_ipv4(abc); diff --git a/src/tests/config/broken_conf_line_empty_string.ini b/src/tests/config/broken_conf_line_empty_string.ini index 74d0e5a..c130384 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.mask_ipv4(123).cookie( +sp.cookie_encryption.cookie( diff --git a/src/tests/config/broken_conf_line_no_closing.ini b/src/tests/config/broken_conf_line_no_closing.ini index bcac291..24dc3f0 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.mask_ipv4(123).cookie("123" +sp.cookie_encryption.cookie("123" diff --git a/src/tests/config/broken_conf_line_too_long.ini b/src/tests/config/broken_conf_line_too_long.ini deleted file mode 100644 index ed057a5..0000000 --- a/src/tests/config/broken_conf_line_too_long.ini +++ /dev/null @@ -1 +0,0 @@ -sp.cookie_encryption.cookie("super_cookie").mask_ipv4(1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111); diff --git a/src/tests/config/broken_conf_lots_of_quotes.ini b/src/tests/config/broken_conf_lots_of_quotes.ini index dfd48e7..310bce5 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.mask_ipv4(123).cookie("this\"is a weird\"\"\"cookie\"name""); +sp.cookie_encryption.cookie("this\"is a weird\"\"\"cookie\"name""); diff --git a/src/tests/config/broken_conf_no_closing_misc.ini b/src/tests/config/broken_conf_no_closing_misc.ini deleted file mode 100644 index 2cb79a8..0000000 --- a/src/tests/config/broken_conf_no_closing_misc.ini +++ /dev/null @@ -1 +0,0 @@ -sp.cookie_encryption.cookie("123").mask_ipv4(123 diff --git a/src/tests/config/broken_conf_wrong_quotes.ini b/src/tests/config/broken_conf_wrong_quotes.ini index c8cc949..1c13e96 100644 --- a/src/tests/config/broken_conf_wrong_quotes.ini +++ b/src/tests/config/broken_conf_wrong_quotes.ini @@ -1 +1 @@ -sp.cookie_encryption.mask_ipv4(123).cookie("\) +sp.cookie_encryption.cookie("\) diff --git a/src/tests/config/config_encrypted_cookies.ini b/src/tests/config/config_encrypted_cookies.ini index 710e863..977d27f 100644 --- a/src/tests/config/config_encrypted_cookies.ini +++ b/src/tests/config/config_encrypted_cookies.ini @@ -1,3 +1,3 @@ -sp.global.secret_key("abcdef"); -sp.cookie_encryption.cookie("super_cookie").mask_ipv4(8).mask_ipv6(2); +sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR"); +sp.cookie_encryption.cookie("super_cookie"); sp.auto_cookie_secure.enable(); diff --git a/src/tests/config/encrypt_cookies_no_env.ini b/src/tests/config/encrypt_cookies_no_env.ini new file mode 100644 index 0000000..9e1c025 --- /dev/null +++ b/src/tests/config/encrypt_cookies_no_env.ini @@ -0,0 +1,2 @@ +sp.global.secret_key("abcdef"); +sp.cookie_encryption.cookie("super_cookie"); diff --git a/src/tests/config/encrypt_cookies_no_key.ini b/src/tests/config/encrypt_cookies_no_key.ini new file mode 100644 index 0000000..1b5cf83 --- /dev/null +++ b/src/tests/config/encrypt_cookies_no_key.ini @@ -0,0 +1,2 @@ +sp.global.cookie_env_var("TEST"); +sp.cookie_encryption.cookie("super_cookie"); diff --git a/src/tests/encrypt_cookies.phpt b/src/tests/encrypt_cookies.phpt index f8bf64f..d581dbc 100644 --- a/src/tests/encrypt_cookies.phpt +++ b/src/tests/encrypt_cookies.phpt @@ -5,7 +5,7 @@ Cookie decryption in ipv4 --INI-- sp.configuration_file={PWD}/config/config_encrypted_cookies.ini --COOKIE-- -super_cookie=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEmXkk3H0xheoOMxoWPEDw1Zd8NAmD9KbB2DSjQ=%3d;awful_cookie=awful_cookie_value; +super_cookie=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP3gV9YJZL/pUeNAjCKFW0U2ywmf1CwHzwd2pWM=;awful_cookie=awful_cookie_value; --ENV-- return << +--INI-- +sp.configuration_file={PWD}/config/encrypt_cookies_no_env.ini +display_errors=1 +display_startup_errors=1 +error_reporting=E_ALL +--COOKIE-- +super_cookie=1337;awful_cookie=awful_cookie_value; +--ENV-- +return << +--EXPECT-- +1 diff --git a/src/tests/encrypt_cookies_no_key.phpt b/src/tests/encrypt_cookies_no_key.phpt new file mode 100644 index 0000000..b54690f --- /dev/null +++ b/src/tests/encrypt_cookies_no_key.phpt @@ -0,0 +1,19 @@ +--TEST-- +Cookie encryption - no encryption key specified +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/encrypt_cookies_no_key.ini +display_errors=1 +display_startup_errors=1 +error_reporting=E_ALL +--COOKIE-- +super_cookie=1337;awful_cookie=awful_cookie_value; +--ENV-- +return << +--EXPECT-- +1 -- cgit v1.3 From cd760451559aa2b9a8a242349fa8aefd83d4515d Mon Sep 17 00:00:00 2001 From: jvoisin Date: Sun, 1 Oct 2017 21:27:54 +0200 Subject: Update the documentation accordingly --- doc/source/config.rst | 44 +++++++++++++++++++++++++++++++++----------- doc/source/features.rst | 13 ++++++------- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/doc/source/config.rst b/doc/source/config.rst index 25a6b73..7170385 100644 --- a/doc/source/config.rst +++ b/doc/source/config.rst @@ -38,7 +38,7 @@ global_strict ^^^^^^^^^^^^^ `default: disabled` -``global_strict`` will enable the `strict `_ mode globally, +``global_strict`` will enable the `strict `_ mode globally, forcing PHP to throw a `TypeError `_ exception if an argument type being passed to a function does not match its corresponding declared parameter type. @@ -53,7 +53,7 @@ harden_random ^^^^^^^^^^^^^ * `default: enabled` * `more `__ - + ``harden_random`` will silently replace the insecure `rand `_ and `mt_rand `_ functions with the secure PRNG `random_int `_. @@ -85,7 +85,7 @@ unserialize_hmac ^^^^^^^^^^^^^^^^ * `default: disabled` * `more `__ - + ``unserialize_hmac`` will add integrity check to ``unserialize`` calls, preventing abritrary code execution in their context. @@ -101,7 +101,7 @@ auto_cookie_secure ^^^^^^^^^^^^^^^^^^ * `default: disabled` * `more `__ - + ``auto_cookie_secure`` will automatically mark cookies as `secure `_ when the web page is requested over HTTPS. @@ -112,17 +112,19 @@ It can either be ``enabled`` or ``disabled``. sp.auto_cookie_secure.enable(); sp.auto_cookie_secure.disable(); +.. _cookie-encryption_config: cookie_encryption ^^^^^^^^^^^^^^^^^ * `default: disabled` * `more `__ - + .. warning:: - To use this feature, you **must** set the :ref:`global.secret_key ` variable. + To use this feature, you **must** set the :ref:`global.secret_key ` and + and the :ref:`global.cookie_env_var ` variables. This design decision prevents attacker from `trivially bruteforcing `_ - session cookies. + or re-using session cookies. ``cookie_secure`` will activate transparent encryption of specific cookies. @@ -133,6 +135,26 @@ It can either be ``enabled`` or ``disabled``, and used in ``simulation`` mode. sp.cookie_encryption.cookie("my_cookie_name"); sp.cookie_encryption.cookie("another_cookie_name"); +Choosing the proper environment variable +"""""""""""""""""""""""""""""""""""""""" + +It's up to you to chose a meaningul environment variable to derive the key from. +Suhosin `is using `_ +the ``REMOTE_ADDR`` one, tying the validity of the cookie to the IP address of the user; +unfortunately, nowadays, people are `roaming `_ a lot on their smartphone, +hopping from WiFi to 4G, … + +This is why we recommend, if possible, to use the *extended master secret* +from TLS connections (`RFC7627 `_) +instead, to make the valitity of the cookie TLS-dependent, by using the ``SSL_SESSION_ID`` variable. + +- In `Apache `_, + it possible to enable by adding ``SSLOptions StdEnvVars`` in your Apache2 configuration. +- In `nginx `_, + you have to use ``fastcgi_param SSL_SESSION_ID $ssl_session_id if_not_empty;``. + +If you're not using TLS (you should.), you can always use the ``REMOTE_ADDR`` one, +or ``X-Real-IP`` if you're behind a reverse proxy. readonly_exec ^^^^^^^^^^^^^ @@ -151,7 +173,7 @@ upload_validation * `default: disabled` * `more `__ -``upload_validation`` will call a given script upon a file upload, with the path +``upload_validation`` will call a given script upon a file upload, with the path to the file being uploaded as argument, and various information about it in the environment: * ``SP_FILENAME``: the name of the uploaded file @@ -192,8 +214,8 @@ Snuffleupagus provides virtual-patching, via the ``disable_functions`` directive Admitting you have a call to ``system()`` that lacks proper user-input validation, thus leading to an **RCE**, this might be the right tool. :: - - # Restrict calls to `system` to `id` in the `id.php` file + + # Allow `id.php` to restrict system() calls to `id` sp.disable_functions.function("system").filename("id.php").param("cmd").value("id").allow(); sp.disable_functions.function("system").filename("id.php").drop() @@ -293,4 +315,4 @@ Miscellaneous examples """""""""""""""""""""" .. literalinclude:: ../../config/examples.ini - :language: python \ No newline at end of file + :language: python diff --git a/doc/source/features.rst b/doc/source/features.rst index fbb2a64..c0fade3 100644 --- a/doc/source/features.rst +++ b/doc/source/features.rst @@ -61,17 +61,16 @@ Session-cookie stealing via XSS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The goto payload for XSS is often to steal cookies. -Like *Suhosin*, we are encrypting the cookies with a secret key, the IP of the user +Like *Suhosin*, we are encrypting the cookies with a secret key, +an environment variable (usually the IP of the user) and its user-agent. This means that an attacker with an XSS won't be able to use -the stolen cookie, since he (often) can't spoof the IP address of the user. +the stolen cookie, since he can't spoof the content of the value of the environment +variable for the user. Please do read the :ref:`documentation about this feature ` +if you're planning to use it. This feature is roughly the same than the `Suhosin one `_. -Users behind the same IP address but with different browsers won't be able to use each other stolen cookies, -except if they can manage to guess the user agent. This isn't especially difficult, -but an invalid decryption will leave a trace in the logs. - -Finally, having a secret server-side key will prevent anyone (even the user himself) +Having a secret server-side key will prevent anyone (even the user himself) from reading the content of the cookie, reducing the impact of an application storing sensitive data client-side. The encryption is done via the `tweetnacl library `_, -- cgit v1.3 From 3552ac0647966ea8bd6ddf0df6ac899421503e8e Mon Sep 17 00:00:00 2001 From: jvoisin Date: Mon, 2 Oct 2017 13:57:19 +0200 Subject: Add a warning if the env var is NULL --- src/sp_cookie_encryption.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c index 69c438d..a65a748 100644 --- a/src/sp_cookie_encryption.c +++ b/src/sp_cookie_encryption.c @@ -25,6 +25,10 @@ static inline void generate_key(unsigned char *key) { if (env_var) { PHP_SHA256Update(&ctx, (unsigned char*)env_var, strlen(env_var)); + } else { + sp_log_err("cookie_encryption", "The environment variable '%s'" + "is empty, cookies are weakly encrypted.", + SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var); } if (encryption_key) { -- cgit v1.3