Merge pull request #19 from nbs-system/9-cookies-encryption-env-var

Allow to chose the environment variable to derive the cookie encryption key from.

doc/source/config.rst

@@ -38,7 +38,7 @@ global_strict
 ^^^^^^^^^^^^^
 `default: disabled`
 
-``global_strict`` will enable the `strict <https://secure.php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict>`_ mode globally,
+``global_strict`` will enable the `strict <https://secure.php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration.strict>`_ mode globally, 
 forcing PHP to throw a `TypeError <https://secure.php.net/manual/en/class.typeerror.php>`_
 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 <features.html#weak-prng-via-rand-mt-rand>`__
-
+ 
 ``harden_random`` will silently replace the insecure `rand <https://secure.php.net/manual/en/function.rand.php>`_
 and `mt_rand <https://secure.php.net/manual/en/function.mt-rand.php>`_ functions with
 the secure PRNG `random_int <https://secure.php.net/manual/en/function.random-int.php>`_.
@@ -85,7 +85,7 @@ unserialize_hmac
 ^^^^^^^^^^^^^^^^
  * `default: disabled`
  * `more <features.html#unserialize-related-magic>`__
-
+ 
 ``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 <features.html#session-cookie-stealing-via-xss>`__
-
+ 
 ``auto_cookie_secure`` will automatically mark cookies as `secure <https://en.wikipedia.org/wiki/HTTP_cookie#Secure_cookie>`_
 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 <features.html#session-cookie-stealing-via-xss>`__
-
+   
 .. warning::
 
-  To use this feature, you **must** set the :ref:`global.secret_key <config_global>` variable.
+  To use this feature, you **must** set the :ref:`global.secret_key <config_global>` and
+  and the :ref:`global.cookie_env_var <config_global>` variables.
   This design decision prevents attacker from
   `trivially bruteforcing <https://www.idontplaydarts.com/2011/11/decrypting-suhosin-sessions-and-cookies/>`_
-  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 <https://www.suhosin.org/stories/configuration.html#suhosin-session-cryptraddr>`_
+the ``REMOTE_ADDR`` one, tying the validity of the cookie to the IP address of the user;
+unfortunately, nowadays, people are `roaming <https://en.wikipedia.org/wiki/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 <https://tools.ietf.org/html/rfc7627>`_)
+instead, to make the valitity of the cookie TLS-dependent, by using the ``SSL_SESSION_ID`` variable.
+
+- In `Apache <https://httpd.apache.org/docs/current/mod/mod_ssl.html>`_,
+  it possible to enable by adding ``SSLOptions StdEnvVars`` in your Apache2 configuration.
+- In `nginx <https://nginx.org/en/docs/http/ngx_http_ssl_module.html#variables>`_,
+  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 <features.html#remote-code-execution-via-file-upload>`__
 
-``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
+   :language: python

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 <cookie-encryption_config>`
+if you're planning to use it.
 
 This feature is roughly the same than the `Suhosin one <https://suhosin.org/stories/configuration.html#transparent-encryption-options>`_.
 
-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 <https://tweetnacl.cr.yp.to/>`_,

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);

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 *);
 

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,22 +66,23 @@ 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);
   if (0 != ret) {
     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;
 }