Skip to content

Commit

Permalink
Error on invalid token for registry auth
Browse files Browse the repository at this point in the history
When using registry operations with authentication there will be now an
error if the given token is not valid.
This is a technically a breaking change because a registry might give
some tokens which will be denied by these new checks.
In practice these tokens cause issues with HTTP so no registry should
generate them.
  • Loading branch information
Akida31 committed Feb 16, 2023
1 parent 2f84e1a commit 3d2e107
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 0 deletions.
21 changes: 21 additions & 0 deletions crates/crates-io/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ impl Registry {
Some(s) => s,
None => bail!("no upload token found, please run `cargo login`"),
};
check_token(token)?;
headers.append(&format!("Authorization: {}", token))?;
}
self.handle.http_headers(headers)?;
Expand Down Expand Up @@ -510,3 +511,23 @@ pub fn is_url_crates_io(url: &str) -> bool {
.map(|u| u.host_str() == Some("crates.io"))
.unwrap_or(false)
}

/// Checks if a token is valid or malformed.
///
/// This check is necessary to prevent sending tokens which create an invalid HTTP request.
/// It would be easier to check just for alphanumeric tokens, but we can't be sure that all
/// registries only create tokens in that format so that is as less restricted as possible.
pub fn check_token(token: &str) -> Result<()> {
let is_valid = token.bytes().all(|b| {
b >= 32 // undefined in ISO-8859-1, in ASCII/ UTF-8 not-printable character
&& b < 128 // utf-8: the first bit signals a multi-byte character
&& b != 127 // 127 is a control character in ascii and not in ISO 8859-1
|| b == b't' // tab is also allowed (even when < 32)
});

if is_valid {
Ok(())
} else {
Err(anyhow::anyhow!("invalid token."))
}
}
1 change: 1 addition & 0 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ pub fn registry_login(
if tok.is_empty() {
bail!("please provide a non-empty token");
}
crates_io::check_token(tok.as_ref().expose())?;
}
}
if &reg_cfg == &new_token {
Expand Down
32 changes: 32 additions & 0 deletions tests/testsuite/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,38 @@ fn empty_login_token() {
.run();
}

#[cargo_test]
fn invalid_login_token() {
let registry = RegistryBuilder::new()
.no_configure_registry()
.no_configure_token()
.build();
setup_new_credentials();

let check_ = |stdin: &str, stderr: &str| {
cargo_process("login")
.replace_crates_io(registry.index_url())
.with_stdout("please paste the token found on [..]/me below")
.with_stdin(stdin)
.with_stderr(stderr)
.with_status(101)
.run();
};
let check = |stdin: &str| {
check_(stdin, "[ERROR] invalid token.");
};
// first check updates index so it must be handled differently
check_(
"😄",
"\
[UPDATING] crates.io index
[ERROR] invalid token.",
);
check("\u{0016}");
check("\u{0000}");
check("你好");
}

#[cargo_test]
fn bad_asymmetric_token_args() {
// These cases are kept brief as the implementation is covered by clap, so this is only smoke testing that we have clap configured correctly.
Expand Down

0 comments on commit 3d2e107

Please sign in to comment.