Change hosts format, remove override method opt, prevent timing attacks

Hosts are now simply `host` instead of `proto:host`
sentanos · Apr 20, 2018 · 66df22e · 66df22e
1 parent 2147071
commit 66df22e
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 30 deletions.
diff --git a/app.json b/app.json
@@ -16,7 +16,7 @@
     },
     "ALLOWED_HOSTS": {
       "description": "If USE_WHITELIST is true, allow only these hosts for proxy requests. Otherwise, these will be used to find the default protocol to use for the host. If it is not included in here, the protocol used will be https. The default hosts below are all Roblox hosts. This is a comma separated list of additional hosts (no spaces) with each entry in the format protocol:host.",
-      "value": "https:roblox.com,https:www.roblox.com,https:api.roblox.com,http:wiki.roblox.com,https:assetgame.roblox.com,https:avatar.roblox.com,https:groups.roblox.com,https:data.roblox.com,https:inventory.roblox.com,https:search.roblox.com,https:auth.roblox.com,https:avatar.roblox.com,https:billing.roblox.com,https:chat.roblox.com,https:develop.roblox.com,https:notifications.roblox.com,https:games.roblox.com"
+      "value": "roblox.com,www.roblox.com,api.roblox.com,wiki.roblox.com,assetgame.roblox.com,avatar.roblox.com,groups.roblox.com,data.roblox.com,inventory.roblox.com,search.roblox.com,auth.roblox.com,avatar.roblox.com,billing.roblox.com,chat.roblox.com,develop.roblox.com,notifications.roblox.com,games.roblox.com"
     },
     "USE_OVERRIDE_STATUS": {
       "description": "If true, requests are always returned with status code 200 but the data contains the true status code of the proxy response.",
@@ -28,11 +28,6 @@
       "required": true,
       "value": true
     },
-    "ALLOW_OVERRIDE_METHOD": {
-      "description": "Enable this to allow changing proxy methods to a method different than the original method. This allows for PUT, DELETE, PATCH, etc. requests with only GETs or POSTs. You will not need to modify this.",
-      "required": true,
-      "value": true
-    },
     "REWRITE_ACCEPT_ENCODING": {
       "description": "If true, the `Accept-Encoding` header will be set to `gzip` because deflate is not supported for APPEND_HEAD.",
       "value": true,

diff --git a/server.js b/server.js
@@ -2,6 +2,7 @@
 const proxy = require('http-proxy');
 const https = require('https');
 const http = require('http');
+const crypto = require('crypto');
 const assert = require('assert');
 const zlib = require('zlib');
 const { URL } = require('url');
@@ -19,23 +20,13 @@ const getHosts = (hosts) => {
   let parsed = [];
   hosts = hosts.split(',');
   for (let i = 0; i < hosts.length; i++) {
-    const iHost = hosts[i];
-    const split = iHost.split(':');
-    if (split.length !== 2) {
-      throw new Error(`Configuration error! Invalid protocol:host pair on item ${iHost}`);
-    }
-    const proto = split[0];
-    if (!ALLOWED_PROTOS.includes(proto)) {
-      throw new Error(`Configuration error! Invalid protocol ${proto}. Only these protocols are allowed: ${ALLOWED_PROTOS.join(', ')}`);
-    }
-    const host = split[1];
+    const host = hosts[i];
     try {
-      (() => new URL(`${proto}://${host}`))();
+      (() => new URL(`${DEFAULT_PROTO}://${host}`))();
     } catch (e) {
-      throw new Error(`Configuration error! Invalid host domain on item ${iHost}`);
+      throw new Error(`Configuration error! Invalid host domain on item ${host}`);
     }
     parsed.push({
-      proto: proto,
       host: host
     });
   }
@@ -49,7 +40,6 @@ const USE_WHITELIST = process.env.USE_WHITELIST === 'true';
 const USE_OVERRIDE_STATUS = process.env.USE_OVERRIDE_STATUS === 'true';
 const REWRITE_ACCEPT_ENCODING = process.env.REWRITE_ACCEPT_ENCODING === 'true';
 const APPEND_HEAD = process.env.APPEND_HEAD === 'true';
-const ALLOW_OVERRIDE_METHOD = process.env.ALLOW_OVERRIDE_METHOD === 'true';
 const ALLOWED_HOSTS = getHosts(process.env.ALLOWED_HOSTS);
 
 assert.ok(ACCESS_KEY);
@@ -172,15 +162,10 @@ const doProxy = (target, proto, req, res) => {
 server.on('request', (req, res) => {
   const method = req.headers['proxy-target-override-method'];
   if (method) {
-    if (ALLOW_OVERRIDE_METHOD) {
-      if (ALLOWED_METHODS.includes(method)) {
-        req.method = method;
-      } else {
-        writeErr(res, 400, 'Invalid target method');
-        return;
-      }
+    if (ALLOWED_METHODS.includes(method)) {
+      req.method = method;
     } else {
-      writeErr(res, 400, 'Method override is not permitted');
+      writeErr(res, 400, 'Invalid target method');
       return;
     }
   }
@@ -195,7 +180,7 @@ server.on('request', (req, res) => {
     req.on('error', (err) => {
       console.error(`Request error: ${err}`);
     });
-    if (accessKey === ACCESS_KEY) {
+    if (crypto.timingSafeEqual(accessKey, ACCESS_KEY)) {
       let parsedTarget;
       try {
         parsedTarget = new URL(`https://${requestedTarget}`);
@@ -210,7 +195,6 @@ server.on('request', (req, res) => {
         const iHost = ALLOWED_HOSTS[i];
         if (requestedHost === iHost.host) {
           hostAllowed = true;
-          hostProto = iHost.proto;
           break;
         }
       }