From 9333ef7053e8e21503006545206b3d97e7a62b62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20C?= <54852465+bcortier-devolutions@users.noreply.github.com> Date: Tue, 15 Dec 2020 10:47:42 -0500 Subject: [PATCH] WAYK-2298: remove saphir dummy HTTP listener (#129) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * WAYK-2298: remove saphir dummy HTTP listener * Panic if no HTTP listener is provided * Update saphir dependency Co-authored-by: Benoît CORTIER --- Cargo.lock | 4 +- Cargo.toml | 4 +- src/config.rs | 45 +++--------- src/http/http_server.rs | 147 +++++++++------------------------------- src/service.rs | 35 ++++------ 5 files changed, 56 insertions(+), 179 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f36d5cd73..4206e7d1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2675,7 +2675,7 @@ checksum = "ef703b7cb59335eae2eb93ceb664c0eb7ea6bf567079d843e09420219668e072" [[package]] name = "saphir" version = "2.8.0" -source = "git+https://github.com/richerarc/saphir.git?rev=d0fcd6adc0a8a5e22c095d41a9a2c6c0d0d91316#d0fcd6adc0a8a5e22c095d41a9a2c6c0d0d91316" +source = "git+https://github.com/richerarc/saphir?rev=5f3602d5165a224b83678a5641e34c77b2325bca#5f3602d5165a224b83678a5641e34c77b2325bca" dependencies = [ "base64 0.13.0", "futures 0.3.8", @@ -2710,7 +2710,7 @@ dependencies = [ [[package]] name = "saphir_macro" version = "2.1.1" -source = "git+https://github.com/richerarc/saphir.git?rev=d0fcd6adc0a8a5e22c095d41a9a2c6c0d0d91316#d0fcd6adc0a8a5e22c095d41a9a2c6c0d0d91316" +source = "git+https://github.com/richerarc/saphir?rev=5f3602d5165a224b83678a5641e34c77b2325bca#5f3602d5165a224b83678a5641e34c77b2325bca" dependencies = [ "http 0.2.1", "proc-macro2 1.0.24", diff --git a/Cargo.toml b/Cargo.toml index 1e8e8d4bf..934e0499a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,8 +56,8 @@ spsc-bip-buffer = { git = "https://github.com/Devolutions/spsc-bip-buffer.git", indexmap = "1.0" [dependencies.saphir] -git = "https://github.com/richerarc/saphir.git" -rev = "d0fcd6adc0a8a5e22c095d41a9a2c6c0d0d91316" +git = "https://github.com/richerarc/saphir" +rev = "5f3602d5165a224b83678a5641e34c77b2325bca" default-features = false features = ["https", "json", "macro", "form"] diff --git a/src/config.rs b/src/config.rs index 9d42ce42a..6598cab62 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,13 +13,10 @@ use std::{ }; use url::Url; -const DEFAULT_HTTP_LISTENER_PORT: u32 = 10256; - const ARG_API_KEY: &str = "api-key"; const ARG_APPLICATION_PROTOCOLS: &str = "application-protocols"; const ARG_UNRESTRICTED: &str = "unrestricted"; const ARG_LISTENERS: &str = "listeners"; -const ARG_API_LISTENER: &str = "api-listener"; const ARG_HOSTNAME: &str = "hostname"; const ARG_FARM_NAME: &str = "farm-name"; const ARG_CERTIFICATE_FILE: &str = "certificate-file"; @@ -96,7 +93,6 @@ pub struct Config { pub log_file: Option, pub application_protocols: Vec, pub certificate: CertificateConfig, - pub api_listener: Url, pub provisioner_public_key: Option, pub delegation_private_key: Option, } @@ -105,11 +101,6 @@ impl Default for Config { fn default() -> Self { let default_hostname = get_default_hostname().unwrap_or_else(|| "localhost".to_string()); - let default_api_listener_url = format!("http://0.0.0.0:{}", DEFAULT_HTTP_LISTENER_PORT); - let api_listener = default_api_listener_url - .parse::() - .unwrap_or_else(|e| panic!("API listener URL is invalid: {}", e)); - Config { service_mode: false, service_name: SERVICE_NAME.to_string(), @@ -132,7 +123,6 @@ impl Default for Config { private_key_file: None, private_key_data: None, }, - api_listener, provisioner_public_key: None, delegation_private_key: None, } @@ -215,8 +205,6 @@ pub struct ConfigFile { pub capture_path: Option, #[serde(rename = "Unrestricted")] pub unrestricted: Option, - #[serde(rename = "ApiListener")] - pub api_listener: Option, } fn get_config_path() -> PathBuf { @@ -322,14 +310,6 @@ impl Config { .takes_value(true) .number_of_values(1), ) - .arg( - Arg::with_name(ARG_API_LISTENER) - .long("api-listener") - .value_name("URL") - .env("DGATEWAY_API_LISTENER") - .help("API HTTP listener URL.") - .takes_value(true), - ) .arg( Arg::with_name(ARG_FARM_NAME) .long("farm-name") @@ -517,12 +497,6 @@ impl Config { config.unrestricted = true; } - if let Some(api_listener) = matches.value_of(ARG_API_LISTENER) { - config.api_listener = api_listener - .parse::() - .unwrap_or_else(|e| panic!("API listener URL is invalid: {}", e)); - } - if let Some(farm_name) = matches.value_of(ARG_FARM_NAME) { config.farm_name = farm_name.to_owned(); } @@ -676,6 +650,14 @@ impl Config { panic!("At least one listener has to be specified."); } + if !config + .listeners + .iter() + .any(|listener| matches!(listener.internal_url.scheme(), "http" | "https" | "ws" | "wss")) + { + panic!("At least one HTTP listener is required"); + } + // early fail if we start as restricted without provisioner key if !config.unrestricted && config.provisioner_public_key.is_none() { panic!("provisioner public key is missing in unrestricted mode"); @@ -776,16 +758,6 @@ impl Config { let unrestricted = config_file.unrestricted.unwrap_or(true); let capture_path = config_file.capture_path; - // We always create a dummy API listener because Saphir needs one. - // However, this API listener is unable to process WebSocket traffic. - // Create the API listener as a dummy listener to make Saphir happy, - // but in fact we just ignore it and use only our gateway listeners. - let default_api_listener_url = format!("http://0.0.0.0:{}", DEFAULT_HTTP_LISTENER_PORT); - let api_listener_url = config_file.api_listener.unwrap_or(default_api_listener_url); - let api_listener = api_listener_url - .parse::() - .unwrap_or_else(|e| panic!("API listener URL is invalid: {}", e)); - Some(Config { unrestricted, api_key, @@ -800,7 +772,6 @@ impl Config { private_key_file, ..Default::default() }, - api_listener, provisioner_public_key, delegation_private_key, ..Default::default() diff --git a/src/http/http_server.rs b/src/http/http_server.rs index 5c306ce9a..1db4dc8cc 100644 --- a/src/http/http_server.rs +++ b/src/http/http_server.rs @@ -6,125 +6,40 @@ use crate::{ }, jet_client::JetAssociationsMap, }; -use futures::FutureExt; -use saphir::{ - error::SaphirError, - server::{Server as SaphirServer, SslConfig}, -}; +use saphir::server::Server as SaphirServer; use slog_scope::info; -use std::sync::{Arc, Mutex}; -use tokio_02::{runtime::Runtime, sync::Notify, task::JoinHandle}; - -pub struct HttpServer { - pub server: Mutex>, - server_runtime: Mutex>, - shutdown_notification: Arc, - join_handle: Mutex>>>, -} - -impl HttpServer { - pub fn new(config: Arc, jet_associations: JetAssociationsMap) -> HttpServer { - let shutdown_notification = Arc::new(Notify::new()); - let shutdown_notification_clone = shutdown_notification.clone(); - - let on_shutdown_check = async move { - shutdown_notification_clone.notified().await; - info!("HTTP server was gracefully stopped"); - } - .boxed(); - - let http_server = SaphirServer::builder() - .configure_middlewares(|middlewares| { - info!("Loading HTTP middlewares"); - - // Only the "create association" should requires authorization. - let mut auth_include_path = vec!["/jet/association"]; - let mut auth_exclude_path = vec!["/jet/association//"]; - - if config.unrestricted { - auth_exclude_path.push("/jet/association/"); - } else { - auth_include_path.push("/jet/association/"); - } +use std::sync::Arc; - middlewares.apply( - AuthMiddleware::new(config.clone()), - auth_include_path, - Some(auth_exclude_path), - ) - }) - .configure_router(|router| { - info!("Loading HTTP controllers"); - let health = HealthController::new(config.clone()); - let jet = JetController::new(config.clone(), jet_associations.clone()); - let session = SessionsController::default(); - info!("Configuring HTTP router"); - router.controller(health).controller(jet).controller(session) - }) - .configure_listener(|listener| { - let listener_host = config.api_listener.host().expect("API listener should be specified"); +pub fn configure_http_server(config: Arc, jet_associations: JetAssociationsMap) -> Result<(), String> { + SaphirServer::builder() + .configure_middlewares(|middlewares| { + info!("Loading HTTP middlewares"); - let listener_port = config.api_listener.port().unwrap_or(8080); - let interface = format!("{}:{}", listener_host, listener_port); + // Only the "create association" should requires authorization. + let mut auth_include_path = vec!["/jet/association"]; + let mut auth_exclude_path = vec!["/jet/association//"]; - let listener_config = listener - .interface(&interface) - .server_name("Saphir Server") - .shutdown(on_shutdown_check, true); - - let cert_config_opt = if let Some(cert_path) = &config.certificate.certificate_file { - Some(SslConfig::FilePath(cert_path.into())) - } else if let Some(cert_data) = &config.certificate.certificate_data { - Some(SslConfig::FileData(cert_data.into())) - } else { - None - }; - - let key_config_opt = if let Some(key_path) = &config.certificate.private_key_file { - Some(SslConfig::FilePath(key_path.into())) - } else if let Some(key_data) = &config.certificate.private_key_data { - Some(SslConfig::FileData(key_data.into())) - } else { - None - }; - - if let (Some(cert_config), Some(key_config)) = (cert_config_opt, key_config_opt) { - listener_config.set_ssl_config(cert_config, key_config) - } else { - listener_config - } - }) - .build(); - let runtime = Runtime::new().expect("failed to create runtime for HTTP server"); - HttpServer { - server: Mutex::new(Some(http_server)), - server_runtime: Mutex::new(Some(runtime)), - shutdown_notification, - join_handle: Mutex::new(None), - } - } - - pub fn start(&self) { - let server = { - let mut guard = self.server.lock().unwrap(); - guard.take().expect("Start server can't be called twice") - }; - - let runtime_guard = self.server_runtime.lock().unwrap(); - let runtime = runtime_guard - .as_ref() - .expect("HTTP server runtime must be present on start"); - let join_handle = runtime.spawn(server.run()); - self.join_handle.lock().unwrap().replace(join_handle); - } - - pub fn stop(&self) { - self.shutdown_notification.notify(); - - if let Some(mut runtime) = self.server_runtime.lock().unwrap().take() { - if let Some(join_handle) = self.join_handle.lock().unwrap().take() { - let _ = runtime.block_on(join_handle); + if config.unrestricted { + auth_exclude_path.push("/jet/association/"); + } else { + auth_include_path.push("/jet/association/"); } - } - } + + middlewares.apply( + AuthMiddleware::new(config.clone()), + auth_include_path, + Some(auth_exclude_path), + ) + }) + .configure_router(|router| { + info!("Loading HTTP controllers"); + let health = HealthController::new(config.clone()); + let jet = JetController::new(config.clone(), jet_associations.clone()); + let session = SessionsController::default(); + info!("Configuring HTTP router"); + router.controller(health).controller(jet).controller(session) + }) + .configure_listener(|listener| listener.server_name("Devolutions Gateway")) + .build_stack_only() + .map_err(|e| e.to_string()) } diff --git a/src/service.rs b/src/service.rs index 33c6ee072..5e7e02012 100644 --- a/src/service.rs +++ b/src/service.rs @@ -27,7 +27,7 @@ use slog_scope_futures::future03::FutureExt; use crate::{ config::Config, - http::http_server::HttpServer, + http::http_server::configure_http_server, jet_client::{JetAssociationsMap, JetClient}, logger, rdp::RdpClient, @@ -44,7 +44,7 @@ type VecOfFuturesType = Vec> + Se #[allow(clippy::large_enum_variant)] // `Running` variant is bigger than `Stopped` but we don't care pub enum GatewayState { Stopped, - Running { http_server: HttpServer, runtime: Runtime }, + Running { runtime: Runtime }, } impl Default for GatewayState { @@ -95,9 +95,6 @@ impl GatewayService { let context = create_context(config, logger).expect("failed to create gateway context"); - // start http server - context.http_server.start(); - let join_all = futures::future::join_all(context.futures); runtime.spawn(async { join_all.await.into_iter().for_each(|future_result| { @@ -105,10 +102,7 @@ impl GatewayService { }); }); - self.state = GatewayState::Running { - http_server: context.http_server, - runtime, - }; + self.state = GatewayState::Running { runtime }; } pub fn stop(&mut self) { @@ -116,12 +110,9 @@ impl GatewayService { GatewayState::Stopped => { info!("Attempted to stop gateway service, but it isn't started"); } - GatewayState::Running { http_server, runtime } => { + GatewayState::Running { runtime } => { info!("Stopping gateway service"); - // stop http server - http_server.stop(); - // stop runtime now runtime.shutdown_background(); @@ -132,7 +123,6 @@ impl GatewayService { } pub struct GatewayContext { - pub http_server: HttpServer, pub futures: VecOfFuturesType, } @@ -163,7 +153,11 @@ pub fn create_context(config: Arc, logger: slog::Logger) -> Result, logger: slog::Logger) -> Result, logger: Logger, ) -> Result<(), String> { - info!("Starting TCP jet server..."); + info!("Starting TCP jet server ({})...", url); let socket_addr = url .with_default_port(|url| get_default_port_from_server_url(url).map_err(|_| ())) @@ -315,7 +309,7 @@ async fn start_tcp_server( ), scheme => panic!("Unsupported routing URL scheme {}", scheme), } - } else if config.is_rdp_supported() { + } else { let mut peeked = [0; 4]; let _ = conn.peek(&mut peeked).await; @@ -327,9 +321,6 @@ async fn start_tcp_server( let rdp_client = RdpClient::new(config.clone(), tls_public_key.clone(), tls_acceptor.clone()); Box::pin(rdp_client.serve(conn)) } - } else { - let jet_client = JetClient::new(config.clone(), jet_associations.clone()); - Box::pin(jet_client.serve(JetTransport::new_tcp(conn))) }; let client_fut = client_fut.with_logger(logger); @@ -353,7 +344,7 @@ async fn start_websocket_server( tls_acceptor: TlsAcceptor, logger: slog::Logger, ) -> Result<(), String> { - info!("Starting websocket server ..."); + info!("Starting websocket server ({})...", websocket_url); let mut websocket_addr = String::new(); websocket_addr.push_str(websocket_url.host_str().unwrap_or("0.0.0.0"));