security: Support proxy with rate limiting and include CI test coverage for nginx rev proxy (#4373)

Previously Etherpad would not pass the correct client IP address through and this caused the rate limiter to limit users behind reverse proxies.  This change allows Etherpad to use a client IP passed from a reverse proxy.

Note to devs: This header can be spoofed and spoofing the header could be used in an attack.  To mitigate additional *steps should be taken by Etherpad site admins IE doing rate limiting at proxy.*  This only really applies to large scale deployments but it's worth noting.

.travis.yml

@@ -99,6 +99,18 @@ jobs:
         - "npm install -g etherpad-load-test"
       script:
         - "tests/frontend/travis/runnerLoadTest.sh"
+    - name: "Test rate limit"
+      install:
+        - "docker network create --subnet=172.23.42.0/16 ep_net"
+        - "docker build -f Dockerfile -t epl-debian-slim ."
+        - "docker build -f tests/ratelimit/Dockerfile.nginx -t nginx-latest ."
+        - "docker build -f tests/ratelimit/Dockerfile.anotherip -t anotherip ."
+        - "docker run -p 8081:80 --rm --network ep_net --ip 172.23.42.1 -d nginx-latest"
+        - "docker run --name etherpad-docker -p 9000:9001 --rm --network ep_net --ip 172.23.42.2 -e 'TRUST_PROXY=true' epl-debian-slim &"
+        - "docker run --rm --network ep_net --ip 172.23.42.3 --name anotherip -dt anotherip"
+        - "./bin/installDeps.sh"
+      script:
+        - "cd tests/ratelimit && bash testlimits.sh"
 
 notifications:
   irc:

src/node/handler/PadMessageHandler.js

@@ -181,10 +181,11 @@ exports.handleMessage = async function(client, message)
   var env = process.env.NODE_ENV || 'development';
 
   if (env === 'production') {
+    const clientIPAddress = remoteAddress[client.id];
     try {
-      await rateLimiter.consume(client.handshake.address); // consume 1 point per event from IP
+      await rateLimiter.consume(clientIPAddress); // consume 1 point per event from IP
     }catch(e){
-      console.warn("Rate limited: ", client.handshake.address, " to reduce the amount of rate limiting that happens edit the rateLimit values in settings.json");
+      console.warn("Rate limited: ", clientIPAddress, " to reduce the amount of rate limiting that happens edit the rateLimit values in settings.json");
       stats.meter('rateLimited').mark();
       client.json.send({disconnect:"rateLimited"});
       return;

src/package-lock.json

@@ -1810,6 +1810,16 @@
       "resolved": "https://registry.npmjs.org/etag/-/etag-1.8.1.tgz",
       "integrity": "sha1-Qa4u62XvpiJorr/qg6x9eSmbCIc="
     },
+    "etherpad-cli-client": {
+      "version": "0.0.9",
+      "resolved": "https://registry.npmjs.org/etherpad-cli-client/-/etherpad-cli-client-0.0.9.tgz",
+      "integrity": "sha1-A+5+fNzA4EZLTu/djn7gzwUaVDs=",
+      "dev": true,
+      "requires": {
+        "async": "*",
+        "socket.io-client": "*"
+      }
+    },
     "etherpad-require-kernel": {
       "version": "1.0.9",
       "resolved": "https://registry.npmjs.org/etherpad-require-kernel/-/etherpad-require-kernel-1.0.9.tgz",

src/package.json

@@ -81,7 +81,8 @@
     "set-cookie-parser": "^2.4.6",
     "superagent": "^3.8.3",
     "supertest": "4.0.2",
-    "wd": "1.12.1"
+    "wd": "1.12.1",
+    "etherpad-cli-client": "0.0.9"
   },
   "engines": {
     "node": ">=10.13.0",

tests/ratelimit/Dockerfile.anotherip

@@ -0,0 +1,4 @@
+FROM node:alpine3.12
+WORKDIR /tmp
+RUN npm i etherpad-cli-client
+COPY ./tests/ratelimit/send_changesets.js /tmp/send_changesets.js

tests/ratelimit/Dockerfile.nginx

@@ -0,0 +1,2 @@
+FROM nginx
+COPY ./tests/ratelimit/nginx.conf /etc/nginx/nginx.conf

tests/ratelimit/nginx.conf

@@ -0,0 +1,26 @@
+events {}
+http {
+  server {
+        access_log  /dev/fd/1;
+        error_log   /dev/fd/2;
+        location / {
+            proxy_pass             http://172.23.42.2:9001/;
+            proxy_set_header       Host $host;
+            proxy_pass_header Server;
+            # be careful, this line doesn't override any proxy_buffering on set in a conf.d/file.conf
+            proxy_buffering off;
+            proxy_set_header X-Real-IP $remote_addr;  # http://wiki.nginx.org/HttpProxyModule
+            proxy_set_header X-Forwarded-For $remote_addr; # EP logs to show the actual remote IP
+            proxy_set_header X-Forwarded-Proto $scheme; # for EP to set secure cookie flag when https is used
+            proxy_set_header Host $host;  # pass the host header                                                   
+            proxy_http_version 1.1;  # recommended with keepalive connections                                                    
+            # WebSocket proxying - from http://nginx.org/en/docs/http/websocket.html
+            proxy_set_header Upgrade $http_upgrade;
+            proxy_set_header Connection $connection_upgrade;
+        }
+	}
+  map $http_upgrade $connection_upgrade {
+    default upgrade;
+    ''      close;
+  }
+}

tests/ratelimit/send_changesets.js

@@ -0,0 +1,25 @@
+try{
+  var etherpad = require("../../src/node_modules/etherpad-cli-client");
+  //ugly
+} catch {
+  var etherpad = require("etherpad-cli-client")
+}
+var pad = etherpad.connect(process.argv[2]);
+pad.on("connected", function(){
+
+  setTimeout(function(){
+    setInterval(function(){
+      pad.append("1");
+    }, process.argv[3]);
+  },500); // wait because CLIENT_READY message is included in ratelimit
+
+  setTimeout(function(){
+    process.exit(0);
+  },11000)
+});
+// in case of disconnect exit code 1
+pad.on("message", function(message){
+  if(message.disconnect == 'rateLimited'){
+    process.exit(1);
+  }
+})

tests/ratelimit/testlimits.sh

@@ -0,0 +1,36 @@
+#!/usr/bin/env bash
+
+#sending changesets every 101ms should not trigger ratelimit
+node send_changesets.js http://127.0.0.1:8081/p/BACKEND_TEST_ratelimit_101ms 101
+if [[ $? -ne 0 ]];then
+  echo "FAILED: ratelimit was triggered when sending every 101 ms"
+  exit 1
+fi
+
+#sending changesets every 99ms should trigger ratelimit
+node send_changesets.js http://127.0.0.1:8081/p/BACKEND_TEST_ratelimit_99ms 99
+if [[ $? -ne 1 ]];then
+  echo "FAILED: ratelimit was not triggered when sending every 99 ms"
+  exit 1
+fi
+
+#sending changesets every 101ms via proxy
+node send_changesets.js http://127.0.0.1:8081/p/BACKEND_TEST_ratelimit_101ms 101 &
+pid1=$!
+
+#sending changesets every 101ms via second IP and proxy
+docker exec anotherip node /tmp/send_changesets.js http://172.23.42.1:80/p/BACKEND_TEST_ratelimit_101ms_via_second_ip 101 &
+pid2=$!
+
+wait $pid1
+exit1=$?
+wait $pid2
+exit2=$?
+
+echo "101ms with proxy returned with ${exit1}"
+echo "101ms via another ip returned with ${exit2}"
+
+if [[ $exit1 -eq 1 || $exit2 -eq 1 ]];then
+  echo "FAILED: ratelimit was triggered during proxy and requests via second ip"
+  exit 1
+fi