f6bacbb58f
On rare occasions, the TLS "client hello" message [1] is delayed after
a connection with the server was established, which causes HTTPS
requests to fail before TLS was even negotiated. In these cases, the
server would incorrectly respond with "HTTP/1.1 400 Bad Request"
instead of closing the connection (or similar).
The reasons for the "client hello" being delayed seem to vary; one
user noticed that the issue went away completely after they turned off
UFW [2]. Another user noticed (during private correspondence) that the
issue only appeared when connecting to their PBS instance via WAN, but
not from within their VPN. In the WAN case a firewall was also
present. The same user kindly provided tcpdumps and strace logs on
request.
The issue was finally reproduced with the following Python script:
import socket
import time
HOST: str = ...
PORT: int = ...
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
sock.connect((HOST, PORT))
time.sleep(1.5) # simulate firewall / proxy / etc. delay
sock.sendall(b"\x16\x03\x01\x02\x00")
data = sock.recv(256)
print(data)
The additional delay before sending the first 5 bytes of the "client
hello" message causes the handshake checking logic to incorrectly fall
back to plain HTTP.
All of this is fixed by the following:
1. Increase the timeout duration to 10 seconds (from 1)
2. Instead of falling back to plain HTTP, refuse to accept the
connection if the TLS handshake wasn't initiated before the
timeout limit is reached
3. Only accept plain HTTP if the first 5 bytes do not correspond to
a TLS handshake fragment [3]
4. Do not take the last number of bytes that were in the buffer into
account; instead, only perform the actual handshake check if
5 bytes are in the peek buffer using some of tokio's low-level
functionality
Regarding 1.: This should be generous enough for any client to be able
to initiate a TLS handshake, despite its surrounding circumstances.
Regarding 4.: While this is not 100% related to the issue, peeking into
the buffer in this manner should ensure that our implementation here
remains correct, even if the kernel's underlying behaviour regarding
edge-triggering is changed [4]. At the same time, there's no need for
busy-waiting and continuously yielding to the event loop anymore.
[1]: https://www.rfc-editor.org/rfc/rfc8446.html#section-4.1.2
[2]: https://forum.proxmox.com/threads/disable-default-http-redirects-on-8007.142312/post-675352
[3]: https://www.rfc-editor.org/rfc/rfc8446.html#section-5.1
[4]: https://lwn.net/Articles/864947/
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
Local cargo config
==================
This repository ships with a ``.cargo/config`` that replaces the crates.io
registry with packaged crates located in ``/usr/share/cargo/registry``.
A similar config is also applied building with dh_cargo. Cargo.lock needs to be
deleted when switching between packaged crates and crates.io, since the
checksums are not compatible.
To reference new dependencies (or updated versions) that are not yet packaged,
the dependency needs to point directly to a path or git source.
Steps for Releases
==================
- Run ./bump.sh <CRATE> [patch|minor|major|<VERSION>]
-- Fill out changelog
-- Confirm bump commit
- Build packages with `make deb`.
-- Don't forget to commit updated d/control!
Adding Crates
=============
1) At the top level:
- Generate the crate: ``cargo new --lib the-name``
- Sort the crate into ``Cargo.toml``'s ``workspace.members``
2) In the new crate's ``Cargo.toml``:
- In ``[package]`` set:
authors.workspace = true
license.workspace = true
edition.workspace = true
exclude.workspace = true
repository.workspace = true
- Add a meaningful ``description``
- Copy ``debian/copyright`` and ``debian/debcargo.toml`` from another subcrate.
Adding a new Dependency
=======================
1) At the top level:
- Add it to ``[workspace.dependencies]`` specifying the version and any
features that should be enabled throughout the workspace
2) In each member's ``Cargo.toml``:
- Add it to the desired dependencies section with ``workspace = true`` and no
version specified.
- If this member requires additional features, add only the extra features to
the member dependency.
Updating a Dependency's Version
===============================
1) At the top level:
- Bump the version in ``[workspace.dependencies]`` as desired.
- Check for deprecations or breakage throughout the workspace.
Notes on Workspace Inheritance
==============================
Common metadata (like authors, license, ..) are inherited throughout the
workspace. If new fields are added that are identical for all crates, they
should be defined in the top-level ``Cargo.toml`` file's
``[workspace.package]`` section, and inherited in all members explicitly by
setting ``FIELD.workspace = true`` in the member's ``[package]`` section.
Dependency information is also inherited throughout the workspace, allowing a
single dependency specification in the top-level Cargo.toml file to be used by
all members.
Some restrictions apply:
- features can only be added in members, never removed (this includes
``default_features = false``!)
- the base feature set at the workspace level should be the minimum (possibly
empty!) set required by all members
- workspace dependency specifications cannot include ``optional``
- if needed, the ``optional`` flag needs to be set at the member level when
using a workspace dependency