Invalid configuration generated with using protocol `HTTPSPROXY`

[cyrus-mc]

When enabling protocol HTTPSPROXY generated configuration results in the following error when connecting via TLS:

error:1400410B:SSL routines:CONNECT_CR_SRVR_HELLO:wrong version number

This is a result of the proxyProtocolStack being set to [ "TLS", "PROXY", "HTTP", "TCP" ], so TLS wrapping proxy wrapping HTTP wrapping TCP.

To Reproduce

Create a listener as such:

apiVersion: getambassador.io/v3alpha1
kind: Listener
metadata:
  name: http
  namespace: emissary
spec:
  port: 8443
  protocol: HTTPSPROXY
  securityModel: SECURE
  l7Depth: 0
  hostBinding:
    namespace:
      from: SELF

If using AWS NLB enable proxy protocol by setting the following annotation:

service.beta.kubernetes.io/aws-load-balancer-proxy-protocol: "*"

When attempting to connect to Emissary via https you will receive error error:1400410B:SSL routines:CONNECT_CR_SRVR_HELLO:wrong version number. Steps to reproduce the behavior:

Expected behavior

With proxy protocol enabled at the NLB and configured via protocol HTTPSPROXY a successful connection should be established.

Versions (please complete the following information):

• Ambassador: 2.2.2
• Kubernetes environment: AWS EKS 1.21

[cyrus-mc]

This is due to the fact that HTTPSPROXY is set to proxyProtocolStack [ "TLS", "PROXY", "HTTP", "TCP" ] which generates configuration:

"listener_filters": [
  {
    "name": "envoy.filters.listener.tls_inspector"
  },
  {
    "name": "envoy.filters.listener.proxy_protocol"
  }
]

The filters are in the incorrect order. Therefore proxyProtocolStack should be [ "PROXY", "TLS", "HTTP", "TCP" ]

[cyrus-mc]

The work around for now is to not set the protocol too HTTPSPROXY but rather explicitly set the protocolStack as shown above. However the documentation seems to point people to using protocol instead.

[cyrus-mc]

Based on conversation in community slack channel it appears that maybe different LB wrap TLS before proxy. I know with Ambassador 1.9 the order of the listeners was always proxy_protocol before tls_inspector so made assumption that that was the common scenario.

[cyrus-mc]

Reading the RFC

In both cases, the protocol simply consists in an easily parsable header placed by the connection initiator at the beginning of each connection

This to me seems to indicate that proxy wrapping TLS is the correct order and any other LB that differ from that are the one offs. So at the very least I think the default for HTTPSPROXY should be as noted here and updated in the PR.

[alexgervais]

Thanks for the contribution @cyrus-mc ! I've assigned the PR review to a maintainer and will rely on the test automation to catch regressions.