Skip to content

Commit

Permalink
Auto merge of #12095 - ehuss:fix-token-redact, r=weihanglo
Browse files Browse the repository at this point in the history
Fix redacting tokens in http debug.

Unfortunately it seems like #8222 didn't properly redact tokens when connecting to an http2 server. There were multiple problems:

* For some reason, curl changes the authorization header to be lowercase when using http2.
* Curl also logs the h2h3 lines separately with a different syntax.

This fixes it by checking for these additional cases.

This also adds a test, but it doesn't actually detect this problem because we don't have an http2 server handy. You can test this yourself by running `CARGO_LOG=trace CARGO_HTTP_DEBUG=true cargo publish --token a-unique-token --allow-dirty --no-verify`, and verifying the output does not contain the given token text.
  • Loading branch information
bors committed May 6, 2023
2 parents 5409b7f + d6c20cf commit 27a41d6
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 3 deletions.
9 changes: 7 additions & 2 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,12 +704,17 @@ pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult<
InfoType::SslDataIn | InfoType::SslDataOut => return,
_ => return,
};
let starts_with_ignore_case = |line: &str, text: &str| -> bool {
line[..line.len().min(text.len())].eq_ignore_ascii_case(text)
};
match str::from_utf8(data) {
Ok(s) => {
for mut line in s.lines() {
if line.starts_with("Authorization:") {
if starts_with_ignore_case(line, "authorization:") {
line = "Authorization: [REDACTED]";
} else if line[..line.len().min(10)].eq_ignore_ascii_case("set-cookie") {
} else if starts_with_ignore_case(line, "h2h3 [authorization:") {
line = "h2h3 [Authorization: [REDACTED]]";
} else if starts_with_ignore_case(line, "set-cookie") {
line = "set-cookie: [REDACTED]";
}
log!(level, "http-debug: {} {}", prefix, line);
Expand Down
73 changes: 72 additions & 1 deletion tests/testsuite/registry_auth.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Tests for registry authentication.

use cargo_test_support::registry::{Package, RegistryBuilder};
use cargo_test_support::compare::match_contains;
use cargo_test_support::registry::{Package, RegistryBuilder, Token};
use cargo_test_support::{project, Execs, Project};

fn cargo(p: &Project, s: &str) -> Execs {
Expand Down Expand Up @@ -517,3 +518,73 @@ Caused by:
)
.run();
}

#[cargo_test]
fn token_not_logged() {
// Checks that the token isn't displayed in debug output (for both HTTP
// index and registry API). Note that this doesn't fully verify the
// correct behavior since we don't have an HTTP2 server, and curl behaves
// significantly differently when using HTTP2.
let crates_io = RegistryBuilder::new()
.http_api()
.http_index()
.auth_required()
.token(Token::Plaintext("a-unique_token".to_string()))
.build();
Package::new("bar", "1.0.0").publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();
let output = cargo(&p, "publish")
.replace_crates_io(crates_io.index_url())
.env("CARGO_HTTP_DEBUG", "true")
.env("CARGO_LOG", "trace")
.exec_with_output()
.unwrap();
let log = String::from_utf8(output.stderr).unwrap();
let lines = "\
[UPDATING] crates.io index
[PACKAGING] foo v0.1.0 [..]
[VERIFYING] foo v0.1.0 [..]
[DOWNLOADING] crates ...
[DOWNLOADED] bar v1.0.0
[COMPILING] bar v1.0.0
[COMPILING] foo v0.1.0 [..]
[FINISHED] [..]
[PACKAGED] 3 files[..]
[UPLOADING] foo v0.1.0[..]
[UPLOADED] foo v0.1.0 to registry `crates-io`
note: Waiting [..]
";
for line in lines.lines() {
match_contains(line, &log, None).unwrap();
}
let authorizations: Vec<_> = log
.lines()
.filter(|line| {
line.contains("http-debug:") && line.to_lowercase().contains("authorization")
})
.collect();
assert!(authorizations.iter().all(|line| line.contains("REDACTED")));
// Total authorizations:
// 1. Initial config.json
// 2. config.json again for verification
// 3. /index/3/b/bar
// 4. /dl/bar/1.0.0/download
// 5. /api/v1/crates/new
// 6. config.json for the "wait for publish"
// 7. /index/3/f/foo for the "wait for publish"
assert_eq!(authorizations.len(), 7);
assert!(!log.contains("a-unique_token"));
}

0 comments on commit 27a41d6

Please sign in to comment.