feat: [torrust#878] extract tsl_config in toml config

josecelano · josecelano · commit 2341c1334dc2 · 2024-06-17T19:40:07.000+01:00
TSL configuration for HTTP trackers and the Tracker API is still
optional. However, when it's provided is enabled. The `ssl_enabled`
field was removed. You can remove the whole `tsl_config` to disable TSL.

If you want to kee a copy in the TOML file you can just comment the
lines.

```toml
[[http_trackers]]
...

[http_trackers.tsl_config]
ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
ssl_key_path = "./storage/tracker/lib/tls/localhost.key"

[http_api]
...

[http_api.tsl_config]
ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
ssl_key_path = "./storage/tracker/lib/tls/localhost.key"
```
diff --git a/Cargo.toml b/Cargo.toml
@@ -37,7 +37,7 @@ axum = { version = "0", features = ["macros"] }
 axum-client-ip = "0"
 axum-extra = { version = "0", features = ["query"] }
 axum-server = { version = "0", features = ["tls-rustls"] }
-camino = { version = "1.1.6", features = ["serde"] }
+camino = { version = "1.1.6", features = ["serde", "serde1"] }
 chrono = { version = "0", default-features = false, features = ["clock"] }
 clap = { version = "4", features = ["derive", "env"] }
 crossbeam-skiplist = "0.1"
diff --git a/docs/containers.md b/docs/containers.md
@@ -330,24 +330,23 @@ The storage folder must contain your certificates:
 
 ```s
 storage/tracker/lib/tls
-    ├── localhost.crt
-    └── localhost.key
+                    ├── localhost.crt
+                    └── localhost.key
+storage/http_api/lib/tls
+                     ├── localhost.crt
+                     └── localhost.key
 ```
 
 You have not enabled it in your `tracker.toml` file:
 
 ```toml
+[http_trackers.tsl_config]
+ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
+ssl_key_path = "./storage/tracker/lib/tls/localhost.key"
 
-[[http_trackers]]
-# ...
-ssl_enabled = true
-# ...
-
-[http_api]
-# ...
-ssl_enabled = true
-# ...
-
+[http_api.tsl_config]
+ssl_cert_path = "./storage/http_api/lib/tls/localhost.crt"
+ssl_key_path = "./storage/http_api/lib/tls/localhost.key"
 ```
 
 > NOTE: you can enable it independently for each HTTP tracker or the API.
diff --git a/packages/configuration/Cargo.toml b/packages/configuration/Cargo.toml
@@ -15,7 +15,7 @@ rust-version.workspace = true
 version.workspace = true
 
 [dependencies]
-camino = { version = "1.1.6", features = ["serde"] }
+camino = { version = "1.1.6", features = ["serde", "serde1"] }
 derive_more = "0"
 figment = { version = "0.10.18", features = ["env", "test", "toml"] }
 serde = { version = "1", features = ["derive"] }
diff --git a/packages/configuration/src/lib.rs b/packages/configuration/src/lib.rs
@@ -13,7 +13,7 @@ use std::sync::Arc;
 use camino::Utf8PathBuf;
 use derive_more::Constructor;
 use serde::{Deserialize, Serialize};
-use serde_with::{serde_as, NoneAsEmptyString};
+use serde_with::serde_as;
 use thiserror::Error;
 use torrust_tracker_located_error::{DynError, LocatedError};
 
@@ -215,24 +215,23 @@ impl From<figment::Error> for Error {
 #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone, Default)]
 pub struct TslConfig {
     /// Path to the SSL certificate file.
-    #[serde_as(as = "NoneAsEmptyString")]
     #[serde(default = "TslConfig::default_ssl_cert_path")]
-    pub ssl_cert_path: Option<Utf8PathBuf>,
+    pub ssl_cert_path: Utf8PathBuf,
+
     /// Path to the SSL key file.
-    #[serde_as(as = "NoneAsEmptyString")]
     #[serde(default = "TslConfig::default_ssl_key_path")]
-    pub ssl_key_path: Option<Utf8PathBuf>,
+    pub ssl_key_path: Utf8PathBuf,
 }
 
 impl TslConfig {
     #[allow(clippy::unnecessary_wraps)]
-    fn default_ssl_cert_path() -> Option<Utf8PathBuf> {
-        Some(Utf8PathBuf::new())
+    fn default_ssl_cert_path() -> Utf8PathBuf {
+        Utf8PathBuf::new()
     }
 
     #[allow(clippy::unnecessary_wraps)]
-    fn default_ssl_key_path() -> Option<Utf8PathBuf> {
-        Some(Utf8PathBuf::new())
+    fn default_ssl_key_path() -> Utf8PathBuf {
+        Utf8PathBuf::new()
     }
 }
 
diff --git a/packages/configuration/src/v1/http_tracker.rs b/packages/configuration/src/v1/http_tracker.rs
@@ -12,28 +12,25 @@ pub struct HttpTracker {
     /// Weather the HTTP tracker is enabled or not.
     #[serde(default = "HttpTracker::default_enabled")]
     pub enabled: bool,
+
     /// The address the tracker will bind to.
     /// The format is `ip:port`, for example `0.0.0.0:6969`. If you want to
     /// listen to all interfaces, use `0.0.0.0`. If you want the operating
     /// system to choose a random port, use port `0`.
     #[serde(default = "HttpTracker::default_bind_address")]
     pub bind_address: SocketAddr,
-    /// Weather the HTTP tracker will use SSL or not.
-    #[serde(default = "HttpTracker::default_ssl_enabled")]
-    pub ssl_enabled: bool,
-    /// TSL config. Only used if `ssl_enabled` is true.
-    #[serde(flatten)]
-    #[serde(default = "TslConfig::default")]
-    pub tsl_config: TslConfig,
+
+    /// TSL config.
+    #[serde(default = "HttpTracker::default_tsl_config")]
+    pub tsl_config: Option<TslConfig>,
 }
 
 impl Default for HttpTracker {
     fn default() -> Self {
         Self {
             enabled: Self::default_enabled(),
             bind_address: Self::default_bind_address(),
-            ssl_enabled: Self::default_ssl_enabled(),
-            tsl_config: TslConfig::default(),
+            tsl_config: Self::default_tsl_config(),
         }
     }
 }
@@ -47,7 +44,7 @@ impl HttpTracker {
         SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 7070)
     }
 
-    fn default_ssl_enabled() -> bool {
-        false
+    fn default_tsl_config() -> Option<TslConfig> {
+        None
     }
 }
diff --git a/packages/configuration/src/v1/mod.rs b/packages/configuration/src/v1/mod.rs
@@ -176,14 +176,16 @@
 //!
 //! ```s
 //! [[http_trackers]]
-//! enabled = true
 //! ...
+//!
+//! [http_trackers.tsl_config]
 //! ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
 //! ssl_key_path = "./storage/tracker/lib/tls/localhost.key"
 //!
 //! [http_api]
-//! enabled = true
 //! ...
+//!
+//! [http_api.tsl_config]
 //! ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
 //! ssl_key_path = "./storage/tracker/lib/tls/localhost.key"
 //! ```
@@ -225,16 +227,10 @@
 //! [[http_trackers]]
 //! enabled = false
 //! bind_address = "0.0.0.0:7070"
-//! ssl_enabled = false
-//! ssl_cert_path = ""
-//! ssl_key_path = ""
 //!
 //! [http_api]
 //! enabled = true
 //! bind_address = "127.0.0.1:1212"
-//! ssl_enabled = false
-//! ssl_cert_path = ""
-//! ssl_key_path = ""
 //!
 //! [http_api.access_tokens]
 //! admin = "MyAccessToken"
@@ -419,16 +415,10 @@ mod tests {
                                 [[http_trackers]]
                                 enabled = false
                                 bind_address = "0.0.0.0:7070"
-                                ssl_enabled = false
-                                ssl_cert_path = ""
-                                ssl_key_path = ""
-
+                                
                                 [http_api]
                                 enabled = true
                                 bind_address = "127.0.0.1:1212"
-                                ssl_enabled = false
-                                ssl_cert_path = ""
-                                ssl_key_path = ""
 
                                 [http_api.access_tokens]
                                 admin = "MyAccessToken"
diff --git a/packages/configuration/src/v1/tracker_api.rs b/packages/configuration/src/v1/tracker_api.rs
@@ -15,19 +15,18 @@ pub struct HttpApi {
     /// Weather the HTTP API is enabled or not.
     #[serde(default = "HttpApi::default_enabled")]
     pub enabled: bool,
+
     /// The address the tracker will bind to.
     /// The format is `ip:port`, for example `0.0.0.0:6969`. If you want to
     /// listen to all interfaces, use `0.0.0.0`. If you want the operating
     /// system to choose a random port, use port `0`.
     #[serde(default = "HttpApi::default_bind_address")]
     pub bind_address: SocketAddr,
-    /// Weather the HTTP API will use SSL or not.
-    #[serde(default = "HttpApi::default_ssl_enabled")]
-    pub ssl_enabled: bool,
+
     /// TSL config. Only used if `ssl_enabled` is true.
-    #[serde(flatten)]
-    #[serde(default = "TslConfig::default")]
-    pub tsl_config: TslConfig,
+    #[serde(default = "HttpApi::default_tsl_config")]
+    pub tsl_config: Option<TslConfig>,
+
     /// Access tokens for the HTTP API. The key is a label identifying the
     /// token and the value is the token itself. The token is used to
     /// authenticate the user. All tokens are valid for all endpoints and have
@@ -41,8 +40,7 @@ impl Default for HttpApi {
         Self {
             enabled: Self::default_enabled(),
             bind_address: Self::default_bind_address(),
-            ssl_enabled: Self::default_ssl_enabled(),
-            tsl_config: TslConfig::default(),
+            tsl_config: Self::default_tsl_config(),
             access_tokens: Self::default_access_tokens(),
         }
     }
@@ -57,8 +55,9 @@ impl HttpApi {
         SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 1212)
     }
 
-    fn default_ssl_enabled() -> bool {
-        false
+    #[allow(clippy::unnecessary_wraps)]
+    fn default_tsl_config() -> Option<TslConfig> {
+        None
     }
 
     fn default_access_tokens() -> AccessTokens {
diff --git a/share/default/config/tracker.container.mysql.toml b/share/default/config/tracker.container.mysql.toml
@@ -3,9 +3,5 @@ driver = "MySQL"
 path = "mysql://db_user:db_user_secret_password@mysql:3306/torrust_tracker"
 
 [[http_trackers]]
-ssl_cert_path = "/var/lib/torrust/tracker/tls/localhost.crt"
-ssl_key_path = "/var/lib/torrust/tracker/tls/localhost.key"
-
-[http_api]
-ssl_cert_path = "/var/lib/torrust/tracker/tls/localhost.crt"
-ssl_key_path = "/var/lib/torrust/tracker/tls/localhost.key"
+bind_address = "0.0.0.0:7070"
+enabled = true
diff --git a/share/default/config/tracker.container.sqlite3.toml b/share/default/config/tracker.container.sqlite3.toml
@@ -1,10 +1,2 @@
 [core.database]
 path = "/var/lib/torrust/tracker/database/sqlite3.db"
-
-[[http_trackers]]
-ssl_cert_path = "/var/lib/torrust/tracker/tls/localhost.crt"
-ssl_key_path = "/var/lib/torrust/tracker/tls/localhost.key"
-
-[http_api]
-ssl_cert_path = "/var/lib/torrust/tracker/tls/localhost.crt"
-ssl_key_path = "/var/lib/torrust/tracker/tls/localhost.key"
diff --git a/share/default/config/tracker.e2e.container.sqlite3.toml b/share/default/config/tracker.e2e.container.sqlite3.toml
@@ -6,12 +6,6 @@ enabled = true
 
 [[http_trackers]]
 enabled = true
-ssl_cert_path = "/var/lib/torrust/tracker/tls/localhost.crt"
-ssl_key_path = "/var/lib/torrust/tracker/tls/localhost.key"
-
-[http_api]
-ssl_cert_path = "/var/lib/torrust/tracker/tls/localhost.crt"
-ssl_key_path = "/var/lib/torrust/tracker/tls/localhost.key"
 
 [health_check_api]
 # Must be bound to wildcard IP to be accessible from outside the container
diff --git a/src/bootstrap/jobs/http_tracker.rs b/src/bootstrap/jobs/http_tracker.rs
@@ -42,7 +42,7 @@ pub async fn start_job(
     if config.enabled {
         let socket = config.bind_address;
 
-        let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config)
+        let tls = make_rust_tls(&config.tsl_config)
             .await
             .map(|tls| tls.expect("it should have a valid http tracker tls configuration"));
 
diff --git a/src/bootstrap/jobs/mod.rs b/src/bootstrap/jobs/mod.rs
@@ -20,27 +20,33 @@ pub struct Started {
     pub address: std::net::SocketAddr,
 }
 
-pub async fn make_rust_tls(enabled: bool, tsl_config: &TslConfig) -> Option<Result<RustlsConfig, Error>> {
-    if !enabled {
-        info!("TLS not enabled");
-        return None;
-    }
+pub async fn make_rust_tls(opt_tsl_config: &Option<TslConfig>) -> Option<Result<RustlsConfig, Error>> {
+    match opt_tsl_config {
+        Some(tsl_config) => {
+            let cert = tsl_config.ssl_cert_path.clone();
+            let key = tsl_config.ssl_key_path.clone();
 
-    if let (Some(cert), Some(key)) = (tsl_config.ssl_cert_path.clone(), tsl_config.ssl_key_path.clone()) {
-        info!("Using https: cert path: {cert}.");
-        info!("Using https: key path: {key}.");
-
-        Some(
-            RustlsConfig::from_pem_file(cert, key)
-                .await
-                .map_err(|err| Error::BadTlsConfig {
-                    source: (Arc::new(err) as DynError).into(),
-                }),
-        )
-    } else {
-        Some(Err(Error::MissingTlsConfig {
-            location: Location::caller(),
-        }))
+            if !cert.exists() || !key.exists() {
+                return Some(Err(Error::MissingTlsConfig {
+                    location: Location::caller(),
+                }));
+            }
+
+            info!("Using https: cert path: {cert}.");
+            info!("Using https: key path: {key}.");
+
+            Some(
+                RustlsConfig::from_pem_file(cert, key)
+                    .await
+                    .map_err(|err| Error::BadTlsConfig {
+                        source: (Arc::new(err) as DynError).into(),
+                    }),
+            )
+        }
+        None => {
+            info!("TLS not enabled");
+            None
+        }
     }
 }
 
@@ -54,29 +60,23 @@ mod tests {
 
     #[tokio::test]
     async fn it_should_error_on_bad_tls_config() {
-        let err = make_rust_tls(
-            true,
-            &TslConfig {
-                ssl_cert_path: Some(Utf8PathBuf::from("bad cert path")),
-                ssl_key_path: Some(Utf8PathBuf::from("bad key path")),
-            },
-        )
+        let err = make_rust_tls(&Some(TslConfig {
+            ssl_cert_path: Utf8PathBuf::from("bad cert path"),
+            ssl_key_path: Utf8PathBuf::from("bad key path"),
+        }))
         .await
         .expect("tls_was_enabled")
         .expect_err("bad_cert_and_key_files");
 
-        assert!(matches!(err, Error::BadTlsConfig { source: _ }));
+        assert!(matches!(err, Error::MissingTlsConfig { location: _ }));
     }
 
     #[tokio::test]
     async fn it_should_error_on_missing_cert_or_key_paths() {
-        let err = make_rust_tls(
-            true,
-            &TslConfig {
-                ssl_cert_path: None,
-                ssl_key_path: None,
-            },
-        )
+        let err = make_rust_tls(&Some(TslConfig {
+            ssl_cert_path: Utf8PathBuf::from(""),
+            ssl_key_path: Utf8PathBuf::from(""),
+        }))
         .await
         .expect("tls_was_enabled")
         .expect_err("missing_config");
diff --git a/src/bootstrap/jobs/tracker_apis.rs b/src/bootstrap/jobs/tracker_apis.rs
@@ -63,7 +63,7 @@ pub async fn start_job(
     if config.enabled {
         let bind_to = config.bind_address;
 
-        let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config)
+        let tls = make_rust_tls(&config.tsl_config)
             .await
             .map(|tls| tls.expect("it should have a valid tracker api tls configuration"));
 
diff --git a/src/lib.rs b/src/lib.rs
diff --git a/src/servers/apis/mod.rs b/src/servers/apis/mod.rs
diff --git a/src/servers/apis/server.rs b/src/servers/apis/server.rs
diff --git a/src/servers/http/server.rs b/src/servers/http/server.rs
diff --git a/tests/servers/api/environment.rs b/tests/servers/api/environment.rs
diff --git a/tests/servers/http/environment.rs b/tests/servers/http/environment.rs
