From 28c36a8a833fdfc189d7916b64756ed0f342abe8 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Wed, 14 Feb 2024 20:43:06 -0800 Subject: [PATCH 1/6] feat: s3 server-side encryption --- object_store/CONTRIBUTING.md | 26 +++- object_store/src/aws/builder.rs | 227 +++++++++++++++++++++++++++++++- object_store/src/aws/client.rs | 20 ++- object_store/src/aws/mod.rs | 56 +++++++- 4 files changed, 316 insertions(+), 13 deletions(-) diff --git a/object_store/CONTRIBUTING.md b/object_store/CONTRIBUTING.md index aeb38e13a51c..d75a148a2c1f 100644 --- a/object_store/CONTRIBUTING.md +++ b/object_store/CONTRIBUTING.md @@ -47,13 +47,11 @@ Setup environment ``` export TEST_INTEGRATION=1 -export OBJECT_STORE_AWS_DEFAULT_REGION=us-east-1 -export OBJECT_STORE_AWS_ACCESS_KEY_ID=test -export OBJECT_STORE_AWS_SECRET_ACCESS_KEY=test -export OBJECT_STORE_AWS_ENDPOINT=http://localhost:4566 +export AWS_DEFAULT_REGION=us-east-1 export AWS_ACCESS_KEY_ID=test export AWS_SECRET_ACCESS_KEY=test -export OBJECT_STORE_BUCKET=test-bucket +export AWS_ENDPOINT=http://localhost:4566 +export AWS_BUCKET_NAME=test-bucket ``` Create a bucket using the AWS CLI @@ -74,6 +72,24 @@ Run tests $ cargo test --features aws ``` +#### Encryption tests + +To run integration tests with encryption, you can set the following environment variables: + +``` +export AWS_SERVER_SIDE_ENCRYPTION=aws:kms +export AWS_SSE_KMS_KEY_ID= +export AWS_SSE_BUCKET_KEY=false +``` + +As well as: + +``` +unset AWS_SSE_BUCKET_KEY +export AWS_SERVER_SIDE_ENCRYPTION=aws:kms:dsse +export AWS_SSE_KMS_KEY_ID= +``` + ### Azure To test the Azure integration diff --git a/object_store/src/aws/builder.rs b/object_store/src/aws/builder.rs index 9a296bc58537..d3baca86be2d 100644 --- a/object_store/src/aws/builder.rs +++ b/object_store/src/aws/builder.rs @@ -27,6 +27,7 @@ use crate::client::TokenCredentialProvider; use crate::config::ConfigValue; use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; use itertools::Itertools; +use reqwest::header::{HeaderMap, HeaderValue}; use serde::{Deserialize, Serialize}; use snafu::{OptionExt, ResultExt, Snafu}; use std::str::FromStr; @@ -83,6 +84,19 @@ enum Error { #[snafu(display("Failed to parse the region for bucket '{}'", bucket))] RegionParse { bucket: String }, + + #[snafu(display("Invalid encryption type: {}. Valid values are \"AES256\", \"sse:kms\", and \"sse:kms:dsse\".", passed))] + InvalidEncryptionType { passed: String }, + + #[snafu(display( + "Invalid encryption header values. Header: {}, source: {}", + header, + source + ))] + InvalidEncryptionHeader { + header: &'static str, + source: Box, + }, } impl From for crate::Error { @@ -160,6 +174,10 @@ pub struct AmazonS3Builder { conditional_put: Option>, /// Ignore tags disable_tagging: ConfigValue, + /// Encryption (See [`S3EncryptionConfigKey`]) + encryption_type: Option>, + encryption_kms_key_id: Option, + encryption_bucket_key_enabled: Option>, } /// Configuration keys for [`AmazonS3Builder`] @@ -322,6 +340,9 @@ pub enum AmazonS3ConfigKey { /// Client options Client(ClientConfigKey), + + /// Encryption options + Encryption(S3EncryptionConfigKey), } impl AsRef for AmazonS3ConfigKey { @@ -346,6 +367,7 @@ impl AsRef for AmazonS3ConfigKey { Self::ConditionalPut => "aws_conditional_put", Self::DisableTagging => "aws_disable_tagging", Self::Client(opt) => opt.as_ref(), + Self::Encryption(opt) => opt.as_ref(), } } } @@ -377,6 +399,13 @@ impl FromStr for AmazonS3ConfigKey { "aws_disable_tagging" | "disable_tagging" => Ok(Self::DisableTagging), // Backwards compatibility "aws_allow_http" => Ok(Self::Client(ClientConfigKey::AllowHttp)), + "aws_server_side_encryption" => Ok(Self::Encryption( + S3EncryptionConfigKey::ServerSideEncryption, + )), + "aws_sse_kms_key_id" => Ok(Self::Encryption(S3EncryptionConfigKey::KmsKeyId)), + "aws_sse_bucket_key_enabled" => { + Ok(Self::Encryption(S3EncryptionConfigKey::BucketKeyEnabled)) + } _ => match s.parse() { Ok(key) => Ok(Self::Client(key)), Err(_) => Err(Error::UnknownConfigurationKey { key: s.into() }.into()), @@ -486,6 +515,15 @@ impl AmazonS3Builder { AmazonS3ConfigKey::ConditionalPut => { self.conditional_put = Some(ConfigValue::Deferred(value.into())) } + AmazonS3ConfigKey::Encryption(key) => match key { + S3EncryptionConfigKey::ServerSideEncryption => { + self.encryption_type = Some(ConfigValue::Deferred(value.into())) + } + S3EncryptionConfigKey::KmsKeyId => self.encryption_kms_key_id = Some(value.into()), + S3EncryptionConfigKey::BucketKeyEnabled => { + self.encryption_bucket_key_enabled = Some(ConfigValue::Deferred(value.into())) + } + }, }; self } @@ -531,6 +569,16 @@ impl AmazonS3Builder { self.conditional_put.as_ref().map(ToString::to_string) } AmazonS3ConfigKey::DisableTagging => Some(self.disable_tagging.to_string()), + AmazonS3ConfigKey::Encryption(key) => match key { + S3EncryptionConfigKey::ServerSideEncryption => { + self.encryption_type.as_ref().map(ToString::to_string) + } + S3EncryptionConfigKey::KmsKeyId => self.encryption_kms_key_id.clone(), + S3EncryptionConfigKey::BucketKeyEnabled => self + .encryption_bucket_key_enabled + .as_ref() + .map(ToString::to_string), + }, } } @@ -759,6 +807,35 @@ impl AmazonS3Builder { self } + /// Use SSE-KMS for server side encryption. + pub fn with_sse_kms_encryption(mut self, kms_key_id: impl Into) -> Self { + self.encryption_type = Some(ConfigValue::Parsed(S3EncryptionType::SseKms)); + if let Some(kms_key_id) = kms_key_id.into().into() { + self.encryption_kms_key_id = Some(kms_key_id); + } + self + } + + /// Use dual server side encryption for server side encryption. + pub fn with_dsse_kms_encryption(mut self, kms_key_id: impl Into) -> Self { + self.encryption_type = Some(ConfigValue::Parsed(S3EncryptionType::DsseKms)); + if let Some(kms_key_id) = kms_key_id.into().into() { + self.encryption_kms_key_id = Some(kms_key_id); + } + self + } + + /// Set whether to enable bucket key for server side encryption. This overrides + /// the bucket default setting for bucket keys. + /// + /// When bucket keys are disabled, each object is encrypted with a unique data key. + /// When bucket keys are enabled, a single data key is used for the entire bucket, + /// reducing overhead of encryption. + pub fn with_bucket_key(mut self, enabled: bool) -> Self { + self.encryption_bucket_key_enabled = Some(ConfigValue::Parsed(enabled)); + self + } + /// Create a [`AmazonS3`] instance from the provided values, /// consuming `self`. pub fn build(mut self) -> Result { @@ -882,6 +959,18 @@ impl AmazonS3Builder { (None, None, false) => format!("https://s3.{region}.amazonaws.com/{bucket}"), }; + let encryption_headers = if let Some(encryption_type) = self.encryption_type { + S3EncryptionHeaders::try_new( + &encryption_type.get()?, + self.encryption_kms_key_id, + self.encryption_bucket_key_enabled + .map(|val| val.get()) + .transpose()?, + )? + } else { + S3EncryptionHeaders::default() + }; + let config = S3Config { region, endpoint: self.endpoint, @@ -897,6 +986,7 @@ impl AmazonS3Builder { checksum, copy_if_not_exists, conditional_put: put_precondition, + encryption_headers, }; let client = Arc::new(S3Client::new(config)?); @@ -912,6 +1002,120 @@ fn parse_bucket_az(bucket: &str) -> Option<&str> { Some(bucket.strip_suffix("--x-s3")?.rsplit_once("--")?.1) } +/// Encryption configuration options for S3. +/// +/// These options are used to configure server-side encryption for S3 objects. +/// To configure them, pass them to [`AmazonS3Builder::with_config`]. +/// +/// Both [SSE-KMS] and [DSSE-KMS] are supported. [SSE-C] is not yet supported. +/// +/// [SSE-KMS]: https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingKMSEncryption.html +/// [DSSE-KMS]: https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingDSSEncryption.html +/// [SSE-C]: https://docs.aws.amazon.com/AmazonS3/latest/userguide/ServerSideEncryptionCustomerKeys.html +#[derive(PartialEq, Eq, Hash, Clone, Debug, Copy, Serialize, Deserialize)] +#[non_exhaustive] +pub enum S3EncryptionConfigKey { + /// Type of encryption to use. If set, must be one of "AES256", "aws:kms", or "aws:kms:dsse". + ServerSideEncryption, + /// The KMS key ID to use for server-side encryption. If set, ServerSideEncryption + /// must be "aws:kms" or "aws:kms:dsse". + KmsKeyId, + /// If set to true, will use the bucket's default KMS key for server-side encryption. + /// If set to false, will disable the use of the bucket's default KMS key for server-side encryption. + BucketKeyEnabled, +} + +impl AsRef for S3EncryptionConfigKey { + fn as_ref(&self) -> &str { + match self { + Self::ServerSideEncryption => "aws_server_side_encryption", + Self::KmsKeyId => "aws_sse_kms_key_id", + Self::BucketKeyEnabled => "aws_sse_bucket_key_enabled", + } + } +} + +#[derive(Debug, Clone)] +enum S3EncryptionType { + S3, + SseKms, + DsseKms, +} + +impl crate::config::Parse for S3EncryptionType { + fn parse(s: &str) -> Result { + match s { + "AES256" => Ok(Self::S3), + "aws:kms" => Ok(Self::SseKms), + "aws:kms:dsse" => Ok(Self::DsseKms), + _ => Err(Error::InvalidEncryptionType { passed: s.into() }.into()), + } + } +} + +impl From<&S3EncryptionType> for &'static str { + fn from(value: &S3EncryptionType) -> Self { + match value { + S3EncryptionType::S3 => "AES256", + S3EncryptionType::SseKms => "aws:kms", + S3EncryptionType::DsseKms => "aws:kms:dsse", + } + } +} + +impl std::fmt::Display for S3EncryptionType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.into()) + } +} + +/// A sequence of headers to be sent for write requests that specify server-side +/// encryption. +/// +/// Whether these headers are sent depends on both the kind of encryption set +/// and the kind of request being made. +#[derive(Default, Clone)] +pub struct S3EncryptionHeaders(pub HeaderMap); + +impl std::fmt::Debug for S3EncryptionHeaders { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // TODO: if we take a user-provided key, hide the key from debug output. + f.debug_map().entries(self.0.iter()).finish() + } +} + +impl S3EncryptionHeaders { + fn try_new( + encryption_type: &S3EncryptionType, + key_id: Option, + bucket_key_enabled: Option, + ) -> Result { + let mut headers = HeaderMap::new(); + headers.insert( + "x-amz-server-side-encryption", + HeaderValue::from_static(encryption_type.into()), + ); + if let Some(key_id) = key_id { + headers.insert( + "x-amz-server-side-encryption-aws-kms-key-id", + key_id + .try_into() + .map_err(|err| Error::InvalidEncryptionHeader { + header: "kms-key-id", + source: Box::new(err), + })?, + ); + } + if let Some(bucket_key_enabled) = bucket_key_enabled { + headers.insert( + "x-amz-server-side-encryption-bucket-key-enabled", + HeaderValue::from_static(if bucket_key_enabled { "true" } else { "false" }), + ); + } + Ok(Self(headers)) + } +} + #[cfg(test)] mod tests { use super::*; @@ -967,7 +1171,10 @@ mod tests { .with_config(AmazonS3ConfigKey::DefaultRegion, &aws_default_region) .with_config(AmazonS3ConfigKey::Endpoint, &aws_endpoint) .with_config(AmazonS3ConfigKey::Token, &aws_session_token) - .with_config(AmazonS3ConfigKey::UnsignedPayload, "true"); + .with_config(AmazonS3ConfigKey::UnsignedPayload, "true") + .with_config("aws_server_side_encryption".parse().unwrap(), "AES256") + .with_config("aws_sse_kms_key_id".parse().unwrap(), "some_key_id") + .with_config("aws_sse_bucket_key_enabled".parse().unwrap(), "true"); assert_eq!( builder @@ -1003,6 +1210,24 @@ mod tests { .unwrap(), "true" ); + assert_eq!( + builder + .get_config_value(&"aws_server_side_encryption".parse().unwrap()) + .unwrap(), + "AES256" + ); + assert_eq!( + builder + .get_config_value(&"aws_sse_kms_key_id".parse().unwrap()) + .unwrap(), + "some_key_id" + ); + assert_eq!( + builder + .get_config_value(&"aws_sse_bucket_key_enabled".parse().unwrap()) + .unwrap(), + "true" + ); } #[test] diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index e06a0ce1846b..ff1adfbd7a17 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -51,8 +51,11 @@ use reqwest::{ }; use serde::{Deserialize, Serialize}; use snafu::{ResultExt, Snafu}; + use std::sync::Arc; +use super::builder::S3EncryptionHeaders; + const VERSION_HEADER: &str = "x-amz-version-id"; /// A specialized `Error` for object store-related errors @@ -181,6 +184,7 @@ pub struct S3Config { pub checksum: Option, pub copy_if_not_exists: Option, pub conditional_put: Option, + pub encryption_headers: S3EncryptionHeaders, } impl S3Config { @@ -320,9 +324,17 @@ impl S3Client { /// Make an S3 PUT request /// /// Returns the ETag - pub fn put_request<'a>(&'a self, path: &'a Path, bytes: Bytes) -> Request<'a> { + pub fn put_request<'a>( + &'a self, + path: &'a Path, + bytes: Bytes, + with_encryption_headers: bool, + ) -> Request<'a> { let url = self.config.path_url(path); let mut builder = self.client.request(Method::PUT, url); + if with_encryption_headers { + builder = builder.headers(self.config.encryption_headers.0.clone()); + } let mut payload_sha256 = None; if let Some(checksum) = self.config.checksum { @@ -481,7 +493,8 @@ impl S3Client { let builder = self .client .request(Method::PUT, url) - .header("x-amz-copy-source", source); + .header("x-amz-copy-source", source) + .headers(self.config.encryption_headers.0.clone()); Request { builder, @@ -499,6 +512,7 @@ impl S3Client { let response = self .client .request(Method::POST, url) + .headers(self.config.encryption_headers.0.clone()) .with_aws_sigv4(credential.authorizer(), None) .send_retry(&self.config.retry_config) .await @@ -523,7 +537,7 @@ impl S3Client { let part = (part_idx + 1).to_string(); let response = self - .put_request(path, data) + .put_request(path, data, false) .query(&[("partNumber", &part), ("uploadId", upload_id)]) .send() .await?; diff --git a/object_store/src/aws/mod.rs b/object_store/src/aws/mod.rs index 4e8852475c34..8a8fb97a5460 100644 --- a/object_store/src/aws/mod.rs +++ b/object_store/src/aws/mod.rs @@ -62,7 +62,7 @@ mod dynamo; mod precondition; mod resolve; -pub use builder::{AmazonS3Builder, AmazonS3ConfigKey}; +pub use builder::{AmazonS3Builder, AmazonS3ConfigKey, S3EncryptionHeaders}; pub use checksum::Checksum; pub use dynamo::DynamoCommit; pub use precondition::{S3ConditionalPut, S3CopyIfNotExists}; @@ -164,7 +164,7 @@ impl Signer for AmazonS3 { #[async_trait] impl ObjectStore for AmazonS3 { async fn put_opts(&self, location: &Path, bytes: Bytes, opts: PutOptions) -> Result { - let mut request = self.client.put_request(location, bytes); + let mut request = self.client.put_request(location, bytes, true); let tags = opts.tags.encoded(); if !tags.is_empty() && !self.client.config.disable_tagging { request = request.header(&TAGS_HEADER, tags); @@ -374,8 +374,9 @@ impl MultiPartStore for AmazonS3 { #[cfg(test)] mod tests { use super::*; - use crate::tests::*; + use crate::{client::get::GetClient, tests::*}; use bytes::Bytes; + use tokio::io::AsyncWriteExt; const NON_EXISTENT_NAME: &str = "nonexistentname"; @@ -394,9 +395,10 @@ mod tests { list_uses_directories_correctly(&integration).await; list_with_delimiter(&integration).await; rename_and_copy(&integration).await; - stream_get(&integration).await; + // stream_get(&integration).await; multipart(&integration, &integration).await; signing(&integration).await; + s3_encryption(&integration).await; // Object tagging is not supported by S3 Express One Zone if config.session_provider.is_none() { @@ -515,4 +517,50 @@ mod tests { v2.list_with_delimiter(Some(&prefix)).await.unwrap(); } + + async fn s3_encryption(store: &AmazonS3) { + crate::test_util::maybe_skip_integration!(); + + let data = Bytes::from(vec![3u8; 1024]); + + let encryption_headers = &store.client.config.encryption_headers; + let expected_encryption = if let Some(encryption_type) = + encryption_headers.0.get("x-amz-server-side-encryption") + { + encryption_type + } else { + eprintln!("Skipping S3 encryption test - encryption not configured"); + return; + }; + + let locations = [ + Path::from("test-encryption-1"), + Path::from("test-encryption-2"), + Path::from("test-encryption-3"), + ]; + + store.put(&locations[0], data.clone()).await.unwrap(); + store.copy(&locations[0], &locations[1]).await.unwrap(); + + let (_, mut writer) = store.put_multipart(&locations[2]).await.unwrap(); + writer.write_all(&data).await.unwrap(); + writer.shutdown().await.unwrap(); + + for location in &locations { + let res = store + .client + .get_request(location, GetOptions::default()) + .await + .unwrap(); + let headers = res.headers(); + assert_eq!( + headers + .get("x-amz-server-side-encryption") + .expect("object is not encrypted"), + expected_encryption + ); + + store.delete(location).await.unwrap(); + } + } } From edae46dce6f0e747a3dbd6660cf7c206364d2861 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Thu, 15 Feb 2024 11:53:55 -0800 Subject: [PATCH 2/6] cleanup --- object_store/src/aws/client.rs | 4 +--- object_store/src/aws/mod.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index ff1adfbd7a17..3e074c21ad17 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use crate::aws::builder::S3EncryptionHeaders; use crate::aws::checksum::Checksum; use crate::aws::credential::{AwsCredential, CredentialExt}; use crate::aws::{ @@ -51,11 +52,8 @@ use reqwest::{ }; use serde::{Deserialize, Serialize}; use snafu::{ResultExt, Snafu}; - use std::sync::Arc; -use super::builder::S3EncryptionHeaders; - const VERSION_HEADER: &str = "x-amz-version-id"; /// A specialized `Error` for object store-related errors diff --git a/object_store/src/aws/mod.rs b/object_store/src/aws/mod.rs index 8a8fb97a5460..5da56b9fed69 100644 --- a/object_store/src/aws/mod.rs +++ b/object_store/src/aws/mod.rs @@ -395,7 +395,7 @@ mod tests { list_uses_directories_correctly(&integration).await; list_with_delimiter(&integration).await; rename_and_copy(&integration).await; - // stream_get(&integration).await; + stream_get(&integration).await; multipart(&integration, &integration).await; signing(&integration).await; s3_encryption(&integration).await; From bcd07112a07922ec0ee33e503ac32ca1b8b59f4b Mon Sep 17 00:00:00 2001 From: Will Jones Date: Thu, 15 Feb 2024 13:11:15 -0800 Subject: [PATCH 3/6] fix instructions --- object_store/CONTRIBUTING.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/object_store/CONTRIBUTING.md b/object_store/CONTRIBUTING.md index d75a148a2c1f..4b0ef1f0e696 100644 --- a/object_store/CONTRIBUTING.md +++ b/object_store/CONTRIBUTING.md @@ -39,7 +39,8 @@ To test the S3 integration against [localstack](https://localstack.cloud/) First start up a container running localstack ``` -$ podman run -d -p 4566:4566 localstack/localstack:2.0 +$ LOCALSTACK_VERSION=sha256:a0b79cb2430f1818de2c66ce89d41bba40f5a1823410f5a7eaf3494b692eed97 +$ podman run -d -p 4566:4566 localstack/localstack@$LOCALSTACK_VERSION $ podman run -d -p 1338:1338 amazon/amazon-ec2-metadata-mock:v1.9.2 --imdsv2 ``` @@ -51,6 +52,7 @@ export AWS_DEFAULT_REGION=us-east-1 export AWS_ACCESS_KEY_ID=test export AWS_SECRET_ACCESS_KEY=test export AWS_ENDPOINT=http://localhost:4566 +export AWS_ALLOW_HTTP=true export AWS_BUCKET_NAME=test-bucket ``` @@ -64,6 +66,7 @@ Or directly with: ``` aws s3 mb s3://test-bucket --endpoint-url=http://localhost:4566 +aws --endpoint-url=http://localhost:4566 dynamodb create-table --table-name test-table --key-schema AttributeName=path,KeyType=HASH AttributeName=etag,KeyType=RANGE --attribute-definitions AttributeName=path,AttributeType=S AttributeName=etag,AttributeType=S --provisioned-throughput ReadCapacityUnits=5,WriteCapacityUnits=5 ``` Run tests @@ -74,12 +77,20 @@ $ cargo test --features aws #### Encryption tests +To create an encryption key for the tests, you can run the following command: + +``` +export AWS_SSE_KMS_KEY_ID=$(aws --endpoint-url=http://localhost:4566 \ + kms create-key --description "test key" | + jq -r '.KeyMetadata.KeyId') +``` + To run integration tests with encryption, you can set the following environment variables: ``` export AWS_SERVER_SIDE_ENCRYPTION=aws:kms -export AWS_SSE_KMS_KEY_ID= export AWS_SSE_BUCKET_KEY=false +cargo test --features aws ``` As well as: @@ -87,7 +98,7 @@ As well as: ``` unset AWS_SSE_BUCKET_KEY export AWS_SERVER_SIDE_ENCRYPTION=aws:kms:dsse -export AWS_SSE_KMS_KEY_ID= +cargo test --features aws ``` ### Azure From 0de983441409dd566c82a78d3f29c5be6e87afc4 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Thu, 15 Feb 2024 13:16:31 -0800 Subject: [PATCH 4/6] also run with encryption in CI --- .github/workflows/object_store.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/object_store.yml b/.github/workflows/object_store.yml index 9bc3afb64497..33e4613afa33 100644 --- a/.github/workflows/object_store.yml +++ b/.github/workflows/object_store.yml @@ -114,6 +114,7 @@ jobs: AWS_ALLOW_HTTP: true AWS_COPY_IF_NOT_EXISTS: dynamo:test-table:2000 AWS_CONDITIONAL_PUT: dynamo:test-table:2000 + AWS_SERVER_SIDE_ENCRYPTION: aws:kms HTTP_URL: "http://localhost:8080" GOOGLE_BUCKET: test-bucket GOOGLE_SERVICE_ACCOUNT: "/tmp/gcs.json" @@ -142,6 +143,9 @@ jobs: aws --endpoint-url=http://localhost:4566 s3 mb s3://test-bucket aws --endpoint-url=http://localhost:4566 dynamodb create-table --table-name test-table --key-schema AttributeName=path,KeyType=HASH AttributeName=etag,KeyType=RANGE --attribute-definitions AttributeName=path,AttributeType=S AttributeName=etag,AttributeType=S --provisioned-throughput ReadCapacityUnits=5,WriteCapacityUnits=5 + KMS_KEY=$(aws --endpoint-url=http://localhost:4566 kms create-key --description "test key") + echo "AWS_SSE_KMS_KEY_ID=$(echo $KMS_KEY | jq -r .KeyMetadata.KeyId)" >> $GITHUB_ENV + - name: Configure Azurite (Azure emulation) # the magical connection string is from # https://docs.microsoft.com/en-us/azure/storage/common/storage-use-azurite?tabs=visual-studio#http-connection-strings From 0f1ff07e0194077d483a72a266d3150d555b561c Mon Sep 17 00:00:00 2001 From: Will Jones Date: Tue, 20 Feb 2024 11:07:03 -0800 Subject: [PATCH 5/6] feedback --- object_store/src/aws/builder.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/object_store/src/aws/builder.rs b/object_store/src/aws/builder.rs index d3baca86be2d..7e8d949b600b 100644 --- a/object_store/src/aws/builder.rs +++ b/object_store/src/aws/builder.rs @@ -1074,15 +1074,8 @@ impl std::fmt::Display for S3EncryptionType { /// /// Whether these headers are sent depends on both the kind of encryption set /// and the kind of request being made. -#[derive(Default, Clone)] -pub struct S3EncryptionHeaders(pub HeaderMap); - -impl std::fmt::Debug for S3EncryptionHeaders { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // TODO: if we take a user-provided key, hide the key from debug output. - f.debug_map().entries(self.0.iter()).finish() - } -} +#[derive(Default, Clone, Debug)] +pub struct S3EncryptionHeaders(HeaderMap); impl S3EncryptionHeaders { fn try_new( @@ -1091,6 +1084,8 @@ impl S3EncryptionHeaders { bucket_key_enabled: Option, ) -> Result { let mut headers = HeaderMap::new(); + // Note: if we later add support for SSE-C, we should be sure to use + // HeaderValue::set_sensitive to prevent the key from being logged. headers.insert( "x-amz-server-side-encryption", HeaderValue::from_static(encryption_type.into()), From 0cc547c21abd7805d995ba3a1aa71d418984e5f1 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Thu, 22 Feb 2024 09:40:43 -0800 Subject: [PATCH 6/6] fix clippy --- object_store/src/aws/builder.rs | 6 ++++++ object_store/src/aws/client.rs | 6 +++--- object_store/src/aws/mod.rs | 18 +++++++++--------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/object_store/src/aws/builder.rs b/object_store/src/aws/builder.rs index 7e8d949b600b..a578d1abbfae 100644 --- a/object_store/src/aws/builder.rs +++ b/object_store/src/aws/builder.rs @@ -1111,6 +1111,12 @@ impl S3EncryptionHeaders { } } +impl From for HeaderMap { + fn from(headers: S3EncryptionHeaders) -> Self { + headers.0 + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/object_store/src/aws/client.rs b/object_store/src/aws/client.rs index 3e074c21ad17..5f2cb0d0e15a 100644 --- a/object_store/src/aws/client.rs +++ b/object_store/src/aws/client.rs @@ -331,7 +331,7 @@ impl S3Client { let url = self.config.path_url(path); let mut builder = self.client.request(Method::PUT, url); if with_encryption_headers { - builder = builder.headers(self.config.encryption_headers.0.clone()); + builder = builder.headers(self.config.encryption_headers.clone().into()); } let mut payload_sha256 = None; @@ -492,7 +492,7 @@ impl S3Client { .client .request(Method::PUT, url) .header("x-amz-copy-source", source) - .headers(self.config.encryption_headers.0.clone()); + .headers(self.config.encryption_headers.clone().into()); Request { builder, @@ -510,7 +510,7 @@ impl S3Client { let response = self .client .request(Method::POST, url) - .headers(self.config.encryption_headers.0.clone()) + .headers(self.config.encryption_headers.clone().into()) .with_aws_sigv4(credential.authorizer(), None) .send_retry(&self.config.retry_config) .await diff --git a/object_store/src/aws/mod.rs b/object_store/src/aws/mod.rs index 5da56b9fed69..b11f4513b6df 100644 --- a/object_store/src/aws/mod.rs +++ b/object_store/src/aws/mod.rs @@ -376,6 +376,7 @@ mod tests { use super::*; use crate::{client::get::GetClient, tests::*}; use bytes::Bytes; + use hyper::HeaderMap; use tokio::io::AsyncWriteExt; const NON_EXISTENT_NAME: &str = "nonexistentname"; @@ -523,15 +524,14 @@ mod tests { let data = Bytes::from(vec![3u8; 1024]); - let encryption_headers = &store.client.config.encryption_headers; - let expected_encryption = if let Some(encryption_type) = - encryption_headers.0.get("x-amz-server-side-encryption") - { - encryption_type - } else { - eprintln!("Skipping S3 encryption test - encryption not configured"); - return; - }; + let encryption_headers: HeaderMap = store.client.config.encryption_headers.clone().into(); + let expected_encryption = + if let Some(encryption_type) = encryption_headers.get("x-amz-server-side-encryption") { + encryption_type + } else { + eprintln!("Skipping S3 encryption test - encryption not configured"); + return; + }; let locations = [ Path::from("test-encryption-1"),