Merge pull request #1054 from chrisradek/fix/signatureCache

Fixes issue where 2 clients from the same service would share a signature cache.

Author: chrisradek

lib/event_listeners.js

@@ -136,20 +136,22 @@ AWS.EventListeners = {
     });
 
     addAsync('SIGN', 'sign', function SIGN(req, done) {
-      if (!req.service.api.signatureVersion) return done(); // none
+      var service = req.service;
+      if (!service.api.signatureVersion) return done(); // none
 
-      req.service.config.getCredentials(function (err, credentials) {
+      service.config.getCredentials(function (err, credentials) {
         if (err) {
           req.response.error = err;
           return done();
         }
 
         try {
           var date = AWS.util.date.getDate();
-          var SignerClass = req.service.getSignerClass(req);
+          var SignerClass = service.getSignerClass(req);
           var signer = new SignerClass(req.httpRequest,
-            req.service.api.signingName || req.service.api.endpointPrefix,
-            req.service.config.signatureCache);
+            service.api.signingName || service.api.endpointPrefix,
+           service.config.signatureCache);
+          signer.setServiceClientId(service._clientId);
 
           // clear old authorization headers
           delete req.httpRequest.headers['Authorization'];

lib/service.js

@@ -2,6 +2,7 @@ var AWS = require('./core');
 var Api = require('./model/api');
 var regionConfig = require('./region_config');
 var inherit = AWS.util.inherit;
+var clientCount = 0;
 
 /**
  * The service class representing an AWS service.
@@ -32,6 +33,7 @@ AWS.Service = inherit({
         enumerable: false,
         configurable: true
       });
+      svc._clientId = ++clientCount;
       return svc;
     }
     this.initialize(config);

lib/signers/request_signer.js

@@ -7,6 +7,14 @@ var inherit = AWS.util.inherit;
 AWS.Signers.RequestSigner = inherit({
   constructor: function RequestSigner(request) {
     this.request = request;
+  },
+
+  setServiceClientId: function setServiceClientId(id) {
+    this.serviceClientId = id;
+  },
+
+  getServiceClientId: function getServiceClientId() {
+    return this.serviceClientId;
   }
 });
 

lib/signers/v4.js

@@ -6,6 +6,16 @@ var inherit = AWS.util.inherit;
  */
 var cachedSecret = {};
 
+/**
+ * @api private
+ */
+var cacheQueue = [];
+
+/**
+ * @api private
+ */
+var maxCacheEntries = 50;
+
 /**
  * @api private
  */
@@ -97,8 +107,18 @@ AWS.Signers.V4 = inherit(AWS.Signers.RequestSigner, {
 
   signature: function signature(credentials, datetime) {
     var cache = null;
+    var cacheIdentifier = this.serviceName + (this.getServiceClientId() ? '_' + this.getServiceClientId() : '');
     if (this.signatureCache) {
-      var cache = cachedSecret[this.serviceName];
+      var cache = cachedSecret[cacheIdentifier];
+      // If there isn't already a cache entry, we'll be adding one
+      if (!cache) {
+        cacheQueue.push(cacheIdentifier);
+        if (cacheQueue.length > maxCacheEntries) {
+          // remove the oldest entry (may not be last one used)
+          delete cachedSecret[cacheQueue.shift()];
+        }
+      }
+
     }
     var date = datetime.substr(0, 8);
 
@@ -117,13 +137,13 @@ AWS.Signers.V4 = inherit(AWS.Signers.RequestSigner, {
         return AWS.util.crypto.hmac(kCredentials, this.stringToSign(datetime), 'hex');
       }
 
-      cachedSecret[this.serviceName] = {
+      cachedSecret[cacheIdentifier] = {
         region: this.request.region, date: date,
         key: kCredentials, akid: credentials.accessKeyId
       };
     }
 
-    var key = cachedSecret[this.serviceName].key;
+    var key = cachedSecret[cacheIdentifier].key;
     return AWS.util.crypto.hmac(key, this.stringToSign(datetime), 'hex');
   },
 

test/signers/v4.spec.coffee

@@ -12,7 +12,17 @@ buildRequest = ->
   req.httpRequest
 
 buildSigner = (request, signatureCache) ->
-  return new AWS.Signers.V4(request || buildRequest(), 'dynamodb', signatureCache || true)
+  if typeof signatureCache != 'boolean'
+    signatureCache = true
+  return new AWS.Signers.V4(request || buildRequest(), 'dynamodb', signatureCache)
+
+buildSignerFromService = (signatureCache) ->
+  if typeof signatureCache != 'boolean'
+    signatureCache = true
+  ddb = new AWS.DynamoDB({region: 'region', endpoint: 'localhost', apiVersion: '2011-12-05'})
+  signer = buildSigner(null, signatureCache)
+  signer.setServiceClientId(ddb._clientId)
+  return signer
 
 describe 'AWS.Signers.V4', ->
   date = new Date(1935346573456)
@@ -34,7 +44,6 @@ describe 'AWS.Signers.V4', ->
       req = buildRequest()
       signer = buildSigner(req)
       expect(signer.request).to.equal(req)
-
   describe 'addAuthorization', ->
     headers = {
       'Content-Type': 'application/x-amz-json-1.0',
@@ -75,6 +84,26 @@ describe 'AWS.Signers.V4', ->
         calls = AWS.util.crypto.hmac.calls
         callCount = calls.length
 
+      it 'will cache a maximum of 50 clients', (done) ->
+        maxCacheEntries = 50
+        clientSigners = (buildSignerFromService() for i in [0..maxCacheEntries-1])
+        callCount = calls.length
+        #Get signature for all clients to store them in cache
+        (clientSigners[i].signature(creds, datetime) for i in [0..clientSigners.length-1])
+        expect(calls.length).to.equal(callCount + (5 * maxCacheEntries))
+        #Signer should use cache
+        callCount = calls.length
+        clientSigners[0].signature(creds, datetime)
+        expect(calls.length).to.equal(callCount + 1)
+        #add a new signer, pushing past cache limit
+        newestSigner = buildSignerFromService()
+        #old signer shouldn't be using cache anymore
+        callCount = calls.length
+        clientSigners[0].signature(creds, datetime)
+        expect(calls.length).to.equal(callCount + 5)
+        done()
+        
+      #Calling signer.signature should call hmac 1 time when caching, and 5 times when not caching
       it 'caches subsequent requests', ->
         signer.signature(creds, datetime)
         expect(calls.length).to.equal(callCount + 1)
@@ -108,6 +137,24 @@ describe 'AWS.Signers.V4', ->
         signer.signature(creds, newDatetime)
         expect(calls.length).to.equal(callCount + 5)
 
+      it 'uses a different cache if client is different', ->
+        signer1 = buildSignerFromService()
+        callCount = calls.length
+        signer1.signature(creds, datetime)
+        expect(calls.length).to.equal(callCount + 5)
+        signer2 = buildSignerFromService()
+        callCount = calls.length
+        signer2.signature(creds, datetime)
+        expect(calls.length).to.equal(callCount + 5)
+
+      it 'works when using the same client', ->
+        signer1 = buildSignerFromService()
+        callCount = calls.length
+        signer1.signature(creds, datetime)
+        expect(calls.length).to.equal(callCount + 5)
+        signer1.signature(creds, datetime)
+        expect(calls.length).to.equal(callCount + 6)
+
   describe 'stringToSign', ->
     it 'should sign correctly generated input string', ->
       expect(signer.stringToSign(datetime)).to.equal 'AWS4-HMAC-SHA256\n' +