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