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

Host header may include port #2276

Closed
james-emerton opened this issue Feb 5, 2020 · 30 comments
Closed

Host header may include port #2276

james-emerton opened this issue Feb 5, 2020 · 30 comments
Assignees
Labels
pinned Issue cannot be marked stale t:bug Something isn't working
Milestone

Comments

@james-emerton
Copy link

Describe the bug
If the Host header provided in a request includes the port, Ambassador responds with a 404 where there is an otherwise valid route.

To Reproduce
Steps to reproduce the behavior:

  1. Setup a Host and Mapping
  2. curl https://myhostname.com will work because curl sends Host: myhostname.com
  3. curl https://myhostname.com -H "Host: myhostname.com:443" will return a 404 with an empty body

Expected behavior
Ambassador should ignore the port portion of the Host header, or at least treat the default ports as equivalent to the absence of a port.

Versions (please complete the following information):

  • Ambassador: AES 1.1.0
  • Kubernetes environment: Digital Ocean Managed Kubernetes
  • Version: 1.16.2-do.3

Additional context
Our client is using Qt's QNetworkAccessManager to send requests, but upon testing Ambassador I have found that requests were failing from our client (but working from browsers, curl, etc.) Initially I assumed this was a bug in Qt, but the RFC states that the Host header may include a port.

@MattSurabian
Copy link

MattSurabian commented Feb 19, 2020

We ran into the same issue and worked around it by using host_regex:true and setting the host pattern like so: "some\.sub\.domain\.tld.*" Also works using . to stand in for the problematic colon: "some\.domain\.tld.9090"

@esomore
Copy link

esomore commented Feb 20, 2020

@MattSurabian how exactly you made it work? I am having the same problem

@stale
Copy link

stale bot commented Apr 20, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue is stale and will be closed label Apr 20, 2020
@jwm
Copy link
Contributor

jwm commented Apr 20, 2020

not stale

@MattSurabian
Copy link

@yokiworks Sorry for the delayed response here, you've probably figured it out or set it down by now. But if you're still wondering here's the mapping object spec as an example of the regex workaround I mentioned above:

  prefix: /
  rewrite: ""
  host: www\.greatwebsite\.com.*
  host_regex: true
  service: great-website-service.master:8080

The UNESCAPED . after com serves as the stand in for the : that will be sent in the host header value and anything that comes after it. The star in the regex ensures that if the host header doesn't include the port number this mapping will still host match.

It should be noted that this work around would also allow host header matching for mutations of the TLD. Specifically, the above example will also host map to www.greatwebsite.computer. This probably isn't an issue in most use cases but it's worth noting as a side effect of this work around.

@stale stale bot removed the stale Issue is stale and will be closed label Apr 20, 2020
@steffen-geissinger-by
Copy link

@MattSurabian which version of ambassador are you using? We tried it with 1.4.2 but could not get it to work. It feels like the regex is only applied to the hostname without the port.

@MattSurabian
Copy link

Been putting off the upgrade to 1.X.X, we're the .8X.0 minor version family still. Was looking to do the migration in a few weeks, sounds like I may be in for at least one surprise when I do. I'd assume the dropped support for this workaround is through Envoy, right?

I'll let you know if I end up with a different work around on the latest version. Sorry for misleading anyone that's running the newer version :-(

@ggwpp
Copy link

ggwpp commented May 19, 2020

Found this problem in 1.4.2 too. With regex it works with only 1 host. If you have multiple host and configure all of them using regex, only 1st host works (Ascending orders).

@fred
Copy link

fred commented May 21, 2020

Same issue with us, exactly same problem. It regular expression it only works with the first host.
version 1.4.2

@jwm
Copy link
Contributor

jwm commented Jun 24, 2020

We upgraded recently to 1.5.3 and hit this. Somewhere between 0.86 and 1.5.3, Ambassador moved from a wildcard domains value in Envoy's configuration to the virtual host's FQDN.

tl;dr

(Some of the TLSContext hostname stuff is handwavey, please feel free to correct me.)

  • I don't see a way explicit port numbers in Mapping.host will ever work with recent Ambassador releases if a matching TLSContext is present, since domains is then taken from the matching TLSContext.host value.
  • I don't see a way to work around this without modifying Ambassador's Envoy config generation code.
  • server_names interpolation should either reject or strip port numbers, since it doesn't make sense to have a port number in an SNI value.

The path toward a fix?

Ideally, the result is that Ambassador implements functionally equivalent logic to envoy#10960, so routing decisions treat an explicit external port as equivalent to implicit-port requests. Then, explicit port requests will work, and do so without a separate Mapping.

It seems the shortest path to that is by:

  • always adding host:externalport to domains
  • always adding an additional match block using host:externalport as the match value.

Figuring out the external port number might be tricky. It looks like some of the k8s service envvars have the external port number (I'm using the :443 HTTPS default), but I'm not sure how universal/portable that assumption is.

Mapping comparison between versions

For example, with these mappings:

---
apiVersion: ambassador/v1
circuit_breakers:
- max_connections: 10000
  max_pending_requests: 10000
connect_timeout_ms: 15000
host: graphql.dev.devoted.com
kind: Mapping
name: orinoco-7dc3eb653-1593020418_graphql_dev_devoted_com
prefix: /
resolver: endpoint
rewrite: /
service: graphql.orinoco-7dc3eb653-1593020418:443
timeout_ms: 1800000
tls: backend-graphql-client

---
apiVersion: ambassador/v1
circuit_breakers:
- max_connections: 10000
  max_pending_requests: 10000
connect_timeout_ms: 15000
host: graphql.dev.devoted.com:443
kind: Mapping
name: orinoco-7dc3eb653-1593020418_graphql_dev_devoted_com:443
prefix: /
resolver: endpoint
rewrite: /
service: graphql.orinoco-7dc3eb653-1593020418:443
timeout_ms: 1800000
tls: backend-graphql-client

Ambassador 0.86 generates this config:

                        "filter_chain_match": {
                            "server_names": [
                                "graphql.prod.devoted.com"
                            ]
                        },
                        "filters": [
                                "config": {
                                    "route_config": {
                                        "virtual_hosts": [
                                            {
                                                "domains": [
                                                    "*"
                                                ],
                                                "name": "backend",
                                                "routes": [
                                                    {
                                                        "match": {
                                                            "case_sensitive": true,
                                                            "headers": [
                                                                {
                                                                    "exact_match": "graphql.prod.devoted.com:443",
                                                                    "name": ":authority"
                                                                }
                                                            ],
                                                            "prefix": "/",
                                                            "runtime_fraction": {
                                                                "default_value": {
                                                                    "denominator": "HUNDRED",
                                                                    "numerator": 0
                                                                },
                                                                "runtime_key": "routing.traffic_shift.cluster_graphql_orinoco_1e5fe20b5_159293-0"
                                                            }
                                                        },
                                                        "route": {
                                                            "cluster": "cluster_graphql_orinoco_1e5fe20b5_159293-0",
                                                            "prefix_rewrite": "/",
                                                            "priority": null,
                                                            "timeout": "1800.000s"
                                                        }
                                                    },
                                                    {
                                                        "match": {
                                                            "case_sensitive": true,
                                                            "headers": [
                                                                {
                                                                    "exact_match": "graphql.prod.devoted.com",
                                                                    "name": ":authority"
                                                                }
                                                            ],
                                                            "prefix": "/",
                                                            "runtime_fraction": {
                                                                "default_value": {
                                                                    "denominator": "HUNDRED",
                                                                    "numerator": 0
                                                                },
                                                                "runtime_key": "routing.traffic_shift.cluster_graphql_orinoco_1e5fe20b5_159293-0"
                                                            }
                                                        },
                                                        "route": {
                                                            "cluster": "cluster_graphql_orinoco_1e5fe20b5_159293-0",
                                                            "prefix_rewrite": "/",
                                                            "priority": null,
                                                            "timeout": "1800.000s"
                                                        }
                                                    },

but the following one with 1.5.3 (domains has changed from * to a FQDN, everything else is the ~same). Since the host:port doesn't match anything in domains, Envoy returns a 404 NR.

                    {
                        "filter_chain_match": {
                            "server_names": [
                                "graphql.dev.devoted.com"
                            ],
                            "transport_protocol": "tls"
                        },
                        "filters": [
                            {
                                "typed_config": {
                                    "route_config": {
                                        "virtual_hosts": [
                                            {
                                                "domains": [
                                                    "graphql.dev.devoted.com",
                                                ],
                                                "name": "ambassador-listener-8443-graphql.dev.devoted.com",
                                                "routes": [

                                                    {
                                                        "match": {
                                                            "case_sensitive": true,
                                                            "headers": [
                                                                {
                                                                    "exact_match": "graphql.dev.devoted.com:443",
                                                                    "name": ":authority"
                                                                },
                                                                {
                                                                    "exact_match": "https",
                                                                    "name": "x-forwarded-proto"
                                                                }
                                                            ],
                                                            "prefix": "/",
                                                            "runtime_fraction": {
                                                                "default_value": {
                                                                    "denominator": "HUNDRED",
                                                                    "numerator": 0
                                                                },
                                                                "runtime_key": "routing.traffic_shift.cluster_graphql_orinoco_0dfd61322_159294-0"
                                                            }
                                                        },
                                                        "route": {
                                                            "cluster": "cluster_graphql_orinoco_0dfd61322_159294-0",
                                                            "prefix_rewrite": "/",
                                                            "priority": null,
                                                            "timeout": "1800.000s"
                                                        }
                                                    },
                                                    {
                                                        "match": {
                                                            "case_sensitive": true,
                                                            "headers": [
                                                                {
                                                                    "exact_match": "graphql.dev.devoted.com",
                                                                    "name": ":authority"
                                                                },
                                                                {
                                                                    "exact_match": "https",
                                                                    "name": "x-forwarded-proto"
                                                                }
                                                            ],
                                                            "prefix": "/",
                                                            "runtime_fraction": {
                                                                "default_value": {
                                                                    "denominator": "HUNDRED",
                                                                    "numerator": 0
                                                                },
                                                                "runtime_key": "routing.traffic_shift.cluster_graphql_orinoco_0dfd61322_159294-0"
                                                            }
                                                        },
                                                        "route": {                                                            "cluster": "cluster_graphql_orinoco_0dfd61322_159294-0",
                                                            "prefix_rewrite": "/",
                                                            "priority": null,
                                                            "timeout": "1800.000s"
                                                        }
                                                    },

Looking to the Envoy upstream for a fix

I found Envoy's troubleshooting guide for cases like this, which backs up the conclusion that domains is the cause.

At first, I thought envoy#10960 (#2818 datawire/envoy#3) would help with the :authority header matching, but:

  • it doesn't address the domains and/or server_names matching
  • it only removes the port number if it matches the Envoy listener the request was received on. Since Ambassador listens on an unprivileged port that often doesn't line up with the externally visible port (on the load balancer, or however traffic gets to Ambassador) the user sees, the port number will never be removed.

Beyond that, every Envoy binary I built segfaulted on startup. I thought Envoy's walk of its protobuf messages on startup caused this because of the protobuf field numbering discontinuity in my backport. Even after fixing that, Envoy still segfaulted with the same backtrace.

Kludging additional values into domains

Since that didn't work out, I started adding graphql.dev.devoted.com:443 to the domains list with this kludge in V2Listener.finalize().

Then, the request is successfully routed:

diff --git python/ambassador/envoy/v2/v2listener.py python/ambassador/envoy/v2/v2listener.py
index 5dbe693b1..1b5b1f28d 100644
--- python/ambassador/envoy/v2/v2listener.py
+++ python/ambassador/envoy/v2/v2listener.py
@@ -914,6 +914,9 @@ class V2Listener(dict):
                 ]
             }

+            if 'graphql' in vhost._hostname:
+                http_config["route_config"]["virtual_hosts"][0]["domains"].append(vhost._hostname + ":443")
+
             filter_chain["filters"] = [
                 {
                     "name": "envoy.http_connection_manager",

server_names and domains gets populated with V2VirtualHost._hostname. When SNI is enabled, I think this value comes from the TLSContext's hosts.

Kludging additional values into domains via TLSContext

Trying a configuration-based approach, adding a host:port (graphql:443) item to the existing TLSContext.hosts (which had contained only graphql) generates the following Envoy config. It doesn't work, since server_names is matching on the SNI value for the incoming request and will never have a port number. Otherwise, it seems this config hunk would work.

                    {
                        "filter_chain_match": {
                            "server_names": [
                                "graphql.staging.devoted.com:443"
                            ],
                            "transport_protocol": "tls"
                        },
                        "filters": [
                            {
                                "name": "envoy.http_connection_manager",
                                "typed_config": {
                                    "route_config": {
                                        "virtual_hosts": [
                                            {
                                                "domains": [
                                                    "graphql.staging.devoted.com:443"
                                                ],
                                                "name": "ambassador-listener-8443-graphql.staging.devoted.com:443",
                                                    {
                                                        "match": {
                                                            "case_sensitive": true,
                                                            "headers": [
                                                                {
                                                                    "exact_match": "graphql.staging.devoted.com:443",
                                                                    "name": ":authority"
                                                                },
                                                                {
                                                                    "exact_match": "https",
                                                                    "name": "x-forwarded-proto"
                                                                }
                                                            ],
                                                            "prefix": "/",
                                                            "runtime_fraction": {
                                                                "default_value": {
                                                                    "denominator": "HUNDRED",
                                                                    "numerator": 0
                                                                },
                                                                "runtime_key": "routing.traffic_shift.cluster_graphql_orinoco_4debe8700_159301-0"
                                                            }
                                                        },
                                                        "route": {
                                                            "cluster": "cluster_graphql_orinoco_4debe8700_159301-0",
                                                            "prefix_rewrite": "/",
                                                            "priority": null,
                                                            "timeout": "1800.000s"
                                                        }
                                                    },

@kflynn
Copy link
Member

kflynn commented Jun 25, 2020

@jwm Thanks for the detailed info!

What I'm thinking of here is

  • allow specifying the port in the Host and/or TLSContext
  • always strip the port in the SNI match specifier

Is that something you could test quickly if we toss a build your way?

@jwm
Copy link
Contributor

jwm commented Jun 25, 2020

@kflynn totally. Thanks for looking into this!

@jhamiltonengineer
Copy link

@jwm I am also facing this issue. I am getting 404's when an external Prometheus stack is trying to scrape a /metrics endpoint that I have behind a mapping. I would also be happy to test.

@sandesh9989
Copy link

I am facing similar issue, i tried workaround with regex it didn't work and getting 404's (working browser and curl). Is there any update on issue

@jorgebsa
Copy link

upgrading ambassador on our production cluster completely broke the monitoring of all our microservices due to this issue, has there been any progress? it seems as if it hasn't been figured out, which means we will probably have to migrate to another solution unfortunately

@matdehaast
Copy link

Note, this has been added as a config on Envoy envoyproxy/envoy#10960.

@kflynn the better approach would be to allow users to specify enabling this flag in the Host file

@kflynn
Copy link
Member

kflynn commented Aug 31, 2020

As is probably obvious, since my last comment here I got pulled in (many) different directions. 🙁 @matdehaast, many thanks! As of 1.7.0, we have that Envoy fix, so let me see if there's a quick way to enable that flag.

@hLudde
Copy link

hLudde commented Sep 14, 2020

Hi, is there an update on this? We are currently facing an issue on integrating a third party solution because of this. Im wondering if i should wait for this to get solved or to go around it another way 😊

@cakuros
Copy link
Contributor

cakuros commented Sep 29, 2020

Hi,
In terms of a workaround the two I've seen done are either:
A) use a Lua script in the Module to strip the port number from the Authority or Host header
B) to use host_regex on the mapping so that both the regular hostname and hostname:port get matched.

I think you can also do a literal match on hostname:port, in a mapping as well, although that duplicates the mappings.

Hope that helps!

@hLudde
Copy link

hLudde commented Oct 2, 2020

Cool, i tried your Lua suggestion and it seems to be working
here is the script for it if anyone needs it

spec:
  config:
    lua_scripts: |
      function envoy_on_request(request_handle)
        if request_handle:headers():get("Host") == "someurl.com:443" then
          request_handle:headers():replace("Host", "someurl.com")
        end
      end

Will this however be suported by ambassador so i dont need to have a Lua script run on each request? I did not get the host_regex to work last time i tried sadly

@jwm
Copy link
Contributor

jwm commented Oct 2, 2020

Hm, did this behavior end up changing in 1.7.x? I upgraded today, and noticed that our local kludge wasn't necessary any more.

In fact, it was breaking the Envoy config, which wound up looking like:

                                        "virtual_hosts": [
                                            {
                                                "domains": [
                                                    "graphql.staging.devoted.com",
                                                    "graphql.staging.devoted.com:443",
                                                    "graphql.staging.devoted.com:443",
                                                    "graphql.staging.devoted.com:443:443"
                                                ],

which caused:

[2020-10-01 22:25:55.563][105][critical][main] [source/server/config_validation/server.cc:60] error initializing configuration '/ambassador/snapshots/econf-tmp.json': Only unique values for domains are permitted. Duplicate entry of domain graphql.staging.devoted.com:443

Removing the kludge (i.e., going back to a stock Ambassador) yields:

                                        "virtual_hosts": [
                                            {
                                                "domains": [
                                                    "graphql.staging.devoted.com",
                                                    "graphql.staging.devoted.com:443"
                                                ],

@jwm
Copy link
Contributor

jwm commented Oct 3, 2020

Oops, ignore that previous comment. There was a TLSContext that I didn't realize had graphql:443 configured on it.

@stale
Copy link

stale bot commented Dec 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue is stale and will be closed label Dec 4, 2020
@jwm
Copy link
Contributor

jwm commented Dec 4, 2020

not stale

@stale stale bot removed the stale Issue is stale and will be closed label Dec 4, 2020
@iNoahNothing iNoahNothing added the pinned Issue cannot be marked stale label Dec 10, 2020
@khussey khussey added the t:bug Something isn't working label Jan 5, 2021
@khussey khussey added this to the 2021 Cycle 1 milestone Jan 5, 2021
@jcornick-alabs jcornick-alabs removed this from the 2021 Cycle 1 milestone Jan 21, 2021
@khussey khussey added this to the 2021 Cycle 2 milestone Mar 2, 2021
@khussey
Copy link
Contributor

khussey commented Mar 9, 2021

This this issue was fixed in the 1.12 release, which is now available.

@dbaumgarten
Copy link

@khussey Are you sure this issue is (still) fixed?
I am trying to reach an endpoint through ambassador using Prometheus, and Prometheus always receives a 404. Investigating this brought me here. Prometheus is including ":443" in the host-header, which Ambassador v1.14.1 really does not seem to like.

Running

curl -v  -H 'Host: somehost.de' https://somehost.de/metrics

works fine.

But using:

curl -v  -H 'Host: somehost.de:443' https://somehost.de/metrics

(which is close to what Prometheus is doing internally) results in a 404.

As far as I understand, Prometheus' behaviour is that of every Go-application and does comply with the relevant RFCs.

We have also tried adding every thinkable variant of wildcards in the route, with no success.

Has this bug just resurfaced, or is there anything else we need to do to make it work?

@cakuros
Copy link
Contributor

cakuros commented Sep 29, 2021

@dbaumgarten

This can be worked around with the below Lua script. It effectively strips the port from the host header prior to evaluating the routing behavior. This will work both with HTTP/1.1 and gRPC.

---
apiVersion: getambassador.io/v2
kind:  Module
metadata:
  name:  ambassador
  namespace: ambassador
spec:
  config:
  # Use the items below for config fields
    lua_scripts: |
      function envoy_on_request(request_handle)
        local authority = request_handle:headers():get(":authority")
        if(string.find(authority, ":") ~= nil)
        then
          local authority_index = string.find(authority, ":")
          local stripped_authority = string.sub(authority, 1, authority_index - 1)
          request_handle:headers():replace(":authority", stripped_authority)
        end
      end

edit: Also, to be clear, the issue you're seeing is slightly different from the one fixed in the above issue. In your case, the problem is that the server name being requested by the application during SNI is mismatched with the Host header. If you were to run curl -H 'Host: somehost.de:443' https://somehost.de:443/metrics, you would get the expected behavior using strip_matching_host_port: true and using a Mapping that specified a hostname without the port.

@dbaumgarten
Copy link

While a workaround is certainly nice, isn't this something that should be fixed "properly"? As far as I understand it, any Go-Application trying to reach something behind ambassador should be having these issues. And the behaviour of adding :443 to the Host-header may be strange, but absolutely valid.

Are you sure this is an SNI-Issue? The returned certificate is correct, just the request-routing is borked.

People above in this thread were describing the exact same issue I had. "Prometheus gets a 404 when scraping", so I don't understand how my issue differs from the original one.

It just seems like the original issue was never really fixed at all.

@bemipefe
Copy link

So if I understood correctly you can't expose ambassador on a port different than the default (either 80 or 443). Otherwise every request's header will be filled with the port in the 'Host' field and Ambassador is not able to handle the mapping correctly. I can only make it working if I use curl with the option -H 'Host: myambassadorhost' otherwise I get a HTTP 404 not found error. This is a quite a bit limiting.

I tried to use the strip_matching_host_port: true but it's not working. The official doc report this sentence:

This only applies if the port matches the underlying Envoy listener port.

Unfortunately I can't understand what this means in practice.
I tried using host_regex: true in the Mapping object definition but as already reported the port is not included in the regex evaluation so it doesn't work.

The only thing that worked in my case was to add the Module object as suggested by @cakuros above.

@rafalwytrykus
Copy link

In addition to the issue described here, I found that Ambassador's host mathing is case-sensitive i.e. it matches a request with -H "Host: somehost.de" with a mapping for somehost.de but not a request with -H "Host: SOMEHOST.de".
As a workaround I extended @cakuros's script:

---
apiVersion: getambassador.io/v2
kind:  Module
metadata:
  name:  ambassador
  namespace: ambassador
spec:
  config:
  # Use the items below for config fields
    lua_scripts: |
      function envoy_on_request(request_handle)
        local authority = string.lower(request_handle:headers():get(":authority"))
        if(string.find(authority, ":") ~= nil)
        then
          local authority_index = string.find(authority, ":")
          local stripped_authority = string.sub(authority, 1, authority_index - 1)
          request_handle:headers():replace(":authority", stripped_authority)
        else
          request_handle:headers():replace(":authority", authority)
        end
      end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Issue cannot be marked stale t:bug Something isn't working
Projects
None yet
Development

No branches or pull requests