Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

malformed authority header for ExtensionService #4278

Closed
cortopy opened this issue Jan 19, 2022 · 12 comments · Fixed by #4587
Closed

malformed authority header for ExtensionService #4278

cortopy opened this issue Jan 19, 2022 · 12 comments · Fixed by #4587
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor.
Milestone

Comments

@cortopy
Copy link

cortopy commented Jan 19, 2022

What steps did you take and what happened:

I wrote an AuthorizationServer using Rust. In Rust, the main package to handle http2 connections is called h2, which happens to be a very strict implementation of the http2 protocol.

I then registered my server as an ExtensionService

When Envoy makes a request, the authority header is set to:

"extension/{{ namespace }}/{{ ExtensionService name }}"

That is an invalid authority header according to the specs. As a result, h2 refuses to handle the request as reported here a while ago

What did you expect to happen:

A valid authority header is sent to the server. Maybe only the ExtensionService name, or the virtual host fqdn?

Anything else you would like to add:

I've tried with h2 and h2c protocols without any diffrence.

I also tried not providing a protocol to see if maybe a http1 request could be handled (Rust has utilities already to convert http1/grpc-web on the fly and it could work) but, in this case, no protocol is set and the server doesn't know what to do with the TCP request

Environment:

  • Contour version: 1.19.1
  • Kubernetes version: (use kubectl version): 1.23
  • Kubernetes installer & version: k0s
  • Cloud provider or hardware configuration: bare metal
  • OS (e.g. from /etc/os-release): Ubuntu
@cortopy cortopy added kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Jan 19, 2022
@sunjayBhatia
Copy link
Member

It is likely Envoy is using the cluster name Contour creates (with slashes etc.) as the :authority header, we can probably look at changing that around

@sunjayBhatia
Copy link
Member

I would think this issue may apply to any h2 service, surprising it has only come up now

@cortopy
Copy link
Author

cortopy commented Jan 20, 2022

@sunjayBhatia thanks for looking into this.

I'm also using Contour to expose grpc-web services. I'm using h2c protocol in the HTTPProxy and this works fine even with http2. In what I've tested, this only happens with the slashes in the authority sent by the ExtensionService

@travisghansen
Copy link

travisghansen commented May 2, 2022

If anyone needs a quick/dirty solution to this I wrote this for similar issues (typically involving uds): https://github.com/democratic-csi/csi-grpc-proxy which can be used to simply proxy the request and rewrites (unconditionally essentially) the :authority header to localhost.

EDIT: scratch that, the above project was created to explicitly deal with the uds scenario so has 0 support for ssl/https :(

@sunjayBhatia sunjayBhatia added this to the 1.22.0 milestone May 2, 2022
@sunjayBhatia sunjayBhatia self-assigned this May 2, 2022
@travisghansen
Copy link

Is there an envoy bug we're aware of that has been created to address this issue?

@youngnick
Copy link
Member

@sunjayBhatia would be the one to know here.

@sunjayBhatia
Copy link
Member

the issue shouldn't be an Envoy one, rather how we set up the gRPC service to connect to the auth/rate limit extension services

since we generate/use a cluster name that isn't a valid hostname, we should set the authority field on the gRPC service: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/grpc_service.proto#envoy-v3-api-msg-config-core-v3-grpcservice-envoygrpc

The :authority header in the grpc request. If this field is not set, the authority header value will be cluster_name. Note that this authority does not override the SNI. The SNI is provided by the transport socket of the cluster.

see:

EnvoyGrpc: &envoy_core_v3.GrpcService_EnvoyGrpc{
ClusterName: authzClusterName,
},

Note on SNI/how to set the proper hostname:

  • we do set the SNI on the transport socket upstream TLS context but only when upstream validation is requested, otherwise it is empty
  • see this open issue as well: Auth: add explicit SNI name support for ExtensionService #2893
  • some options on how to set authority field:
    • I would say we should probably prefer to not change the cluster name to minimize possible churn of xDS resources (though this is probably a minimal change) and to maintain the same pattern across all clusters
    • if SNI set, use that
    • normalize cluster name to a valid hostname and set in authority (currently is extension/<extensions-service-namespace>/<extension-service-name>)
    • use normalized backing service name as authority (rather than extension service resource namespace/name)
    • add a new field to extension service to allow hostname configurability
    • any other ideas people have

@travisghansen
Copy link

travisghansen commented May 3, 2022

Forgive the ignorance here (don't really have much experience with Contour), but the :authority header should just be the host being dialed right? SNI is fine but when someone has configured h2c it doesn't help.

To me this value should be whatever is here: https://github.com/travisghansen/external-auth-server/blob/master/examples/contour.yaml#L41 (since I assume that's what's being used to dial)

@sunjayBhatia
Copy link
Member

Contour configures clusters for k8s services (that are not ExternalName services) to use envoys EDS to discover addresses, so Envoy won’t use that DNS name to dial the auth service, rather the address from the services endpoints

But yeah, when using cleartext gRPC we need a valid hostname since we have no SNI to use, that could very well be set to the service name/namespace if we choose

@placydo
Copy link
Contributor

placydo commented Jun 9, 2022

Hey, any update for this one? We are currently using ProjectContour with EAS (both working great - tbh far exceeding our expectations) - but cannot update EAS to newest version because of this.

@sunjayBhatia
Copy link
Member

This is planned for the upcoming release, I’ll be picking this up soon 👍🏽

sunjayBhatia added a commit to sunjayBhatia/contour that referenced this issue Jun 21, 2022
- so that cluster name, which may not be a valid host, is not used
- if SNI is set on ExtensionService, it is passed through as authority
- otherwise, / is replaced with . in extension cluster name to be used
as authority

Fixes: projectcontour#4278

Signed-off-by: Sunjay Bhatia <[email protected]>
@sunjayBhatia
Copy link
Member

see #4587 if you want to leave some feedback or try it out early 👍🏽

sunjayBhatia added a commit that referenced this issue Jun 24, 2022
- so that cluster name, which may not be a valid host, is not used
- if SNI is set on ExtensionService, it is passed through as authority
- otherwise, / is replaced with . in extension cluster name to be used
as authority

Fixes: #4278

Signed-off-by: Sunjay Bhatia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants