From ff699e4a25b334be25eea07add2036877db83b50 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 31 Jan 2025 14:41:42 +0000 Subject: [PATCH 1/4] Tokens are ASCII-only --- crates/crates-io/lib.rs | 2 +- tests/testsuite/login.rs | 2 +- tests/testsuite/publish.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/crates-io/lib.rs b/crates/crates-io/lib.rs index ae7f9c4daa3..c1265974d5d 100644 --- a/crates/crates-io/lib.rs +++ b/crates/crates-io/lib.rs @@ -545,7 +545,7 @@ pub fn check_token(token: &str) -> Result<()> { Ok(()) } else { Err(Error::InvalidToken( - "token contains invalid characters.\nOnly printable ISO-8859-1 characters \ + "token contains invalid characters.\nOnly printable ASCII characters \ are allowed as it is sent in a HTTPS header.", )) } diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index cac0e718af5..088cc6f6266 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -182,7 +182,7 @@ fn invalid_login_token() { Caused by: token contains invalid characters. - Only printable ISO-8859-1 characters are allowed as it is sent in a HTTPS header. + Only printable ASCII characters are allowed as it is sent in a HTTPS header. ", 101, ) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 4e322431bd0..6fd99767f27 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3616,7 +3616,7 @@ fn invalid_token() { Caused by: token contains invalid characters. - Only printable ISO-8859-1 characters are allowed as it is sent in a HTTPS header. + Only printable ASCII characters are allowed as it is sent in a HTTPS header. "#]]) .with_status(101) From 39398fb18a53818b6d6a5396c7dc09bfa0b47011 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 31 Jan 2025 14:42:02 +0000 Subject: [PATCH 2/4] Check token validity when loading registry config --- src/cargo/util/auth/mod.rs | 12 ++++++++++++ tests/testsuite/publish.rs | 5 +---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/auth/mod.rs b/src/cargo/util/auth/mod.rs index 2dc66f56241..752eabab6a8 100644 --- a/src/cargo/util/auth/mod.rs +++ b/src/cargo/util/auth/mod.rs @@ -9,6 +9,7 @@ use cargo_credential::{ Action, CacheControl, Credential, CredentialResponse, LoginOptions, Operation, RegistryInfo, Secret, }; +use crates_io::check_token; use core::fmt; use serde::Deserialize; @@ -236,6 +237,17 @@ pub fn registry_credential_config_raw( return Ok(cfg.clone()); } let cfg = registry_credential_config_raw_uncached(gctx, sid)?; + if let Some(RegistryConfig { + token: Some(token), .. + }) = &cfg + { + check_token(&token.val.as_deref().expose()).with_context(|| { + format!( + "Token for {sid} is invalid (defined in {})", + token.definition + ) + })?; + } cache.insert(*sid, cfg.clone()); return Ok(cfg); } diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 6fd99767f27..de9c039d0c3 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -3609,10 +3609,7 @@ fn invalid_token() { .env("CARGO_REGISTRY_TOKEN", "\x16") .with_stderr_data(str![[r#" [UPDATING] crates.io index -[PACKAGING] foo v0.0.1 ([ROOT]/foo) -[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) -[UPLOADING] foo v0.0.1 ([ROOT]/foo) -[ERROR] failed to publish to registry at http://127.0.0.1:[..]/ +[ERROR] Token for registry `crates-io` is invalid (defined in environment variable `CARGO_REGISTRY_TOKEN`) Caused by: token contains invalid characters. From bf825dc8eba9524a173929924dd3e8192b2e80fa Mon Sep 17 00:00:00 2001 From: Kornel Date: Mon, 3 Feb 2025 16:15:36 +0000 Subject: [PATCH 3/4] Test HTTP header encoding and syntax --- crates/cargo-test-support/src/registry.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index c547921ceb9..7f9cb3d3514 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -794,7 +794,10 @@ impl HttpServer { line.clear(); break; } - let (name, value) = line.split_once(':').unwrap(); + if line.as_bytes().iter().any(|&b| b > 127 || b == 0) { + panic!("Invalid char in HTTP header: {line:?}"); + } + let (name, value) = line.split_once(':').expect("http header syntax"); let name = name.trim().to_ascii_lowercase(); let value = value.trim().to_string(); match name.as_str() { From e70f340c60381ec7880fbe5c39a3cb1f47f6cce1 Mon Sep 17 00:00:00 2001 From: Kornel Date: Mon, 3 Feb 2025 16:09:21 +0000 Subject: [PATCH 4/4] Prevent invalid tokens from breaking HTTP header syntax --- src/cargo/core/package.rs | 2 ++ src/cargo/sources/registry/http_remote.rs | 2 ++ tests/testsuite/credential_process.rs | 2 +- tests/testsuite/registry_auth.rs | 28 +++++++++++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 3d7c84a4266..87b396e2a11 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -1,3 +1,4 @@ +use crates_io::check_token; use std::cell::{Cell, Ref, RefCell, RefMut}; use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; @@ -743,6 +744,7 @@ impl<'a, 'gctx> Downloads<'a, 'gctx> { // Add authorization header. if let Some(authorization) = authorization { let mut headers = curl::easy::List::new(); + check_token(&authorization)?; headers.append(&format!("Authorization: {}", authorization))?; handle.http_headers(headers)?; } diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index 43742c07330..762dec57921 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -15,6 +15,7 @@ use crate::util::{auth, Filesystem, GlobalContext, IntoUrl, Progress, ProgressSt use anyhow::Context as _; use cargo_credential::Operation; use cargo_util::paths; +use crates_io::check_token; use curl::easy::{Easy, List}; use curl::multi::{EasyHandle, Multi}; use std::cell::RefCell; @@ -663,6 +664,7 @@ impl<'gctx> RegistryData for HttpRegistry<'gctx> { self.auth_error_headers.clone(), true, )?; + check_token(&authorization)?; headers.append(&format!("Authorization: {}", authorization))?; trace!(target: "network", "including authorization for {}", full_url); } diff --git a/tests/testsuite/credential_process.rs b/tests/testsuite/credential_process.rs index 5ebabfad62f..8517aeddf76 100644 --- a/tests/testsuite/credential_process.rs +++ b/tests/testsuite/credential_process.rs @@ -94,7 +94,7 @@ fn credential_provider_auth_failure() { .auth_required() .alternative() .no_configure_token() - .credential_provider(&["cargo:token-from-stdout", "true"]) + .credential_provider(&["cargo:token-from-stdout", "echo", "incorrect token"]) .build(); cargo_process("install libc --registry=alternative") diff --git a/tests/testsuite/registry_auth.rs b/tests/testsuite/registry_auth.rs index 360402965c4..29f1f13586a 100644 --- a/tests/testsuite/registry_auth.rs +++ b/tests/testsuite/registry_auth.rs @@ -390,6 +390,34 @@ Caused by: .run(); } +#[cargo_test] +fn syntactically_invalid_token() { + let _registry = RegistryBuilder::new() + .alternative() + .auth_required() + .no_configure_token() + .http_index() + .build(); + + let p = make_project(); + cargo(&p, "build") + .env("CARGO_REGISTRIES_ALTERNATIVE_TOKEN", "\t\n悪") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `alternative` index +[ERROR] failed to get `bar` as a dependency of package `foo v0.0.1 ([ROOT]/foo)` + +Caused by: + Token for registry `alternative` is invalid (defined in environment variable `CARGO_REGISTRIES_ALTERNATIVE_TOKEN`) + +Caused by: + token contains invalid characters. + Only printable ASCII characters are allowed as it is sent in a HTTPS header. + +"#]]) + .run(); +} + #[cargo_test] fn incorrect_token_git() { let _registry = RegistryBuilder::new()