security: when served over https, set the "secure" flag for "express_sid" and "language" cookie

The mechanism used for determining if the application is being served over SSL
is wrapped by the "express-session" library for "express_sid", and manual for
the "language" cookie, but it's very similar in both cases.

The "secure" flag is set if one of these is true:

1. we are directly serving Etherpad over SSL using the native nodejs
   functionality, via the "ssl" options in settings.json

2. Etherpad is being served in plaintext by nodejs, but we are using a reverse
   proxy for terminating the SSL for us;
   In this case, the user has to be instructed to properly set trustProxy: true
   in settings.json, and the information wheter the application is over SSL or
   not will be extracted from the X-Forwarded-Proto HTTP header.

Please note that this will not be compatible with applications being served over
http and https at the same time.

The change on webaccess.js amends 009b61b338, which did not work when the SSL
termination was performed by a reverse proxy.

Reference for automatic "express_sid" configuration:
https://github.com/expressjs/session/blob/v1.17.0/README.md#cookiesecure

Closes #3561.

CHANGELOG.md

@@ -1,5 +1,6 @@
 # 1.8
 * SECURITY: change referrer policy so that Etherpad addresses aren't leaked when links are clicked (discussion: https://github.com/ether/etherpad-lite/pull/3636)
+* SECURITY: set the "secure" flag for the session cookies when served over SSL. From now on it will not be possible to serve the same instance both in cleartext and over SSL
 
 # 1.8-beta.1
 * FEATURE: code was migrated to `async`/`await`, getting rid of a lot of callbacks (see https://github.com/ether/etherpad-lite/issues/3540)

doc/docker.md

@@ -71,6 +71,7 @@ Available options:
 * `DB_FILENAME`: in case `DB_TYPE` is `DirtyDB`, the database filename. Default: `var/dirty.db`
 * `ADMIN_PASSWORD`: the password for the `admin` user (leave unspecified if you do not want to create it)
 * `USER_PASSWORD`: the password for the first user `user` (leave unspecified if you do not want to create it)
+* `TRUST_PROXY`: set to `true` if you are using a reverse proxy in front of Etherpad (for example: Traefik for SSL termination via Let's Encrypt). This will affect security and correctness of the logs if not done
 * `LOGLEVEL`: valid values are `DEBUG`, `INFO`, `WARN` and `ERROR`
 
 ### Examples

settings.json.docker

@@ -287,8 +287,14 @@
 
   /*
    * When you use NGINX or another proxy/load-balancer set this to true.
+   *
+   * This is especially necessary when the reverse proxy performs SSL
+   * termination, otherwise the cookies will not have the "secure" flag.
+   *
+   * The other effect will be that the logs will contain the real client's IP,
+   * instead of the reverse proxy's IP.
    */
-  "trustProxy": false,
+  "trustProxy": "${TRUST_PROXY:false}",
 
   /*
    * Privacy: disable IP logging

settings.json.template

@@ -290,6 +290,12 @@
 
   /*
    * When you use NGINX or another proxy/load-balancer set this to true.
+   *
+   * This is especially necessary when the reverse proxy performs SSL
+   * termination, otherwise the cookies will not have the "secure" flag.
+   *
+   * The other effect will be that the logs will contain the real client's IP,
+   * instead of the reverse proxy's IP.
    */
   "trustProxy": false,
 

src/node/hooks/express/specialpages.js

@@ -45,7 +45,24 @@ exports.expressCreateServer = function (hook_name, args, cb) {
     // Or if language cookie doesn't exist
     if (req.cookies.language === undefined)
     {
-      res.cookie('language', settings.padOptions.lang);
+      cookieOptions = {
+        /* req.protocol may be 'https' because either:
+         *
+         * 1. we are directly serving the nodejs application over SSL, using
+         *    the "ssl" options in settings.json
+         *
+         * 2. we are serving the nodejs application in plaintext, but we are
+         *    using a reverse proxy that terminates SSL for us. In this case,
+         *    the user has to set trustProxy = true in settings.json, and thus
+         *    req.protocol will reflect the value of the X-Forwarded-Proto HTTP
+         *    header
+         *
+         * Please note that this will not be compatible with applications being
+         * served over http and https at the same time.
+         */
+        secure: (req.protocol === 'https'),
+      }
+      res.cookie('language', settings.padOptions.lang, cookieOptions);
     }
 
     // The below might break for pads being rewritten

src/node/hooks/express/webaccess.js

@@ -131,7 +131,27 @@ exports.expressConfigure = function (hook_name, args, cb) {
     name: 'express_sid',
     proxy: true,
     cookie: {
-      secure: !!settings.ssl,
+      /*
+       * The automatic express-session mechanism for determining if the
+       * application is being served over ssl is similar to the one used for
+       * setting the language cookie, which check if one of these conditions is
+       * true:
+       *
+       * 1. we are directly serving the nodejs application over SSL, using the
+       *    "ssl" options in settings.json
+       *
+       * 2. we are serving the nodejs application in plaintext, but we are using
+       *    a reverse proxy that terminates SSL for us. In this case, the user
+       *    has to set trustProxy = true in settings.json, and the information
+       *    wheter the application is over SSL or not will be extracted from the
+       *    X-Forwarded-Proto HTTP header
+       *
+       * Please note that this will not be compatible with applications being
+       * served over http and https at the same time.
+       *
+       * reference: https://github.com/expressjs/session/blob/v1.17.0/README.md#cookiesecure
+       */
+      secure: 'auto',
     }
   }));