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

s3.upload does not re-use IRSA temporary credentials #3481

Closed
3 tasks done
jacknagel opened this issue Oct 6, 2020 · 11 comments · Fixed by #3700
Closed
3 tasks done

s3.upload does not re-use IRSA temporary credentials #3481

jacknagel opened this issue Oct 6, 2020 · 11 comments · Fixed by #3700
Assignees
Labels
bug This issue is a bug.

Comments

@jacknagel
Copy link

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug
When using IAM roles for service accounts in EKS, new credentials are requested from STS on each call to s3.upload.

We noticed a service making a large number of AssumeRoleWithWebIdentity requests to STS. After investigating, it was determined that the returned credentials do not appear to be re-used across multiple calls to s3.upload.

Is the issue in the browser/Node.js?
Node.js

If on Node.js, are you running this on AWS Lambda?
No

Details of the browser/Node.js version
v12.18.0

SDK version number
v2.767.0

To Reproduce (observed behavior)

This code demonstrates the issue (you must be using IRSA, of course).

const aws = require('aws-sdk')
const { randomBytes } = require('crypto')

aws.config.update({
  logger: console
})

const s3 = new aws.S3({
  apiVersion: '2006-03-01',
  params: {
    Bucket: '<bucket name>'
  }
});

(async function() {
  // Fetchs credentials once, reuses them on each subsequent API call
  for (let i = 0; i < 5; i++) {
    const key = randomBytes(32).toString('hex')
    await s3.putObject({ Key: key, Body: Buffer.from('test') }).promise()
  }

  // Fetches new credentials each time s3.upload is invoked
  for (let i = 0; i < 5; i++) {
    const key = randomBytes(32).toString('hex')
    await s3.upload({ Key: key, Body: Buffer.from('test') }).promise()
  }
})()

Expected behavior
Credentials are cached and re-used across all API calls until they expire.

Additional context
Our service performs additional S3 operations (getObject, deleteObject) and the temporary credentials appear to be re-used appropriately.

@jacknagel jacknagel added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 6, 2020
@mbelang
Copy link

mbelang commented Oct 9, 2020

according to https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts-minimum-sdk.html, it should work starting at 2.521.0 🤔

I did experience the same thing though.

I'm on 2.658.0.

@mbelang
Copy link

mbelang commented Oct 9, 2020

there might be an error in the doc as https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md#25230 seems to be where it was released...

@jacknagel
Copy link
Author

To be clear, this issue is not about IRSA support in general - we use it extensively with this SDK and it works fine. This is specifically about s3.upload not-reusing cached IRSA credentials, which all other functions seem to do seamlessly.

@mbelang
Copy link

mbelang commented Oct 26, 2020

@jacknagel thx for clarifying. I suppose that, in my case, the problem comes from the fact that https://github.com/outline/outline/blob/2c1a111dee9dc55134bd5853f8e10e4017bb694e/server/utils/s3.js#L17 is passing the credential to the SDK instead of letting the SDK trying to find the right credentials? Is that right? I'm kinda new to node SDK but I do think python SDK works like that.

@jacknagel
Copy link
Author

I don't know, but continuing to troubleshoot your problem in this issue is just confusing things with the bug I've reported. Open your own ticket, please.

@ajredniwja ajredniwja added the investigating Issue has been looked at and needs deep dive work by OSDS. label Jan 12, 2021
@ajredniwja
Copy link
Contributor

Hey @jacknagel thanks for opening this issue and apologies for delayed response, I am not quite able to reproduce the issue using the code example as both the calls use putObject(), am I missing anything there? Also how do you determine an additional creds call?

@ajredniwja ajredniwja added the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jan 12, 2021
@jacknagel
Copy link
Author

@ajredniwja

I am not quite able to reproduce the issue using the code example as both the calls use putObject(), am I missing anything there?

In the provided example, upload calls putObject directly due to the small file size, but that doesn't change the outcome.

Also how do you determine an additional creds call?

I have the SDK configured to log to the console, and the STS API calls are logged.

As I mentioned, this occurs when using token file credentials (i.e., the underlying API call is assumeRoleWithWebIdentity). We don't use any other type of credentials for this application, so I haven't tested them.

When I run the example code in our EKS clusters, I see the following (secrets redacted, of course).

For the first loop (calling putObject), there is a single assumeRoleWithWebIdentity request, and then five putObject requests. For the second loop (calling upload), there is a separate assumeRoleWithWebIdentity request made before each of the underlying putObject calls.

[AWS sts 200 0.093s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.171s 0 retries] putObject({
  Key: '528950803be8fb0d2d0c694ac22eac062cb72557e0fcf94ef5b750e479a9f900',
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>'
})
[AWS s3 200 0.081s 0 retries] putObject({
  Key: 'ab728fbc3ebf55100a6efdc51adbd31ccf97fe723ba67c191ac1c4821af1e5e3',
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>'
})
[AWS s3 200 0.067s 0 retries] putObject({
  Key: '47c995e85cf1f23e53b432f8165eda196e68d88bc79af8ac7c4e405ee321a97e',
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>'
})
[AWS s3 200 0.049s 0 retries] putObject({
  Key: '65467f5501a2ada49f2fd19419e12e8755a2ca17c712491e3aa3acc9123d3331',
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>'
})
[AWS s3 200 0.061s 0 retries] putObject({
  Key: '47a7fd614f22e3452a52f3f98991a5636e597d9e17654067f150f41bad9047ae',
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>'
})
[AWS sts 200 0.192s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.239s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: '68349ed6f621438813c92b9c1f5b2a974fc9b47176ac65907e41cceef0a9b7ea'
})
[AWS sts 200 0.124s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.166s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: '294eb6d435b3c630d5f750bfc01559e503d6006b589b41accec18beb19437eaf'
})
[AWS sts 200 0.19s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.243s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: '7296abd8b8134de2f105e1e4225f5a4e34f1b3666a1525c1718fc6df1febde1a'
})
[AWS sts 200 0.044s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.094s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: 'f46ff44aeb08ab78152287fffe4729ab0913052cd2f52337e230a6654d87f506'
})
[AWS sts 200 0.163s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.248s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: '4db9279d8eed806482081a688803e61ee3f39a475da6bd579743b2c22155d376'
})

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jan 13, 2021
@jacknagel
Copy link
Author

jacknagel commented Mar 21, 2021

@ajredniwja I've tracked down why this is happening. It's this line:

var config = AWS.util.copy(service._originalConfig || {});
that causes the credentials to get re-fetched each time s3.upload is called (since each call creates a new ManagedUpload client).

If I change it from var config = AWS.util.copy(service._originalConfig || {}) to var config = AWS.util.copy(service.config), the credentials are re-used across calls to s3.upload.

It was changed in this commit: e8f040e, so I'm not sure what the correct fix is.

@jacknagel
Copy link
Author

@AllanZhengYP This change: e8f040e (#3109) altered the behavior of ManagedUpload (and by extension s3.upload) such that cached credentials are never re-used. The result for us is that every call to s3.upload requests new temporary credentials from STS.

This bug is currently blocking us from fully rolling out EKS IRSA to our apps.

@0xknon
Copy link

0xknon commented Apr 3, 2021

I am also trying to implement IRSA on my cluster. I am able to request for the assumeRoleWithWebIdentity

[AWS sts 200 2.505s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACTED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})

After that, when I use aws-sdk to putObject or upload object to my S3 bucket, I get the error message:

InvalidToken: The provided token is malformed or otherwise invalid.

However, when I use aws-cli to run aws s3 cp <path> <s3 uri> inside the Pod, the service account provided the credential correctly and uploaded the file successfully. Does this also happen to you guys?

@trivikr trivikr removed needs-triage This issue or PR still needs to be triaged. investigating Issue has been looked at and needs deep dive work by OSDS. labels Apr 7, 2021
@jacknagel
Copy link
Author

@AllanZhengYP Thanks for working on the fix. Unfortunately, it doesn't seem to have fully solved the problem. If I run the following script:

const AWS = require('aws-sdk')
const { randomBytes } = require('crypto')

AWS.config.update({
  logger: console
})

console.log('AWS SDK version: ' + AWS.VERSION)

const s3 = new AWS.S3({
  apiVersion: '2006-03-01',
  params: {
    Bucket: '<REDACTED>'
  }
});

(async function() {
  for (let i = 0; i < 3; i++) {
    const key = randomBytes(32).toString('hex')
    await s3.upload({ Key: key, Body: Buffer.from('test') }).promise()
  }
})()

I still see credentials being re-fetched on every call to s3.upload:

AWS SDK version: 2.885.0
[AWS sts 200 0.498s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACATED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.839s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: 'acdeaef4a58eea73c1f067b9143353cb606f6bb6764ed56d9ea6d4402969a207'
})
[AWS sts 200 0.401s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACATED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.696s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: '216bc6d80834a7c86b86a8e2c7b78a2e87f7d15b2888eb93033f434136af329e'
})
[AWS sts 200 0.416s 0 retries] assumeRoleWithWebIdentity({
  WebIdentityToken: '<REDACATED>',
  RoleArn: '<REDACTED>',
  RoleSessionName: 'token-file-web-identity'
})
[AWS s3 200 0.718s 0 retries] putObject({
  Body: <Buffer 74 65 73 74>,
  Bucket: '<REDACTED>',
  Key: '1cf67111f7717a8f91518fa2b0667c5d93a3077272eb79268b08dd1527b47581'
})

Any ideas?

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

Successfully merging a pull request may close this issue.

6 participants