From 4fafa8ae5a7bcd700f368bbe6016e0b0fb2cc892 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 27 Dec 2017 15:43:33 +0100 Subject: Implement simulation mode for cookies (de/en)cryption This should close #102 This commit can be useful for two use-cases: 1. When deploying Snuffleupagus on big CMS like Magento, and not knowing what cookies are modified via javascript. 2. When deploying Snuffleupagus on big websites: you don't want to disconnect every single user at once. When simulation is enabled, if the decryption fails, a log message is now issued, and the cookie value taken as it (since odds are that it's non-encrypted). --- src/sp_config.h | 1 + src/sp_config_keywords.c | 1 + src/sp_cookie_encryption.c | 31 ++++++++++++++++------ .../config/config_encrypted_cookies_simulation.ini | 3 +++ ...pt_cookies_invalid_decryption_short_cookie.phpt | 24 +++++++++++++++++ ...rypt_cookies_invalid_decryption_simulation.phpt | 27 +++++++++++++++++++ 6 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 src/tests/config/config_encrypted_cookies_simulation.ini create mode 100644 src/tests/encrypt_cookies_invalid_decryption_short_cookie.phpt create mode 100644 src/tests/encrypt_cookies_invalid_decryption_simulation.phpt diff --git a/src/sp_config.h b/src/sp_config.h index 86513f9..3a7a79c 100644 --- a/src/sp_config.h +++ b/src/sp_config.h @@ -58,6 +58,7 @@ typedef struct { bool enable; } sp_config_disable_xxe; typedef struct { enum samesite_type {strict=1, lax=2} samesite; bool encrypt; + bool simulation; } sp_cookie; typedef struct { diff --git a/src/sp_config_keywords.c b/src/sp_config_keywords.c index 2d294ee..32363b8 100644 --- a/src/sp_config_keywords.c +++ b/src/sp_config_keywords.c @@ -111,6 +111,7 @@ int parse_cookie(char *line) { sp_config_functions sp_config_funcs_cookie_encryption[] = { {parse_str, SP_TOKEN_NAME, &name}, {parse_str, SP_TOKEN_SAMESITE, &samesite}, + {parse_empty, SP_TOKEN_SIMULATION, &cookie->simulation}, {parse_empty, SP_TOKEN_ENCRYPT, &cookie->encrypt}, {0}}; diff --git a/src/sp_cookie_encryption.c b/src/sp_cookie_encryption.c index c749040..04c864f 100644 --- a/src/sp_cookie_encryption.c +++ b/src/sp_cookie_encryption.c @@ -63,9 +63,17 @@ int decrypt_cookie(zval *pDest, int num_args, va_list args, if (ZSTR_LEN(debase64) < crypto_secretbox_NONCEBYTES + crypto_secretbox_ZEROBYTES) { - sp_log_msg("cookie_encryption", SP_LOG_DROP, - "Buffer underflow tentative detected in cookie encryption handling."); - return ZEND_HASH_APPLY_REMOVE; + if (true == cookie->simulation) { + sp_log_msg("cookie_encryption", SP_LOG_SIMULATION, + "Buffer underflow tentative detected in cookie encryption handling " + "for %s. Using the cookie 'as it' instead of decrypting it.", + ZSTR_VAL(hash_key->key)); + return ZEND_HASH_APPLY_KEEP; + } else { + sp_log_msg("cookie_encryption", SP_LOG_DROP, + "Buffer underflow tentative detected in cookie encryption handling."); + return ZEND_HASH_APPLY_REMOVE; + } } generate_key(key); @@ -78,11 +86,18 @@ int decrypt_cookie(zval *pDest, int num_args, va_list args, ZSTR_LEN(debase64) - crypto_secretbox_NONCEBYTES, (unsigned char *)ZSTR_VAL(debase64), key); - if (ret == -1) { - sp_log_msg("cookie_encryption", SP_LOG_DROP, - "Something went wrong with the decryption of %s.", - ZSTR_VAL(hash_key->key)); - return ZEND_HASH_APPLY_REMOVE; + if (-1 == ret) { + if (true == cookie->simulation) { + sp_log_msg("cookie_encryption", SP_LOG_SIMULATION, + "Something went wrong with the decryption of %s. Using the cookie " + "'as it' instead of decrypting it", ZSTR_VAL(hash_key->key)); + return ZEND_HASH_APPLY_KEEP; + } else { + sp_log_msg("cookie_encryption", SP_LOG_DROP, + "Something went wrong with the decryption of %s.", + ZSTR_VAL(hash_key->key)); + return ZEND_HASH_APPLY_REMOVE; + } } ZVAL_STRINGL(pDest, (char *)(decrypted + crypto_secretbox_ZEROBYTES), diff --git a/src/tests/config/config_encrypted_cookies_simulation.ini b/src/tests/config/config_encrypted_cookies_simulation.ini new file mode 100644 index 0000000..32e24a1 --- /dev/null +++ b/src/tests/config/config_encrypted_cookies_simulation.ini @@ -0,0 +1,3 @@ +sp.global.secret_key("abcdef").cookie_env_var("REMOTE_ADDR"); +sp.cookie.name("super_cookie").encrypt().simulation(); +sp.auto_cookie_secure.enable(); diff --git a/src/tests/encrypt_cookies_invalid_decryption_short_cookie.phpt b/src/tests/encrypt_cookies_invalid_decryption_short_cookie.phpt new file mode 100644 index 0000000..e5b6bfc --- /dev/null +++ b/src/tests/encrypt_cookies_invalid_decryption_short_cookie.phpt @@ -0,0 +1,24 @@ +--TEST-- +Cookie encryption - invalid decryption in simulation mode with a short cookie +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/config_encrypted_cookies_simulation.ini +display_errors=1 +display_startup_errors=1 +error_reporting=E_ALL +--COOKIE-- +super_cookie=AAA;awful_cookie=awful_cookie_value; +--ENV-- +return << +--EXPECT-- +array(2) { + ["super_cookie"]=> + string(3) "AAA" + ["awful_cookie"]=> + string(18) "awful_cookie_value" +} diff --git a/src/tests/encrypt_cookies_invalid_decryption_simulation.phpt b/src/tests/encrypt_cookies_invalid_decryption_simulation.phpt new file mode 100644 index 0000000..0bd1dc8 --- /dev/null +++ b/src/tests/encrypt_cookies_invalid_decryption_simulation.phpt @@ -0,0 +1,27 @@ +--TEST-- +Cookie encryption - invalid decryption in simulation mode +--SKIPIF-- + +--INI-- +sp.configuration_file={PWD}/config/config_encrypted_cookies_simulation.ini +display_errors=1 +display_startup_errors=1 +error_reporting=E_ALL +--COOKIE-- +super_cookie=Wk9NR1RISVNJU05PVEVOQ1JZUFRFREFUQUxMV0hBVFRIRUhFTExJU0hIRUxMQVJFWU9VRE9JTkdaT01Hb2htYXliZXRoaXNpc2Fub2xkc2Vzc2lvbmNvb2tpZQo=;awfulcookie=awfulcookievalue; +--ENV-- +return << +--EXPECT-- +1337 +array(2) { + ["super_cookie"]=> + string(124) "Wk9NR1RISVNJU05PVEVOQ1JZUFRFREFUQUxMV0hBVFRIRUhFTExJU0hIRUxMQVJFWU9VRE9JTkdaT01Hb2htYXliZXRoaXNpc2Fub2xkc2Vzc2lvbmNvb2tpZQo=" + ["awfulcookie"]=> + string(16) "awfulcookievalue" +} -- cgit v1.3