Kong / charts

Helm chart for Kong
Apache License 2.0
249 stars 480 forks source link

SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER #522

Closed ichasco-heytrade closed 2 years ago

ichasco-heytrade commented 2 years ago

Hi, I am deploying Kong with istio and I get this error when I try to deploy 2.6.3 Chart version. With the 2.1.0 chart version works well. I am not able to see what has changed that affects me.

curl:

upstream connect error or disconnect/reset before headers. reset reason: connection failure, transport failure reason: TLS error: 268435703:SSL routines:OPENSSL_internal:WRONG_VERSION_NUMBER%

Kong Logs:

kong-kong-7484798d46-tss7m proxy 10.30.11.127 - - [05/Jan/2022:12:58:58 +0000] "\x16\x03\x01\x00\xCD\x01\x00\x00\xC9\x03\x03-\xFB\xAA(0\xA9\xEA1\x80\x9B\xDA$I\xA3yu6\xEC>6\xAA\x9D\x0B\xAC\xFA\x06\xB7\x1C\x0E\xC7\x8D\x8A\x00\x00\x0C\xC0+\xCC\xA9\xC0/\xCC\xA8\xC0,\xC00\x01\x00\x00\x94\x00\x00\x00;\x009\x00\x006outbound_.80_._.kong-kong-proxy.kong.svc.cluster.local\x00\x17\x00\x00\xFF\x01\x00\x01\x00\x00" 400 12 "-" "-"
kong-kong-7484798d46-tss7m proxy 10.30.11.127 - - [05/Jan/2022:12:58:58 +0000] "\x16\x03\x01\x00\xCD\x01\x00\x00\xC9\x03\x03\xDFBU\xF4n\xF8\xC4\xBEM\xCC2\xF8\x8A\xEB\x8D\x8DJ\xF2\x0E\xE1a^\xF9\x07l\xF8\xA2c\x88\xB8" 400 12 "-" "-"
kong-kong-7484798d46-tss7m proxy 10.30.11.127 - - [05/Jan/2022:12:58:58 +0000] "\x16\x03\x01\x00\xCD\x01\x00\x00\xC9\x03\x03\xEFhO\xC9X\xE0\xCD\x18\xF6\xC2_\x1BUy\xDB\x86*\xEE\x03\x10<R\xFF/\xBD+T\x93}L\xEE\xB9\x00\x00\x0C\xC0+\xCC\xA9\xC0/\xCC\xA8\xC0,\xC00\x01\x00\x00\x94\x00\x00\x00;\x009\x00\x006outbound_.80_._.kong-kong-proxy.kong.svc.cluster.local\x00\x17\x00\x00\xFF\x01\x00\x01\x00\x00" 400 12 "-" "-"

thanks

rainest commented 2 years ago

The non-plaintext requests in the logs suggest you're sending HTTPS traffic to the HTTP port. I'm unaware of anything that would have affected that between those versions, but can you double-check your proxy Service configuration and the resulting Service's port mappings versus the KONG_PROXY_LISTEN in the Deployment environment.

ruiengana commented 2 years ago

Same here.

Everything works fine up to chart 2.6.1 but when deploying using chart 2.6.2 problem arises.

Not sure, but might be related to https://github.com/Kong/charts/commit/0565c0372d2561ff96bc111eb63e686f807b2d56

rainest commented 2 years ago

@ruiengana are you using a mesh proxy (and which, if so)? Those shouldn't make a difference otherwise.

ruiengana commented 2 years ago

@ruiengana are you using a mesh proxy (and which, if so)? Those shouldn't make a difference otherwise.

Yes, we use Istio over Kong.

rainest commented 2 years ago

What are the requests in question? Should they be using HTTPS or no?

The original log looks a bit odd since it contains what appears to be SNI for the proxy Service hostname, which isn't something I'd normally expect to see (requests inbound to the proxy would normally use some other hostname that the proxy is configured to route to some upstream). Does this affect all requests ore just some?

https://istio.io/latest/docs/ops/configuration/traffic-management/tls-configuration/ indicates that for traffic inbound to a Service:

a new TLS connection will never be originated from the sidecar.

so I'm not sure why this would have changed--applications should have been TLS or not to the proxy already, and that should remain the case. Do you have Ingresses that are set up to route traffic back into the proxy? That could maybe explain this, if those Ingresses are routing to the proxy HTTP port but Istio is adding TLS to traffic outbound of the proxy.

ruiengana commented 2 years ago

In our setup Kong Proxy is expecting HTTP. The Istio Sidecar manages TLS / mTLS and sends simple HTTP to Kong Proxy.

ruiengana commented 2 years ago

Just tested and confirmed that issue is caused by https://github.com/Kong/charts/commit/0565c0372d2561ff96bc111eb63e686f807b2d56

I deployed using chart 2.6.2, confirmed error, then edited the deployment to revert above commit and put annotations back and suddenly everything starts working as expected.

Looking closely, the issue is caused specifically by the annotation traffic.sidecar.istio.io/includeInboundPorts: "" which makes Kong sidecar passthrough all inbound traffic. All our inbound traffic is HTTPS and since Kong sidecar is not translating it back to HTTP before handing off to Kong Proxy, hence the HTTPS traffic into Proxy HTTP port.

The real issue is that default annotation traffic.sidecar.istio.io/includeInboundPorts: "" from chart only makes sense when Kong is setup as Ingress Controller, which is not the case for us. We have a standard Nginx as Ingress Controller in front of Kong and it's that Nginx that holds the annotation traffic.sidecar.istio.io/includeInboundPorts: ""

Hope this make sense.

rainest commented 2 years ago

Ah, that makes more sense then--you'll need to just remove that with an explicit podAnnotations: {} in your values.yaml. We do want it in there by default because using Kong as an ingress controller is our default and most common configuration.

To confirm, overriding that does make it work again? @ichasco-heytrade can you confirm the same?

ruiengana commented 2 years ago

Ah, that makes more sense then--you'll need to just remove that with an explicit podAnnotations: {} in your values.yaml. We do want it in there by default because using Kong as an ingress controller is our default and most common configuration.

To confirm, overriding that does make it work again? @ichasco-heytrade can you confirm the same?

Won't work for us because I still need to add sidecar.istio.io/inject: "true" annotation to Pod and can't find any other way to unset default values.

Can we put something in the chart to only add the annotation traffic.sidecar.istio.io/includeInboundPorts: "" when the ingressController is enabled via an explicit test in the Helm template?

rainest commented 2 years ago

Ah, this is another case where Helm's value merging doesn't work in our favor: if the set is not empty, you get any default keys.

You can override the key though, does setting traffic.sidecar.istio.io/includeInboundPorts: "*" clear it?

ruiengana commented 2 years ago

Adding traffic.sidecar.istio.io/includeInboundPorts: "*" to podAnnotations worked, but it really feels like a workaround.

I think right fix should be change the Helm template to only inlcude the annotation if Kong is set as Ingress Controller. Would you consider a PR for it?

PS : Thanks for the help :)

rainest commented 2 years ago

I'd prefer adding documentation in a comment above the values.yaml section. Having a conditional in the template hides some of the annotation set and makes it impossible to override without complicated tooling like we use for environment variables.

A mesh handling inbound traffic isn't really tied to use of the ingress controller, it's dependent on where Kong is in the network topology--there are users that have internal Kong instances that should have inbound traffic routed through the mesh, but are also using the controller to populate configuration--so we can't as readily infer the correct setting here based on other content in the values.

I'll go ahead and close this since we've found a solution to the problem. @ruiengana let me know if you'd like to make the values.yaml comment PR; I can make the change if not.