wjones127 commented on code in PR #6230:
URL: https://github.com/apache/arrow-rs/pull/6230#discussion_r1718611372
##########
object_store/src/aws/builder.rs:
##########
@@ -813,6 +827,16 @@ impl AmazonS3Builder {
self
}
+ /// Use SSE-C for server side encryption.
+ pub fn with_ssec_encryption(mut self, customer_key_base64: impl
Into<String>) -> Self {
Review Comment:
I'm confused, should this parameter already be base64 encoded? I ask because
internally it looks like you are encoding it?
```suggestion
pub fn with_ssec_encryption(mut self, customer_key: impl Into<String>)
-> Self {
```
##########
object_store/src/aws/builder.rs:
##########
@@ -813,6 +827,16 @@ impl AmazonS3Builder {
self
}
+ /// Use SSE-C for server side encryption.
+ pub fn with_ssec_encryption(mut self, customer_key_base64: impl
Into<String>) -> Self {
+ self.encryption_type =
Some(ConfigValue::Parsed(S3EncryptionType::SseC));
+ if let Some(customer_key_64) = customer_key_base64.into().into() {
+ self.encryption_customer_key_base64 =
+ Some(BASE64_STANDARD.encode(customer_key_64.clone()));
+ }
Review Comment:
Why is this conditional? Seems like you could just do this:
```suggestion
self.encryption_customer_key_base64 =
Some(BASE64_STANDARD.encode(customer_key.into()));
```
##########
object_store/CONTRIBUTING.md:
##########
@@ -101,6 +101,52 @@ export AWS_SERVER_SIDE_ENCRYPTION=aws:kms:dsse
cargo test --features aws
```
+#### SSE-C Encryption tests
+
+Unfortunately, localstack does not support SSE-C encryption
(https://github.com/localstack/localstack/issues/11356).
+
+We will use
[MinIO](https://min.io/docs/minio/container/operations/server-side-encryption.html)
to test SSE-C encryption.
+
+First, create a self-signed certificate to enable HTTPS for MinIO, as SSE-C
requires HTTPS.
+
+```shell
+mkdir ~/certs
+cd ~/certs
+openssl genpkey -algorithm RSA -out private.key
+openssl req -new -key private.key -out request.csr -subj
"/C=US/ST=State/L=City/O=Organization/OU=Unit/CN=example.com/[email protected]"
+openssl x509 -req -days 365 -in request.csr -signkey private.key -out
public.crt
+rm request.csr
+```
+
+Second, start MinIO with the self-signed certificate.
+
+```shell
+docker run -d \
+ -p 9000:9000 \
+ --name minio \
+ -v ${HOME}/certs:/root/.minio/certs \
+ -e "MINIO_ROOT_USER=minio" \
+ -e "MINIO_ROOT_PASSWORD=minio123" \
+ minio/minio server /data
+```
+
+Create a test bucket.
+
+```shell
+export AWS_ACCESS_KEY_ID=minio
+export AWS_SECRET_ACCESS_KEY=minio123
Review Comment:
```suggestion
export AWS_BUCKET_NAME=test-bucket
export AWS_ACCESS_KEY_ID=minio
export AWS_SECRET_ACCESS_KEY=minio123
export AWS_ENDPOINT=https://localhost:9000
```
##########
object_store/src/aws/builder.rs:
##########
@@ -813,6 +827,16 @@ impl AmazonS3Builder {
self
}
+ /// Use SSE-C for server side encryption.
Review Comment:
Should probably say whether it should already be base64 encoded.
##########
object_store/src/aws/mod.rs:
##########
@@ -605,4 +606,71 @@ mod tests {
store.delete(location).await.unwrap();
}
}
+
+ /// See CONTRIBUTING.md for the MinIO setup for this test.
+ #[tokio::test]
+ async fn test_s3_ssec_encryption_with_minio() {
+ if std::env::var("TEST_S3_SSEC_ENCRYPTION").is_err() {
+ eprintln!("Skipping S3 SSE-C encryption test");
+ return;
+ }
+ eprintln!("Running S3 SSE-C encryption test");
+
+ let customer_key = "1234567890abcdef1234567890abcdef";
+ let expected_md5 = "JMwgiexXqwuPqIPjYFmIZQ==";
+
+ let store = AmazonS3Builder::new()
+ .with_bucket_name("test-bucket")
+ .with_access_key_id("minio")
+ .with_secret_access_key("minio123")
+ .with_ssec_encryption(customer_key)
+ .with_endpoint("https://localhost:9000";)
Review Comment:
Instead of hard coding these values in the binary, could we take them from
environment variables? That way we can just change the environment variables to
point them at an AWS S3 bucket to re-use the test.
```suggestion
let store = AmazonS3Builder::from_env()
.with_ssec_encryption(customer_key)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
