Merge pull request #1044 from Jigsaw-Code/bemasc-routing-test

Add a pre-routing filter

src/shadowbox/server/manager_service.spec.ts

@@ -798,7 +798,7 @@ describe('bindService', () => {
     `${PREFIX}/does-not-exist`,
   ].forEach(path => {
     it(`404 (${path})`, async () => {
-      // Ensure no methods are called.
+      // Ensure no methods are called on the Service.
       spyOnAllFunctions(service);
       jasmine.setDefaultSpyStrategy(fail);
       bindService(server, PREFIX, service);
@@ -807,12 +807,64 @@ describe('bindService', () => {
       const response = await fetch(url);
       const body = await response.json();
 
+      expect(response.status).toEqual(404);
       expect(body).toEqual({
         code: 'ResourceNotFound',
         message: `${path} does not exist`
       });
     });
   });
+
+  // This is primarily a reverse testcase for the unauthorized case.
+  it(`standard routing for authorized queries`, async () => {
+    bindService(server, PREFIX, service);
+    // Verify that ordinary routing goes through the Router.
+    spyOn(server.router, "lookup").and.callThrough();
+
+    // This is an authorized request, so it will pass the prefix filter
+    // and reach the Router.
+    url.pathname = `${PREFIX}`;
+    const response = await fetch(url);
+    expect(response.status).toEqual(404);
+    await response.json();
+
+    expect(server.router.lookup).toHaveBeenCalled();
+  });
+
+  // Check that unauthorized queries are rejected without ever reaching
+  // the routing stage.
+  [
+    '/',
+    '/T',
+    '/TestApiPre',
+    '/TestApi123456',
+    '/TestApi123456789',
+  ].forEach(path => {
+    it(`no routing for unauthorized queries (${path})`, async () => {
+      bindService(server, PREFIX, service);
+      // Ensure no methods are called on the Router.
+      spyOnAllFunctions(server.router);
+      jasmine.setDefaultSpyStrategy(fail);
+
+      // Try bare pathname.
+      url.pathname = path;
+      const response1 = await fetch(url);
+      expect(response1.status).toEqual(404);
+      await response1.json();
+
+      // Try a subpath that would exist if this were a valid prefix
+      url.pathname = `${path}/server`;
+      const response2 = await fetch(url);
+      expect(response2.status).toEqual(404);
+      await response2.json();
+
+      // Try an arbitrary subpath
+      url.pathname = `${path}/does-not-exist`;
+      const response3 = await fetch(url);
+      expect(response3.status).toEqual(404);
+      await response3.json();
+    });
+  });
 });
 
 class ShadowsocksManagerServiceBuilder {

src/shadowbox/server/manager_service.ts

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import * as crypto from 'crypto';
 import * as ipRegex from 'ip-regex';
 import * as restify from 'restify';
 import * as restifyErrors from 'restify-errors';
@@ -74,8 +75,34 @@ enum HttpSuccess {
   NO_CONTENT = 204,
 }
 
+// Similar to String.startsWith(), but constant-time.
+function timingSafeStartsWith(input: string, prefix: string): boolean {
+  const prefixBuf = Buffer.from(prefix);
+  const inputBuf = Buffer.from(input);
+  const L = Math.min(inputBuf.length, prefixBuf.length);
+  const inputOverlap = inputBuf.slice(0, L);
+  const prefixOverlap = prefixBuf.slice(0, L);
+  const match = crypto.timingSafeEqual(inputOverlap, prefixOverlap);
+  return inputBuf.length >= prefixBuf.length && match;
+}
+
+// Returns a pre-routing hook that injects a 404 if the request does not
+// start with `apiPrefix`.  This filter runs in constant time.
+function prefixFilter(apiPrefix: string): restify.RequestHandler {
+  return (req: restify.Request, res: restify.Response, next: restify.Next) => {
+    if (timingSafeStartsWith(req.path(), apiPrefix)) {
+      return next();
+    }
+    // This error matches the router's default 404 response.
+    next(new restifyErrors.ResourceNotFoundError('%s does not exist', req.path()));
+  };
+}
+
 export function bindService(
     apiServer: restify.Server, apiPrefix: string, service: ShadowsocksManagerService) {
+  // Reject unauthorized requests in constant time before they reach the routing step.
+  apiServer.pre(prefixFilter(apiPrefix));
+
   apiServer.put(`${apiPrefix}/name`, service.renameServer.bind(service));
   apiServer.get(`${apiPrefix}/server`, service.getServer.bind(service));
   apiServer.put(