From 65795e1001c8cf8147500afc7723ef513a225ad6 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Mon, 8 May 2023 16:07:25 +0800 Subject: [PATCH] fix(services/s3): Return error if credential load fail instead skip (#2233) * fix(services/s3): Return error if credential load fail instead skip Signed-off-by: Xuanwo * set_temporary for reqsign error Signed-off-by: Xuanwo * Fix OPENDAL_S3_ALLOW_ANONYMOUS Signed-off-by: Xuanwo * Bump MSRV to 1.65 Signed-off-by: Xuanwo --------- Signed-off-by: Xuanwo --- .github/workflows/ci.yml | 4 +- .github/workflows/service_test_s3.yml | 1 + Cargo.lock | 115 +++++++++++++++++++------- Cargo.toml | 2 +- core/Cargo.toml | 2 +- core/src/raw/http_util/error.rs | 1 + core/src/services/azblob/core.rs | 3 +- core/src/services/s3/backend.rs | 14 +++- core/src/services/s3/core.rs | 22 +++-- core/src/services/wasabi/backend.rs | 2 +- core/src/services/wasabi/core.rs | 18 ++-- 11 files changed, 130 insertions(+), 54 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9cf59d55835d..e89aba3da58a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,8 +103,8 @@ jobs: check_msrv: runs-on: ubuntu-latest env: - # OpenDAL's MSRV is 1.64. - OPENDAL_MSRV: "1.64" + # OpenDAL's MSRV is 1.65. + OPENDAL_MSRV: "1.65" steps: - uses: actions/checkout@v3 - name: Setup msrv of rust diff --git a/.github/workflows/service_test_s3.yml b/.github/workflows/service_test_s3.yml index dd71f731ad85..605bc8a88ad8 100644 --- a/.github/workflows/service_test_s3.yml +++ b/.github/workflows/service_test_s3.yml @@ -177,3 +177,4 @@ jobs: OPENDAL_S3_TEST: on OPENDAL_S3_BUCKET: test OPENDAL_S3_ENDPOINT: "http://127.0.0.1:9000" + OPENDAL_S3_ALLOW_ANONYMOUS: on diff --git a/Cargo.lock b/Cargo.lock index 29253a125e5f..d8d3d6dbb38a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -806,6 +806,28 @@ version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "520fbf3c07483f94e3e3ca9d0cfd913d7718ef2483d2cfd91c0d9e91474ab913" +[[package]] +name = "const-random" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "368a7a772ead6ce7e1de82bfb04c485f3db8ec744f72925af5735e29a22cc18e" +dependencies = [ + "const-random-macro", + "proc-macro-hack", +] + +[[package]] +name = "const-random-macro" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d7d6ab3c3a2282db210df5f02c4dab6e0a7057af0fb7ebd4070f30fe05c0ddb" +dependencies = [ + "getrandom 0.2.8", + "once_cell", + "proc-macro-hack", + "tiny-keccak", +] + [[package]] name = "const_fn_assert" version = "0.1.2" @@ -936,6 +958,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crunchy" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a81dae078cea95a014a339291cec439d2f232ebe854a9d672b796c6afafa9b7" + [[package]] name = "crypto-common" version = "0.1.6" @@ -1042,7 +1070,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "907076dfda823b0b36d2a1bb5f90c96660a5bbcd7729e10727f07858f22c4edc" dependencies = [ "cfg-if", - "hashbrown", + "hashbrown 0.12.3", "lock_api", "once_cell", "parking_lot_core 0.9.7", @@ -1075,9 +1103,9 @@ checksum = "eaa37046cc0f6c3cc6090fbdbf73ef0b8ef4cfcc37f6befc0020f63e8cf121e1" [[package]] name = "der" -version = "0.6.1" +version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1a467a65c5e759bce6e65eaf91cc29f466cdc57cb65777bd646872a8a1fd4de" +checksum = "05e58dffcdcc8ee7b22f0c1f71a69243d7c2d9ad87b5a14361f2424a1565c219" dependencies = [ "const-oid", "pem-rfc7468", @@ -1161,9 +1189,12 @@ dependencies = [ [[package]] name = "dlv-list" -version = "0.3.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0688c2a7f92e427f44895cd63841bff7b29f8d7a1648b9e7e07a4a365b2e1257" +checksum = "d529fd73d344663edfd598ccb3f344e46034db51ebd103518eae34338248ad73" +dependencies = [ + "const-random", +] [[package]] name = "doc-comment" @@ -1522,9 +1553,12 @@ name = "hashbrown" version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" -dependencies = [ - "ahash", -] + +[[package]] +name = "hashbrown" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43a3c133739dddd0d2990f9a4bdf8eb4b21ef50e4851ca85ab661199821d510e" [[package]] name = "hdfs-sys" @@ -1595,6 +1629,15 @@ dependencies = [ "digest", ] +[[package]] +name = "home" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5444c27eef6923071f7ebcc33e3444508466a76f7a2b93da00ed6e19f30c1ddb" +dependencies = [ + "windows-sys 0.48.0", +] + [[package]] name = "hostname" version = "0.3.1" @@ -1781,7 +1824,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1885e79c1fc4b10f0e172c475f458b7f7b93061064d98c3293e98c5ba0c8b399" dependencies = [ "autocfg", - "hashbrown", + "hashbrown 0.12.3", ] [[package]] @@ -2929,12 +2972,12 @@ dependencies = [ [[package]] name = "ordered-multimap" -version = "0.4.3" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ccd746e37177e1711c20dd619a1620f34f5c8b569c53590a72dedd5344d8924a" +checksum = "4ed8acf08e98e744e5384c8bc63ceb0364e68a6854187221c18df61c4797690e" dependencies = [ "dlv-list", - "hashbrown", + "hashbrown 0.13.2", ] [[package]] @@ -3044,9 +3087,9 @@ dependencies = [ [[package]] name = "pem-rfc7468" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24d159833a9105500e0398934e205e0773f0b27529557134ecfc51c27646adac" +checksum = "88b39c9bfcfc231068454382784bb460aae594343fb030d46e9f50a645418412" dependencies = [ "base64ct", ] @@ -3091,21 +3134,20 @@ checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" [[package]] name = "pkcs1" -version = "0.4.1" +version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eff33bdbdfc54cc98a2eca766ebdec3e1b8fb7387523d5c9c9a2891da856f719" +checksum = "c8ffb9f10fa047879315e6625af03c164b16962a5368d724ed16323b68ace47f" dependencies = [ "der", "pkcs8", "spki", - "zeroize", ] [[package]] name = "pkcs8" -version = "0.9.0" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9eca2c590a5f85da82668fa685c09ce2888b9430e83299debf1f34b65fd4a4ba" +checksum = "f950b2377845cebe5cf8b5165cb3cc1a5e0fa5cfa3e1f7f55707d8fd82e0a7b7" dependencies = [ "der", "spki", @@ -3251,6 +3293,12 @@ dependencies = [ "version_check", ] +[[package]] +name = "proc-macro-hack" +version = "0.5.20+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" + [[package]] name = "proc-macro2" version = "1.0.52" @@ -3666,19 +3714,18 @@ checksum = "456c603be3e8d448b072f410900c09faf164fbce2d480456f50eea6e25f9c848" [[package]] name = "reqsign" -version = "0.9.1" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3875de6d7d0468315194cb8332913aae0d6803cfd5c7f089dc174ff5350f7b29" +checksum = "9152cbebffeac8d76d0882eae64eef9ccdd3956b49e844b45d8f3de10ea1efd8" dependencies = [ "anyhow", "async-trait", "base64 0.21.0", - "bytes", "chrono", - "dirs 5.0.0", "form_urlencoded", "hex", "hmac", + "home", "http", "jsonwebtoken", "log", @@ -3784,11 +3831,12 @@ dependencies = [ [[package]] name = "rsa" -version = "0.8.2" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "55a77d189da1fee555ad95b7e50e7457d91c0e089ec68ca69ad2989413bbdab4" +checksum = "72f1471dbb4be5de45050e8ef7040625298ccb9efe941419ac2697088715925f" dependencies = [ "byteorder", + "const-oid", "digest", "num-bigint-dig", "num-integer", @@ -3805,9 +3853,9 @@ dependencies = [ [[package]] name = "rust-ini" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6d5f2436026b4f6e79dc829837d467cc7e9a55ee40e750d716713540715a2df" +checksum = "7e2a3bcec1f113553ef1c88aae6c020a369d03d55b58de9869a0908930385091" dependencies = [ "cfg-if", "ordered-multimap", @@ -4250,9 +4298,9 @@ dependencies = [ [[package]] name = "spki" -version = "0.6.0" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67cf02bbac7a337dc36e4f5a693db6c21e7863f45070f7064577eb4367a3212b" +checksum = "9d1e996ef02c474957d681f1b05213dfb0abab947b446a62d37770b23500184a" dependencies = [ "base64ct", "der", @@ -4481,6 +4529,15 @@ dependencies = [ "time-core", ] +[[package]] +name = "tiny-keccak" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c9d3793400a45f954c52e73d068316d76b6f4e36977e3fcebb13a2721e80237" +dependencies = [ + "crunchy", +] + [[package]] name = "tinytemplate" version = "1.2.1" diff --git a/Cargo.toml b/Cargo.toml index 3758de7bdb1a..23c124b433d5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ edition = "2021" homepage = "https://opendal.apache.org/" license = "Apache-2.0" repository = "https://github.com/apache/incubator-opendal" -rust-version = "1.64" +rust-version = "1.65" version = "0.33.3" [workspace.dependencies] diff --git a/core/Cargo.toml b/core/Cargo.toml index 166795101616..b00c051a07ef 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -198,7 +198,7 @@ redis = { version = "0.22", features = [ "tokio-comp", "connection-manager", ], optional = true } -reqsign = { version = "0.9.1", default-features = false, optional = true } +reqsign = { version = "0.10.0", default-features = false, optional = true } reqwest = { version = "0.11.13", features = [ "stream", ], default-features = false } diff --git a/core/src/raw/http_util/error.rs b/core/src/raw/http_util/error.rs index 72543768ff96..421970f66cd3 100644 --- a/core/src/raw/http_util/error.rs +++ b/core/src/raw/http_util/error.rs @@ -100,6 +100,7 @@ pub fn new_request_credential_error(err: anyhow::Error) -> Error { ErrorKind::Unexpected, "loading credentail to sign http request", ) + .set_temporary() .with_operation("reqsign::LoadCredential") .set_source(err) } diff --git a/core/src/services/azblob/core.rs b/core/src/services/azblob/core.rs index 62665a0e5480..94022fbcf970 100644 --- a/core/src/services/azblob/core.rs +++ b/core/src/services/azblob/core.rs @@ -19,6 +19,7 @@ use std::fmt; use std::fmt::Debug; use std::fmt::Formatter; use std::fmt::Write; +use std::time::Duration; use http::header::HeaderName; use http::header::CONTENT_LENGTH; @@ -83,7 +84,7 @@ impl AzblobCore { let cred = self.load_credential().await?; self.signer - .sign_query(req, &cred) + .sign_query(req, Duration::from_secs(3600), &cred) .map_err(new_request_sign_error) } diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs index 176db6b7ebf9..35819982bf10 100644 --- a/core/src/services/s3/backend.rs +++ b/core/src/services/s3/backend.rs @@ -302,6 +302,7 @@ pub struct S3Builder { disable_config_load: bool, disable_ec2_metadata: bool, + allow_anonymous: bool, enable_virtual_host_style: bool, http_client: Option, @@ -626,6 +627,13 @@ impl S3Builder { self } + /// Allow anonymous will allow opendal to send request without signing + /// when credentail is not loaded. + pub fn allow_anonymous(&mut self) -> &mut Self { + self.allow_anonymous = true; + self + } + /// Enable virtual host style so that opendal will send API requests /// in virtual host style instead of path style. /// @@ -756,6 +764,9 @@ impl Builder for S3Builder { map.get("enable_virtual_host_style") .filter(|v| *v == "on" || *v == "true") .map(|_| builder.enable_virtual_host_style()); + map.get("allow_anonymous") + .filter(|v| *v == "on" || *v == "true") + .map(|_| builder.allow_anonymous()); map.get("default_storage_class") .map(|v| builder.default_storage_class(v)); @@ -876,7 +887,7 @@ impl Builder for S3Builder { let endpoint = self.build_endpoint(®ion); debug!("backend use endpoint: {endpoint}"); - let mut loader = AwsLoader::new(client.client(), cfg).with_allow_anonymous(); + let mut loader = AwsLoader::new(client.client(), cfg); if self.disable_ec2_metadata { loader = loader.with_disable_ec2_metadata(); } @@ -906,6 +917,7 @@ impl Builder for S3Builder { server_side_encryption_customer_key, server_side_encryption_customer_key_md5, default_storage_class, + allow_anonymous: self.allow_anonymous, signer, loader, client, diff --git a/core/src/services/s3/core.rs b/core/src/services/s3/core.rs index e27fbbf3117c..0eca12668410 100644 --- a/core/src/services/s3/core.rs +++ b/core/src/services/s3/core.rs @@ -21,8 +21,6 @@ use std::fmt::Formatter; use std::fmt::Write; use std::time::Duration; -use backon::ExponentialBuilder; -use backon::Retryable; use bytes::Bytes; use http::header::HeaderName; use http::header::CACHE_CONTROL; @@ -34,7 +32,6 @@ use http::header::IF_NONE_MATCH; use http::HeaderValue; use http::Request; use http::Response; -use once_cell::sync::Lazy; use reqsign::AwsCredential; use reqsign::AwsLoader; use reqsign::AwsV4Signer; @@ -69,9 +66,6 @@ mod constants { pub const RESPONSE_CACHE_CONTROL: &str = "response-cache-control"; } -static BACKOFF: Lazy = - Lazy::new(|| ExponentialBuilder::default().with_jitter()); - pub struct S3Core { pub bucket: String, pub endpoint: String, @@ -82,6 +76,7 @@ pub struct S3Core { pub server_side_encryption_customer_key: Option, pub server_side_encryption_customer_key_md5: Option, pub default_storage_class: Option, + pub allow_anonymous: bool, pub signer: AwsV4Signer, pub loader: AwsLoader, @@ -102,15 +97,24 @@ impl Debug for S3Core { impl S3Core { /// If credential is not found, we will not sign the request. async fn load_credential(&self) -> Result> { - let cred = { || self.loader.load() } - .retry(&*BACKOFF) + let cred = self + .loader + .load() .await .map_err(new_request_credential_error)?; if let Some(cred) = cred { Ok(Some(cred)) - } else { + } else if self.allow_anonymous { + // If allow_anonymous has been set, we will not sign the request. Ok(None) + } else { + // Mark this error as temporary since it could be caused by AWS STS. + Err(Error::new( + ErrorKind::PermissionDenied, + "no valid credential found, please check configuration or try again", + ) + .set_temporary()) } } diff --git a/core/src/services/wasabi/backend.rs b/core/src/services/wasabi/backend.rs index 1882d75f7e2b..3260f6e9f004 100644 --- a/core/src/services/wasabi/backend.rs +++ b/core/src/services/wasabi/backend.rs @@ -851,7 +851,7 @@ impl Builder for WasabiBuilder { let endpoint = self.build_endpoint(®ion); debug!("backend use endpoint: {endpoint}"); - let mut loader = AwsLoader::new(client.client(), cfg).with_allow_anonymous(); + let mut loader = AwsLoader::new(client.client(), cfg); if self.disable_ec2_metadata { loader = loader.with_disable_ec2_metadata(); } diff --git a/core/src/services/wasabi/core.rs b/core/src/services/wasabi/core.rs index 4fdc385f1093..b604e7f5c651 100644 --- a/core/src/services/wasabi/core.rs +++ b/core/src/services/wasabi/core.rs @@ -21,8 +21,6 @@ use std::fmt::Formatter; use std::fmt::Write; use std::time::Duration; -use backon::ExponentialBuilder; -use backon::Retryable; use bytes::Bytes; use http::header::HeaderName; use http::header::CACHE_CONTROL; @@ -32,7 +30,6 @@ use http::header::CONTENT_TYPE; use http::HeaderValue; use http::Request; use http::Response; -use once_cell::sync::Lazy; use reqsign::AwsCredential; use reqsign::AwsLoader; use reqsign::AwsV4Signer; @@ -73,9 +70,6 @@ mod constants { pub const OVERWRITE: &str = "Overwrite"; } -static BACKOFF: Lazy = - Lazy::new(|| ExponentialBuilder::default().with_jitter()); - pub struct WasabiCore { pub bucket: String, pub endpoint: String, @@ -105,15 +99,21 @@ impl Debug for WasabiCore { impl WasabiCore { /// If credential is not found, we will not sign the request. async fn load_credential(&self) -> Result> { - let cred = { || self.loader.load() } - .retry(&*BACKOFF) + let cred = self + .loader + .load() .await .map_err(new_request_credential_error)?; if let Some(cred) = cred { Ok(Some(cred)) } else { - Ok(None) + // Mark this error as temporary since it could be caused by AWS STS. + Err(Error::new( + ErrorKind::PermissionDenied, + "no valid credential found, please check configuration or try again", + ) + .set_temporary()) } }