Closed Bug 1809893 Opened 1 year ago Closed 1 year ago

Early Hints: wpt invalid-headers-in-early-hints.h2.window.js.ini is failing

Categories

(Core :: Networking: HTTP, defect, P3)

defect

Tracking

()

RESOLVED FIXED
112 Branch
Tracking Status
firefox112 --- fixed

People

(Reporter: manuel, Assigned: manuel)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

No description provided.
Severity: -- → S4
Priority: -- → P3
Whiteboard: [necko-triaged]

This was only a

[preload-with-invalid-as.h2.window.html]
  expected:
    if (os == "android") and fission: [OK, TIMEOUT]
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → INVALID

Oh, I confused it with preload-with-invalid-as.h2.window.js.ini. Content is:

[invalid-headers-in-early-hints.h2.window.html]
  [Early Hints contains invalid header: newline byte]
    expected: FAIL

  [Early Hints contains invalid header: nul byte]
    expected: FAIL

Still needs investigation.

Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee: nobody → manuel
Status: REOPENED → ASSIGNED

HTTP2 RFC7540#10.3

Similarly, HTTP/2 allows header field values that are not valid.
While most of the values that can be encoded will not alter header
field parsing, carriage return (CR, ASCII 0xd), line feed (LF, ASCII
0xa), and the zero character (NUL, ASCII 0x0) might be exploited by
an attacker if they are translated verbatim. Any request or response
that contains a character not permitted in a header field value MUST
be treated as malformed (Section 8.1.2.6). Valid characters are
defined by the "field-content" ABNF rule in Section 3.2 of [RFC7230].

In section 8.1.2.6:

Clients MUST NOT accept a malformed response.

I don't read in here that we as a client should close the http2 stream with error PROTOCOL_ERROR. I read that we should not accept the 103 response and must not process the link headers in there. It kinda depends on what "the response" refers to. Either the "103 Early Hints" response containing the header or everything within that http2 stream.

in the previous passage it describes how intermediaries should handle http errors:

Intermediaries that process HTTP requests or responses (i.e., any
intermediary not acting as a tunnel) MUST NOT forward a malformed
request or response. Malformed requests or responses that are
detected MUST be treated as a stream error (Section 5.4.2) of type
PROTOCOL_ERROR.

But I think that doesn't apply to us, because we aren't an intermediary.

It looks like the test expects us to terminate the stream.

Also from https://httpwg.org/specs/rfc8297.html#rfc.section.2

Aside from performance optimizations, such evaluation of the 103 (Early Hints) response's header fields MUST NOT affect how the final response is processed.

The current parse function handling invalid characters in header values from Bug 1511181 is based on http2-draft8, where it just says

In particular, header field
names or values that contain characters not permitted by HTTP/1.1,
including carriage return (U+000D) or line feed (U+000A) MUST NOT be
translated verbatim, as stipulated in [HTTP-p1], Section 3.2.4.

The current implementation does transform them to space character. Which is a correct implementation for that draft.

https://searchfox.org/mozilla-central/rev/d85572c1963f72e8bef2787d900e0a8ffd8e6728/netwerk/protocol/http/Http2Compression.cpp#533-541

// Look for CR OR LF in value - could be smuggling Sec 10.3
// can map to space safely
for (const char* cPtr = value.BeginReading();
     cPtr && cPtr < value.EndReading(); ++cPtr) {
  if (*cPtr == '\r' || *cPtr == '\n') {
    char* wPtr = const_cast<char*>(cPtr);
    *wPtr = ' ';
  }
}

But the final rfc tells a different story as laid out above. We should close the stream with the error PROTOCOL_ERROR.

Attachment #9318964 - Attachment description: Bug 1809893 - Treat invalid characters in header values as protocol violation r=#necko → Bug 1809893 - http2: treat invalid characters in header values as protocol violation r=#necko
See Also: → 1663836
Pushed by mbucher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02e4916231d6
http2: treat invalid characters in header values as protocol violation r=necko-reviewers,valentin,kershaw
See Also: → 1818483
Status: ASSIGNED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: