Minor improvements to HTTPS handling in Agama's web server

rust/agama-server/Cargo.toml

@@ -56,6 +56,7 @@ futures-util = { version = "0.3.30", default-features = false, features = [
 ] }
 libsystemd = "0.7.0"
 subprocess = "0.2.9"
+gethostname = "0.4.3"
 
 [[bin]]
 name = "agama-dbus-server"

rust/agama-server/src/agama-web-server.rs

@@ -6,6 +6,7 @@ use std::{
 
 use agama_lib::{auth::AuthToken, connection_to};
 use agama_server::{
+    cert::Certificate,
     l10n::helpers,
     logs::init_logging,
     web::{self, run_monitor},
@@ -21,7 +22,7 @@ use futures_util::pin_mut;
 use hyper::body::Incoming;
 use hyper_util::rt::{TokioExecutor, TokioIo};
 use hyper_util::server::conn::auto::Builder;
-use openssl::ssl::{Ssl, SslAcceptor, SslFiletype, SslMethod};
+use openssl::ssl::{Ssl, SslAcceptor, SslMethod};
 use tokio::sync::broadcast::channel;
 use tokio_openssl::SslStream;
 use tower::Service;
@@ -79,15 +80,13 @@ struct ServeArgs {
     #[arg(long, default_value = None)]
     address2: Option<String>,
 
-    /// Path to the SSL private key file in PEM format
-    #[arg(long, default_value = None)]
-    key: Option<String>,
+    #[arg(long, default_value = "/etc/agama.d/ssl/key.pem")]
+    key: Option<PathBuf>,
 
-    /// Path to the SSL certificate file in PEM format
-    #[arg(long, default_value = None)]
-    cert: Option<String>,
+    #[arg(long, default_value = "/etc/agama.d/ssl/cert.pem")]
+    cert: Option<PathBuf>,
 
-    /// The D-Bus address for connecting to the Agama service
+    // Agama D-Bus address
     #[arg(long, default_value = "unix:path=/run/agama/bus")]
     dbus_address: String,
 
@@ -97,28 +96,49 @@ struct ServeArgs {
 }
 
 impl ServeArgs {
-    /// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one
-    fn ssl_acceptor(&self) -> Result<SslAcceptor, openssl::error::ErrorStack> {
-        let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?;
+    /// Returns true of given path to certificate points to an existing file
+    fn valid_cert_path(&self) -> bool {
+        self.cert.as_ref().is_some_and(|c| Path::new(&c).exists())
+    }
 
-        if let (Some(cert), Some(key)) = (self.cert.clone(), self.key.clone()) {
-            tracing::info!("Loading PEM certificate: {}", cert);
-            tls_builder.set_certificate_file(PathBuf::from(cert), SslFiletype::PEM)?;
+    /// Returns true of given path to key points to an existing file
+    fn valid_key_path(&self) -> bool {
+        self.key.as_ref().is_some_and(|k| Path::new(&k).exists())
+    }
+
+    /// Takes options provided by user and loads / creates Certificate struct according to them
+    fn to_certificate(&self) -> anyhow::Result<Certificate> {
+        if self.valid_cert_path() && self.valid_key_path() {
+            let cert = self.cert.clone().unwrap();
+            let key = self.key.clone().unwrap();
 
-            tracing::info!("Loading PEM key: {}", key);
-            tls_builder.set_private_key_file(PathBuf::from(key), SslFiletype::PEM)?;
+            // read the provided certificate
+            Certificate::read(cert.as_path(), key.as_path())
         } else {
-            let (cert, key) = agama_server::cert::create_certificate()?;
+            // ask for self-signed certificate
+            let certificate = Certificate::new()?;
+
+            // write the certificate for the later use
+            // for now do not care if writing self generated certificate failed or not, in the
+            // worst case we will generate new one ... which will surely be better
+            let _ = certificate.write();
 
-            tls_builder.set_private_key(&key)?;
-            tls_builder.set_certificate(&cert)?;
+            Ok(certificate)
         }
+    }
+}
 
-        // check that the key belongs to the certificate
-        tls_builder.check_private_key()?;
+/// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one
+fn ssl_acceptor(certificate: &Certificate) -> Result<SslAcceptor, openssl::error::ErrorStack> {
+    let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?;
 
-        Ok(tls_builder.build())
-    }
+    tls_builder.set_private_key(&certificate.key)?;
+    tls_builder.set_certificate(&certificate.cert)?;
+
+    // check that the key belongs to the certificate
+    tls_builder.check_private_key()?;
+
+    Ok(tls_builder.build())
 }
 
 /// Checks whether the connection uses SSL or not
@@ -305,7 +325,7 @@ async fn serve_command(args: ServeArgs) -> anyhow::Result<()> {
     let service = web::service(config, tx, dbus, web_ui_dir).await?;
     // TODO: Move elsewhere? Use a singleton? (It would be nice to use the same
     // generated self-signed certificate on both ports.)
-    let ssl_acceptor = if let Ok(ssl_acceptor) = args.ssl_acceptor() {
+    let ssl_acceptor = if let Ok(ssl_acceptor) = ssl_acceptor(&args.to_certificate()?) {
         ssl_acceptor
     } else {
         return Err(anyhow::anyhow!("SSL initialization failed"));

rust/agama-server/src/cert.rs

@@ -1,84 +1,128 @@
+use anyhow;
+use gethostname::gethostname;
 use openssl::asn1::Asn1Time;
 use openssl::bn::{BigNum, MsbOption};
-use openssl::error::ErrorStack;
 use openssl::hash::MessageDigest;
 use openssl::pkey::{PKey, Private};
 use openssl::rsa::Rsa;
 use openssl::x509::extension::{BasicConstraints, SubjectAlternativeName, SubjectKeyIdentifier};
 use openssl::x509::{X509NameBuilder, X509};
+use std::{
+    fs,
+    io::{self, Write},
+    os::unix::fs::OpenOptionsExt,
+    path::Path,
+};
 
-// TODO: move the certificate related functions into a struct
-//
-// struct Certificate {
-//     certificate: X509,
-//     key: PKey<Private>,
-// }
-//
-// impl Certificate {
-//     // read from file, support some default location
-//     // (like /etc/agama.d/ssl/{certificate,key}.pem ?)
-//     pub read(cert: &str, key: &str) -> Result<Self>;
-//     // generate a self-signed certificate
-//     pub new() -> Self
-//     // dump to file
-//     pub write(...)
-// }
-
-/// Generates a self-signed SSL certificate
-/// see https://github.com/sfackler/rust-openssl/blob/master/openssl/examples/mk_certs.rs
-pub fn create_certificate() -> Result<(X509, PKey<Private>), ErrorStack> {
-    let rsa = Rsa::generate(2048)?;
-    let key = PKey::from_rsa(rsa)?;
-
-    let mut x509_name = X509NameBuilder::new()?;
-    x509_name.append_entry_by_text("O", "Agama")?;
-    x509_name.append_entry_by_text("CN", "localhost")?;
-    let x509_name = x509_name.build();
-
-    let mut builder = X509::builder()?;
-    builder.set_version(2)?;
-    let serial_number = {
-        let mut serial = BigNum::new()?;
-        serial.rand(159, MsbOption::MAYBE_ZERO, false)?;
-        serial.to_asn1_integer()?
-    };
-    builder.set_serial_number(&serial_number)?;
-    builder.set_subject_name(&x509_name)?;
-    builder.set_issuer_name(&x509_name)?;
-    builder.set_pubkey(&key)?;
-
-    let not_before = Asn1Time::days_from_now(0)?;
-    builder.set_not_before(&not_before)?;
-    let not_after = Asn1Time::days_from_now(365)?;
-    builder.set_not_after(&not_after)?;
-
-    builder.append_extension(BasicConstraints::new().critical().ca().build()?)?;
-
-    builder.append_extension(
-        SubjectAlternativeName::new()
-            // use the default Agama host name
-            // TODO: use the gethostname crate and use the current real hostname
-            .dns("agama")
-            // use the default name for the mDNS/Avahi
-            // TODO: check which name is actually used by mDNS, to avoid
-            // conflicts it might actually use something like agama-2.local
-            .dns("agama.local")
-            .build(&builder.x509v3_context(None, None))?,
-    )?;
-
-    let subject_key_identifier =
-        SubjectKeyIdentifier::new().build(&builder.x509v3_context(None, None))?;
-    builder.append_extension(subject_key_identifier)?;
-
-    builder.sign(&key, MessageDigest::sha256())?;
-    let cert = builder.build();
-
-    // for debugging you might dump the certificate to a file:
-    // use std::io::Write;
-    // let mut cert_file = std::fs::File::create("agama_cert.pem").unwrap();
-    // let mut key_file = std::fs::File::create("agama_key.pem").unwrap();
-    // cert_file.write_all(cert.to_pem().unwrap().as_ref()).unwrap();
-    // key_file.write_all(key.private_key_to_pem_pkcs8().unwrap().as_ref()).unwrap();
-
-    Ok((cert, key))
+const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl";
+
+/// Structure to handle and store certificate and private key which is later
+/// used for establishing HTTPS connection
+pub struct Certificate {
+    pub cert: X509,
+    pub key: PKey<Private>,
+}
+
+impl Certificate {
+    /// Writes cert, key to (for now well known) location(s)
+    pub fn write(&self) -> anyhow::Result<()> {
+        // check and create default dir if needed
+        if !Path::new(DEFAULT_CERT_DIR).is_dir() {
+            std::fs::create_dir_all(DEFAULT_CERT_DIR)?;
+        }
+
+        if let Ok(bytes) = self.cert.to_pem() {
+            write_and_restrict(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), &bytes)?;
+        }
+        if let Ok(bytes) = self.key.private_key_to_pem_pkcs8() {
+            write_and_restrict(Path::new(DEFAULT_CERT_DIR).join("key.pem"), &bytes)?;
+        }
+
+        Ok(())
+    }
+
+    /// Reads certificate and corresponding private key from given paths
+    pub fn read<T: AsRef<Path>>(cert: T, key: T) -> anyhow::Result<Self> {
+        let cert_bytes = std::fs::read(cert)?;
+        let key_bytes = std::fs::read(key)?;
+
+        let cert = X509::from_pem(&cert_bytes.as_slice());
+        let key = PKey::private_key_from_pem(&key_bytes.as_slice());
+
+        match (cert, key) {
+            (Ok(c), Ok(k)) => Ok(Certificate { cert: c, key: k }),
+            _ => Err(anyhow::anyhow!("Failed to read certificate")),
+        }
+    }
+
+    /// Creates a self-signed certificate
+    pub fn new() -> anyhow::Result<Self> {
+        let rsa = Rsa::generate(2048)?;
+        let key = PKey::from_rsa(rsa)?;
+
+        let hostname = gethostname()
+            .into_string()
+            .unwrap_or(String::from("localhost"));
+        let mut x509_name = X509NameBuilder::new()?;
+        x509_name.append_entry_by_text("O", "Agama")?;
+        x509_name.append_entry_by_text("CN", hostname.as_str())?;
+        let x509_name = x509_name.build();
+
+        let mut builder = X509::builder()?;
+        builder.set_version(2)?;
+        let serial_number = {
+            let mut serial = BigNum::new()?;
+            serial.rand(159, MsbOption::MAYBE_ZERO, false)?;
+            serial.to_asn1_integer()?
+        };
+        builder.set_serial_number(&serial_number)?;
+        builder.set_subject_name(&x509_name)?;
+        builder.set_issuer_name(&x509_name)?;
+        builder.set_pubkey(&key)?;
+
+        let not_before = Asn1Time::days_from_now(0)?;
+        builder.set_not_before(&not_before)?;
+        let not_after = Asn1Time::days_from_now(365)?;
+        builder.set_not_after(&not_after)?;
+
+        builder.append_extension(BasicConstraints::new().critical().ca().build()?)?;
+
+        builder.append_extension(
+            SubjectAlternativeName::new()
+                // use the default Agama host name
+                // TODO: use the gethostname crate and use the current real hostname
+                .dns("agama")
+                // use the default name for the mDNS/Avahi
+                // TODO: check which name is actually used by mDNS, to avoid
+                // conflicts it might actually use something like agama-2.local
+                .dns("agama.local")
+                .build(&builder.x509v3_context(None, None))?,
+        )?;
+
+        let subject_key_identifier =
+            SubjectKeyIdentifier::new().build(&builder.x509v3_context(None, None))?;
+        builder.append_extension(subject_key_identifier)?;
+
+        builder.sign(&key, MessageDigest::sha256())?;
+        let cert = builder.build();
+
+        Ok(Certificate {
+            cert: cert,
+            key: key,
+        })
+    }
+}
+
+/// Writes buf into a file at path and sets the file permissions for the root only access
+fn write_and_restrict<T: AsRef<Path>>(path: T, buf: &[u8]) -> io::Result<()> {
+    let mut file = fs::OpenOptions::new()
+        .create(true)
+        .truncate(true)
+        .write(true)
+        .mode(0o400)
+        .open(path)?;
+
+    file.write_all(buf)?;
+
+    Ok(())
 }

rust/package/agama.changes

@@ -1,3 +1,13 @@
+-------------------------------------------------------------------
+Fri Jun  7 05:58:48 UTC 2024 - Michal Filka <[email protected]>
+
+- Improvements in HTTPS setup
+  - self-signed certificate contains hostname
+  - self-signed certificate is stored into default location
+  - before creating new self-signed certificate a default location
+    (/etc/agama.d/ssl) is checked for a certificate 
+  - gh#openSUSE/agama#1228
+
 -------------------------------------------------------------------
 Wed Jun  5 13:53:59 UTC 2024 - José Iván López González <[email protected]>