[pbs-devel] [PATCH proxmox-backup] fix #5217: api: send missing header when upgrading to HTTP/2

[Max Carrara <m.carrara@proxmox.com>]

The "Connection: upgrade" header is occasionally, but not strictly,
expected to be included in the response sent by the server when an
upgrade to a different protocol is requested by the client. A detailed
explanation follows below.

Neither RFC 9110 (HTTP Semantics) [0] or RFC 7540 (HTTP/2) [1]
*explicitly state* that the "Connection: upgrade" header must be
included *in the server's response* when a client requests an upgrade
to a different protocol. For clients, however, it is specified [2]:

> A sender of Upgrade MUST also send an "Upgrade" connection option in
> the Connection header field (Section 7.6.1) to inform intermediaries
> not to forward this field.

Yet, the example for a response provided in RFC 9110 [3] does include
the header:

> HTTP/1.1 101 Switching Protocols
> Connection: upgrade
> Upgrade: websocket
>
> [... data stream switches to websocket with an appropriate response
> (as defined by new protocol) to the "GET /hello" request ...]

The example in RFC 7540 [4] also includes the header:

> HTTP/1.1 101 Switching Protocols
> Connection: Upgrade
> Upgrade: h2c
>
> [ HTTP/2 connection ...

This would explain why certain reverse proxies (like Caddy) expect the
header to be included in the response.

Additionally, RFC 9113 [5], which obsoletes RFC 7540 [1], mentions:

> The HTTP/1.1 Upgrade mechanism is deprecated and no longer specified
> in this document. It was never widely deployed, with plaintext
> HTTP/2 users choosing to use the prior-knowledge implementation
> instead.

Caddy therefore does not deviate from the HTTP/1.1 and HTTP/2
specifications; neither to we. I therefore conclude that it's best to
just include the header in this specific response anyway, as no harm
is done by doing so.

This fixes bug #5217 [6] and allows PBS to be deployed behind Caddy.
Also tested with nginx, which still works as expected.

[0]: https://datatracker.ietf.org/doc/html/rfc9110
[1]: https://datatracker.ietf.org/doc/html/rfc7540
[2]: https://datatracker.ietf.org/doc/html/rfc9110#section-7.8-14
[3]: https://datatracker.ietf.org/doc/html/rfc9110#section-7.8-13
[4]: https://datatracker.ietf.org/doc/html/rfc7540#section-3.2
[5]: https://datatracker.ietf.org/doc/html/rfc9113#appendix-B-2.3
[6]: https://bugzilla.proxmox.com/show_bug.cgi?id=5217

Signed-off-by: Max Carrara <m.carrara@proxmox.com>
---
 src/api2/backup/mod.rs | 3 ++-
 src/api2/reader/mod.rs | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index 18fad745..013043dd 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -3,7 +3,7 @@
 use anyhow::{bail, format_err, Error};
 use futures::*;
 use hex::FromHex;
-use hyper::header::{HeaderValue, UPGRADE};
+use hyper::header::{HeaderValue, CONNECTION, UPGRADE};
 use hyper::http::request::Parts;
 use hyper::{Body, Request, Response, StatusCode};
 use serde::Deserialize;
@@ -318,6 +318,7 @@ fn upgrade_to_backup_protocol(
 
         let response = Response::builder()
             .status(StatusCode::SWITCHING_PROTOCOLS)
+            .header(CONNECTION, HeaderValue::from_static("upgrade"))
             .header(
                 UPGRADE,
                 HeaderValue::from_static(PROXMOX_BACKUP_PROTOCOL_ID_V1!()),
diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
index b1a5612b..42b42838 100644
--- a/src/api2/reader/mod.rs
+++ b/src/api2/reader/mod.rs
@@ -3,7 +3,7 @@
 use anyhow::{bail, format_err, Error};
 use futures::*;
 use hex::FromHex;
-use hyper::header::{self, HeaderValue, UPGRADE};
+use hyper::header::{self, HeaderValue, CONNECTION, UPGRADE};
 use hyper::http::request::Parts;
 use hyper::{Body, Request, Response, StatusCode};
 use serde::Deserialize;
@@ -209,6 +209,7 @@ fn upgrade_to_backup_reader_protocol(
 
         let response = Response::builder()
             .status(StatusCode::SWITCHING_PROTOCOLS)
+            .header(CONNECTION, HeaderValue::from_static("upgrade"))
             .header(
                 UPGRADE,
                 HeaderValue::from_static(PROXMOX_BACKUP_READER_PROTOCOL_ID_V1!()),
-- 
2.39.2