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

Add S3 SSE-C config support for synapse-s3-storage-provider #2220

Merged
merged 13 commits into from
Jan 10, 2023

Conversation

xangelix
Copy link
Contributor

@xangelix xangelix commented Oct 30, 2022

Adds support for SSE-C

Blocked by:

  • Upstream: Changes in roles/matrix-synapse/templates/synapse/customizations/Dockerfile.j2 can be reverted after Add S3 SSE-C support matrix-org/synapse-s3-storage-provider#84

  • Testing: I've tested under Wasabi S3, and generic S3 implementations should be well-covered by the upstream PR, but it may be good to get some more reports and note any popular providers that do not support this header option or require additional configuration in the web gui

@spantaleev
Copy link
Owner

That's nice! Thanks for adding support for SSE-C! Let's wait for the upstream PR to get merged before merging this.


On a related note, I personally don't see data encryption at rest as very beneficial. You're still sending all your data and all the keys to [YOUR S3 PROVIDER]. They say they don't store the keys, but.. It's always better to not give them the keys at all, than to give them keys and hope they don't misuse them.

I'd rather send them pre-encrypted binary files so that they don't know what they're storing.. with no means to decrypt it there.
I was hoping that synapse-s3-storage-provider would get support for encrypting/decrypting files client-side. Not sure what the best way to do that would be (gpg?), but it sounds like a nice feature.

@xangelix
Copy link
Contributor Author

xangelix commented Nov 2, 2022

I agree 100%, and SSE-C does have that constant criticism. 'Theoretically' though, it's a comfortable improvement for some vendors, adding another small layer of security. There are also some other interesting use cases for it that aren't really relevant here.

Wasabi's overall security implementation is a bit different than AWS, as I'm sure all the other S3 providers.
https://wasabi-support.zendesk.com/hc/en-us/articles/115001693992-How-secure-is-my-data-
https://wasabi-support.zendesk.com/hc/en-us/articles/4414850567963
https://wasabi.com/security/

I'll admit that most of my concern is in the increasing use of perceptual and non-perceptual hashing lookups by storage providers if I'm to use the S3 storage provider extension to synapse. I'd really prefer not to be hit by copyright notices (or even hate speech notices?? as in the recent example that went viral with Google Drive) from an S3 provider because of random users on a matrix server messaging each other, but that may be where we're heading. Of course, this doesn't do much to stop that if they hash it before encryption, and an e2e solution like you've mentioned would be leaps and bounds better. So many necessary layers of encryption on top of one another... I'll perhaps feel better once all of the bridges I'm using fully support encryption, then I'll consider enforcing e2e for the entire server. Maybe there are ways to migrate old user data from non-encrypted channels I should look into.

@xangelix
Copy link
Contributor Author

xangelix commented Jan 3, 2023

I ended up opting to recommend against its usage instead of including a paragraph about what it does and its limitations.
Let me know how you feel about the wording in the docs but I think this is otherwise good to go.

@xangelix xangelix marked this pull request as ready for review January 3, 2023 23:02
@spantaleev
Copy link
Owner

Is the 1.1.2 release likely to happen soon? We could just wait for that and not have separate code paths or people stuck on the custom git version afterwards.

@xangelix
Copy link
Contributor Author

xangelix commented Jan 4, 2023

Looks like the request is now in, so probably worth waiting. 👍
matrix-org/synapse-s3-storage-provider#89

@xangelix
Copy link
Contributor Author

xangelix commented Jan 9, 2023

Version bump was quick and worth it. Should be ready to go!

@spantaleev spantaleev merged commit d241636 into spantaleev:master Jan 10, 2023
@spantaleev
Copy link
Owner

Thank you!

spantaleev added a commit that referenced this pull request Jan 19, 2023
This is an advanced feature with dubious usefulness.
Putting it in the default config just confuses people.

Related to #2220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants