-
Notifications
You must be signed in to change notification settings - Fork 620
Description
Checkboxes for prior research
- I've gone through Developer Guide and API reference
- I've checked AWS Forums and StackOverflow.
- I've searched for previous similar issues and didn't find any solution.
Describe the bug
This is yet another reiteration of #4736. Because I'm pretty sure something is not working as intend here, and cannot figure out how to convert a UInt8Array
to a string that is a valid key for SSECustomerKey in the S3 APIs.
Basically, if I have a UInt8Array
of 32 random bytes for an AES256 key, there doesn't seem to be any way to pass that to SSECustomerKey
that complies with the tyepscript types for CopyObjectCommandInput
, PutObjectCommandInput
, GetObjectCommandInput
, etc.
SDK version number
@aws-sdk/package-name@version, ...
Which JavaScript Runtime is this issue in?
Node.js
Details of the browser/Node.js/ReactNative version
18.18.0
Reproduction Steps
import {S3Client, PutObjectCommand} from '@aws-sdk/client-s3';
import * as crypto from 'crypto';
import {promisify} from 'util';
const randomBytes = promisify(crypto.randomBytes);
const client = new S3Client();
key = await randomBytes(32);
const response = await client.send(new PutObjectCommnd({Bucket: 'mybucket', Body: 'This is a test', SSECustomerKey: key.toString(), SSECustomerAlgorithm: 'AES256});
Observed Behavior
The request fails with a 400, with the error "InvalidArgument: The secret key was invalid for the specified algorithm."
This happens regardless of the encoding used. I have tried calling toString
on the key with the following encodings:
- utf8
- binary
- base64
- hex
All with the same result.
Expected Behavior
It should be possible to use an arbitrary, randomly generated key for the SSE customer key, even if it isn't a valid UTF-8 string.
Possible Solution
As @DavidZbarsky-at pointed out in the original issue. It loos like the code actually handles being passed in a UInt8Array
directly, and indeed if I change the reproduction steps to pass in the random bytes directly (SSECustomerKey: key
), then the command succeeds.
So probably just accept that type?
Another possible solution would be to not have the middleware base64 encode the SSECustomerKey value, and let the user do the base64 encoding themselves. However that would not be backwards compatible, since it would break any usage that depended on the library doing the base64 encoding for you.
Maybe it could do some heuristics to look at the length of the string, and if it looks like it is already base64 encoded, then just use it as is?
Additional Information/Context
From my inspection of looking at the code at https://github.com/aws/aws-sdk-js-v3/blob/f5c19c0c314ea1e02f067c169c8d6e7dacb733ac/packages/middleware-ssec/src/index.ts#L36C6-L53C8
I believe what is happening is:
If the key is a string, then we encode it using "utf-8", then base64 encode that to a string that we use in the actual request.
However, if you have a key that is not a valid utf8 string, which is common if it is randomly generated, then it isn't possible to convert it as a string that will encode using the utf8 encoding back to the original bytes. If you encode it using "utf8", then any invalid sequences will be transcoded as U+FFFD. If you use the binary
or latin1
encoding, then the utf8 encoding will corrupt any characters that are multi-byte in utf-8 (i.e. non-ascii characters).
Regarding this comment in the previous thread:
this is actually not considered as a bug. The type expected for SSECustomerKey is a string and needs to be provided as so. This type is actually defined by the service itself and the SDK just follows its directives regarding typing.
I think that would be true if the library wasn't doing any pre-processing before passing the value on to the S3 service. However, this library is doing preprocessing. Namely base64 encoding the key. Since base64 encoding transforms binary data into a string, I think that the type of this parameter should accept a UInt8Array.
If there is some way to encode a UInt8Array
of random bytes into a string that would work with this, I'd love to hear about it. Although, that still has the issue that it unnecessarily goes from a byte array, to a string, and then back to a byte array.
One final note: The SDK documentation for SSECustomerKey makes no mention that the library performs this base64 encoding. So, since the expected type was a string, and the documentation for the http API said that the value needed to be base64 encoded, I assumed that I was responsible for base64 encoding the key. So, either way that documentation should probably be updated to explain what format the key is expected to be in, and whether the base64 encoding is the responsibility of the caller or the library.