From 0142dcc2eb76db75e4121ceb81763cbacca9d6f2 Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Sun, 5 Apr 2020 08:11:57 -0700 Subject: [PATCH] fix(jwt token secret): Better handling when token secret is misconfigured --- docker/mainnet/start-local-mainnet.sh | 5 ++- docker/testnet/start-local-testnet.sh | 5 ++- src/middleware/route-ratelimit.js | 53 +++++++++++++++++---------- test/v3/rate-limits.js | 20 ++++++++++ 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/docker/mainnet/start-local-mainnet.sh b/docker/mainnet/start-local-mainnet.sh index 7b98420..97d08ad 100755 --- a/docker/mainnet/start-local-mainnet.sh +++ b/docker/mainnet/start-local-mainnet.sh @@ -17,10 +17,11 @@ export BLOCKBOOK_URL=https://172.17.0.1:9131/ # Allow node.js to make network calls to https using self-signed certificate. export NODE_TLS_REJECT_UNAUTHORIZED=0 -export JWT_AUTH_SERVER=http://172.17.0.1:5001/ - # Redis DB export REDIS_PORT=6379 export REDIS_HOST=172.17.0.1 +# JWT Token Secret +export TOKENSECRET=somelongsecretvalue + npm start diff --git a/docker/testnet/start-local-testnet.sh b/docker/testnet/start-local-testnet.sh index 7fadb90..f51b15d 100755 --- a/docker/testnet/start-local-testnet.sh +++ b/docker/testnet/start-local-testnet.sh @@ -14,10 +14,11 @@ export BLOCKBOOK_URL=https://172.17.0.1:19131/ # Allow node.js to make network calls to https using self-signed certificate. export NODE_TLS_REJECT_UNAUTHORIZED=0 -export JWT_AUTH_SERVER=http://172.17.0.1:5001/ - # Redis DB export REDIS_PORT=6380 export REDIS_HOST=172.17.0.1 +# JWT Token Secret +export TOKENSECRET=somelongsecretvalue + npm start diff --git a/src/middleware/route-ratelimit.js b/src/middleware/route-ratelimit.js index 4037360..1fbdd0e 100644 --- a/src/middleware/route-ratelimit.js +++ b/src/middleware/route-ratelimit.js @@ -32,6 +32,7 @@ class RateLimits { this.jwt = jwt this.rateLimiter = new RateLimiterRedis(rateLimitOptions) + this.config = config } // Used to disconnect from the Redis DB. @@ -52,7 +53,7 @@ class RateLimits { let userId let decoded = {} - // Create a res.locals object if not passed in. + // Create a req.locals object if not passed in. if (!req.locals) { req.locals = { // default values @@ -62,26 +63,34 @@ class RateLimits { } } + // Create a res.locals object if it does not exist. This is used for + // debugging. + if (!res.locals) { + res.locals = { + rateLimitTriggered: false + } + } + // Decode the JWT token if one exists. if (req.locals.jwtToken) { - // const jwtOptions = { - // algorithms: ['ES256'] - // } + try { + decoded = _this.jwt.verify(req.locals.jwtToken, _this.config.apiTokenSecret) + // console.log(`decoded: ${JSON.stringify(decoded, null, 2)}`) - // const pemPublicKey = keyEncoder.encodePublic(publicKey, 'raw', 'pem') - - // Validate the JWT token. - // decoded = _this.jwt.verify( - // req.locals.jwtToken, - // pemPublicKey, - // jwtOptions - // ) - - wlogger.info(`Last three letters of token secret: ${config.apiTokenSecret.slice(-3)}`) - decoded = _this.jwt.verify(req.locals.jwtToken, config.apiTokenSecret) - // console.log(`decoded: ${JSON.stringify(decoded, null, 2)}`) - - userId = decoded.id + userId = decoded.id + } catch (err) { + // This handler will be triggered if the JWT token does not match the + // token secret. + wlogger.error( + `Last three letters of token secret: ${_this.config.apiTokenSecret.slice( + -3 + )}` + ) + wlogger.error( + 'Error trying to decode JWT token in route-ratelimit.js/newRateLimit(): ', + err + ) + } } else { wlogger.debug('No JWT token found!') } @@ -97,10 +106,12 @@ class RateLimits { wlogger.debug(`resource: ${resource}`) let key = userId || req.ip + res.locals.key = key // Feedback for tests. // const pointsToConsume = userId ? 1 : 30 decoded.resource = resource const pointsToConsume = _this.calcPoints(decoded) + res.locals.pointsToConsume = pointsToConsume // Feedback for tests. wlogger.info( `User ${key} consuming ${pointsToConsume} point for resource ${resource}.` @@ -113,7 +124,11 @@ class RateLimits { await _this.rateLimiter.consume(key, pointsToConsume) } catch (err) { - // console.log(`err: `, err) + // console.log('err: ', err) + + // Used for returning data for tests. + res.locals.rateLimitTriggered = true + // console.log('res.locals: ', res.locals) // Rate limited was triggered res.status(429) // https://github.com/Bitcoin-com/rest.bitcoin.com/issues/330 diff --git a/test/v3/rate-limits.js b/test/v3/rate-limits.js index 910dc65..02a1530 100644 --- a/test/v3/rate-limits.js +++ b/test/v3/rate-limits.js @@ -424,6 +424,26 @@ describe('#route-ratelimits & jwt-auth', () => { 'next should NOT be called if rate limit was triggered.' ) }) + + it('should handle misconfigured token secret', async () => { + // Create a new instance of the rate limit so we start with zeroed tracking. + rateLimits = new RateLimits() + + req.baseUrl = '/v3' + req.path = '/control/getNetworkInfo' + req.url = req.path + req.method = 'GET' + + req.locals.jwtToken = 'some-token' + + next.reset() // reset the stubbed next() function. + + await rateLimits.rateLimitByResource(req, res, next) + + // Issues with token secret should treat incoming requests as anonymous + // calls with 30 points or 3 RPM. + assert.equal(res.locals.pointsToConsume, 30) + }) }) })