Aws-sdk-net: S3 request signing broken for S3-compatible services

[Sumo-MBryant]

If one is to use an S3-compatible service (e.g. for integration testing by mocking S3), signed requests are no longer valid as of this commit fbea7531858a2801c0e7734aaf15b72c04b0732d, which appears to use a heuristic to detect whether a request is being made to S3.

When using an S3-compatible service the ClientConfig.ServiceUrl might no longer satisfy the heuristic which appears to check the endpoint host name, and as such the resourcePath is double encoded and no longer signs correctly.

This is the heuristic:

https://github.com/aws/aws-sdk-net/blob/fbea7531858a2801c0e7734aaf15b72c04b0732d/sdk/src/Core/Amazon.Runtime/Internal/Util/S3Uri.cs#L35

and its use:

https://github.com/aws/aws-sdk-net/blob/fbea7531858a2801c0e7734aaf15b72c04b0732d/sdk/src/Core/Amazon.Util/AWSSDKUtils.cs#L318-L324

Expected Behavior

When connecting to an S3-compatible service using ClientConfig.ServiceUrl, all S3 requests including those using characters that are transformed during URL encoding should be correctly signed according the version 4 signature scheme.

The CanonicalizeResourcePath() function should only single encode requests.

Current Behavior

The S3-compatible service rejects the request that include characters that are transformed during URL encoding.

Consider an attempt to store an object to a content bucket and object key: Alp?ha/Be%ta /Test&ing.

CanonicalizeResourcePath() was called with the following arguments:

endpoint = "http://192.168.99.100:33886/"
resourcePath = "/content/Alp?ha/Be%ta /Test&ing"
detectPreEncode = true

The function logged the following:

Double encoded /content/Alp?ha/Be%ta /Test&ing with endpoint http://192.168.99.100:33886/ for canonicalization: /content/Alp%253Fha/Be%2525ta%2520/Test%2526ing

Possible Solution

1. It appears that the that signing code is invoked from the S3 client code which intrinsically knows only single URL encoding is required. Expose the functionality through the AWS4Signer such that all S3 requests are only single encoded.

2. If that isn't possible, provide some means for configuration to disable the heuristic and double encoding.

Context

We have some integration tests that ensure our code correctly handles paths sensitive to URL encoding when storing objects in S3. Upon upgrade to AWSSDK.Core 3.3.21.15 and later, this test started to fail.

Your Environment

• AWSSDK.Core version used: 3.3.21.20
• Service assembly and version used: AWSSDK.S3 3.3.18
• Operating System and version: Microsoft Windows 10
• Targeted .NET platform: .NET Core 2.0

[sstevenkang]

Thanks for reporting this with great detail. We are not currently prioritizing work to accommodate for non-S3 services. We'll add this into our backlog as a feature request.

[Genbox]

@sstevenkang This has severe security complications as people implement workarounds by disabling signing. I would urge you to reconsider prioritizing this issue as it seems like a simple fix.

[mechanic22]

Does anyone know of a workaround for this issue?

[Sumo-MBryant]

Our original workaround was to pin AWSSDK.Core to an old version without the issue. This only works if you don't want to use new features such as AWSSDK.SecretsManager for which there is no version that is compatible with an "old" version of AWSSDK.Core.

We've disabled the affected tests for the time being.

[maxima120]

@Sumo-MBryant please share the Core version without the issue. I rolled back as far as 3.3.22 and it still fails.

[maxima120]

I would urge you to reconsider prioritizing this issue as it seems like a simple fix.

they don't like people using other platforms. Since Amazon pays devs' salaries, the "open sourceness" is a farce.

[Sumo-MBryant]

We used 3.3.19 to avoid the problem.

[langebo]

Is there any chance we can at least get an ETA or is this welcome issue for amazon? We use the workaround described by @Sumo-MBryant to support foreign s3 compatible storage providers.

We would also like to add healthchecks in regard of network connectivity to s3, but the library we use requires a higher version of AWSSDK.Core.

[Adriien-M]

I downgraded Core library to 3.3.19 as suggested by @Sumo-Bryant but honestly, it is not acceptable from Amazon to not fix this issue...

[kevinolinger]

Downgrading is not a option if you need to write to a Snowball Edge endpoint, as the lower version doesn't allow disabling chunk encoding which is required for the endpoint. This bug has been open for a year and half now, I'm starting think AWS doesn't care too much about the dotnet SDK.

[Genbox]

Due to this and many other issues, I've created a modern implementation of the S3 API. You can find it here: https://github.com/Genbox/SimpleS3

It is not ready for production yet, but I encourage you to try it out and give feedback.

[harshavardhana]

@Genbox have you looked at https://github.com/minio/minio-dotnet

[LordMike]

@harshavardhana to be fair, that API really sucks if you want to do anything besides exactly downloading a file or uploading one.. F.ex. it insists that downloading objects is by writing to local streams .. so you can never read from the network ..

[harshavardhana]

@LordMike where does it say that ?

[LordMike]

@LordMike where does it say that ?

https://github.com/minio/minio-dotnet/blob/master/Minio/ApiEndpoints/IObjectOperations.cs#L38

You're meant to handle all fetching of data in that action, if you wanted to let some external code handle reading - you can't.. So in effect, you must write / copy the data from the provided stream, to something else (file, memorystream, ..) before continuing..

[Genbox]

@harshavardhana I tried it in a project I'm working on since we are running S3 in production and Minio for on-premise (where S3 client doesn't work). I had a lot of trouble with proxy support and configurability, and I also wanted something where you don't store the access key as a string as you can't properly clear the key material from memory.

I think it is good to have a more rich ecosystem than just a single client - minio-dotnet and SimpleS3 both contribute to that right now.

[harshavardhana]

@LordMike that is the part perhaps you are confused here, GetObjectAsync returns stream in a callback all your application has to do is provide a callback to look at the stream. Async was implemented with community feedback as a more idiomatic style.

                await minio.GetObjectAsync(bucketName, objectName, 
                (stream) =>
                {
                     // Uncomment to print the file on output console
                    // stream.CopyTo(Console.OpenStandardOutput());
                });

I would love to see more feedback on the project issue itself, to see if there are other varying use-cases which this library doesn't cover.

@harshavardhana I tried it in a project I'm working on since we are running S3 in production and Minio for on-premise (where S3 client doesn't work). I had a lot of trouble with proxy support and configurability, and I also wanted something where you don't store the access key as a string as you can't properly clear the key material from memory.

@Genbox Not able to follow did you ever open an issue for this?.

I totally agree it would be nicer of AWS S3 .Net SDK fixes the support here.

[maxima120]

Could you two please get a room and stop promoting your own projects. I am tired of having dozens email exposing you love hate relationships to the whole world. This is Amazon project. Thank you.

[harshavardhana]

Could you two please get a room and stop promoting your own projects. I am tired of having dozens email exposing you love hate relationships to the whole world. This is Amazon project. Thank you.

FWIW - these libraries wouldn't exist if the .Net SDK was fixed. Sorry, the intention wasn't to hijack the thread but to show alternatives since there is no current intention to fix it as mentioned here https://github.com/aws/aws-sdk-net/issues/933#issuecomment-382148606

[SrutiG]

The latest version of Core (3.3.105.0) fixes this issue by no longer double encoding S3 requests with a different service URL