'Invalid Upgrade request header: spdy/3.1' - HTTP/1.1 request parsed as HTTP/2?

[OJFord]

I'm using Caddy to reverse_proxy a Kubernetes API server.

realapiserver:443 {
  reverse_proxy {
    to {http.request.host}:443
    transport http {
      tls
    }
  }

  tls internal
}

Port-forwarding (kubectl port-forward for example, and others that use it, such as kubectl exec and helm) requests result in an error like:

Mar 14 10:05:31 ip-10-11-23-114 caddy[14088]: {"level":"error","ts":1647252331.2734516,"logger":"http.log.error","msg":"http2: invalid Upgrade request header: ["spdy/3.1"]","request":{"remote_addr":"10.2.16.0:56412","proto":"HTTP/1.1","method":"POST","host":"realapiserver","uri":"/api/v1/namespaces/ns/pods/pod/portforward","headers":{"User-Agent":["kubectl/v1.23.4 (linux/amd64) kubernetes/e6c093d"],"Content-Length":["0"],"Kubectl-Session":["hunter2"],"X-Stream-Protocol-Version":["portforward.k8s.io"],"Authorization":["Bearer hunter2"],"Connection":["Upgrade"],"Kubectl-Command":["kubectl port-forward"],"Upgrade":["SPDY/3.1"]},"tls":{"resumed":false,"version":772,"cipher_suite":4865,"proto":"","proto_mutual":true,"server_name":"realapiserver"}},"duration":0.000156429,"status":502,"err_id":"rwd8dzxch","err_trace":"reverseproxy.statusError (reverseproxy.go:886)"}

This seems to come from net/http2: https://cs.opensource.google/go/x/net/+/27dd8689:http2/transport.go;l=1079-1081

	if v := req.Header.Get("Upgrade"); v != "" {
		return fmt.Errorf("http2: invalid Upgrade request header: %q", req.Header["Upgrade"])
	}

Which is all well and good, HTTP/2 'purposefully does not support upgrade to another protocol': https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2.2

But as we can see from the "proto" field above, this was an HTTP/1.1 request.

---

$ uname -srm
Linux 5.13.0-1017-aws aarch64
$ caddy version
v2.4.6 h1:HGkGICFGvyrodcqOOclHKfvJC0qTU7vny/7FhYp9hNw=
$ caddy list-modules
# ...
  Standard modules: 83

dns.providers.route53
layer4
layer4.handlers.echo
layer4.handlers.proxy
layer4.handlers.proxy_protocol
layer4.handlers.tee
layer4.handlers.throttle
layer4.handlers.tls
layer4.matchers.http
layer4.matchers.ip
layer4.matchers.proxy_protocol
layer4.matchers.ssh
layer4.matchers.tls
layer4.matchers.xmpp
layer4.proxy.selection_policies.first
layer4.proxy.selection_policies.ip_hash
layer4.proxy.selection_policies.least_conn
layer4.proxy.selection_policies.random
layer4.proxy.selection_policies.random_choose
layer4.proxy.selection_policies.round_robin
tls.handshake_match.alpn

  Non-standard modules: 21

  Unknown modules: 0

[OJFord]

Ah, this is because versions defaults to 1.1 2, which causes the HTTP2 transport to be configured (cf. 97d918df3e89a3cc4dd5e5c06967bb85870c5ce3).

Probably the request should be inspected for HTTP/2-incompatible features such as this to avoid the error, I may attempt a PR for that. (Edit: though that's not really possible of course since the transport is provisioned ahead of any (and to serve multiple) requests...)

[OJFord]

My solution is to create a matcher:

@http2notallowable {
   header_regexp Upgrade .+
   header_regexp !Content-Transfer-Encoding (?:chunked|)
   header_regexp !Connection (?i:close|keep-alive|)
}

(EDIT: Or not! Seems negated header matchers can't be used with a value, (it validates ok, but seems to be ignored) so this doesn't work as intended.)

And then duplicate my reverse_proxy block:

reverse_proxy @http2notallowable {
  transport {
    tls
    versions 1.1  # override default versions, otherwise as in OP
  }

  # ...
}

reverse_proxy {
  # this one *second* and exactly as in OP
  transport {
    tls
  }

  # ...
}

It would be nice if reverse_proxy did something equivalent automatically if versions is going to default to 1.1 2. It could test the request against http2.checkConnHeaders(req) directly, which is what I've done my best to reproduce in a Caddy matcher here.

[francislavoie]

Seems like we should omit the Upgrade header if it's not a known-good value, or something:

https://github.com/caddyserver/caddy/blob/a9c7e94a38e4f4719a5c1cf7f3e11e8cf427f04e/modules/caddyhttp/reverseproxy/reverseproxy.go#L535-L595

[OJFord]

@francislavoie The only acceptable value is the empty string. But this is no good for proxying HTTP/1.x requests that do want to upgrade! Only solution for those is not to permit HTTP/2, set versions 1.1 or similar so that Caddy does not configure the HTTP2 transport: https://github.com/caddyserver/caddy/blob/ab0455922ae01bde1a7a5b3bf58eb993efc02db7/modules/caddyhttp/reverseproxy/httptransport.go#L236-L240

[mholt]

@OJFord Are you looking for the not matcher? Like:

@http2notallowable {
   not {
      header_regexp Content-Transfer-Encoding (?:chunked|)
      header_regexp Connection (?i:close|keep-alive|)
   }
   header_regexp Upgrade .+
}

[OJFord]

Ah thanks @mholt, yes, that'd probably fix my workaround, I'll test it soon. I just checked to remind myself what unblocked this; we're currently just fixing versions 1.1 entirely, i.e. like the 'http2notallowable', but without filtering matches at all.

I don't think that's a complete solution for the issue though? I'm not sure what the fix is, but it seems wrong that this happens in the first place - it's never correct, it's just an unusual protocol so doesn't trap many people. AIUI anyway?

[mholt]

@OJFord Yeah, possibly. I need to revisit how the Go std lib decides when to use HTTP/2 vs. HTTP/1.1. Any updates from testing that matcher?

[francislavoie]

Is there anything actionable for us here? I'm not sure there is. If this stays inactive, we'll probably close this.

[OJFord]

@francislavoie I haven't yet got around to testing the suggested fix to my proposed workaround, but yes as I understand it the underlying issue is somewhat separate in that SPDY requests are misidentified as HTTP/2.

[mholt]

Wait, isn't SPDY an obsolete protocol?

[OJFord]

@mholt In the sense that there's no reason to choose it over HTTP/2 for new work I believe yes (I'm really no expert though) - like HTTP/1.1 is also 'obsolete' in that sense.

This issue arose with a recent-ish (as in, within a couple of years at most) version of Kubernetes.

(edit: Wikipedia says SPDY/4 (draft) basically became HTTP/2, with more divergence from earlier SPDY. Perhaps fair enough to treat SPDY/4 as HTTP/2 then, but SPDY/3.1 (as here) is not.)

[mholt]

Yeah, ok. So I'm not sure that I feel compelled to support obsolete protocols, especially an old version of an obsolete protocol...

Even if we did, I suspect that change would have to happen upstream in a dependency like Go or x/net or something.

[mholt]

@OJFord Respectfully, I think I will close this issue now if that's OK with you. Feel free to continue discussion -- as it's quite possible I'm missing something important. But my recommendation at this point would be to upgrade your client if possible.

Appreciate your participation and patience!

[OJFord]

Sure, no worries.