public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH http-server 1/1] apiserver: send TLS close notify at the end of streamed responses
@ 2026-11-03 15:52 Michael Köppl
  2026-11-03 16:19 ` Michael Köppl
  2026-11-05 17:50 ` applied: " Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Köppl @ 2026-11-03 15:52 UTC (permalink / raw)
  To: pve-devel

A streamed response (e.g. for the single file restore download), has no
Content-Length nor chunked encoding, so the client can only tell where
the body ends by the connection being closed. The response accounced
'Connection: Keep-Alive' and, at the end of the stream, push_shutdown()
shut down the socket directly, bypassing the TLS layer. The client then
receives a TCP FIN without a TLS close notify [0].

Without a close notify, a client cannot tell a complete body from a
truncated one. OpenSSL 3 reports this as an unexpected EOF, which makes
curl fail with error 56 even though all data arrived. Even though
browsers usually accept this, this affects users who might use this for
scripted downloads from endpoints such as the file-restore one.

Do not keep streamed response alive. finish_response() then disconnects
the client once all data is written and, since commit 07e56cc ("fix
unexpected EOF for client when closing TLS session"),
client_do_disconnect() sends a close notify before shutting down the
socket. Drop the push_shutdown() call, as it would shut down the socket
before that if the write buffer is already empty.

[0] https://www.rfc-editor.org/info/rfc9846/#name-closure-alerts

Fixes: 389ad88 ("support streaming data form fh to client")
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
Ran into this while writing an E2E test that downloads a single file
from a PBS backup through the file-restore API endpoint with curl,
which failed with exit code 56 even though the file was complete.
I tested this manually with curl, as well as the GUI file restore. On a
3-node cluster, downloaded a single file from a backup on node 2
proxied through node 1, without and with this patch applied. Node 1
receives the stream completely and without error in both cases.

 src/PVE/APIServer/AnyEvent.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index 449b1c0..9258b6e 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -274,7 +274,6 @@ sub response_stream {
                     # write out any remaining data
                     $reqhdl->push_write($hdl->{rbuf}) if length($hdl->{rbuf}) > 0;
                     $hdl->{rbuf} = "";
-                    $reqhdl->push_shutdown();
                     $self->finish_response($reqstate);
                 }
             };
@@ -366,6 +365,8 @@ sub response {
         $resp->remove_header("Content-Length");
     }

+    $reqstate->{keep_alive} = 0 if $stream_fh;
+
     if ($reqstate->{keep_alive} > 0) {
         $resp->push_header('Connection' => 'Keep-Alive');
     } else {
--
2.47.3





^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH http-server 1/1] apiserver: send TLS close notify at the end of streamed responses
  2026-11-03 15:52 [PATCH http-server 1/1] apiserver: send TLS close notify at the end of streamed responses Michael Köppl
@ 2026-11-03 16:19 ` Michael Köppl
  2026-11-05 17:50 ` applied: " Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Köppl @ 2026-11-03 16:19 UTC (permalink / raw)
  To: Michael Köppl, pve-devel

I was made aware and simply forgot that I should probably give an
example how to reproduce this.

1. Create a container
2. Back it up to PBS
3. The script below covers a simplified form of what the E2E test does.
   Running it as root on a test node, pointing it at the container
   backup like `repro.sh pbs:backup/ct/100/2026-11-03T12:00:00Z` will
   return the data (the container's hostname) successfully downloaded,
   but curl will show error 56.

```
#!/bin/bash
# e.g. repro.sh pbs:backup/ct/100/2026-11-03T12:00:00Z

volid=$1
storage=${volid%%:*}
ticket=$(perl -MPVE::AccessControl -e 'print PVE::AccessControl::assemble_ticket("root\@pam")')
filepath=$(printf '/root.pxar.didx/etc/hostname' | base64 -w0)

curl -sSk -G -H "Cookie: PVEAuthCookie=$ticket" \
    --data-urlencode "volume=$volid" --data-urlencode "filepath=$filepath" \
    "https://localhost:8006/api2/json/nodes/$(hostname)/storage/$storage/file-restore/download"
echo "exit code: $?"
```

Thanks, @Maximiliano for pointing this out.

On Tue Nov 3, 2026 at 4:52 PM CET, Michael Köppl wrote:
> A streamed response (e.g. for the single file restore download), has no
> Content-Length nor chunked encoding, so the client can only tell where
> the body ends by the connection being closed. The response accounced
> 'Connection: Keep-Alive' and, at the end of the stream, push_shutdown()
> shut down the socket directly, bypassing the TLS layer. The client then

[snip]




^ permalink raw reply	[flat|nested] 3+ messages in thread

* applied: [PATCH http-server 1/1] apiserver: send TLS close notify at the end of streamed responses
  2026-11-03 15:52 [PATCH http-server 1/1] apiserver: send TLS close notify at the end of streamed responses Michael Köppl
  2026-11-03 16:19 ` Michael Köppl
@ 2026-11-05 17:50 ` Thomas Lamprecht
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2026-11-05 17:50 UTC (permalink / raw)
  To: pve-devel, Michael Köppl

On Tue, 03 Nov 2026 16:52:23 +0100, Michael Köppl wrote:
> A streamed response (e.g. for the single file restore download), has no
> Content-Length nor chunked encoding, so the client can only tell where
> the body ends by the connection being closed. The response accounced
> 'Connection: Keep-Alive' and, at the end of the stream, push_shutdown()
> shut down the socket directly, bypassing the TLS layer. The client then
> receives a TCP FIN without a TLS close notify [0].
> 
> [...]

Applied, with s/apiserver/response/ for the subject, thanks!

[1/1] apiserver: send TLS close notify at the end of streamed responses
      commit: d4d74a6de412f9a946089cbfccec62b519557c73

Though in local tests the patch alone did not yet make the close notify a
reliable end-of-download signal, so I applied three commits before it:

- c2d96d0: with output still queued at the end of a stream, the close
  notify could only get queued and the following shutdown lost it. curl
  thus still failed with error 56 after receiving a complete download.

- 7ba3802: a disconnect aborting a stream could send a close notify too,
  making a truncated download look complete. That goes back to 07e56cc,
  but clients can only rely on the close notify if it's not sent then.

- 124c146: on a reused connection, a pipelined request could replace the
  source of an active stream, so the client received the other response
  as the body. With push_shutdown() gone, that could now end with a
  clean close notify too.




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-05 17:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 15:52 [PATCH http-server 1/1] apiserver: send TLS close notify at the end of streamed responses Michael Köppl
2026-11-03 16:19 ` Michael Köppl
2026-11-05 17:50 ` applied: " Thomas Lamprecht

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