Update core/edge url when changing cert configuration (#2725)

- ensure https schema in Settings::{defguard_url, public_proxy_url} when switching to CA/own cert/LE
- don't modify schema when switching to no cert in case ssl is not terminated by reverse proxy

Author: j-chmielewski

crates/defguard_core/src/cert_settings.rs

@@ -4,7 +4,7 @@ use defguard_certs::{
     parse_pem_certificate,
 };
 use defguard_common::db::models::{
-    Certificates, CoreCertSource, ProxyCertSource, settings::update_current_settings,
+    Certificates, CoreCertSource, ProxyCertSource, Settings, settings::update_current_settings,
 };
 use serde::{Deserialize, Serialize};
 use sqlx::PgPool;
@@ -94,6 +94,14 @@ pub struct ExternalUrlSettingsConfig {
     pub key_pem: Option<String>,
 }
 
+fn ensure_https(url: &str) -> String {
+    if let Some(rest) = url.strip_prefix("http://") {
+        format!("https://{rest}")
+    } else {
+        url.to_owned()
+    }
+}
+
 /// Core logic for applying internal URL certificate settings using the current Defguard URL.
 /// Returns cert info if a certificate was generated/uploaded, `None` for `ssl_type = None`.
 pub async fn apply_internal_url_settings(
@@ -106,11 +114,17 @@ pub async fn apply_internal_url_settings(
         defguard_url, config.ssl_type,
     );
 
-    let mut settings = defguard_common::db::models::Settings::get_current_settings();
-    settings.defguard_url = defguard_url.to_string();
-    update_current_settings(pool, settings).await?;
+    let mut settings = Settings::get_current_settings();
+    let mut transaction = pool.begin().await?;
+
+    // Modify url schema if necessary
+    settings.defguard_url = match config.ssl_type {
+        InternalSslType::None => defguard_url.to_string(),
+        InternalSslType::DefguardCa | InternalSslType::OwnCert => ensure_https(defguard_url),
+    };
+    update_current_settings(&mut *transaction, settings).await?;
 
-    let mut certs = Certificates::get_or_default(pool)
+    let mut certs = Certificates::get_or_default(&mut *transaction)
         .await
         .map_err(WebError::from)?;
 
@@ -120,7 +134,10 @@ pub async fn apply_internal_url_settings(
             certs.core_http_cert_pem = None;
             certs.core_http_cert_key_pem = None;
             certs.core_http_cert_expiry = None;
-            certs.save(pool).await.map_err(WebError::from)?;
+            certs
+                .save(&mut *transaction)
+                .await
+                .map_err(WebError::from)?;
             None
         }
         InternalSslType::DefguardCa => {
@@ -156,7 +173,10 @@ pub async fn apply_internal_url_settings(
             certs.core_http_cert_pem = Some(cert_pem);
             certs.core_http_cert_key_pem = Some(key_pem);
             certs.core_http_cert_expiry = Some(expiry);
-            certs.save(pool).await.map_err(WebError::from)?;
+            certs
+                .save(&mut *transaction)
+                .await
+                .map_err(WebError::from)?;
 
             Some(CertInfoResponse {
                 common_name: info.subject_common_name,
@@ -181,7 +201,10 @@ pub async fn apply_internal_url_settings(
             certs.core_http_cert_pem = Some(cert_pem_str);
             certs.core_http_cert_key_pem = Some(key_pem_str);
             certs.core_http_cert_expiry = Some(expiry);
-            certs.save(pool).await.map_err(WebError::from)?;
+            certs
+                .save(&mut *transaction)
+                .await
+                .map_err(WebError::from)?;
 
             Some(CertInfoResponse {
                 common_name: info.subject_common_name,
@@ -192,6 +215,7 @@ pub async fn apply_internal_url_settings(
         }
     };
 
+    transaction.commit().await?;
     Ok(cert_info)
 }
 
@@ -207,28 +231,37 @@ pub async fn apply_external_url_settings(
         public_proxy_url, config.ssl_type,
     );
 
-    let mut certs = Certificates::get_or_default(pool)
+    let mut transaction = pool.begin().await?;
+    let mut certs = Certificates::get_or_default(&mut *transaction)
         .await
         .map_err(WebError::from)?;
 
-    let hostname = if matches!(
-        config.ssl_type,
-        ExternalSslType::LetsEncrypt | ExternalSslType::DefguardCa
-    ) {
-        let url = public_proxy_url.trim();
-        if url.is_empty() {
-            return Err(WebError::BadRequest(
-                "Public proxy URL is not configured".to_string(),
-            ));
+    // Modify url schema if necessary
+    let mut settings = Settings::get_current_settings();
+    settings.public_proxy_url = match config.ssl_type {
+        ExternalSslType::None => public_proxy_url.to_string(),
+        ExternalSslType::LetsEncrypt | ExternalSslType::DefguardCa | ExternalSslType::OwnCert => {
+            ensure_https(public_proxy_url)
+        }
+    };
+    update_current_settings(&mut *transaction, settings).await?;
+
+    let hostname = match config.ssl_type {
+        ExternalSslType::None | ExternalSslType::OwnCert => String::new(),
+        ExternalSslType::DefguardCa | ExternalSslType::LetsEncrypt => {
+            let url = public_proxy_url.trim();
+            if url.is_empty() {
+                return Err(WebError::BadRequest(
+                    "Public proxy URL is not configured".to_string(),
+                ));
+            }
+
+            reqwest::Url::parse(url)
+                .ok()
+                .and_then(|u| u.host_str().map(ToString::to_string))
+                .filter(|host| !host.is_empty())
+                .unwrap_or_else(|| url.to_string())
         }
-
-        reqwest::Url::parse(url)
-            .ok()
-            .and_then(|u| u.host_str().map(ToString::to_string))
-            .filter(|host| !host.is_empty())
-            .unwrap_or_else(|| url.to_string())
-    } else {
-        String::new()
     };
 
     let cert_info = match config.ssl_type {
@@ -239,7 +272,10 @@ pub async fn apply_external_url_settings(
             certs.proxy_http_cert_pem = None;
             certs.proxy_http_cert_key_pem = None;
             certs.proxy_http_cert_expiry = None;
-            certs.save(pool).await.map_err(WebError::from)?;
+            certs
+                .save(&mut *transaction)
+                .await
+                .map_err(WebError::from)?;
             None
         }
         ExternalSslType::LetsEncrypt => {
@@ -278,7 +314,10 @@ pub async fn apply_external_url_settings(
             certs.proxy_http_cert_pem = Some(cert_pem);
             certs.proxy_http_cert_key_pem = Some(key_pem);
             certs.proxy_http_cert_expiry = Some(expiry);
-            certs.save(pool).await.map_err(WebError::from)?;
+            certs
+                .save(&mut *transaction)
+                .await
+                .map_err(WebError::from)?;
 
             Some(CertInfoResponse {
                 common_name: info.subject_common_name,
@@ -304,7 +343,10 @@ pub async fn apply_external_url_settings(
             certs.proxy_http_cert_pem = Some(cert_pem_str);
             certs.proxy_http_cert_key_pem = Some(key_pem_str);
             certs.proxy_http_cert_expiry = Some(expiry);
-            certs.save(pool).await.map_err(WebError::from)?;
+            certs
+                .save(&mut *transaction)
+                .await
+                .map_err(WebError::from)?;
 
             Some(CertInfoResponse {
                 common_name: info.subject_common_name,
@@ -315,5 +357,6 @@ pub async fn apply_external_url_settings(
         }
     };
 
+    transaction.commit().await?;
     Ok(cert_info)
 }

crates/defguard_core/src/handlers/core_certs.rs

@@ -1,6 +1,9 @@
 use axum::{Extension, Json, extract::State, http::StatusCode};
 use defguard_certs::{CertificateInfo, der_to_pem, parse_pem_certificate};
-use defguard_common::db::models::Certificates;
+use defguard_common::{
+    db::models::{Certificates, Settings},
+    types::proxy::ProxyControlMessage,
+};
 use serde_json::json;
 use sqlx::PgPool;
 
@@ -25,12 +28,7 @@ fn cert_common_name(cert_pem: Option<&str>) -> Option<String> {
 async fn broadcast_proxy_https_certs(appstate: &AppState, cert_pem: String, key_pem: String) {
     if let Err(err) = appstate
         .proxy_control_tx
-        .send(
-            defguard_common::types::proxy::ProxyControlMessage::BroadcastHttpsCerts {
-                cert_pem,
-                key_pem,
-            },
-        )
+        .send(ProxyControlMessage::BroadcastHttpsCerts { cert_pem, key_pem })
         .await
     {
         error!("Failed to broadcast HttpsCerts to proxies: {err:?}");
@@ -41,7 +39,7 @@ async fn broadcast_proxy_https_certs(appstate: &AppState, cert_pem: String, key_
 async fn clear_proxy_https_certs(appstate: &AppState) {
     if let Err(err) = appstate
         .proxy_control_tx
-        .send(defguard_common::types::proxy::ProxyControlMessage::ClearHttpsCerts)
+        .send(ProxyControlMessage::ClearHttpsCerts)
         .await
     {
         error!("Failed to broadcast ClearHttpsCerts to proxies: {err:?}");
@@ -78,7 +76,7 @@ pub(crate) async fn set_internal_url_settings(
         "User {} applying core internal URL certificate settings",
         session.user.username
     );
-    let settings = defguard_common::db::models::Settings::get_current_settings();
+    let settings = Settings::get_current_settings();
     let cert_info = apply_internal_url_settings(&pool, &settings.defguard_url, config).await?;
     reload_core_web_server(&appstate);
     info!(
@@ -116,7 +114,7 @@ pub(crate) async fn set_external_url_settings(
         "User {} applying proxy external URL certificate settings",
         session.user.username
     );
-    let settings = defguard_common::db::models::Settings::get_current_settings();
+    let settings = Settings::get_current_settings();
     let ssl_type = config.ssl_type.clone();
     let cert_info = apply_external_url_settings(&pool, &settings.public_proxy_url, config).await?;
 

crates/defguard_core/tests/integration/api/core_certs.rs

@@ -48,6 +48,9 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
         .send()
         .await;
     assert_eq!(response.status(), StatusCode::CREATED);
+    let mut settings = Settings::get(&pool).await.unwrap().unwrap();
+    // Don't touch the URL if setting no cert
+    assert_eq!(settings.defguard_url, "https://defguard.example.com");
 
     let saved = Certificates::get(&pool).await.unwrap().unwrap();
     assert_eq!(saved.core_http_cert_source, CoreCertSource::None);
@@ -57,12 +60,17 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
 
     seed_ca(&pool).await;
 
+    settings.defguard_url = "http://defguard.example.com".to_string();
+    update_current_settings(&pool, settings).await.unwrap();
     let response = client
         .post("/api/v1/core/cert/internal_url_settings")
         .json(&json!({ "ssl_type": "defguard_ca" }))
         .send()
         .await;
     assert_eq!(response.status(), StatusCode::CREATED);
+    let mut settings = Settings::get(&pool).await.unwrap().unwrap();
+    // Url schema changed to https
+    assert_eq!(settings.defguard_url, "https://defguard.example.com");
 
     let body: serde_json::Value = response.json::<serde_json::Value>().await;
     assert!(!body["cert_info"].is_null());
@@ -79,6 +87,8 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
             .contains("BEGIN CERTIFICATE")
     );
 
+    settings.defguard_url = "http://defguard.example.com".to_string();
+    update_current_settings(&pool, settings).await.unwrap();
     let (cert_pem, key_pem) = generate_test_cert_pem("uploaded.example.com");
     let response = client
         .post("/api/v1/core/cert/internal_url_settings")
@@ -90,6 +100,9 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
         .send()
         .await;
     assert_eq!(response.status(), StatusCode::CREATED);
+    let mut settings = Settings::get(&pool).await.unwrap().unwrap();
+    // Url schema changed to https
+    assert_eq!(settings.defguard_url, "https://defguard.example.com");
 
     let body: serde_json::Value = response.json::<serde_json::Value>().await;
     assert_eq!(body["cert_info"]["common_name"], "uploaded.example.com");
@@ -98,6 +111,8 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
     assert_eq!(saved.core_http_cert_source, CoreCertSource::Custom);
     assert!(saved.core_http_cert_expiry.is_some());
 
+    settings.defguard_url = "http://defguard.example.com".to_string();
+    update_current_settings(&pool, settings).await.unwrap();
     let (_, mismatched_key_pem) = generate_test_cert_pem("different.example.com");
     let response = client
         .post("/api/v1/core/cert/internal_url_settings")
@@ -109,6 +124,9 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
         .send()
         .await;
     assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+    let mut settings = Settings::get(&pool).await.unwrap().unwrap();
+    // Url schema unchanged on errors
+    assert_eq!(settings.defguard_url, "http://defguard.example.com");
 
     let response = client
         .post("/api/v1/core/cert/internal_url_settings")
@@ -120,6 +138,8 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
         .await;
     assert_eq!(response.status(), StatusCode::BAD_REQUEST);
 
+    settings.defguard_url = "http://defguard.example.com".to_string();
+    update_current_settings(&pool, settings).await.unwrap();
     let (expired_cert_pem, expired_key_pem) = generate_expired_test_cert_pem("expired.example.com");
     let response = client
         .post("/api/v1/core/cert/internal_url_settings")
@@ -131,6 +151,9 @@ async fn test_internal_url_settings_endpoint(_: PgPoolOptions, options: PgConnec
         .send()
         .await;
     assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+    let settings = Settings::get(&pool).await.unwrap().unwrap();
+    // Url schema unchanged on errors
+    assert_eq!(settings.defguard_url, "http://defguard.example.com");
     let body: serde_json::Value = response.json().await;
     assert_eq!(body["msg"], "Certificate has expired");
 }