security: Fix authentication bypass vulnerability

Before, anyone who could create a socket.io connection to Etherpad
could read, modify, and create pads at will without authenticating
first.

The `checkAccess` middleware in `webaccess.js` normally handles
authentication and authorization, but it does not run for `/socket.io`
requests. This means that the connection handler in `socketio.js` must
handle authentication and authorization. However, before this change:
  * The handler did not require a signed `express_sid` cookie.
  * After loading the express-session state, the handler did not check
    to see if the user had authenticated.

Now the handler requires a signed `express_sid` cookie, and it ensures
that `socket.request.session.user` is non-null if authentication is
required. (`socket.request.session.user` is non-null if and only if
the user has authenticated.)

src/node/hooks/express/socketio.js

@@ -39,43 +39,36 @@ exports.expressCreateServer = function (hook_name, args, cb) {
     cookie: false,
   });
 
-  /* Require an express session cookie to be present, and load the
+  // REQUIRE a signed express-session cookie to be present, then load the session. See
-   * session. See http://www.danielbaulig.de/socket-ioexpress for more
+  // http://www.danielbaulig.de/socket-ioexpress for more info. After the session is loaded, ensure
-   * info */
+  // that the user has authenticated (if authentication is required).
+  //
+  // !!!WARNING!!! Requests to /socket.io are NOT subject to the checkAccess middleware in
+  // webaccess.js. If this handler fails to check for a signed express-session cookie or fails to
+  // check whether the user has authenticated, then any random person on the Internet can read,
+  // modify, or create any pad (unless the pad is password protected or an HTTP API session is
+  // required).
   var cookieParserFn = cookieParser(webaccess.secret, {});
-
   io.use(function(socket, accept) {
     var data = socket.request;
-    // Use a setting if we want to allow load Testing
+    if (!data.headers.cookie && settings.loadTest) return accept(null, true);
-
+    cookieParserFn(data, {}, function(err) {
-    // Sometimes browsers might not have cookies at all, for example Safari in iFrames Cross domain
+      if (err) {
-    // https://github.com/ether/etherpad-lite/issues/4031
-    // if requireSession is false we can allow them to still get on the pad.
-    // Note that this does make security less tight because any socketIO connection can be established without
-    // any logic on the client to do any handshaking..  I am not concerned about this though, the real solution
-    // here is to implement rateLimiting on SocketIO ACCEPT_COMMIT messages.
-
-    if(!data.headers.cookie && (settings.loadTest || !settings.requireSession)){
-      accept(null, true);
-    }else{
-      if (!data.headers.cookie) return accept('No session cookie transmitted.', false);
-    }
-    if(data.headers.cookie){
-      cookieParserFn(data, {}, function(err){
-        if(err) {
         console.error(err);
-          accept("Couldn't parse request cookies. ", false);
+        accept("Couldn't parse request cookies.", false);
         return;
       }
-
       data.sessionID = data.signedCookies.express_sid;
-        args.app.sessionStore.get(data.sessionID, function (err, session) {
+      if (!data.sessionID) return accept('Signed express_sid cookie is required', false);
+      args.app.sessionStore.get(data.sessionID, function(err, session) {
         if (err || !session) return accept('Bad session / session has expired', false);
         data.session = new sessionModule.Session(data, session);
+        if (settings.requireAuthentication && data.session.user == null) {
+          return accept('Authentication required', false);
+        }
         accept(null, true);
       });
     });
-    }
   });
 
   // var socketIOLogger = log4js.getLogger("socket.io");