envoy: tls_inspector needlessly injected, causing incorrect behavior

Title: tls_inspector needlessly injected, causing incorrect behavior

Description:

When a filter chain is configured with: HTTP inspector, no tls inspector, a match on application_protocol, no match on transport_protocol, we get the following log: adding listener '0.0.0.0:15006': filter chain match rules require TLS Inspector listener filter, but it isn't configured, trying to inject it (this might fail if Envoy is compiled without it)

This is incorrect - tls inspector should not be needed here, since the http inspector will add the application protocol. This is caused by the code here: https://github.com/envoyproxy/envoy/blob/22683a0a24ffbb0cdeb4111eec5ec90246bec9cb/source/server/listener_impl.cc#L41

I would expect in this case, no TLS inspector is added. As a meta comment, automatically injecting this seems inconsistent with Envoys “fail fast” mentality but probably hard to change the behavior now.

Please note this is note benign - normally we would not care about the tls inspector being added since at worst its a trivial performance impact. However, we have recently moved to using ListenerFilterChainMatchPredicate to selectively enable listener filters to allow server first protocols to succeed. Because this is automatically injected, these are timing out here, forcing us to explicitly add the tls inspector in all cases.

Interestingly, we could just add transport_protocol = "raw_buffer" to the match, but then we will break all of our other matches due to https://github.com/envoyproxy/envoy/issues/12572

Repro steps:

This config will easily reproduce

admin:
  access_log_path: /tmp/admin_access.log
  address:
    socket_address:
      address: 127.0.0.1
      port_value: 9901
      protocol: TCP
static_resources:
  listeners:
  - address:
      socketAddress:
        address: 0.0.0.0
        portValue: 15006
    filterChains:
    - filterChainMatch:
        applicationProtocols:
        - http/1.0
        - http/1.1
        - h2c
      filters:
      - name: envoy.filters.network.tcp_proxy
        typedConfig:
          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
          cluster: InboundPassthroughClusterIpv6
          statPrefix: InboundPassthroughClusterIpv6
      name: virtualInbound
    listenerFilters:
    - name: envoy.filters.listener.http_inspector
      typedConfig:
        '@type': type.googleapis.com/envoy.extensions.filters.listener.http_inspector.v3.HttpInspector

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 16 (15 by maintainers)

Most upvoted comments

@PiotrSikora that will work for us, I think we will be fine with it auto injected or not by working around it in the control plane with explicit raw-buffer (except doing it right this time…). So it’s not strictly required but it seems like the preferred behavior for us and others.