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.

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 <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
\ 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 <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/>`_,