aws / aws-sdk-js

AWS SDK for JavaScript in the browser and Node.js
https://aws.amazon.com/developer/language/javascript/
Apache License 2.0
7.59k stars 1.55k forks source link

getSignedUrl not always returning a valid signed-url #3305

Closed Mnkras closed 5 months ago

Mnkras commented 4 years ago

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

Describe the bug Sometimes when calling s3.getSignedUrl(method, params) we receive an invalid signed URL from the SDK and no error is thrown. EG:https://s3.us-west-2.amazonaws.com/ This happens on both getObject and putObject methods, and retrying the call produces the expected results.

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 Node v10.21.0

SDK version number v2.694.0

To Reproduce (observed behavior) Excerpt from our code

const AWS = require('aws-sdk');
const path = require('path');
const log = require('../logger');
const config = require('config');
const bucket = config.s3.uploads.bucket;
const region = config.s3.uploads.region;
const s3 = new AWS.S3({
    apiVersion: '2006-03-01',
    signatureVersion: 'v4',
    region
});

async function getSignedUrl(method, params) {
    try {
        const preSignedUrl = s3.getSignedUrl(method, params);
        if (!preSignedUrl.includes("X-Amz-Signature")) {
            log.error(`received a non valid ${method} presigned url : ${preSignedUrl}`);
            throw new Error ("Illegal presigned url returned!");
        }
        return preSignedUrl;
    } catch (err)  {
        log.error(err, `Error retrieving ${method} pre-signed url`);
        throw err;
    }
}

Params being:

            const uploadlinkparams = {
                Key: prefix,
                Bucket: bucket,
                Expires: 60 * 60, // one hour
                Tagging: "" // This header needs to exist otherwise we can't add tags
            };

            const downloadlinkparams = {
                Key: prefix,
                Bucket: bucket,
                Expires: downloadttl ? downloadttl * 60 : 60 * 60 * 24 * 7 // 7 days if not specified
            };

Expected behavior We expect to receive a valid pre-signed url, or an error to be thrown by the SDK

Screenshots N/A

Additional context This code runs on EC2 instances. I cannot reproduce locally outside of AWS. It is hard to reproduce in AWS.

We see the following calls made before the failure: PUT //169.254.169.254/latest/api/token [200] GET //169.254.169.254/latest/meta-data/iam/security-credentials/ [200] GET //169.254.169.254/latest/meta-data/iam/security-credentials/InstanceProfile [200]

With the failure message being: received a non valid putObject presigned url : https://s3.us-west-2.amazonaws.com/

ajredniwja commented 4 years ago

@Mnkras thank-you for reaching out to us with your issue, can you share how you are setting your credentials? I am not able to quite reproduce it.

sandorvasas commented 4 years ago

I'm not sure this is the same error, but we've been getting 502 errors when trying to generate a signed url. Only sometimes and only for that endpoint of our server.

Mnkras commented 4 years ago

@ajredniwja The SDK is grabbing the credentials from the EC2 instance metadata service.

@sandorvasas That seems like an un-related issue, you may want to open a ticket with AWS Support as a 502 comes from the server.

ajredniwja commented 4 years ago

@ajredniwja The SDK is grabbing the credentials from the EC2 instance metadata service.

Is it possible for you to set the credentials explicitly and not touch the EC2 instance metadata and see if that resolves the problem?

Mnkras commented 4 years ago

When I was running locally with explicitly set credentials I didn't have any issues, just when using the metadata service, my guess is that the function is returning before it has creds, and for some reason isn't returning an error...

ajredniwja commented 4 years ago

That is probably the cause, you might wanna wrap wrap the function in a promise and increase the timeout. If your application allows then setting it explicitly should solve the problem as well.

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/EC2MetadataCredentials.html Also if you can try performing a referesh() too which I think you might have been doing already.

Mnkras commented 4 years ago

Can do, can we change this synchronous function to throw an error when it can't build a valid signed url? this sent us down a rabbit-hole that was super hard for us to reproduce.

ajredniwja commented 4 years ago

@Mnkras That can be regarded as a feature request but I think it has already been raised but since there is a workaround for that the priority for that is really low.

Also it will be beneficial for your app to use getSignedUrlPromise

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#getSignedUrlPromise-property

It doesn't solve your problem but I thought it can save you some lines of code.

Mnkras commented 4 years ago

Yep switched to getSignedUrlPromise seems good so far.

I think there should be some documentation at-least about this failure-mode.

github-actions[bot] commented 3 years ago

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.

Mnkras commented 3 years ago

Commenting to keep this open

sshadmand commented 2 years ago

As of today, it seems like a call to this function (promise or not) returns the bare legacy https://s3.amazonaws.com/ while only https://s3.[region].amazonaws.com/ is allowed.

JarwisMaster commented 2 years ago

There must be a method that does not include 'bucket name' in the path. If i use https://[bucket].s3-[region].amazonaws.com/ as S3 andpoint method add bucket name in path to file and i got https://[bucket].s3-[region].amazonaws.com/[bucket]/. If i use https://s3.amazonaws.com/ -> i got correct route but with permanent redirect to https://[bucket].s3-[region].amazonaws.com/ and cors errors in browser

Joachimzeelmaekers commented 2 years ago

As of today, it seems like a call to this function (promise or not) returns the bare legacy https://s3.amazonaws.com/ while only https://s3.[region].amazonaws.com/ is allowed.

@sshadmand I had the same issue, but I resolved it by adding an explicitly signature version like this:

  private getAwsClient() {
    const options: AWS.S3.ClientConfiguration = {
      region: this.region,
      logger: process.stdout,
      s3ForcePathStyle: true,
      signatureVersion: 'v4',
    };
    return new AWS.S3(options);
  }

  public generateUrlForBucketAndKey(
    bucketName: string,
    keyName: string,
    expiresInSeconds = 300,
  ): Promise<string> {
    return this.getAwsClient().getSignedUrlPromise('getObject', {
      Bucket: bucketName,
      Key: keyName,
      Expires: expiresInSeconds,
    });
  }
ArunprasadRajendran commented 2 years ago

when we get the image or video file from the aws getSignedUrl why the getting files are showing in the source page folder while doing the inspect element? How to avoid or clear the image files. Please anyone can explain immediately.

cokia commented 2 years ago

I had the same problem.

I inserted a correct access token, but some requests received only basepath, and some requests received normal url.

I was able to get the correct url when I modified "getSignedUrl" to "await getSignedUrlPromise".

GuyMev commented 2 years ago

Having the same problem here, tried changing to await getSignedUrlPromise without luck

joelcolucci commented 1 year ago

I ran into this issue. I solved it by adding signatureVersion to the AWS SDK config (see below). Without it getSignedUrlPromise will return a url without the signature field query string parameter. This causes the URL not to work.

import AWS from 'aws-sdk';

AWS.config.update({
  signatureVersion: 'v4'
});

const s3 = new AWS.S3();
kellertk commented 5 months ago

In order for this to work, you need to specify a signatureVersion, as @joelcolucci, @Joachimzeelmaekers, and other have noticed.