HTTP2 authority header is not set to resolver.Address.ServerName (?)

[fho]

Hello,

according to the godoc and this comment,
the HTTP2 authority header field is set to the resolver.Address.ServerName value that is returned by a resolver.
I can not reproduce this with grpc-go 1.38.0 nor with v1.39.0-dev.0.20210603231021-5c164e2b8f22.

If I understand the code correctly, the http2 authority header is set here to cc.authority and cc.authority is set depending on the parameters of DialContext.
Therefore HTTP2 authority field is never set to resolver.Address.ServerName.

Do I understand something wrong or is this a bug?

(When I set a custom authority via the grpc.WithAuthority() dial option, the authority is set accordingly.)

Some more context, why this became an issue for me:
I'm maintaining a consul grpc-go resolver.
The resolver accepts a custom URL via grpc.Dial() in the format: consul://[<consul-server>]/<serviceName>[?<OPT>[&<OPT>]...].
It queries a consul-server for the addresses of serviceName and returns it addresses.
The resolver was only setting the resolver.Address.Addr, not resolver.Address.ServerName.
When the GRPC-Requests are send out the HTTP2 authority header field is set to a bogus value, to the <serviceName>[?<OPT>[&<OPT>]...] part of the URL. This part is parsed here as authority.
This was not a problem when communicating with grpc-go servers, they seem to ignore the bogus authority value.
When communicating with grpc-server written in Node based on grpc-js 1.3.2, this caused that GRPC-Calls fail with a PROTOCOL_ERROR.
Setting resolver.Address.ServerName in my resolver to the same then resolver.Address.Addr does not have any effect.

[menghanl]

What that godoc says is, the ServerName is used as the authority in the TLS handshake, not set as the :authority header.

It seems the server is complaining because :authority contains invalid characters (one of "?=&,", not sure which).
For target "consul://10.10.0.1:1234/user-service?scheme=https&tags=primary,eu&health=fallbackToUnhealthy"

We have several options:

1. we can escape
   • this was mentioned in authority header is set to empty string #2628 (comment), the decision was postponed at the time, but seems it's time to revisit
2. we can strip the invalid characters (this sounds like a bad idea)
3. we can use the ServerName in the address returned by the resolver
   • this is essentially what proposed by this issue

@dfawley what do you think?

@markdroth @ejona86
What would C and Java send in :authority header if the channel target is "consul://10.10.0.1:1234/user-service?scheme=https&tags=primary,eu&health=fallbackToUnhealthy"?
And what do you think of the options mentioned above?

[dfawley]

Another potential option:

4. RFC-3986 parse the target and strip off query params. (We'd still need to deal with illegal characters, potentially, even with this)

[fho]

Thanks a lot for the fast response.

If I use a dial URL like consul://10.10.0.1:1234/user-service, the authority value that should be set in the http-request is not part of this URL.
Therefore with solutions that are only based on the dial URL (1, 2, 4) we should still end with an invalid http2 authority header.

In my example the resolver (10.10.0.1:1234) would resolve user-service to e.g. host 192.168.0.1.
The grpc/http2-request is sent to http://192.168.0.1, therefore the valid authority value should also be 192.168.0.1.
Or am I misunderstanding something?

[ejona86]

Java and C both let the name resolver convert from the target string to the authority. Does Go not allow the same? Note that it must be a local computation based on the target string; no looking up in a foreign DB. With that approach the Consul name resolver would determine what the authority should be, which appears to be "strip off the query params" in this case.

While I think it'd be an abuse of Address.ServerName to use it in this case (it is a dangerous tool and we should avoid it when possible), I'm surprised to hear the setting doesn't change the :authority on-the-wire. That sounds like a bad idea. I'd have expected that to be a functional workaround.

I am aware of cases where we expect to have "invalid" characters in authority, namely / for some cases internal to Google. So I do think gRPC at some point will need to percent-encode the authority at times for proper compatibility.

It looks like the current behavior of grpc-java disallows percent-encoding when calling channelBuilder.overrideAuthority(String). But I don't see any validation in grpc-java for the authority returned by the NameResolver.

Aside: percent-encoding can "naturally" come up with IPv6 scopes, but we ignore the case. https://datatracker.ietf.org/doc/html/rfc6874#section-2 . Note that Windows interface names have spaces by default, so those also have to be percent encoded. But that's a different discussion because they are supposed to be removed from the host header (although then it seems the server cannot create absolute URLs in the response). https://datatracker.ietf.org/doc/html/rfc6874#section-4

[markdroth]

Yeah, as Eric said, in C-core, the resolver determines what authority to use for a given URI, so it should be totally up to the consul resolver implementation what to do here. However, note that the default behavior in the base class (used if not overridden by an individual resolver implementation) is to use to path part of the URI, stripping off the leading /, if any. Since the path is extracted via RFC-3986 parsing, our default behavior for a URI of consul://10.10.0.1:1234/user-service?scheme=https&tags=primary,eu&health=fallbackToUnhealthy would be an authority of user-service.

[ejona86]

In Java the customary behavior is essentially targetUri.getPath().substring(1), with just an extra check that the path starts with '/'. gRPC passes a pre-parsed URI to the name resolver, so just using the path is easy and the name resolver would have to go through extra work to include the query parameters.

[menghanl]

Thanks for the replies. It seems a main difference is the resolver interface

Java and C both let the name resolver convert from the target string to the authority. Does Go not allow the same?

No, this isn't available in Go. But it isn't too hard to add one.
We can make an optional interface for the resolver to implement, and if it's implemented, the returned string will be used as :authority.

While I think it'd be an abuse of Address.ServerName to use it in this case (it is a dangerous tool and we should avoid it when possible), I'm surprised to hear the setting doesn't change the :authority on-the-wire.

@ejona86 Are you suggesting we should also set :authority to Address.ServerName?
Or you are saying we shouldn't all it to change the authority for handshakes even.
The godoc of this fields does mention the security concern, and ask users to only populate with trusted values.

[ejona86]

While I think it'd be an abuse of Address.ServerName to use it in this case (it is a dangerous tool and we should avoid it when possible), I'm surprised to hear the setting doesn't change the :authority on-the-wire.

Are you suggesting we should also set :authority to Address.ServerName?

Yes, that is what I was suggesting. Generally speaking we shouldn't use an :authority unless we verified the server is authenticated for that authority (e.g., via TLS). C allows per-RPC authority overrides by users and does guarantee that behavior. Java allows per-RPC authority overrides but does not verify the TLS certificate against the name; we consider that a bug/missing feature.

I'm not aware of any case where we need to use an :authority for a host that does not have a certificate for that authority.

[easwars]

With #4817, we changed the way grpc-go parses the target URI and made it more compliant with RFC 3986.

We still haven't added the ability for name resolvers to return authority given the target string, and will probably not be doing that very soon unless we have strong demand for it.

@fho : Could you please let us know if you are still facing the same issue with the latest version of grpc-go. Thank you.

[github-actions]

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

[fho]

@easwars

We don't experience the issue with grpc/grpc-js 1.5.10 and google.golang.org/grpc v1.46.2.

I can't say if #4817 fixed it in particular. I was not able to reproduce the issue with an old internal commit of our repository in a reasonable time.

[easwars]

Thanks for checking @fho.