all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Max Carrara <m.carrara@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v1 proxmox 2/3] rest-server: connection: log peer address on error
Date: Fri,  5 Jul 2024 18:20:19 +0200	[thread overview]
Message-ID: <20240705162020.1203734-3-m.carrara@proxmox.com> (raw)
In-Reply-To: <20240705162020.1203734-1-m.carrara@proxmox.com>

.. in order to make debugging easier and logs more helpful.

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
 proxmox-rest-server/src/connection.rs | 42 ++++++++++++++++-----------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/proxmox-rest-server/src/connection.rs b/proxmox-rest-server/src/connection.rs
index 243348c0..470021d7 100644
--- a/proxmox-rest-server/src/connection.rs
+++ b/proxmox-rest-server/src/connection.rs
@@ -2,6 +2,7 @@
 //!
 //! Hyper building block.
 
+use std::net::SocketAddr;
 use std::os::unix::io::AsRawFd;
 use std::path::PathBuf;
 use std::pin::Pin;
@@ -257,6 +258,7 @@ impl From<(ClientSender, InsecureClientSender)> for Sender {
 
 struct AcceptState {
     pub socket: InsecureClientStream,
+    pub peer: SocketAddr,
     pub acceptor: Arc<Mutex<SslAcceptor>>,
     pub accept_counter: Arc<()>,
 }
@@ -276,9 +278,9 @@ impl AcceptBuilder {
         let mut shutdown_future = crate::shutdown_future().fuse();
 
         loop {
-            let socket = futures::select! {
+            let (socket, peer) = futures::select! {
                 res = self.try_setup_socket(&listener).fuse() => match res {
-                    Ok(socket) => socket,
+                    Ok(socket_peer) => socket_peer,
                     Err(err) => {
                         log::error!("couldn't set up TCP socket: {err}");
                         continue;
@@ -291,12 +293,13 @@ impl AcceptBuilder {
             let accept_counter = Arc::clone(&accept_counter);
 
             if Arc::strong_count(&accept_counter) > self.max_pending_accepts {
-                log::error!("connection rejected - too many open connections");
+                log::error!("[{peer}] connection rejected - too many open connections");
                 continue;
             }
 
             let state = AcceptState {
                 socket,
+                peer,
                 acceptor,
                 accept_counter,
             };
@@ -328,7 +331,7 @@ impl AcceptBuilder {
     async fn try_setup_socket(
         &self,
         listener: &TcpListener,
-    ) -> Result<InsecureClientStream, Error> {
+    ) -> Result<(InsecureClientStream, SocketAddr), Error> {
         let (socket, peer) = match listener.accept().await {
             Ok(connection) => connection,
             Err(error) => {
@@ -338,10 +341,10 @@ impl AcceptBuilder {
 
         socket
             .set_nodelay(true)
-            .context("error while setting TCP_NODELAY on socket")?;
+            .with_context(|| format!("[{peer}] error while setting TCP_NODELAY on socket"))?;
 
         proxmox_sys::linux::socket::set_tcp_keepalive(socket.as_raw_fd(), self.tcp_keepalive_time)
-            .context("error while setting SO_KEEPALIVE on socket")?;
+            .with_context(|| format!("[{peer}] error while setting SO_KEEPALIVE on socket"))?;
 
         #[cfg(feature = "rate-limited-stream")]
         let socket = match self.lookup_rate_limiter.clone() {
@@ -349,13 +352,12 @@ impl AcceptBuilder {
             None => RateLimitedStream::with_limiter(socket, None, None),
         };
 
-        #[cfg(not(feature = "rate-limited-stream"))]
-        let _peer = peer;
-
-        Ok(socket)
+        Ok((socket, peer))
     }
 
     async fn do_accept_tls(state: AcceptState, flags: AcceptFlags, secure_sender: ClientSender) {
+        let peer = state.peer;
+
         let ssl = {
             // limit acceptor_guard scope
             // Acceptor can be reloaded using the command socket "reload-certificate" command
@@ -364,7 +366,9 @@ impl AcceptBuilder {
             match openssl::ssl::Ssl::new(acceptor_guard.context()) {
                 Ok(ssl) => ssl,
                 Err(err) => {
-                    log::error!("failed to create Ssl object from Acceptor context - {err}");
+                    log::error!(
+                        "[{peer}] failed to create Ssl object from Acceptor context - {err}"
+                    );
                     return;
                 }
             }
@@ -373,7 +377,9 @@ impl AcceptBuilder {
         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}");
+                log::error!(
+                    "[{peer}] failed to create SslStream using ssl and connection socket - {err}"
+                );
                 return;
             }
         };
@@ -388,17 +394,17 @@ impl AcceptBuilder {
         match result {
             Ok(Ok(())) => {
                 if secure_sender.send(Ok(secure_stream)).await.is_err() && flags.is_debug {
-                    log::error!("detected closed connection channel");
+                    log::error!("[{peer}] detected closed connection channel");
                 }
             }
             Ok(Err(err)) => {
                 if flags.is_debug {
-                    log::error!("https handshake failed - {err}");
+                    log::error!("[{peer}] https handshake failed - {err}");
                 }
             }
             Err(_) => {
                 if flags.is_debug {
-                    log::error!("https handshake timeout");
+                    log::error!("[{peer}] https handshake timeout");
                 }
             }
         }
@@ -412,6 +418,8 @@ impl AcceptBuilder {
         secure_sender: ClientSender,
         insecure_sender: InsecureClientSender,
     ) {
+        let peer = state.peer;
+
         let client_initiates_handshake = {
             #[cfg(feature = "rate-limited-stream")]
             let socket_ref = state.socket.inner();
@@ -422,7 +430,7 @@ impl AcceptBuilder {
             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}");
+                    log::error!("[{peer}] error checking for TLS handshake: {err}");
                     return;
                 }
             }
@@ -432,7 +440,7 @@ impl AcceptBuilder {
             let insecure_stream = Box::pin(state.socket);
 
             if insecure_sender.send(Ok(insecure_stream)).await.is_err() && flags.is_debug {
-                log::error!("detected closed connection channel")
+                log::error!("[{peer}] detected closed connection channel")
             }
 
             return;
-- 
2.39.2



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


  parent reply	other threads:[~2024-07-05 16:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-05 16:20 [pve-devel] [PATCH v1 proxmox 0/3] Fix #5105: Overhaul TLS Handshake Checking Logic Max Carrara
2024-07-05 16:20 ` [pve-devel] [PATCH v1 proxmox 1/3] rest-server: connection: clean up accept data flow Max Carrara
2024-07-05 16:20 ` Max Carrara [this message]
2024-07-05 16:20 ` [pve-devel] [PATCH v1 proxmox 3/3] fix #5105: rest-server: connection: overhaul TLS handshake check logic Max Carrara
2024-07-08  5:49 ` [pve-devel] [PATCH v1 proxmox 0/3] Fix #5105: Overhaul TLS Handshake Checking Logic 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=20240705162020.1203734-3-m.carrara@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pve-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal