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

SSE-C support in S3 via External Storage #33283

Closed
xangelix opened this issue Jul 19, 2022 · 7 comments · Fixed by nextcloud/documentation#12530 or #50514
Closed

SSE-C support in S3 via External Storage #33283

xangelix opened this issue Jul 19, 2022 · 7 comments · Fixed by nextcloud/documentation#12530 or #50514
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: external storage feature: object storage

Comments

@xangelix
Copy link

Is your feature request related to a problem? Please describe.
SSE-C will soon be supported in primary storage.
It would be really great to be able to use this feature with external storage's implementation of S3 as well.

Describe the solution you'd like
I would like to be able to mount an S3 bucket with this option enabled in the standard settings as with any other S3 external storage configuration. Providing the option in the GUI is ideal, but not necessary for my use case--though I'm not sure if there's a way to add external mounts through nextcloud configuration files at this time.

Describe alternatives you've considered
I wrestled with various fuse implementations such as s3fs and geesefs which provide the SSE-C feature, and mounting as a local volume in external storage, but this proved a permissions nightmare and unstable in my container environment.

Additional context
None

@xangelix xangelix added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Jul 19, 2022
@warrenatharrisdotnetwork

a workaround is to use s3fs which does support sse_C. Lets not take time away from these developers with feature requests when their product is barely functional as is.

@xangelix
Copy link
Author

xangelix commented Jul 22, 2022

fuse implementations of S3 are inherently lacking in many ways. geesefs is a gigantic improvement but fails to work with Wasabi as required in my environment. Beyond all that, even under cgroupsv2, my environment's security requirements do not support this option.

Implementing the required headers for this feature seems like a reasonable request, and one that I would be happy to place a bounty on once when/if this issue is accepted/triaged.

@icassassin
Copy link

Just to comment on using s3fs for this: this solution isn't ideal, and possibly very detrimental depending on the terms of your S3 provider. I've noticed that when I upload a file through Nextcloud it seems to make 3 different operations on the file system. My provider has a minimum time of storage, and because of that, a file upload occupies the used space equal to the file size and deleted space equal to 2*file size, which is somewhat unsustainable with such a service. What I think is happening is that when you upload a file, it stores the upload chunks on the primary drive, and then rebuilds the full file onto the s3fs, and then renames the file, and maybe does something with permissions? In any case, from my S3 provider, it looks like I've copied the file 3 times and deleted it twice, and I'm guessing for some users that's going to be a serious problem.

@joshtrichards joshtrichards changed the title S3 SSE-C support for external storage SSE-C support in S3 via External Storage Aug 22, 2023
@asheroto
Copy link

This is supported now, yeah? Maybe this issue can be closed?

@joshtrichards
Copy link
Member

No, it's supported for Primary Storage, not External Storage at the moment.

@lobeck
Copy link
Contributor

lobeck commented Jan 22, 2025

Had a brief look at the code and it shouldn't be too hard to implement. Might take a look in the next couple of days.

@joshtrichards
Copy link
Member

Hi @lobeck - For reference when doing the External Storage implementation, the original PRs that added SSE-C for Primary Storage are:

lobeck added a commit to lobeck/server that referenced this issue Jan 22, 2025
Just need to add the parameter and fix two calls. All other logic is already implemented and implicitly called through the S3ConnectionTrait

fixes nextcloud#33283
lobeck added a commit to lobeck/server that referenced this issue Jan 22, 2025
Just need to add the parameter and fix two calls. All other logic is already implemented and implicitly called through the S3ConnectionTrait

fixes nextcloud#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
lobeck added a commit to lobeck/documentation that referenced this issue Jan 22, 2025
relates to nextcloud/server#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
lobeck added a commit to lobeck/server that referenced this issue Jan 22, 2025
Just need to add the parameter and fix two calls. All other logic is already implemented and implicitly called through the S3ConnectionTrait

fixes nextcloud#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
lobeck added a commit to lobeck/server that referenced this issue Jan 24, 2025
Just need to add the parameter and fix two calls. All other logic is already implemented and implicitly called through the S3ConnectionTrait

fixes nextcloud#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
st3iny pushed a commit that referenced this issue Jan 28, 2025
Just need to add the parameter and fix two calls. All other logic is already implemented and implicitly called through the S3ConnectionTrait

fixes #33283

Signed-off-by: Christian Becker <christian@dabecka.de>
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
AndyScherzinger pushed a commit to lobeck/documentation that referenced this issue Jan 30, 2025
relates to nextcloud/server#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
lobeck added a commit to lobeck/documentation that referenced this issue Jan 30, 2025
relates to nextcloud/server#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
backportbot bot pushed a commit to nextcloud/documentation that referenced this issue Jan 30, 2025
relates to nextcloud/server#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
backportbot bot pushed a commit to nextcloud/documentation that referenced this issue Jan 30, 2025
relates to nextcloud/server#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
backportbot bot pushed a commit to nextcloud/documentation that referenced this issue Jan 30, 2025
relates to nextcloud/server#33283

Signed-off-by: Christian Becker <christian@dabecka.de>
backportbot bot pushed a commit that referenced this issue Jan 31, 2025
Just need to add the parameter and fix two calls. All other logic is already implemented and implicitly called through the S3ConnectionTrait

fixes #33283

Signed-off-by: Christian Becker <christian@dabecka.de>
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: external storage feature: object storage
Projects
None yet
6 participants