diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index d2bd9c166df..dac0457d9e0 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -36,7 +36,7 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { ops::registry_login( config, - args.get_one("token").map(String::as_str), + args.get_one::("token").map(|s| s.as_str().into()), args.get_one("registry").map(String::as_str), args.flag("generate-keypair"), args.flag("secret-key"), diff --git a/src/bin/cargo/commands/owner.rs b/src/bin/cargo/commands/owner.rs index 170dd69e71e..493072b7b3a 100644 --- a/src/bin/cargo/commands/owner.rs +++ b/src/bin/cargo/commands/owner.rs @@ -1,6 +1,7 @@ use crate::command_prelude::*; use cargo::ops::{self, OwnersOptions}; +use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("owner") @@ -34,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let registry = args.registry(config)?; let opts = OwnersOptions { krate: args.get_one::("crate").cloned(), - token: args.get_one::("token").cloned(), + token: args.get_one::("token").cloned().map(Secret::from), index: args.get_one::("index").cloned(), to_add: args .get_many::("add") diff --git a/src/bin/cargo/commands/publish.rs b/src/bin/cargo/commands/publish.rs index 7dd6414bada..c831d399f59 100644 --- a/src/bin/cargo/commands/publish.rs +++ b/src/bin/cargo/commands/publish.rs @@ -36,7 +36,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { &ws, &PublishOpts { config, - token: args.get_one::("token").map(|s| s.to_string()), + token: args + .get_one::("token") + .map(|s| s.to_string().into()), index, verify: !args.flag("no-verify"), allow_dirty: args.flag("allow-dirty"), diff --git a/src/bin/cargo/commands/yank.rs b/src/bin/cargo/commands/yank.rs index dda766dd13c..3dee522793b 100644 --- a/src/bin/cargo/commands/yank.rs +++ b/src/bin/cargo/commands/yank.rs @@ -1,6 +1,7 @@ use crate::command_prelude::*; use cargo::ops; +use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("yank") @@ -37,7 +38,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { config, krate.map(|s| s.to_string()), version.map(|s| s.to_string()), - args.get_one::("token").cloned(), + args.get_one::("token").cloned().map(Secret::from), args.get_one::("index").cloned(), args.flag("undo"), registry, diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index c1684d0ef31..091c26b6070 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -30,7 +30,7 @@ use crate::ops; use crate::ops::Packages; use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_IO_REGISTRY}; use crate::util::auth::{ - paserk_public_from_paserk_secret, {self, AuthorizationError}, + paserk_public_from_paserk_secret, Secret, {self, AuthorizationError}, }; use crate::util::config::{Config, SslVersionConfig, SslVersionConfigRange}; use crate::util::errors::CargoResult; @@ -45,11 +45,11 @@ use crate::{drop_print, drop_println, version}; pub enum RegistryCredentialConfig { None, /// The authentication token. - Token(String), + Token(Secret), /// Process used for fetching a token. Process((PathBuf, Vec)), /// Secret Key and subject for Asymmetric tokens. - AsymmetricKey((String, Option)), + AsymmetricKey((Secret, Option)), } impl RegistryCredentialConfig { @@ -71,9 +71,9 @@ impl RegistryCredentialConfig { pub fn is_asymmetric_key(&self) -> bool { matches!(self, Self::AsymmetricKey(..)) } - pub fn as_token(&self) -> Option<&str> { + pub fn as_token(&self) -> Option> { if let Self::Token(v) = self { - Some(&*v) + Some(v.as_deref()) } else { None } @@ -85,7 +85,7 @@ impl RegistryCredentialConfig { None } } - pub fn as_asymmetric_key(&self) -> Option<&(String, Option)> { + pub fn as_asymmetric_key(&self) -> Option<&(Secret, Option)> { if let Self::AsymmetricKey(v) = self { Some(v) } else { @@ -96,7 +96,7 @@ impl RegistryCredentialConfig { pub struct PublishOpts<'cfg> { pub config: &'cfg Config, - pub token: Option, + pub token: Option>, pub index: Option, pub verify: bool, pub allow_dirty: bool, @@ -174,7 +174,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let (mut registry, reg_ids) = registry( opts.config, - opts.token.as_deref(), + opts.token.as_ref().map(Secret::as_deref), opts.index.as_deref(), publish_registry.as_deref(), true, @@ -512,7 +512,7 @@ fn wait_for_publish( /// * `token_required`: If `true`, the token will be set. fn registry( config: &Config, - token_from_cmdline: Option<&str>, + token_from_cmdline: Option>, index: Option<&str>, registry: Option<&str>, force_update: bool, @@ -786,7 +786,7 @@ fn http_proxy_exists(config: &Config) -> CargoResult { pub fn registry_login( config: &Config, - token: Option<&str>, + token: Option>, reg: Option<&str>, generate_keypair: bool, secret_key_required: bool, @@ -795,7 +795,7 @@ pub fn registry_login( let source_ids = get_source_id(config, None, reg)?; let reg_cfg = auth::registry_credential_config(config, &source_ids.original)?; - let login_url = match registry(config, token, None, reg, false, None) { + let login_url = match registry(config, token.clone(), None, reg, false, None) { Ok((registry, _)) => Some(format!("{}/me", registry.host())), Err(e) if e.is::() => e .downcast::() @@ -830,29 +830,33 @@ pub fn registry_login( } _ => (None, None), }; - let secret_key: String; + let secret_key: Secret; if generate_keypair { assert!(!secret_key_required); let kp = AsymmetricKeyPair::::generate().unwrap(); - let mut key = String::new(); - FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap(); - secret_key = key; + secret_key = Secret::default().map(|mut key| { + FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap(); + key + }); } else if secret_key_required { assert!(!generate_keypair); drop_println!(config, "please paste the API secret key below"); - let mut line = String::new(); - let input = io::stdin(); - input - .lock() - .read_line(&mut line) - .with_context(|| "failed to read stdin")?; - secret_key = line.trim().to_string(); + secret_key = Secret::default() + .map(|mut line| { + let input = io::stdin(); + input + .lock() + .read_line(&mut line) + .with_context(|| "failed to read stdin") + .map(|_| line.trim().to_string()) + }) + .transpose()?; } else { secret_key = old_secret_key .cloned() .ok_or_else(|| anyhow!("need a secret_key to set a key_subject"))?; } - if let Some(p) = paserk_public_from_paserk_secret(&secret_key) { + if let Some(p) = paserk_public_from_paserk_secret(secret_key.as_deref()) { drop_println!(config, "{}", &p); } else { bail!("not a validly formatted PASERK secret key"); @@ -866,7 +870,7 @@ pub fn registry_login( )); } else { new_token = RegistryCredentialConfig::Token(match token { - Some(token) => token.to_string(), + Some(token) => token.owned(), None => { if let Some(login_url) = login_url { drop_println!( @@ -890,7 +894,7 @@ pub fn registry_login( .with_context(|| "failed to read stdin")?; // Automatically remove `cargo login` from an inputted token to // allow direct pastes from `registry.host()`/me. - line.replace("cargo login", "").trim().to_string() + Secret::from(line.replace("cargo login", "").trim().to_string()) } }); @@ -938,7 +942,7 @@ pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> { pub struct OwnersOptions { pub krate: Option, - pub token: Option, + pub token: Option>, pub index: Option, pub to_add: Option>, pub to_remove: Option>, @@ -960,7 +964,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { let (mut registry, _) = registry( config, - opts.token.as_deref(), + opts.token.as_ref().map(Secret::as_deref), opts.index.as_deref(), opts.registry.as_deref(), true, @@ -1019,7 +1023,7 @@ pub fn yank( config: &Config, krate: Option, version: Option, - token: Option, + token: Option>, index: Option, undo: bool, reg: Option, @@ -1051,7 +1055,7 @@ pub fn yank( let (mut registry, _) = registry( config, - token.as_deref(), + token.as_ref().map(Secret::as_deref), index.as_deref(), reg.as_deref(), true, diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index e0e8f1dd6b4..467abc2635a 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -10,6 +10,7 @@ use serde::Deserialize; use std::collections::HashMap; use std::error::Error; use std::io::{Read, Write}; +use std::ops::Deref; use std::path::PathBuf; use std::process::{Command, Stdio}; use time::format_description::well_known::Rfc3339; @@ -21,6 +22,102 @@ use crate::ops::RegistryCredentialConfig; use super::config::CredentialCacheValue; +/// A wrapper for values that should not be printed. +/// +/// This type does not implement `Display`, and has a `Debug` impl that hides +/// the contained value. +/// +/// ``` +/// # use cargo::util::auth::Secret; +/// let token = Secret::from("super secret string"); +/// assert_eq!(format!("{:?}", token), "Secret { inner: \"REDACTED\" }"); +/// ``` +/// +/// Currently, we write a borrowed `Secret` as `Secret<&T>`. +/// The [`as_deref`](Secret::as_deref) and [`owned`](Secret::owned) methods can +/// be used to convert back and forth between `Secret` and `Secret<&str>`. +#[derive(Default, Clone, PartialEq, Eq)] +pub struct Secret { + inner: T, +} + +impl Secret { + /// Unwraps the contained value. + /// + /// Use of this method marks the boundary of where the contained value is + /// hidden. + pub fn expose(self) -> T { + self.inner + } + + /// Converts a `Secret` to a `Secret<&T::Target>`. + /// ``` + /// # use cargo::util::auth::Secret; + /// let owned: Secret = Secret::from(String::from("token")); + /// let borrowed: Secret<&str> = owned.as_deref(); + /// ``` + pub fn as_deref(&self) -> Secret<&::Target> + where + T: Deref, + { + Secret::from(self.inner.deref()) + } + + /// Converts a `Secret` to a `Secret<&T>`. + pub fn as_ref(&self) -> Secret<&T> { + Secret::from(&self.inner) + } + + /// Converts a `Secret` to a `Secret` by applying `f` to the contained value. + pub fn map(self, f: F) -> Secret + where + F: FnOnce(T) -> U, + { + Secret::from(f(self.inner)) + } +} + +impl Secret<&T> { + /// Converts a `Secret` containing a borrowed type to a `Secret` containing the + /// corresponding owned type. + /// ``` + /// # use cargo::util::auth::Secret; + /// let borrowed: Secret<&str> = Secret::from("token"); + /// let owned: Secret = borrowed.owned(); + /// ``` + pub fn owned(&self) -> Secret<::Owned> { + Secret::from(self.inner.to_owned()) + } +} + +impl Secret> { + /// Converts a `Secret>` to a `Result, E>`. + pub fn transpose(self) -> Result, E> { + self.inner.map(|v| Secret::from(v)) + } +} + +impl> Secret { + /// Checks if the contained value is empty. + pub fn is_empty(&self) -> bool { + self.inner.as_ref().is_empty() + } +} + +impl From for Secret { + fn from(inner: T) -> Self { + Self { inner } + } +} + +impl fmt::Debug for Secret { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Secret") + .field("inner", &"REDACTED") + .finish() + } +} + /// Get the credential configuration for a `SourceId`. pub fn registry_credential_config( config: &Config, @@ -61,9 +158,9 @@ pub fn registry_credential_config( return registry_credential_config_inner( true, None, - token, + token.map(Secret::from), credential_process, - secret_key, + secret_key.map(Secret::from), secret_key_subject, config, ); @@ -152,9 +249,9 @@ pub fn registry_credential_config( registry_credential_config_inner( false, name.as_deref(), - token, + token.map(Secret::from), credential_process, - secret_key, + secret_key.map(Secret::from), secret_key_subject, config, ) @@ -163,9 +260,9 @@ pub fn registry_credential_config( fn registry_credential_config_inner( is_crates_io: bool, name: Option<&str>, - token: Option, + token: Option>, credential_process: Option, - secret_key: Option, + secret_key: Option>, secret_key_subject: Option, config: &Config, ) -> CargoResult { @@ -298,14 +395,14 @@ my-registry = {{ index = "{}" }} } // Store a token in the cache for future calls. -pub fn cache_token(config: &Config, sid: &SourceId, token: &str) { +pub fn cache_token(config: &Config, sid: &SourceId, token: Secret<&str>) { let url = sid.canonical_url(); config.credential_cache().insert( url.clone(), CredentialCacheValue { from_commandline: true, independent_of_endpoint: true, - token_value: token.to_string(), + token_value: token.owned(), }, ); } @@ -320,7 +417,7 @@ pub fn auth_token( mutation: Option>, ) -> CargoResult { match auth_token_optional(config, sid, mutation.as_ref())? { - Some(token) => Ok(token), + Some(token) => Ok(token.expose()), None => Err(AuthorizationError { sid: sid.clone(), login_url: login_url.cloned(), @@ -335,7 +432,7 @@ fn auth_token_optional( config: &Config, sid: &SourceId, mutation: Option<&'_ Mutation<'_>>, -) -> CargoResult> { +) -> CargoResult>> { let mut cache = config.credential_cache(); let url = sid.canonical_url(); @@ -353,15 +450,21 @@ fn auth_token_optional( let credential = registry_credential_config(config, sid)?; let (independent_of_endpoint, token) = match credential { RegistryCredentialConfig::None => return Ok(None), - RegistryCredentialConfig::Token(config_token) => (true, config_token.to_string()), + RegistryCredentialConfig::Token(config_token) => (true, config_token), RegistryCredentialConfig::Process(process) => { // todo: PASETO with process - run_command(config, &process, sid, Action::Get)?.unwrap() + let (independent_of_endpoint, token) = + run_command(config, &process, sid, Action::Get)?.unwrap(); + (independent_of_endpoint, Secret::from(token)) } RegistryCredentialConfig::AsymmetricKey((secret_key, secret_key_subject)) => { - let secret: AsymmetricSecretKey = - secret_key.as_str().try_into()?; - let public: AsymmetricPublicKey = (&secret).try_into()?; + let secret: Secret> = + secret_key.map(|key| key.as_str().try_into()).transpose()?; + let public: AsymmetricPublicKey = secret + .as_ref() + .map(|key| key.try_into()) + .transpose()? + .expose(); let kip: pasetors::paserk::Id = (&public).try_into()?; let iat = OffsetDateTime::now_utc(); @@ -413,18 +516,22 @@ fn auth_token_optional( ( false, - pasetors::version3::PublicToken::sign( - &secret, - serde_json::to_string(&message) - .expect("cannot serialize") - .as_bytes(), - Some( - serde_json::to_string(&footer) - .expect("cannot serialize") - .as_bytes(), - ), - None, - )?, + secret + .map(|secret| { + pasetors::version3::PublicToken::sign( + &secret, + serde_json::to_string(&message) + .expect("cannot serialize") + .as_bytes(), + Some( + serde_json::to_string(&footer) + .expect("cannot serialize") + .as_bytes(), + ), + None, + ) + }) + .transpose()?, ) } }; @@ -435,7 +542,7 @@ fn auth_token_optional( CredentialCacheValue { from_commandline: false, independent_of_endpoint, - token_value: token.to_string(), + token_value: token.clone(), }, ); } @@ -519,6 +626,7 @@ pub fn login(config: &Config, sid: &SourceId, token: RegistryCredentialConfig) - let token = token .as_token() .expect("credential_process cannot use login with a secret_key") + .expose() .to_owned(); run_command(config, &process, sid, Action::Store(token))?; } @@ -530,9 +638,15 @@ pub fn login(config: &Config, sid: &SourceId, token: RegistryCredentialConfig) - } /// Checks that a secret key is valid, and returns the associated public key in Paserk format. -pub(crate) fn paserk_public_from_paserk_secret(secret_key: &str) -> Option { - let secret: AsymmetricSecretKey = secret_key.try_into().ok()?; - let public: AsymmetricPublicKey = (&secret).try_into().ok()?; +pub(crate) fn paserk_public_from_paserk_secret(secret_key: Secret<&str>) -> Option { + let secret: Secret> = + secret_key.map(|key| key.try_into()).transpose().ok()?; + let public: AsymmetricPublicKey = secret + .as_ref() + .map(|key| key.try_into()) + .transpose() + .ok()? + .expose(); let mut paserk_pub_key = String::new(); FormatAsPaserk::fmt(&public, &mut paserk_pub_key).unwrap(); Some(paserk_pub_key) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 3b6b7f521d6..731ea6cd6a0 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -70,6 +70,7 @@ use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRootConfig}; use crate::ops::{self, RegistryCredentialConfig}; +use crate::util::auth::Secret; use crate::util::errors::CargoResult; use crate::util::validate_package_name; use crate::util::CanonicalUrl; @@ -137,6 +138,7 @@ enum WhyLoad { } /// A previously generated authentication token and the data needed to determine if it can be reused. +#[derive(Debug)] pub struct CredentialCacheValue { /// If the command line was used to override the token then it must always be reused, /// even if reading the configuration files would lead to a different value. @@ -144,17 +146,7 @@ pub struct CredentialCacheValue { /// If nothing depends on which endpoint is being hit, then we can reuse the token /// for any future request even if some of the requests involve mutations. pub independent_of_endpoint: bool, - pub token_value: String, -} - -impl fmt::Debug for CredentialCacheValue { - /// This manual implementation helps ensure that the token value is redacted from all logs. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialCacheValue") - .field("from_commandline", &self.from_commandline) - .field("token_value", &"REDACTED") - .finish() - } + pub token_value: Secret, } /// Configuration information for cargo. This is not specific to a build, it is information @@ -2191,7 +2183,7 @@ pub fn save_credentials( // login with token let key = "token".to_string(); - let value = ConfigValue::String(token, path_def.clone()); + let value = ConfigValue::String(token.expose(), path_def.clone()); let map = HashMap::from([(key, value)]); let table = CV::Table(map, path_def.clone()); @@ -2206,7 +2198,7 @@ pub fn save_credentials( // login with key let key = "secret-key".to_string(); - let value = ConfigValue::String(secret_key, path_def.clone()); + let value = ConfigValue::String(secret_key.expose(), path_def.clone()); let mut map = HashMap::from([(key, value)]); if let Some(key_subject) = key_subject { let key = "secret-key-subject".to_string();