From fcba8a385805405d2733ddb56391e187604dcc34 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 5 May 2022 14:56:31 +0200 Subject: [PATCH 1/4] Expose option for setting TLS handshake timeout --- actix-http/src/lib.rs | 2 ++ actix-http/src/service.rs | 62 ++++++++++++++++++++++++++++++-- actix-http/tests/test_openssl.rs | 8 +++-- actix-web/CHANGES.md | 1 + actix-web/src/server.rs | 33 +++++++++++++++-- 5 files changed, 100 insertions(+), 6 deletions(-) diff --git a/actix-http/src/lib.rs b/actix-http/src/lib.rs index 360cb86fc54..2544d6012d9 100644 --- a/actix-http/src/lib.rs +++ b/actix-http/src/lib.rs @@ -69,6 +69,8 @@ pub use self::payload::{BoxedPayloadStream, Payload, PayloadStream}; pub use self::requests::{Request, RequestHead, RequestHeadType}; pub use self::responses::{Response, ResponseBuilder, ResponseHead}; pub use self::service::HttpService; +#[cfg(any(feature = "openssl", feature = "rustls"))] +pub use self::service::TlsAcceptorConfig; /// A major HTTP protocol version. #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index f4fe625a395..e3de9d23b0f 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -181,6 +181,25 @@ where } } +#[cfg(any(feature = "openssl", feature = "rustls"))] +pub struct TlsAcceptorConfig { + pub(crate) handshake_timeout: Option, +} + +#[cfg(any(feature = "openssl", feature = "rustls"))] +impl TlsAcceptorConfig { + pub fn new(handshake_timeout: Option) -> Self { + Self { handshake_timeout } + } + + pub fn handshake_timeout(self, dur: std::time::Duration) -> Self { + Self { + handshake_timeout: Some(dur), + ..self + } + } +} + #[cfg(feature = "openssl")] mod openssl { use actix_service::ServiceFactoryExt as _; @@ -230,7 +249,27 @@ mod openssl { Error = TlsError, InitError = (), > { - Acceptor::new(acceptor) + self.openssl_with_config(acceptor, TlsAcceptorConfig::new(None)) + } + + /// Create OpenSSL based service with configuration. + pub fn openssl_with_config( + self, + acceptor: SslAcceptor, + tls_acceptor_config: TlsAcceptorConfig, + ) -> impl ServiceFactory< + TcpStream, + Config = (), + Response = (), + Error = TlsError, + InitError = (), + > { + let mut acceptor = Acceptor::new(acceptor); + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { + acceptor.set_handshake_timeout(handshake_timeout); + } + + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") }) @@ -293,8 +332,23 @@ mod rustls { { /// Create Rustls based service. pub fn rustls( + self, + config: ServerConfig, + ) -> impl ServiceFactory< + TcpStream, + Config = (), + Response = (), + Error = TlsError, + InitError = (), + > { + self.rustls_with_config(config, TlsAcceptorConfig::new(None)) + } + + /// Create Rustls based service with configuration. + pub fn rustls_with_config( self, mut config: ServerConfig, + tls_acceptor_config: TlsAcceptorConfig, ) -> impl ServiceFactory< TcpStream, Config = (), @@ -306,7 +360,11 @@ mod rustls { protos.extend_from_slice(&config.alpn_protocols); config.alpn_protocols = protos; - Acceptor::new(config) + let mut acceptor = Acceptor::new(config); + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { + acceptor.set_handshake_timeout(handshake_timeout); + } + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") }) diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index 35321ac9886..d0bb73f683e 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -2,13 +2,14 @@ extern crate tls_openssl as openssl; +use std::time::Duration; use std::{convert::Infallible, io}; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, error::PayloadError, header::{self, HeaderValue}, - Error, HttpService, Method, Request, Response, StatusCode, Version, + Error, HttpService, Method, Request, Response, StatusCode, TlsAcceptorConfig, Version, }; use actix_http_test::test_server; use actix_service::{fn_service, ServiceFactoryExt}; @@ -89,7 +90,10 @@ async fn h2_1() -> io::Result<()> { assert_eq!(req.version(), Version::HTTP_2); ok::<_, Error>(Response::ok()) }) - .openssl(tls_config()) + .openssl_with_config( + tls_config(), + TlsAcceptorConfig::new(Some(Duration::from_secs(5))), + ) .map_err(|_| ()) }) .await; diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index cb82ef653c9..7e474c85e14 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -6,6 +6,7 @@ - Add `Route::wrap()` to allow individual routes to use middleware. [#2725] - Add `ServiceConfig::default_service()`. [#2338] [#2743] - Implement `ResponseError` for `std::convert::Infallible` +- Add `tls_handshake_timeout` server configuration option to configure TLS handshake timeout [#2752] ### Fixed - Clear connection-level data on `HttpRequest` drop. [#2742] diff --git a/actix-web/src/server.rs b/actix-web/src/server.rs index 99812600c67..47b243352a5 100644 --- a/actix-web/src/server.rs +++ b/actix-web/src/server.rs @@ -18,6 +18,9 @@ use actix_tls::accept::openssl::reexports::{AlpnError, SslAcceptor, SslAcceptorB #[cfg(feature = "rustls")] use actix_tls::accept::rustls::reexports::ServerConfig as RustlsServerConfig; +#[cfg(any(feature = "openssl", feature = "rustls"))] +use actix_http::TlsAcceptorConfig; + use crate::{config::AppConfig, Error}; struct Socket { @@ -30,6 +33,8 @@ struct Config { keep_alive: KeepAlive, client_request_timeout: Duration, client_disconnect_timeout: Duration, + #[cfg(any(feature = "openssl", feature = "rustls"))] + tls_handshake_timeout: Option, } /// An HTTP Server. @@ -92,6 +97,8 @@ where keep_alive: KeepAlive::default(), client_request_timeout: Duration::from_secs(5), client_disconnect_timeout: Duration::from_secs(1), + #[cfg(any(feature = "rustls", feature = "openssl"))] + tls_handshake_timeout: None, })), backlog: 1024, sockets: Vec::new(), @@ -225,6 +232,22 @@ where self } + #[cfg(any(feature = "openssl", feature = "rustls"))] + /// Set TLS handshake timeout. + /// + /// Defines a timeout for TLS handshake. If the TLS handshake does not complete + /// within this time, the connection is closed. + /// + /// By default handshake timeout is set to 3000 milliseconds. + pub fn tls_handshake_timeout(self, dur: Duration) -> Self { + self.config + .lock() + .unwrap() + .tls_handshake_timeout + .replace(dur); + self + } + #[doc(hidden)] #[deprecated(since = "4.0.0", note = "Renamed to `client_disconnect_timeout`.")] pub fn client_shutdown(self, dur: u64) -> Self { @@ -379,7 +402,10 @@ where svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .openssl(acceptor.clone()) + .openssl_with_config( + acceptor.clone(), + TlsAcceptorConfig::new(c.tls_handshake_timeout), + ) })?; Ok(self) @@ -437,7 +463,10 @@ where svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .rustls(config.clone()) + .rustls_with_config( + config.clone(), + TlsAcceptorConfig::new(c.tls_handshake_timeout), + ) })?; Ok(self) From d64f526e6a6651229f3fd0010067f178b9958ef4 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 27 Jun 2022 02:44:29 +0100 Subject: [PATCH 2/4] Update CHANGES.md --- actix-web/CHANGES.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index bdf8fa456ec..0144cb912fe 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -1,19 +1,23 @@ # Changelog ## Unreleased - 2022-xx-xx -- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. ### Added - Add `ServiceRequest::{parts, request}()` getter methods. [#2786] +- Add configuration options for TLS handshake timeout via `HttpServer::{rustls, openssl}_with_config` methods. [#2752] +### Changed +- Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. + +[#2752]: https://github.com/actix/actix-web/pull/2752 [#2786]: https://github.com/actix/actix-web/pull/2786 + ## 4.1.0 - 2022-06-11 ### Added - Add `ServiceRequest::extract()` to make it easier to use extractors when writing middlewares. [#2647] - Add `Route::wrap()` to allow individual routes to use middleware. [#2725] - Add `ServiceConfig::default_service()`. [#2338] [#2743] - Implement `ResponseError` for `std::convert::Infallible` -- Add `tls_handshake_timeout` server configuration option to configure TLS handshake timeout [#2752] ### Changed - Minimum supported Rust version (MSRV) is now 1.56 due to transitive `hashbrown` dependency. From f1b3b532b7eca4bb7a7a87bf3ff2fb71f182664c Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 27 Jun 2022 03:04:57 +0100 Subject: [PATCH 3/4] document new items --- actix-http/src/lib.rs | 1 + actix-http/src/service.rs | 19 +++++++++++-------- actix-web/src/server.rs | 24 +++++++++++++++--------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/actix-http/src/lib.rs b/actix-http/src/lib.rs index 2544d6012d9..184049860f5 100644 --- a/actix-http/src/lib.rs +++ b/actix-http/src/lib.rs @@ -25,6 +25,7 @@ )] #![doc(html_logo_url = "https://actix.rs/img/logo.png")] #![doc(html_favicon_url = "https://actix.rs/favicon.ico")] +#![cfg_attr(docsrs, feature(doc_cfg))] pub use ::http::{uri, uri::Uri}; pub use ::http::{Method, StatusCode, Version}; diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index e3de9d23b0f..469a95ba0a6 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -181,17 +181,17 @@ where } } +/// Configuration options used when accepting TLS connection. #[cfg(any(feature = "openssl", feature = "rustls"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] +#[derive(Debug, Default)] pub struct TlsAcceptorConfig { pub(crate) handshake_timeout: Option, } #[cfg(any(feature = "openssl", feature = "rustls"))] impl TlsAcceptorConfig { - pub fn new(handshake_timeout: Option) -> Self { - Self { handshake_timeout } - } - + /// Set TLS handshake timeout duration. pub fn handshake_timeout(self, dur: std::time::Duration) -> Self { Self { handshake_timeout: Some(dur), @@ -249,10 +249,10 @@ mod openssl { Error = TlsError, InitError = (), > { - self.openssl_with_config(acceptor, TlsAcceptorConfig::new(None)) + self.openssl_with_config(acceptor, TlsAcceptorConfig::default()) } - /// Create OpenSSL based service with configuration. + /// Create OpenSSL based service with custom TLS acceptor configuration. pub fn openssl_with_config( self, acceptor: SslAcceptor, @@ -265,6 +265,7 @@ mod openssl { InitError = (), > { let mut acceptor = Acceptor::new(acceptor); + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { acceptor.set_handshake_timeout(handshake_timeout); } @@ -341,10 +342,10 @@ mod rustls { Error = TlsError, InitError = (), > { - self.rustls_with_config(config, TlsAcceptorConfig::new(None)) + self.rustls_with_config(config, TlsAcceptorConfig::default()) } - /// Create Rustls based service with configuration. + /// Create Rustls based service with custom TLS acceptor configuration. pub fn rustls_with_config( self, mut config: ServerConfig, @@ -361,9 +362,11 @@ mod rustls { config.alpn_protocols = protos; let mut acceptor = Acceptor::new(config); + if let Some(handshake_timeout) = tls_acceptor_config.handshake_timeout { acceptor.set_handshake_timeout(handshake_timeout); } + acceptor .map_init_err(|_| { unreachable!("TLS acceptor service factory does not error on init") diff --git a/actix-web/src/server.rs b/actix-web/src/server.rs index 47b243352a5..169eafab055 100644 --- a/actix-web/src/server.rs +++ b/actix-web/src/server.rs @@ -232,19 +232,21 @@ where self } - #[cfg(any(feature = "openssl", feature = "rustls"))] /// Set TLS handshake timeout. /// /// Defines a timeout for TLS handshake. If the TLS handshake does not complete /// within this time, the connection is closed. /// /// By default handshake timeout is set to 3000 milliseconds. + #[cfg(any(feature = "openssl", feature = "rustls"))] + #[cfg_attr(docsrs, doc(cfg(any(feature = "openssl", feature = "rustls"))))] pub fn tls_handshake_timeout(self, dur: Duration) -> Self { self.config .lock() .unwrap() .tls_handshake_timeout .replace(dur); + self } @@ -399,13 +401,15 @@ where .into_factory() .map_err(|err| err.into().error_response()); + let acceptor_config = match c.tls_handshake_timeout { + Some(dur) => TlsAcceptorConfig::default().handshake_timeout(dur), + None => TlsAcceptorConfig::default(), + }; + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .openssl_with_config( - acceptor.clone(), - TlsAcceptorConfig::new(c.tls_handshake_timeout), - ) + .openssl_with_config(acceptor.clone(), acceptor_config) })?; Ok(self) @@ -460,13 +464,15 @@ where .into_factory() .map_err(|err| err.into().error_response()); + let acceptor_config = match c.tls_handshake_timeout { + Some(dur) => TlsAcceptorConfig::default().handshake_timeout(dur), + None => TlsAcceptorConfig::default(), + }; + svc.finish(map_config(fac, move |_| { AppConfig::new(true, host.clone(), addr) })) - .rustls_with_config( - config.clone(), - TlsAcceptorConfig::new(c.tls_handshake_timeout), - ) + .rustls_with_config(config.clone(), acceptor_config) })?; Ok(self) From 12785710e9ec28ed36c4b083996c1364915dfc7f Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 27 Jun 2022 03:09:49 +0100 Subject: [PATCH 4/4] fix tests --- actix-http/src/service.rs | 2 +- actix-http/tests/test_openssl.rs | 5 ++--- actix-http/tests/test_rustls.rs | 8 ++++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index 469a95ba0a6..27029cb8e4b 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -195,7 +195,7 @@ impl TlsAcceptorConfig { pub fn handshake_timeout(self, dur: std::time::Duration) -> Self { Self { handshake_timeout: Some(dur), - ..self + // ..self } } } diff --git a/actix-http/tests/test_openssl.rs b/actix-http/tests/test_openssl.rs index d0bb73f683e..b97b2e45b8b 100644 --- a/actix-http/tests/test_openssl.rs +++ b/actix-http/tests/test_openssl.rs @@ -2,8 +2,7 @@ extern crate tls_openssl as openssl; -use std::time::Duration; -use std::{convert::Infallible, io}; +use std::{convert::Infallible, io, time::Duration}; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, @@ -92,7 +91,7 @@ async fn h2_1() -> io::Result<()> { }) .openssl_with_config( tls_config(), - TlsAcceptorConfig::new(Some(Duration::from_secs(5))), + TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)), ) .map_err(|_| ()) }) diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index 550375296d2..2bbf1524b54 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -8,13 +8,14 @@ use std::{ net::{SocketAddr, TcpStream as StdTcpStream}, sync::Arc, task::Poll, + time::Duration, }; use actix_http::{ body::{BodyStream, BoxBody, SizedStream}, error::PayloadError, header::{self, HeaderName, HeaderValue}, - Error, HttpService, Method, Request, Response, StatusCode, Version, + Error, HttpService, Method, Request, Response, StatusCode, TlsAcceptorConfig, Version, }; use actix_http_test::test_server; use actix_rt::pin; @@ -160,7 +161,10 @@ async fn h2_1() -> io::Result<()> { assert_eq!(req.version(), Version::HTTP_2); ok::<_, Error>(Response::ok()) }) - .rustls(tls_config()) + .rustls_with_config( + tls_config(), + TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)), + ) }) .await;