As per [RFC9110] the Deflate encoding is a "zlib" data format. This
makes the rest-server compatible with the http-client.
[RFC9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
Since hyper can spawn() more tasks, when we stop passing `WorkerTask`
references down the stack, we still need to be able to *inherit* the
current logging context. Hyper provides a way to replace its used
`spawn()` method, so we need to provide a way to reuse the logging
context.
Instead of having the `FileLogger` and warn counter separately
available with local-only access, put them behind an Arc<Mutex<>>.
Previously they already *were* behind an Arc<Mutex<>> as part of the
WorkerTaskState.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Enable the tracing-system by setting the LOGGER task local variable
to a instance of a FileLogger and initializing the WARN_COUNTER.
Removed the task_log! macros and some occurences.
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
[WB: remove flog! import in doctests]
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
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>
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>
Adds a privileged_addr to ApiConfig, and some helpers for
hyper (both server and client)
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
We want to get a 401 error at HTTP level.
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
The `Redirector` is a simple `Service` that redirects HTTP requests
to HTTPS and can be served by a `hyper::Server`.
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
The new public function `accept_tls_optional()` is added, which
accepts both plain TCP streams and TCP streams running TLS. Plain TCP
streams are sent along via a separate channel in order to clearly
distinguish between "secure" and "insecure" connections.
Furthermore, instead of `AcceptBuilder` itself holding a reference to
an `SslAcceptor`, its public functions now take the acceptor as an
argument. The public functions' names are changed to distinguish
between their functionality in a more explicit manner:
* `accept()` --> `accept_tls()`
* NEW --> `accept_tls_optional()`
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
We had two call sites deriving the directory "shard" where the task
log file is actually saved to, this can lead to ugly bugs and is
better done in a central single-source-of-truth way.
While at it factor out the creation of the log file (and it's shard
directory) to avoid crowding the WorkerTask new fn to much.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
technically an empty string is not valid json, but when sending an api
request without any parameters, treating the empty body as an empty
parameter hash instead of an error, makes the the api more robust for
clients
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Dropping a copy leaves the original intact
See:
https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy
I assume the `drop` was used to silence a 'unused variable' warning,
so I silenced it by other means.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
To the existing `.alias(item)`, add a
`.aliases(into-item-iter)` similar to how `Extend` works.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
The connection submodule now allows building an "acceptor"
for hyper connections which can either take an explicit ssl
acceptor, or builds a default one with a self signed
certificate.
The rate-limited-stream feature enables a method to
lookup/update rate limiters for connections.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
rest-server can now optionally provide a PeerAddress
implementation for RateLimitedStream by activating its
'rate-limited-stream' feature
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Instead of a ServerAdapter for the index page and
authentication checking (which don't relate to each other),
provide a `.with_auth_handler` and `.with_index_handler`
builder for ApiConfig separately.
Both are optional. Without an index handler, it'll produce a
404. Without an auth handler, an `AuthError::NoData` is
returned.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
instead, allow attaching routers to path prefixes and also
add an optional non-formatting router
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
This is what actually defines the API server after all.
The ApiService trait in between is a hyper impl detail.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
We pulled in proxmox-http with the client feature solely to
implement the `Service` trait on
`SslStream<RateLimitedStream<TcpStream>>`.
All those `Service` impls are the same: provide a peer
address and return an `ApiService`.
Let's put the `peer_addr()` call into a trait and build from
there.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
it's the version currently shipped by bookworm, so let's unify this widely-used
dependency to make bootstrapping easier.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
while these are (currently) only used by a single member each, having *all*
dependency versions specified in the top level Cargo.toml only makes the whole
process of managing them less error-prone.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
so that we no longer have to (or forget to) bump the version in multiple places.
notable changes:
- outdated versions have been unified
- proxmox-metrics -> proxmox-async no longer uses explicit empty features
(proxmox-async doesn't provide any anyway)
- proxmox-subscription -> proxmox-http no longer uses explicit default_features
= false (proxmox-http has an empty default feature anyway)
- missing path dependencies added (mainly proxmox-rest-server)
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
noteworthy changes:
- proxmox-http had a default_features_false dep on hyper, which is dropped (the
default feature is empty anyway)
- hyper, libc, nix, tokio and url versions are unified
- missing (cosmetic) bindgen feature on zstd enabled everywhere
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
no real change for PBS usage - the ApiHandler enum is marked
non_exhaustive now because it has extra values if the new (enabled by
default) "server" feature is enabled.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
these would cause failures when building the sub-crates directly from
their sub-directory.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
There is a race upon reload, where it can happen that:
1. systemd forks off /bin/kill -HUP $MAINPID
2. Current instance forks off new one and notifies systemd with the
new MAINPID.
3. systemd sets new MAINPID.
4. systemd receives SIGCHLD for the kill process (which is the current
control process for the service) and reads the PID of the old
instance from the PID file, resetting MAINPID to the PID of the old
instance.
5. Old instance exits.
6. systemd receives SIGCHLD for the old instance, reads the PID of the
old instance from the PID file once more. systemd sees that the
MAINPID matches the child PID and considers the service exited.
7. systemd receivese notification from the new PID and is confused.
The service won't get active, because the notification wasn't
handled.
To fix it, update the PID file before sending the MAINPID
notification, similar to what a comment in systemd's
src/core/service.c suggests:
> /* Forking services may occasionally move to a new PID.
> * As long as they update the PID file before exiting the old
> * PID, they're fine. */
but for our Type=notify "before sending the notification" rather than
"before exiting", because otherwise, the mix-up in 4. could still
happen (although it might not actually be problematic without the
mix-up in 6., it still seems better to avoid).
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
namely 'StreamingSync' and 'StreamingAsync'
in rest-server by using the new formatter function,
and in the debug binary by using 'to_value'
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
that takes the data in form of a `Box<dyn SerializableReturn + Send>`
instead of a Value.
Implement it in json and extjs formatter, by starting a thread and
stream the serialized data via a `BufWriter<SenderWriter>` and use
the Receiver side as a stream for the response body.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
