public inbox for pve-devel@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 0/3] Fix #5105: Overhaul TLS Handshake Checking Logic
Date: Fri,  5 Jul 2024 18:20:17 +0200	[thread overview]
Message-ID: <20240705162020.1203734-1-m.carrara@proxmox.com> (raw)

Fix #5105: Overhaul TLS Handshake Checking Logic
================================================

This series fixes bug #5105 [1] by overhauling the TLS handshake
checking logic, which is performed when using a connection acceptor
variant with optional TLS.

In the case of PBS (the only place where this is used, to my knowledge),
any requests made over plain HTTP are redirected to the same host, but
clients are instructed to use HTTPS instead.

The TLS handshake checking logic determines whether the client uses HTTP
or HTTPS by peeking into the stream buffer -- if the first 5 received
bytes look like a TLS handshake fragment, the connection is passed on to
OpenSSL before being accepted. Otherwise the connection is assumed to be
unencrypted, i.e. plain HTTP.

However, this logic contains two errors:

  1. The timeout duration is too short - one second is too little
  2. When a timeout occurs, the connection is assumed to be unencrypted
     (and thus plain HTTP)

The patches 01 and 02 are mainly done in preparation for patch 03 (which
contains the actual fix), improving the overall quality of the code and
including the peer's address in error logs.

Please see the individual patches for more information.

Special thanks go to Stefan Hanreich whose advice helped identifying
many individual puzzle pieces comprising this issue.

References
----------

[1]: https://bugzilla.proxmox.com/show_bug.cgi?id=5105

Summary of Changes
------------------

Max Carrara (3):
  rest-server: connection: clean up accept data flow
  rest-server: connection: log peer address on error
  fix #5105: rest-server: connection: overhaul TLS handshake check logic

 proxmox-rest-server/src/connection.rs | 165 +++++++++++++-------------
 1 file changed, 85 insertions(+), 80 deletions(-)

-- 
2.39.2



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


             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 Max Carrara [this message]
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 ` [pve-devel] [PATCH v1 proxmox 2/3] rest-server: connection: log peer address on error Max Carrara
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-1-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 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