Deny requerst if :authority field is invalid only with CONNECT method

[arthurlm]

Looking at RFCs:

1. RFC 7540: HTTP V2@section 8.1.2.3. Request Pseudo-Header Fields.

The ":authority" pseudo-header field includes the authority
portion of the target URI ([RFC3986], Section 3.2). The authority
MUST NOT include the deprecated "userinfo" subcomponent for "http"
or "https" schemed URIs.

All HTTP/2 requests MUST include exactly one valid value for the
":method", ":scheme", and ":path" pseudo-header fields, unless it is
a CONNECT request (Section 8.3). An HTTP request that omits
mandatory pseudo-header fields is malformed (Section 8.1.2.6).

2. RFC 3986: Uniform Resource Identifier@section 3.2. Authority

Many URI schemes include a hierarchical element for a naming
authority so that governance of the name space defined by the
remainder of the URI is delegated to that authority (which may, in
turn, delegate it further).

The authority component is preceded by a double slash ("//") and is
terminated by the next slash ("/"), question mark ("?"), or number
sign ("#") character, or by the end of the URI.

Non-validating parsers (those that merely separate a URI reference into
its major components) will often ignore the subcomponent structure of
authority, treating it as an opaque string from the double-slash to
the first terminating delimiter, until such time as the URI is
dereferenced.

3. RFC 7540: HTTP V2@section-8.3. The CONNECT Method

In HTTP/2, the CONNECT method is used to establish a tunnel over a
single HTTP/2 stream to a remote host for similar purposes. The HTTP
header field mapping works as defined in Section 8.1.2.3 ("Request
Pseudo-Header Fields"), with a few differences. Specifically:

o The ":authority" pseudo-header field contains the host and port to
connect to (equivalent to the authority-form of the request-target
of CONNECT requests (see [RFC7230], Section 5.3)).

A CONNECT request that does not conform to these restrictions is
malformed (Section 8.1.2.6).

So, from my understanding:

• Following (1):

  • :authority field is not mandatory in HTTP2.
  • It should be a valid URI without scheme and userinfo.

• Following (2):

  • URI can include sub path component (even if / character is not expected for regular authority).
  • There are HTTP2 clients in the wild that might send "invalid" :authority. It should be treated as opaque string.

• Following (3):

  • :authority field should be a valid host and port in the case of CONNECT method.

NOTE 1: I have not read the whole RFCs but just read in details the mentioned sections. If anyone have better understanding of this RFCs, please feel free to comment / edit this PR 😉 !

NOTE 2: Please have a look at my first comment about k8s usage with tonic for more details.

---

This change could fix a lot of already referenced issues / PRs using h2 and gRPC / tonic.

• malformed authority error with HTTP2 and gRPC #525
• Support for h2 over UDS #442
• Do not require authority for UNIX domain sockets #487
• malformed authority header for ExtensionService projectcontour/contour#4278

It will also:

• Improve support for gRPC call using go clients
• Allow people to develop nice k8s extension with tonic (which is not possible for now without forking h2)
• Help h2 to better fit to HTTP2 RFC specification 😁

[bachp]

This makes it possible to use h2 with k8s.

[bachp]

@seanmonstar do you see any issue with this change?

[Forsworns]

Any update on this one? Just met the same problem when communicate with k8s via tonic :(

[tiagolobocastro]

Any update on this? Would be good to stop using a fork :)

[mythi]

Any update on this one? Just met the same problem when communicate with k8s via tonic :(

k8s 1.26 kubelet sets localhost authority for device-plugin and CSI.

[arthurlm]

Even if recent version of k8s set localhost for :authority field:

• not everyone has access to latest version of k8s (and in big company it can takes years)
• in cloud as service provider (EKS for example) latest version of k8s is still 1.24
• h2 still does not respect HTTP2 RFC without this change

I do not really understand why maintainers do not want to review / merge / close this PR.
I guess it is probably dues to some lack of time or PR visibility / interest.

I still hope this will be merged or discussed one day 😄.
So this is why I have not closed this for now...

[mythi]

@arthurlm FWIW I agree with you. Just wanted to share what I know k8s kubelets can do today.