-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
Support SSE-C for S3 object storage #941
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
server/src/storage/s3.rs
Outdated
@@ -84,6 +86,12 @@ pub struct S3Config { | |||
#[arg(long, env = "P_S3_BUCKET", value_name = "bucket-name", required = true)] | |||
pub bucket_name: String, | |||
|
|||
/// Server side encryption to use for operations with objects. | |||
/// Currently, this only supports SSE-C. Value should be | |||
/// like AES256:<base64_encoded_encryption_key>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was meant to be of type SSE-C:AES256:<base64_encoded_encryption_key>
. Will add that later today.
@MihirLuthra parseable is now using object_store version 0.11.0, can you please update this PR to use the same. |
Sure, will do tonight. |
60f5f5c
to
40ed381
Compare
Updated. Did a confirmation test against S3, works correctly. |
Actually, just clicked me that we don't really need an Let me know if you want me to change that. No particular issue with using fork as well (temporary anyway) |
@MihirLuthra you can do that, no need to use fork then. |
Done |
1b8f187
to
8858e9e
Compare
@MihirLuthra I have renamed the env var and related variables in the change. Thanks for the enhancement! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to merge
Fixes #919.
This PR introduces new args/envs allowing users to use SSE-C for encrypting the objects in
S3
.To achieve that, an arg
--object-sse
and envP_OBJECT_SSE
have been exposed. Example usage:Fortunately,
object_store
crate had already added a way to defineSSE-C
key fromAmazonS3Builder
: apache/arrow-rs#6230. But they have not published a new version to crates.io containing this change.Although, even if they did, it won't be usable yet. Apparently, the version of datafusion being used in
parseable
is not compatible with anything above0.10.2
yet. So, for now, I have created a fork ofarrow-rs
with a branch checked out ofobject_store_0.10.2
tag. On top of that, I have cherry picked commits from:Can be seen here: https://github.com/MihirLuthra/arrow-rs/tree/mihir/I-919-0.10.2-with-sse-c. This version of arrow-rs has been used in
parseable
by[patch.crates-io]
.In the changes here, I have used my own fork but if this way is acceptable, a fork would be needed in
parseablehq
organization instead.Other ways could be:
0.11.0
and askarrow-rs
maintainers to publish new version on crates.io. I think this would be a new issue in itself.SSE-C
change to 0.10 and introduce a new 0.10.3.I have tested this with my AWS Account. If parseable is started with
SSE-C
configured, then an attempt to download object without the key looks like this:But following works if correct key and md5 hash given:
For my testing, keys were generated as:
This PR has: