refactor: use secrecy SerectString to hold secrets option (#3804)

* build: centralize secrecy dependency

Signed-off-by: tison <wander4096@gmail.com>

* add secrecy to sql crate

Signed-off-by: tison <wander4096@gmail.com>

* try impl

Signed-off-by: tison <wander4096@gmail.com>

* update test

Signed-off-by: tison <wander4096@gmail.com>

* make linters happy

Signed-off-by: tison <wander4096@gmail.com>

* bundle secrecy

Signed-off-by: tison <wander4096@gmail.com>

* bundle secrecy

Signed-off-by: tison <wander4096@gmail.com>

* replace secrecy

Signed-off-by: tison <wander4096@gmail.com>

* tidy clones

Signed-off-by: tison <wander4096@gmail.com>

* fixup

Signed-off-by: tison <wander4096@gmail.com>

* fixup

Signed-off-by: tison <wander4096@gmail.com>

* updated

Signed-off-by: tison <wander4096@gmail.com>

* Apply suggestions from code review

Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>

* use BTreeMap

Signed-off-by: tison <wander4096@gmail.com>

* tidy

Signed-off-by: tison <wander4096@gmail.com>

---------

Signed-off-by: tison <wander4096@gmail.com>
Co-authored-by: LFC <990479+MichaelScofield@users.noreply.github.com>

licenserc.toml

@@ -22,6 +22,7 @@ includes = [
 excludes = [
     # copied sources
     "src/common/base/src/readable_size.rs",
+    "src/common/base/src/secrets.rs",
     "src/servers/src/repeated_field.rs",
     "src/servers/src/http/test_helpers.rs",
 ]

src/auth/Cargo.toml

@@ -14,12 +14,12 @@ workspace = true
 [dependencies]
 api.workspace = true
 async-trait.workspace = true
+common-base.workspace = true
 common-error.workspace = true
 common-macro.workspace = true
 common-telemetry.workspace = true
 digest = "0.10"
 notify.workspace = true
-secrecy = { version = "0.8", features = ["serde", "alloc"] }
 sha1 = "0.10"
 snafu.workspace = true
 sql.workspace = true

src/auth/src/common.rs

@@ -14,8 +14,8 @@
 
 use std::sync::Arc;
 
+use common_base::secrets::SecretString;
 use digest::Digest;
-use secrecy::SecretString;
 use sha1::Sha1;
 use snafu::{ensure, OptionExt};
 

src/auth/src/tests.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use secrecy::ExposeSecret;
+use common_base::secrets::ExposeSecret;
 
 use crate::error::{
     AccessDeniedSnafu, Result, UnsupportedPasswordTypeSnafu, UserNotFoundSnafu,

src/auth/src/user_provider.rs

@@ -21,7 +21,7 @@ use std::io;
 use std::io::BufRead;
 use std::path::Path;
 
-use secrecy::ExposeSecret;
+use common_base::secrets::ExposeSecret;
 use snafu::{ensure, OptionExt, ResultExt};
 
 use crate::common::{Identity, Password};

src/cmd/src/standalone.rs

@@ -631,7 +631,7 @@ mod tests {
         match &dn_opts.storage.providers[1] {
             datanode::config::ObjectStoreConfig::S3(s3_config) => {
                 assert_eq!(
-                    "Secret([REDACTED alloc::string::String])".to_string(),
+                    "SecretBox<alloc::string::String>([REDACTED])".to_string(),
                     format!("{:?}", s3_config.access_key_id)
                 );
             }

src/common/base/Cargo.toml

@@ -16,6 +16,7 @@ common-macro.workspace = true
 paste = "1.0"
 serde = { version = "1.0", features = ["derive"] }
 snafu.workspace = true
+zeroize = { version = "1.6", default-features = false, features = ["alloc"] }
 
 [dev-dependencies]
 toml.workspace = true

src/common/base/src/lib.rs

@@ -17,6 +17,7 @@ pub mod buffer;
 pub mod bytes;
 #[allow(clippy::all)]
 pub mod readable_size;
+pub mod secrets;
 
 use core::any::Any;
 use std::sync::{Arc, Mutex, MutexGuard};

src/common/base/src/secrets.rs

@@ -0,0 +1,218 @@
+// This file is copied from: https://github.com/iqlusioninc/crates/blob/f98d4ccf/secrecy/src/lib.rs.
+
+//! [`SecretBox`] wrapper type for more carefully handling secret values
+//! (e.g. passwords, cryptographic keys, access tokens or other credentials)
+//!
+//! # Goals
+//!
+//! - Make secret access explicit and easy-to-audit via the
+//!   [`ExposeSecret`] and [`ExposeSecretMut`] traits.
+//! - Prevent accidental leakage of secrets via channels like debug logging
+//! - Ensure secrets are wiped from memory on drop securely
+//!   (using the [`zeroize`] crate)
+//!
+//! Presently this crate favors a simple, `no_std`-friendly, safe i.e.
+//! `forbid(unsafe_code)`-based implementation and does not provide more advanced
+//! memory protection mechanisms e.g. ones based on `mlock(2)`/`mprotect(2)`.
+//! We may explore more advanced protection mechanisms in the future.
+//! Those who don't mind `std` and `libc` dependencies should consider using
+//! the [`secrets`](https://crates.io/crates/secrets) crate.
+//!
+//! # `serde` support
+//!
+//! When the `serde` feature of this crate is enabled, the [`SecretBox`] type will
+//! receive a [`Deserialize`] impl for all `SecretBox<T>` types where
+//! `T: DeserializeOwned`. This allows *loading* secret values from data
+//! deserialized from `serde` (be careful to clean up any intermediate secrets
+//! when doing this, e.g. the unparsed input!)
+//!
+//! To prevent exfiltration of secret values via `serde`, by default `SecretBox<T>`
+//! does *not* receive a corresponding [`Serialize`] impl. If you would like
+//! types of `SecretBox<T>` to be serializable with `serde`, you will need to impl
+//! the [`SerializableSecret`] marker trait on `T`
+
+use std::fmt::Debug;
+use std::{any, fmt};
+
+use serde::{de, ser, Deserialize, Serialize};
+use zeroize::{Zeroize, ZeroizeOnDrop};
+
+/// Wrapper type for strings that contains secrets. See also [SecretBox].
+pub type SecretString = SecretBox<String>;
+
+impl From<String> for SecretString {
+    fn from(value: String) -> Self {
+        SecretString::new(Box::new(value))
+    }
+}
+
+/// Wrapper type for values that contains secrets, which attempts to limit
+/// accidental exposure and ensure secrets are wiped from memory when dropped.
+/// (e.g. passwords, cryptographic keys, access tokens or other credentials)
+///
+/// Access to the secret inner value occurs through the [`ExposeSecret`]
+/// or [`ExposeSecretMut`] traits, which provide methods for accessing the inner secret value.
+pub struct SecretBox<S: Zeroize> {
+    inner_secret: Box<S>,
+}
+
+impl<S: Zeroize> Zeroize for SecretBox<S> {
+    fn zeroize(&mut self) {
+        self.inner_secret.as_mut().zeroize()
+    }
+}
+
+impl<S: Zeroize> Drop for SecretBox<S> {
+    fn drop(&mut self) {
+        self.zeroize()
+    }
+}
+
+impl<S: Zeroize> ZeroizeOnDrop for SecretBox<S> {}
+
+impl<S: Zeroize> From<Box<S>> for SecretBox<S> {
+    fn from(source: Box<S>) -> Self {
+        Self::new(source)
+    }
+}
+
+impl<S: Zeroize> SecretBox<S> {
+    /// Create a secret value using a pre-boxed value.
+    pub fn new(boxed_secret: Box<S>) -> Self {
+        Self {
+            inner_secret: boxed_secret,
+        }
+    }
+}
+
+impl<S: Zeroize + Default> SecretBox<S> {
+    /// Create a secret value using a function that can initialize the vale in-place.
+    pub fn new_with_mut(ctr: impl FnOnce(&mut S)) -> Self {
+        let mut secret = Self::default();
+        ctr(secret.expose_secret_mut());
+        secret
+    }
+}
+
+impl<S: Zeroize + Clone> SecretBox<S> {
+    /// Create a secret value using the provided function as a constructor.
+    ///
+    /// The implementation makes an effort to zeroize the locally constructed value
+    /// before it is copied to the heap, and constructing it inside the closure minimizes
+    /// the possibility of it being accidentally copied by other code.
+    ///
+    /// **Note:** using [`Self::new`] or [`Self::new_with_mut`] is preferable when possible,
+    /// since this method's safety relies on empyric evidence and may be violated on some targets.
+    pub fn new_with_ctr(ctr: impl FnOnce() -> S) -> Self {
+        let mut data = ctr();
+        let secret = Self {
+            inner_secret: Box::new(data.clone()),
+        };
+        data.zeroize();
+        secret
+    }
+
+    /// Same as [`Self::new_with_ctr`], but the constructor can be fallible.
+    ///
+    ///
+    /// **Note:** using [`Self::new`] or [`Self::new_with_mut`] is preferable when possible,
+    /// since this method's safety relies on empyric evidence and may be violated on some targets.
+    pub fn try_new_with_ctr<E>(ctr: impl FnOnce() -> Result<S, E>) -> Result<Self, E> {
+        let mut data = ctr()?;
+        let secret = Self {
+            inner_secret: Box::new(data.clone()),
+        };
+        data.zeroize();
+        Ok(secret)
+    }
+}
+
+impl<S: Zeroize + Default> Default for SecretBox<S> {
+    fn default() -> Self {
+        Self {
+            inner_secret: Box::<S>::default(),
+        }
+    }
+}
+
+impl<S: Zeroize> Debug for SecretBox<S> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "SecretBox<{}>([REDACTED])", any::type_name::<S>())
+    }
+}
+
+impl<S> Clone for SecretBox<S>
+where
+    S: Clone + Zeroize,
+{
+    fn clone(&self) -> Self {
+        SecretBox {
+            inner_secret: self.inner_secret.clone(),
+        }
+    }
+}
+
+impl<S: Zeroize> ExposeSecret<S> for SecretBox<S> {
+    fn expose_secret(&self) -> &S {
+        self.inner_secret.as_ref()
+    }
+}
+
+impl<S: Zeroize> ExposeSecretMut<S> for SecretBox<S> {
+    fn expose_secret_mut(&mut self) -> &mut S {
+        self.inner_secret.as_mut()
+    }
+}
+
+/// Expose a reference to an inner secret
+pub trait ExposeSecret<S> {
+    /// Expose secret: this is the only method providing access to a secret.
+    fn expose_secret(&self) -> &S;
+}
+
+/// Expose a mutable reference to an inner secret
+pub trait ExposeSecretMut<S> {
+    /// Expose secret: this is the only method providing access to a secret.
+    fn expose_secret_mut(&mut self) -> &mut S;
+}
+
+/// Marker trait for secret types which can be [`Serialize`]-d by [`serde`].
+///
+/// When the `serde` feature of this crate is enabled and types are marked with
+/// this trait, they receive a [`Serialize` impl][1] for `SecretBox<T>`.
+/// (NOTE: all types which impl `DeserializeOwned` receive a [`Deserialize`]
+/// impl)
+///
+/// This is done deliberately to prevent accidental exfiltration of secrets
+/// via `serde` serialization.
+///
+/// If you really want to have `serde` serialize those types, use the
+/// [`serialize_with`][2] attribute to specify a serializer that exposes the secret.
+///
+/// [1]: https://docs.rs/secrecy/latest/secrecy/struct.Secret.html#implementations
+/// [2]: https://serde.rs/field-attrs.html#serialize_with
+pub trait SerializableSecret: Serialize {}
+
+impl<'de, T> Deserialize<'de> for SecretBox<T>
+where
+    T: Zeroize + Clone + de::DeserializeOwned + Sized,
+{
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        Self::try_new_with_ctr(|| T::deserialize(deserializer))
+    }
+}
+
+impl<T> Serialize for SecretBox<T>
+where
+    T: Zeroize + SerializableSecret + Serialize + Sized,
+{
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: ser::Serializer,
+    {
+        self.expose_secret().serialize(serializer)
+    }
+}

src/datanode/Cargo.toml

@@ -50,7 +50,6 @@ prometheus.workspace = true
 prost.workspace = true
 query.workspace = true
 reqwest.workspace = true
-secrecy = { version = "0.8", features = ["serde", "alloc"] }
 serde.workspace = true
 servers.workspace = true
 session.workspace = true

src/datanode/src/config.rs

@@ -15,6 +15,7 @@
 //! Datanode configurations
 
 use common_base::readable_size::ReadableSize;
+use common_base::secrets::SecretString;
 use common_grpc::channel_manager::{
     DEFAULT_MAX_GRPC_RECV_MESSAGE_SIZE, DEFAULT_MAX_GRPC_SEND_MESSAGE_SIZE,
 };
@@ -24,7 +25,6 @@ use common_wal::config::DatanodeWalConfig;
 use file_engine::config::EngineConfig as FileEngineConfig;
 use meta_client::MetaClientOptions;
 use mito2::config::MitoConfig;
-use secrecy::SecretString;
 use serde::{Deserialize, Serialize};
 use servers::export_metrics::ExportMetricsOption;
 use servers::heartbeat_options::HeartbeatOptions;
@@ -285,7 +285,7 @@ pub enum RegionEngineConfig {
 
 #[cfg(test)]
 mod tests {
-    use secrecy::ExposeSecret;
+    use common_base::secrets::ExposeSecret;
 
     use super::*;
 
@@ -308,7 +308,7 @@ mod tests {
         match &opts.storage.store {
             ObjectStoreConfig::S3(cfg) => {
                 assert_eq!(
-                    "Secret([REDACTED alloc::string::String])".to_string(),
+                    "SecretBox<alloc::string::String>([REDACTED])".to_string(),
                     format!("{:?}", cfg.access_key_id)
                 );
                 assert_eq!("access_key_id", cfg.access_key_id.expose_secret());