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/emailAddress=em...@example.com" +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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org