Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send in x-amz-security-token header when configured (for use with IAM roles) #70

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

lloydcotten
Copy link

First of all I apologize for the commit spam.

I added s3.session_token key to the copy_entries array in main.c. Also added a condition to check for the AWS_SESSION_TOKEN env variable and assign to the config when set.

In http_connection.c I added a condition in the http_connection_make_request method to check to see if the s3.session_token has been set and add the x-amz-security-token header to the request.

I chose AWS_SESSION_TOKEN as the env name as it makes it compatible with the AWS CLI. I referred to it as "session token" throughout the code and config for consistency.

@wizzard
Copy link
Member

wizzard commented Sep 25, 2014

The code looks ok to me.

@lloydcotten
Copy link
Author

There's a bit of a flaw with this design, which I forgot to take into account. The token expires and right now one must kill riosfs, and restart to get an updated token from environment variables or set the token in the XML config and send in USR1 signal to refresh config.

Would you be opposed to scratching this method and instead doing it like s3fs-fuse does it? That is, add a new option -o iam_role=[role] and when set retrieve the credentials directly from 169.254.169.254? It would cache them in memory and check to see if they are valid on every connection and refresh only when expired.

What's your thoughts?

@henningpeters
Copy link
Contributor

I think this is indeed the only way to go. Continuously restarting the server on auth failures is not an option. Looks like a much bigger change then, though. I think if you propose a working solution we are very happy to merge it into master.

@lloydcotten
Copy link
Author

Possibly - I'll see what I can do. I'm not super familiar with glib2 and libevent, but I'll give it a go. The response is sent back in JSON - right now I'm thinking of just parsing it with gregex from glib2 so as not to add a dependency, unless you can suggest a better way.

@henningpeters
Copy link
Contributor

Don't be afraid of doing the right thing. I just took a look at how s3fs does it and I don't like it. They parse the JSON via some 20 lines of ugly string parsing. Take a minimalistic JSON parser, such as jsmn (http://zserge.bitbucket.org/jsmn.html).

Another thing I don't like about how s3fs does it is that the role request is within the normal request path. It should be triggered by timers asynchronously to normal operation. Libevent to your help, you can set timers and it includes a simple HTTP client. According to AWS docs:

These security credentials are temporary and we rotate them automatically. We make new credentials available at least five minutes prior to the expiration of the old credentials.

IAM roles are a neat feature and obviously it has some good use cases, hence if we decided to support it (which we did) then we should do it right and not half-heartedly. We are here to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants