cloudflare / cloudflare/pingora

HTTP/2 multiple Cookie headers not concatenated when proxied to HTTP/1.1 upstream (RFC 9113 §8.2.3)

Open
#892 5 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
27.4k
Forks
1.7k
Avg merge
6h 22m
Merged PRs (30d)
3

Description

### Summary

When Pingora receives an HTTP/2 request containing multiple `Cookie` header fields (which is valid per RFC 9113 §8.2.3) and proxies it to an HTTP/1.1 upstream, the individual `Cookie` headers are forwarded as separate header lines instead of being concatenated into a single header using `"; "` as the delimiter.

This violates RFC 9113 §8.2.3, which states:

> If there are multiple Cookie header fields after decompression, these **MUST** be concatenated into a single octet string using the two-octet delimiter of `0x3B, 0x20` (the ASCII string `"; "`) before being passed into a non-HTTP/2 context, such as an HTTP/1.1 connection...

### Reproduction

**Environment:**
- Pingora proxy with H2 enabled on the downstream side
- HTTP/1.1 upstream backend

**Steps:**

1. Configure a Pingora-based proxy accepting HTTP/2 from clients
2. Set up an HTTP/1.1 backend that logs or inspects raw request headers
3. Send an HTTP/2 request with multiple `Cookie` header fields:

```
# Using nghttp to send separate cookie headers over HTTP/2
nghttp -v -H "cookie: session_id=abc123" -H "cookie: preferred_language=en" https://proxy-host/path
```

4. Observe the headers received by the HTTP/1.1 upstream

**Expected behavior:**

The upstream receives a single `Cookie` header:
```
Cookie: session_id=abc123; preferred_language=en
```

**Actual behavior:**

The upstream receives two separate `Cookie` headers:
```
Cookie: session_id=abc123
Cookie: preferred_language=en
```

### Real-world impact

Modern browsers (Chrome, Firefox, Safari) routinely split cookies into multiple `cookie` header fields when communicating over HTTP/2, as permitted by RFC 9113 §8.2.3 for improved HPACK compression.

Many HTTP/1.1 backend frameworks only read the **first** `Cookie` header, causing session cookies to be silently dropped. A concrete example:

- **GitLab** (Rails): When proxied through an H2-accepting reverse proxy, the `_gitlab_session` cookie is lost if it arrives in a separate header line from other cookies. This causes CSRF token verification to fail, resulting in **HTTP 422** on all state-changing requests (form submissions, API calls with CSRF protection).

This affects any backend that does not handle multiple `Cookie` headers, which is the majority of HTTP/1.1 implementations since RFC 6265 §5.4 specifies that the user agent "MUST NOT attach more than one header field named Cookie."

### Workaround

Disable HTTP/2 on the downstream side so clients send cookies as a single header line (HTTP/1.1 behavior).

### Suggested fixes

#### Fix A: Cookie header concatenation (RFC-compliant)

The concatenation should occur in the H2-to-H1.1 proxy path. Based on code review, the most appropriate location appears to be:

- **`pingora-proxy/src/proxy_h1.rs`** — in `proxy_to_h1_upstream()`, before writing headers to the upstream connection
- Alternatively, **`pingora-http/src/lib.rs`** — as a `RequestHeader` method (e.g., `concatenate_cookie_headers()`) callable from the proxy layer

The logic would be:
```rust
// Pseudocode
if downstream_is_h2 && upstream_is_h1 {
let cookies: Vec<&[u8]> = request.headers.get_all("cookie")
.iter()
.map(|v| v.as_bytes())
.collect();
if cookies.len() > 1 {
let merged = cookies.join(b"; ");
request.headers.remove("cookie");
request.headers.insert("cookie", merged);
}
}
```

#### Fix B: ALPN-based protocol matching (avoids conversion entirely)

An alternative architectural approach is to **eliminate the H2→H1.1 conversion** by matching client-side and upstream-side protocols via ALPN negotiation. We used this strategy in a production MITM proxy and it eliminates an entire class of header translation bugs, not just cookies.

The approach mirrors Chromium's `RequiresHTTP11()` mechanism:

```
┌──────────────────────────────────────┐
│ Protocol Cache │
│ domain → { Http2 | Http11Only | │
│ AlpnFailed | 421 } │
└──────────┬───────────────────────────┘

Client ──TLS──► Proxy ──TLS──► Upstream
ALPN ALPN
│ │
▼ ▼
┌─────────────────────────────────────────────────┐
│ Step 1: Upstream ALPN → discover protocol │
│ Step 2: If mismatch → cache + GOAWAY/close │
│ Step 3: Client reconnects │
│ Step 4: Check cache → force client ALPN │
│ Step 5: Both sides same protocol → no convert │
└─────────────────────────────────────────────────┘
```

**Key implementation details:**

1. **Protocol cache** — Per-domain cache storing upstream protocol support. Entries track `AlpnFailed`, `Http11Only`, `MisdirectedRequest` (421), with TTL-based expiry and failure threshold counting.

2. **Client ALPN enforcement** — Before the client TLS handshake, check the protocol cache. If the upstream is known to be H1.1-only, **advertise only `http/1.1` in the client-side ALPN** so the browser never negotiates H2 for that domain.

3. **Mismatch recovery** — When a mismatch is detected (client H2 but upstream H1.1):
- Record the failure in the protocol cache
- Terminate the connection (H2: GOAWAY frame, H1.1: close)
- Browser automatically reconnects → cache forces H1.1 ALPN → both sides match

4. **Strict match enforcement** — The proxy only operates in matched-protocol mode:
```rust
match (client_protocol, upstream_protocol) {
(Http2, Http2) => { /* H2 pass-through: headers forwarded as-is */ }
(Http11, Http11) => { /* H1.1 pass-through: headers forwarded as-is */ }
_ => { /* Should not reach here — handled by ALPN logic */ }
}
```

**Advantages:**
- Eliminates *all* H2↔H1.1 translation bugs (not just Cookie, but also pseudo-header stripping, Transfer-Encoding conflicts, etc.)
- Zero-cost at steady state (cache hit → correct ALPN from the start)
- First-request penalty is one extra round-trip (connection drop + reconnect), amortized by cache

**Tradeoffs:**
- Domains that only support H1.1 will always use H1.1 end-to-end (no H2 multiplexing benefit between client and proxy)
- Requires per-domain protocol cache management (TTL expiry, failure counting)

We validated this approach against 50+ production sites (banking, e-commerce, SaaS) including the GitLab CSRF scenario described above, with zero cookie-related failures after deployment.

### Prior art

This is a known issue across the HTTP proxy ecosystem:

| Project | Issue | Status |
|---------|-------|--------|
| hyperium/hyper | [#2528](https://github.com/hyperium/hyper/issues/2528) | Open (2021) |
| python-hyper/h2 | [#497](https://github.com/python-hyper/h2/issues/497) | **Fixed** (added `normalize_inbound_headers`) |
| HAProxy | [discourse thread](https://discourse.haproxy.org/t/h2-cookie-header-splitted-header/1742) | Workaround via Lua |
| Varnish Cache | [#2291](https://github.com/varnishcache/varnish-cache/issues/2291) | Manual `std.collect()` |
| imbolc/tower-cookies | [#8](https://github.com/imbolc/tower-cookies/issues/8) | Fixed |
| yesodweb/wai (Haskell) | [#486](https://github.com/yesodweb/wai/issues/486) | Open |
| httpwg/http-extensions | [#2541](https://github.com/httpwg/http-extensions/issues/2541) | RFC-level discussion |

### Note

Pingora v0.4.0 already addressed the reverse direction — `Set-Cookie` response header casing during H2→H1.1 downgrade ([changelog](https://github.com/cloudflare/pingora/blob/main/CHANGELOG.md)). This issue covers the complementary request-direction `Cookie` header concatenation.

### References

- [RFC 9113 §8.2.3 — HTTP/2 Header Field Ordering](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.3)
- [RFC 7540 §8.1.2.5 — Compressing the Cookie Header Field](https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2.5) (predecessor)
- [RFC 6265 §5.4 — The Cookie Header](https://www.rfc-editor.org/rfc/rfc6265#section-5.4)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.