From fdf6662391f87cda803be3c1787a31bbbcbae98b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Thu, 29 Feb 2024 11:26:46 +0100 Subject: [PATCH 01/24] Web server - support SSL (HTTPS) communication - Use either the cerfificate specified via command line arguments or generate a self-signed certificate - Redirect external HTTP requests to HTTPS - Allow HTTP for internal connections (http://localhost) - Optionally listen on a secondary address (to allow listening on both HTTP/80 and HTTPS/433 ports) --- rust/Cargo.lock | 91 ++++++-- rust/agama-server/Cargo.toml | 5 + rust/agama-server/src/agama-web-server.rs | 253 ++++++++++++++++++++-- rust/agama-server/src/cert.rs | 63 ++++++ rust/agama-server/src/lib.rs | 1 + rust/package/agama.changes | 13 ++ 6 files changed, 397 insertions(+), 29 deletions(-) create mode 100644 rust/agama-server/src/cert.rs diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 8bef766f82..c6ebe6c0ff 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -55,12 +55,16 @@ dependencies = [ "cidr", "clap", "config", + "futures-util", "gettext-rs", "http-body-util", + "hyper", + "hyper-util", "jsonwebtoken", "log", "macaddr", "once_cell", + "openssl", "pam", "rand", "regex", @@ -72,6 +76,7 @@ dependencies = [ "systemd-journal-logger", "thiserror", "tokio", + "tokio-openssl", "tokio-stream", "tower", "tower-http", @@ -1131,6 +1136,21 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foreign-types" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" +dependencies = [ + "foreign-types-shared", +] + +[[package]] +name = "foreign-types-shared" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" + [[package]] name = "form_urlencoded" version = "1.2.1" @@ -1167,9 +1187,9 @@ dependencies = [ [[package]] name = "futures-core" -version = "0.3.29" +version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb1d22c66e66d9d72e1758f0bd7d4fd0bee04cad842ee34587d68c07e45d088c" +checksum = "dfc6580bb841c5a68e9ef15c77ccc837b40a7504914d52e47b8b0e9bbda25a1d" [[package]] name = "futures-io" @@ -1204,9 +1224,9 @@ dependencies = [ [[package]] name = "futures-macro" -version = "0.3.29" +version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53b153fd91e4b0147f4aced87be237c98248656bb01050b96bf3ee89220a8ddb" +checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", @@ -1215,21 +1235,21 @@ dependencies = [ [[package]] name = "futures-sink" -version = "0.3.29" +version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e36d3378ee38c2a36ad710c5d30c2911d752cb941c00c72dbabfb786a7970817" +checksum = "9fb8e00e87438d937621c1c6269e53f536c14d3fbd6a042bb24879e57d474fb5" [[package]] name = "futures-task" -version = "0.3.29" +version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "efd193069b0ddadc69c46389b740bbccdd97203899b48d09c5f7969591d6bae2" +checksum = "38d84fa142264698cdce1a9f9172cf383a0c82de1bddcf3092901442c4097004" [[package]] name = "futures-util" -version = "0.3.29" +version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a19526d624e703a3179b3d322efec918b6246ea0fa51d41124525f00f1cc8104" +checksum = "3d6401deb83407ab3da39eba7e33987a73c3df0c82b4bb5813ee871c19c41d48" dependencies = [ "futures-core", "futures-macro", @@ -1431,9 +1451,9 @@ checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" [[package]] name = "hyper" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb5aa53871fc917b1a9ed87b683a5d86db645e23acb32c2e0785a353e522fb75" +checksum = "186548d73ac615b32a73aafe38fb4f56c0d340e110e5a200bcadbaf2e199263a" dependencies = [ "bytes", "futures-channel", @@ -1445,6 +1465,7 @@ dependencies = [ "httpdate", "itoa", "pin-project-lite", + "smallvec", "tokio", ] @@ -2013,6 +2034,32 @@ version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" +[[package]] +name = "openssl" +version = "0.10.64" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95a0481286a310808298130d22dd1fef0fa571e05a8f44ec801801e84b216b1f" +dependencies = [ + "bitflags 2.4.1", + "cfg-if", + "foreign-types", + "libc", + "once_cell", + "openssl-macros", + "openssl-sys", +] + +[[package]] +name = "openssl-macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.48", +] + [[package]] name = "openssl-probe" version = "0.1.5" @@ -2021,9 +2068,9 @@ checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" [[package]] name = "openssl-sys" -version = "0.9.97" +version = "0.9.101" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3eaad34cdd97d81de97964fc7f29e2d104f483840d906ef56daa1912338460b" +checksum = "dda2b0f344e78efc2facf7d195d098df0dd72151b26ab98da807afc26c198dff" dependencies = [ "cc", "libc", @@ -2765,9 +2812,9 @@ dependencies = [ [[package]] name = "smallvec" -version = "1.11.2" +version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4dccd0940a2dcdf68d092b8cbab7dc0ad8fa938bf95787e1b916b0e3d0e8e970" +checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7" [[package]] name = "socket2" @@ -3010,6 +3057,18 @@ dependencies = [ "syn 2.0.48", ] +[[package]] +name = "tokio-openssl" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ffab79df67727f6acf57f1ff743091873c24c579b1e2ce4d8f53e47ded4d63d" +dependencies = [ + "futures-util", + "openssl", + "openssl-sys", + "tokio", +] + [[package]] name = "tokio-stream" version = "0.1.14" diff --git a/rust/agama-server/Cargo.toml b/rust/agama-server/Cargo.toml index 25cc8ccba5..cf379e9952 100644 --- a/rust/agama-server/Cargo.toml +++ b/rust/agama-server/Cargo.toml @@ -48,6 +48,11 @@ chrono = { version = "0.4.34", default-features = false, features = [ ] } pam = "0.8.0" serde_with = "3.6.1" +openssl = "0.10.64" +hyper = "1.2.0" +hyper-util = "0.1.3" +tokio-openssl = "0.6.4" +futures-util = { version = "0.3.30", default-features = false, features = ["alloc"] } [[bin]] name = "agama-dbus-server" diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index d8a16fc565..907257a658 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -1,11 +1,22 @@ -use std::process::{ExitCode, Termination}; - +use std::{path::PathBuf, pin::Pin}; +// use agama_lib::connection; use agama_dbus_server::{ l10n::helpers, web::{self, run_monitor}, }; +use axum::{ + http::{Request, Response}, + Router, +}; use clap::{Parser, Subcommand}; +use futures_util::pin_mut; +use hyper::body::Incoming; +use hyper_util::rt::{TokioExecutor, TokioIo}; +use openssl::ssl::{Ssl, SslAcceptor, SslFiletype, SslMethod}; +use std::process::{ExitCode, Termination}; use tokio::sync::broadcast::channel; +use tokio_openssl::SslStream; +use tower::Service; use tracing_subscriber::prelude::*; use utoipa::OpenApi; @@ -13,10 +24,28 @@ use utoipa::OpenApi; enum Commands { /// Start the API server. Serve { - // Address to listen on (":::3000" listens for both IPv6 and IPv4 + // Address to listen to (":::3000" listens for both IPv6 and IPv4 // connections unless manually disabled in /proc/sys/net/ipv6/bindv6only) - #[arg(long, default_value = ":::3000")] + #[arg(long, default_value = ":::3000", help = "Primary address to listen on")] address: String, + #[arg( + long, + default_value = "", + help = "Optional secondary address to listen to" + )] + address2: String, + #[arg( + long, + default_value = "", + help = "Path to the SSL private key file in PEM format" + )] + key: String, + #[arg( + long, + default_value = "", + help = "Path to the SSL certificate file in PEM format" + )] + cert: String, }, /// Display the API documentation in OpenAPI format. Openapi, @@ -32,23 +61,216 @@ struct Cli { pub command: Commands, } +// check whether the connection uses SSL or not +async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { + // a buffer for reading the first byte from the TCP connection + let mut buf = [0u8; 1]; + + // peek() receives the data without removing it from the stream, + // the data is not consumed, it will be read from the stream again later + stream.peek(&mut buf).await.unwrap(); + + // SSL3.0/TLS1.x starts with byte 0x16 + // SSL2 starts with 0x80 (but should not be used as it is considered) + // see https://stackoverflow.com/q/3897883 + // otherwise consider the stream as a plain HTTP stream possibly starting with + // "GET ... HTTP/1.1" or "POST ... HTTP/1.1" or a similar line + buf[0] == 0x16u8 || buf[0] == 0x80u8 +} + +// build a SSL acceptor using a provided SSL certificate or generate a self-signed one +fn create_ssl_acceptor(cert_file: &String, key_file: &String) -> SslAcceptor { + let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server()).unwrap(); + + if cert_file.is_empty() && key_file.is_empty() { + let (cert, key) = agama_dbus_server::cert::create_certificate().unwrap(); + tracing::info!("Generated self signed certificate: {:#?}", cert); + tls_builder.set_private_key(key.as_ref()).unwrap(); + tls_builder.set_certificate(cert.as_ref()).unwrap(); + + // 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(); + } else { + tracing::info!("Loading PEM certificate: {}", cert_file); + tls_builder + .set_certificate_file(PathBuf::from(cert_file), SslFiletype::PEM) + .unwrap(); + + tracing::info!("Loading PEM key: {}", key_file); + tls_builder + .set_private_key_file(PathBuf::from(key_file), SslFiletype::PEM) + .unwrap(); + } + + // check that the key belongs to the certificate + tls_builder.check_private_key().unwrap(); + + tls_builder.build() +} + +// build a response for the HTTP -> HTTPS redirection +// returns 308 permanent redirect +fn redirect_https(host: &str, uri: &hyper::Uri) -> Response { + let builder = Response::builder() + // build the redirection target URL + .header("Location", format!("https://{}{}", host, uri)) + .status(hyper::StatusCode::PERMANENT_REDIRECT); + + builder.body(String::from("")).unwrap() +} + +// build an error response for the HTTP -> HTTPS redirection when we cannot build +// the redirect response from the original request +// returns error 400 +fn redirect_error() -> Response { + let builder = Response::builder().status(hyper::StatusCode::BAD_REQUEST); + + let msg = "HTTP protocol is not allowed for external requests, please use HTTPS.\n"; + builder.body(String::from(msg)).unwrap() +} + +// build a router for the HTTP -> HTTPS redirection +// if the redirection URL cannot be built from the original request it returns error 400 +// instead of the redirection +fn https_redirect() -> Router { + // see https://docs.rs/axum/latest/axum/routing/struct.Router.html#example + let redirect_service = tower::service_fn(|req: axum::extract::Request| async move { + let request_host = req.headers().get("host"); + match request_host { + // missing host in the request header, we cannot build the redirection URL, return error 400 + None => return Ok(redirect_error()), + Some(host) => { + let host_string = host.to_str(); + match host_string { + // invalid host value in the request + Err(_) => return Ok(redirect_error()), + Ok(host_str) => return Ok(redirect_https(host_str, req.uri())), + } + } + } + }); + + Router::new() + .route_service( + // the wildcard path below does not match an empty path, we need to match it explicitly + "/", + redirect_service, + ) + .route_service("/*path", redirect_service) +} + +// start the web server +async fn start_server(address: String, service: Router, ssl_acceptor: SslAcceptor) { + tracing::info!("Starting Agama web server at {}", address); + + // see https://github.com/tokio-rs/axum/blob/main/examples/low-level-openssl/src/main.rs + // how to use axum with openSSL + let listener = tokio::net::TcpListener::bind(&address) + .await + .unwrap_or_else(|_| panic!("could not listen on {}", &address)); + + pin_mut!(listener); + + let redirector = https_redirect(); + + loop { + let tower_service = service.clone(); + let redirector_service = redirector.clone(); + let tls_acceptor = ssl_acceptor.clone(); + + // Wait for a new tcp connection + let (tcp_stream, addr) = listener.accept().await.unwrap(); + + tokio::spawn(async move { + if is_ssl_stream(&tcp_stream).await { + // handle HTTPS connection + let ssl = Ssl::new(tls_acceptor.context()).unwrap(); + let mut tls_stream = SslStream::new(ssl, tcp_stream).unwrap(); + if let Err(err) = SslStream::accept(Pin::new(&mut tls_stream)).await { + tracing::error!("Error during TSL handshake from {}: {}", addr, err); + } + + let stream = TokioIo::new(tls_stream); + let hyper_service = + hyper::service::service_fn(move |request: Request| { + tower_service.clone().call(request) + }); + + let ret = hyper_util::server::conn::auto::Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(stream, hyper_service) + .await; + + if let Err(err) = ret { + tracing::error!("Error serving connection from {}: {}", addr, err); + } + } else { + // handle HTTP connection + let stream = TokioIo::new(tcp_stream); + let hyper_service = + hyper::service::service_fn(move |request: Request| { + // check if it is local connection or external + // the to_canonical() converts IPv4-mapped IPv6 addresses + // to plain IPv4, then is_loopback() works correctly for the IPv4 connections + if addr.ip().to_canonical().is_loopback() { + // accept plain HTTP on the local connection + tower_service.clone().call(request) + } else { + // redirect external connections to HTTPS + redirector_service.clone().call(request) + } + }); + + let ret = hyper_util::server::conn::auto::Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(stream, hyper_service) + .await; + + if let Err(err) = ret { + tracing::error!("Error serving connection from {}: {}", addr, err); + } + } + }); + } +} + /// Start serving the API. -async fn serve_command(address: &str) -> anyhow::Result<()> { +async fn serve_command( + address: &String, + address2: &String, + cert: &String, + key: &String, +) -> anyhow::Result<()> { let journald = tracing_journald::layer().expect("could not connect to journald"); tracing_subscriber::registry().with(journald).init(); - let listener = tokio::net::TcpListener::bind(address) - .await - .unwrap_or_else(|_| panic!("could not listen on {}", address)); - let (tx, _) = channel(16); run_monitor(tx.clone()).await?; let config = web::ServiceConfig::load().unwrap(); let service = web::service(config, tx); - axum::serve(listener, service) - .await - .expect("could not mount app on listener"); + let ssl_acceptor = create_ssl_acceptor(cert, key); + + let mut servers = vec![]; + if !address.is_empty() { + servers.push(tokio::spawn(start_server( + address.clone(), + service.clone(), + ssl_acceptor.clone(), + ))); + } + if !address2.is_empty() { + servers.push(tokio::spawn(start_server( + address2.clone(), + service.clone(), + ssl_acceptor.clone(), + ))); + } + + futures_util::future::join_all(servers).await; + Ok(()) } @@ -60,7 +282,12 @@ fn openapi_command() -> anyhow::Result<()> { async fn run_command(cli: Cli) -> anyhow::Result<()> { match cli.command { - Commands::Serve { address } => serve_command(&address).await, + Commands::Serve { + address, + address2, + key, + cert, + } => serve_command(&address, &address2, &cert, &key).await, Commands::Openapi => openapi_command(), } } diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs new file mode 100644 index 0000000000..f7a3ba4c21 --- /dev/null +++ b/rust/agama-server/src/cert.rs @@ -0,0 +1,63 @@ +use openssl::error::ErrorStack; +use openssl::pkey::{PKey, Private}; +use openssl::rsa::Rsa; +use openssl::x509::extension::{KeyUsage, SubjectAlternativeName}; +use openssl::x509::{X509NameBuilder, X509}; + +use openssl::asn1::Asn1Time; +use openssl::bn::{BigNum, MsbOption}; +use openssl::hash::MessageDigest; +use openssl::x509::extension::{BasicConstraints, SubjectKeyIdentifier}; + +// Generate 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 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_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( + KeyUsage::new() + .critical() + .key_cert_sign() + .crl_sign() + .build()? + )?; + + builder.append_extension( + SubjectAlternativeName::new() + .dns("agama") + .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((cert, key)) +} diff --git a/rust/agama-server/src/lib.rs b/rust/agama-server/src/lib.rs index 421b9e3efd..c848c85105 100644 --- a/rust/agama-server/src/lib.rs +++ b/rust/agama-server/src/lib.rs @@ -1,4 +1,5 @@ pub mod error; +pub mod cert; pub mod l10n; pub mod network; pub mod questions; diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 656cd074eb..1ec250c748 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,16 @@ +------------------------------------------------------------------- +Thu Feb 29 09:49:18 UTC 2024 - Ladislav Slezák + +- Web server: + - Accept also IPv6 connections + - Added SSL (HTTPS) support + - Use either the cerfificate specified via command line + arguments or generate a self-signed certificate + - Redirect external HTTP requests to HTTPS + - Allow HTTP for internal connections (http://localhost) + - Optionally listen on a secondary address + (to allow listening on both HTTP/80 and HTTPS/433 ports) + ------------------------------------------------------------------- Tue Feb 27 15:55:28 UTC 2024 - Imobach Gonzalez Sosa From 10a6500da4d56565eb95e96ae423a1e4c79f87ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Thu, 29 Feb 2024 13:35:07 +0100 Subject: [PATCH 02/24] Fixes by clippy --- rust/agama-server/src/agama-web-server.rs | 12 ++++++------ rust/agama-server/tests/common/mod.rs | 6 ++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 907257a658..86632f8f0b 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -142,12 +142,12 @@ fn https_redirect() -> Router { let request_host = req.headers().get("host"); match request_host { // missing host in the request header, we cannot build the redirection URL, return error 400 - None => return Ok(redirect_error()), + None => Ok(redirect_error()), Some(host) => { let host_string = host.to_str(); match host_string { // invalid host value in the request - Err(_) => return Ok(redirect_error()), + Err(_) => Ok(redirect_error()), Ok(host_str) => return Ok(redirect_https(host_str, req.uri())), } } @@ -238,8 +238,8 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto /// Start serving the API. async fn serve_command( - address: &String, - address2: &String, + address: &str, + address2: &str, cert: &String, key: &String, ) -> anyhow::Result<()> { @@ -256,14 +256,14 @@ async fn serve_command( let mut servers = vec![]; if !address.is_empty() { servers.push(tokio::spawn(start_server( - address.clone(), + address.to_owned(), service.clone(), ssl_acceptor.clone(), ))); } if !address2.is_empty() { servers.push(tokio::spawn(start_server( - address2.clone(), + address2.to_owned(), service.clone(), ssl_acceptor.clone(), ))); diff --git a/rust/agama-server/tests/common/mod.rs b/rust/agama-server/tests/common/mod.rs index cd77539774..4525c96fa4 100644 --- a/rust/agama-server/tests/common/mod.rs +++ b/rust/agama-server/tests/common/mod.rs @@ -41,6 +41,12 @@ pub trait ServerState {} impl ServerState for Started {} impl ServerState for Stopped {} +impl Default for DBusServer { + fn default() -> Self { + Self::new() + } +} + impl DBusServer { pub fn new() -> Self { let uuid = Uuid::new_v4(); From 05b1b098a8ef9fae55cf0f7ef3f79094d6366b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Thu, 29 Feb 2024 13:37:25 +0100 Subject: [PATCH 03/24] fmt fixes --- rust/agama-server/src/cert.rs | 10 +++++----- rust/agama-server/src/lib.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index f7a3ba4c21..d2a3eb422f 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -42,14 +42,14 @@ pub fn create_certificate() -> Result<(X509, PKey), ErrorStack> { .critical() .key_cert_sign() .crl_sign() - .build()? + .build()?, )?; builder.append_extension( - SubjectAlternativeName::new() - .dns("agama") - .dns("agama.local") - .build(&builder.x509v3_context(None, None))? + SubjectAlternativeName::new() + .dns("agama") + .dns("agama.local") + .build(&builder.x509v3_context(None, None))?, )?; let subject_key_identifier = diff --git a/rust/agama-server/src/lib.rs b/rust/agama-server/src/lib.rs index c848c85105..da17034be4 100644 --- a/rust/agama-server/src/lib.rs +++ b/rust/agama-server/src/lib.rs @@ -1,5 +1,5 @@ -pub mod error; pub mod cert; +pub mod error; pub mod l10n; pub mod network; pub mod questions; From fb7cd41ab0d5281c11103662bd9fbe3097790887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Thu, 29 Feb 2024 14:04:32 +0100 Subject: [PATCH 04/24] Cleanup --- rust/agama-server/src/agama-web-server.rs | 28 ++++++++++------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 86632f8f0b..6e8665b48e 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -1,5 +1,3 @@ -use std::{path::PathBuf, pin::Pin}; -// use agama_lib::connection; use agama_dbus_server::{ l10n::helpers, web::{self, run_monitor}, @@ -14,6 +12,7 @@ use hyper::body::Incoming; use hyper_util::rt::{TokioExecutor, TokioIo}; use openssl::ssl::{Ssl, SslAcceptor, SslFiletype, SslMethod}; use std::process::{ExitCode, Termination}; +use std::{path::PathBuf, pin::Pin}; use tokio::sync::broadcast::channel; use tokio_openssl::SslStream; use tower::Service; @@ -24,14 +23,14 @@ use utoipa::OpenApi; enum Commands { /// Start the API server. Serve { - // Address to listen to (":::3000" listens for both IPv6 and IPv4 + // Address/port to listen on (":::3000" listens for both IPv6 and IPv4 // connections unless manually disabled in /proc/sys/net/ipv6/bindv6only) #[arg(long, default_value = ":::3000", help = "Primary address to listen on")] address: String, #[arg( long, default_value = "", - help = "Optional secondary address to listen to" + help = "Optional secondary address to listen on" )] address2: String, #[arg( @@ -155,11 +154,8 @@ fn https_redirect() -> Router { }); Router::new() - .route_service( - // the wildcard path below does not match an empty path, we need to match it explicitly - "/", - redirect_service, - ) + // the wildcard path below does not match an empty path, we need to match it explicitly + .route_service("/", redirect_service) .route_service("/*path", redirect_service) } @@ -254,13 +250,13 @@ async fn serve_command( let ssl_acceptor = create_ssl_acceptor(cert, key); let mut servers = vec![]; - if !address.is_empty() { - servers.push(tokio::spawn(start_server( - address.to_owned(), - service.clone(), - ssl_acceptor.clone(), - ))); - } + servers.push(tokio::spawn(start_server( + address.to_owned(), + service.clone(), + ssl_acceptor.clone(), + ))); + + // optionally listen on the secondary address/port if !address2.is_empty() { servers.push(tokio::spawn(start_server( address2.to_owned(), From f64696557dd3d8457cbc373cd417f9697a9dd854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 1 Mar 2024 10:48:57 +0100 Subject: [PATCH 05/24] Documentation update --- rust/WEB-SERVER.md | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/rust/WEB-SERVER.md b/rust/WEB-SERVER.md index 75534d03ee..b92c900596 100644 --- a/rust/WEB-SERVER.md +++ b/rust/WEB-SERVER.md @@ -51,12 +51,26 @@ $ sudo ./target/debug/agama-web-server serve If it fails to compile, please check whether `clang-devel` and `pam-devel` are installed. -You can add a `--listen` flag if you want to use a different port: +By default the server uses port 3000 and listens on all network interfaces. You +can use the `--address` option if you want to use a different port or a specific +network interface: ``` -$ sudo ./target/debug/agama-web-server serve --listen 0.0.0.0:5678 +$ sudo ./target/debug/agama-web-server serve --address :::5678 ``` +Some more examples: + +- Both IPv6 and IPv4, all interfaces: `--address :::5678` +- Both IPv6 and IPv4, only local loopback : `--address ::1:5678` +- IPv4 only, all interfaces: `--address 0.0.0.0:5678` +- IPv4 only, only local loopback : `--address 127.0.0.1:5678` +- IPv4, only specific interface: `--address 192.168.1.2:5678` (use the IP + address of that interface) + +The server can optionally listen on a secondary address, use the `--address2` +option for that. + ## Trying the server You can check whether the server is up and running by just performing a ping: @@ -105,3 +119,28 @@ Now, you can use the following command to connect: $ websocat ws://localhost:3000/ws -H "Authorization: Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MDg1MTA5MzB9.3HmKAC5u4H_FigMqEa9e74OFAq40UldjlaExrOGqE0U" ``` + +## SSL/TLS (HTTPS) Support + +The web server supports encrypted communication using the HTTPS protocol. + +The SSL certificate used by the server can be specified by the `--cert` and +`--key` command line options which should point to the PEM files: + +``` +$ sudo ./target/debug/agama-web-server serve --cert certificate.pem --key key.pem +``` +The certificate is expected in the PEM format, if you have a certificate in +another format you can convert it using the openSSL tools. + +If a SSL certificate is not specified via command line then the server generates +a self-signed certificate. Currently it is only kept in memory and generated +again at each start. + +The HTTPS protocol is required for external connections, the HTTP connections +are automatically redirected to HTTPS. *But it still means that the original +HTTP communication can be intercepted by an attacker, do not rely on this +redirection!* + +For internal connections coming from the same machine (via the +`http://localhost` URL) the unencrypted HTTP communication is allowed. From 0653aaa4fb6d2a29954bb3c85e0535d87fc6d7ac Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Fri, 8 Mar 2024 13:42:15 +0100 Subject: [PATCH 06/24] Turned some unwrap calls into a more reasonable handling --- rust/agama-server/src/agama-web-server.rs | 77 ++++++++++++----------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 6e8665b48e..d205b509bb 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -67,25 +67,25 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { // peek() receives the data without removing it from the stream, // the data is not consumed, it will be read from the stream again later - stream.peek(&mut buf).await.unwrap(); - - // SSL3.0/TLS1.x starts with byte 0x16 - // SSL2 starts with 0x80 (but should not be used as it is considered) - // see https://stackoverflow.com/q/3897883 - // otherwise consider the stream as a plain HTTP stream possibly starting with - // "GET ... HTTP/1.1" or "POST ... HTTP/1.1" or a similar line - buf[0] == 0x16u8 || buf[0] == 0x80u8 + stream.peek(&mut buf) + .await + // SSL3.0/TLS1.x starts with byte 0x16 + // SSL2 starts with 0x80 (but should not be used as it is considered) + // see https://stackoverflow.com/q/3897883 + // otherwise consider the stream as a plain HTTP stream possibly starting with + // "GET ... HTTP/1.1" or "POST ... HTTP/1.1" or a similar line + .is_ok_and(|_| buf[0] == 0x16u8 || buf[0] == 0x80u8) } // build a SSL acceptor using a provided SSL certificate or generate a self-signed one -fn create_ssl_acceptor(cert_file: &String, key_file: &String) -> SslAcceptor { - let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server()).unwrap(); +fn create_ssl_acceptor(cert_file: &String, key_file: &String) -> Result { + let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; if cert_file.is_empty() && key_file.is_empty() { - let (cert, key) = agama_dbus_server::cert::create_certificate().unwrap(); + let (cert, key) = agama_dbus_server::cert::create_certificate()?; tracing::info!("Generated self signed certificate: {:#?}", cert); - tls_builder.set_private_key(key.as_ref()).unwrap(); - tls_builder.set_certificate(cert.as_ref()).unwrap(); + tls_builder.set_private_key(key.as_ref())?; + tls_builder.set_certificate(cert.as_ref())?; // for debugging you might dump the certificate to a file: // use std::io::Write; @@ -96,19 +96,17 @@ fn create_ssl_acceptor(cert_file: &String, key_file: &String) -> SslAcceptor { } else { tracing::info!("Loading PEM certificate: {}", cert_file); tls_builder - .set_certificate_file(PathBuf::from(cert_file), SslFiletype::PEM) - .unwrap(); + .set_certificate_file(PathBuf::from(cert_file), SslFiletype::PEM)?; tracing::info!("Loading PEM key: {}", key_file); tls_builder - .set_private_key_file(PathBuf::from(key_file), SslFiletype::PEM) - .unwrap(); + .set_private_key_file(PathBuf::from(key_file), SslFiletype::PEM)?; } // check that the key belongs to the certificate - tls_builder.check_private_key().unwrap(); + tls_builder.check_private_key()?; - tls_builder.build() + Ok(tls_builder.build()) } // build a response for the HTTP -> HTTPS redirection @@ -119,7 +117,9 @@ fn redirect_https(host: &str, uri: &hyper::Uri) -> Response { .header("Location", format!("https://{}{}", host, uri)) .status(hyper::StatusCode::PERMANENT_REDIRECT); - builder.body(String::from("")).unwrap() + // according to documentation this can fail only if builder was previosly fed with data + // which failed to parse into an internal representation (e.g. invalid header) + builder.body(String::from("")).expect("Failed to create redirection request") } // build an error response for the HTTP -> HTTPS redirection when we cannot build @@ -129,7 +129,9 @@ fn redirect_error() -> Response { let builder = Response::builder().status(hyper::StatusCode::BAD_REQUEST); let msg = "HTTP protocol is not allowed for external requests, please use HTTPS.\n"; - builder.body(String::from(msg)).unwrap() + // according to documentation this can fail only if builder was previosly fed with data + // which failed to parse into an internal representation (e.g. invalid header) + builder.body(String::from(msg)).expect("Failed to create an error response") } // build a router for the HTTP -> HTTPS redirection @@ -138,18 +140,10 @@ fn redirect_error() -> Response { fn https_redirect() -> Router { // see https://docs.rs/axum/latest/axum/routing/struct.Router.html#example let redirect_service = tower::service_fn(|req: axum::extract::Request| async move { - let request_host = req.headers().get("host"); - match request_host { - // missing host in the request header, we cannot build the redirection URL, return error 400 - None => Ok(redirect_error()), - Some(host) => { - let host_string = host.to_str(); - match host_string { - // invalid host value in the request - Err(_) => Ok(redirect_error()), - Ok(host_str) => return Ok(redirect_https(host_str, req.uri())), - } - } + if let Some(host) = req.headers().get("host").and_then(|h| h.to_str().ok()) { + Ok(redirect_https(host, req.uri())) + } else { + Ok(redirect_error()) } }); @@ -178,8 +172,8 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto let redirector_service = redirector.clone(); let tls_acceptor = ssl_acceptor.clone(); - // Wait for a new tcp connection - let (tcp_stream, addr) = listener.accept().await.unwrap(); + // Wait for a new tcp connection; if it fails we cannot do much, so print an error and die + let (tcp_stream, addr) = listener.accept().await.expect("Failed to open port for listening"); tokio::spawn(async move { if is_ssl_stream(&tcp_stream).await { @@ -245,9 +239,16 @@ async fn serve_command( let (tx, _) = channel(16); run_monitor(tx.clone()).await?; - let config = web::ServiceConfig::load().unwrap(); - let service = web::service(config, tx); - let ssl_acceptor = create_ssl_acceptor(cert, key); + let service = if let Ok(config) = web::ServiceConfig::load() { + web::service(config, tx) + } else { + return Err(anyhow::anyhow!("Failed to load the service configuration")) + }; + let ssl_acceptor = if let Ok(ssl_acceptor) = create_ssl_acceptor(cert, key) { + ssl_acceptor + } else { + return Err(anyhow::anyhow!("SSL initialization failed")); + }; let mut servers = vec![]; servers.push(tokio::spawn(start_server( From 7b8c729d3cf4d396ff26bf92c76454305b51f7b6 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Fri, 8 Mar 2024 13:55:22 +0100 Subject: [PATCH 07/24] Formatting --- rust/agama-server/src/agama-web-server.rs | 29 +++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index d205b509bb..ddb57eb1ae 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -67,7 +67,8 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { // peek() receives the data without removing it from the stream, // the data is not consumed, it will be read from the stream again later - stream.peek(&mut buf) + stream + .peek(&mut buf) .await // SSL3.0/TLS1.x starts with byte 0x16 // SSL2 starts with 0x80 (but should not be used as it is considered) @@ -78,7 +79,10 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { } // build a SSL acceptor using a provided SSL certificate or generate a self-signed one -fn create_ssl_acceptor(cert_file: &String, key_file: &String) -> Result { +fn create_ssl_acceptor( + cert_file: &String, + key_file: &String +) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; if cert_file.is_empty() && key_file.is_empty() { @@ -95,12 +99,10 @@ fn create_ssl_acceptor(cert_file: &String, key_file: &String) -> Result Response { // according to documentation this can fail only if builder was previosly fed with data // which failed to parse into an internal representation (e.g. invalid header) - builder.body(String::from("")).expect("Failed to create redirection request") + builder + .body(String::from("")) + .expect("Failed to create redirection request") } // build an error response for the HTTP -> HTTPS redirection when we cannot build @@ -131,7 +135,9 @@ fn redirect_error() -> Response { let msg = "HTTP protocol is not allowed for external requests, please use HTTPS.\n"; // according to documentation this can fail only if builder was previosly fed with data // which failed to parse into an internal representation (e.g. invalid header) - builder.body(String::from(msg)).expect("Failed to create an error response") + builder + .body(String::from(msg)) + .expect("Failed to create an error response") } // build a router for the HTTP -> HTTPS redirection @@ -173,7 +179,10 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto let tls_acceptor = ssl_acceptor.clone(); // Wait for a new tcp connection; if it fails we cannot do much, so print an error and die - let (tcp_stream, addr) = listener.accept().await.expect("Failed to open port for listening"); + let (tcp_stream, addr) = listener + .accept() + .await + .expect("Failed to open port for listening"); tokio::spawn(async move { if is_ssl_stream(&tcp_stream).await { @@ -242,7 +251,7 @@ async fn serve_command( let service = if let Ok(config) = web::ServiceConfig::load() { web::service(config, tx) } else { - return Err(anyhow::anyhow!("Failed to load the service configuration")) + return Err(anyhow::anyhow!("Failed to load the service configuration")); }; let ssl_acceptor = if let Ok(ssl_acceptor) = create_ssl_acceptor(cert, key) { ssl_acceptor From 08b586fbd2375b998afbbeacb4eca35133f3f27c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Wed, 13 Mar 2024 10:43:52 +0100 Subject: [PATCH 08/24] Code review fixes --- rust/agama-server/src/cert.rs | 4 +--- rust/package/agama.changes | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index d2a3eb422f..ecaf1c520f 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -1,13 +1,11 @@ use openssl::error::ErrorStack; use openssl::pkey::{PKey, Private}; use openssl::rsa::Rsa; -use openssl::x509::extension::{KeyUsage, SubjectAlternativeName}; use openssl::x509::{X509NameBuilder, X509}; - use openssl::asn1::Asn1Time; use openssl::bn::{BigNum, MsbOption}; use openssl::hash::MessageDigest; -use openssl::x509::extension::{BasicConstraints, SubjectKeyIdentifier}; +use openssl::x509::extension::{BasicConstraints, SubjectKeyIdentifier, KeyUsage, SubjectAlternativeName}; // Generate a self-signed SSL certificate // see https://github.com/sfackler/rust-openssl/blob/master/openssl/examples/mk_certs.rs diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 1ec250c748..44559ff4e3 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -2,8 +2,8 @@ Thu Feb 29 09:49:18 UTC 2024 - Ladislav Slezák - Web server: - - Accept also IPv6 connections - - Added SSL (HTTPS) support + - Accept also IPv6 connections (gh#openSUSE/agama#1057) + - Added SSL (HTTPS) support (gh#openSUSE/agama#1062) - Use either the cerfificate specified via command line arguments or generate a self-signed certificate - Redirect external HTTP requests to HTTPS From 06345b17cc76e8434c70b728fa757bcd50f3aae5 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 19 Mar 2024 10:18:11 +0100 Subject: [PATCH 09/24] Minor tweaks to documentation --- rust/agama-server/src/agama-web-server.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index ddb57eb1ae..ab25fbb589 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -60,7 +60,7 @@ struct Cli { pub command: Commands, } -// check whether the connection uses SSL or not +/// Checks whether the connection uses SSL or not async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { // a buffer for reading the first byte from the TCP connection let mut buf = [0u8; 1]; @@ -78,7 +78,7 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { .is_ok_and(|_| buf[0] == 0x16u8 || buf[0] == 0x80u8) } -// build a SSL acceptor using a provided SSL certificate or generate a self-signed one +/// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one fn create_ssl_acceptor( cert_file: &String, key_file: &String @@ -111,8 +111,8 @@ fn create_ssl_acceptor( Ok(tls_builder.build()) } -// build a response for the HTTP -> HTTPS redirection -// returns 308 permanent redirect +/// Builds a response for the HTTP -> HTTPS redirection +/// returns (HTTP response status code) 308 permanent redirect fn redirect_https(host: &str, uri: &hyper::Uri) -> Response { let builder = Response::builder() // build the redirection target URL @@ -126,9 +126,9 @@ fn redirect_https(host: &str, uri: &hyper::Uri) -> Response { .expect("Failed to create redirection request") } -// build an error response for the HTTP -> HTTPS redirection when we cannot build -// the redirect response from the original request -// returns error 400 +/// Builds an error response for the HTTP -> HTTPS redirection when we cannot build +/// the redirect response from the original request +/// returns error 400 fn redirect_error() -> Response { let builder = Response::builder().status(hyper::StatusCode::BAD_REQUEST); @@ -140,9 +140,9 @@ fn redirect_error() -> Response { .expect("Failed to create an error response") } -// build a router for the HTTP -> HTTPS redirection -// if the redirection URL cannot be built from the original request it returns error 400 -// instead of the redirection +/// Builds a router for the HTTP -> HTTPS redirection +/// if the redirection URL cannot be built from the original request it returns error 400 +/// instead of the redirection fn https_redirect() -> Router { // see https://docs.rs/axum/latest/axum/routing/struct.Router.html#example let redirect_service = tower::service_fn(|req: axum::extract::Request| async move { @@ -159,7 +159,7 @@ fn https_redirect() -> Router { .route_service("/*path", redirect_service) } -// start the web server +/// Starts the web server async fn start_server(address: String, service: Router, ssl_acceptor: SslAcceptor) { tracing::info!("Starting Agama web server at {}", address); From f959ff953cf940406c0263b9bab9a68fbf6fba20 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 19 Mar 2024 10:34:10 +0100 Subject: [PATCH 10/24] Formatting --- rust/agama-server/src/agama-web-server.rs | 2 +- rust/agama-server/src/cert.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index ab25fbb589..c9b1968a5c 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -81,7 +81,7 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { /// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one fn create_ssl_acceptor( cert_file: &String, - key_file: &String + key_file: &String, ) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index ecaf1c520f..dad1099283 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -1,11 +1,13 @@ +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, SubjectKeyIdentifier, KeyUsage, SubjectAlternativeName +}; use openssl::x509::{X509NameBuilder, X509}; -use openssl::asn1::Asn1Time; -use openssl::bn::{BigNum, MsbOption}; -use openssl::hash::MessageDigest; -use openssl::x509::extension::{BasicConstraints, SubjectKeyIdentifier, KeyUsage, SubjectAlternativeName}; // Generate a self-signed SSL certificate // see https://github.com/sfackler/rust-openssl/blob/master/openssl/examples/mk_certs.rs From c85a5d58e64875ca61b46ca55538752c31e914d8 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 19 Mar 2024 10:54:05 +0100 Subject: [PATCH 11/24] Minor tweaks in documentation and formatting --- rust/agama-server/src/cert.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index dad1099283..6051e5dbc0 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -5,12 +5,12 @@ use openssl::hash::MessageDigest; use openssl::pkey::{PKey, Private}; use openssl::rsa::Rsa; use openssl::x509::extension::{ - BasicConstraints, SubjectKeyIdentifier, KeyUsage, SubjectAlternativeName + BasicConstraints, SubjectKeyIdentifier, KeyUsage, SubjectAlternativeName, }; use openssl::x509::{X509NameBuilder, X509}; -// Generate a self-signed SSL certificate -// see https://github.com/sfackler/rust-openssl/blob/master/openssl/examples/mk_certs.rs +/// 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)?; From a9f9b1d30cc4953b6b0f933c446eefcdcb2d9be9 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Tue, 19 Mar 2024 14:19:37 +0100 Subject: [PATCH 12/24] Use struct for command agama-server serve options --- rust/agama-server/src/agama-web-server.rs | 65 ++++++++++++----------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index c9b1968a5c..91746827b1 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -6,7 +6,7 @@ use axum::{ http::{Request, Response}, Router, }; -use clap::{Parser, Subcommand}; +use clap::{Args, Parser, Subcommand}; use futures_util::pin_mut; use hyper::body::Incoming; use hyper_util::rt::{TokioExecutor, TokioIo}; @@ -22,30 +22,7 @@ use utoipa::OpenApi; #[derive(Subcommand, Debug)] enum Commands { /// Start the API server. - Serve { - // Address/port to listen on (":::3000" listens for both IPv6 and IPv4 - // connections unless manually disabled in /proc/sys/net/ipv6/bindv6only) - #[arg(long, default_value = ":::3000", help = "Primary address to listen on")] - address: String, - #[arg( - long, - default_value = "", - help = "Optional secondary address to listen on" - )] - address2: String, - #[arg( - long, - default_value = "", - help = "Path to the SSL private key file in PEM format" - )] - key: String, - #[arg( - long, - default_value = "", - help = "Path to the SSL certificate file in PEM format" - )] - cert: String, - }, + Serve(ServeArgs), /// Display the API documentation in OpenAPI format. Openapi, } @@ -60,6 +37,32 @@ struct Cli { pub command: Commands, } +#[derive(Args, Debug)] +struct ServeArgs { + // Address/port to listen on (":::3000" listens for both IPv6 and IPv4 + // connections unless manually disabled in /proc/sys/net/ipv6/bindv6only) + #[arg(long, default_value = ":::3000", help = "Primary address to listen on")] + address: String, + #[arg( + long, + default_value = "", + help = "Optional secondary address to listen on" + )] + address2: String, + #[arg( + long, + default_value = "", + help = "Path to the SSL private key file in PEM format" + )] + key: String, + #[arg( + long, + default_value = "", + help = "Path to the SSL certificate file in PEM format" + )] + cert: String, +} + /// Checks whether the connection uses SSL or not async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { // a buffer for reading the first byte from the TCP connection @@ -288,12 +291,12 @@ fn openapi_command() -> anyhow::Result<()> { async fn run_command(cli: Cli) -> anyhow::Result<()> { match cli.command { - Commands::Serve { - address, - address2, - key, - cert, - } => serve_command(&address, &address2, &cert, &key).await, + Commands::Serve(options) => serve_command( + &options.address, + &options.address2, + &options.cert, + &options.key, + ).await, Commands::Openapi => openapi_command(), } } From ba448782a4440b334a982682e7ecd4c2f5bb2785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Wed, 20 Mar 2024 09:23:45 +0100 Subject: [PATCH 13/24] Added comments --- rust/agama-server/src/cert.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 6051e5dbc0..0c3a4a1e27 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -47,7 +47,9 @@ pub fn create_certificate() -> Result<(X509, PKey), ErrorStack> { builder.append_extension( SubjectAlternativeName::new() + // use the default Agama host name .dns("agama") + // use the default name for the mDNS/Avahi .dns("agama.local") .build(&builder.x509v3_context(None, None))?, )?; From 5799151a812c091fc12a9bdbce5fc15402b172b4 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Wed, 20 Mar 2024 10:36:22 +0100 Subject: [PATCH 14/24] Refactoring. Added a bit of OOP. Simplified some repetetive actions --- rust/agama-server/src/agama-web-server.rs | 105 ++++++++++------------ 1 file changed, 45 insertions(+), 60 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 91746827b1..9fb34fe692 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -63,6 +63,40 @@ struct ServeArgs { cert: String, } +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())?; + + if self.cert.is_empty() && self.key.is_empty() { + let (cert, key) = agama_dbus_server::cert::create_certificate()?; + + tracing::info!("Generated self signed certificate: {:#?}", cert); + + tls_builder.set_private_key(&key)?; + tls_builder.set_certificate(&cert)?; + + // 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(); + } else { + tracing::info!("Loading PEM certificate: {}", self.cert); + tls_builder.set_certificate_file(PathBuf::from(self.cert.clone()), SslFiletype::PEM)?; + + tracing::info!("Loading PEM key: {}", self.key); + tls_builder.set_private_key_file(PathBuf::from(self.key.clone()), SslFiletype::PEM)?; + } + + // 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 async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { // a buffer for reading the first byte from the TCP connection @@ -81,39 +115,6 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { .is_ok_and(|_| buf[0] == 0x16u8 || buf[0] == 0x80u8) } -/// Builds an SSL acceptor using a provided SSL certificate or generates a self-signed one -fn create_ssl_acceptor( - cert_file: &String, - key_file: &String, -) -> Result { - let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; - - if cert_file.is_empty() && key_file.is_empty() { - let (cert, key) = agama_dbus_server::cert::create_certificate()?; - tracing::info!("Generated self signed certificate: {:#?}", cert); - tls_builder.set_private_key(key.as_ref())?; - tls_builder.set_certificate(cert.as_ref())?; - - // 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(); - } else { - tracing::info!("Loading PEM certificate: {}", cert_file); - tls_builder.set_certificate_file(PathBuf::from(cert_file), SslFiletype::PEM)?; - - tracing::info!("Loading PEM key: {}", key_file); - tls_builder.set_private_key_file(PathBuf::from(key_file), SslFiletype::PEM)?; - } - - // check that the key belongs to the certificate - tls_builder.check_private_key()?; - - Ok(tls_builder.build()) -} - /// Builds a response for the HTTP -> HTTPS redirection /// returns (HTTP response status code) 308 permanent redirect fn redirect_https(host: &str, uri: &hyper::Uri) -> Response { @@ -239,12 +240,7 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto } /// Start serving the API. -async fn serve_command( - address: &str, - address2: &str, - cert: &String, - key: &String, -) -> anyhow::Result<()> { +async fn serve_command(options: ServeArgs) -> anyhow::Result<()> { let journald = tracing_journald::layer().expect("could not connect to journald"); tracing_subscriber::registry().with(journald).init(); @@ -256,28 +252,22 @@ async fn serve_command( } else { return Err(anyhow::anyhow!("Failed to load the service configuration")); }; - let ssl_acceptor = if let Ok(ssl_acceptor) = create_ssl_acceptor(cert, key) { + let ssl_acceptor = if let Ok(ssl_acceptor) = options.ssl_acceptor() { ssl_acceptor } else { return Err(anyhow::anyhow!("SSL initialization failed")); }; - let mut servers = vec![]; - servers.push(tokio::spawn(start_server( - address.to_owned(), - service.clone(), - ssl_acceptor.clone(), - ))); - - // optionally listen on the secondary address/port - if !address2.is_empty() { - servers.push(tokio::spawn(start_server( - address2.to_owned(), - service.clone(), - ssl_acceptor.clone(), - ))); + let mut addresses = vec![options.address]; + + if !options.address2.is_empty() { + addresses.push(options.address2); } + let servers: Vec<_> = addresses.iter().map(|a| + tokio::spawn(start_server(a.clone(), service.clone(), ssl_acceptor.clone()) + )).collect(); + futures_util::future::join_all(servers).await; Ok(()) @@ -291,12 +281,7 @@ fn openapi_command() -> anyhow::Result<()> { async fn run_command(cli: Cli) -> anyhow::Result<()> { match cli.command { - Commands::Serve(options) => serve_command( - &options.address, - &options.address2, - &options.cert, - &options.key, - ).await, + Commands::Serve(options) => serve_command(options).await, Commands::Openapi => openapi_command(), } } From 52322694f1bada328cb0f2f519ce6ef95080000c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Wed, 20 Mar 2024 10:45:03 +0100 Subject: [PATCH 15/24] Small fixes --- rust/agama-server/src/agama-web-server.rs | 7 ++++--- 1 file 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 9fb34fe692..aa4a9296e9 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -10,6 +10,7 @@ use clap::{Args, Parser, Subcommand}; 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 std::process::{ExitCode, Termination}; use std::{path::PathBuf, pin::Pin}; @@ -108,7 +109,7 @@ async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { .peek(&mut buf) .await // SSL3.0/TLS1.x starts with byte 0x16 - // SSL2 starts with 0x80 (but should not be used as it is considered) + // SSL2 starts with 0x80 (but should not be used as it is considered insecure) // see https://stackoverflow.com/q/3897883 // otherwise consider the stream as a plain HTTP stream possibly starting with // "GET ... HTTP/1.1" or "POST ... HTTP/1.1" or a similar line @@ -203,7 +204,7 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto tower_service.clone().call(request) }); - let ret = hyper_util::server::conn::auto::Builder::new(TokioExecutor::new()) + let ret = Builder::new(TokioExecutor::new()) .serve_connection_with_upgrades(stream, hyper_service) .await; @@ -227,7 +228,7 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto } }); - let ret = hyper_util::server::conn::auto::Builder::new(TokioExecutor::new()) + let ret = Builder::new(TokioExecutor::new()) .serve_connection_with_upgrades(stream, hyper_service) .await; From d3870eb6109e75d3ac59bf7c073439032d8f8fc4 Mon Sep 17 00:00:00 2001 From: Michal Filka Date: Wed, 20 Mar 2024 10:46:23 +0100 Subject: [PATCH 16/24] Formatting --- rust/agama-server/src/agama-web-server.rs | 13 ++++++++++--- rust/agama-server/src/cert.rs | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index aa4a9296e9..4797777185 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -265,9 +265,16 @@ async fn serve_command(options: ServeArgs) -> anyhow::Result<()> { addresses.push(options.address2); } - let servers: Vec<_> = addresses.iter().map(|a| - tokio::spawn(start_server(a.clone(), service.clone(), ssl_acceptor.clone()) - )).collect(); + let servers: Vec<_> = addresses + .iter() + .map(|a| { + tokio::spawn(start_server( + a.clone(), + service.clone(), + ssl_acceptor.clone(), + )) + }) + .collect(); futures_util::future::join_all(servers).await; diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 0c3a4a1e27..22dd492e70 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -5,7 +5,7 @@ use openssl::hash::MessageDigest; use openssl::pkey::{PKey, Private}; use openssl::rsa::Rsa; use openssl::x509::extension::{ - BasicConstraints, SubjectKeyIdentifier, KeyUsage, SubjectAlternativeName, + BasicConstraints, KeyUsage, SubjectAlternativeName, SubjectKeyIdentifier, }; use openssl::x509::{X509NameBuilder, X509}; From a7ba46083bc2f25378fd146151cc22fc093f3395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Thu, 21 Mar 2024 17:32:03 +0100 Subject: [PATCH 17/24] code review fixes --- rust/agama-server/src/agama-web-server.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 908a9b9ccd..5d5731355b 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 { } /// Checks whether the connection uses SSL or not +/// `stream`: the TCP stream containing a request from client async fn is_ssl_stream(stream: &tokio::net::TcpStream) -> bool { // a buffer for reading the first byte from the TCP connection let mut buf = [0u8; 1]; @@ -174,7 +175,12 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto // how to use axum with openSSL let listener = tokio::net::TcpListener::bind(&address) .await - .unwrap_or_else(|_| panic!("could not listen on {}", &address)); + .unwrap_or_else(|error| { + let msg = format!("Error: could not listen on {}: {}", &address, error); + tracing::error!(msg); + panic!("{}", msg) + } + ); pin_mut!(listener); From 3b20f95b4d7fc76040d5da9a8e00cd5cbf9175f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Thu, 21 Mar 2024 18:18:59 +0100 Subject: [PATCH 18/24] Fix formatting --- rust/agama-server/src/agama-web-server.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 5d5731355b..f5eaf5f9fa 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -176,11 +176,10 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto let listener = tokio::net::TcpListener::bind(&address) .await .unwrap_or_else(|error| { - let msg = format!("Error: could not listen on {}: {}", &address, error); - tracing::error!(msg); - panic!("{}", msg) - } - ); + let msg = format!("Error: could not listen on {}: {}", &address, error); + tracing::error!(msg); + panic!("{}", msg) + }); pin_mut!(listener); From 7972c8d822835f8b58dd11ca5346dceb30933cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 22 Mar 2024 09:29:12 +0100 Subject: [PATCH 19/24] Split HTTP and HTTPS handling --- rust/agama-server/src/agama-web-server.rs | 100 +++++++++++++--------- 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index f5eaf5f9fa..9236cf962d 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -167,6 +167,63 @@ fn https_redirect() -> Router { .route_service("/*path", redirect_service) } +/// handle the HTTPS connection +async fn handle_https_stream( + tls_acceptor: SslAcceptor, + addr: std::net::SocketAddr, + tcp_stream: tokio::net::TcpStream, + service: axum::Router, +) { + // handle HTTPS connection + let ssl = Ssl::new(tls_acceptor.context()).unwrap(); + let mut tls_stream = SslStream::new(ssl, tcp_stream).unwrap(); + if let Err(err) = SslStream::accept(Pin::new(&mut tls_stream)).await { + tracing::error!("Error during TSL handshake from {}: {}", addr, err); + } + + let stream = TokioIo::new(tls_stream); + let hyper_service = + hyper::service::service_fn(move |request: Request| service.clone().call(request)); + + let ret = Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(stream, hyper_service) + .await; + + if let Err(err) = ret { + tracing::error!("Error serving connection from {}: {}", addr, err); + } +} + +/// handle the HTTP connection +async fn handle_http_stream( + addr: std::net::SocketAddr, + tcp_stream: tokio::net::TcpStream, + service: axum::Router, + redirector_service: axum::Router, +) { + let stream = TokioIo::new(tcp_stream); + let hyper_service = hyper::service::service_fn(move |request: Request| { + // check if it is local connection or external + // the to_canonical() converts IPv4-mapped IPv6 addresses + // to plain IPv4, then is_loopback() works correctly for the IPv4 connections + if addr.ip().to_canonical().is_loopback() { + // accept plain HTTP on the local connection + service.clone().call(request) + } else { + // redirect external connections to HTTPS + redirector_service.clone().call(request) + } + }); + + let ret = Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(stream, hyper_service) + .await; + + if let Err(err) = ret { + tracing::error!("Error serving connection from {}: {}", addr, err); + } +} + /// Starts the web server async fn start_server(address: String, service: Router, ssl_acceptor: SslAcceptor) { tracing::info!("Starting Agama web server at {}", address); @@ -199,49 +256,10 @@ async fn start_server(address: String, service: Router, ssl_acceptor: SslAccepto tokio::spawn(async move { if is_ssl_stream(&tcp_stream).await { // handle HTTPS connection - let ssl = Ssl::new(tls_acceptor.context()).unwrap(); - let mut tls_stream = SslStream::new(ssl, tcp_stream).unwrap(); - if let Err(err) = SslStream::accept(Pin::new(&mut tls_stream)).await { - tracing::error!("Error during TSL handshake from {}: {}", addr, err); - } - - let stream = TokioIo::new(tls_stream); - let hyper_service = - hyper::service::service_fn(move |request: Request| { - tower_service.clone().call(request) - }); - - let ret = Builder::new(TokioExecutor::new()) - .serve_connection_with_upgrades(stream, hyper_service) - .await; - - if let Err(err) = ret { - tracing::error!("Error serving connection from {}: {}", addr, err); - } + handle_https_stream(tls_acceptor, addr, tcp_stream, tower_service).await; } else { // handle HTTP connection - let stream = TokioIo::new(tcp_stream); - let hyper_service = - hyper::service::service_fn(move |request: Request| { - // check if it is local connection or external - // the to_canonical() converts IPv4-mapped IPv6 addresses - // to plain IPv4, then is_loopback() works correctly for the IPv4 connections - if addr.ip().to_canonical().is_loopback() { - // accept plain HTTP on the local connection - tower_service.clone().call(request) - } else { - // redirect external connections to HTTPS - redirector_service.clone().call(request) - } - }); - - let ret = Builder::new(TokioExecutor::new()) - .serve_connection_with_upgrades(stream, hyper_service) - .await; - - if let Err(err) = ret { - tracing::error!("Error serving connection from {}: {}", addr, err); - } + handle_http_stream(addr, tcp_stream, tower_service, redirector_service).await; } }); } From 7ed3e58e279ffcef1f0c146b73a5042b03be1938 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 22 Mar 2024 09:55:50 +0100 Subject: [PATCH 20/24] Do not continue when SSL handshake fails --- rust/agama-server/src/agama-web-server.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 9236cf962d..16fd1aa9e7 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -179,18 +179,19 @@ async fn handle_https_stream( let mut tls_stream = SslStream::new(ssl, tcp_stream).unwrap(); if let Err(err) = SslStream::accept(Pin::new(&mut tls_stream)).await { tracing::error!("Error during TSL handshake from {}: {}", addr, err); - } + } else { + let stream = TokioIo::new(tls_stream); + let hyper_service = hyper::service::service_fn(move |request: Request| { + service.clone().call(request) + }); - let stream = TokioIo::new(tls_stream); - let hyper_service = - hyper::service::service_fn(move |request: Request| service.clone().call(request)); + let ret = Builder::new(TokioExecutor::new()) + .serve_connection_with_upgrades(stream, hyper_service) + .await; - let ret = Builder::new(TokioExecutor::new()) - .serve_connection_with_upgrades(stream, hyper_service) - .await; - - if let Err(err) = ret { - tracing::error!("Error serving connection from {}: {}", addr, err); + if let Err(err) = ret { + tracing::error!("Error serving connection from {}: {}", addr, err); + } } } From 7234398fb43241b4c8d43b1beae06f7950fc3417 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 22 Mar 2024 10:03:55 +0100 Subject: [PATCH 21/24] Less qualification --- rust/agama-server/src/agama-web-server.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index 16fd1aa9e7..ef4ff7dda1 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -4,6 +4,7 @@ use agama_server::{ web::{self, run_monitor}, }; use axum::{ + extract::Request as AxumRequest, http::{Request, Response}, Router, }; @@ -153,7 +154,7 @@ fn redirect_error() -> Response { /// instead of the redirection fn https_redirect() -> Router { // see https://docs.rs/axum/latest/axum/routing/struct.Router.html#example - let redirect_service = tower::service_fn(|req: axum::extract::Request| async move { + let redirect_service = tower::service_fn(|req: AxumRequest| async move { if let Some(host) = req.headers().get("host").and_then(|h| h.to_str().ok()) { Ok(redirect_https(host, req.uri())) } else { From 883da06cf1e421135df8195962532427aab320b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 22 Mar 2024 11:22:08 +0100 Subject: [PATCH 22/24] Added TODO marks for future enhancements --- rust/agama-server/src/cert.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 22dd492e70..6326522ec2 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -9,6 +9,23 @@ use openssl::x509::extension::{ }; 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(...) +// } + /// 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> { @@ -48,8 +65,11 @@ pub fn create_certificate() -> Result<(X509, PKey), ErrorStack> { 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))?, )?; From dbec47f1647ebc8086709242d1bf30ad11836386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 22 Mar 2024 15:41:17 +0100 Subject: [PATCH 23/24] More review fixes --- rust/agama-server/src/agama-web-server.rs | 9 ++------- rust/agama-server/src/cert.rs | 7 +++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index ef4ff7dda1..ce0e23fdaf 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -79,13 +79,6 @@ impl ServeArgs { tls_builder.set_private_key(&key)?; tls_builder.set_certificate(&cert)?; - - // 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(); } else { tracing::info!("Loading PEM certificate: {}", self.cert); tls_builder.set_certificate_file(PathBuf::from(self.cert.clone()), SslFiletype::PEM)?; @@ -280,6 +273,8 @@ async fn serve_command(args: ServeArgs) -> anyhow::Result<()> { let dbus = connection_to(&args.dbus_address).await?; let service = web::service(config, tx, dbus).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() { ssl_acceptor } else { diff --git a/rust/agama-server/src/cert.rs b/rust/agama-server/src/cert.rs index 6326522ec2..db02bf951b 100644 --- a/rust/agama-server/src/cert.rs +++ b/rust/agama-server/src/cert.rs @@ -81,5 +81,12 @@ pub fn create_certificate() -> Result<(X509, PKey), ErrorStack> { 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)) } From 1e09613e902e8e36067efce171cf9927931ec120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Fri, 22 Mar 2024 17:33:28 +0100 Subject: [PATCH 24/24] Use None as defaults --- rust/agama-server/src/agama-web-server.rs | 36 +++++++++++++---------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/rust/agama-server/src/agama-web-server.rs b/rust/agama-server/src/agama-web-server.rs index ce0e23fdaf..a484b75a0f 100644 --- a/rust/agama-server/src/agama-web-server.rs +++ b/rust/agama-server/src/agama-web-server.rs @@ -48,24 +48,28 @@ struct ServeArgs { address: String, #[arg( long, - default_value = "", + default_value = None, help = "Optional secondary address to listen on" )] - address2: String, + address2: Option, #[arg( long, - default_value = "", + default_value = None, help = "Path to the SSL private key file in PEM format" )] - key: String, + key: Option, #[arg( long, - default_value = "", + default_value = None, help = "Path to the SSL certificate file in PEM format" )] - cert: String, + cert: Option, // Agama D-Bus address - #[arg(long, default_value = "unix:path=/run/agama/bus")] + #[arg( + long, + default_value = "unix:path=/run/agama/bus", + help = "The D-Bus address for connecting to the Agama service" + )] dbus_address: String, } @@ -74,17 +78,17 @@ impl ServeArgs { fn ssl_acceptor(&self) -> Result { let mut tls_builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?; - if self.cert.is_empty() && self.key.is_empty() { + 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)?; + + tracing::info!("Loading PEM key: {}", key); + tls_builder.set_private_key_file(PathBuf::from(key), SslFiletype::PEM)?; + } else { let (cert, key) = agama_server::cert::create_certificate()?; tls_builder.set_private_key(&key)?; tls_builder.set_certificate(&cert)?; - } else { - tracing::info!("Loading PEM certificate: {}", self.cert); - tls_builder.set_certificate_file(PathBuf::from(self.cert.clone()), SslFiletype::PEM)?; - - tracing::info!("Loading PEM key: {}", self.key); - tls_builder.set_private_key_file(PathBuf::from(self.key.clone()), SslFiletype::PEM)?; } // check that the key belongs to the certificate @@ -283,8 +287,8 @@ async fn serve_command(args: ServeArgs) -> anyhow::Result<()> { let mut addresses = vec![args.address]; - if !args.address2.is_empty() { - addresses.push(args.address2); + if let Some(a) = args.address2 { + addresses.push(a) } let servers: Vec<_> = addresses