authority header is set to empty string

[jkryl]

Please answer these questions before submitting your issue.

What version of gRPC are you using?

v1.9.2

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

go version go1.10.4 linux/amd64

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

linux ubuntu bionic

What did you do?

call DialContext() to connect to gRPC server (on behalf of csc - test utility for k8s CSI)

What did you expect to see?

successful grpc call

What did you see instead?

gRPC server rejects the request because authority header is set to empty string

I'm not a grpc expert so my conclusions below may not be correct.

I'm using tower-grpc rust server and not sure if other servers are so picky as well, but IMHO it does the right thing when rejecting a request with :authority header set to empty string. According to the spec the header SHOULD be set by the client. So that means it doesn't have to be. The approach taken by grpc-go seems to be the worst one. It does not know what to put into the header, but it still sends the header. It should skip the header if it cannot derive a valid content for the header from the input parameters.

The reason why I tripped over this problem was that I tried to use following endpoint with csc program tcp://127.0.0.1:10125 (csc according to the doc can do only tcp or unix schemes). I know I can use insecure flag and explicitly set authority option, which csc does not allow me to do. In any case it should not result into invalid content of authority header. Returning error would also be an option although I would rather prefer not to set the header at all.

[menghanl]

:authority is set to the dial target unless it's overridden explicitly by credentials. The target is parsed as defined in https://github.com/grpc/grpc/blob/master/doc/naming.md, and Endpoint will be used as :authority.

In your example, the target doesn't follow the URI scheme. So target parsing will fail. But :authority should be set to the unparsed string tcp://127.0.0.1:10125.
If I didn't miss anything, this is the behavior at head.

There was a bug in target parsing, fixed in 90dca43. It's only available after release 1.11.0.
Can you update your gRPC and try again? Thanks!

[jkryl]

Thanks for a quick reply! I tried version 1.11.0 but it did not help. Moreover I think that the solution described above (and implemented in the later versions) is not quite correct. http2 servers expect authority to have a form of hostname [+port] (the part of URI between scheme and path). Sending something that is not well understood in authority header can fail the whole request on server side (as it does in my case). Empty string is as bad as tcp://....

The problem seems bigger than just unknown schemes. I tried to use unix:// scheme which should be according to naming.md doc supported. Well, it turns out that at least with version 1.11.0 the authority header is empty again. The scheme is understood by grpc, but it tries to extract authority value using a kind of default algorithm which fails (yields empty string). And what should be the authority value in case of unix domain socket? If we inserted the path to the socket to authority header (i.e. /tmp/sock) it would be wrong because slashes are definitely not expected in authority section of the URI.

I think it boils down to a simple rule of not setting the authority header if we are not sure what the value should be. And in case of unix:// names it applies to all values.

[jkryl]

FYI: in the meantime there has been a good discussion with folks who implement http2 library for rust (hyperium/h2#345). The conclusion is, that sending arbitrary string in authority header is violation of http2 spec and they will not tolerate or implement workarounds for misbehaving clients.

So a short summary based on that is that: grpc-go should be fixed to disable implicit authority header for unix domain socket transport. And in a broader context, for all other transports which valid authority value cannot be figured out for.

[entropitor]

This issue is also causing problems with NodeJS servers (nodejs/node#32326). It seems that because of this issue, it's impossible to write kubernetes CSI drivers in any other language as kubernetes is written in go and thus suffering from this bug.

I think for UDS it makes more sense to set the authority to localhost that is compliant with the spec

[menghanl]

A quick solution is to escape characters in ":authority" (what to escape is TBD, we might do it the way that only certain characters are allowed).

[entropitor]

That would work!

[menghanl]

Escaping ":authority" will take more time for all languages to come to agreement, and there's no explicit spec on how it should work.
For now, we will solve the UDS problem first by set the authority to localhost.

But because of the way UDS works today (the URL is parsed in the dialer, which happens too late), we may need to either add a resolver to handle scheme unix, or hardcode UDS to have authority localhost.

[jkryl]

Setting authority header to localhost would probably work.

@GarrettGutierrez1 : Is there any ETA for when the fix could land in the master branch? The reason I ask is that AFAIK at least two grpc server implementations (nodejs and rust) are impacted by this and will not implement a temporary workaround for this bug. Although it might look like a corner case it has quite a big impact on anyone writing grpc servers in other languages than go in k8s env as a lot of grpc communication happens over UDS in k8s (i.e. CSI plugins). Thanks!

[kate-goldenring]

I am seconding @jkryl 's request for an update, @GarrettGutierrez1. I am running into this issue when using a rust grpc server to communicate with kubelet over UDS. While I am currently using a patch, it would be great to get changes upstreamed.

[menghanl]

There's a pending PR #3730. It currently only handles "unix", and sets authority to "localhost". Please give it a try. Thanks!

[jkryl]

I can confirm that the fix in PR #3730 solves the problem for me 👍

[entropitor]

@menghanl Could this be released soon as well? Given how many other projects have this bug included in their projects and basically blocks all extensions for those applications in different languages

[menghanl]

@entropitor The fix will be included in the upcoming 1.31 release (planned next week).