feat(dgw): support for PFX files

README.md

@@ -80,9 +80,18 @@ Stable options are:
         * `Base64Url`
         * `Base64UrlPad`
 
-- **DelegationPrivateKeyFile** (_FilePath_): Path to the delegation private key (used to decipher sensitive data from tokens).
+- **DelegationPrivateKeyFile** (_FilePath_): Path to the delegation private key which is used to
+    decipher sensitive data from tokens.
 
-- **TlsCertificateSource** (_String_): Source for the TLS certificate. Possible values are: `External` (default), `System`.
+- **TlsCertificateSource** (_String_): Source for the TLS certificate.
+
+    Possible values:
+
+    * `External` (default): Retrieve a certificate stored on the file system.
+        See the options **TlsCertificateFile**, **TlsPrivateKeyFile** and **TlsPrivateKeyPassword**.
+
+    * `System`: Retrieve the certificate managed by the system certificate store. 
+        See the options **TlsCertificateSubjectName**, **TlsCertificateStoreName** and **TlsCertificateStoreLocation**.
 
 - **TlsCertificateSubjectName** (_String_): Subject name of the certificate to use for TLS when using system source.
 
@@ -100,6 +109,17 @@ Stable options are:
 
 - **TlsPrivateKeyFile** (_FilePath_): Path to the private key to use for TLS.
 
+- **TlsPrivateKeyPassword** (_String_): Password to use for decrypting the TLS private key.
+
+    It's important to understand that using this option in order to use an encrypted private key
+    does not inherently enhance security beyond using a plain, unencrypted private key. In fact,
+    storing the password in plain text within the configuration file is _discouraged_ because it
+    provides minimal security advantages over the unencrypted private key. If an unauthorized person
+    gains access to both the configuration file and the private key, they can easily retrieve the
+    password, making it as vulnerable as an unencrypted private key. To bolster security, consider
+    additional measures like securing access to the files or using the system certificate store (see
+    **TlsCertificateSource** option).
+
 - **Listeners** (_Array_): Array of listener URLs.
 
     Each element has the following schema: 
@@ -207,9 +227,9 @@ anymore by default.
 
 See [this page from Microsoft documentation][microsoft_tls] to learn how to properly configure SChannel.
 
-### More
+## Knowledge base
 
-Read [our knowledge base](https://docs.devolutions.net/kb/devolutions-gateway/).
+Read more on [our knowledge base](https://docs.devolutions.net/kb/devolutions-gateway/).
 
 ## Cookbook
 

devolutions-gateway/Cargo.toml

@@ -49,7 +49,7 @@ typed-builder = "0.17"
 backoff = "0.4"
 
 # Security, crypto…
-picky = { version = "7.0.0-rc.8", default-features = false, features = ["jose", "x509"] }
+picky = { version = "7.0.0-rc.8", default-features = false, features = ["jose", "x509", "pkcs12"] }
 zeroize = { version = "1.6", features = ["derive"] }
 multibase = "0.9"
 

devolutions-gateway/src/api/kdc_proxy.rs

@@ -108,7 +108,7 @@ async fn kdc_proxy(
             .map_err(HttpError::internal().with_msg("unable to read KDC reply message").err())?
     } else {
         // we assume that ticket length is not greater than 2048
-        let mut buff = [0; 2048];
+        let mut buf = [0; 2048];
 
         let port = portpicker::pick_unused_port().ok_or_else(|| HttpError::internal().msg("No free ports"))?;
 
@@ -134,15 +134,15 @@ async fn kdc_proxy(
 
         trace!("Reading KDC reply...");
 
-        let n = udp_socket.recv(&mut buff).await.map_err(
+        let n = udp_socket.recv(&mut buf).await.map_err(
             HttpError::internal()
                 .with_msg("unable to read reply from the KDC server")
                 .err(),
         )?;
 
         let mut reply_buf = Vec::new();
         reply_buf.extend_from_slice(&(n as u32).to_be_bytes());
-        reply_buf.extend_from_slice(&buff[0..n]);
+        reply_buf.extend_from_slice(&buf[0..n]);
         reply_buf
     };
 

devolutions-gateway/src/config.rs

@@ -126,19 +126,28 @@ impl Conf {
                 None
             }
             dto::CertSource::External => {
-                let certificates = conf_file
+                let certificate_path = conf_file
                     .tls_certificate_file
                     .as_ref()
-                    .context("TLS certificate file is missing")?
-                    .pipe_deref(read_rustls_certificate_file)
-                    .context("TLS certificate")?;
-
-                let private_key = conf_file
-                    .tls_private_key_file
-                    .as_ref()
-                    .context("TLS private key file is missing")?
-                    .pipe_deref(read_rustls_priv_key_file)
-                    .context("TLS private key")?;
+                    .context("TLS usage implied, but TLS certificate file is missing")?;
+
+                let (certificates, private_key) = match certificate_path.extension() {
+                    Some("pfx" | "p12") => read_pfx_file(certificate_path, conf_file.tls_private_key_password.as_ref())
+                        .context("read PFX/PKCS12 file")?,
+                    None | Some(_) => {
+                        let certificates =
+                            read_rustls_certificate_file(certificate_path).context("read TLS certificate")?;
+
+                        let private_key = conf_file
+                            .tls_private_key_file
+                            .as_ref()
+                            .context("TLS private key file is missing")?
+                            .pipe_deref(read_rustls_priv_key_file)
+                            .context("read TLS private key")?;
+
+                        (certificates, private_key)
+                    }
+                };
 
                 let cert_source = crate::tls::CertificateSource::External {
                     certificates,
@@ -151,7 +160,7 @@ impl Conf {
                 let cert_subject_name = conf_file
                     .tls_certificate_subject_name
                     .clone()
-                    .context("TLS certificate subject name is missing")?;
+                    .context("TLS usage implied, but TLS certificate subject name is missing")?;
 
                 let store_location = conf_file.tls_certificate_store_location.unwrap_or_default();
 
@@ -373,6 +382,117 @@ fn default_hostname() -> Option<String> {
     hostname::get().ok()?.into_string().ok()
 }
 
+fn read_pfx_file(
+    path: &Utf8Path,
+    password: Option<&dto::Password>,
+) -> anyhow::Result<(Vec<rustls::Certificate>, rustls::PrivateKey)> {
+    use picky::pkcs12::{
+        Pfx, Pkcs12AttributeKind, Pkcs12CryptoContext, Pkcs12ParsingParams, SafeBagKind, SafeContentsKind,
+    };
+    use picky::x509::certificate::CertType;
+    use std::cmp::Ordering;
+
+    let crypto_context = password
+        .map(|pwd| Pkcs12CryptoContext::new_with_password(pwd.get()))
+        .unwrap_or_else(|| Pkcs12CryptoContext::new_without_password());
+    let parsing_params = Pkcs12ParsingParams::default();
+
+    let pfx_contents = normalize_data_path(path, &get_data_dir())
+        .pipe_ref(std::fs::read)
+        .with_context(|| format!("failed to read file at {path}"))?;
+
+    let pfx = Pfx::from_der(&pfx_contents, &crypto_context, &parsing_params).context("failed to decode PFX")?;
+
+    // Build an iterator over all the safe bags of the PFX
+    let safe_bags_it = pfx
+        .safe_contents()
+        .iter()
+        .flat_map(|safe_contents| match safe_contents.kind() {
+            SafeContentsKind::SafeBags(safe_bags) => safe_bags.iter(),
+            SafeContentsKind::EncryptedSafeBags { safe_bags, .. } => safe_bags.iter(),
+            SafeContentsKind::Unknown => std::slice::Iter::default(),
+        })
+        .flat_map(|safe_bag| {
+            if let SafeBagKind::Nested(safe_bags) = safe_bag.kind() {
+                safe_bags.iter()
+            } else {
+                std::slice::from_ref(safe_bag).iter()
+            }
+        });
+
+    let mut certificates = Vec::new();
+    let mut private_keys = Vec::new();
+
+    // Iterate on all safe bags, and collect all certificates and private keys along their local key id (which is optional)
+    for safe_bag in safe_bags_it {
+        let local_key_id = safe_bag.attributes().iter().find_map(|attr| {
+            if let Pkcs12AttributeKind::LocalKeyId(key_id) = attr.kind() {
+                Some(key_id.as_slice())
+            } else {
+                None
+            }
+        });
+
+        match safe_bag.kind() {
+            SafeBagKind::PrivateKey(key) | SafeBagKind::EncryptedPrivateKey { key, .. } => {
+                private_keys.push((key, local_key_id))
+            }
+            SafeBagKind::Certificate(cert) => certificates.push((cert, local_key_id)),
+            _ => {}
+        }
+    }
+
+    // Sort certificates such that: Leaf < Unknown < Intermediate < Root (stable sort usage is deliberate)
+    certificates.sort_by(|(lhs, _), (rhs, _)| match (lhs.ty(), rhs.ty()) {
+        // Equality
+        (CertType::Leaf, CertType::Leaf) => Ordering::Equal,
+        (CertType::Unknown, CertType::Unknown) => Ordering::Equal,
+        (CertType::Intermediate, CertType::Intermediate) => Ordering::Equal,
+        (CertType::Root, CertType::Root) => Ordering::Equal,
+
+        // Leaf
+        (CertType::Leaf, _) => Ordering::Less,
+        (_, CertType::Leaf) => Ordering::Greater,
+
+        // Unknown
+        (CertType::Unknown, _) => Ordering::Less,
+        (_, CertType::Unknown) => Ordering::Greater,
+
+        // Intermediate
+        (CertType::Intermediate, CertType::Root) => Ordering::Less,
+        (CertType::Root, CertType::Intermediate) => Ordering::Greater,
+    });
+
+    // Find the first certificate that is "closer" to being a leaf
+    let (_, leaf_local_key_id) = certificates.first().context("leaf certificate not found")?;
+
+    // If there is a local key id, find the key with this same local key id, otherwise take the first key
+    let private_key = if let Some(leaf_local_key_id) = *leaf_local_key_id {
+        private_keys
+            .into_iter()
+            .find_map(|(pk, local_key_id)| match local_key_id {
+                Some(local_key_id) if local_key_id == leaf_local_key_id => Some(pk),
+                _ => None,
+            })
+    } else {
+        private_keys.into_iter().map(|(pk, _)| pk).next()
+    };
+
+    let private_key = private_key.context("leaf private key not found")?.clone();
+    let private_key = private_key
+        .to_pkcs8()
+        .map(rustls::PrivateKey)
+        .context("invalid private key")?;
+
+    let certificates = certificates
+        .into_iter()
+        .map(|(cert, _)| cert.to_der().map(rustls::Certificate))
+        .collect::<Result<_, _>>()
+        .context("invalid certificate")?;
+
+    Ok((certificates, private_key))
+}
+
 fn read_rustls_certificate_file(path: &Utf8Path) -> anyhow::Result<Vec<rustls::Certificate>> {
     read_rustls_certificate(Some(path), None).transpose().unwrap()
 }
@@ -577,6 +697,8 @@ fn to_listener_urls(conf: &dto::ListenerConf, hostname: &str, auto_ipv6: bool) -
 pub mod dto {
     use std::collections::HashMap;
 
+    use serde::{de, ser};
+
     use super::*;
 
     /// Source of truth for Gateway configuration
@@ -620,6 +742,9 @@ pub mod dto {
         /// Private key to use for TLS
         #[serde(alias = "PrivateKeyFile", skip_serializing_if = "Option::is_none")]
         pub tls_private_key_file: Option<Utf8PathBuf>,
+        /// Password to use for decrypting the TLS private key
+        #[serde(skip_serializing_if = "Option::is_none")]
+        pub tls_private_key_password: Option<Password>,
         /// Subject name of the certificate to use for TLS
         #[serde(skip_serializing_if = "Option::is_none")]
         pub tls_certificate_subject_name: Option<String>,
@@ -690,6 +815,7 @@ pub mod dto {
                 tls_certificate_source: None,
                 tls_certificate_file: None,
                 tls_private_key_file: None,
+                tls_private_key_password: None,
                 tls_certificate_subject_name: None,
                 tls_certificate_store_name: None,
                 tls_certificate_store_location: None,
@@ -987,4 +1113,64 @@ pub mod dto {
         CurrentService,
         LocalMachine,
     }
+
+    #[derive(PartialEq, Eq, Clone, zeroize::Zeroize)]
+    pub struct Password(String);
+
+    impl fmt::Debug for Password {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.debug_struct("Password").finish_non_exhaustive()
+        }
+    }
+
+    impl Password {
+        /// Do not copy the return value without wrapping into some "Zeroize"able structure.
+        pub fn get(&self) -> &str {
+            &self.0
+        }
+    }
+
+    impl<'de> de::Deserialize<'de> for Password {
+        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+        where
+            D: serde::Deserializer<'de>,
+        {
+            struct V;
+
+            impl<'de> de::Visitor<'de> for V {
+                type Value = Password;
+
+                fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                    formatter.write_str("a string")
+                }
+
+                fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
+                where
+                    E: de::Error,
+                {
+                    Ok(Password(v))
+                }
+
+                fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+                where
+                    E: de::Error,
+                {
+                    Ok(Password(v.to_owned()))
+                }
+            }
+
+            let password = deserializer.deserialize_string(V)?;
+
+            Ok(password)
+        }
+    }
+
+    impl ser::Serialize for Password {
+        fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+        where
+            S: serde::Serializer,
+        {
+            serializer.serialize_str(&self.0)
+        }
+    }
 }