clawhdf5-remote, h5rs: URLs' credentials are never shown
Every RemoteError message and HttpStorage's Debug output held the URL as given, with any user:password@ and the query string — for a presigned S3/GCS/Azure URL, its signature or token. An application logging the error leaked the credential. - New clawhdf5_remote::redact_url: no userinfo, no fragment, query values replaced by REDACTED (plain key names kept). - HttpStorage formats every message with the redacted URL, and scrubs the URL's secret parts from errors of the HTTP client (whose texts can echo the URI); Debug shows the redacted URL. storage_for_url's and the object store URL errors are redacted too. HttpStorage::url() still returns the URL as given, documented as not for logging. - h5rs prints FILE arguments that are URLs redacted: in errors and in dump/stat/check/diff output. - The test server can force a status and send a wrong Content-Range. Tests: 404, 403 (at open and on a read), wrong Content-Range (at open and on a read), no range support, encoded body, ETag change, timeout, connection closed and bad scheme errors, Display and Debug, contain none of the secrets; h5rs likewise for every subcommand. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -52,6 +52,11 @@ pub struct Shared {
|
||||
/// when checking ranges) and serve zeros past its real end: a hostile
|
||||
/// server lying about the length.
|
||||
pub fake_total: AtomicU64,
|
||||
/// When non-zero, answer every request for a served file with this
|
||||
/// status (and an empty body).
|
||||
pub force_status: AtomicU32,
|
||||
/// Answer ranges with a `Content-Range` one byte off.
|
||||
pub wrong_range: AtomicBool,
|
||||
/// Requests for a served path (every status); requests for other
|
||||
/// paths are not counted.
|
||||
pub requests: AtomicU64,
|
||||
@@ -256,6 +261,11 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> {
|
||||
if delay > 0 {
|
||||
std::thread::sleep(Duration::from_millis(delay));
|
||||
}
|
||||
let forced = s.force_status.load(Ordering::SeqCst);
|
||||
if forced != 0 {
|
||||
write!(out, "HTTP/1.1 {forced} Forced\r\nContent-Length: 0\r\n\r\n")?;
|
||||
continue;
|
||||
}
|
||||
if s.fail_next
|
||||
.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |n| n.checked_sub(1))
|
||||
.is_ok()
|
||||
@@ -309,7 +319,11 @@ fn serve(conn: TcpStream, s: &Shared) -> std::io::Result<()> {
|
||||
Some(Ok((a, b))) => (
|
||||
"206 Partial Content",
|
||||
slice_or_zeros(&data, a, b, &mut padded),
|
||||
format!("Content-Range: bytes {a}-{b}/{len}\r\n"),
|
||||
if s.wrong_range.load(Ordering::SeqCst) {
|
||||
format!("Content-Range: bytes {}-{}/{len}\r\n", a + 1, b + 1)
|
||||
} else {
|
||||
format!("Content-Range: bytes {a}-{b}/{len}\r\n")
|
||||
},
|
||||
),
|
||||
None => ("200 OK", &data[..], String::new()),
|
||||
};
|
||||
|
||||
@@ -604,3 +604,121 @@ fn download_is_bounded_by_its_limit_not_the_claimed_length() {
|
||||
);
|
||||
assert_eq!(server.requests(), 0, "refused before reading");
|
||||
}
|
||||
|
||||
/// A URL's credentials — userinfo, and the query string of a presigned
|
||||
/// URL — never appear in an error message or a `Debug` output, whatever
|
||||
/// failed.
|
||||
#[test]
|
||||
fn credentials_never_appear_in_errors_or_debug() {
|
||||
const SECRETS: [&str; 4] = ["hunter2", "SECRETSIG", "AKIDSECRET", "user:"];
|
||||
fn clean(what: &str, text: &str) {
|
||||
for s in SECRETS {
|
||||
assert!(!text.contains(s), "{what}: {s} leaked in {text}");
|
||||
}
|
||||
}
|
||||
fn check_err<T>(what: &str, r: Result<T, Error>) {
|
||||
let Err(e) = r else {
|
||||
panic!("{what}: expected an error")
|
||||
};
|
||||
clean(what, &format!("{e}"));
|
||||
clean(what, &format!("{e:?}"));
|
||||
}
|
||||
let bytes = multi_block_file();
|
||||
let server = Server::start(vec![("/m.h5".into(), bytes.clone())]);
|
||||
let secret_url = |path: &str| {
|
||||
format!(
|
||||
"http://user:hunter2@{}{path}?X-Amz-Credential=AKIDSECRET&X-Amz-Signature=SECRETSIG",
|
||||
server.addr
|
||||
)
|
||||
};
|
||||
let url = secret_url("/m.h5");
|
||||
let mut opts = quick();
|
||||
opts.http.retries = 0;
|
||||
|
||||
// Opening works through such a URL, and Debug shows it redacted.
|
||||
let (http, _) = HttpStorage::open(&url, opts.http.clone()).unwrap();
|
||||
let debug = format!("{http:?}");
|
||||
clean("Debug", &debug);
|
||||
assert!(debug.contains("X-Amz-Signature=REDACTED"), "{debug}");
|
||||
assert_eq!(http.url(), url, "the URL itself is kept for requests");
|
||||
|
||||
check_err("404", open_url_with(&secret_url("/missing.h5"), &opts));
|
||||
server.shared.force_status.store(403, Ordering::SeqCst);
|
||||
check_err("403", open_url_with(&url, &opts));
|
||||
server.shared.force_status.store(0, Ordering::SeqCst);
|
||||
server.shared.wrong_range.store(true, Ordering::SeqCst);
|
||||
check_err("bad range at open", open_url_with(&url, &opts));
|
||||
server.shared.wrong_range.store(false, Ordering::SeqCst);
|
||||
server.shared.ignore_range.store(true, Ordering::SeqCst);
|
||||
check_err("no range support", open_url_with(&url, &opts));
|
||||
server.shared.ignore_range.store(false, Ordering::SeqCst);
|
||||
server.shared.gzip_label.store(true, Ordering::SeqCst);
|
||||
check_err("encoded body", open_url_with(&url, &opts));
|
||||
server.shared.gzip_label.store(false, Ordering::SeqCst);
|
||||
|
||||
// Errors of reads after the open, through a File.
|
||||
let f = open_url_with(&url, &opts).unwrap();
|
||||
server.shared.wrong_range.store(true, Ordering::SeqCst);
|
||||
check_err(
|
||||
"bad range",
|
||||
f.dataset("big")
|
||||
.map(|d| d.read_f64())
|
||||
.and_then(|r| r)
|
||||
.map_err(Error::Hdf5),
|
||||
);
|
||||
server.shared.wrong_range.store(false, Ordering::SeqCst);
|
||||
let f = open_url_with(&url, &opts).unwrap();
|
||||
server.shared.force_status.store(403, Ordering::SeqCst);
|
||||
check_err(
|
||||
"403 on a read",
|
||||
f.dataset("big")
|
||||
.map(|d| d.read_f64())
|
||||
.and_then(|r| r)
|
||||
.map_err(Error::Hdf5),
|
||||
);
|
||||
server.shared.force_status.store(0, Ordering::SeqCst);
|
||||
let f = open_url_with(&url, &opts).unwrap();
|
||||
let mut other = bytes.clone();
|
||||
let n = other.len();
|
||||
other[n / 2] ^= 0xff;
|
||||
server.shared.put("/m.h5", other);
|
||||
check_err(
|
||||
"ETag change",
|
||||
f.dataset("big")
|
||||
.map(|d| d.read_f64())
|
||||
.and_then(|r| r)
|
||||
.map_err(Error::Hdf5),
|
||||
);
|
||||
// A timeout.
|
||||
let mut slow = opts.clone();
|
||||
slow.http.timeout = Duration::from_millis(100);
|
||||
server.shared.delay_ms.store(1000, Ordering::SeqCst);
|
||||
check_err("timeout", open_url_with(&url, &slow));
|
||||
server.shared.delay_ms.store(0, Ordering::SeqCst);
|
||||
// A connection closed unanswered, a bad scheme.
|
||||
let port = {
|
||||
let l = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
let port = l.local_addr().unwrap().port();
|
||||
std::thread::spawn(move || {
|
||||
for c in l.incoming() {
|
||||
drop(c);
|
||||
}
|
||||
});
|
||||
port
|
||||
};
|
||||
check_err(
|
||||
"connection closed",
|
||||
open_url_with(
|
||||
&format!("http://user:[email protected]:{port}/a.h5?X-Amz-Signature=SECRETSIG"),
|
||||
&opts,
|
||||
),
|
||||
);
|
||||
check_err(
|
||||
"scheme",
|
||||
open_url("ftp://user:[email protected]/a.h5?X-Amz-Signature=SECRETSIG"),
|
||||
);
|
||||
assert_eq!(
|
||||
clawhdf5_remote::redact_url("https://me:pw@host:8/d/f.h5?X-Amz-Signature=abc&a=1#frag"),
|
||||
"https://host:8/d/f.h5?X-Amz-Signature=REDACTED&a=REDACTED"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user