From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id BA0221FF2C6
	for <inbox@lore.proxmox.com>; Tue,  9 Jul 2024 15:16:28 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 1057A1FA38;
	Tue,  9 Jul 2024 15:16:51 +0200 (CEST)
From: Max Carrara <m.carrara@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Tue,  9 Jul 2024 15:16:35 +0200
Message-Id: <20240709131637.266675-2-m.carrara@proxmox.com>
X-Mailer: git-send-email 2.39.2
In-Reply-To: <20240709131637.266675-1-m.carrara@proxmox.com>
References: <20240709131637.266675-1-m.carrara@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.028 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH v3 proxmox 1/3] rest-server: connection: clean
 up accept data flow
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.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
Changes v2 --> v3:
  * 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