tempesta icon indicating copy to clipboard operation
tempesta copied to clipboard

Unify HTTP/1 `req->host` and HTTP/2 `:authority` processing

Open krizhanovsky opened this issue 3 years ago • 2 comments

At the moment we store URI authority part in req->host for HTTP/1, but we have a designated TFW_HTTP_HDR_H2_AUTHORITY special header for HTTP/2 for :authority. This complicates the logic (e.g. see https://github.com/tempesta-tech/tempesta/pull/1629#discussion_r880487824 discussion). Need to store URI authority for HTTP/1 in the special header, just as we do this for HTTP/2, and adjust all the places processing req->host.

No need any special testing - these changes are well covered by the existing test suite.

krizhanovsky avatar May 24 '22 13:05 krizhanovsky

Current issue may contribute to https://github.com/tempesta-tech/tempesta/issues/1667

const-t avatar Aug 10 '22 14:08 const-t

Http2 pseudo-headers must be matched by HTTP Tables rules e.g hdr :authority == "test.com" -> test_host;. After unifying the processing logic need to ensure, that Authority(Authority from URI for HTTP), Host, Forwarded headers are processes in such order for H2 and H1.

Also we should to consider to rework our Host header parsing logic to satisfy this rule from RFC 7230 5.4:

If the target URI includes an authority component, then a client MUST send a field-value for Host that is identical to that authority component

const-t avatar Aug 25 '22 16:08 const-t

If send h2 request with host: (empty), we will receive response status code - 400 and Warning: failed to parse request. #1705 hides warning. But http 1.1 request will be processed successfully.

RomanBelozerov avatar Nov 08 '22 11:11 RomanBelozerov

Please also fix https://github.com/tempesta-tech/tempesta/wiki/HTTP-tables

host - the host part from URI in HTTP request line, or the value of "Host" header field or value of "host" parameter from "Forwarded" header field. The Host part in URI takes priority over the Host header field value and "host" parameter from "Forwarded" header.

Which makes false statement about URI priority over Host header value and does not consider http2. See better description for http_host_required in https://github.com/tempesta-tech/tempesta/wiki/HTTP-security

Also please make Frang loaded by default to enforce default enabled http_host_required. Probably we need to update all frang tests for this.

krizhanovsky avatar Apr 07 '23 14:04 krizhanovsky

Fixed in #1862

dmpetroff avatar May 11 '23 15:05 dmpetroff