add support from upgrading http->https in location header

[andrewzah]

Creating this per our discussion in the last contour community meeting on 5/4.

Right now we're accomplishing this with the following filter + lua code:

diff --git a/internal/xdscache/v3/listener.go b/internal/xdscache/v3/listener.go
index ac529a28..7e49380e 100644
--- a/internal/xdscache/v3/listener.go
+++ b/internal/xdscache/v3/listener.go
@@ -485,6 +485,8 @@ func (v *listenerVisitor) visit(vertex dag.Vertex) {
                                AddFilter(envoy_v3.FilterMisdirectedRequests(vh.VirtualHost.Name)).
                                DefaultFilters().
                                AddFilter(authFilter).
+                               AddFilter(envoy_v3.RewriteLocationHeader()).
                                RouteConfigName(path.Join("https", vh.VirtualHost.Name)).
                                MetricsPrefix(vh.ListenerName).
                                AccessLoggers(v.ListenerConfig.newSecureAccessLog()).

diff --git a/internal/envoy/v3/listener.go b/internal/envoy/v3/listener.go
index 89921133..3213f491 100644
--- a/internal/envoy/v3/listener.go
+++ b/internal/envoy/v3/listener.go
@@ -688,3 +688,85 @@ func ContainsFallbackFilterChain(filterchains []*envoy_listener_v3.FilterChain)
+
+func RewriteLocationHeader() *http.HttpFilter {
+       code := `
+   function envoy_on_response(response_handle)
+     local headers = response_handle:headers()
+     local locationHeader = headers:get("location")

+     if locationHeader ~= nil then
+       modified = string.gsub(locationHeader, "http://", "https://")
+       response_handle:headers():replace("location", modified)
+     end
+   end
+ `
+       return &http.HttpFilter{
+               Name: "envoy.filters.http.lua",
+               ConfigType: &http.HttpFilter_TypedConfig{
+                       TypedConfig: protobuf.MustMarshalAny(&lua.Lua{
+                               InlineCode: code,
+                       }),
+               },
+       }
+}

and the lua code extracted:

function envoy_on_response(response_handle)
  local headers = response_handle:headers()
  local locationHeader = headers:get("location")

  if locationHeader ~= nil then
    modified = string.gsub(locationHeader, "http://", "https://")
    response_handle:headers():replace("location", modified)
  end
end

---

See the first use case mentioned in this issue: #3006

[youngnick]

So, the requirement here is "Rewrite all Location headers to HTTPS, always"? And you are currently doing this globally for Contour? I can see adding a boolean thing somewhere that could replicate this, which dodges the support and related issues from #3006.

Sounds like a config file option like rewriteLocationToSecure or something might cover this. @andrewzah, do you think that would be what you want? Then, Lua is an implementation detail, if Envoy ever adds a way to do this without Lua, then we can switch to that seamlessly.

[205g0]

Just checking out Contour and stumbled over this issue, does this mean that Contour has no simple toggle or something like a built-in middleware to upgrade http reuqests to https? Didn't find anything in the docs but skimmed them briefly only...

[andrewzah]

@youngnick that should work well.

@205g0 [edit; for this particular situation] not yet, no. You can use a lua filter with the above code to handle upgrading http->https for now.

[stevesloka]

@205g0 at a high level, Contour does support an automatic 301 redirect from http->https if you're using HTTPProxy for ingress resources. The issue that @andrewzah is referencing is a little different from basic redirects.

[205g0]

@stevesloka thx for the info. Ok I am confused now haha, someone who likes standard Ingress with terse annotations and will upgrade to Gateway API in future, should I really setup a new cluster which will run for some time on HTTPProxy?? I know it will be supported beyond Gateway's GA but yeah...

[205g0]

@stevesloka I checked the API spec for HTTPProxy at https://projectcontour.io/docs/main/config/api/ and couldn't find anything about to-https-redirects in that CRD, even if I go down the rabbit hole into the sub-APIs there's even anything about simple redirects. Hope I just missed it or the HTTPProxy CRD isn't capable of doing redirects neither?! 😶

[youngnick]

@205g0 HTTPProxy doesn't have a way to ask for redirect to HTTPS, because it's on by default, unless you specifically disable it by creating an insecure proxy that matches the same domain name and route. If you just ask for foo.com, and supply TLS details, Contour will configure the HTTP->HTTPS redirect for you.

@andrewzah's case is about changing the Location header unconditionally from http://foo.com to https://foo.com, regardless of what you specified in your browser. Very different thing, kind of an edge case. But Contour will do HTTP->HTTPS redirect by default for HTTPProxy.

Also, HTTPProxy will be supported until two requirements are fulfilled, at least:

• Gateway API supports everything HTTPProxy does
• The community is happy with us deprecating it.

I don't see either of those things happening for a long time.

[205g0]

@youngnick thanks for the quick clarifications.

Having a HTTP->HTTPS redirect turned on by default is a great design decision. I assume, this default redirect also keeps the path?! So if someone typed http://example.com/app it redirects (btw with a 301, permanent one?!) to http://example.com/app/, right?

Apologies for such nitty-gritty questions, hope I can just try it out myself soon, have been playing around with the Ingress only.

[youngnick]

@205g0, yes that's correct. All paths for http://example.com/ get 301-ed to the equivalent https://example.com/ path by default.

[xaleeks]

Just wanted to reiterate Nick’s answer on whether it’s worth investing in HTTPProxy before Gateway API is ready. Yes, absolutely it is. Some other reasons I can think of would be

• Gateway API is still nascent and prone to changes. HTTPProxy is battle tested and provides much more compelling capabilities right now. We're working on bringing HTTPRoute up to parity with HTTPProxy
• Take a look at the ongoing releases and you’ll see a lot of feature work on HTTPProxy on making it even better.
• Even when gateway API matures and is at parity with HTTPPRoxy, we will support both for some period of time and discuss strategy of possibly moving away from HTTPRoxy with the community first. This requires buy in and a comfortable cushion for people to migrate. This is a long ways off

@youngnick what's your take on landing this request?

[youngnick]

I understand the request is to have a way to say "All HTTP requests should be forwarded to the equivalent HTTPS request, always". Rather than doing this with Lua, I think it would be better to have a way to mandate the behavior we already have with HTTPProxy, which does this for you by default. So, we have a boolean in the config file that would make all methods (Ingress, HTTPProxy, Gateway maybe?) work like this:

• You configure a HTTPS endpoint, you get a redirect for HTTP.
• You configure a HTTP endpoint, Contour says no, do this with HTTPS.

The main thing here is to make a way to mandate that this is the only acceptable behavior, I think, rather than the specific Lua implementation. Is that correct @andrewzah ?

[andrewzah]

So, the requirement here is "Rewrite all Location headers to HTTPS, always"?

This is correct. Something like a boolean value would work for this specific scenario.

[andrewzah]

@xaleeks @youngnick However, a more robust solution would be to allow regex editing of header values in general.

Right now there are set and remove, but a replace option that takes a regex would useful outside of this specific scenario.

[ccravens]

I actually think it would be better to create a more generic solution to this. I think what we need is a way to do extend the responseHeadersPolicy to include a replace function for standard regex replace. Right now you can only set and remove but we need the ability to replace what's in the current value because we don't necessarily know what's in there. I documented this pretty extensively here: #2776

[youngnick]

I agree that the replace functionality could be used to replicate the solution in the initial issue comment, but isn't that just rewriting the request as it's sent to Envoy? I don't think that's the same thing as ensuring that you can only request secure config from Contour (which seems to be what we have ended up with).

@andrewzah, are you looking to have this because you want to:

• mandate that all config submitted to Contour cannot ask for a HTTP site to be served, or
• you want the request being sent to Envoy to have its Host header modified in either the request or the response?

If it's the former, then we're talking about some sort of config setting that says "I seriously don't want anyone to be able to do HTTP, at all, and they should get 301 redirects without the option of changing them". If it's the latter, then something like #2776 would be a better solution, agreed.

@ccravens, I'm going to do a quick update on #2776 as well, some stuff has changed since we last touched that one.

[github-actions]

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

• Mark this Issue as fresh by commenting
• Close this Issue
• Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

[github-actions]

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

• Mark this Issue as fresh by commenting
• Close this Issue
• Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack