From 0203af0b09099ee2d22d7f4936d2f05afe4664e0 Mon Sep 17 00:00:00 2001 From: ChinYing-Li Date: Sat, 18 Dec 2021 15:28:38 -0500 Subject: [PATCH] Use SecretString for AuthInfo.password to avoid credential leaking --- kube-client/Cargo.toml | 1 + kube-client/src/client/auth/mod.rs | 3 +- kube-client/src/client/config_ext.rs | 3 +- kube-client/src/config/file_config.rs | 62 +++++++++++++++++++++++++-- 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/kube-client/Cargo.toml b/kube-client/Cargo.toml index 2f5105038..711d05eb0 100644 --- a/kube-client/Cargo.toml +++ b/kube-client/Cargo.toml @@ -69,6 +69,7 @@ hyper-timeout = {version = "0.4.1", optional = true } tame-oauth = { version = "0.6.0", features = ["gcp"], optional = true } pin-project = { version = "1.0.4", optional = true } rand = { version = "0.8.3", optional = true } +secrecy = { version = "0.8.0", features = ['alloc', 'serde'] } tracing = { version = "0.1.29", features = ["log"], optional = true } hyper-openssl = { version = "0.9.1", optional = true } diff --git a/kube-client/src/client/auth/mod.rs b/kube-client/src/client/auth/mod.rs index 981df201a..0633937be 100644 --- a/kube-client/src/client/auth/mod.rs +++ b/kube-client/src/client/auth/mod.rs @@ -7,6 +7,7 @@ use http::{ HeaderValue, Request, }; use jsonpath_lib::select as jsonpath_select; +use secrecy::SecretString; use serde::{Deserialize, Serialize}; use thiserror::Error; use tokio::sync::Mutex; @@ -82,7 +83,7 @@ pub enum Error { #[allow(clippy::large_enum_variant)] pub(crate) enum Auth { None, - Basic(String, String), + Basic(String, SecretString), Bearer(String), RefreshableToken(RefreshableToken), } diff --git a/kube-client/src/client/config_ext.rs b/kube-client/src/client/config_ext.rs index f8b618e01..12a0ce201 100644 --- a/kube-client/src/client/config_ext.rs +++ b/kube-client/src/client/config_ext.rs @@ -1,3 +1,4 @@ +use secrecy::ExposeSecret; use tower::{filter::AsyncFilterLayer, util::Either}; #[cfg(any(feature = "native-tls", feature = "rustls-tls", feature = "openssl-tls"))] @@ -170,7 +171,7 @@ impl ConfigExt for Config { Ok(match Auth::try_from(&self.auth_info).map_err(Error::Auth)? { Auth::None => None, Auth::Basic(user, pass) => Some(AuthLayer(Either::A( - AddAuthorizationLayer::basic(&user, &pass).as_sensitive(true), + AddAuthorizationLayer::basic(&user, pass.expose_secret()).as_sensitive(true), ))), Auth::Bearer(token) => Some(AuthLayer(Either::A( AddAuthorizationLayer::bearer(&token).as_sensitive(true), diff --git a/kube-client/src/config/file_config.rs b/kube-client/src/config/file_config.rs index 6db8e4adb..507056960 100644 --- a/kube-client/src/config/file_config.rs +++ b/kube-client/src/config/file_config.rs @@ -4,7 +4,8 @@ use std::{ path::{Path, PathBuf}, }; -use serde::{Deserialize, Serialize}; +use secrecy::{ExposeSecret, SecretString}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use super::{KubeconfigError, LoadDataError}; @@ -118,16 +119,40 @@ pub struct NamedAuthInfo { pub auth_info: AuthInfo, } +fn serialize_password(pw: &Option, serializer: S) -> Result +where + S: Serializer, +{ + match pw { + Some(pw_string) => serializer.serialize_str(pw_string.expose_secret()), + None => serializer.serialize_none(), + } +} + +fn deserialize_password<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let buffer = String::deserialize(deserializer); + match buffer { + Ok(pw_string) => Ok(Some(SecretString::new(pw_string))), + Err(e) => Err(e), + } +} + /// AuthInfo stores information to tell cluster who you are. #[derive(Clone, Debug, Serialize, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq))] pub struct AuthInfo { /// The username for basic authentication to the kubernetes cluster. #[serde(skip_serializing_if = "Option::is_none")] pub username: Option, /// The password for basic authentication to the kubernetes cluster. - #[serde(skip_serializing_if = "Option::is_none")] - pub password: Option, + #[serde(skip_serializing_if = "Option::is_none", default)] + #[serde( + serialize_with = "serialize_password", + deserialize_with = "deserialize_password" + )] + pub password: Option, /// The bearer token for authentication to the kubernetes cluster. #[serde(skip_serializing_if = "Option::is_none")] @@ -174,6 +199,13 @@ pub struct AuthInfo { pub exec: Option, } +#[cfg(test)] +impl PartialEq for AuthInfo { + fn eq(&self, other: &Self) -> bool { + serde_json::to_value(&self).unwrap() == serde_json::to_value(&other).unwrap() + } +} + /// AuthProviderConfig stores auth for specified cloud provider. #[derive(Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] @@ -627,4 +659,26 @@ users: assert_eq!(cfg, Kubeconfig::default()); } + + #[test] + fn authinfo_debug_does_not_output_password() { + let authinfo_yaml = r#" +username: user +password: kube_rs +"#; + let authinfo: AuthInfo = serde_yaml::from_str(&authinfo_yaml).unwrap(); + let authinfo_debug_output = format!("{:?}", authinfo); + let expected_output = "AuthInfo { \ + username: Some(\"user\"), \ + password: Some(Secret([REDACTED alloc::string::String])), \ + token: None, token_file: None, client_certificate: None, \ + client_certificate_data: None, client_key: None, \ + client_key_data: None, impersonate: None, \ + impersonate_groups: None, \ + auth_provider: None, \ + exec: None \ + }"; + + assert_eq!(authinfo_debug_output, expected_output) + } }