Skip to content

Commit a78a05f

Browse files
CopilotovertrueCopilot
authored
feat: wire up insecure TLS option for S3 client (#17)
* Initial plan * feat: add insecure TLS support for S3 client connections When an alias is configured with `insecure = true` (via `rc alias set --insecure`), the S3 client now uses a reqwest-based HTTP connector that calls `danger_accept_invalid_certs(true)`, allowing connections to servers using self-signed TLS certificates. Also supports `ca_bundle` for custom CA certificate bundles. Implements the custom connector by wrapping reqwest::Client in the aws-smithy-runtime-api HttpConnector and HttpClient traits, and wiring it into the aws-config builder when insecure mode or ca_bundle is set. Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com> * Update crates/s3/src/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: 安正超 <anzhengchao@gmail.com> * Update crates/s3/src/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: 安正超 <anzhengchao@gmail.com> * Update crates/s3/src/client.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: 安正超 <anzhengchao@gmail.com> * refactor(s3): address PR review feedback on ReqwestConnector - Switch CA bundle file read from std::fs::read (blocking) to tokio::fs::read (async), avoiding blocking the async runtime - Fix streaming body handling: return a clear ConnectorError instead of silently sending an empty body when request.body().bytes() is None - Make ReqwestConnector::new() async and update call site to .await - Convert unit tests to #[tokio::test] async fns to match async new() - Remove the unreliable reqwest_connector_invalid_ca_bundle_contents test: reqwest with rustls does not validate certificate DER content at build time, making this test non-deterministic across versions Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com> --------- Signed-off-by: 安正超 <anzhengchao@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com> Co-authored-by: 安正超 <anzhengchao@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 9fb5cc6 commit a78a05f

4 files changed

Lines changed: 185 additions & 6 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ aws-sdk-s3 = "1.119"
2727
aws-config = { version = "1.8", features = ["behavior-version-latest"] }
2828
aws-credential-types = "1.2"
2929
aws-smithy-types = "1.3"
30+
aws-smithy-runtime-api = "1.9"
3031

3132
# CLI
3233
clap = { version = "4.5", features = ["derive", "env"] }
@@ -54,6 +55,7 @@ console = "0.16"
5455
dirs = "6.0"
5556
jiff = { version = "0.2", features = ["serde"] }
5657
humansize = "2.1"
58+
bytes = "1.11"
5759
url = "2.5"
5860
futures = "0.3"
5961
async-trait = "0.1"

crates/s3/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ aws-sdk-s3.workspace = true
2020
aws-config.workspace = true
2121
aws-credential-types.workspace = true
2222
aws-smithy-types.workspace = true
23+
aws-smithy-runtime-api.workspace = true
2324

2425
# Async
2526
tokio.workspace = true
@@ -35,6 +36,7 @@ tracing.workspace = true
3536

3637
# Utilities
3738
jiff.workspace = true
39+
bytes.workspace = true
3840
url.workspace = true
3941

4042
# HTTP client for Admin API

crates/s3/src/client.rs

Lines changed: 179 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,153 @@
33
//! Wraps aws-sdk-s3 and implements the ObjectStore trait from rc-core.
44
55
use async_trait::async_trait;
6-
6+
use aws_smithy_runtime_api::client::http::{
7+
HttpClient, HttpConnector, HttpConnectorFuture, HttpConnectorSettings, SharedHttpConnector,
8+
};
9+
use aws_smithy_runtime_api::client::orchestrator::HttpRequest;
10+
use aws_smithy_runtime_api::client::result::ConnectorError;
11+
use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents;
12+
use aws_smithy_runtime_api::http::{Response, StatusCode};
13+
use aws_smithy_types::body::SdkBody;
14+
use bytes::Bytes;
715
use jiff::Timestamp;
816
use rc_core::{
917
Alias, Capabilities, Error, ListOptions, ListResult, ObjectInfo, ObjectStore, ObjectVersion,
1018
RemotePath, Result,
1119
};
1220

21+
/// Custom HTTP connector using reqwest, supporting insecure TLS (skip cert verification)
22+
/// and custom CA bundles. Used when `alias.insecure = true` or `alias.ca_bundle.is_some()`.
23+
#[derive(Debug, Clone)]
24+
struct ReqwestConnector {
25+
client: reqwest::Client,
26+
}
27+
28+
impl ReqwestConnector {
29+
async fn new(insecure: bool, ca_bundle: Option<&str>) -> Result<Self> {
30+
// NOTE: When `insecure = true`, `danger_accept_invalid_certs` disables all TLS
31+
// certificate verification. Any CA bundle provided will still be added to the
32+
// trust store but is rendered ineffective for this connection.
33+
let mut builder = reqwest::Client::builder().danger_accept_invalid_certs(insecure);
34+
35+
if let Some(bundle_path) = ca_bundle {
36+
// Use tokio::fs::read to avoid blocking the async runtime thread.
37+
let pem = tokio::fs::read(bundle_path).await.map_err(|e| {
38+
Error::Network(format!("Failed to read CA bundle '{bundle_path}': {e}"))
39+
})?;
40+
let cert = reqwest::Certificate::from_pem(&pem)
41+
.map_err(|e| Error::Network(format!("Invalid CA bundle '{bundle_path}': {e}")))?;
42+
builder = builder.add_root_certificate(cert);
43+
}
44+
45+
let client = builder
46+
.build()
47+
.map_err(|e| Error::Network(format!("Failed to build HTTP client: {e}")))?;
48+
Ok(Self { client })
49+
}
50+
}
51+
52+
impl HttpConnector for ReqwestConnector {
53+
fn call(&self, request: HttpRequest) -> HttpConnectorFuture {
54+
let client = self.client.clone();
55+
HttpConnectorFuture::new(async move {
56+
// Extract request parts before consuming the request
57+
let uri = request.uri().to_string();
58+
let method_str = request.method().to_string();
59+
let headers = request.headers().clone();
60+
61+
// Try to get the body as buffered in-memory bytes.
62+
// For streaming bodies (e.g., large file uploads), bytes() returns None and we
63+
// return a clear error rather than silently sending an empty body, which would
64+
// cause signature mismatches or server-side failures.
65+
let body_bytes = match request.body().bytes() {
66+
Some(b) => Bytes::copy_from_slice(b),
67+
None => {
68+
return Err(ConnectorError::user(
69+
"Streaming request bodies are not supported in insecure/ca_bundle TLS mode; \
70+
use in-memory data for uploads with this connector"
71+
.into(),
72+
));
73+
}
74+
};
75+
76+
// Build reqwest method
77+
let method = reqwest::Method::from_bytes(method_str.as_bytes())
78+
.map_err(|e| ConnectorError::user(Box::new(e)))?;
79+
80+
// Build reqwest URL
81+
let url = reqwest::Url::parse(&uri).map_err(|e| ConnectorError::user(Box::new(e)))?;
82+
83+
// Build reqwest request
84+
let mut req = reqwest::Request::new(method, url);
85+
86+
// Copy headers; S3 headers are all ASCII so failures here are unexpected
87+
for (name, value) in headers.iter() {
88+
match (
89+
reqwest::header::HeaderName::from_bytes(name.as_bytes()),
90+
reqwest::header::HeaderValue::from_bytes(value.as_bytes()),
91+
) {
92+
(Ok(header_name), Ok(header_value)) => {
93+
req.headers_mut().append(header_name, header_value);
94+
}
95+
_ => {
96+
tracing::warn!("Skipping non-convertible request header: {}", name);
97+
}
98+
}
99+
}
100+
101+
// Set body
102+
*req.body_mut() = Some(reqwest::Body::from(body_bytes));
103+
104+
// Execute
105+
let resp = client
106+
.execute(req)
107+
.await
108+
.map_err(|e| ConnectorError::io(Box::new(e)))?;
109+
110+
// Convert response
111+
let status = StatusCode::try_from(resp.status().as_u16())
112+
.map_err(|e| ConnectorError::other(Box::new(e), None))?;
113+
let resp_headers = resp.headers().clone();
114+
let body = resp
115+
.bytes()
116+
.await
117+
.map_err(|e| ConnectorError::io(Box::new(e)))?;
118+
119+
let mut sdk_response = Response::new(status, SdkBody::from(body));
120+
for (name, value) in &resp_headers {
121+
match value.to_str() {
122+
Ok(value_str) => {
123+
sdk_response
124+
.headers_mut()
125+
.append(name.as_str().to_owned(), value_str.to_owned());
126+
}
127+
Err(_) => {
128+
tracing::warn!("Skipping non-UTF8 response header: {}", name.as_str());
129+
}
130+
}
131+
}
132+
133+
Ok(sdk_response)
134+
})
135+
}
136+
}
137+
138+
impl HttpClient for ReqwestConnector {
139+
fn http_connector(
140+
&self,
141+
_settings: &HttpConnectorSettings,
142+
_components: &RuntimeComponents,
143+
) -> SharedHttpConnector {
144+
// NOTE: `ReqwestConnector` is preconfigured (e.g., insecure/CA-bundle options) when it
145+
// is constructed, and does not currently apply `HttpConnectorSettings`. This means
146+
// behavior in this mode may differ from the default connector w.r.t. SDK HTTP settings.
147+
// If alignment is required, map relevant fields from `HttpConnectorSettings` onto the
148+
// internal `reqwest::Client` when constructing the connector.
149+
SharedHttpConnector::new(self.clone())
150+
}
151+
}
152+
13153
/// S3 client wrapper
14154
pub struct S3Client {
15155
inner: aws_sdk_s3::Client,
@@ -34,13 +174,21 @@ impl S3Client {
34174
"rc-static-credentials",
35175
);
36176

37-
// Build SDK config
38-
let config = aws_config::defaults(aws_config::BehaviorVersion::latest())
177+
// Build SDK config loader
178+
let mut config_loader = aws_config::defaults(aws_config::BehaviorVersion::latest())
39179
.credentials_provider(credentials)
40180
.region(aws_config::Region::new(region))
41-
.endpoint_url(&endpoint)
42-
.load()
43-
.await;
181+
.endpoint_url(&endpoint);
182+
183+
// When insecure mode is enabled or a custom CA bundle is provided, use the reqwest
184+
// connector which supports danger_accept_invalid_certs and custom root certificates.
185+
if alias.insecure || alias.ca_bundle.is_some() {
186+
let connector =
187+
ReqwestConnector::new(alias.insecure, alias.ca_bundle.as_deref()).await?;
188+
config_loader = config_loader.http_client(connector);
189+
}
190+
191+
let config = config_loader.load().await;
44192

45193
// Build S3 client with path-style addressing for compatibility
46194
let s3_config = aws_sdk_s3::config::Builder::from(&config)
@@ -736,4 +884,29 @@ mod tests {
736884
assert_eq!(info.key, "test.txt");
737885
assert_eq!(info.size_bytes, Some(1024));
738886
}
887+
888+
#[tokio::test]
889+
async fn reqwest_connector_insecure_without_ca_bundle_succeeds() {
890+
// When insecure is true and no CA bundle is provided, the connector should be created.
891+
let connector = ReqwestConnector::new(true, None).await;
892+
assert!(
893+
connector.is_ok(),
894+
"Expected insecure connector creation to succeed"
895+
);
896+
}
897+
898+
#[tokio::test]
899+
async fn reqwest_connector_invalid_ca_bundle_path_surfaces_error() {
900+
// Use an obviously invalid path (empty string) to trigger a read error.
901+
let result = ReqwestConnector::new(false, Some("")).await;
902+
match result {
903+
Err(Error::Network(msg)) => {
904+
assert!(
905+
msg.contains("Failed to read CA bundle"),
906+
"Unexpected error message: {msg}"
907+
);
908+
}
909+
other => panic!("Expected Error::Network for invalid path, got: {:?}", other),
910+
}
911+
}
739912
}

0 commit comments

Comments
 (0)