From aee28982f9ebd9cb6a8eb74d9be18dcf92b1283a Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Sun, 10 Aug 2025 09:37:48 -0700 Subject: [PATCH 01/15] Sample generic implementation --- Cargo.toml | 4 +- src/azure/builder.rs | 95 ++++++++++++++++++++++++++++++++------------ src/crypto.rs | 95 ++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/parse.rs | 12 ++++-- 5 files changed, 177 insertions(+), 30 deletions(-) create mode 100644 src/crypto.rs diff --git a/Cargo.toml b/Cargo.toml index eeb14ddf..e7d97bdc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,7 @@ serde = { version = "1.0", default-features = false, features = ["derive"], opti serde_json = { version = "1.0", default-features = false, features = ["std"], optional = true } serde_urlencoded = { version = "0.7", optional = true } tokio = { version = "1.29.0", features = ["sync", "macros", "rt", "time", "io-util"] } +openssl = "0.10.73" [target.'cfg(target_family="unix")'.dev-dependencies] nix = { version = "0.30.0", features = ["fs"] } @@ -70,7 +71,8 @@ wasm-bindgen-futures = "0.4.18" [features] default = ["fs"] -cloud = ["serde", "serde_json", "quick-xml", "hyper", "reqwest", "reqwest/stream", "chrono/serde", "base64", "rand", "ring", "http-body-util", "form_urlencoded", "serde_urlencoded"] +ring = ["dep:ring"] +cloud = ["serde", "serde_json", "quick-xml", "hyper", "reqwest", "reqwest/stream", "chrono/serde", "base64", "rand", "dep:ring", "http-body-util", "form_urlencoded", "serde_urlencoded"] azure = ["cloud", "httparse"] fs = ["walkdir"] gcp = ["cloud", "rustls-pemfile"] diff --git a/src/azure/builder.rs b/src/azure/builder.rs index 182bdf04..de2d7704 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -23,6 +23,9 @@ use crate::azure::credential::{ use crate::azure::{AzureCredential, AzureCredentialProvider, MicrosoftAzure, STORE}; use crate::client::{http_connector, HttpConnector, TokenCredentialProvider}; use crate::config::ConfigValue; +#[cfg(feature = "ring")] +use crate::crypto::ring_crypto::RingProvider; +use crate::crypto::CryptoProvider; use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; use percent_encoding::percent_decode_str; use serde::{Deserialize, Serialize}; @@ -118,8 +121,10 @@ impl From for crate::Error { /// .with_container_name(BUCKET_NAME) /// .build(); /// ``` -#[derive(Default, Clone)] -pub struct MicrosoftAzureBuilder { +#[derive(Clone)] +pub struct MicrosoftAzureBuilder { + /// Crypto provider. See [`crypto::CryptoProvider`] + crypto_provider: T, /// Account name account_name: Option, /// Access key @@ -182,6 +187,13 @@ pub struct MicrosoftAzureBuilder { http_connector: Option>, } +#[cfg(feature = "ring")] +impl Default for MicrosoftAzureBuilder { + fn default() -> Self { + Self::new(RingProvider::default()) + } +} + /// Configuration keys for [`MicrosoftAzureBuilder`] /// /// Configuration via keys can be done via [`MicrosoftAzureBuilder::with_config`] @@ -476,7 +488,7 @@ impl FromStr for AzureConfigKey { } } -impl std::fmt::Debug for MicrosoftAzureBuilder { +impl std::fmt::Debug for MicrosoftAzureBuilder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -486,10 +498,41 @@ impl std::fmt::Debug for MicrosoftAzureBuilder { } } -impl MicrosoftAzureBuilder { +impl MicrosoftAzureBuilder { /// Create a new [`MicrosoftAzureBuilder`] with default values. - pub fn new() -> Self { - Default::default() + pub fn new(provider: T) -> Self { + Self { + crypto_provider: provider, + account_name: None, + access_key: None, + container_name: None, + bearer_token: None, + client_id: None, + client_secret: None, + tenant_id: None, + sas_query_pairs: None, + sas_key: None, + authority_host: None, + url: None, + use_emulator: false.into(), + endpoint: None, + msi_endpoint: None, + object_id: None, + msi_resource_id: None, + federated_token_file: None, + use_azure_cli: false.into(), + retry_config: RetryConfig::default(), + client_options: ClientOptions::default(), + credentials: None, + skip_signature: false.into(), + use_fabric_endpoint: false.into(), + disable_tagging: false.into(), + fabric_token_service_url: None, + fabric_workload_host: None, + fabric_session_token: None, + fabric_cluster_identifier: None, + http_connector: None, + } } /// Create an instance of [`MicrosoftAzureBuilder`] with values pre-populated from environment variables. @@ -509,23 +552,23 @@ impl MicrosoftAzureBuilder { /// .with_container_name("foo") /// .build(); /// ``` - pub fn from_env() -> Self { - let mut builder = Self::default(); + pub fn from_env(mut self) -> Self { + // let mut builder = Self::default(); for (os_key, os_value) in std::env::vars_os() { if let (Some(key), Some(value)) = (os_key.to_str(), os_value.to_str()) { if key.starts_with("AZURE_") { if let Ok(config_key) = key.to_ascii_lowercase().parse() { - builder = builder.with_config(config_key, value); + self = self.with_config(config_key, value); } } } } if let Ok(text) = std::env::var(MSI_ENDPOINT_ENV_KEY) { - builder = builder.with_msi_endpoint(text); + self = self.with_msi_endpoint(text); } - builder + self } /// Parse available connection info form a well-known storage URL. @@ -1095,7 +1138,7 @@ mod tests { #[test] fn azure_blob_test_urls() { - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("abfss://file_system@account.dfs.core.windows.net/") .unwrap(); @@ -1103,7 +1146,7 @@ mod tests { assert_eq!(builder.container_name, Some("file_system".to_string())); assert!(!builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("az://container@account.dfs.core.windows.net/path-part/file") .unwrap(); @@ -1111,7 +1154,7 @@ mod tests { assert_eq!(builder.container_name, Some("container".to_string())); assert!(!builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("abfss://file_system@account.dfs.fabric.microsoft.com/") .unwrap(); @@ -1119,19 +1162,19 @@ mod tests { assert_eq!(builder.container_name, Some("file_system".to_string())); assert!(builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder.parse_url("abfs://container/path").unwrap(); assert_eq!(builder.container_name, Some("container".to_string())); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder.parse_url("az://container").unwrap(); assert_eq!(builder.container_name, Some("container".to_string())); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder.parse_url("az://container/path").unwrap(); assert_eq!(builder.container_name, Some("container".to_string())); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("https://account.dfs.core.windows.net/") .unwrap(); @@ -1139,7 +1182,7 @@ mod tests { assert!(!builder.use_fabric_endpoint.get().unwrap()); let mut builder = - MicrosoftAzureBuilder::new().with_container_name("explicit_container_name"); + MicrosoftAzureBuilder::default().with_container_name("explicit_container_name"); builder .parse_url("https://account.blob.core.windows.net/") .unwrap(); @@ -1150,7 +1193,7 @@ mod tests { ); assert!(!builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("https://account.blob.core.windows.net/container") .unwrap(); @@ -1158,7 +1201,7 @@ mod tests { assert_eq!(builder.container_name, Some("container".to_string())); assert!(!builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("https://account.dfs.fabric.microsoft.com/") .unwrap(); @@ -1166,7 +1209,7 @@ mod tests { assert_eq!(builder.container_name, None); assert!(builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("https://account.dfs.fabric.microsoft.com/container") .unwrap(); @@ -1174,7 +1217,7 @@ mod tests { assert_eq!(builder.container_name.as_deref(), Some("container")); assert!(builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("https://account.blob.fabric.microsoft.com/") .unwrap(); @@ -1182,7 +1225,7 @@ mod tests { assert_eq!(builder.container_name, None); assert!(builder.use_fabric_endpoint.get().unwrap()); - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); builder .parse_url("https://account.blob.fabric.microsoft.com/container") .unwrap(); @@ -1199,7 +1242,7 @@ mod tests { "https://blob.mydomain/", "https://blob.foo.dfs.core.windows.net/", ]; - let mut builder = MicrosoftAzureBuilder::new(); + let mut builder = MicrosoftAzureBuilder::default(); for case in err_cases { builder.parse_url(case).unwrap_err(); } @@ -1218,7 +1261,7 @@ mod tests { let builder = options .into_iter() - .fold(MicrosoftAzureBuilder::new(), |builder, (key, value)| { + .fold(MicrosoftAzureBuilder::default(), |builder, (key, value)| { builder.with_config(key.parse().unwrap(), value) }); assert_eq!(builder.client_id.unwrap(), azure_client_id); diff --git a/src/crypto.rs b/src/crypto.rs new file mode 100644 index 00000000..34c231c2 --- /dev/null +++ b/src/crypto.rs @@ -0,0 +1,95 @@ +use std::fmt::Display; + +type DynError = Box; + +#[derive(Debug)] +pub struct CryptoError { + inner: DynError, +} + +impl Display for CryptoError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Crypto error: {}", self.inner) + } +} + +impl std::error::Error for CryptoError {} + +impl From for CryptoError { + fn from(err: DynError) -> Self { + CryptoError { inner: err } + } +} + +/// TODO(jakedern): Docs +pub trait CryptoProvider { + fn digest_sha256(bytes: &[u8]) -> Result, CryptoError>; + fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, CryptoError>; + fn hex_digest(bytes: &[u8]) -> Result { + let digest = Self::digest_sha256(bytes)?; + Ok(hex_encode(digest.as_ref())) + } +} + +pub(crate) fn hex_encode(bytes: &[u8]) -> String { + use std::fmt::Write; + let mut out = String::with_capacity(bytes.len() * 2); + for byte in bytes { + let _ = write!(out, "{byte:02x}"); + } + out +} + +/// TODO(jakedern): Docs +#[cfg(feature = "ring")] +pub mod ring_crypto { + use super::{CryptoError, CryptoProvider}; + + pub struct RingProvider; + + impl CryptoProvider for RingProvider { + fn digest_sha256(bytes: &[u8]) -> Result, CryptoError> { + let digest = ring::digest::digest(&ring::digest::SHA256, bytes); + Ok(digest) + } + + fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, CryptoError> { + let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret); + let tag = ring::hmac::sign(&key, bytes); + Ok(tag) + } + } + + impl Default for RingProvider { + fn default() -> Self { + RingProvider + } + } +} + +// pub mod openssl_crypto { +// use openssl::hash::MessageDigest; +// use openssl::pkey::PKey; +// use openssl::sign::Signer; +// +// use super::{CryptoError, CryptoProvider, DynError}; +// +// pub struct OpenSslCrypto; +// +// impl CryptoProvider for OpenSslCrypto { +// fn digest_sha256(bytes: &[u8]) -> Result, CryptoError> { +// let digest = openssl::hash::hash(MessageDigest::sha256(), bytes) +// .map_err(|e| Box::new(e) as DynError)?; +// Ok(digest) +// } +// +// fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, CryptoError> { +// let key = PKey::hmac(secret).map_err(|e| Box::new(e) as DynError)?; +// let mut signer = +// Signer::new(MessageDigest::sha256(), &key).map_err(|e| Box::new(e) as DynError)?; +// signer.update(bytes).map_err(|e| Box::new(e) as DynError)?; +// let hmac = signer.sign_to_vec().map_err(|e| Box::new(e) as DynError)?; +// Ok(hmac) +// } +// } +// } diff --git a/src/lib.rs b/src/lib.rs index bb9f8b10..0d758aa2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -513,6 +513,7 @@ pub mod azure; pub mod buffered; #[cfg(not(target_arch = "wasm32"))] pub mod chunked; +pub mod crypto; pub mod delimited; #[cfg(feature = "gcp")] pub mod gcp; diff --git a/src/parse.rs b/src/parse.rs index b1f653c5..73217961 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -142,7 +142,7 @@ impl ObjectStoreScheme { macro_rules! builder_opts { ($builder:ty, $url:expr, $options:expr) => {{ let builder = $options.into_iter().fold( - <$builder>::new().with_url($url.to_string()), + <$builder>::default().with_url($url.to_string()), |builder, (key, value)| match key.as_ref().parse() { Ok(k) => builder.with_config(k, value), Err(_) => builder, @@ -209,9 +209,15 @@ where ObjectStoreScheme::GoogleCloudStorage => { builder_opts!(crate::gcp::GoogleCloudStorageBuilder, url, _options) } - #[cfg(feature = "azure")] + #[cfg(all(feature = "azure", feature = "ring"))] ObjectStoreScheme::MicrosoftAzure => { - builder_opts!(crate::azure::MicrosoftAzureBuilder, url, _options) + use crate::crypto::ring_crypto::RingProvider; + + builder_opts!( + crate::azure::MicrosoftAzureBuilder, + url, + _options + ) } #[cfg(feature = "http")] ObjectStoreScheme::Http => { From 8c56123ba415cd551477f1ecf3885d0a2d7439a5 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Mon, 11 Aug 2025 10:28:26 -0700 Subject: [PATCH 02/15] More crypto plumbing --- src/azure/builder.rs | 17 +++++++------ src/azure/client.rs | 52 ++++++++++++++++++++++----------------- src/azure/credential.rs | 54 ++++++++++++++++++++++++++++------------- src/azure/mod.rs | 30 +++++++++++++---------- src/crypto.rs | 27 +++++++++++---------- 5 files changed, 108 insertions(+), 72 deletions(-) diff --git a/src/azure/builder.rs b/src/azure/builder.rs index de2d7704..c61b86b1 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -29,6 +29,7 @@ use crate::crypto::CryptoProvider; use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; use percent_encoding::percent_decode_str; use serde::{Deserialize, Serialize}; +use std::marker::PhantomData; use std::str::FromStr; use std::sync::Arc; use url::Url; @@ -123,8 +124,7 @@ impl From for crate::Error { /// ``` #[derive(Clone)] pub struct MicrosoftAzureBuilder { - /// Crypto provider. See [`crypto::CryptoProvider`] - crypto_provider: T, + _crypto_provider: PhantomData, /// Account name account_name: Option, /// Access key @@ -190,7 +190,7 @@ pub struct MicrosoftAzureBuilder { #[cfg(feature = "ring")] impl Default for MicrosoftAzureBuilder { fn default() -> Self { - Self::new(RingProvider::default()) + Self::new() } } @@ -500,9 +500,9 @@ impl std::fmt::Debug for MicrosoftAzureBuilder { impl MicrosoftAzureBuilder { /// Create a new [`MicrosoftAzureBuilder`] with default values. - pub fn new(provider: T) -> Self { + pub fn new() -> Self { Self { - crypto_provider: provider, + _crypto_provider: PhantomData::default(), account_name: None, access_key: None, container_name: None, @@ -945,7 +945,7 @@ impl MicrosoftAzureBuilder { } /// Configure a connection to container with given name on Microsoft Azure Blob store. - pub fn build(mut self) -> Result { + pub fn build(mut self) -> Result> { if let Some(url) = self.url.take() { self.parse_url(&url)?; } @@ -1092,7 +1092,10 @@ impl MicrosoftAzureBuilder { let http_client = http.connect(&config.client_options)?; let client = Arc::new(AzureClient::new(config, http_client)); - Ok(MicrosoftAzure { client }) + Ok(MicrosoftAzure { + client, + _crypto_provider: PhantomData::default(), + }) } } diff --git a/src/azure/client.rs b/src/azure/client.rs index 1e96aac0..548cb1b1 100644 --- a/src/azure/client.rs +++ b/src/azure/client.rs @@ -24,6 +24,7 @@ use crate::client::header::{get_put_result, HeaderConfig}; use crate::client::list::ListClient; use crate::client::retry::{RetryContext, RetryExt}; use crate::client::{GetOptionsExt, HttpClient, HttpError, HttpRequest, HttpResponse}; +use crate::crypto::CryptoProvider; use crate::list::{PaginatedListOptions, PaginatedListResult}; use crate::multipart::PartId; use crate::util::{deserialize_rfc1123, GetRange}; @@ -43,6 +44,7 @@ use http::{ use rand::Rng as _; use serde::{Deserialize, Serialize}; use std::collections::HashMap; +use std::marker::PhantomData; use std::sync::Arc; use std::time::Duration; use url::Url; @@ -264,7 +266,7 @@ impl PutRequest<'_> { Self { builder, ..self } } - async fn send(self) -> Result { + async fn send(self) -> Result { let credential = self.config.get_credential().await?; let sensitive = credential .as_deref() @@ -273,7 +275,7 @@ impl PutRequest<'_> { let response = self .builder .header(CONTENT_LENGTH, self.payload.content_length()) - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(self.idempotent) @@ -507,15 +509,20 @@ async fn parse_blob_batch_delete_body( } #[derive(Debug)] -pub(crate) struct AzureClient { +pub(crate) struct AzureClient { + _crypto_provider: PhantomData, config: AzureConfig, client: HttpClient, } -impl AzureClient { +impl AzureClient { /// create a new instance of [AzureClient] pub(crate) fn new(config: AzureConfig, client: HttpClient) -> Self { - Self { config, client } + Self { + _crypto_provider: PhantomData::default(), + config, + client, + } } /// Returns the config @@ -569,7 +576,7 @@ impl AzureClient { } }; - let response = builder.header(&BLOB_TYPE, "BlockBlob").send().await?; + let response = builder.header(&BLOB_TYPE, "BlockBlob").send::().await?; Ok(get_put_result(response.headers(), VERSION_HEADER) .map_err(|source| Error::Metadata { source })?) } @@ -588,7 +595,7 @@ impl AzureClient { self.put_request(path, payload) .query(&[("comp", "block"), ("blockid", &block_id)]) .idempotent(true) - .send() + .send::() .await?; Ok(PartId { content_id }) @@ -620,7 +627,7 @@ impl AzureClient { .with_extensions(extensions) .query(&[("comp", "blocklist")]) .idempotent(true) - .send() + .send::() .await?; Ok(get_put_result(response.headers(), VERSION_HEADER) @@ -628,10 +635,10 @@ impl AzureClient { } /// Make an Azure Delete request - pub(crate) async fn delete_request( + pub(crate) async fn delete_request( &self, path: &Path, - query: &T, + query: &Q, ) -> Result<()> { let credential = self.get_credential().await?; let url = self.config.path_url(path); @@ -644,7 +651,7 @@ impl AzureClient { .delete(url.as_str()) .query(query) .header(&DELETE_SNAPSHOTS, "include") - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -676,7 +683,7 @@ impl AzureClient { // Each subrequest must be authorized individually [1] and we use // the CredentialExt for this. // [1]: https://learn.microsoft.com/en-us/rest/api/storageservices/blob-batch?tabs=microsoft-entra-id#request-body - .with_azure_authorization(credential, &self.config.account) + .with_azure_authorization::(credential, &self.config.account) .into_parts() .1 .unwrap(); @@ -723,7 +730,7 @@ impl AzureClient { ) .header(CONTENT_LENGTH, HeaderValue::from(body_bytes.len())) .body(body_bytes) - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .send_retry(&self.config.retry_config) .await .map_err(|source| Error::BulkDeleteRequest { source })?; @@ -768,7 +775,7 @@ impl AzureClient { .map(|c| c.sensitive_request()) .unwrap_or_default(); builder - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(overwrite) @@ -809,7 +816,7 @@ impl AzureClient { .post(url.as_str()) .body(body) .query(&[("restype", "service"), ("comp", "userdelegationkey")]) - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(true) @@ -831,7 +838,7 @@ impl AzureClient { /// /// Depending on the type of credential, this will either use the account key or a user delegation key. /// Since delegation keys are acquired ad-hoc, the signer aloows for signing multiple urls with the same key. - pub(crate) async fn signer(&self, expires_in: Duration) -> Result { + pub(crate) async fn signer(&self, expires_in: Duration) -> Result> { let credential = self.get_credential().await?; let signed_start = chrono::Utc::now(); let signed_expiry = signed_start + expires_in; @@ -873,7 +880,7 @@ impl AzureClient { .client .get(url.as_str()) .query(&[("comp", "tags")]) - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -888,7 +895,7 @@ impl AzureClient { } #[async_trait] -impl GetClient for AzureClient { +impl GetClient for AzureClient { const STORE: &'static str = STORE; const HEADER_CONFIG: HeaderConfig = HeaderConfig { @@ -943,7 +950,7 @@ impl GetClient for AzureClient { let response = builder .with_get_options(options) - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable_request() .sensitive(sensitive) .send(ctx) @@ -968,7 +975,7 @@ impl GetClient for AzureClient { } #[async_trait] -impl ListClient for Arc { +impl ListClient for Arc> { /// Make an Azure List request async fn list_request( &self, @@ -1016,7 +1023,7 @@ impl ListClient for Arc { .get(url.as_str()) .extensions(opts.extensions) .query(&query) - .with_azure_authorization(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account) .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -1205,6 +1212,7 @@ pub(crate) struct UserDelegationKey { #[cfg(test)] mod tests { use super::*; + use crate::crypto::ring_crypto::RingProvider; use crate::StaticCredentialProvider; use bytes::Bytes; use regex::bytes::Regex; @@ -1417,7 +1425,7 @@ mod tests { client_options: Default::default(), }; - let client = AzureClient::new(config, HttpClient::new(Client::new())); + let client = AzureClient::::new(config, HttpClient::new(Client::new())); let credential = client.get_credential().await.unwrap(); let paths = &[Path::from("a"), Path::from("b"), Path::from("c")]; diff --git a/src/azure/credential.rs b/src/azure/credential.rs index 37208842..1da48dff 100644 --- a/src/azure/credential.rs +++ b/src/azure/credential.rs @@ -21,7 +21,7 @@ use crate::client::builder::{add_query_pairs, HttpRequestBuilder}; use crate::client::retry::RetryExt; use crate::client::token::{TemporaryToken, TokenCache}; use crate::client::{CredentialProvider, HttpClient, HttpError, HttpRequest, TokenProvider}; -use crate::util::hmac_sha256; +use crate::crypto::{self, CryptoProvider}; use crate::RetryConfig; use async_trait::async_trait; use base64::prelude::{BASE64_STANDARD, BASE64_URL_SAFE_NO_PAD}; @@ -37,6 +37,7 @@ use serde::Deserialize; use std::borrow::Cow; use std::collections::HashMap; use std::fmt::Debug; +use std::marker::PhantomData; use std::ops::Deref; use std::process::Command; use std::str; @@ -94,6 +95,9 @@ pub enum Error { #[error("Generating SAS keys with SAS tokens auth is not supported")] SASforSASNotSupported, + + #[error("Error performing cryptographic operations")] + CryptoError(crypto::Error), } pub(crate) type Result = std::result::Result; @@ -107,6 +111,12 @@ impl From for crate::Error { } } +impl From for Error { + fn from(value: crypto::Error) -> Self { + Error::CryptoError(value) + } +} + /// A shared Azure Storage Account Key #[derive(Debug, Clone, Eq, PartialEq)] pub struct AzureAccessKey(Vec); @@ -163,7 +173,8 @@ pub mod authority_hosts { pub const AZURE_PUBLIC_CLOUD: &str = "https://login.microsoftonline.com"; } -pub(crate) struct AzureSigner { +pub(crate) struct AzureSigner { + _crypto_provider: PhantomData, signing_key: AzureAccessKey, start: DateTime, end: DateTime, @@ -171,7 +182,7 @@ pub(crate) struct AzureSigner { delegation_key: Option, } -impl AzureSigner { +impl AzureSigner { pub(crate) fn new( signing_key: AzureAccessKey, account: String, @@ -180,6 +191,7 @@ impl AzureSigner { delegation_key: Option, ) -> Self { Self { + _crypto_provider: PhantomData::default(), signing_key, account, start, @@ -200,7 +212,7 @@ impl AzureSigner { ), None => string_to_sign_service_sas(url, method, &self.account, &self.start, &self.end), }; - let auth = hmac_sha256(&self.signing_key.0, str_to_sign); + let auth = T::hmac_sha256(&self.signing_key.0, &str_to_sign.as_bytes())?; url.query_pairs_mut().extend_pairs(query_pairs); url.query_pairs_mut() .append_pair("sig", BASE64_STANDARD.encode(auth).as_str()); @@ -222,34 +234,36 @@ fn add_date_and_version_headers(request: &mut HttpRequest) { /// Authorize a [`HttpRequest`] with an [`AzureAuthorizer`] #[derive(Debug)] -pub struct AzureAuthorizer<'a> { +pub struct AzureAuthorizer<'a, T> { + _crypto_provider: PhantomData, credential: &'a AzureCredential, account: &'a str, } -impl<'a> AzureAuthorizer<'a> { +impl<'a, T: CryptoProvider> AzureAuthorizer<'a, T> { /// Create a new [`AzureAuthorizer`] pub fn new(credential: &'a AzureCredential, account: &'a str) -> Self { AzureAuthorizer { + _crypto_provider: PhantomData::default(), credential, account, } } /// Authorize `request` - pub fn authorize(&self, request: &mut HttpRequest) { + pub fn authorize(&self, request: &mut HttpRequest) -> Result<()> { add_date_and_version_headers(request); match self.credential { AzureCredential::AccessKey(key) => { let url = Url::parse(&request.uri().to_string()).unwrap(); - let signature = generate_authorization( + let signature = generate_authorization::( request.headers(), &url, request.method(), self.account, key, - ); + )?; // "signature" is a base 64 encoded string so it should never // contain illegal characters @@ -268,13 +282,15 @@ impl<'a> AzureAuthorizer<'a> { add_query_pairs(request.uri_mut(), query_pairs); } } + + Ok(()) } } pub(crate) trait CredentialExt { /// Apply authorization to requests against azure storage accounts /// - fn with_azure_authorization( + fn with_azure_authorization( self, credential: &Option>, account: &str, @@ -282,7 +298,7 @@ pub(crate) trait CredentialExt { } impl CredentialExt for HttpRequestBuilder { - fn with_azure_authorization( + fn with_azure_authorization( self, credential: &Option>, account: &str, @@ -292,7 +308,7 @@ impl CredentialExt for HttpRequestBuilder { match credential.as_deref() { Some(credential) => { - AzureAuthorizer::new(credential, account).authorize(&mut request); + AzureAuthorizer::::new(credential, account).authorize(&mut request); } None => { add_date_and_version_headers(&mut request); @@ -305,16 +321,20 @@ impl CredentialExt for HttpRequestBuilder { /// Generate signed key for authorization via access keys /// -fn generate_authorization( +fn generate_authorization( h: &HeaderMap, u: &Url, method: &Method, account: &str, key: &AzureAccessKey, -) -> String { +) -> Result { let str_to_sign = string_to_sign(h, u, method, account); - let auth = hmac_sha256(&key.0, str_to_sign); - format!("SharedKey {}:{}", account, BASE64_STANDARD.encode(auth)) + let auth = T::hmac_sha256(&key.0, str_to_sign.as_bytes())?; + Ok(format!( + "SharedKey {}:{}", + account, + BASE64_STANDARD.encode(auth) + )) } fn add_if_exists<'a>(h: &'a HeaderMap, key: &HeaderName) -> &'a str { @@ -1190,7 +1210,7 @@ mod tests { let server = MockServer::new().await; let endpoint = server.url(); - let store = MicrosoftAzureBuilder::new() + let store = MicrosoftAzureBuilder::default() .with_account("test") .with_container_name("test") .with_allow_http(true) diff --git a/src/azure/mod.rs b/src/azure/mod.rs index f65bf9f3..25bb1339 100644 --- a/src/azure/mod.rs +++ b/src/azure/mod.rs @@ -23,6 +23,7 @@ //! //! Unused blocks will automatically be dropped after 7 days. use crate::{ + crypto::CryptoProvider, multipart::{MultipartStore, PartId}, path::Path, signer::Signer, @@ -32,9 +33,9 @@ use crate::{ use async_trait::async_trait; use futures::stream::{BoxStream, StreamExt, TryStreamExt}; use reqwest::Method; -use std::fmt::Debug; use std::sync::Arc; use std::time::Duration; +use std::{fmt::Debug, marker::PhantomData}; use url::Url; use crate::client::get::GetClientExt; @@ -58,11 +59,12 @@ const STORE: &str = "MicrosoftAzure"; /// Interface for [Microsoft Azure Blob Storage](https://azure.microsoft.com/en-us/services/storage/blobs/). #[derive(Debug)] -pub struct MicrosoftAzure { - client: Arc, +pub struct MicrosoftAzure { + client: Arc>, + _crypto_provider: PhantomData, } -impl MicrosoftAzure { +impl MicrosoftAzure { /// Returns the [`AzureCredentialProvider`] used by [`MicrosoftAzure`] pub fn credentials(&self) -> &AzureCredentialProvider { &self.client.config().credentials @@ -74,7 +76,7 @@ impl MicrosoftAzure { } } -impl std::fmt::Display for MicrosoftAzure { +impl std::fmt::Display for MicrosoftAzure { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -86,7 +88,7 @@ impl std::fmt::Display for MicrosoftAzure { } #[async_trait] -impl ObjectStore for MicrosoftAzure { +impl ObjectStore for MicrosoftAzure { async fn put_opts( &self, location: &Path, @@ -157,7 +159,7 @@ impl ObjectStore for MicrosoftAzure { } #[async_trait] -impl Signer for MicrosoftAzure { +impl Signer for MicrosoftAzure { /// Create a URL containing the relevant [Service SAS] query parameters that authorize a request /// via `method` to the resource at `path` valid for the duration specified in `expires_in`. /// @@ -260,7 +262,7 @@ impl MultipartUpload for AzureMultiPartUpload { } #[async_trait] -impl MultipartStore for MicrosoftAzure { +impl MultipartStore for MicrosoftAzure { async fn create_multipart(&self, _: &Path) -> Result { Ok(String::new()) } @@ -294,7 +296,7 @@ impl MultipartStore for MicrosoftAzure { } #[async_trait] -impl PaginatedListStore for MicrosoftAzure { +impl PaginatedListStore for MicrosoftAzure { async fn list_paginated( &self, prefix: Option<&str>, @@ -307,6 +309,7 @@ impl PaginatedListStore for MicrosoftAzure { #[cfg(test)] mod tests { use super::*; + use crate::crypto::ring_crypto::RingProvider; use crate::integration::*; use crate::tests::*; use bytes::Bytes; @@ -314,7 +317,7 @@ mod tests { #[tokio::test] async fn azure_blob_test() { maybe_skip_integration!(); - let integration = MicrosoftAzureBuilder::from_env().build().unwrap(); + let integration = MicrosoftAzureBuilder::default().from_env().build().unwrap(); put_get_delete_list(&integration).await; get_opts(&integration).await; @@ -332,8 +335,9 @@ mod tests { let validate = !integration.client.config().disable_tagging; tagging( - Arc::new(MicrosoftAzure { + Arc::new(MicrosoftAzure:: { client: Arc::clone(&integration.client), + _crypto_provider: PhantomData::default(), }), validate, |p| { @@ -357,7 +361,7 @@ mod tests { let client_id = std::env::var("AZURE_CLIENT_ID").unwrap(); let client_secret = std::env::var("AZURE_CLIENT_SECRET").unwrap(); let tenant_id = std::env::var("AZURE_TENANT_ID").unwrap(); - let integration = MicrosoftAzureBuilder::new() + let integration = MicrosoftAzureBuilder::default() .with_account(account) .with_container_name(container) .with_client_id(client_id) @@ -386,7 +390,7 @@ mod tests { let azure_client_id = "object_store:fake_access_key_id".to_string(); let azure_storage_account_name = "object_store:fake_secret_key".to_string(); let azure_storage_token = "object_store:fake_default_region".to_string(); - let builder = MicrosoftAzureBuilder::new() + let builder = MicrosoftAzureBuilder::default() .with_config(AzureConfigKey::ClientId, &azure_client_id) .with_config(AzureConfigKey::AccountName, &azure_storage_account_name) .with_config(AzureConfigKey::Token, &azure_storage_token); diff --git a/src/crypto.rs b/src/crypto.rs index 34c231c2..28fd1f67 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -1,31 +1,31 @@ -use std::fmt::Display; +use std::fmt::{Debug, Display}; type DynError = Box; #[derive(Debug)] -pub struct CryptoError { +pub struct Error { inner: DynError, } -impl Display for CryptoError { +impl Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "Crypto error: {}", self.inner) } } -impl std::error::Error for CryptoError {} +impl std::error::Error for Error {} -impl From for CryptoError { +impl From for Error { fn from(err: DynError) -> Self { - CryptoError { inner: err } + Error { inner: err } } } /// TODO(jakedern): Docs -pub trait CryptoProvider { - fn digest_sha256(bytes: &[u8]) -> Result, CryptoError>; - fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, CryptoError>; - fn hex_digest(bytes: &[u8]) -> Result { +pub trait CryptoProvider: Send + Sync + Debug + 'static { + fn digest_sha256(bytes: &[u8]) -> Result, Error>; + fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, Error>; + fn hex_digest(bytes: &[u8]) -> Result { let digest = Self::digest_sha256(bytes)?; Ok(hex_encode(digest.as_ref())) } @@ -43,17 +43,18 @@ pub(crate) fn hex_encode(bytes: &[u8]) -> String { /// TODO(jakedern): Docs #[cfg(feature = "ring")] pub mod ring_crypto { - use super::{CryptoError, CryptoProvider}; + use super::{CryptoProvider, Error}; + #[derive(Debug, Clone, Copy)] pub struct RingProvider; impl CryptoProvider for RingProvider { - fn digest_sha256(bytes: &[u8]) -> Result, CryptoError> { + fn digest_sha256(bytes: &[u8]) -> Result, Error> { let digest = ring::digest::digest(&ring::digest::SHA256, bytes); Ok(digest) } - fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, CryptoError> { + fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, Error> { let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret); let tag = ring::hmac::sign(&key, bytes); Ok(tag) From 225ef2281e5d1ed00fb9999579c908bdfc6afc98 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Mon, 11 Aug 2025 10:37:30 -0700 Subject: [PATCH 03/15] More plumbing --- src/azure/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/azure/mod.rs b/src/azure/mod.rs index 25bb1339..a4bf5f02 100644 --- a/src/azure/mod.rs +++ b/src/azure/mod.rs @@ -64,7 +64,7 @@ pub struct MicrosoftAzure { _crypto_provider: PhantomData, } -impl MicrosoftAzure { +impl MicrosoftAzure { /// Returns the [`AzureCredentialProvider`] used by [`MicrosoftAzure`] pub fn credentials(&self) -> &AzureCredentialProvider { &self.client.config().credentials @@ -76,7 +76,7 @@ impl MicrosoftAzure { } } -impl std::fmt::Display for MicrosoftAzure { +impl std::fmt::Display for MicrosoftAzure { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -220,21 +220,21 @@ impl Signer for MicrosoftAzure { /// complete -> PUT block list /// abort -> No equivalent; blocks are simply dropped after 7 days #[derive(Debug)] -struct AzureMultiPartUpload { +struct AzureMultiPartUpload { part_idx: usize, - state: Arc, + state: Arc>, opts: PutMultipartOptions, } #[derive(Debug)] -struct UploadState { +struct UploadState { location: Path, parts: Parts, - client: Arc, + client: Arc>, } #[async_trait] -impl MultipartUpload for AzureMultiPartUpload { +impl MultipartUpload for AzureMultiPartUpload { fn put_part(&mut self, data: PutPayload) -> UploadPart { let idx = self.part_idx; self.part_idx += 1; From bfc24393645fc2716a13a818dff72c2025d46ea6 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Mon, 11 Aug 2025 10:49:57 -0700 Subject: [PATCH 04/15] Make request building fallible --- src/azure/client.rs | 28 ++++++++++++---------- src/azure/credential.rs | 10 ++++---- src/crypto.rs | 53 +++++++++++++++++++++-------------------- 3 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/azure/client.rs b/src/azure/client.rs index 548cb1b1..9948bd10 100644 --- a/src/azure/client.rs +++ b/src/azure/client.rs @@ -275,7 +275,7 @@ impl PutRequest<'_> { let response = self .builder .header(CONTENT_LENGTH, self.payload.content_length()) - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(self.idempotent) @@ -651,7 +651,7 @@ impl AzureClient { .delete(url.as_str()) .query(query) .header(&DELETE_SNAPSHOTS, "include") - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -669,7 +669,7 @@ impl AzureClient { boundary: &str, paths: &[Path], credential: &Option>, - ) -> Vec { + ) -> Result> { let mut body_bytes = Vec::with_capacity(paths.len() * 2048); for (idx, path) in paths.iter().enumerate() { @@ -683,7 +683,7 @@ impl AzureClient { // Each subrequest must be authorized individually [1] and we use // the CredentialExt for this. // [1]: https://learn.microsoft.com/en-us/rest/api/storageservices/blob-batch?tabs=microsoft-entra-id#request-body - .with_azure_authorization::(credential, &self.config.account) + .with_azure_authorization::(credential, &self.config.account)? .into_parts() .1 .unwrap(); @@ -701,7 +701,7 @@ impl AzureClient { extend(&mut body_bytes, boundary.as_bytes()); extend(&mut body_bytes, b"--"); extend(&mut body_bytes, b"\r\n"); - body_bytes + Ok(body_bytes) } pub(crate) async fn bulk_delete_request(&self, paths: Vec) -> Result>> { @@ -715,7 +715,7 @@ impl AzureClient { let random_bytes = rand::random::<[u8; 16]>(); // 128 bits let boundary = format!("batch_{}", BASE64_STANDARD_NO_PAD.encode(random_bytes)); - let body_bytes = self.build_bulk_delete_body(&boundary, &paths, &credential); + let body_bytes = self.build_bulk_delete_body(&boundary, &paths, &credential)?; // Send multipart request let url = self.config.path_url(&Path::from("/")); @@ -730,7 +730,7 @@ impl AzureClient { ) .header(CONTENT_LENGTH, HeaderValue::from(body_bytes.len())) .body(body_bytes) - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .send_retry(&self.config.retry_config) .await .map_err(|source| Error::BulkDeleteRequest { source })?; @@ -775,7 +775,7 @@ impl AzureClient { .map(|c| c.sensitive_request()) .unwrap_or_default(); builder - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(overwrite) @@ -816,7 +816,7 @@ impl AzureClient { .post(url.as_str()) .body(body) .query(&[("restype", "service"), ("comp", "userdelegationkey")]) - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(true) @@ -880,7 +880,7 @@ impl AzureClient { .client .get(url.as_str()) .query(&[("comp", "tags")]) - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -950,7 +950,7 @@ impl GetClient for AzureClient { let response = builder .with_get_options(options) - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable_request() .sensitive(sensitive) .send(ctx) @@ -1023,7 +1023,7 @@ impl ListClient for Arc> { .get(url.as_str()) .extensions(opts.extensions) .query(&query) - .with_azure_authorization::(&credential, &self.config.account) + .with_azure_authorization::(&credential, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -1432,7 +1432,9 @@ mod tests { let boundary = "batch_statictestboundary".to_string(); - let body_bytes = client.build_bulk_delete_body(&boundary, paths, &credential); + let body_bytes = client + .build_bulk_delete_body(&boundary, paths, &credential) + .unwrap(); // Replace Date header value with a static date let re = Regex::new("Date:[^\r]+").unwrap(); diff --git a/src/azure/credential.rs b/src/azure/credential.rs index 1da48dff..23b05705 100644 --- a/src/azure/credential.rs +++ b/src/azure/credential.rs @@ -287,14 +287,14 @@ impl<'a, T: CryptoProvider> AzureAuthorizer<'a, T> { } } -pub(crate) trait CredentialExt { +pub(crate) trait CredentialExt: Sized { /// Apply authorization to requests against azure storage accounts /// fn with_azure_authorization( self, credential: &Option>, account: &str, - ) -> Self; + ) -> Result; } impl CredentialExt for HttpRequestBuilder { @@ -302,20 +302,20 @@ impl CredentialExt for HttpRequestBuilder { self, credential: &Option>, account: &str, - ) -> Self { + ) -> Result { let (client, request) = self.into_parts(); let mut request = request.expect("request valid"); match credential.as_deref() { Some(credential) => { - AzureAuthorizer::::new(credential, account).authorize(&mut request); + AzureAuthorizer::::new(credential, account).authorize(&mut request)?; } None => { add_date_and_version_headers(&mut request); } } - Self::from_parts(client, request) + Ok(Self::from_parts(client, request)) } } diff --git a/src/crypto.rs b/src/crypto.rs index 28fd1f67..664834cc 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -68,29 +68,30 @@ pub mod ring_crypto { } } -// pub mod openssl_crypto { -// use openssl::hash::MessageDigest; -// use openssl::pkey::PKey; -// use openssl::sign::Signer; -// -// use super::{CryptoError, CryptoProvider, DynError}; -// -// pub struct OpenSslCrypto; -// -// impl CryptoProvider for OpenSslCrypto { -// fn digest_sha256(bytes: &[u8]) -> Result, CryptoError> { -// let digest = openssl::hash::hash(MessageDigest::sha256(), bytes) -// .map_err(|e| Box::new(e) as DynError)?; -// Ok(digest) -// } -// -// fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, CryptoError> { -// let key = PKey::hmac(secret).map_err(|e| Box::new(e) as DynError)?; -// let mut signer = -// Signer::new(MessageDigest::sha256(), &key).map_err(|e| Box::new(e) as DynError)?; -// signer.update(bytes).map_err(|e| Box::new(e) as DynError)?; -// let hmac = signer.sign_to_vec().map_err(|e| Box::new(e) as DynError)?; -// Ok(hmac) -// } -// } -// } +pub mod openssl_crypto { + use openssl::hash::MessageDigest; + use openssl::pkey::PKey; + use openssl::sign::Signer; + + use super::{CryptoProvider, DynError, Error}; + + #[derive(Debug, Clone, Copy)] + pub struct OpenSslCrypto; + + impl CryptoProvider for OpenSslCrypto { + fn digest_sha256(bytes: &[u8]) -> Result, Error> { + let digest = openssl::hash::hash(MessageDigest::sha256(), bytes) + .map_err(|e| Box::new(e) as DynError)?; + Ok(digest) + } + + fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, Error> { + let key = PKey::hmac(secret).map_err(|e| Box::new(e) as DynError)?; + let mut signer = + Signer::new(MessageDigest::sha256(), &key).map_err(|e| Box::new(e) as DynError)?; + signer.update(bytes).map_err(|e| Box::new(e) as DynError)?; + let hmac = signer.sign_to_vec().map_err(|e| Box::new(e) as DynError)?; + Ok(hmac) + } + } +} From aacabd335880f82d49cbcd72cea210b29acfe21c Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Tue, 12 Aug 2025 16:40:45 -0700 Subject: [PATCH 05/15] Scaffold object safe implementation --- src/azure/builder.rs | 79 +++++++++++++++++------------- src/azure/client.rs | 104 +++++++++++++++++++++++++++++----------- src/azure/credential.rs | 58 ++++++++++++---------- src/azure/mod.rs | 34 ++++++------- src/crypto.rs | 89 ++++++++++++++++++++-------------- src/parse.rs | 8 +--- 6 files changed, 226 insertions(+), 146 deletions(-) diff --git a/src/azure/builder.rs b/src/azure/builder.rs index c61b86b1..71684f46 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -23,13 +23,10 @@ use crate::azure::credential::{ use crate::azure::{AzureCredential, AzureCredentialProvider, MicrosoftAzure, STORE}; use crate::client::{http_connector, HttpConnector, TokenCredentialProvider}; use crate::config::ConfigValue; -#[cfg(feature = "ring")] -use crate::crypto::ring_crypto::RingProvider; -use crate::crypto::CryptoProvider; +use crate::crypto::CryptoProviderRef; use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; use percent_encoding::percent_decode_str; use serde::{Deserialize, Serialize}; -use std::marker::PhantomData; use std::str::FromStr; use std::sync::Arc; use url::Url; @@ -91,6 +88,9 @@ enum Error { #[error("Configuration key: '{}' is not known.", key)] UnknownConfigurationKey { key: String }, + + #[error("Missing crypto provider. Please enabled the default crypto provider or configure one explicitly.")] + MissingCryptoProvider {}, } impl From for crate::Error { @@ -123,8 +123,9 @@ impl From for crate::Error { /// .build(); /// ``` #[derive(Clone)] -pub struct MicrosoftAzureBuilder { - _crypto_provider: PhantomData, +pub struct MicrosoftAzureBuilder { + /// Crypto provider + crypto_provider: Option, /// Account name account_name: Option, /// Access key @@ -187,13 +188,6 @@ pub struct MicrosoftAzureBuilder { http_connector: Option>, } -#[cfg(feature = "ring")] -impl Default for MicrosoftAzureBuilder { - fn default() -> Self { - Self::new() - } -} - /// Configuration keys for [`MicrosoftAzureBuilder`] /// /// Configuration via keys can be done via [`MicrosoftAzureBuilder::with_config`] @@ -488,7 +482,7 @@ impl FromStr for AzureConfigKey { } } -impl std::fmt::Debug for MicrosoftAzureBuilder { +impl std::fmt::Debug for MicrosoftAzureBuilder { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -498,11 +492,17 @@ impl std::fmt::Debug for MicrosoftAzureBuilder { } } -impl MicrosoftAzureBuilder { +impl Default for MicrosoftAzureBuilder { + fn default() -> Self { + Self::new() + } +} + +impl MicrosoftAzureBuilder { /// Create a new [`MicrosoftAzureBuilder`] with default values. pub fn new() -> Self { - Self { - _crypto_provider: PhantomData::default(), + let mut b = Self { + crypto_provider: None, account_name: None, access_key: None, container_name: None, @@ -514,25 +514,33 @@ impl MicrosoftAzureBuilder { sas_key: None, authority_host: None, url: None, - use_emulator: false.into(), + use_emulator: ConfigValue::default(), endpoint: None, msi_endpoint: None, object_id: None, msi_resource_id: None, federated_token_file: None, - use_azure_cli: false.into(), - retry_config: RetryConfig::default(), - client_options: ClientOptions::default(), - credentials: None, - skip_signature: false.into(), - use_fabric_endpoint: false.into(), - disable_tagging: false.into(), + use_azure_cli: ConfigValue::default(), + skip_signature: ConfigValue::default(), + use_fabric_endpoint: ConfigValue::default(), + disable_tagging: ConfigValue::default(), fabric_token_service_url: None, fabric_workload_host: None, fabric_session_token: None, fabric_cluster_identifier: None, + retry_config: RetryConfig::default(), + client_options: ClientOptions::default(), + credentials: None, http_connector: None, - } + }; + + #[cfg(feature = "ring")] + { + use crate::crypto::ring_crypto::RingProvider; + b = b.with_crypto(Arc::new(RingProvider::default())); + }; + + b } /// Create an instance of [`MicrosoftAzureBuilder`] with values pre-populated from environment variables. @@ -571,6 +579,12 @@ impl MicrosoftAzureBuilder { self } + /// TODO(jakedern): Docs + pub fn with_crypto(mut self, crypto_provider: CryptoProviderRef) -> Self { + self.crypto_provider = Some(crypto_provider); + self + } + /// Parse available connection info form a well-known storage URL. /// /// The supported url schemes are: @@ -945,7 +959,7 @@ impl MicrosoftAzureBuilder { } /// Configure a connection to container with given name on Microsoft Azure Blob store. - pub fn build(mut self) -> Result> { + pub fn build(mut self) -> Result { if let Some(url) = self.url.take() { self.parse_url(&url)?; } @@ -1089,13 +1103,14 @@ impl MicrosoftAzureBuilder { credentials: auth, }; + let crypto_provider = self + .crypto_provider + .ok_or(Error::MissingCryptoProvider {})?; + let http_client = http.connect(&config.client_options)?; - let client = Arc::new(AzureClient::new(config, http_client)); + let client = Arc::new(AzureClient::new(config, http_client, crypto_provider)); - Ok(MicrosoftAzure { - client, - _crypto_provider: PhantomData::default(), - }) + Ok(MicrosoftAzure { client }) } } diff --git a/src/azure/client.rs b/src/azure/client.rs index 9948bd10..e242e443 100644 --- a/src/azure/client.rs +++ b/src/azure/client.rs @@ -24,7 +24,7 @@ use crate::client::header::{get_put_result, HeaderConfig}; use crate::client::list::ListClient; use crate::client::retry::{RetryContext, RetryExt}; use crate::client::{GetOptionsExt, HttpClient, HttpError, HttpRequest, HttpResponse}; -use crate::crypto::CryptoProvider; +use crate::crypto::{CryptoProvider, CryptoProviderRef}; use crate::list::{PaginatedListOptions, PaginatedListResult}; use crate::multipart::PartId; use crate::util::{deserialize_rfc1123, GetRange}; @@ -44,7 +44,6 @@ use http::{ use rand::Rng as _; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use std::marker::PhantomData; use std::sync::Arc; use std::time::Duration; use url::Url; @@ -200,6 +199,7 @@ impl AzureConfig { /// A builder for a put request allowing customisation of the headers and query string struct PutRequest<'a> { + crypto_provider: &'a dyn CryptoProvider, path: &'a Path, config: &'a AzureConfig, payload: PutPayload, @@ -266,7 +266,7 @@ impl PutRequest<'_> { Self { builder, ..self } } - async fn send(self) -> Result { + async fn send(self) -> Result { let credential = self.config.get_credential().await?; let sensitive = credential .as_deref() @@ -275,7 +275,7 @@ impl PutRequest<'_> { let response = self .builder .header(CONTENT_LENGTH, self.payload.content_length()) - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization(&credential, self.crypto_provider, &self.config.account)? .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(self.idempotent) @@ -509,17 +509,21 @@ async fn parse_blob_batch_delete_body( } #[derive(Debug)] -pub(crate) struct AzureClient { - _crypto_provider: PhantomData, +pub(crate) struct AzureClient { + crypto_provider: CryptoProviderRef, config: AzureConfig, client: HttpClient, } -impl AzureClient { +impl AzureClient { /// create a new instance of [AzureClient] - pub(crate) fn new(config: AzureConfig, client: HttpClient) -> Self { + pub(crate) fn new( + config: AzureConfig, + client: HttpClient, + crypto_provider: CryptoProviderRef, + ) -> Self { Self { - _crypto_provider: PhantomData::default(), + crypto_provider, config, client, } @@ -534,11 +538,17 @@ impl AzureClient { self.config.get_credential().await } - fn put_request<'a>(&'a self, path: &'a Path, payload: PutPayload) -> PutRequest<'a> { + fn put_request<'a>( + &'a self, + path: &'a Path, + payload: PutPayload, + crypto_provider: &'a dyn CryptoProvider, + ) -> PutRequest<'a> { let url = self.config.path_url(path); let builder = self.client.request(Method::PUT, url.as_str()); PutRequest { + crypto_provider, path, builder, payload, @@ -562,7 +572,7 @@ impl AzureClient { } = opts; let builder = self - .put_request(path, payload) + .put_request(path, payload, self.crypto_provider.as_ref()) .with_attributes(attributes) .with_extensions(extensions) .with_tags(tags); @@ -576,7 +586,7 @@ impl AzureClient { } }; - let response = builder.header(&BLOB_TYPE, "BlockBlob").send::().await?; + let response = builder.header(&BLOB_TYPE, "BlockBlob").send().await?; Ok(get_put_result(response.headers(), VERSION_HEADER) .map_err(|source| Error::Metadata { source })?) } @@ -592,10 +602,10 @@ impl AzureClient { let content_id = format!("{part_idx:032x}"); let block_id = BASE64_STANDARD.encode(&content_id); - self.put_request(path, payload) + self.put_request(path, payload, self.crypto_provider.as_ref()) .query(&[("comp", "block"), ("blockid", &block_id)]) .idempotent(true) - .send::() + .send() .await?; Ok(PartId { content_id }) @@ -621,13 +631,13 @@ impl AzureClient { let payload = BlockList { blocks }.to_xml().into(); let response = self - .put_request(path, payload) + .put_request(path, payload, self.crypto_provider.as_ref()) .with_attributes(attributes) .with_tags(tags) .with_extensions(extensions) .query(&[("comp", "blocklist")]) .idempotent(true) - .send::() + .send() .await?; Ok(get_put_result(response.headers(), VERSION_HEADER) @@ -651,7 +661,11 @@ impl AzureClient { .delete(url.as_str()) .query(query) .header(&DELETE_SNAPSHOTS, "include") - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -683,7 +697,11 @@ impl AzureClient { // Each subrequest must be authorized individually [1] and we use // the CredentialExt for this. // [1]: https://learn.microsoft.com/en-us/rest/api/storageservices/blob-batch?tabs=microsoft-entra-id#request-body - .with_azure_authorization::(credential, &self.config.account)? + .with_azure_authorization( + credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .into_parts() .1 .unwrap(); @@ -730,7 +748,11 @@ impl AzureClient { ) .header(CONTENT_LENGTH, HeaderValue::from(body_bytes.len())) .body(body_bytes) - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .send_retry(&self.config.retry_config) .await .map_err(|source| Error::BulkDeleteRequest { source })?; @@ -775,7 +797,11 @@ impl AzureClient { .map(|c| c.sensitive_request()) .unwrap_or_default(); builder - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(overwrite) @@ -816,7 +842,11 @@ impl AzureClient { .post(url.as_str()) .body(body) .query(&[("restype", "service"), ("comp", "userdelegationkey")]) - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .retryable(&self.config.retry_config) .sensitive(sensitive) .idempotent(true) @@ -838,7 +868,7 @@ impl AzureClient { /// /// Depending on the type of credential, this will either use the account key or a user delegation key. /// Since delegation keys are acquired ad-hoc, the signer aloows for signing multiple urls with the same key. - pub(crate) async fn signer(&self, expires_in: Duration) -> Result> { + pub(crate) async fn signer(&self, expires_in: Duration) -> Result { let credential = self.get_credential().await?; let signed_start = chrono::Utc::now(); let signed_expiry = signed_start + expires_in; @@ -854,6 +884,7 @@ impl AzureClient { signed_start, signed_expiry, Some(key), + self.crypto_provider.clone(), )) } Some(AzureCredential::AccessKey(key)) => Ok(AzureSigner::new( @@ -862,6 +893,7 @@ impl AzureClient { signed_start, signed_expiry, None, + self.crypto_provider.clone(), )), None => Err(Error::SASwithSkipSignature.into()), _ => Err(Error::SASforSASNotSupported.into()), @@ -880,7 +912,11 @@ impl AzureClient { .client .get(url.as_str()) .query(&[("comp", "tags")]) - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -895,7 +931,7 @@ impl AzureClient { } #[async_trait] -impl GetClient for AzureClient { +impl GetClient for AzureClient { const STORE: &'static str = STORE; const HEADER_CONFIG: HeaderConfig = HeaderConfig { @@ -950,7 +986,11 @@ impl GetClient for AzureClient { let response = builder .with_get_options(options) - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .retryable_request() .sensitive(sensitive) .send(ctx) @@ -975,7 +1015,7 @@ impl GetClient for AzureClient { } #[async_trait] -impl ListClient for Arc> { +impl ListClient for Arc { /// Make an Azure List request async fn list_request( &self, @@ -1023,7 +1063,11 @@ impl ListClient for Arc> { .get(url.as_str()) .extensions(opts.extensions) .query(&query) - .with_azure_authorization::(&credential, &self.config.account)? + .with_azure_authorization( + &credential, + self.crypto_provider.as_ref(), + &self.config.account, + )? .retryable(&self.config.retry_config) .sensitive(sensitive) .send() @@ -1425,7 +1469,11 @@ mod tests { client_options: Default::default(), }; - let client = AzureClient::::new(config, HttpClient::new(Client::new())); + let client = AzureClient::new( + config, + HttpClient::new(Client::new()), + Arc::from(RingProvider::default()), + ); let credential = client.get_credential().await.unwrap(); let paths = &[Path::from("a"), Path::from("b"), Path::from("c")]; diff --git a/src/azure/credential.rs b/src/azure/credential.rs index 23b05705..5a25b6c1 100644 --- a/src/azure/credential.rs +++ b/src/azure/credential.rs @@ -21,7 +21,7 @@ use crate::client::builder::{add_query_pairs, HttpRequestBuilder}; use crate::client::retry::RetryExt; use crate::client::token::{TemporaryToken, TokenCache}; use crate::client::{CredentialProvider, HttpClient, HttpError, HttpRequest, TokenProvider}; -use crate::crypto::{self, CryptoProvider}; +use crate::crypto::{CryptoProvider, CryptoProviderRef}; use crate::RetryConfig; use async_trait::async_trait; use base64::prelude::{BASE64_STANDARD, BASE64_URL_SAFE_NO_PAD}; @@ -37,7 +37,6 @@ use serde::Deserialize; use std::borrow::Cow; use std::collections::HashMap; use std::fmt::Debug; -use std::marker::PhantomData; use std::ops::Deref; use std::process::Command; use std::str; @@ -97,7 +96,7 @@ pub enum Error { SASforSASNotSupported, #[error("Error performing cryptographic operations")] - CryptoError(crypto::Error), + Crypto(Box), } pub(crate) type Result = std::result::Result; @@ -111,12 +110,6 @@ impl From for crate::Error { } } -impl From for Error { - fn from(value: crypto::Error) -> Self { - Error::CryptoError(value) - } -} - /// A shared Azure Storage Account Key #[derive(Debug, Clone, Eq, PartialEq)] pub struct AzureAccessKey(Vec); @@ -173,8 +166,8 @@ pub mod authority_hosts { pub const AZURE_PUBLIC_CLOUD: &str = "https://login.microsoftonline.com"; } -pub(crate) struct AzureSigner { - _crypto_provider: PhantomData, +pub(crate) struct AzureSigner { + crypto_provider: CryptoProviderRef, signing_key: AzureAccessKey, start: DateTime, end: DateTime, @@ -182,16 +175,17 @@ pub(crate) struct AzureSigner { delegation_key: Option, } -impl AzureSigner { +impl AzureSigner { pub(crate) fn new( signing_key: AzureAccessKey, account: String, start: DateTime, end: DateTime, delegation_key: Option, + crypto_provider: CryptoProviderRef, ) -> Self { Self { - _crypto_provider: PhantomData::default(), + crypto_provider: crypto_provider, signing_key, account, start, @@ -212,7 +206,10 @@ impl AzureSigner { ), None => string_to_sign_service_sas(url, method, &self.account, &self.start, &self.end), }; - let auth = T::hmac_sha256(&self.signing_key.0, &str_to_sign.as_bytes())?; + let auth = self + .crypto_provider + .hmac_sha256(&self.signing_key.0, &str_to_sign.as_bytes()) + .map_err(|e| Error::Crypto(Box::new(e)))?; url.query_pairs_mut().extend_pairs(query_pairs); url.query_pairs_mut() .append_pair("sig", BASE64_STANDARD.encode(auth).as_str()); @@ -234,17 +231,21 @@ fn add_date_and_version_headers(request: &mut HttpRequest) { /// Authorize a [`HttpRequest`] with an [`AzureAuthorizer`] #[derive(Debug)] -pub struct AzureAuthorizer<'a, T> { - _crypto_provider: PhantomData, +pub struct AzureAuthorizer<'a> { + crypto_provider: &'a dyn CryptoProvider, credential: &'a AzureCredential, account: &'a str, } -impl<'a, T: CryptoProvider> AzureAuthorizer<'a, T> { +impl<'a> AzureAuthorizer<'a> { /// Create a new [`AzureAuthorizer`] - pub fn new(credential: &'a AzureCredential, account: &'a str) -> Self { + pub fn new( + credential: &'a AzureCredential, + account: &'a str, + crypto_provider: &'a dyn CryptoProvider, + ) -> Self { AzureAuthorizer { - _crypto_provider: PhantomData::default(), + crypto_provider, credential, account, } @@ -257,12 +258,13 @@ impl<'a, T: CryptoProvider> AzureAuthorizer<'a, T> { match self.credential { AzureCredential::AccessKey(key) => { let url = Url::parse(&request.uri().to_string()).unwrap(); - let signature = generate_authorization::( + let signature = generate_authorization( request.headers(), &url, request.method(), self.account, key, + self.crypto_provider, )?; // "signature" is a base 64 encoded string so it should never @@ -290,17 +292,19 @@ impl<'a, T: CryptoProvider> AzureAuthorizer<'a, T> { pub(crate) trait CredentialExt: Sized { /// Apply authorization to requests against azure storage accounts /// - fn with_azure_authorization( + fn with_azure_authorization( self, credential: &Option>, + crypto_provider: &dyn CryptoProvider, account: &str, ) -> Result; } impl CredentialExt for HttpRequestBuilder { - fn with_azure_authorization( + fn with_azure_authorization( self, credential: &Option>, + crypto_provider: &dyn CryptoProvider, account: &str, ) -> Result { let (client, request) = self.into_parts(); @@ -308,7 +312,8 @@ impl CredentialExt for HttpRequestBuilder { match credential.as_deref() { Some(credential) => { - AzureAuthorizer::::new(credential, account).authorize(&mut request)?; + AzureAuthorizer::new(credential, account, crypto_provider) + .authorize(&mut request)?; } None => { add_date_and_version_headers(&mut request); @@ -321,15 +326,18 @@ impl CredentialExt for HttpRequestBuilder { /// Generate signed key for authorization via access keys /// -fn generate_authorization( +fn generate_authorization( h: &HeaderMap, u: &Url, method: &Method, account: &str, key: &AzureAccessKey, + crypto_provider: &dyn CryptoProvider, ) -> Result { let str_to_sign = string_to_sign(h, u, method, account); - let auth = T::hmac_sha256(&key.0, str_to_sign.as_bytes())?; + let auth = crypto_provider + .hmac_sha256(&key.0, str_to_sign.as_bytes()) + .map_err(|e| Error::Crypto(Box::new(e)))?; Ok(format!( "SharedKey {}:{}", account, diff --git a/src/azure/mod.rs b/src/azure/mod.rs index a4bf5f02..74d2c170 100644 --- a/src/azure/mod.rs +++ b/src/azure/mod.rs @@ -23,7 +23,6 @@ //! //! Unused blocks will automatically be dropped after 7 days. use crate::{ - crypto::CryptoProvider, multipart::{MultipartStore, PartId}, path::Path, signer::Signer, @@ -33,9 +32,9 @@ use crate::{ use async_trait::async_trait; use futures::stream::{BoxStream, StreamExt, TryStreamExt}; use reqwest::Method; +use std::fmt::Debug; use std::sync::Arc; use std::time::Duration; -use std::{fmt::Debug, marker::PhantomData}; use url::Url; use crate::client::get::GetClientExt; @@ -59,12 +58,11 @@ const STORE: &str = "MicrosoftAzure"; /// Interface for [Microsoft Azure Blob Storage](https://azure.microsoft.com/en-us/services/storage/blobs/). #[derive(Debug)] -pub struct MicrosoftAzure { - client: Arc>, - _crypto_provider: PhantomData, +pub struct MicrosoftAzure { + client: Arc, } -impl MicrosoftAzure { +impl MicrosoftAzure { /// Returns the [`AzureCredentialProvider`] used by [`MicrosoftAzure`] pub fn credentials(&self) -> &AzureCredentialProvider { &self.client.config().credentials @@ -76,7 +74,7 @@ impl MicrosoftAzure { } } -impl std::fmt::Display for MicrosoftAzure { +impl std::fmt::Display for MicrosoftAzure { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -88,7 +86,7 @@ impl std::fmt::Display for MicrosoftAzure { } #[async_trait] -impl ObjectStore for MicrosoftAzure { +impl ObjectStore for MicrosoftAzure { async fn put_opts( &self, location: &Path, @@ -159,7 +157,7 @@ impl ObjectStore for MicrosoftAzure { } #[async_trait] -impl Signer for MicrosoftAzure { +impl Signer for MicrosoftAzure { /// Create a URL containing the relevant [Service SAS] query parameters that authorize a request /// via `method` to the resource at `path` valid for the duration specified in `expires_in`. /// @@ -220,21 +218,21 @@ impl Signer for MicrosoftAzure { /// complete -> PUT block list /// abort -> No equivalent; blocks are simply dropped after 7 days #[derive(Debug)] -struct AzureMultiPartUpload { +struct AzureMultiPartUpload { part_idx: usize, - state: Arc>, + state: Arc, opts: PutMultipartOptions, } #[derive(Debug)] -struct UploadState { +struct UploadState { location: Path, parts: Parts, - client: Arc>, + client: Arc, } #[async_trait] -impl MultipartUpload for AzureMultiPartUpload { +impl MultipartUpload for AzureMultiPartUpload { fn put_part(&mut self, data: PutPayload) -> UploadPart { let idx = self.part_idx; self.part_idx += 1; @@ -262,7 +260,7 @@ impl MultipartUpload for AzureMultiPartUpload { } #[async_trait] -impl MultipartStore for MicrosoftAzure { +impl MultipartStore for MicrosoftAzure { async fn create_multipart(&self, _: &Path) -> Result { Ok(String::new()) } @@ -296,7 +294,7 @@ impl MultipartStore for MicrosoftAzure { } #[async_trait] -impl PaginatedListStore for MicrosoftAzure { +impl PaginatedListStore for MicrosoftAzure { async fn list_paginated( &self, prefix: Option<&str>, @@ -309,7 +307,6 @@ impl PaginatedListStore for MicrosoftAzure { #[cfg(test)] mod tests { use super::*; - use crate::crypto::ring_crypto::RingProvider; use crate::integration::*; use crate::tests::*; use bytes::Bytes; @@ -335,9 +332,8 @@ mod tests { let validate = !integration.client.config().disable_tagging; tagging( - Arc::new(MicrosoftAzure:: { + Arc::new(MicrosoftAzure { client: Arc::clone(&integration.client), - _crypto_provider: PhantomData::default(), }), validate, |p| { diff --git a/src/crypto.rs b/src/crypto.rs index 664834cc..1a037255 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -1,33 +1,47 @@ -use std::fmt::{Debug, Display}; +use std::fmt::Debug; +use std::sync::Arc; -type DynError = Box; +pub type CryptoProviderRef = Arc; + +/// TODO(jakedern): Docs +pub trait CryptoProvider: Send + Sync + Debug + 'static { + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result; + fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result; +} #[derive(Debug)] -pub struct Error { - inner: DynError, +pub struct Digest(Vec); + +impl From<&[u8]> for Digest { + fn from(bytes: &[u8]) -> Self { + Digest(bytes.to_vec()) + } } -impl Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Crypto error: {}", self.inner) +impl AsRef<[u8]> for Digest { + fn as_ref(&self) -> &[u8] { + &self.0 } } -impl std::error::Error for Error {} +#[derive(Debug)] +pub struct Tag(Vec); -impl From for Error { - fn from(err: DynError) -> Self { - Error { inner: err } +impl AsRef<[u8]> for Tag { + fn as_ref(&self) -> &[u8] { + &self.0 } } -/// TODO(jakedern): Docs -pub trait CryptoProvider: Send + Sync + Debug + 'static { - fn digest_sha256(bytes: &[u8]) -> Result, Error>; - fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, Error>; - fn hex_digest(bytes: &[u8]) -> Result { - let digest = Self::digest_sha256(bytes)?; - Ok(hex_encode(digest.as_ref())) +impl From<&[u8]> for Tag { + fn from(bytes: &[u8]) -> Self { + Tag(bytes.to_vec()) + } +} + +impl From> for Tag { + fn from(bytes: Vec) -> Self { + Tag(bytes) } } @@ -43,21 +57,21 @@ pub(crate) fn hex_encode(bytes: &[u8]) -> String { /// TODO(jakedern): Docs #[cfg(feature = "ring")] pub mod ring_crypto { - use super::{CryptoProvider, Error}; + use super::{CryptoProvider, Digest, Tag}; #[derive(Debug, Clone, Copy)] pub struct RingProvider; impl CryptoProvider for RingProvider { - fn digest_sha256(bytes: &[u8]) -> Result, Error> { + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { let digest = ring::digest::digest(&ring::digest::SHA256, bytes); - Ok(digest) + Ok(digest.as_ref().into()) } - fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, Error> { + fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret); let tag = ring::hmac::sign(&key, bytes); - Ok(tag) + Ok(tag.as_ref().into()) } } @@ -73,25 +87,30 @@ pub mod openssl_crypto { use openssl::pkey::PKey; use openssl::sign::Signer; - use super::{CryptoProvider, DynError, Error}; + use super::{CryptoProvider, Digest, Tag}; #[derive(Debug, Clone, Copy)] pub struct OpenSslCrypto; impl CryptoProvider for OpenSslCrypto { - fn digest_sha256(bytes: &[u8]) -> Result, Error> { - let digest = openssl::hash::hash(MessageDigest::sha256(), bytes) - .map_err(|e| Box::new(e) as DynError)?; - Ok(digest) + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { + let digest = openssl::hash::hash(MessageDigest::sha256(), bytes)?; + Ok(digest.as_ref().into()) } - fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result, Error> { - let key = PKey::hmac(secret).map_err(|e| Box::new(e) as DynError)?; - let mut signer = - Signer::new(MessageDigest::sha256(), &key).map_err(|e| Box::new(e) as DynError)?; - signer.update(bytes).map_err(|e| Box::new(e) as DynError)?; - let hmac = signer.sign_to_vec().map_err(|e| Box::new(e) as DynError)?; - Ok(hmac) + fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { + let key = PKey::hmac(secret)?; + let mut signer = Signer::new(MessageDigest::sha256(), &key)?; + signer.update(bytes)?; + let hmac = signer.sign_to_vec()?; + Ok(hmac.into()) + } + } + + impl From for crate::Error { + fn from(value: openssl::error::ErrorStack) -> Self { + // TODO(jakedern) + todo!() } } } diff --git a/src/parse.rs b/src/parse.rs index 73217961..bc814f25 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -211,13 +211,7 @@ where } #[cfg(all(feature = "azure", feature = "ring"))] ObjectStoreScheme::MicrosoftAzure => { - use crate::crypto::ring_crypto::RingProvider; - - builder_opts!( - crate::azure::MicrosoftAzureBuilder, - url, - _options - ) + builder_opts!(crate::azure::MicrosoftAzureBuilder, url, _options) } #[cfg(feature = "http")] ObjectStoreScheme::Http => { From 753c5529816133b18a785e9184ae4f4e1a627da7 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 08:26:55 -0700 Subject: [PATCH 06/15] Basic azure integration test, other cleanup --- Cargo.toml | 6 +++- src/azure/builder.rs | 31 ++++++++++++++--- src/azure/client.rs | 4 ++- src/azure/mod.rs | 2 +- src/crypto.rs | 81 ++++++++++++++++++++++++++++++++------------ src/util.rs | 2 +- tests/crypto.rs | 47 +++++++++++++++++++++++++ 7 files changed, 143 insertions(+), 30 deletions(-) create mode 100644 tests/crypto.rs diff --git a/Cargo.toml b/Cargo.toml index e7d97bdc..11bb1f17 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -70,7 +70,7 @@ web-time = { version = "1.1.0" } wasm-bindgen-futures = "0.4.18" [features] -default = ["fs"] +default = ["fs", "ring"] ring = ["dep:ring"] cloud = ["serde", "serde_json", "quick-xml", "hyper", "reqwest", "reqwest/stream", "chrono/serde", "base64", "rand", "dep:ring", "http-body-util", "form_urlencoded", "serde_urlencoded"] azure = ["cloud", "httparse"] @@ -107,3 +107,7 @@ features = ["js"] name = "get_range_file" path = "tests/get_range_file.rs" required-features = ["fs"] + +[[test]] +name = "crypto" +path = "tests/crypto.rs" diff --git a/src/azure/builder.rs b/src/azure/builder.rs index 71684f46..863e772e 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -560,23 +560,23 @@ impl MicrosoftAzureBuilder { /// .with_container_name("foo") /// .build(); /// ``` - pub fn from_env(mut self) -> Self { - // let mut builder = Self::default(); + pub fn from_env() -> Self { + let mut builder = Self::default(); for (os_key, os_value) in std::env::vars_os() { if let (Some(key), Some(value)) = (os_key.to_str(), os_value.to_str()) { if key.starts_with("AZURE_") { if let Ok(config_key) = key.to_ascii_lowercase().parse() { - self = self.with_config(config_key, value); + builder = builder.with_config(config_key, value); } } } } if let Ok(text) = std::env::var(MSI_ENDPOINT_ENV_KEY) { - self = self.with_msi_endpoint(text); + builder = builder.with_msi_endpoint(text); } - self + builder } /// TODO(jakedern): Docs @@ -1151,6 +1151,8 @@ fn split_sas(sas: &str) -> Result, Error> { #[cfg(test)] mod tests { + use crate::crypto; + use super::*; use std::collections::HashMap; @@ -1317,4 +1319,23 @@ mod tests { panic!("{key} not propagated as ClientConfigKey"); } } + + #[test] + fn azure_test_crypto_configuration() { + let builder = MicrosoftAzureBuilder::default() + .with_container_name("testcontainer") + .with_account("testaccount") + .with_crypto(Arc::from(crypto::noop_crypto::NoopCrypto {})); + + let bytes = b"hello world"; + assert_eq!( + builder + .crypto_provider + .unwrap() + .digest_sha256(bytes) + .unwrap() + .as_ref(), + bytes + ); + } } diff --git a/src/azure/client.rs b/src/azure/client.rs index e242e443..bc019d81 100644 --- a/src/azure/client.rs +++ b/src/azure/client.rs @@ -1256,7 +1256,6 @@ pub(crate) struct UserDelegationKey { #[cfg(test)] mod tests { use super::*; - use crate::crypto::ring_crypto::RingProvider; use crate::StaticCredentialProvider; use bytes::Bytes; use regex::bytes::Regex; @@ -1452,7 +1451,10 @@ mod tests { } #[tokio::test] + #[cfg(feature = "ring")] async fn test_build_bulk_delete_body() { + use crate::crypto::ring_crypto::RingProvider; + let credential_provider = Arc::new(StaticCredentialProvider::new( AzureCredential::BearerToken("static-token".to_string()), )); diff --git a/src/azure/mod.rs b/src/azure/mod.rs index 74d2c170..ce27356f 100644 --- a/src/azure/mod.rs +++ b/src/azure/mod.rs @@ -314,7 +314,7 @@ mod tests { #[tokio::test] async fn azure_blob_test() { maybe_skip_integration!(); - let integration = MicrosoftAzureBuilder::default().from_env().build().unwrap(); + let integration = MicrosoftAzureBuilder::from_env().build().unwrap(); put_get_delete_list(&integration).await; get_opts(&integration).await; diff --git a/src/crypto.rs b/src/crypto.rs index 1a037255..1a34e25f 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -1,3 +1,20 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + use std::fmt::Debug; use std::sync::Arc; @@ -82,35 +99,57 @@ pub mod ring_crypto { } } -pub mod openssl_crypto { - use openssl::hash::MessageDigest; - use openssl::pkey::PKey; - use openssl::sign::Signer; - +#[cfg(test)] +pub mod noop_crypto { use super::{CryptoProvider, Digest, Tag}; - #[derive(Debug, Clone, Copy)] - pub struct OpenSslCrypto; + pub struct NoopCrypto; - impl CryptoProvider for OpenSslCrypto { + impl CryptoProvider for NoopCrypto { fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { - let digest = openssl::hash::hash(MessageDigest::sha256(), bytes)?; - Ok(digest.as_ref().into()) + Ok(Digest(bytes.to_vec())) } - - fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { - let key = PKey::hmac(secret)?; - let mut signer = Signer::new(MessageDigest::sha256(), &key)?; - signer.update(bytes)?; - let hmac = signer.sign_to_vec()?; - Ok(hmac.into()) + fn hmac_sha256(&self, _secret: &[u8], bytes: &[u8]) -> crate::Result { + Ok(Tag(bytes.to_vec())) } } - impl From for crate::Error { - fn from(value: openssl::error::ErrorStack) -> Self { - // TODO(jakedern) - todo!() + impl Default for NoopCrypto { + fn default() -> Self { + NoopCrypto } } } + +// pub mod openssl_crypto { +// use openssl::hash::MessageDigest; +// use openssl::pkey::PKey; +// use openssl::sign::Signer; +// +// use super::{CryptoProvider, Digest, Tag}; +// +// #[derive(Debug, Clone, Copy)] +// pub struct OpenSslCrypto; +// +// impl CryptoProvider for OpenSslCrypto { +// fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { +// let digest = openssl::hash::hash(MessageDigest::sha256(), bytes)?; +// Ok(digest.as_ref().into()) +// } +// +// fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { +// let key = PKey::hmac(secret)?; +// let mut signer = Signer::new(MessageDigest::sha256(), &key)?; +// signer.update(bytes)?; +// let hmac = signer.sign_to_vec()?; +// Ok(hmac.into()) +// } +// } +// +// impl From for crate::Error { +// fn from(value: openssl::error::ErrorStack) -> Self { +// // TODO(jakedern) +// todo!() +// } +// } +// } diff --git a/src/util.rs b/src/util.rs index 4f297d95..9be85790 100644 --- a/src/util.rs +++ b/src/util.rs @@ -42,7 +42,7 @@ where Ok(chrono::TimeZone::from_utc_datetime(&chrono::Utc, &naive)) } -#[cfg(any(feature = "aws", feature = "azure"))] +#[cfg(feature = "aws")] pub(crate) fn hmac_sha256(secret: impl AsRef<[u8]>, bytes: impl AsRef<[u8]>) -> ring::hmac::Tag { let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret.as_ref()); ring::hmac::sign(&key, bytes.as_ref()) diff --git a/tests/crypto.rs b/tests/crypto.rs new file mode 100644 index 00000000..15c71fd9 --- /dev/null +++ b/tests/crypto.rs @@ -0,0 +1,47 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Tests combinations of crypto-related features. This module is meant to be +//! run both with and without the `ring` feature enabled to make sure that both +//! scenarios are covered. + +#[test] +#[cfg(feature = "azure")] +fn test_azure_default_crypto() { + let builder = object_store::azure::MicrosoftAzureBuilder::default() + .with_container_name("testcontainer") + .with_account("testaccount"); + + #[cfg(feature = "ring")] + { + // Builder should build just fine with the default crypto provider + builder.build().unwrap(); + } + + #[cfg(not(feature = "ring"))] + { + let res = builder.build(); + assert!( + res.is_err(), + "Builder should fail without crypto configured" + ); + assert!(res + .unwrap_err() + .to_string() + .contains("Missing crypto provider.")); + } +} From 982146e22b6dbe3ed0c4e0aa9fa08ee03c7b1f64 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 09:21:14 -0700 Subject: [PATCH 07/15] Initial AWS implementation --- src/aws/builder.rs | 74 +++++++++++++++++++++++++++-- src/aws/client.rs | 32 ++++++++++--- src/aws/credential.rs | 103 ++++++++++++++++++++++++++++++---------- src/aws/mod.rs | 13 ++++- src/azure/builder.rs | 6 +-- src/azure/credential.rs | 1 + src/crypto.rs | 9 ---- src/util.rs | 6 --- 8 files changed, 190 insertions(+), 54 deletions(-) diff --git a/src/aws/builder.rs b/src/aws/builder.rs index 06503cad..dc7d7552 100644 --- a/src/aws/builder.rs +++ b/src/aws/builder.rs @@ -26,6 +26,7 @@ use crate::aws::{ }; use crate::client::{http_connector, HttpConnector, TokenCredentialProvider}; use crate::config::ConfigValue; +use crate::crypto::CryptoProviderRef; use crate::{ClientConfigKey, ClientOptions, Result, RetryConfig, StaticCredentialProvider}; use base64::prelude::BASE64_STANDARD; use base64::Engine; @@ -87,6 +88,9 @@ enum Error { header: &'static str, source: Box, }, + + #[error("Missing crypto provider. Please enabled the default crypto provider or configure one explicitly.")] + MissingCryptoProvider {}, } impl From for crate::Error { @@ -120,8 +124,10 @@ impl From for crate::Error { /// .with_secret_access_key(SECRET_KEY) /// .build(); /// ``` -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone)] pub struct AmazonS3Builder { + /// Crypto provider + crypto_provider: Option, /// Access key id access_key_id: Option, /// Secret access_key @@ -486,10 +492,55 @@ impl FromStr for AmazonS3ConfigKey { } } +impl Default for AmazonS3Builder { + fn default() -> Self { + Self::new() + } +} + impl AmazonS3Builder { /// Create a new [`AmazonS3Builder`] with default values. pub fn new() -> Self { - Default::default() + let mut builder = Self { + crypto_provider: None, + access_key_id: None, + secret_access_key: None, + region: None, + bucket_name: None, + endpoint: None, + token: None, + url: None, + retry_config: RetryConfig::default(), + imdsv1_fallback: ConfigValue::default(), + virtual_hosted_style_request: ConfigValue::default(), + s3_express: ConfigValue::default(), + unsigned_payload: ConfigValue::default(), + checksum_algorithm: None, + metadata_endpoint: Some(DEFAULT_METADATA_ENDPOINT.to_string()), + container_credentials_relative_uri: None, + container_credentials_full_uri: None, + container_authorization_token_file: None, + client_options: ClientOptions::default(), + credentials: None, + skip_signature: ConfigValue::default(), + copy_if_not_exists: None, + conditional_put: ConfigValue::default(), + disable_tagging: ConfigValue::default(), + encryption_type: None, + encryption_kms_key_id: None, + encryption_bucket_key_enabled: None, + encryption_customer_key_base64: None, + request_payer: ConfigValue::default(), + http_connector: None, + }; + + #[cfg(feature = "ring")] + { + use crate::crypto::ring_crypto::RingProvider; + builder = builder.with_crypto(Arc::new(RingProvider::default())); + }; + + builder } /// Fill the [`AmazonS3Builder`] with regular AWS environment variables @@ -537,6 +588,12 @@ impl AmazonS3Builder { builder } + /// TODO(jakedern): Docs + pub fn with_crypto(mut self, crypto_provider: CryptoProviderRef) -> Self { + self.crypto_provider = Some(crypto_provider); + self + } + /// Parse available connection info form a well-known storage URL. /// /// The supported url schemes are: @@ -1096,6 +1153,10 @@ impl AmazonS3Builder { )) as _ }; + let crypto_provider = self + .crypto_provider + .ok_or(Error::MissingCryptoProvider {})?; + let (session_provider, zonal_endpoint) = match self.s3_express.get()? { true => { let zone = parse_bucket_az(&bucket).ok_or_else(|| { @@ -1109,6 +1170,7 @@ impl AmazonS3Builder { let session = Arc::new( TokenCredentialProvider::new( SessionProvider { + crypto_provider: crypto_provider.clone(), endpoint: endpoint.clone(), region: region.clone(), credentials: Arc::clone(&credentials), @@ -1148,6 +1210,7 @@ impl AmazonS3Builder { }; let config = S3Config { + crypto_provider: crypto_provider.clone(), region, bucket, bucket_endpoint, @@ -1166,9 +1229,12 @@ impl AmazonS3Builder { }; let http_client = http.connect(&config.client_options)?; - let client = Arc::new(S3Client::new(config, http_client)); + let client = Arc::new(S3Client::new(config, http_client, crypto_provider.clone())); - Ok(AmazonS3 { client }) + Ok(AmazonS3 { + client, + crypto_provider, + }) } } diff --git a/src/aws/client.rs b/src/aws/client.rs index aee3af89..59d25e63 100644 --- a/src/aws/client.rs +++ b/src/aws/client.rs @@ -33,6 +33,7 @@ use crate::client::s3::{ InitiateMultipartUploadResult, ListResponse, PartMetadata, }; use crate::client::{GetOptionsExt, HttpClient, HttpError, HttpResponse}; +use crate::crypto::{CryptoProvider, CryptoProviderRef}; use crate::list::{PaginatedListOptions, PaginatedListResult}; use crate::multipart::PartId; use crate::{ @@ -192,6 +193,7 @@ impl From for Error { #[derive(Debug)] pub(crate) struct S3Config { + pub crypto_provider: CryptoProviderRef, pub region: String, pub bucket: String, pub bucket_endpoint: String, @@ -224,6 +226,7 @@ impl S3Config { }; Ok(SessionCredential { + crypto_provider: self.crypto_provider.as_ref(), credential, session_token: self.session_provider.is_some(), config: self, @@ -244,6 +247,7 @@ impl S3Config { } struct SessionCredential<'a> { + crypto_provider: &'a dyn CryptoProvider, credential: Option>, session_token: bool, config: &'a S3Config, @@ -251,10 +255,14 @@ struct SessionCredential<'a> { impl SessionCredential<'_> { fn authorizer(&self) -> Option> { - let mut authorizer = - AwsAuthorizer::new(self.credential.as_deref()?, "s3", &self.config.region) - .with_sign_payload(self.config.sign_payload) - .with_request_payer(self.config.request_payer); + let mut authorizer = AwsAuthorizer::new( + self.credential.as_deref()?, + self.crypto_provider, + "s3", + &self.config.region, + ) + .with_sign_payload(self.config.sign_payload) + .with_request_payer(self.config.request_payer); if self.session_token { let token = HeaderName::from_static("x-amz-s3session-token"); @@ -291,6 +299,7 @@ impl From for crate::Error { /// A builder for a request allowing customisation of the headers and query string pub(crate) struct Request<'a> { + crypto_provider: &'a dyn CryptoProvider, path: &'a Path, config: &'a S3Config, builder: HttpRequestBuilder, @@ -421,6 +430,7 @@ impl Request<'_> { let credential = match self.use_session_creds { true => self.config.get_session_credential().await?, false => SessionCredential { + crypto_provider: self.crypto_provider, credential: self.config.get_credential().await?, session_token: false, config: self.config, @@ -456,16 +466,26 @@ impl Request<'_> { pub(crate) struct S3Client { pub config: S3Config, pub client: HttpClient, + pub crypto_provider: CryptoProviderRef, } impl S3Client { - pub(crate) fn new(config: S3Config, client: HttpClient) -> Self { - Self { config, client } + pub(crate) fn new( + config: S3Config, + client: HttpClient, + crypto_provider: CryptoProviderRef, + ) -> Self { + Self { + config, + client, + crypto_provider, + } } pub(crate) fn request<'a>(&'a self, method: Method, path: &'a Path) -> Request<'a> { let url = self.config.path_url(path); Request { + crypto_provider: self.crypto_provider.as_ref(), path, builder: self.client.request(method, url), payload: None, diff --git a/src/aws/credential.rs b/src/aws/credential.rs index 7e2681d4..8dc4a298 100644 --- a/src/aws/credential.rs +++ b/src/aws/credential.rs @@ -20,7 +20,8 @@ use crate::client::builder::HttpRequestBuilder; use crate::client::retry::RetryExt; use crate::client::token::{TemporaryToken, TokenCache}; use crate::client::{HttpClient, HttpError, HttpRequest, TokenProvider}; -use crate::util::{hex_digest, hex_encode, hmac_sha256}; +use crate::crypto::{CryptoProvider, CryptoProviderRef}; +use crate::util::{hex_digest, hex_encode}; use crate::{CredentialProvider, Result, RetryConfig}; use async_trait::async_trait; use bytes::Buf; @@ -91,13 +92,24 @@ impl AwsCredential { /// Signs a string /// /// - fn sign(&self, to_sign: &str, date: DateTime, region: &str, service: &str) -> String { + fn sign( + &self, + crypto_provider: &dyn CryptoProvider, + to_sign: &str, + date: DateTime, + region: &str, + service: &str, + ) -> Result { let date_string = date.format("%Y%m%d").to_string(); - let date_hmac = hmac_sha256(format!("AWS4{}", self.secret_key), date_string); - let region_hmac = hmac_sha256(date_hmac, region); - let service_hmac = hmac_sha256(region_hmac, service); - let signing_hmac = hmac_sha256(service_hmac, b"aws4_request"); - hex_encode(hmac_sha256(signing_hmac, to_sign).as_ref()) + let date_hmac = crypto_provider.hmac_sha256( + format!("AWS4{}", self.secret_key).as_bytes(), + date_string.as_bytes(), + )?; + let region_hmac = crypto_provider.hmac_sha256(date_hmac.as_ref(), region.as_bytes())?; + let service_hmac = crypto_provider.hmac_sha256(region_hmac.as_ref(), service.as_bytes())?; + let signing_hmac = crypto_provider.hmac_sha256(&service_hmac.as_ref(), b"aws4_request")?; + let signed = crypto_provider.hmac_sha256(signing_hmac.as_ref(), to_sign.as_bytes())?; + Ok(hex_encode(signed.as_ref())) } } @@ -106,6 +118,7 @@ impl AwsCredential { /// [AWS SigV4]: https://docs.aws.amazon.com/general/latest/gr/sigv4-calculate-signature.html #[derive(Debug)] pub struct AwsAuthorizer<'a> { + crypto_provider: &'a dyn CryptoProvider, date: Option>, credential: &'a AwsCredential, service: &'a str, @@ -124,8 +137,14 @@ const ALGORITHM: &str = "AWS4-HMAC-SHA256"; impl<'a> AwsAuthorizer<'a> { /// Create a new [`AwsAuthorizer`] - pub fn new(credential: &'a AwsCredential, service: &'a str, region: &'a str) -> Self { + pub fn new( + credential: &'a AwsCredential, + crypto_provider: &'a dyn CryptoProvider, + service: &'a str, + region: &'a str, + ) -> Self { Self { + crypto_provider, credential, service, region, @@ -170,7 +189,12 @@ impl<'a> AwsAuthorizer<'a> { /// * Otherwise it is set to the hex encoded SHA256 of the request body /// /// [AWS SigV4]: https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html - pub fn authorize(&self, request: &mut HttpRequest, pre_calculated_digest: Option<&[u8]>) { + #[must_use] + pub fn authorize( + &self, + request: &mut HttpRequest, + pre_calculated_digest: Option<&[u8]>, + ) -> Result<()> { let url = Url::parse(&request.uri().to_string()).unwrap(); if let Some(ref token) = self.credential.token { @@ -229,9 +253,13 @@ impl<'a> AwsAuthorizer<'a> { ); // sign the string - let signature = self - .credential - .sign(&string_to_sign, date, self.region, self.service); + let signature = self.credential.sign( + self.crypto_provider, + &string_to_sign, + date, + self.region, + self.service, + )?; // build the actual auth header let authorisation = format!( @@ -243,9 +271,11 @@ impl<'a> AwsAuthorizer<'a> { request .headers_mut() .insert(&AUTHORIZATION, authorization_val); + + Ok(()) } - pub(crate) fn sign(&self, method: Method, url: &mut Url, expires_in: Duration) { + pub(crate) fn sign(&self, method: Method, url: &mut Url, expires_in: Duration) -> Result<()> { let date = self.date.unwrap_or_else(Utc::now); let scope = self.scope(date); @@ -294,12 +324,18 @@ impl<'a> AwsAuthorizer<'a> { digest, ); - let signature = self - .credential - .sign(&string_to_sign, date, self.region, self.service); + let signature = self.credential.sign( + self.crypto_provider, + &string_to_sign, + date, + self.region, + self.service, + )?; url.query_pairs_mut() .append_pair("X-Amz-Signature", &signature); + + Ok(()) } #[allow(clippy::too_many_arguments)] @@ -374,7 +410,7 @@ impl CredentialExt for HttpRequestBuilder { Some(authorizer) => { let (client, request) = self.into_parts(); let mut request = request.expect("request valid"); - authorizer.authorize(&mut request, payload_sha256); + authorizer.authorize(&mut request, payload_sha256).unwrap(); Self::from_parts(client, request) } @@ -801,6 +837,7 @@ async fn eks_credential( /// #[derive(Debug)] pub(crate) struct SessionProvider { + pub crypto_provider: CryptoProviderRef, pub endpoint: String, pub region: String, pub credentials: AwsCredentialProvider, @@ -816,7 +853,8 @@ impl TokenProvider for SessionProvider { retry: &RetryConfig, ) -> Result>> { let creds = self.credentials.get_credential().await?; - let authorizer = AwsAuthorizer::new(&creds, "s3", &self.region); + let authorizer = + AwsAuthorizer::new(&creds, self.crypto_provider.as_ref(), "s3", &self.region); let bytes = client .get(format!("{}?session", self.endpoint)) @@ -853,12 +891,14 @@ mod tests { use crate::aws::{AmazonS3Builder, AmazonS3ConfigKey}; use crate::client::mock_server::MockServer; use crate::client::HttpClient; + use crate::crypto; use http::Response; use reqwest::{Client, Method}; use std::env; // Test generated using https://docs.aws.amazon.com/general/latest/gr/sigv4-signed-request-examples.html #[test] + #[cfg(feature = "ring")] fn test_sign_with_signed_payload() { let client = HttpClient::new(Client::new()); @@ -886,6 +926,7 @@ mod tests { .unwrap(); let signer = AwsAuthorizer { + crypto_provider: &crypto::ring_crypto::RingProvider {}, date: Some(date), credential: &credential, service: "ec2", @@ -895,11 +936,12 @@ mod tests { request_payer: false, }; - signer.authorize(&mut request, None); + signer.authorize(&mut request, None).unwrap(); assert_eq!(request.headers().get(&AUTHORIZATION).unwrap(), "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20220806/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=a3c787a7ed37f7fdfbfd2d7056a3d7c9d85e6d52a2bfbec73793c0be6e7862d4") } #[test] + #[cfg(feature = "ring")] fn test_sign_with_signed_payload_request_payer() { let client = HttpClient::new(Client::new()); @@ -927,6 +969,7 @@ mod tests { .unwrap(); let signer = AwsAuthorizer { + crypto_provider: &crypto::ring_crypto::RingProvider {}, date: Some(date), credential: &credential, service: "ec2", @@ -936,11 +979,12 @@ mod tests { request_payer: true, }; - signer.authorize(&mut request, None); + signer.authorize(&mut request, None).unwrap(); assert_eq!(request.headers().get(&AUTHORIZATION).unwrap(), "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20220806/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date;x-amz-request-payer, Signature=7030625a9e9b57ed2a40e63d749f4a4b7714b6e15004cab026152f870dd8565d") } #[test] + #[cfg(feature = "ring")] fn test_sign_with_unsigned_payload() { let client = HttpClient::new(Client::new()); @@ -968,6 +1012,7 @@ mod tests { .unwrap(); let authorizer = AwsAuthorizer { + crypto_provider: &crypto::ring_crypto::RingProvider {}, date: Some(date), credential: &credential, service: "ec2", @@ -977,11 +1022,12 @@ mod tests { request_payer: false, }; - authorizer.authorize(&mut request, None); + authorizer.authorize(&mut request, None).unwrap(); assert_eq!(request.headers().get(&AUTHORIZATION).unwrap(), "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20220806/us-east-1/ec2/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=653c3d8ea261fd826207df58bc2bb69fbb5003e9eb3c0ef06e4a51f2a81d8699"); } #[test] + #[cfg(feature = "ring")] fn signed_get_url() { // Values from https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html let credential = AwsCredential { @@ -995,6 +1041,7 @@ mod tests { .with_timezone(&Utc); let authorizer = AwsAuthorizer { + crypto_provider: &crypto::ring_crypto::RingProvider {}, date: Some(date), credential: &credential, service: "s3", @@ -1005,7 +1052,9 @@ mod tests { }; let mut url = Url::parse("https://examplebucket.s3.amazonaws.com/test.txt").unwrap(); - authorizer.sign(Method::GET, &mut url, Duration::from_secs(86400)); + authorizer + .sign(Method::GET, &mut url, Duration::from_secs(86400)) + .unwrap(); assert_eq!( url, @@ -1023,6 +1072,7 @@ mod tests { } #[test] + #[cfg(feature = "ring")] fn signed_get_url_request_payer() { // Values from https://docs.aws.amazon.com/AmazonS3/latest/API/sigv4-query-string-auth.html let credential = AwsCredential { @@ -1036,6 +1086,7 @@ mod tests { .with_timezone(&Utc); let authorizer = AwsAuthorizer { + crypto_provider: &crypto::ring_crypto::RingProvider {}, date: Some(date), credential: &credential, service: "s3", @@ -1046,7 +1097,9 @@ mod tests { }; let mut url = Url::parse("https://examplebucket.s3.amazonaws.com/test.txt").unwrap(); - authorizer.sign(Method::GET, &mut url, Duration::from_secs(86400)); + authorizer + .sign(Method::GET, &mut url, Duration::from_secs(86400)) + .unwrap(); assert_eq!( url, @@ -1065,6 +1118,7 @@ mod tests { } #[test] + #[cfg(feature = "ring")] fn test_sign_port() { let client = HttpClient::new(Client::new()); @@ -1091,6 +1145,7 @@ mod tests { .unwrap(); let authorizer = AwsAuthorizer { + crypto_provider: &crypto::ring_crypto::RingProvider {}, date: Some(date), credential: &credential, service: "s3", @@ -1100,7 +1155,7 @@ mod tests { request_payer: false, }; - authorizer.authorize(&mut request, None); + authorizer.authorize(&mut request, None).unwrap(); assert_eq!(request.headers().get(&AUTHORIZATION).unwrap(), "AWS4-HMAC-SHA256 Credential=H20ABqCkLZID4rLe/20220809/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=9ebf2f92872066c99ac94e573b4e1b80f4dbb8a32b1e8e23178318746e7d1b4d") } diff --git a/src/aws/mod.rs b/src/aws/mod.rs index 8dac2bd7..b9f341fc 100644 --- a/src/aws/mod.rs +++ b/src/aws/mod.rs @@ -40,6 +40,7 @@ use crate::aws::client::{CompleteMultipartMode, PutPartPayload, RequestError, S3 use crate::client::get::GetClientExt; use crate::client::list::{ListClient, ListClientExt}; use crate::client::CredentialProvider; +use crate::crypto::CryptoProviderRef; use crate::multipart::{MultipartStore, PartId}; use crate::signer::Signer; use crate::util::STRICT_ENCODE_SET; @@ -83,6 +84,7 @@ pub use credential::{AwsAuthorizer, AwsCredential}; #[derive(Debug, Clone)] pub struct AmazonS3 { client: Arc, + crypto_provider: CryptoProviderRef, } impl std::fmt::Display for AmazonS3 { @@ -139,8 +141,13 @@ impl Signer for AmazonS3 { /// ``` async fn signed_url(&self, method: Method, path: &Path, expires_in: Duration) -> Result { let credential = self.credentials().get_credential().await?; - let authorizer = AwsAuthorizer::new(&credential, "s3", &self.client.config.region) - .with_request_payer(self.client.config.request_payer); + let authorizer = AwsAuthorizer::new( + &credential, + self.crypto_provider.as_ref(), + "s3", + &self.client.config.region, + ) + .with_request_payer(self.client.config.request_payer); let path_url = self.path_url(path); let mut url = path_url.parse().map_err(|e| Error::Generic { @@ -496,6 +503,7 @@ mod tests { use crate::client::get::GetClient; use crate::client::retry::RetryContext; use crate::client::SpawnedReqwestConnector; + use crate::crypto; use crate::integration::*; use crate::tests::*; use crate::ClientOptions; @@ -596,6 +604,7 @@ mod tests { tagging( Arc::new(AmazonS3 { client: Arc::clone(&integration.client), + crypto_provider: Arc::from(crypto::noop_crypto::NoopCrypto {}), }), !config.disable_tagging, |p| { diff --git a/src/azure/builder.rs b/src/azure/builder.rs index 863e772e..11c0906e 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -501,7 +501,7 @@ impl Default for MicrosoftAzureBuilder { impl MicrosoftAzureBuilder { /// Create a new [`MicrosoftAzureBuilder`] with default values. pub fn new() -> Self { - let mut b = Self { + let mut builder = Self { crypto_provider: None, account_name: None, access_key: None, @@ -537,10 +537,10 @@ impl MicrosoftAzureBuilder { #[cfg(feature = "ring")] { use crate::crypto::ring_crypto::RingProvider; - b = b.with_crypto(Arc::new(RingProvider::default())); + builder = builder.with_crypto(Arc::new(RingProvider::default())); }; - b + builder } /// Create an instance of [`MicrosoftAzureBuilder`] with values pre-populated from environment variables. diff --git a/src/azure/credential.rs b/src/azure/credential.rs index 5a25b6c1..b1d00de6 100644 --- a/src/azure/credential.rs +++ b/src/azure/credential.rs @@ -252,6 +252,7 @@ impl<'a> AzureAuthorizer<'a> { } /// Authorize `request` + #[must_use] pub fn authorize(&self, request: &mut HttpRequest) -> Result<()> { add_date_and_version_headers(request); diff --git a/src/crypto.rs b/src/crypto.rs index 1a34e25f..8bebc0f8 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -62,15 +62,6 @@ impl From> for Tag { } } -pub(crate) fn hex_encode(bytes: &[u8]) -> String { - use std::fmt::Write; - let mut out = String::with_capacity(bytes.len() * 2); - for byte in bytes { - let _ = write!(out, "{byte:02x}"); - } - out -} - /// TODO(jakedern): Docs #[cfg(feature = "ring")] pub mod ring_crypto { diff --git a/src/util.rs b/src/util.rs index 9be85790..98b9c8ee 100644 --- a/src/util.rs +++ b/src/util.rs @@ -42,12 +42,6 @@ where Ok(chrono::TimeZone::from_utc_datetime(&chrono::Utc, &naive)) } -#[cfg(feature = "aws")] -pub(crate) fn hmac_sha256(secret: impl AsRef<[u8]>, bytes: impl AsRef<[u8]>) -> ring::hmac::Tag { - let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret.as_ref()); - ring::hmac::sign(&key, bytes.as_ref()) -} - /// Collect a stream into [`Bytes`] avoiding copying in the event of a single chunk pub async fn collect_bytes(mut stream: S, size_hint: Option) -> Result where From b57ee3054d3a158c8c76b66b72f29fc47b886069 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 09:27:41 -0700 Subject: [PATCH 08/15] Initial aws tests --- src/aws/builder.rs | 19 +++++++++++++++++++ tests/crypto.rs | 25 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/aws/builder.rs b/src/aws/builder.rs index dc7d7552..00974e59 100644 --- a/src/aws/builder.rs +++ b/src/aws/builder.rs @@ -1419,6 +1419,8 @@ impl From for HeaderMap { #[cfg(test)] mod tests { + use crate::crypto; + use super::*; use std::collections::HashMap; @@ -1797,6 +1799,23 @@ mod tests { assert!( debug_str.contains("TokenCredentialProvider"), "expected TokenCredentialProvider but got: {debug_str}" + ) + } + + fn aws_test_crypto_configuration() { + let builder = AmazonS3Builder::default() + .with_bucket_name("testbucket") + .with_crypto(Arc::from(crypto::noop_crypto::NoopCrypto {})); + + let bytes = b"hello world"; + assert_eq!( + builder + .crypto_provider + .unwrap() + .digest_sha256(bytes) + .unwrap() + .as_ref(), + bytes ); } } diff --git a/tests/crypto.rs b/tests/crypto.rs index 15c71fd9..e09aaa77 100644 --- a/tests/crypto.rs +++ b/tests/crypto.rs @@ -45,3 +45,28 @@ fn test_azure_default_crypto() { .contains("Missing crypto provider.")); } } + +#[test] +#[cfg(feature = "aws")] +fn test_aws_default_crypto() { + let builder = object_store::aws::AmazonS3Builder::default().with_bucket_name("testbucket"); + + #[cfg(feature = "ring")] + { + // Builder should build ok with the default crypto provider + builder.build().unwrap(); + } + + #[cfg(not(feature = "ring"))] + { + let res = builder.build(); + assert!( + res.is_err(), + "Builder should fail without crypto configured" + ); + assert!(res + .unwrap_err() + .to_string() + .contains("Missing crypto provider.")); + } +} From 3a89994141244f1ad3db2007df16fa2f5c0a830b Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 09:59:07 -0700 Subject: [PATCH 09/15] Add digest_all api --- src/aws/client.rs | 24 ++++++------ src/aws/mod.rs | 2 +- src/crypto.rs | 97 +++++++++++++++++++++++++++++++---------------- 3 files changed, 77 insertions(+), 46 deletions(-) diff --git a/src/aws/client.rs b/src/aws/client.rs index 59d25e63..967e3fcc 100644 --- a/src/aws/client.rs +++ b/src/aws/client.rs @@ -33,7 +33,7 @@ use crate::client::s3::{ InitiateMultipartUploadResult, ListResponse, PartMetadata, }; use crate::client::{GetOptionsExt, HttpClient, HttpError, HttpResponse}; -use crate::crypto::{CryptoProvider, CryptoProviderRef}; +use crate::crypto::{self, CryptoProvider, CryptoProviderRef}; use crate::list::{PaginatedListOptions, PaginatedListResult}; use crate::multipart::PartId; use crate::{ @@ -53,8 +53,6 @@ use itertools::Itertools; use md5::{Digest, Md5}; use percent_encoding::{utf8_percent_encode, PercentEncode}; use quick_xml::events::{self as xml_events}; -use ring::digest; -use ring::digest::Context; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -303,7 +301,7 @@ pub(crate) struct Request<'a> { path: &'a Path, config: &'a S3Config, builder: HttpRequestBuilder, - payload_sha256: Option, + payload_sha256: Option, payload: Option, use_session_creds: bool, idempotent: bool, @@ -404,18 +402,18 @@ impl Request<'_> { Self { builder, ..self } } - pub(crate) fn with_payload(mut self, payload: PutPayload) -> Self { + pub(crate) fn with_payload(mut self, payload: PutPayload) -> Result { if (!self.config.skip_signature && self.config.sign_payload) || self.config.checksum.is_some() { - let mut sha256 = Context::new(&digest::SHA256); - payload.iter().for_each(|x| sha256.update(x)); - let payload_sha256 = sha256.finish(); + let payload_sha256 = self + .crypto_provider + .digest_all_sha256(&mut payload.iter().map(|p| p.as_ref()))?; if let Some(Checksum::SHA256) = self.config.checksum { self.builder = self .builder - .header(SHA256_CHECKSUM, BASE64_STANDARD.encode(payload_sha256)); + .header(SHA256_CHECKSUM, BASE64_STANDARD.encode(&payload_sha256)); } self.payload_sha256 = Some(payload_sha256); } @@ -423,7 +421,7 @@ impl Request<'_> { let content_length = payload.content_length(); self.builder = self.builder.header(CONTENT_LENGTH, content_length); self.payload = Some(payload); - self + Ok(self) } pub(crate) async fn send(self) -> Result { @@ -554,8 +552,8 @@ impl S3Client { let mut builder = self.client.request(Method::POST, url); - let digest = digest::digest(&digest::SHA256, &body); - builder = builder.header(SHA256_CHECKSUM, BASE64_STANDARD.encode(digest)); + let digest = self.crypto_provider.digest_sha256(&body)?; + builder = builder.header(SHA256_CHECKSUM, BASE64_STANDARD.encode(digest.as_ref())); // S3 *requires* DeleteObjects to include a Content-MD5 header: // https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html @@ -705,7 +703,7 @@ impl S3Client { .idempotent(true); request = match data { - PutPartPayload::Part(payload) => request.with_payload(payload), + PutPartPayload::Part(payload) => request.with_payload(payload)?, PutPartPayload::Copy(path) => request.header( "x-amz-copy-source", &format!("{}/{}", self.config.bucket, encode_path(path)), diff --git a/src/aws/mod.rs b/src/aws/mod.rs index b9f341fc..01806158 100644 --- a/src/aws/mod.rs +++ b/src/aws/mod.rs @@ -179,7 +179,7 @@ impl ObjectStore for AmazonS3 { let request = self .client .request(Method::PUT, location) - .with_payload(payload) + .with_payload(payload)? .with_attributes(attributes) .with_tags(tags) .with_extensions(extensions) diff --git a/src/crypto.rs b/src/crypto.rs index 8bebc0f8..dac94254 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -22,6 +22,8 @@ pub type CryptoProviderRef = Arc; /// TODO(jakedern): Docs pub trait CryptoProvider: Send + Sync + Debug + 'static { + fn digest_all_sha256(&self, payloads: &mut dyn Iterator) + -> crate::Result; fn digest_sha256(&self, bytes: &[u8]) -> crate::Result; fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result; } @@ -71,6 +73,18 @@ pub mod ring_crypto { pub struct RingProvider; impl CryptoProvider for RingProvider { + fn digest_all_sha256( + &self, + payloads: &mut dyn Iterator, + ) -> crate::Result { + let mut hasher = ring::digest::Context::new(&ring::digest::SHA256); + for payload in payloads { + hasher.update(payload); + } + + Ok(hasher.finish().as_ref().into()) + } + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { let digest = ring::digest::digest(&ring::digest::SHA256, bytes); Ok(digest.as_ref().into()) @@ -97,6 +111,13 @@ pub mod noop_crypto { pub struct NoopCrypto; impl CryptoProvider for NoopCrypto { + fn digest_all_sha256( + &self, + _payloads: &mut dyn Iterator, + ) -> crate::Result { + Ok(Digest(vec![])) + } + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { Ok(Digest(bytes.to_vec())) } @@ -112,35 +133,47 @@ pub mod noop_crypto { } } -// pub mod openssl_crypto { -// use openssl::hash::MessageDigest; -// use openssl::pkey::PKey; -// use openssl::sign::Signer; -// -// use super::{CryptoProvider, Digest, Tag}; -// -// #[derive(Debug, Clone, Copy)] -// pub struct OpenSslCrypto; -// -// impl CryptoProvider for OpenSslCrypto { -// fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { -// let digest = openssl::hash::hash(MessageDigest::sha256(), bytes)?; -// Ok(digest.as_ref().into()) -// } -// -// fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { -// let key = PKey::hmac(secret)?; -// let mut signer = Signer::new(MessageDigest::sha256(), &key)?; -// signer.update(bytes)?; -// let hmac = signer.sign_to_vec()?; -// Ok(hmac.into()) -// } -// } -// -// impl From for crate::Error { -// fn from(value: openssl::error::ErrorStack) -> Self { -// // TODO(jakedern) -// todo!() -// } -// } -// } +pub mod openssl_crypto { + use openssl::hash::MessageDigest; + use openssl::pkey::PKey; + use openssl::sign::Signer; + + use super::{CryptoProvider, Digest, Tag}; + + #[derive(Debug, Clone, Copy)] + pub struct OpenSslCrypto; + + impl CryptoProvider for OpenSslCrypto { + fn digest_all_sha256( + &self, + payloads: &mut dyn Iterator, + ) -> crate::Result { + let mut hasher = openssl::hash::Hasher::new(MessageDigest::sha256())?; + for p in payloads { + hasher.update(p)?; + } + + Ok(hasher.finish()?.as_ref().into()) + } + + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { + let digest = openssl::hash::hash(MessageDigest::sha256(), bytes)?; + Ok(digest.as_ref().into()) + } + + fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { + let key = PKey::hmac(secret)?; + let mut signer = Signer::new(MessageDigest::sha256(), &key)?; + signer.update(bytes)?; + let hmac = signer.sign_to_vec()?; + Ok(hmac.into()) + } + } + + impl From for crate::Error { + fn from(value: openssl::error::ErrorStack) -> Self { + // TODO(jakedern) + todo!() + } + } +} From a4765d078b41bc20ea3917c23c432c53df7ec580 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 10:05:55 -0700 Subject: [PATCH 10/15] Default implementation --- src/crypto.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/crypto.rs b/src/crypto.rs index dac94254..d604af12 100644 --- a/src/crypto.rs +++ b/src/crypto.rs @@ -18,14 +18,25 @@ use std::fmt::Debug; use std::sync::Arc; +use crate::util::hex_encode; + pub type CryptoProviderRef = Arc; /// TODO(jakedern): Docs pub trait CryptoProvider: Send + Sync + Debug + 'static { fn digest_all_sha256(&self, payloads: &mut dyn Iterator) -> crate::Result; - fn digest_sha256(&self, bytes: &[u8]) -> crate::Result; + fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result; + + fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { + self.digest_all_sha256(&mut [bytes].into_iter()) + } + + fn hex_digest(&self, bytes: &[u8]) -> crate::Result { + let digest = self.digest_sha256(bytes)?; + Ok(hex_encode(digest.as_ref())) + } } #[derive(Debug)] @@ -85,11 +96,6 @@ pub mod ring_crypto { Ok(hasher.finish().as_ref().into()) } - fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { - let digest = ring::digest::digest(&ring::digest::SHA256, bytes); - Ok(digest.as_ref().into()) - } - fn hmac_sha256(&self, secret: &[u8], bytes: &[u8]) -> crate::Result { let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret); let tag = ring::hmac::sign(&key, bytes); @@ -121,6 +127,7 @@ pub mod noop_crypto { fn digest_sha256(&self, bytes: &[u8]) -> crate::Result { Ok(Digest(bytes.to_vec())) } + fn hmac_sha256(&self, _secret: &[u8], bytes: &[u8]) -> crate::Result { Ok(Tag(bytes.to_vec())) } From bfecbb0dd267c97dd7d1e910610b446bbdfef9b0 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 17:16:40 -0700 Subject: [PATCH 11/15] More aws cleanup --- src/aws/credential.rs | 18 ++++++++++-------- src/util.rs | 8 -------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/aws/credential.rs b/src/aws/credential.rs index 8dc4a298..b3fab2a5 100644 --- a/src/aws/credential.rs +++ b/src/aws/credential.rs @@ -21,7 +21,7 @@ use crate::client::retry::RetryExt; use crate::client::token::{TemporaryToken, TokenCache}; use crate::client::{HttpClient, HttpError, HttpRequest, TokenProvider}; use crate::crypto::{CryptoProvider, CryptoProviderRef}; -use crate::util::{hex_digest, hex_encode}; +use crate::util::hex_encode; use crate::{CredentialProvider, Result, RetryConfig}; use async_trait::async_trait; use bytes::Buf; @@ -219,7 +219,7 @@ impl<'a> AwsAuthorizer<'a> { None => match request.body().is_empty() { true => EMPTY_SHA256_HASH.to_string(), false => match request.body().as_bytes() { - Some(bytes) => hex_digest(bytes), + Some(bytes) => self.crypto_provider.hex_digest(bytes)?, None => STREAMING_PAYLOAD.to_string(), }, }, @@ -250,7 +250,7 @@ impl<'a> AwsAuthorizer<'a> { &canonical_headers, &signed_headers, &digest, - ); + )?; // sign the string let signature = self.credential.sign( @@ -322,7 +322,7 @@ impl<'a> AwsAuthorizer<'a> { &canonical_headers, &signed_headers, digest, - ); + )?; let signature = self.credential.sign( self.crypto_provider, @@ -348,7 +348,7 @@ impl<'a> AwsAuthorizer<'a> { canonical_headers: &str, signed_headers: &str, digest: &str, - ) -> String { + ) -> Result { // Each path segment must be URI-encoded twice (except for Amazon S3 which only gets // URI-encoded once). // see https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html @@ -370,15 +370,17 @@ impl<'a> AwsAuthorizer<'a> { digest ); - let hashed_canonical_request = hex_digest(canonical_request.as_bytes()); + let hashed_canonical_request = self + .crypto_provider + .hex_digest(canonical_request.as_bytes())?; - format!( + Ok(format!( "{}\n{}\n{}\n{}", ALGORITHM, date.format("%Y%m%dT%H%M%SZ"), scope, hashed_canonical_request - ) + )) } fn scope(&self, date: DateTime) -> String { diff --git a/src/util.rs b/src/util.rs index 98b9c8ee..b4f5e21b 100644 --- a/src/util.rs +++ b/src/util.rs @@ -301,15 +301,7 @@ pub(crate) const STRICT_ENCODE_SET: percent_encoding::AsciiSet = percent_encodin .remove(b'_') .remove(b'~'); -/// Computes the SHA256 digest of `body` returned as a hex encoded string -#[cfg(any(feature = "aws", feature = "gcp"))] -pub(crate) fn hex_digest(bytes: &[u8]) -> String { - let digest = ring::digest::digest(&ring::digest::SHA256, bytes); - hex_encode(digest.as_ref()) -} - /// Returns `bytes` as a lower-case hex encoded string -#[cfg(any(feature = "aws", feature = "gcp"))] pub(crate) fn hex_encode(bytes: &[u8]) -> String { use std::fmt::Write; let mut out = String::with_capacity(bytes.len() * 2); From 540c078aeff39ea93b3d4fac17723818e0666067 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 17:34:37 -0700 Subject: [PATCH 12/15] initial gcp implementation --- src/azure/builder.rs | 14 +++++++------- src/azure/credential.rs | 2 +- src/gcp/builder.rs | 32 +++++++++++++++++++++++++++++--- src/gcp/credential.rs | 30 +++++++++++++++++++++--------- src/gcp/mod.rs | 4 +++- 5 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/azure/builder.rs b/src/azure/builder.rs index 11c0906e..16c912ec 100644 --- a/src/azure/builder.rs +++ b/src/azure/builder.rs @@ -494,13 +494,6 @@ impl std::fmt::Debug for MicrosoftAzureBuilder { impl Default for MicrosoftAzureBuilder { fn default() -> Self { - Self::new() - } -} - -impl MicrosoftAzureBuilder { - /// Create a new [`MicrosoftAzureBuilder`] with default values. - pub fn new() -> Self { let mut builder = Self { crypto_provider: None, account_name: None, @@ -542,6 +535,13 @@ impl MicrosoftAzureBuilder { builder } +} + +impl MicrosoftAzureBuilder { + /// Create a new [`MicrosoftAzureBuilder`] with default values. + pub fn new() -> Self { + Self::default() + } /// Create an instance of [`MicrosoftAzureBuilder`] with values pre-populated from environment variables. /// diff --git a/src/azure/credential.rs b/src/azure/credential.rs index b1d00de6..a7caf104 100644 --- a/src/azure/credential.rs +++ b/src/azure/credential.rs @@ -95,7 +95,7 @@ pub enum Error { #[error("Generating SAS keys with SAS tokens auth is not supported")] SASforSASNotSupported, - #[error("Error performing cryptographic operations")] + #[error("Error performing cryptographic operations.")] Crypto(Box), } diff --git a/src/gcp/builder.rs b/src/gcp/builder.rs index f22d66d2..9ab6cfcd 100644 --- a/src/gcp/builder.rs +++ b/src/gcp/builder.rs @@ -17,6 +17,7 @@ use crate::client::{http_connector, HttpConnector, TokenCredentialProvider}; use crate::config::ConfigValue; +use crate::crypto::CryptoProviderRef; use crate::gcp::client::{GoogleCloudStorageClient, GoogleCloudStorageConfig}; use crate::gcp::credential::{ ApplicationDefaultCredentials, InstanceCredentialProvider, ServiceAccountCredentials, @@ -65,6 +66,9 @@ enum Error { #[error("GCP credential error: {}", source)] Credential { source: credential::Error }, + + #[error("Missing crypto provider. Please enabled the default crypto provider or configure one explicitly.")] + MissingCryptoProvider {}, } impl From for crate::Error { @@ -94,6 +98,8 @@ impl From for crate::Error { /// ``` #[derive(Debug, Clone)] pub struct GoogleCloudStorageBuilder { + /// Crypto provider + crypto_provider: Option, /// Bucket name bucket_name: Option, /// Url @@ -209,7 +215,8 @@ impl FromStr for GoogleConfigKey { impl Default for GoogleCloudStorageBuilder { fn default() -> Self { - Self { + let mut builder = Self { + crypto_provider: None, bucket_name: None, service_account_path: None, service_account_key: None, @@ -221,14 +228,22 @@ impl Default for GoogleCloudStorageBuilder { skip_signature: Default::default(), signing_credentials: None, http_connector: None, - } + }; + + #[cfg(feature = "ring")] + { + use crate::crypto::ring_crypto::RingProvider; + builder = builder.with_crypto(Arc::new(RingProvider::default())); + }; + + builder } } impl GoogleCloudStorageBuilder { /// Create a new [`GoogleCloudStorageBuilder`] with default values. pub fn new() -> Self { - Default::default() + Self::default() } /// Create an instance of [`GoogleCloudStorageBuilder`] with values pre-populated from environment variables. @@ -269,6 +284,12 @@ impl GoogleCloudStorageBuilder { builder } + /// TODO(jakedern): Docs + pub fn with_crypto(mut self, crypto_provider: CryptoProviderRef) -> Self { + self.crypto_provider = Some(crypto_provider); + self + } + /// Parse available connection info form a well-known storage URL. /// /// The supported url schemes are: @@ -577,8 +598,13 @@ impl GoogleCloudStorageBuilder { skip_signature: self.skip_signature.get()?, }; + let crypto_provider = self + .crypto_provider + .ok_or(Error::MissingCryptoProvider {})?; + let http_client = http.connect(&config.client_options)?; Ok(GoogleCloudStorage { + crypto_provider, client: Arc::new(GoogleCloudStorageClient::new(config, http_client)?), }) } diff --git a/src/gcp/credential.rs b/src/gcp/credential.rs index 1e067f53..a3ac1e3c 100644 --- a/src/gcp/credential.rs +++ b/src/gcp/credential.rs @@ -20,8 +20,9 @@ use crate::client::builder::HttpRequestBuilder; use crate::client::retry::RetryExt; use crate::client::token::TemporaryToken; use crate::client::{HttpClient, HttpError, TokenProvider}; +use crate::crypto::CryptoProvider; use crate::gcp::{GcpSigningCredentialProvider, STORE}; -use crate::util::{hex_digest, hex_encode, STRICT_ENCODE_SET}; +use crate::util::{hex_encode, STRICT_ENCODE_SET}; use crate::{RetryConfig, StaticCredentialProvider}; use async_trait::async_trait; use base64::prelude::BASE64_URL_SAFE_NO_PAD; @@ -92,6 +93,9 @@ pub enum Error { #[error("Error reading pem file: {}", source)] ReadPem { source: std::io::Error }, + + #[error("Error performing cryptographic operations.")] + Crypto(Box), } impl From for crate::Error { @@ -750,15 +754,20 @@ fn trim_header_value(value: &str) -> String { /// /// [Google SigV4]: https://cloud.google.com/storage/docs/access-control/signed-urls #[derive(Debug)] -pub(crate) struct GCSAuthorizer { +pub(crate) struct GCSAuthorizer<'a> { + crypto_provider: &'a dyn CryptoProvider, date: Option>, credential: Arc, } -impl GCSAuthorizer { +impl<'a> GCSAuthorizer<'a> { /// Create a new [`GCSAuthorizer`] - pub(crate) fn new(credential: Arc) -> Self { + pub(crate) fn new( + credential: Arc, + crypto_provider: &'a dyn CryptoProvider, + ) -> Self { Self { + crypto_provider, date: None, credential, } @@ -788,7 +797,7 @@ impl GCSAuthorizer { .append_pair("X-Goog-Expires", &expires_in.as_secs().to_string()) .append_pair("X-Goog-SignedHeaders", &signed_headers); - let string_to_sign = self.string_to_sign(date, &method, url, &headers); + let string_to_sign = self.string_to_sign(date, &method, url, &headers)?; let signature = match &self.credential.private_key { Some(key) => key.sign(&string_to_sign)?, None => client.sign_blob(&string_to_sign, email).await?, @@ -892,18 +901,21 @@ impl GCSAuthorizer { request_method: &Method, url: &Url, headers: &HeaderMap, - ) -> String { + ) -> Result { let canonical_request = Self::canonicalize_request(url, request_method, headers); - let hashed_canonical_req = hex_digest(canonical_request.as_bytes()); + let hashed_canonical_req = self + .crypto_provider + .hex_digest(canonical_request.as_bytes()) + .map_err(|e| Error::Crypto(Box::new(e)))?; let scope = self.scope(date); - format!( + Ok(format!( "{}\n{}\n{}\n{}", "GOOG4-RSA-SHA256", date.format("%Y%m%dT%H%M%SZ"), scope, hashed_canonical_req - ) + )) } } diff --git a/src/gcp/mod.rs b/src/gcp/mod.rs index 442b24fe..beb23afc 100644 --- a/src/gcp/mod.rs +++ b/src/gcp/mod.rs @@ -38,6 +38,7 @@ use std::sync::Arc; use std::time::Duration; use crate::client::CredentialProvider; +use crate::crypto::CryptoProviderRef; use crate::gcp::credential::GCSAuthorizer; use crate::signer::Signer; use crate::{ @@ -75,6 +76,7 @@ pub type GcpSigningCredentialProvider = /// Interface for [Google Cloud Storage](https://cloud.google.com/storage/). #[derive(Debug, Clone)] pub struct GoogleCloudStorage { + crypto_provider: CryptoProviderRef, client: Arc, } @@ -259,7 +261,7 @@ impl Signer for GoogleCloudStorage { })?; let signing_credentials = self.signing_credentials().get_credential().await?; - let authorizer = GCSAuthorizer::new(signing_credentials); + let authorizer = GCSAuthorizer::new(signing_credentials, self.crypto_provider.as_ref()); authorizer .sign(method, &mut url, expires_in, &self.client) From f4dbf3f63ba69baea20d016e2cb62cade8340ae9 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 17:39:21 -0700 Subject: [PATCH 13/15] Tests for gcp --- tests/crypto.rs | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/tests/crypto.rs b/tests/crypto.rs index e09aaa77..b61b7417 100644 --- a/tests/crypto.rs +++ b/tests/crypto.rs @@ -28,8 +28,9 @@ fn test_azure_default_crypto() { #[cfg(feature = "ring")] { - // Builder should build just fine with the default crypto provider - builder.build().unwrap(); + builder + .build() + .expect("default crypto should be configured"); } #[cfg(not(feature = "ring"))] @@ -53,8 +54,36 @@ fn test_aws_default_crypto() { #[cfg(feature = "ring")] { - // Builder should build ok with the default crypto provider - builder.build().unwrap(); + builder + .build() + .expect("default crypto should be configured"); + } + + #[cfg(not(feature = "ring"))] + { + let res = builder.build(); + assert!( + res.is_err(), + "Builder should fail without crypto configured" + ); + assert!(res + .unwrap_err() + .to_string() + .contains("Missing crypto provider.")); + } +} + +#[test] +#[cfg(feature = "gcp")] +fn test_gcp_default_crypto() { + let builder = + object_store::gcp::GoogleCloudStorageBuilder::default().with_bucket_name("testbucket"); + + #[cfg(feature = "ring")] + { + builder + .build() + .expect("default crypto should be configured"); } #[cfg(not(feature = "ring"))] From d0e85fac5f55cdc43969854d53dcef939d51c07e Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 17:44:18 -0700 Subject: [PATCH 14/15] gcp test --- src/gcp/builder.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/gcp/builder.rs b/src/gcp/builder.rs index 9ab6cfcd..670fd922 100644 --- a/src/gcp/builder.rs +++ b/src/gcp/builder.rs @@ -612,6 +612,8 @@ impl GoogleCloudStorageBuilder { #[cfg(test)] mod tests { + use crate::crypto; + use super::*; use std::collections::HashMap; use std::io::Write; @@ -764,4 +766,22 @@ mod tests { panic!("{key} not propagated as ClientConfigKey"); } } + + #[test] + fn gcp_test_crypto_configuration() { + let builder = GoogleCloudStorageBuilder::default() + .with_bucket_name("testbucket") + .with_crypto(Arc::from(crypto::noop_crypto::NoopCrypto {})); + + let bytes = b"hello world"; + assert_eq!( + builder + .crypto_provider + .unwrap() + .digest_sha256(bytes) + .unwrap() + .as_ref(), + bytes + ); + } } From 624c3e39c7c92eda9ce01077f796583efb768716 Mon Sep 17 00:00:00 2001 From: Jake Dern Date: Wed, 13 Aug 2025 17:50:50 -0700 Subject: [PATCH 15/15] Add feature tests to CI --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da2b8c46..b3d6b669 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -150,6 +150,13 @@ jobs: - name: Run object_store tests run: cargo test --features=aws,azure,gcp,http + - name: Run crypto feature tests + run: | + # With ring + cargo test crypto --no-default-features --features=aws,azure,gcp,ring + # Without ring + cargo test crypto --no-default-features --features=aws,azure,gcp + # Don't rerun doc tests (some of them rely on features other than aws) - name: Run object_store tests (AWS native conditional put) run: cargo test --lib --tests --features=aws