```
(I know these are the same files because I’m also printing out the client-side MD5 sum and the `put_object` request options, which include the filename)
It turns out that [the ETag response header](https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html) behaves… differently when using SSE-C:
> Objects encrypted by server-side encryption with customer-provided keys (SSE-C) or AWS Key Management Service (AWS KMS) keys (SSE-KMS) have ETags that are not an MD5 digest of their object data.
The only way I can find to do integrity verification of uploads with SSE-C is to send the `Content-MD5` request header, and let S3 do the corruption detection. Note also that [the already-uploaded check](https://github.com/discourse/discourse/blob/main/lib/file_store/to_s3_migration.rb#L239) will also break when using SSE-C, but at least that can be disabled using `SKIP_ETAG_VERIFY`.
I’m not submitting a PR straight off because there’s two different ways to approach this:
1. just extend the `SKIP_ETAG_VERIFY` to encompass post-upload verification, which is cheap and hacky, and requires users to know that their use of SSE-C means they’ll have to turn that on; or
2. Switch to using the Content-MD5 header (preferably always) to do upload integrity protection, which has the benefit of working for everyone, at the cost of a far larger PR.
(As an aside, I’m rather disturbed that nobody has hit this before – is _nobody_ using Discourse with SSE-C for uploads?!?)
---
### Author:  [@RGJ](https://meta.discourse.org/u/RGJ)
#### Post date: [1 juni 2023 om 08:52 UTC](https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853/2 "2023-06-01T08:52:36Z")
> [@mpalmer](#):
>
> (As an aside, I’m rather disturbed that nobody has hit this before – is _nobody_ using Discourse with SSE-C for uploads?!?)
My guess is that “secure uploads” is used in only very specific cases, which makes 99.8% of all uploads being served publicly anyway, which renders any kind of encryption-at-rest useless?
---
### Author:  [@sam](https://meta.discourse.org/u/sam)
#### Post date: [2 juni 2023 om 02:02 UTC](https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853/4 "2023-06-02T02:02:55Z")
You are not wrong here Matt, this is the first time this has been brought up as far as I know.
(2) seems like the correct approach to me if you can swing it.
Otherwise I guess you will not have confidence the files are all there properly.
Once you get all the uploads into your S3 bucket is the general operation of Discourse OK? Are other changes needed for day to day use (thinking things like inventory may break - maybe changes are needed for pre-signed urls, etc…)
---
### Author:  [@mpalmer](https://meta.discourse.org/u/mpalmer)
#### Post date: [4 juni 2023 om 06:17 UTC](https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853/5 "2023-06-04T06:17:12Z")
I’ll try to swing it. Thankfully, finding everywhere that mentions “etag” should be enough to find all the code locations that need changing, and the test suite is nicely laid out so I can find what needs to be run and changed fairly easily.
As far as other things that may break go, I haven’t managed to get as far as a working site just yet, and I’ll keep reporting/PRing as I go along. Inventory _shouldn’t_ break, because only the file content is encrypted, not the names. I probably won’t find out if pre-signed URLs will break, as I’m not using them.
---
### Author:  [@mpalmer](https://meta.discourse.org/u/mpalmer)
#### Post date: [5 juni 2023 om 02:20 UTC](https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853/6 "2023-06-05T02:20:31Z")
I’ve just opened [this PR](https://github.com/discourse/discourse/pull/21923) which removes ETag-based upload verification during S3 migration. Turns out it’s _only_ used during migration to S3; on regular upload, it’s just YOLO’d, which made my life a lot easier. With this PR, and the [ACL disabling site setting PR](https://github.com/discourse/discourse/pull/21769) previously submitted, I have a completed restore now. Next step: testing features.
---
### Author:  [@JammyDodger](https://meta.discourse.org/u/JammyDodger)
#### Post date: [7 juli 2023 om 11:27 UTC](https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853/12 "2023-07-07T11:27:31Z")
I think this has now been merged. 🥳
>
>
> They don't work for server-side encryption with customer keys, and so instead we… just use Content-MD5 to ensure there was no corruption in transit, which is the best we can do.
>
> See also: https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853
>
> No tests were changed because, as far as I can tell, there are no tests for this area of the codebase.
---
### Author:  [@JammyDodger](https://meta.discourse.org/u/JammyDodger)
#### Post date: [10 juli 2023 om 07:00 UTC](https://meta.discourse.org/t/s3-uploads-incompatible-with-server-side-encryption/266853/13 "2023-07-10T07:00:57Z")
This topic was automatically closed after 2 days. New replies are no longer allowed.