diff --git a/src/shadowbox/server/manager_service.spec.ts b/src/shadowbox/server/manager_service.spec.ts index 363594a0..c65f28dd 100644 --- a/src/shadowbox/server/manager_service.spec.ts +++ b/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 { diff --git a/src/shadowbox/server/manager_service.ts b/src/shadowbox/server/manager_service.ts index 7ea57c7f..d4791af6 100644 --- a/src/shadowbox/server/manager_service.ts +++ b/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 { + if (input.length < prefix.length) { + return false; + } + const prefixBuf = Buffer.from(prefix); + const inputBuf = Buffer.from(input); + const overlap = inputBuf.slice(0, prefixBuf.length); + return crypto.timingSafeEqual(overlap, prefixBuf); +} + +// 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(