Missing content-security-policy header in response using http2 client

[ruizb]

Version

18.13.0

Platform

Darwin XXX 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000 arm64 arm Darwin

(it also happens on Ubuntu 22.04)

Subsystem

http2

What steps will reproduce the bug?

Create a http2 client and perform a request to an endpoint that sets a content-security-policy HTTP header.

const http2 = require("node:http2");

const session = http2.connect("https://plantview.i.mercedes-benz.com");

session.on("error", console.error);

const req = session.request({ ":path": "/" });
req.end();

req.on("response", (headers) => {
  // should display true
  console.log(Object.keys(headers).includes("content-security-policy"));
});

req.on("data", (data) => {});

req.on("end", () => {
  session.close();
});

How often does it reproduce? Is there a required condition?

It happens consistently.

What is the expected behavior? Why is that the expected behavior?

The content-security-policy header should be present in the headers of the response event.

What do you see instead?

The content-security-policy header is missing.

Additional information

The header is visible on Node.js 18.12.1 and missing as of Node.js 18.13.0.

It works fine using node:https:

Reproduction example

```js const https = require("node:https"); const req2 = https.request("https://plantview.i.mercedes-benz.com"); req2.end(); req2.on("response", (req) => { console.log( "https", Object.keys(req.headers).includes("content-security-policy") ); }); req2.on("data", (data) => {}); req2.on("end", () => { session.close(); }); ```

[targos]

I confirm, compared to curl --http2 -I https://plantview.i.mercedes-benz.com, it's the only missing header.

@nodejs/http2

[targos]

Regression happened between Node.js 19.2.0 and 19.3.0. Diff: https://github.com/nodejs/node/compare/v19.2.0...v19.3.0 It contains an update of nghttp2: https://github.com/nodejs/node/commit/60c9ac5178a85ea24efb5fa871ea20426000351f

[targos]

It seems that the header is considered invalid by nghttp2 because our callback for https://nghttp2.org/documentation/nghttp2_session_callbacks_set_on_invalid_header_callback2.html is called (and we ignore invalid headers).

[targos]

This was introduced by https://github.com/nghttp2/nghttp2/pull/1792 I confirm that the following diff reverts to the old behavior:

diff --git a/src/node_http2.cc b/src/node_http2.cc
index 0d0faaaa75..9da13ac158 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -119,6 +119,8 @@ Http2Options::Http2Options(Http2State* http2_state, SessionType type) {
   // are required to buffer.
   nghttp2_option_set_no_auto_window_update(option, 1);

+  nghttp2_option_set_no_rfc9113_leading_and_trailing_ws_validation(option, 1);
+
   // Enable built in support for receiving ALTSVC and ORIGIN frames (but
   // only on client side sessions
   if (type == NGHTTP2_SESSION_CLIENT) {

Leaving it to @nodejs/http2 now.

[tobyfoo]

Thanks for following up on this, @ruizb and @targos.

FYI we fixed the malformed CSP being served from the URL you quoted in the initial issue description above - There was a leading and a trailing whitespace in the CSP header value.

To continue reproducing this issue I've put this URL together, https://d3di3buep9fpzy.cloudfront.net/default/csp-malformed-reply and will leave it online until this issue is resolved.

HTH and thanks Toby

[mcollina]

I don't think this is a regression. That header is invalid, so we are filtering it out. IMHO it's the correct behavior. I don't think there is anything to fix.

@ShogunPanda @ronag wdyt? allowing that seems a potential vulnerability vector, and as such it's better to keep it filtered.

[ShogunPanda]

I agree. Let's follow the RFC!

[targos]

We may want to add an option to let the user set the lenient flag, especially when other software like cURL (it also uses nghttp2) work.

[mcollina]

Agreed