feat(575): using a mix of secrecy + serde::skip_serializing to avoid …

…leaks of secrets in logs Signed-off-by: gabrik <[email protected]>
eclipse-zenoh · Nov 2, 2023 · 9a26013 · 9a26013
1 parent 2ca6bf1
commit 9a26013
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 50 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -128,6 +128,7 @@ rustls = { version = "0.21.5", features = ["dangerous_configuration"] }
 rustls-native-certs = "0.6.2"
 rustls-pemfile = "1.0.2"
 schemars = "0.8.12"
+secrecy = {version = "0.8.0", features = ["serde", "alloc"]}
 serde = { version = "1.0.154", default-features = false, features = [
   "derive",
 ] } # Default features are disabled due to usage in no_std crates

diff --git a/commons/zenoh-config/Cargo.toml b/commons/zenoh-config/Cargo.toml
@@ -35,3 +35,4 @@ zenoh-core = { workspace = true }
 zenoh-protocol = { workspace = true }
 zenoh-result = { workspace = true }
 zenoh-util = { workspace = true }
+secrecy = { workspace = true }
diff --git a/commons/zenoh-config/src/lib.rs b/commons/zenoh-config/src/lib.rs
@@ -16,6 +16,7 @@
 pub mod defaults;
 mod include;
 use include::recursive_include;
+use secrecy::{CloneableSecret, DebugSecret, Secret, SerializableSecret, Zeroize};
 use serde::{
     de::{self, MapAccess, Visitor},
     Deserialize, Serialize,
@@ -46,6 +47,29 @@ use zenoh_protocol::{
 use zenoh_result::{bail, zerror, ZResult};
 use zenoh_util::LibLoader;
 
+// Wrappers for secrecy of values
+#[derive(serde::Serialize, serde::Deserialize, Debug, Clone)]
+pub struct SecretString(String);
+
+impl Deref for SecretString {
+    type Target = String;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl SerializableSecret for SecretString {}
+impl DebugSecret for SecretString {}
+impl CloneableSecret for SecretString {}
+impl Zeroize for SecretString {
+    fn zeroize(&mut self) {
+        self.0 = "".to_string();
+    }
+}
+
+pub type SecretValue = Secret<SecretString>;
+
 pub type ValidationFunction = std::sync::Arc<
     dyn Fn(
             &str,
@@ -294,14 +318,17 @@ validated_struct::validator! {
                     client_private_key: Option<String>,
                     client_certificate: Option<String>,
                     server_name_verification: Option<bool>,
-                    pub private : #[derive(Default)]
-                    Base64Data {
-                        root_ca_certificate_base64: Option<String>,
-                        server_private_key_base64: Option<String>,
-                        server_certificate_base64: Option<String>,
-                        client_private_key_base64 : Option<String>,
-                        client_certificate_base64 : Option<String>,
-                    }
+                    // Skip serializing field because they contain secrets
+                    #[serde(skip_serializing)]
+                    root_ca_certificate_base64: Option<SecretValue>,
+                    #[serde(skip_serializing)]
+                    server_private_key_base64:  Option<SecretValue>,
+                    #[serde(skip_serializing)]
+                    server_certificate_base64: Option<SecretValue>,
+                    #[serde(skip_serializing)]
+                    client_private_key_base64 :  Option<SecretValue>,
+                    #[serde(skip_serializing)]
+                    client_certificate_base64 :  Option<SecretValue>,
                 }
                 ,
                 pub unixpipe: #[derive(Default)]

diff --git a/io/zenoh-links/zenoh-link-quic/Cargo.toml b/io/zenoh-links/zenoh-link-quic/Cargo.toml
@@ -42,4 +42,5 @@ zenoh-protocol = { workspace = true }
 zenoh-result = { workspace = true }
 zenoh-sync = { workspace = true }
 zenoh-util = { workspace = true }
-base64 = { workspace = true }
+base64 = { workspace = true }
+secrecy = {workspace = true }
diff --git a/io/zenoh-links/zenoh-link-quic/src/lib.rs b/io/zenoh-links/zenoh-link-quic/src/lib.rs
@@ -24,6 +24,7 @@ use config::{
     TLS_SERVER_CERTIFICATE_FILE, TLS_SERVER_NAME_VERIFICATION, TLS_SERVER_PRIVATE_KEY_BASE64,
     TLS_SERVER_PRIVATE_KEY_FILE,
 };
+use secrecy::ExposeSecret;
 use std::net::SocketAddr;
 use zenoh_config::Config;
 use zenoh_core::zconfigurable;
@@ -74,50 +75,50 @@ impl ConfigurationInspector<Config> for QuicConfigurator {
 
         let c = config.transport().link().tls();
 
-        match (
-            c.root_ca_certificate(),
-            c.private().root_ca_certificate_base64(),
-        ) {
+        match (c.root_ca_certificate(), c.root_ca_certificate_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'root_ca_certificate' and 'root_ca_certificate_base64' can be present!")
             }
             (Some(ca_certificate), None) => {
                 ps.push((TLS_ROOT_CA_CERTIFICATE_FILE, ca_certificate));
             }
             (None, Some(ca_certificate)) => {
-                ps.push((TLS_ROOT_CA_CERTIFICATE_BASE64, ca_certificate));
+                ps.push((
+                    TLS_ROOT_CA_CERTIFICATE_BASE64,
+                    ca_certificate.expose_secret(),
+                ));
             }
             _ => {}
         }
 
-        match (
-            c.server_private_key(),
-            c.private().server_private_key_base64(),
-        ) {
+        match (c.server_private_key(), c.server_private_key_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'server_private_key' and 'server_private_key_base64' can be present!")
             }
             (Some(server_private_key), None) => {
                 ps.push((TLS_SERVER_PRIVATE_KEY_FILE, server_private_key));
             }
             (None, Some(server_private_key)) => {
-                ps.push((TLS_SERVER_PRIVATE_KEY_BASE64, server_private_key));
+                ps.push((
+                    TLS_SERVER_PRIVATE_KEY_BASE64,
+                    server_private_key.expose_secret(),
+                ));
             }
             _ => {}
         }
 
-        match (
-            c.server_certificate(),
-            c.private().server_certificate_base64(),
-        ) {
+        match (c.server_certificate(), c.server_certificate_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'server_certificate' and 'server_certificate_base64' can be present!")
             }
             (Some(server_certificate), None) => {
                 ps.push((TLS_SERVER_CERTIFICATE_FILE, server_certificate));
             }
             (None, Some(server_certificate)) => {
-                ps.push((TLS_SERVER_CERTIFICATE_BASE64, server_certificate));
+                ps.push((
+                    TLS_SERVER_CERTIFICATE_BASE64,
+                    server_certificate.expose_secret(),
+                ));
             }
             _ => {}
         }

diff --git a/io/zenoh-links/zenoh-link-tls/Cargo.toml b/io/zenoh-links/zenoh-link-tls/Cargo.toml
@@ -41,4 +41,5 @@ zenoh-protocol = { workspace = true }
 zenoh-result = { workspace = true }
 zenoh-sync = { workspace = true }
 zenoh-util = { workspace = true }
-base64 = { workspace = true }
+base64 = { workspace = true }
+secrecy = {workspace = true }
diff --git a/io/zenoh-links/zenoh-link-tls/src/lib.rs b/io/zenoh-links/zenoh-link-tls/src/lib.rs
@@ -26,6 +26,7 @@ use config::{
     TLS_ROOT_CA_CERTIFICATE_FILE, TLS_SERVER_CERTIFICATE_BASE64, TLS_SERVER_CERTIFICATE_FILE,
     TLS_SERVER_NAME_VERIFICATION, TLS_SERVER_PRIVATE_KEY_BASE_64, TLS_SERVER_PRIVATE_KEY_FILE,
 };
+use secrecy::ExposeSecret;
 use std::{convert::TryFrom, net::SocketAddr};
 use zenoh_config::Config;
 use zenoh_core::zconfigurable;
@@ -71,50 +72,50 @@ impl ConfigurationInspector<Config> for TlsConfigurator {
 
         let c = config.transport().link().tls();
 
-        match (
-            c.root_ca_certificate(),
-            c.private().root_ca_certificate_base64(),
-        ) {
+        match (c.root_ca_certificate(), c.root_ca_certificate_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'root_ca_certificate' and 'root_ca_certificate_base64' can be present!")
             }
             (Some(ca_certificate), None) => {
                 ps.push((TLS_ROOT_CA_CERTIFICATE_FILE, ca_certificate));
             }
             (None, Some(ca_certificate)) => {
-                ps.push((TLS_ROOT_CA_CERTIFICATE_BASE64, ca_certificate));
+                ps.push((
+                    TLS_ROOT_CA_CERTIFICATE_BASE64,
+                    ca_certificate.expose_secret(),
+                ));
             }
             _ => {}
         }
 
-        match (
-            c.server_private_key(),
-            c.private().server_private_key_base64(),
-        ) {
+        match (c.server_private_key(), c.server_private_key_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'server_private_key' and 'server_private_key_base64' can be present!")
             }
             (Some(server_private_key), None) => {
                 ps.push((TLS_SERVER_PRIVATE_KEY_FILE, server_private_key));
             }
             (None, Some(server_private_key)) => {
-                ps.push((TLS_SERVER_PRIVATE_KEY_BASE_64, server_private_key));
+                ps.push((
+                    TLS_SERVER_PRIVATE_KEY_BASE_64,
+                    server_private_key.expose_secret(),
+                ));
             }
             _ => {}
         }
 
-        match (
-            c.server_certificate(),
-            c.private().server_certificate_base64(),
-        ) {
+        match (c.server_certificate(), c.server_certificate_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'server_certificate' and 'server_certificate_base64' can be present!")
             }
             (Some(server_certificate), None) => {
                 ps.push((TLS_SERVER_CERTIFICATE_FILE, server_certificate));
             }
             (None, Some(server_certificate)) => {
-                ps.push((TLS_SERVER_CERTIFICATE_BASE64, server_certificate));
+                ps.push((
+                    TLS_SERVER_CERTIFICATE_BASE64,
+                    server_certificate.expose_secret(),
+                ));
             }
             _ => {}
         }
@@ -126,34 +127,34 @@ impl ConfigurationInspector<Config> for TlsConfigurator {
             };
         }
 
-        match (
-            c.client_private_key(),
-            c.private().client_private_key_base64(),
-        ) {
+        match (c.client_private_key(), c.client_private_key_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'client_private_key' and 'client_private_key_base64' can be present!")
             }
             (Some(client_private_key), None) => {
                 ps.push((TLS_CLIENT_PRIVATE_KEY_FILE, client_private_key));
             }
             (None, Some(client_private_key)) => {
-                ps.push((TLS_CLIENT_PRIVATE_KEY_BASE64, client_private_key));
+                ps.push((
+                    TLS_CLIENT_PRIVATE_KEY_BASE64,
+                    client_private_key.expose_secret(),
+                ));
             }
             _ => {}
         }
 
-        match (
-            c.client_certificate(),
-            c.private().client_certificate_base64(),
-        ) {
+        match (c.client_certificate(), c.client_certificate_base64()) {
             (Some(_), Some(_)) => {
                 bail!("Only one between 'client_certificate' and 'client_certificate_base64' can be present!")
             }
             (Some(client_certificate), None) => {
                 ps.push((TLS_CLIENT_CERTIFICATE_FILE, client_certificate));
             }
             (None, Some(client_certificate)) => {
-                ps.push((TLS_CLIENT_CERTIFICATE_BASE64, client_certificate));
+                ps.push((
+                    TLS_CLIENT_CERTIFICATE_BASE64,
+                    client_certificate.expose_secret(),
+                ));
             }
             _ => {}
         }