authority header set to empty string

[edrevo]

What version of gRPC are you using?

1.31.1

What version of Go are you using (go version)?

go version go1.15 linux/amd64

What operating system (Linux, Windows, …) and version?

Ubuntu 20.04

What did you do?

I am writing server that communicates with the dockerfile frontend in buildkit. This client/server communication is quite atypical since it doesn't happen through TCP or UDS, but instead it happens directly through stdio.

The dockerfile frontend (which uses the go-grpc lib) is sending an HTTP2 request with an empty authority header, and the server, which is implemented using h2 is (rightfully) rejecting it since an empty string isn't a valid authority.

This issue was already opened in #2628 but the fix involved was special-casing the UDS code path, which doesn't cover my scenario.

What did you expect to see?

A valid authority header. I don't really care about the content as long as it is RFC-compliant. In my case, I have verified that I am still hitting the following code path:

https://github.com/grpc/grpc-go/blob/master/clientconn.go#L276

Which returns an empty string. I have been able to fix it locally by adding the following code, but I am not knowledgeable about Go, GRPC or HTTP2 so I have no idea whether the change is a good idea or not:

	} else {
		// Use endpoint from "scheme://authority/endpoint" as the default
		// authority for ClientConn.
		cc.authority = cc.parsedTarget.Endpoint
		if cc.authority == "" {
			cc.authority = "default"
		}
	}

What did you see instead?

An empty authority header:

[2020-08-28T16:16:14Z DEBUG h2::server] malformed headers: malformed authority (b""): empty string

[GarrettGutierrez1]

Hello @edrevo, can you tell me what the value of cc.target is at this line?

[edrevo]

Hi @GarrettGutierrez1, thanks for taking a look at this issue. I'll try to get the exact value, but it is hard for me since I have no knowledge of Go (I woulnd't even know my way around changing the log level in grpc-go, for example) and the stdin/out is used for the grpc communication and it is not very easy to access the stderr of the dockerfile frontend.

I guess my point is that even if we find out the value in this case, the grpc-go project should always send RFC compliant headers and the current code doesn't guarantee it since cc.parsedTarget.Endpoint can be the empty string, so there should be a last-resort guard in the code to prevent sending requests that a server library will outright reject even before giving control to the developer using that lib. Basically, I'm arguing that any compliant value for that header is better than the empty string, even if it is a wrong value, because it will be "less wrong" than the empty string.

[edrevo]

println(cc.target) prints an empty string in the line you requested

[edrevo]

any updates on this @GarrettGutierrez1? i basically have no other workaround to this bug than forking buildkit and grpc-go. Would a PR that sets the authority to localhost as a last line of defense be welcome?

[GarrettGutierrez1]

Hi @edrevo. Just to double check, can you tell me the value of target at this line? If Dial is being called with an empty target that may be an issue.

[edrevo]

target is also empty at that line

[GarrettGutierrez1]

It looks like in your case, the authority is being set to the target, which is the empty string. An empty string for the target could be handled more gracefully by gRPC, but in the meantime it should work to set the target to something like localhost, this will prevent you from having to fork grpc-go to work around the issue.

[edrevo]

@dfawley, I see this issue has been marked as "Requires Reporter Clarification". Can you be more explicit as to what clarification is needed?

[dfawley]

The target string determines the authority directly. In this case, you have it set to the empty string. If you instead dial with the target set to "localhost" or "stdio", etc, you should see the authority set to the corresponding value. Since you are using a custom dialer to connect to stdio, the value of the target field is otherwise unimportant in your usage. Please give that a try and let us know if that fixes things for you.

[edrevo]

Unfortunately, I am not the owner of the buildkit project, which is part of moby/Docker. I'll open a bug on their side to see if they can use a non-empty target.

However, I still think grpc-go should always be RFC-compliant, which means one of two options:

1. Return an error if a client uses an empty target
2. Handle the empty target in a way such that the authority header complies with the spec

Do you agree with that?

[dfawley]

Unfortunately, I am not the owner of the buildkit project, which is part of moby/Docker. I'll open a bug on their side to see if they can use a non-empty target.

Can you make the change locally and confirm this fixes the problem for you? I strongly suspect it would but would like to be certain.

Do you agree with that?

I'm unsure exactly what to do at this point; more research is needed.

RFC7540 says:

Clients that generate HTTP/2 requests directly SHOULD use the ":authority" pseudo-header field instead of the Host header field.

I don't see anything in RFC7540 that indicates the field is required to be non-empty. Regarding the Host header field, RFC7230 says:

If the authority component is missing or undefined for the target URI, then a client MUST send a Host header field with an empty field-value.

To me this seems to indicate what we're doing is actually right; if the Dial target is the empty string, then no authority was provided, which would be encoded in the Host header as the empty string in HTTP1, but since this is HTTP/2, we put an empty string value in the :authority header instead. I'm not sure what reference you are using that states :authority must be non-empty, but if you could share it here it would be helpful.

For reference, the related h2 issue is here: hyperium/h2#345

[edrevo]

The HTTP/2 spec states (https://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2.3):

The :authority pseudo-header field includes the authority portion of the target URI

So, if the :authority pseudo-header exists, it must be a valid "authority portion of the target URI". The URI spec says an authority follows the following syntax (https://www.rfc-editor.org/rfc/rfc3986.html#section-3.2):

authority = [ userinfo "@" ] host [ ":" port ]

The "host" section of that same RFC says:

If the URI scheme defines a default for host, then that default
applies when the host subcomponent is undefined or when the
registered name is empty (zero length). For example, the "file" URI
scheme is defined so that no authority, an empty host, and
"localhost" all mean the end-user's machine, whereas the "http"
scheme considers a missing authority or empty host invalid.

This is confirmed if you go back to the HTTP1.1 spec (https://httpwg.org/specs/rfc7230.html#rfc.section.2.7.1):

A sender MUST NOT generate an "http" URI with an empty host identifier. A recipient that processes such a URI reference MUST reject it as invalid.

Which is why, ultimately, h2 is rejecting the requests with an empty :authority

[edrevo]

Can you make the change locally and confirm this fixes the problem for you? I strongly suspect it would but would like to be certain.

I opened a PR to buildkit setting the target to "localhost" and verified that it did fix my problem :)

[dfawley]

It's a bit unclear whether this situation is valid, and would depend upon how you interpret the meaning of "generating" or "processing" an "http URI".

Is "setting :scheme, :authority, and :path headers in an HTTP/2 request" the same as "generating an http URI"? If so, then it is invalid for us to do that. Similarly: is "receiving a set of :scheme/:authority/:path headers in an HTTP/2 request" the same as "processing such a URI reference"? If so, then h2 is correct to reject it.

Per RFC 3986, a URI is a string representation (e.g. "http://host/path") of a resource:

A Uniform Resource Identifier (URI) is a compact sequence of characters that identifies an abstract or physical resource.

Strictly speaking, I'd interpret it as: no URI has been generated or processed. In this case, we are okay to emit these headers and h2 should allow the request.

Practically speaking, I don't see any reason for h2 to reject requests with an empty ":authority" header, given that a semantically-equivalent request can easily be sent (next paragraph), and h2 would accept that. So, even if you argue that "receiving three headers" is the same as "processing a URI", there appears to be no value to rejecting these requests.

Our client could sidestep the spec ambiguity by omitting ":authority" if it would be empty, and setting the "Host" header to the empty string instead. This is explicitly allowed by the spec to support HTTP/1.0 interoperability.

IMO h2 should treat an empty ":authority" header the same way as a missing ":authority" header and an empty "Host" header. This is what the golang http2 server does, and what I believe most other HTTP/2 servers do.

...

There may also be a client API problem. Allowing name resolvers to emit empty addresses seems invalid. How can the transport be expected to connect when there is no address provided? For your client, the custom dialer just ignores the address (this usage is also a bit unusual). That said, it's hard to change any of this, as it will break existing users, so there may be nothing we can do here but improve the documentation.

[edrevo]

Many thanks for the detailed explanation @dfawley. I understand your position and your interpretation of the spec. I don't think h2 is the only other HTTP/2 server that interprets that an empty authority is invalid, since issue #3730 also seemed to affect Typescript-based servers too.

Let me ping @seanmonstar (the maintainer of the H2 project): Sean, would you be open to allow empy strings in the authority header in h2? Whatever the resolution for this issue is, I think it would be good to have an interop story between grpc-go and h2 that works.

[seanmonstar]

An empty string does seem to fit the spec, that'd be fine. I think the linked h2 issue describes when the :authority was a file path, which is different from this. Happy to merge a fix!

[edrevo]

Great! Closing this issue then. I'll open a PR to h2 asap.

[dfawley]

Cool, thanks @seanmonstar.

Reopening as I believe @GarrettGutierrez1 still has some related work in progress about this. We might still want to avoid the questionable situation by doing what I suggested above (omit :authority; set Host as empty).

[dfawley]

I think the Host header is not necessary here at this time, since it sounds like there aren't any other problems with our current :authority handling. We can certainly revisit this decision in the future if necessary.