Add AWS session token as optional parameter for S3

[jossoco]

If using temporary security credentials, for example when on an EC2 instance with IAM role policies setting S3 access, requests need to include the session token.

[gsuess]

Are you sure that the client can be trusted with the session token?

[jossoco]

I'm not sure I understand your question exactly; this code itself runs on the server, correct? Do you mean is it safe to add the token to the request? As far as I know it's standard procedure for calls made with temporary security credentials.

[gsuess]

Yes it is running on the server, but you are effectively sending the token to the client, because the client uploads it directly to S3.

[gsuess]

As session tokens are by definition temporary, I am thinking we should not support constant Strings, but functions only, so that people will not be walking into a trap here. Also expiry time does matter. Slingshot could provide the desired expiry time as an argument.

[jossoco]

Cool; makes sense to me, updated.

[jossoco]

Are there any other issues that need to be addressed?

[gsuess]

Sorry for the delay.

It looks good to me. But I am wondering about the practicality of this. As far as I understand you get a temporary AWSAcceesKeyId, AWSSecretAccessKey and the AWSSessionToken from one single functional call, but you have to distribute it to three different callback functions, which must be quite cumbersome.

So I am looking for easier way...

Would you like to share your code where you use EC2MetadataCredentials? I am curious how you have approached it.

[jossoco]

I see your concern; I suppose a different approach would be passing a function to the directive which returns an object containing each of the three credentials. Let me know if you think that is preferable.

Here is essentially the code I am using:

var Future = Npm.require('fibers/future');

var fetchTemporaryCredentials = function () {
  var future = new Future();
  AWS.config.credentials = new AWS.EC2MetadataCredentials();
  AWS.config.credentials.get(function (err) {
    if (err != null) {
      future.throw(err);
    } else {
      future.return();
    }
  });
  return future.wait();
};

var getTemporaryCredential = function (key) {
  if (AWS.config.credentials == null || AWS.config.credentials.needsRefresh()) {
    fetchTemporaryCredentials();
  }
  return AWS.config.credentials[key];
};

var getAccessKeyId = function () {
  return getTemporaryCredential('accessKeyId');
};

var getSecretAccessKey = function () {
  return getTemporaryCredential('secretAccessKey');
};

var getSessionToken = function () {
  return getTemporaryCredential('sessionToken');
};

Slingshot.createDirective('myUploads', Slingshot.S3Storage, {
  bucket: 'myBucket',
  AWSAccessKeyId: getAccessKeyId,
  AWSSecretAccessKey: getSecretAccessKey,
  AWSSessionToken: getSessionToken
});

[gsuess]

Thank you. That is a reasonable approach.

I finally found a way how to test temporary keys without an EC2 instance.

I agree to your suggested solution, however slingshot will not take that in easily as it will insist on getting the keys when the directive is instantiated.

I think that can be circumvented by extending Slingshot.S3Storage into a spearate version, say Slingshot.S3Storage.TempCredentials which will not accept the keys at all, but take a single function instead.

So basically something like this:

var getSessionToken = Meteor.wrapAsync(function (params, callback) {
    sts.getSessionToken(params, callback);
});

Slingshot.createDirective('myUploads', Slingshot.S3Storage.TempCredentials, {
  bucket: 'myBucket',
  sessionToken: function (expiry) {
         return sts.getSessionToken({
            DurationSeconds: Math.round((expiry - Date.now()) / 1000 )
         });
  }
});

[jossoco]

I'm not sure I understand this. Where is the code gets the keys when the directive is instantiated? And why is that an issue? This code has been working correctly for me.

[gsuess]

I was not referring to your code, as far as I can tell should work fine. I was referring to your other suggested approach:

I see your concern; I suppose a different approach would be passing a function to the directive which returns an object containing each of the three credentials. Let me know if you think that is preferable.

(I think that would be preferable and I am working on it right now)

My snippet shows how it will be used with AWS.STS::getSessionToken, which is similar to AWS.EC2MetadataCredentials, but works when not running on an EC2 instance.

I will push the actual implementation of that API soon.

[jossoco]

Ok, I understand. What I was imagining was simply an optional param, a function that returns all of the temporary credentials. So its use would be something this with EC2MetadataCredentials (and based on the documentation for STS, I think something similar could be done with that):

Slingshot.createDirective('myUploads', Slingshot.S3Storage, {
  bucket: 'myBucket',
  temporaryCredentials: function () {
    if (AWS.config.credentials == null || AWS.config.credentials.needsRefresh()) {
      fetchTemporaryCredentials();
    }
    return {
      AWSAccessKeyId: AWS.config.credentials.accessKeyId,
      AWSSecretAccessKey: AWS.config.credentials.secretAccessKey,
      AWSSessionToken: AWS.config.credentials.sessionToken
    };
  }
});

I'm confused as to why a separate directive type would be needed.

[gsuess]

The problem is that Slingshot will insist that you have AWSAccessKeyId and AWSSecretAccessKey set when you instantiate the directive, while there is no reason for you have those set when you deal with temporary keys.

So the extension removes that requirement and replaces it with the requirement to have a function that retrieves the temporary keys whenever they are actually needed.

I've got that working now (tested with sts.getSessionToken ) in this branch. Can you please test it with EC2MetadataCredentials while I update the readme? The directive property name for now is sessionCredentials, though I like the name temporaryCredentials from your snippet better :)

[jossoco]

Ah, yes I understand now. This is working fine for me with EC2MetadataCredentials :)

[gsuess]

Great thank you, I would like to add your snippet to the readme. I think your approach would be very handy to those running on EC2. Can you please check if this refactored version work for you:

var credentials = new AWS.EC2MetadataCredentials();

var updateCredentials = Meteor.wrapAsync(credentials.get, credentials);

Slingshot.createDirective('myUploads', Slingshot.S3Storage.TempCredentials, {
  bucket: 'myBucket',
  temporaryCredentials: function () {
    if (credentials.needsRefresh()) {
      updateCredentials();
    }

    return {
      AccessKeyId: credentials.accessKeyId,
      SecretAccessKey: credentials.secretAccessKey,
      SessionToken: credentials.sessionToken
    };
  }
});

[jossoco]

Yep, that code works fine. Thanks for getting this change in!