Merge pull request #183 from cipherstash/dont-log-pem-on-error

fix: ensure pem data is never logged

Author: tobyhede

README.md

@@ -527,12 +527,20 @@ schema_reload_interval = "60"
 
 [tls]
 # Certificate path
-# Env: CS_TLS__CERTIFICATE
-certificate = "./server.cert"
+# Env: CS_TLS__CERTIFICATE_PATH
+certificate_path = "./server.cert"
 
 # Private Key path
-# Env: CS_TLS__PRIVATE_KEY
-private_key = "./server.key"
+# Env: CS_TLS__PRIVATE_KEY_PATH
+private_key_path = "./server.key"
+
+# Certificate path
+# Env: CS_TLS__CERTIFICATE_PEM
+certificate_pem = "..."
+
+# Private Key path
+# Env: CS_TLS__PRIVATE_KEY_PEM
+private_key_pem = "..."
 
 
 [auth]

docs/errors.md

@@ -21,6 +21,8 @@
   - [Unknown table](#encrypt-unknown-table)
   - [Unknown index term](#encrypt-unknown-index-term)
 
+- Configuration Errors:
+  - [Missing or invalid TLS configuration](#config-missing-or-invalid-tls)
 <!-- ---------------------------------------------------------------------------------------------------- -->
 
 <!-- ---------------------------------------------------------------------------------------------------- -->
@@ -339,3 +341,40 @@ Unknown Index Term for column '{column_name}' in table '{table_name}'.
 1. Define the encrypted configuration using [EQL](https://github.com/cipherstash/encrypt-query-language).
 
 
+<!-- ---------------------------------------------------------------------------------------------------- -->
+
+
+
+<!-- ---------------------------------------------------------------------------------------------------- -->
+
+# Configuration Errors
+
+
+## Database <a id='config-missing-or-invalid-tls'></a>
+
+There was a problem with the Tls configuration.
+
+
+### Error message
+
+```
+# PEM-based configuration
+Invalid Transport Layer Security (TLS) certificate.
+Invalid Transport Layer Security (TLS) private key.
+
+# Path-based configuration
+Missing Transport Layer Security (TLS) certificate at path: {path}.
+Missing Transport Layer Security (TLS) private key at path: {path}.
+```
+
+### How to Fix
+
+If using path-based configuration:
+Check that the certificate and private key exists at the specified path.
+Check that the certificate and private key are valid.
+
+If using PEM-based configuration:
+Check that the certificate and private key are valid.
+
+
+<!-- ---------------------------------------------------------------------------------------------------- -->

mise.local.example.toml

@@ -23,15 +23,16 @@ CS_EQL_VERSION="eql-0.5.4"
 # the respective .pem contents (starting with '-----BEGIN').
 # When TYPE is "Path", CERTIFICATE and PRIVATE_KEY must contain
 # the paths to the respoctive files.
-CS_TLS__TYPE = "Path"
-CS_TLS__CERTIFICATE = "tests/tls/server.cert"
-CS_TLS__PRIVATE_KEY = "tests/tls/server.key"
-# CS_TLS__TYPE = "Pem"
-# certificate = """-----BEGIN CERTIFICATE-----
+
+CS_TLS__CERTIFICATE_PATH = "tests/tls/server.cert"
+CS_TLS__PRIVATE_KEY_PATH = "tests/tls/server.key"
+
+
+# CS_TLS__CERTIFICATE_PEM = """-----BEGIN CERTIFICATE-----
 # ...your certificate content here...
 # -----END CERTIFICATE-----
 # """
-# private_key = """-----BEGIN PRIVATE KEY-----
+# CS_TLS__PRIVATE_KEY_PEM = """-----BEGIN PRIVATE KEY-----
 # ...your private key content here...
 # -----END PRIVATE KEY-----
 # """

packages/cipherstash-proxy/src/config/tls.rs

@@ -4,85 +4,77 @@ use rustls_pki_types::{pem::PemObject, CertificateDer, PrivateKeyDer};
 use serde::Deserialize;
 use tracing::debug;
 
-use crate::log::CONFIG;
+use crate::{error::TlsConfigError, log::CONFIG};
 
 ///
 /// Server TLS Configuration
 /// This is listener/inbound connection config
 ///
 #[derive(Clone, Debug, Deserialize)]
-#[serde(tag = "type")]
+#[serde(untagged)]
 pub enum TlsConfig {
     Pem {
-        certificate: String,
-        private_key: String,
+        certificate_pem: String,
+        private_key_pem: String,
     },
     Path {
-        certificate: String,
-        private_key: String,
+        certificate_path: String,
+        private_key_path: String,
     },
 }
 
-#[derive(Clone, Debug, Deserialize)]
-pub struct KeyCertPair {
-    pub certificate: String,
-    pub private_key: String,
-}
-
 impl TlsConfig {
-    pub fn cert_exists(&self) -> bool {
+    pub fn check_cert(&self) -> Result<(), TlsConfigError> {
         match self {
-            TlsConfig::Pem { certificate, .. } => {
-                debug!(target: CONFIG, msg = "TLS certificate is a pem string (content omitted)");
+            TlsConfig::Pem {
+                certificate_pem: certificate,
+                ..
+            } => {
+                debug!(target: CONFIG, msg = "TLS certificate from PEM string");
                 let certs = CertificateDer::pem_slice_iter(certificate.as_bytes())
                     .collect::<Result<Vec<_>, _>>()
                     .unwrap_or(Vec::new());
-                !certs.is_empty()
+                if certs.is_empty() {
+                    return Err(TlsConfigError::InvalidCertificate);
+                }
             }
-            TlsConfig::Path { certificate, .. } => {
-                debug!(target: CONFIG, msg = "TLS certificate is a path: {}", certificate);
-                PathBuf::from(certificate).exists()
+            TlsConfig::Path {
+                certificate_path, ..
+            } => {
+                debug!(target: CONFIG, msg = "TLS certificate from path", certificate_path);
+                if !PathBuf::from(certificate_path).exists() {
+                    return Err(TlsConfigError::MissingCertificate {
+                        path: certificate_path.to_owned(),
+                    });
+                }
             }
         }
+        Ok(())
     }
 
-    pub fn private_key_exists(&self) -> bool {
+    pub fn check_private_key(&self) -> Result<(), TlsConfigError> {
         match self {
-            TlsConfig::Pem { private_key, .. } => {
-                debug!(target: CONFIG, msg = "TLS private_key is a pem string (content omitted)");
-                PrivateKeyDer::from_pem_slice(private_key.as_bytes()).is_ok()
+            TlsConfig::Pem {
+                private_key_pem: private_key,
+                ..
+            } => {
+                debug!(target: CONFIG, msg = "TLS private key from PEM string");
+                if PrivateKeyDer::from_pem_slice(private_key.as_bytes()).is_err() {
+                    return Err(TlsConfigError::InvalidPrivateKey);
+                }
             }
-            TlsConfig::Path { private_key, .. } => {
-                debug!(target: CONFIG, msg = "TLS private_key is a path: {}", private_key);
-                PathBuf::from(private_key).exists()
+            TlsConfig::Path {
+                private_key_path, ..
+            } => {
+                debug!(target: CONFIG, msg = "TLS private key from path", private_key_path);
+                if !PathBuf::from(private_key_path).exists() {
+                    return Err(TlsConfigError::MissingPrivateKey {
+                        path: private_key_path.to_owned(),
+                    });
+                }
             }
         }
-    }
-
-    pub fn certificate(&self) -> &str {
-        match self {
-            Self::Pem { certificate, .. } | Self::Path { certificate, .. } => certificate,
-        }
-    }
-
-    pub fn private_key(&self) -> &str {
-        match self {
-            Self::Pem { private_key, .. } | Self::Path { private_key, .. } => private_key,
-        }
-    }
-
-    pub fn certificate_err_msg(&self) -> &str {
-        match self {
-            Self::Pem { .. } => "Transport Layer Security (TLS) Certificate is invalid",
-            Self::Path { .. } => "Transport Layer Security (TLS) Certificate not found",
-        }
-    }
-
-    pub fn private_key_err_msg(&self) -> &str {
-        match self {
-            Self::Pem { .. } => "Transport Layer Security (TLS) Private key is invalid",
-            Self::Path { .. } => "Transport Layer Security (TLS) Private key not found",
-        }
+        Ok(())
     }
 }
 
@@ -92,21 +84,21 @@ mod tests {
 
     fn test_config_with_path() -> TlsConfig {
         TlsConfig::Path {
-            certificate: "../../tests/tls/server.cert".to_string(),
-            private_key: "../../tests/tls/server.key".to_string(),
+            certificate_path: "../../tests/tls/server.cert".to_string(),
+            private_key_path: "../../tests/tls/server.key".to_string(),
         }
     }
 
     fn test_config_with_invalid_path() -> TlsConfig {
         TlsConfig::Path {
-            certificate: "/path/to/non-existent/file".to_string(),
-            private_key: "/path/to/non-existent/file".to_string(),
+            certificate_path: "/path/to/non-existent/file".to_string(),
+            private_key_path: "/path/to/non-existent/file".to_string(),
         }
     }
 
     fn test_config_with_pem() -> TlsConfig {
         TlsConfig::Pem {
-            certificate: "\
+            certificate_pem: "\
 -----BEGIN CERTIFICATE-----
 MIIDKzCCAhOgAwIBAgIUMXfu7Mj22j+e9Gt2gjV73TBg20wwDQYJKoZIhvcNAQEL
 BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI1MDEyNjAxNDkzMVoXDTI2MDEy
@@ -128,7 +120,7 @@ TU/T2RF2sDsSHrUIVMeifhYc0jfNlRwnUG5liN9BiGo1QxNZ9jGY/3ts5eu8+XM=
 -----END CERTIFICATE-----
 "
             .to_string(),
-            private_key: "\
+            private_key_pem: "\
 -----BEGIN PRIVATE KEY-----
 MIIEugIBADANBgkqhkiG9w0BAQEFAASCBKQwggSgAgEAAoIBAQCm6o6q/Q/wg97t
 OZAY7Yd47QOM+shXJhgK0lcTJSE9K6rbR4+Nvo/IJ4CUbzvd8lbj59IXpg/Sexvs
@@ -164,32 +156,32 @@ B+qwsnNEiDoJhgYj+cQ=
 
     fn test_config_with_invalid_pem() -> TlsConfig {
         TlsConfig::Pem {
-            certificate: "-----INVALID PEM-----".to_string(),
-            private_key: "-----INVALID PEM-----".to_string(),
+            certificate_pem: "-----INVALID PEM-----".to_string(),
+            private_key_pem: "-----INVALID PEM-----".to_string(),
         }
     }
 
     #[test]
     fn test_tls_cert_exists_with_path() {
-        assert!(test_config_with_path().cert_exists());
-        assert!(!test_config_with_invalid_path().cert_exists());
+        assert!(test_config_with_path().check_cert().is_ok());
+        assert!(test_config_with_invalid_path().check_cert().is_err());
     }
 
     #[test]
     fn test_tls_cert_exists_with_pem() {
-        assert!(test_config_with_pem().cert_exists());
-        assert!(!test_config_with_invalid_pem().cert_exists());
+        assert!(test_config_with_pem().check_cert().is_ok());
+        assert!(test_config_with_invalid_pem().check_cert().is_err());
     }
 
     #[test]
     fn test_tls_private_key_exists_with_path() {
-        assert!(test_config_with_path().private_key_exists());
-        assert!(!test_config_with_invalid_path().private_key_exists());
+        assert!(test_config_with_path().check_private_key().is_ok());
+        assert!(test_config_with_invalid_path().check_private_key().is_err());
     }
 
     #[test]
     fn test_tls_private_key_exists_with_pem() {
-        assert!(test_config_with_pem().private_key_exists());
-        assert!(!test_config_with_invalid_pem().private_key_exists());
+        assert!(test_config_with_pem().check_private_key().is_ok());
+        assert!(test_config_with_invalid_pem().check_private_key().is_err());
     }
 }

packages/cipherstash-proxy/src/error.rs

@@ -104,6 +104,9 @@ pub enum ConfigError {
     #[error(transparent)]
     Database(#[from] tokio_postgres::Error),
 
+    #[error(transparent)]
+    FileOrEnvironment(#[from] config::ConfigError),
+
     #[error("Dataset id is not a valid UUID.")]
     InvalidDatasetId,
 
@@ -122,9 +125,6 @@ pub enum ConfigError {
     #[error("Expected an Encrypt configuration table")]
     MissingEncryptConfigTable,
 
-    #[error("Missing Transport Layer Security (TLS) certificate")]
-    MissingCertificate,
-
     #[error(transparent)]
     Parse(#[from] serde_json::Error),
 
@@ -135,7 +135,34 @@ pub enum ConfigError {
     TlsRequired,
 
     #[error(transparent)]
-    FileOrEnvironment(#[from] config::ConfigError),
+    TlsConfigError(#[from] TlsConfigError),
+}
+
+#[derive(Error, Debug)]
+pub enum TlsConfigError {
+    #[error(
+        "Invalid Transport Layer Security (TLS) certificate. For help visit {}#config-missing-or-invalid-tls",
+        ERROR_DOC_BASE_URL
+    )]
+    InvalidCertificate,
+
+    #[error(
+        "Invalid Transport Layer Security (TLS) private key. For help visit {}#config-missing-or-invalid-tls",
+        ERROR_DOC_BASE_URL
+    )]
+    InvalidPrivateKey,
+
+    #[error(
+        "Missing Transport Layer Security (TLS) certificate at path: {path}. For help visit {}#config-missing-or-invalid-tls",
+        ERROR_DOC_BASE_URL
+    )]
+    MissingCertificate { path: String },
+
+    #[error(
+        "Missing Transport Layer Security (TLS) private key at path: {path}. For help visit {}#config-missing-or-invalid-tls",
+        ERROR_DOC_BASE_URL
+    )]
+    MissingPrivateKey { path: String },
 }
 
 #[derive(Error, Debug)]

packages/cipherstash-proxy/src/main.rs

@@ -175,21 +175,15 @@ async fn init(mut config: TandemConfig) -> Encrypt {
 
     match config.tls {
         Some(ref mut tls) => {
-            if !tls.cert_exists() {
-                error!(
-                    msg = tls.certificate_err_msg(),
-                    certificate = ?tls.certificate().lines().next().unwrap_or("") // show first line of PEM, or path (_should_ be 1 line)
-                );
+            _ = tls.check_cert().inspect_err(|err| {
+                error!(msg = err.to_string());
                 std::process::exit(exitcode::CONFIG);
-            }
+            });
 
-            if !tls.private_key_exists() {
-                error!(
-                    msg = tls.private_key_err_msg(),
-                    private_key = ?tls.private_key().lines().next().unwrap_or("") // show first line of PEM, or path (_should_ be 1 line)
-                );
+            _ = tls.check_private_key().inspect_err(|err| {
+                error!(msg = err.to_string());
                 std::process::exit(exitcode::CONFIG);
-            };
+            });
 
             match tls::configure_server(tls) {
                 Ok(_) => {