public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Max Carrara <m.carrara@proxmox.com>
To: pbs-devel@lists.proxmox.com
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
Subject: [pbs-devel] [PATCH v2 proxmox 1/3] rest-server: connection: clean up accept data flow
Date: Mon,  8 Jul 2024 18:48:15 +0200	[thread overview]
Message-ID: <20240708164817.689324-2-m.carrara@proxmox.com> (raw)
In-Reply-To: <20240708164817.689324-1-m.carrara@proxmox.com>

This adds the structs `AcceptState` and `AcceptFlags` and adapts
relevant method signatures of `AcceptBuilder` accordingly. This makes
it easier to add further parameters in the future.

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
Changes v1 --> v2:
  * none

 proxmox-rest-server/src/connection.rs | 72 ++++++++++++++-------------
 1 file changed, 38 insertions(+), 34 deletions(-)

diff --git a/proxmox-rest-server/src/connection.rs b/proxmox-rest-server/src/connection.rs
index 34b585cb..243348c0 100644
--- a/proxmox-rest-server/src/connection.rs
+++ b/proxmox-rest-server/src/connection.rs
@@ -255,6 +255,16 @@ impl From<(ClientSender, InsecureClientSender)> for Sender {
     }
 }
 
+struct AcceptState {
+    pub socket: InsecureClientStream,
+    pub acceptor: Arc<Mutex<SslAcceptor>>,
+    pub accept_counter: Arc<()>,
+}
+
+struct AcceptFlags {
+    pub is_debug: bool,
+}
+
 impl AcceptBuilder {
     async fn accept_connections(
         self,
@@ -285,24 +295,26 @@ impl AcceptBuilder {
                 continue;
             }
 
+            let state = AcceptState {
+                socket,
+                acceptor,
+                accept_counter,
+            };
+
+            let flags = AcceptFlags {
+                is_debug: self.debug,
+            };
+
             match sender {
                 Sender::Secure(ref secure_sender) => {
-                    let accept_future = Self::do_accept_tls(
-                        socket,
-                        acceptor,
-                        accept_counter,
-                        self.debug,
-                        secure_sender.clone(),
-                    );
+                    let accept_future = Self::do_accept_tls(state, flags, secure_sender.clone());
 
                     tokio::spawn(accept_future);
                 }
                 Sender::SecureAndInsecure(ref secure_sender, ref insecure_sender) => {
                     let accept_future = Self::do_accept_tls_optional(
-                        socket,
-                        acceptor,
-                        accept_counter,
-                        self.debug,
+                        state,
+                        flags,
                         secure_sender.clone(),
                         insecure_sender.clone(),
                     );
@@ -343,17 +355,11 @@ impl AcceptBuilder {
         Ok(socket)
     }
 
-    async fn do_accept_tls(
-        socket: InsecureClientStream,
-        acceptor: Arc<Mutex<SslAcceptor>>,
-        accept_counter: Arc<()>,
-        debug: bool,
-        secure_sender: ClientSender,
-    ) {
+    async fn do_accept_tls(state: AcceptState, flags: AcceptFlags, secure_sender: ClientSender) {
         let ssl = {
             // limit acceptor_guard scope
             // Acceptor can be reloaded using the command socket "reload-certificate" command
-            let acceptor_guard = acceptor.lock().unwrap();
+            let acceptor_guard = state.acceptor.lock().unwrap();
 
             match openssl::ssl::Ssl::new(acceptor_guard.context()) {
                 Ok(ssl) => ssl,
@@ -364,7 +370,7 @@ impl AcceptBuilder {
             }
         };
 
-        let secure_stream = match tokio_openssl::SslStream::new(ssl, socket) {
+        let secure_stream = match tokio_openssl::SslStream::new(ssl, state.socket) {
             Ok(stream) => stream,
             Err(err) => {
                 log::error!("failed to create SslStream using ssl and connection socket - {err}");
@@ -381,41 +387,39 @@ impl AcceptBuilder {
 
         match result {
             Ok(Ok(())) => {
-                if secure_sender.send(Ok(secure_stream)).await.is_err() && debug {
+                if secure_sender.send(Ok(secure_stream)).await.is_err() && flags.is_debug {
                     log::error!("detected closed connection channel");
                 }
             }
             Ok(Err(err)) => {
-                if debug {
+                if flags.is_debug {
                     log::error!("https handshake failed - {err}");
                 }
             }
             Err(_) => {
-                if debug {
+                if flags.is_debug {
                     log::error!("https handshake timeout");
                 }
             }
         }
 
-        drop(accept_counter); // decrease reference count
+        drop(state.accept_counter); // decrease reference count
     }
 
     async fn do_accept_tls_optional(
-        socket: InsecureClientStream,
-        acceptor: Arc<Mutex<SslAcceptor>>,
-        accept_counter: Arc<()>,
-        debug: bool,
+        state: AcceptState,
+        flags: AcceptFlags,
         secure_sender: ClientSender,
         insecure_sender: InsecureClientSender,
     ) {
         let client_initiates_handshake = {
             #[cfg(feature = "rate-limited-stream")]
-            let socket = socket.inner();
+            let socket_ref = state.socket.inner();
 
             #[cfg(not(feature = "rate-limited-stream"))]
-            let socket = &socket;
+            let socket_ref = &state.socket;
 
-            match Self::wait_for_client_tls_handshake(socket).await {
+            match Self::wait_for_client_tls_handshake(socket_ref).await {
                 Ok(initiates_handshake) => initiates_handshake,
                 Err(err) => {
                     log::error!("error checking for TLS handshake: {err}");
@@ -425,16 +429,16 @@ impl AcceptBuilder {
         };
 
         if !client_initiates_handshake {
-            let insecure_stream = Box::pin(socket);
+            let insecure_stream = Box::pin(state.socket);
 
-            if insecure_sender.send(Ok(insecure_stream)).await.is_err() && debug {
+            if insecure_sender.send(Ok(insecure_stream)).await.is_err() && flags.is_debug {
                 log::error!("detected closed connection channel")
             }
 
             return;
         }
 
-        Self::do_accept_tls(socket, acceptor, accept_counter, debug, secure_sender).await
+        Self::do_accept_tls(state, flags, secure_sender).await
     }
 
     async fn wait_for_client_tls_handshake(incoming_stream: &TcpStream) -> Result<bool, Error> {
-- 
2.39.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


  reply	other threads:[~2024-07-08 16:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-08 16:48 [pbs-devel] [PATCH v2 proxmox 0/3] Fix #5105: Overhaul TLS Handshake Checking Logic Max Carrara
2024-07-08 16:48 ` Max Carrara [this message]
2024-07-08 16:48 ` [pbs-devel] [PATCH v2 proxmox 2/3] rest-server: connection: log peer address on error Max Carrara
2024-07-08 16:48 ` [pbs-devel] [PATCH v2 proxmox 3/3] fix #5105: rest-server: connection: overhaul TLS handshake check logic Max Carrara
2024-07-09 10:29   ` Wolfgang Bumiller
2024-07-09 11:32     ` Max Carrara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240708164817.689324-2-m.carrara@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=w.bumiller@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal