From 4a91eb16f167e2b80ac90010675179c2e01ab47e Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Thu, 6 Jun 2024 08:35:51 +0200 Subject: [PATCH 01/17] Store self-generated certificate for later use --- rust/agama-server/src/agama-web-server.rs | 6 ++++++ rust/agama-server/src/cert.rs | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 1cf0d150b4..5e1c5726da 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -101,6 +101,7 @@ impl ServeArgs { fn ssl_acceptor(&self) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; + // use default or explicitly provided certificate if any 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)?; @@ -108,8 +109,13 @@ impl ServeArgs { tracing::info!("Loading PEM key: {}", key); tls_builder.set_private_key_file(PathBuf::from(key), SslFiletype::PEM)?; } else { + tracing::info!("Creating self-signed certificate"); + + // create a self-signed certificate if needed and store it for later use let (cert, key) = agama_server::cert::create_certificate()?; + agama_server::cert::write_certificate(cert.clone(), key.clone()); + tls_builder.set_private_key(&key)?; tls_builder.set_certificate(&cert)?; } diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index d1e00550d0..ba86705e8f 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -1,9 +1,11 @@ +use anyhow; 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 std::{ fs, path::Path }; use openssl::x509::extension::{BasicConstraints, SubjectAlternativeName, SubjectKeyIdentifier}; use openssl::x509::{X509NameBuilder, X509}; @@ -24,6 +26,15 @@ use openssl::x509::{X509NameBuilder, X509}; // pub write(...) // } +const DEFAULT_CERT_FILE: &str = "/run/agama/certificate.pem"; +const DEFAULT_KEY_FILE: &str = "/run/agama/key.pem"; + +pub fn write_certificate(cert: X509, key: PKey) { + if let Ok(bytes) = cert.to_pem() { + fs::write(Path::new(DEFAULT_CERT_FILE), bytes); + } +} + /// 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), ErrorStack> { From 0b5b27e11b89a6aac4a413ac062571ffbd210a5e Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Fri, 17 May 2024 10:43:31 +0200 Subject: [PATCH 02/17] Adding hostname into certificate --- rust/Cargo.lock | 11 +++++++++++ rust/agama-server/Cargo.toml | 1 + rust/agama-server/src/cert.rs | 9 +++++++-- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index a16bc51ca7..a506b415f3 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -110,6 +110,7 @@ dependencies = [ "clap", "config", "futures-util", + "gethostname", "gettext-rs", "http-body-util", "hyper 1.2.0", @@ -1384,6 +1385,16 @@ dependencies = [ "version_check", ] +[[package]] +name = "gethostname" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0176e0459c2e4a1fe232f984bca6890e681076abb9934f6cea7c326f3fc47818" +dependencies = [ + "libc", + "windows-targets 0.48.5", +] + [[package]] name = "getrandom" version = "0.2.12" diff --git a/rust/agama-server/Cargo.toml b/rust/agama-server/Cargo.toml index f39e0766d3..52386a3005 100644 --- a/rust/agama-server/Cargo.toml +++ b/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" diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index ba86705e8f..0e0287251c 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -1,4 +1,5 @@ use anyhow; +use gethostname::gethostname; use openssl::asn1::Asn1Time; use openssl::bn::{BigNum, MsbOption}; use openssl::error::ErrorStack; @@ -26,13 +27,16 @@ use openssl::x509::{X509NameBuilder, X509}; // pub write(...) // } -const DEFAULT_CERT_FILE: &str = "/run/agama/certificate.pem"; +const DEFAULT_CERT_FILE: &str = "/run/agama/cert.pem"; const DEFAULT_KEY_FILE: &str = "/run/agama/key.pem"; pub fn write_certificate(cert: X509, key: PKey) { if let Ok(bytes) = cert.to_pem() { fs::write(Path::new(DEFAULT_CERT_FILE), bytes); } + if let Ok(bytes) = key.public_key_to_pem() { + fs::write(Path::new(DEFAULT_KEY_FILE), bytes); + } } /// Generates a self-signed SSL certificate @@ -41,9 +45,10 @@ pub fn create_certificate() -> Result<(X509, PKey), ErrorStack> { let rsa = Rsa::generate(2048)?; let key = PKey::from_rsa(rsa)?; + let hostname = gethostname().into_string().map_err(|e| openssl::ssl::into_io_error()?)?; let mut x509_name = X509NameBuilder::new()?; x509_name.append_entry_by_text("O", "Agama")?; - x509_name.append_entry_by_text("CN", "localhost")?; + x509_name.append_entry_by_text("CN", hostname)?; let x509_name = x509_name.build(); let mut builder = X509::builder()?; From a36daa65caba26769c5e3a9f7d4399ac8a5b3cb4 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Fri, 17 May 2024 10:05:50 +0200 Subject: [PATCH 03/17] Some documentation --- rust/agama-server/src/cert.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 0e0287251c..1217787d7c 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -30,6 +30,7 @@ use openssl::x509::{X509NameBuilder, X509}; const DEFAULT_CERT_FILE: &str = "/run/agama/cert.pem"; const DEFAULT_KEY_FILE: &str = "/run/agama/key.pem"; +/// Writes the certificate and the key to the well known location pub fn write_certificate(cert: X509, key: PKey) { if let Ok(bytes) = cert.to_pem() { fs::write(Path::new(DEFAULT_CERT_FILE), bytes); @@ -45,10 +46,10 @@ pub fn create_certificate() -> Result<(X509, PKey), ErrorStack> { let rsa = Rsa::generate(2048)?; let key = PKey::from_rsa(rsa)?; - let hostname = gethostname().into_string().map_err(|e| openssl::ssl::into_io_error()?)?; + 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)?; + x509_name.append_entry_by_text("CN", hostname.as_str())?; let x509_name = x509_name.build(); let mut builder = X509::builder()?; From 0d596694108b4b825a66fbc3dce4b4c4540408bd Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Fri, 17 May 2024 10:14:37 +0200 Subject: [PATCH 04/17] Minor refactoring, turned write_certificate into function --- rust/agama-server/src/agama-web-server.rs | 3 ++- rust/agama-server/src/cert.rs | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 5e1c5726da..1d252be02c 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -114,7 +114,8 @@ impl ServeArgs { // create a self-signed certificate if needed and store it for later use let (cert, key) = agama_server::cert::create_certificate()?; - agama_server::cert::write_certificate(cert.clone(), key.clone()); + // tries to write generated self-signed certificate. Nobody cares if it fails + let _ = agama_server::cert::write_certificate(cert.clone(), key.clone()); tls_builder.set_private_key(&key)?; tls_builder.set_certificate(&cert)?; diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 1217787d7c..17e2d2e8c6 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -31,13 +31,15 @@ const DEFAULT_CERT_FILE: &str = "/run/agama/cert.pem"; const DEFAULT_KEY_FILE: &str = "/run/agama/key.pem"; /// Writes the certificate and the key to the well known location -pub fn write_certificate(cert: X509, key: PKey) { +pub fn write_certificate(cert: X509, key: PKey) -> anyhow::Result<()> { if let Ok(bytes) = cert.to_pem() { - fs::write(Path::new(DEFAULT_CERT_FILE), bytes); + fs::write(Path::new(DEFAULT_CERT_FILE), bytes)?; } if let Ok(bytes) = key.public_key_to_pem() { - fs::write(Path::new(DEFAULT_KEY_FILE), bytes); + fs::write(Path::new(DEFAULT_KEY_FILE), bytes)?; } + + Ok(()) } /// Generates a self-signed SSL certificate From fbd5a737714482e6c25afb8dc11f3602a5164673 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 21 May 2024 10:16:46 +0200 Subject: [PATCH 05/17] Modified and improved default way how to store self generated cert --- rust/agama-server/src/cert.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 17e2d2e8c6..e5ad924bd0 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -27,16 +27,20 @@ use openssl::x509::{X509NameBuilder, X509}; // pub write(...) // } -const DEFAULT_CERT_FILE: &str = "/run/agama/cert.pem"; -const DEFAULT_KEY_FILE: &str = "/run/agama/key.pem"; +const DEFAULT_CERT_DIR: &str = "/run/agama/ssl"; /// Writes the certificate and the key to the well known location pub fn write_certificate(cert: X509, key: PKey) -> 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) = cert.to_pem() { - fs::write(Path::new(DEFAULT_CERT_FILE), bytes)?; + fs::write(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), bytes)?; } if let Ok(bytes) = key.public_key_to_pem() { - fs::write(Path::new(DEFAULT_KEY_FILE), bytes)?; + fs::write(Path::new(DEFAULT_CERT_DIR).join("key.pem"), bytes)?; } Ok(()) From ab6318913d6f0b847c2f58229e91dfef3547763f Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 21 May 2024 10:51:56 +0200 Subject: [PATCH 06/17] Modified default value of cert source --- rust/agama-server/src/agama-web-server.rs | 13 ++++++++----- rust/agama-server/src/cert.rs | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 1d252be02c..363d2bc2b8 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -79,12 +79,15 @@ struct ServeArgs { #[arg(long, default_value = None)] address2: Option, - /// Path to the SSL private key file in PEM format - #[arg(long, default_value = None)] + #[arg( + long, + default_value = "/run/agama/ssl/key.pem", + )] key: Option, - - /// Path to the SSL certificate file in PEM format - #[arg(long, default_value = None)] + #[arg( + long, + default_value = "/run/agama/ssl/cert.pem", + )] cert: Option, /// The D-Bus address for connecting to the Agama service diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index e5ad924bd0..d5b67243f9 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -27,7 +27,7 @@ use openssl::x509::{X509NameBuilder, X509}; // pub write(...) // } -const DEFAULT_CERT_DIR: &str = "/run/agama/ssl"; +pub const DEFAULT_CERT_DIR: &str = "/run/agama/ssl"; /// Writes the certificate and the key to the well known location pub fn write_certificate(cert: X509, key: PKey) -> anyhow::Result<()> { From 21db741e3000fd690ff6a19905a60c5fd4026cae Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Wed, 22 May 2024 08:13:17 +0200 Subject: [PATCH 07/17] Changed default location where to search for certificates --- rust/agama-server/src/agama-web-server.rs | 5 +++-- rust/agama-server/src/cert.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 363d2bc2b8..8fd0910239 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -81,12 +81,13 @@ struct ServeArgs { #[arg( long, - default_value = "/run/agama/ssl/key.pem", + default_value = "/etc/agama.d/ssl/key.pem", )] key: Option, + #[arg( long, - default_value = "/run/agama/ssl/cert.pem", + default_value = "/etc/agama.d/ssl/cert.pem", )] cert: Option, diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index d5b67243f9..d8591633f1 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -27,7 +27,7 @@ use openssl::x509::{X509NameBuilder, X509}; // pub write(...) // } -pub const DEFAULT_CERT_DIR: &str = "/run/agama/ssl"; +pub const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl"; /// Writes the certificate and the key to the well known location pub fn write_certificate(cert: X509, key: PKey) -> anyhow::Result<()> { From 6f3e2dde10d730520c376c2d2953f11fdd00e939 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Wed, 22 May 2024 14:23:55 +0200 Subject: [PATCH 08/17] Some more checks on user's input --- rust/agama-server/src/agama-web-server.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 8fd0910239..1b0db3ea31 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -106,12 +106,16 @@ impl ServeArgs { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; // use default or explicitly provided certificate if any - 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)?; + if self.cert.clone().is_some_and(|c| Path::new(&c).exists()) && self.key.clone().is_some_and(|k| Path::new(&k).exists()) { + if let Some(cert) = &self.cert { + tracing::info!("Loading PEM certificate: {}", cert); + tls_builder.set_certificate_file(PathBuf::from(cert), SslFiletype::PEM)?; + } - tracing::info!("Loading PEM key: {}", key); - tls_builder.set_private_key_file(PathBuf::from(key), SslFiletype::PEM)?; + if let Some(key) = &self.key { + tracing::info!("Loading PEM key: {}", key); + tls_builder.set_private_key_file(PathBuf::from(key), SslFiletype::PEM)?; + } } else { tracing::info!("Creating self-signed certificate"); From f82e247b7671bce1ff91422c492b899d51579509 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Wed, 22 May 2024 14:40:33 +0200 Subject: [PATCH 09/17] Cleanup based on the review --- rust/agama-server/src/cert.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index d8591633f1..2ef117b76d 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -6,7 +6,7 @@ use openssl::error::ErrorStack; use openssl::hash::MessageDigest; use openssl::pkey::{PKey, Private}; use openssl::rsa::Rsa; -use std::{ fs, path::Path }; +use std::{fs, path::Path}; use openssl::x509::extension::{BasicConstraints, SubjectAlternativeName, SubjectKeyIdentifier}; use openssl::x509::{X509NameBuilder, X509}; From 271184d1f65b7a0ab8dd138346f3480443ef8ab4 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Thu, 23 May 2024 09:55:50 +0200 Subject: [PATCH 10/17] Updated changelog --- rust/package/agama.changes | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 70ede6fcec..46d0454db2 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Fri Jun 7 05:58:48 UTC 2024 - Michal Filka + +- 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 + ------------------------------------------------------------------- Wed Jun 5 13:53:59 UTC 2024 - José Iván López González From 6336dbeeed5f58bf2b51176a4824a15165d3f704 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 4 Jun 2024 08:43:45 +0200 Subject: [PATCH 11/17] Refactoring. Created Certificate struct and some cleanup --- rust/agama-server/src/agama-web-server.rs | 73 +++++---- rust/agama-server/src/cert.rs | 182 +++++++++++----------- 2 files changed, 135 insertions(+), 120 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 1b0db3ea31..5dd999f4d0 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -21,7 +21,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; @@ -91,8 +91,11 @@ struct ServeArgs { )] cert: Option, - /// The D-Bus address for connecting to the Agama service - #[arg(long, default_value = "unix:path=/run/agama/bus")] + // Agama D-Bus address + #[arg( + long, + default_value = "unix:path=/run/agama/bus", + )] dbus_address: String, // Directory containing the web UI code @@ -101,39 +104,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 { - let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; - - // use default or explicitly provided certificate if any - if self.cert.clone().is_some_and(|c| Path::new(&c).exists()) && self.key.clone().is_some_and(|k| Path::new(&k).exists()) { - if let Some(cert) = &self.cert { - tracing::info!("Loading PEM certificate: {}", cert); - tls_builder.set_certificate_file(PathBuf::from(cert), SslFiletype::PEM)?; - } + /// Returns true of given path to certificate points to an existing file + fn valid_cert_path(&self) -> bool { + self.cert.clone().is_some_and(|c| Path::new(&c).exists()) + } - if let Some(key) = &self.key { - tracing::info!("Loading PEM key: {}", key); - tls_builder.set_private_key_file(PathBuf::from(key), SslFiletype::PEM)?; - } - } else { - tracing::info!("Creating self-signed certificate"); + /// Returns true of given path to key points to an existing file + fn valid_key_path(&self) -> bool { + self.key.clone().is_some_and(|k| Path::new(&k).exists()) + } - // create a self-signed certificate if needed and store it for later use - let (cert, key) = agama_server::cert::create_certificate()?; + /// Takes options provided by user and loads / creates Certificate struct according to them + fn to_certificate(&self) -> anyhow::Result { + return if self.valid_cert_path() && self.valid_key_path() { + let cert = self.cert.clone().unwrap(); + let key = self.key.clone().unwrap(); - // tries to write generated self-signed certificate. Nobody cares if it fails - let _ = agama_server::cert::write_certificate(cert.clone(), key.clone()); + // read the provided certificate + agama_server::cert::Certificate::read(cert.as_path(), key.as_path()) + } else { + // ask for self-signed certificate + let certificate = agama_server::cert::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: &agama_server::cert::Certificate) -> Result { + 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 @@ -320,7 +333,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")); diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 2ef117b76d..e682fdc027 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -2,7 +2,6 @@ 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; @@ -10,98 +9,101 @@ use std::{fs, path::Path}; use openssl::x509::extension::{BasicConstraints, SubjectAlternativeName, SubjectKeyIdentifier}; use openssl::x509::{X509NameBuilder, X509}; -// TODO: move the certificate related functions into a struct -// -// struct Certificate { -// certificate: X509, -// key: PKey, -// } -// -// impl Certificate { -// // read from file, support some default location -// // (like /etc/agama.d/ssl/{certificate,key}.pem ?) -// pub read(cert: &str, key: &str) -> Result; -// // generate a self-signed certificate -// pub new() -> Self -// // dump to file -// pub write(...) -// } - -pub const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl"; - -/// Writes the certificate and the key to the well known location -pub fn write_certificate(cert: X509, key: PKey) -> 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)?; - } +const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl"; + +pub struct Certificate { + pub cert: X509, + pub key: PKey, +} + +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() { + fs::write(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), bytes)?; + } + if let Ok(bytes) = self.key.private_key_to_pem_pkcs8() { + fs::write(Path::new(DEFAULT_CERT_DIR).join("key.pem"), bytes)?; + } - if let Ok(bytes) = cert.to_pem() { - fs::write(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), bytes)?; + Ok(()) } - if let Ok(bytes) = key.public_key_to_pem() { - fs::write(Path::new(DEFAULT_CERT_DIR).join("key.pem"), bytes)?; + + /// Reads cert from given path + pub fn read(cert: &Path, key: &Path) -> anyhow::Result { + 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()); + + if let (Ok(c),Ok(k)) = (cert, key) { + Ok(Certificate { + cert: c, + key: k, + }) + } else { + Err(anyhow::anyhow!("Failed to read certificate")) + } } - Ok(()) -} + /// Creates a self-signed certificate + pub fn new() -> anyhow::Result { + let rsa = Rsa::generate(2048)?; + let key = PKey::from_rsa(rsa)?; -/// 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), ErrorStack> { - 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(¬_before)?; - let not_after = Asn1Time::days_from_now(365)?; - builder.set_not_after(¬_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)) + 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(¬_before)?; + let not_after = Asn1Time::days_from_now(365)?; + builder.set_not_after(¬_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 + }) + } } From 22600a7af38f7a6f5c5af2862b3f0832de26f1d2 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 4 Jun 2024 09:04:44 +0200 Subject: [PATCH 12/17] Fixed what was missed in rebase on master --- rust/agama-server/src/agama-web-server.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 5dd999f4d0..73e6001138 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -83,13 +83,13 @@ struct ServeArgs { long, default_value = "/etc/agama.d/ssl/key.pem", )] - key: Option, + key: Option, #[arg( long, default_value = "/etc/agama.d/ssl/cert.pem", )] - cert: Option, + cert: Option, // Agama D-Bus address #[arg( From 70a2a390df370cc92797056e4d690b50d9b349d3 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 4 Jun 2024 09:21:14 +0200 Subject: [PATCH 13/17] Fixed formatting --- rust/agama-server/src/agama-web-server.rs | 21 +++++++-------------- rust/agama-server/src/cert.rs | 17 ++++++++--------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 73e6001138..b543a6308f 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -79,23 +79,14 @@ struct ServeArgs { #[arg(long, default_value = None)] address2: Option, - #[arg( - long, - default_value = "/etc/agama.d/ssl/key.pem", - )] + #[arg(long, default_value = "/etc/agama.d/ssl/key.pem")] key: Option, - #[arg( - long, - default_value = "/etc/agama.d/ssl/cert.pem", - )] + #[arg(long, default_value = "/etc/agama.d/ssl/cert.pem")] cert: Option, // Agama D-Bus address - #[arg( - long, - default_value = "unix:path=/run/agama/bus", - )] + #[arg(long, default_value = "unix:path=/run/agama/bus")] dbus_address: String, // Directory containing the web UI code @@ -132,12 +123,14 @@ impl ServeArgs { let _ = certificate.write(); Ok(certificate) - } + }; } } /// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one -fn ssl_acceptor(certificate: &agama_server::cert::Certificate) -> Result { +fn ssl_acceptor( + certificate: &agama_server::cert::Certificate, +) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; tls_builder.set_private_key(&certificate.key)?; diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index e682fdc027..59cb5e5862 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -5,9 +5,9 @@ use openssl::bn::{BigNum, MsbOption}; use openssl::hash::MessageDigest; use openssl::pkey::{PKey, Private}; use openssl::rsa::Rsa; -use std::{fs, path::Path}; use openssl::x509::extension::{BasicConstraints, SubjectAlternativeName, SubjectKeyIdentifier}; use openssl::x509::{X509NameBuilder, X509}; +use std::{fs, path::Path}; const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl"; @@ -20,7 +20,7 @@ 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() { + if !Path::new(DEFAULT_CERT_DIR).is_dir() { std::fs::create_dir_all(DEFAULT_CERT_DIR)?; } @@ -42,11 +42,8 @@ impl Certificate { let cert = X509::from_pem(&cert_bytes.as_slice()); let key = PKey::private_key_from_pem(&key_bytes.as_slice()); - if let (Ok(c),Ok(k)) = (cert, key) { - Ok(Certificate { - cert: c, - key: k, - }) + if let (Ok(c), Ok(k)) = (cert, key) { + Ok(Certificate { cert: c, key: k }) } else { Err(anyhow::anyhow!("Failed to read certificate")) } @@ -57,7 +54,9 @@ impl Certificate { let rsa = Rsa::generate(2048)?; let key = PKey::from_rsa(rsa)?; - let hostname = gethostname().into_string().unwrap_or(String::from("localhost")); + 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())?; @@ -103,7 +102,7 @@ impl Certificate { Ok(Certificate { cert: cert, - key: key + key: key, }) } } From d70a618ae1c37ea976aad09a4f0e18f3c2381a6e Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Wed, 5 Jun 2024 10:21:14 +0200 Subject: [PATCH 14/17] Tweaks based on the review --- rust/agama-server/src/agama-web-server.rs | 17 +++++++++-------- rust/package/agama.changes | 1 + 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index b543a6308f..de9de25ca0 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/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}, @@ -97,25 +98,25 @@ struct ServeArgs { impl ServeArgs { /// Returns true of given path to certificate points to an existing file fn valid_cert_path(&self) -> bool { - self.cert.clone().is_some_and(|c| Path::new(&c).exists()) + self.cert.as_ref().is_some_and(|c| Path::new(&c).exists()) } /// Returns true of given path to key points to an existing file fn valid_key_path(&self) -> bool { - self.key.clone().is_some_and(|k| Path::new(&k).exists()) + 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 { - return if self.valid_cert_path() && self.valid_key_path() { + fn to_certificate(&self) -> anyhow::Result { + if self.valid_cert_path() && self.valid_key_path() { let cert = self.cert.clone().unwrap(); let key = self.key.clone().unwrap(); // read the provided certificate - agama_server::cert::Certificate::read(cert.as_path(), key.as_path()) + Certificate::read(cert.as_path(), key.as_path()) } else { // ask for self-signed certificate - let certificate = agama_server::cert::Certificate::new()?; + 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 @@ -123,13 +124,13 @@ impl ServeArgs { let _ = certificate.write(); Ok(certificate) - }; + } } } /// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one fn ssl_acceptor( - certificate: &agama_server::cert::Certificate, + certificate: &Certificate, ) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 46d0454db2..a488c5da83 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -6,6 +6,7 @@ Fri Jun 7 05:58:48 UTC 2024 - Michal Filka - 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 From 3e5fceaf06953817bbd9f347990ee57d10561c26 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Thu, 6 Jun 2024 10:51:11 +0200 Subject: [PATCH 15/17] More changes according to the review, doc, cleanup --- rust/agama-server/src/agama-web-server.rs | 4 +--- rust/agama-server/src/cert.rs | 13 +++++++------ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index de9de25ca0..607a3dd572 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -129,9 +129,7 @@ impl ServeArgs { } /// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one -fn ssl_acceptor( - certificate: &Certificate, -) -> Result { +fn ssl_acceptor(certificate: &Certificate) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; tls_builder.set_private_key(&certificate.key)?; diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 59cb5e5862..380f2d0f56 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -11,6 +11,8 @@ use std::{fs, path::Path}; 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, @@ -34,18 +36,17 @@ impl Certificate { Ok(()) } - /// Reads cert from given path - pub fn read(cert: &Path, key: &Path) -> anyhow::Result { + /// Reads certificate and corresponding private key from given paths + pub fn read>(cert: T, key: T) -> anyhow::Result { 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()); - if let (Ok(c), Ok(k)) = (cert, key) { - Ok(Certificate { cert: c, key: k }) - } else { - Err(anyhow::anyhow!("Failed to read certificate")) + match (cert, key) { + (Ok(c), Ok(k)) => Ok(Certificate { cert: c, key: k }), + _ => Err(anyhow::anyhow!("Failed to read certificate")), } } From 39858c46c84e7b3a7ec2933d287d6d18189a854b Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Thu, 6 Jun 2024 06:40:36 +0200 Subject: [PATCH 16/17] Restrict permissions for the key file. --- rust/agama-server/src/cert.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 380f2d0f56..15b7d68669 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -7,7 +7,7 @@ use openssl::pkey::{PKey, Private}; use openssl::rsa::Rsa; use openssl::x509::extension::{BasicConstraints, SubjectAlternativeName, SubjectKeyIdentifier}; use openssl::x509::{X509NameBuilder, X509}; -use std::{fs, path::Path}; +use std::{fs, io::Write, os::unix::fs::OpenOptionsExt, path::Path}; const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl"; @@ -30,7 +30,14 @@ impl Certificate { fs::write(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), bytes)?; } if let Ok(bytes) = self.key.private_key_to_pem_pkcs8() { - fs::write(Path::new(DEFAULT_CERT_DIR).join("key.pem"), bytes)?; + let path = Path::new(DEFAULT_CERT_DIR).join("key.pem"); + let mut file = fs::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .mode(0o400) + .open(path)?; + file.write_all(&bytes)?; } Ok(()) From f40236cda61fb09ab5a5bb73a8721954d9e8baa2 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Thu, 6 Jun 2024 08:33:01 +0200 Subject: [PATCH 17/17] Refactoring code for file write and setting restricted permissions. --- rust/agama-server/src/cert.rs | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 15b7d68669..d928382981 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -7,7 +7,12 @@ 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::Write, os::unix::fs::OpenOptionsExt, path::Path}; +use std::{ + fs, + io::{self, Write}, + os::unix::fs::OpenOptionsExt, + path::Path, +}; const DEFAULT_CERT_DIR: &str = "/etc/agama.d/ssl"; @@ -27,17 +32,10 @@ impl Certificate { } if let Ok(bytes) = self.cert.to_pem() { - fs::write(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), bytes)?; + write_and_restrict(Path::new(DEFAULT_CERT_DIR).join("cert.pem"), &bytes)?; } if let Ok(bytes) = self.key.private_key_to_pem_pkcs8() { - let path = Path::new(DEFAULT_CERT_DIR).join("key.pem"); - let mut file = fs::OpenOptions::new() - .create(true) - .truncate(true) - .write(true) - .mode(0o400) - .open(path)?; - file.write_all(&bytes)?; + write_and_restrict(Path::new(DEFAULT_CERT_DIR).join("key.pem"), &bytes)?; } Ok(()) @@ -114,3 +112,17 @@ impl Certificate { }) } } + +/// Writes buf into a file at path and sets the file permissions for the root only access +fn write_and_restrict>(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(()) +}