Skip to content

Commit a2021c0

Browse files
gaius-qigaius-qi
authored
feat(http): add sensitive header removal on redirect (#1699)
* feat(http): add sensitive header removal on redirect Signed-off-by: Gaius <gaius.qi@gmail.com> * chore(deps): update dragonfly-client dependencies to 1.2.12 Signed-off-by: Gaius <gaius.qi@gmail.com> --------- Signed-off-by: Gaius <gaius.qi@gmail.com>
1 parent c70d810 commit a2021c0

File tree

3 files changed

+115
-42
lines changed

3 files changed

+115
-42
lines changed

‎Cargo.lock‎

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

‎Cargo.toml‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ members = [
1313
]
1414

1515
[workspace.package]
16-
version = "1.2.11"
16+
version = "1.2.12"
1717
authors = ["The Dragonfly Developers"]
1818
homepage = "https://d7y.io/"
1919
repository = "https://github.com/dragonflyoss/client.git"
@@ -23,14 +23,14 @@ readme = "README.md"
2323
edition = "2021"
2424

2525
[workspace.dependencies]
26-
dragonfly-client = { path = "dragonfly-client", version = "1.2.11" }
27-
dragonfly-client-core = { path = "dragonfly-client-core", version = "1.2.11" }
28-
dragonfly-client-config = { path = "dragonfly-client-config", version = "1.2.11" }
29-
dragonfly-client-storage = { path = "dragonfly-client-storage", version = "1.2.11" }
30-
dragonfly-client-backend = { path = "dragonfly-client-backend", version = "1.2.11" }
31-
dragonfly-client-metric = { path = "dragonfly-client-metric", version = "1.2.11" }
32-
dragonfly-client-util = { path = "dragonfly-client-util", version = "1.2.11" }
33-
dragonfly-client-init = { path = "dragonfly-client-init", version = "1.2.11" }
26+
dragonfly-client = { path = "dragonfly-client", version = "1.2.12" }
27+
dragonfly-client-core = { path = "dragonfly-client-core", version = "1.2.12" }
28+
dragonfly-client-config = { path = "dragonfly-client-config", version = "1.2.12" }
29+
dragonfly-client-storage = { path = "dragonfly-client-storage", version = "1.2.12" }
30+
dragonfly-client-backend = { path = "dragonfly-client-backend", version = "1.2.12" }
31+
dragonfly-client-metric = { path = "dragonfly-client-metric", version = "1.2.12" }
32+
dragonfly-client-util = { path = "dragonfly-client-util", version = "1.2.12" }
33+
dragonfly-client-init = { path = "dragonfly-client-init", version = "1.2.12" }
3434
dragonfly-api = "=2.2.24"
3535
thiserror = "2.0"
3636
futures = "0.3.32"

‎dragonfly-client-backend/src/http.rs‎

Lines changed: 97 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
//!
4545
//! Authentication is handled through custom HTTP headers configured in the dfdaemon
4646
//! configuration file or passed directly in the request headers.
47-
//!
47+
4848
use crate::{
4949
Backend, Body, ExistsRequest, GetRequest, GetResponse, PutRequest, PutResponse, StatRequest,
5050
StatResponse, DEFAULT_USER_AGENT, KEEP_ALIVE_INTERVAL, MAX_RETRY_TIMES, POOL_MAX_IDLE_PER_HOST,
@@ -74,6 +74,7 @@ use std::time::{Duration, Instant};
7474
use tokio::sync::Mutex;
7575
use tokio_util::io::StreamReader;
7676
use tracing::{debug, error, info, instrument};
77+
use url::Url;
7778

7879
/// HTTP_SCHEME is the HTTP scheme.
7980
pub const HTTP_SCHEME: &str = "http";
@@ -311,7 +312,7 @@ impl HTTP {
311312
}
312313

313314
/// Get the cached temporary redirect URL if exists and not expired.
314-
async fn get_temporary_redirect_url(&self, url: &str) -> String {
315+
async fn get_temporary_redirect_url(&self, url: &str) -> Option<String> {
315316
let mut temporary_redirects = self.temporary_redirects.lock().await;
316317
if let Some(entry) = temporary_redirects.get(url) {
317318
if entry.created_at + self.cache_temporary_redirect_ttl > Instant::now() {
@@ -320,14 +321,14 @@ impl HTTP {
320321
url, entry.url
321322
);
322323

323-
return entry.url.clone();
324+
return Some(entry.url.clone());
324325
} else {
325326
debug!("cached temporary redirect for {} expired", url);
326327
temporary_redirects.pop(url);
327328
}
328329
}
329330

330-
url.to_string()
331+
None
331332
}
332333

333334
/// Store the temporary redirect URL in the cache.
@@ -380,30 +381,56 @@ impl Backend for HTTP {
380381
self.make_request_headers(&mut request_header, None)?;
381382

382383
// Check if we have a cached temporary redirect for this URL.
383-
let target_url = self.get_temporary_redirect_url(&request.url).await;
384+
let (request_url, request_header) =
385+
match self.get_temporary_redirect_url(&request.url).await {
386+
Some(redirect_url) => {
387+
let mut redirect_headers = request_header.clone();
388+
remove_sensitive_headers(
389+
&mut redirect_headers,
390+
&redirect_url.parse()?,
391+
&request.url.parse()?,
392+
);
393+
394+
(redirect_url, redirect_headers)
395+
}
396+
None => (request.url.clone(), request_header),
397+
};
384398

385399
// The signature in the signed URL generated by the object storage client will include
386400
// the request method. Therefore, the signed URL of the GET method cannot be requested
387401
// through the HEAD method. Use GET request to replace of HEAD request
388402
// to get header and status code.
389403
let response = match self
390404
.client(request.client_cert.clone(), self.enable_hickory_dns)?
391-
.get(&target_url)
405+
.get(&request_url)
392406
.headers(request_header.clone())
393407
.timeout(request.timeout)
394408
.send()
395409
.await
396410
{
397411
Ok(response) if response.status() == reqwest::StatusCode::TEMPORARY_REDIRECT => {
398412
if let Some(location) = response.headers().get(LOCATION) {
399-
let target_url = location.to_str().or_err(ErrorType::ParseError)?;
400-
self.store_temporary_redirect_url(&request.url, target_url)
413+
let redirect_url = location.to_str().or_err(ErrorType::ParseError)?;
414+
debug!(
415+
"stat request got 307 Temporary Redirect, following redirect {} -> {}",
416+
request.url, redirect_url
417+
);
418+
419+
self.store_temporary_redirect_url(&request.url, redirect_url)
401420
.await;
402421

422+
// Strips sensitive headers when following a cross-origin redirect.
423+
let mut redirect_headers = request_header.clone();
424+
remove_sensitive_headers(
425+
&mut redirect_headers,
426+
&redirect_url.parse()?,
427+
&request.url.parse()?,
428+
);
429+
403430
match self
404431
.client(request.client_cert.clone(), self.enable_hickory_dns)?
405-
.get(target_url)
406-
.headers(request_header.clone())
432+
.get(redirect_url)
433+
.headers(redirect_headers)
407434
.timeout(request.timeout)
408435
.send()
409436
.await
@@ -412,7 +439,7 @@ impl Backend for HTTP {
412439
Err(err) => {
413440
error!(
414441
"stat request failed {} {}: {}",
415-
request.task_id, target_url, err
442+
request.task_id, redirect_url, err
416443
);
417444

418445
return Ok(StatResponse {
@@ -428,7 +455,7 @@ impl Backend for HTTP {
428455
} else {
429456
error!(
430457
"stat request got 307 Temporary Redirect without Location header {} {}",
431-
request.task_id, target_url
458+
request.task_id, request_url
432459
);
433460

434461
return Ok(StatResponse {
@@ -456,7 +483,7 @@ impl Backend for HTTP {
456483

457484
match self
458485
.client(request.client_cert.clone(), self.enable_hickory_dns)?
459-
.head(&target_url)
486+
.head(&request_url)
460487
.headers(request_header.clone())
461488
.timeout(request.timeout)
462489
.send()
@@ -466,7 +493,7 @@ impl Backend for HTTP {
466493
Err(err) => {
467494
error!(
468495
"stat request failed with HEAD {} {}: {}",
469-
request.task_id, target_url, err
496+
request.task_id, request_url, err
470497
);
471498

472499
return Ok(StatResponse {
@@ -484,7 +511,7 @@ impl Backend for HTTP {
484511
Err(err) => {
485512
error!(
486513
"stat request failed with GET {} {}: {}",
487-
request.task_id, target_url, err
514+
request.task_id, request_url, err
488515
);
489516

490517
return Ok(StatResponse {
@@ -507,7 +534,7 @@ impl Backend for HTTP {
507534

508535
debug!(
509536
"stat response {} {}: {:?} {:?} {:?}",
510-
request.task_id, target_url, response_status_code, content_length, response_header
537+
request.task_id, request_url, response_status_code, content_length, response_header
511538
);
512539

513540
// Drop the response body to avoid reading it.
@@ -542,10 +569,24 @@ impl Backend for HTTP {
542569
self.make_request_headers(&mut request_header, request.range)?;
543570

544571
// Check if we have a cached temporary redirect for this URL.
545-
let target_url = self.get_temporary_redirect_url(&request.url).await;
572+
let (request_url, request_header) =
573+
match self.get_temporary_redirect_url(&request.url).await {
574+
Some(redirect_url) => {
575+
let mut redirect_headers = request_header.clone();
576+
remove_sensitive_headers(
577+
&mut redirect_headers,
578+
&redirect_url.parse()?,
579+
&request.url.parse()?,
580+
);
581+
582+
(redirect_url, redirect_headers)
583+
}
584+
None => (request.url.clone(), request_header),
585+
};
586+
546587
let mut response = match self
547588
.client(request.client_cert.clone(), self.enable_hickory_dns)?
548-
.get(&target_url)
589+
.get(&request_url)
549590
.headers(request_header.clone())
550591
.timeout(request.timeout)
551592
.send()
@@ -555,7 +596,7 @@ impl Backend for HTTP {
555596
Err(err) => {
556597
error!(
557598
"get request failed {} {} {}: {}",
558-
request.task_id, request.piece_id, target_url, err
599+
request.task_id, request.piece_id, request_url, err
559600
);
560601

561602
return Ok(GetResponse {
@@ -571,14 +612,27 @@ impl Backend for HTTP {
571612
// If the response is a 307 Temporary Redirect, follow the redirect manually.
572613
if response.status() == reqwest::StatusCode::TEMPORARY_REDIRECT {
573614
if let Some(location) = response.headers().get(LOCATION) {
574-
let target_url = location.to_str().or_err(ErrorType::ParseError)?;
575-
self.store_temporary_redirect_url(&request.url, target_url)
615+
let redirect_url = location.to_str().or_err(ErrorType::ParseError)?;
616+
debug!(
617+
"get request got 307 Temporary Redirect, following redirect {} -> {}",
618+
request.url, redirect_url
619+
);
620+
621+
self.store_temporary_redirect_url(&request.url, redirect_url)
576622
.await;
577623

624+
// Strips sensitive headers when following a cross-origin redirect.
625+
let mut redirect_headers = request_header.clone();
626+
remove_sensitive_headers(
627+
&mut redirect_headers,
628+
&redirect_url.parse()?,
629+
&request.url.parse()?,
630+
);
631+
578632
response = match self
579633
.client(request.client_cert.clone(), self.enable_hickory_dns)?
580-
.get(target_url)
581-
.headers(request_header.clone())
634+
.get(redirect_url)
635+
.headers(redirect_headers)
582636
.timeout(request.timeout)
583637
.send()
584638
.await
@@ -587,7 +641,7 @@ impl Backend for HTTP {
587641
Err(err) => {
588642
error!(
589643
"get request failed {} {} {}: {}",
590-
request.task_id, request.piece_id, target_url, err
644+
request.task_id, request.piece_id, redirect_url, err
591645
);
592646

593647
return Ok(GetResponse {
@@ -717,6 +771,25 @@ impl Backend for HTTP {
717771
}
718772
}
719773

774+
/// Strips sensitive headers when following a cross-origin redirect.
775+
///
776+
/// This replicates the behavior of reqwest's internal `remove_sensitive_headers`:
777+
/// https://github.com/seanmonstar/reqwest/blob/v0.12.15/src/redirect.rs#L134
778+
///
779+
/// Per RFC 9110 §15.4, user agents SHOULD strip credentials when redirecting
780+
/// to a different origin.
781+
fn remove_sensitive_headers(headers: &mut HeaderMap, next: &Url, previous: &Url) {
782+
if next.host() != previous.host()
783+
|| next.port_or_known_default() != previous.port_or_known_default()
784+
{
785+
headers.remove(reqwest::header::AUTHORIZATION);
786+
headers.remove(reqwest::header::COOKIE);
787+
headers.remove("cookie2");
788+
headers.remove(reqwest::header::PROXY_AUTHORIZATION);
789+
headers.remove(reqwest::header::WWW_AUTHENTICATE);
790+
}
791+
}
792+
720793
#[cfg(test)]
721794
mod tests {
722795
use super::*;

0 commit comments

Comments
 (0)