Skip to content

Commit

Permalink
Allow opting out of request signing (#4927) (#4929)
Browse files Browse the repository at this point in the history
  • Loading branch information
tustvold authored Oct 16, 2023
1 parent bb8e42f commit 57cd094
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 18 deletions.
24 changes: 14 additions & 10 deletions object_store/src/aws/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ pub struct S3Config {
pub retry_config: RetryConfig,
pub client_options: ClientOptions,
pub sign_payload: bool,
pub skip_signature: bool,
pub checksum: Option<Checksum>,
pub copy_if_not_exists: Option<S3CopyIfNotExists>,
}
Expand Down Expand Up @@ -234,8 +235,11 @@ impl S3Client {
&self.config
}

async fn get_credential(&self) -> Result<Arc<AwsCredential>> {
self.config.credentials.get_credential().await
async fn get_credential(&self) -> Result<Option<Arc<AwsCredential>>> {
Ok(match self.config.skip_signature {
false => Some(self.config.credentials.get_credential().await?),
true => None,
})
}

/// Make an S3 PUT request <https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html>
Expand Down Expand Up @@ -271,7 +275,7 @@ impl S3Client {
let response = builder
.query(query)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -299,7 +303,7 @@ impl S3Client {
.request(Method::DELETE, url)
.query(query)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -390,7 +394,7 @@ impl S3Client {
.header(CONTENT_TYPE, "application/xml")
.body(body)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -459,7 +463,7 @@ impl S3Client {

builder
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -490,7 +494,7 @@ impl S3Client {
.client
.request(Method::POST, url)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -535,7 +539,7 @@ impl S3Client {
.query(&[("uploadId", upload_id)])
.body(body)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -567,7 +571,7 @@ impl GetClient for S3Client {
let response = builder
.with_get_options(options)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down Expand Up @@ -621,7 +625,7 @@ impl ListClient for S3Client {
.request(Method::GET, &url)
.query(&query)
.with_aws_sigv4(
credential.as_ref(),
credential.as_deref(),
&self.config.region,
"s3",
self.config.sign_payload,
Expand Down
21 changes: 13 additions & 8 deletions object_store/src/aws/credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ pub trait CredentialExt {
/// Sign a request <https://docs.aws.amazon.com/general/latest/gr/sigv4_signing.html>
fn with_aws_sigv4(
self,
credential: &AwsCredential,
credential: Option<&AwsCredential>,
region: &str,
service: &str,
sign_payload: bool,
Expand All @@ -302,20 +302,25 @@ pub trait CredentialExt {
impl CredentialExt for RequestBuilder {
fn with_aws_sigv4(
self,
credential: &AwsCredential,
credential: Option<&AwsCredential>,
region: &str,
service: &str,
sign_payload: bool,
payload_sha256: Option<&[u8]>,
) -> Self {
let (client, request) = self.build_split();
let mut request = request.expect("request valid");
match credential {
Some(credential) => {
let (client, request) = self.build_split();
let mut request = request.expect("request valid");

AwsAuthorizer::new(credential, service, region)
.with_sign_payload(sign_payload)
.authorize(&mut request, payload_sha256);
AwsAuthorizer::new(credential, service, region)
.with_sign_payload(sign_payload)
.authorize(&mut request, payload_sha256);

Self::from_parts(client, request)
Self::from_parts(client, request)
}
None => self,
}
}
}

Expand Down
44 changes: 44 additions & 0 deletions object_store/src/aws/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ pub struct AmazonS3Builder {
client_options: ClientOptions,
/// Credentials
credentials: Option<AwsCredentialProvider>,
/// Skip signing requests
skip_signature: ConfigValue<bool>,
/// Copy if not exists
copy_if_not_exists: Option<ConfigValue<S3CopyIfNotExists>>,
}
Expand Down Expand Up @@ -586,6 +588,9 @@ pub enum AmazonS3ConfigKey {
/// See [`S3CopyIfNotExists`]
CopyIfNotExists,

/// Skip signing request
SkipSignature,

/// Client options
Client(ClientConfigKey),
}
Expand All @@ -608,6 +613,7 @@ impl AsRef<str> for AmazonS3ConfigKey {
Self::ContainerCredentialsRelativeUri => {
"aws_container_credentials_relative_uri"
}
Self::SkipSignature => "aws_skip_signature",
Self::CopyIfNotExists => "copy_if_not_exists",
Self::Client(opt) => opt.as_ref(),
}
Expand Down Expand Up @@ -642,6 +648,7 @@ impl FromStr for AmazonS3ConfigKey {
"aws_container_credentials_relative_uri" => {
Ok(Self::ContainerCredentialsRelativeUri)
}
"aws_skip_signature" | "skip_signature" => Ok(Self::SkipSignature),
"copy_if_not_exists" => Ok(Self::CopyIfNotExists),
// Backwards compatibility
"aws_allow_http" => Ok(Self::Client(ClientConfigKey::AllowHttp)),
Expand Down Expand Up @@ -753,6 +760,7 @@ impl AmazonS3Builder {
AmazonS3ConfigKey::Client(key) => {
self.client_options = self.client_options.with_config(key, value)
}
AmazonS3ConfigKey::SkipSignature => self.skip_signature.parse(value),
AmazonS3ConfigKey::CopyIfNotExists => {
self.copy_if_not_exists = Some(ConfigValue::Deferred(value.into()))
}
Expand Down Expand Up @@ -823,6 +831,7 @@ impl AmazonS3Builder {
AmazonS3ConfigKey::ContainerCredentialsRelativeUri => {
self.container_credentials_relative_uri.clone()
}
AmazonS3ConfigKey::SkipSignature => Some(self.skip_signature.to_string()),
AmazonS3ConfigKey::CopyIfNotExists => {
self.copy_if_not_exists.as_ref().map(ToString::to_string)
}
Expand Down Expand Up @@ -977,6 +986,14 @@ impl AmazonS3Builder {
self
}

/// If enabled, [`AmazonS3`] will not fetch credentials and will not sign requests
///
/// This can be useful when interacting with public S3 buckets that deny authorized requests
pub fn with_skip_signature(mut self, skip_signature: bool) -> Self {
self.skip_signature = skip_signature.into();
self
}

/// Sets the [checksum algorithm] which has to be used for object integrity check during upload.
///
/// [checksum algorithm]: https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html
Expand Down Expand Up @@ -1146,6 +1163,7 @@ impl AmazonS3Builder {
retry_config: self.retry_config,
client_options: self.client_options,
sign_payload: !self.unsigned_payload.get()?,
skip_signature: self.skip_signature.get()?,
checksum,
copy_if_not_exists,
};
Expand Down Expand Up @@ -1505,4 +1523,30 @@ mod s3_resolve_bucket_region_tests {

assert!(result.is_err());
}

#[tokio::test]
#[ignore = "Tests shouldn't call use remote services by default"]
async fn test_disable_creds() {
// https://registry.opendata.aws/daylight-osm/
let v1 = AmazonS3Builder::new()
.with_bucket_name("daylight-map-distribution")
.with_region("us-west-1")
.with_access_key_id("local")
.with_secret_access_key("development")
.build()
.unwrap();

let prefix = Path::from("release");

v1.list_with_delimiter(Some(&prefix)).await.unwrap_err();

let v2 = AmazonS3Builder::new()
.with_bucket_name("daylight-map-distribution")
.with_region("us-west-1")
.with_skip_signature(true)
.build()
.unwrap();

v2.list_with_delimiter(Some(&prefix)).await.unwrap();
}
}

0 comments on commit 57cd094

Please sign in to comment.