HttpConnPoolImplMixed could result in many unused upstream connections

[roelfdutoit]

Title: HttpConnPoolImplMixed unused connections

Description:
Consider a scenario with the following conditions:

• HttpConnPoolImplMixed is required (possibly because some percentage of upstream servers are expected to still use http/1.1)
• downstream negotiates h2
• upstream is likely to negotiate h2
• downstream creates an initial burst of h2 streams

HttpConnPoolImplMixed::instantiateActiveClient sets the concurrent_stream_limit to 1, which results in a new upstream connection for every downstream h2 stream, and each of the upstream connections would start a TLS handshake. The first connection that negotiates h2 would then be associated with all/most of the downstream streams, resulting in a bunch of unused h2 connections.

I understand that the connection-per-stream behavior is expected if upstream is mostly http/1.1, but could this be considered an amplification attack if upstream is expected to negotiate h2 most of the time, given that every downstream stream will result in an expensive TLS handshake?

[roelfdutoit]

References: #3431 and #13922

[htuch]

@alyssawilk thoughts?

[alyssawilk]

You can never be sure which way this is going to go, and you either take a CPU penalty by assuming H1 (and doing unnecessary handshakes) or a latency penalty and assuming H2 (and queueing streams for a connection which can't handle them in parallel). The current implementation defaults to the former, but there's no reason we can't allow the latter with a config knob.
I don't think establishing a connection per stream qualifies as an amplification attack as literally that's what HTTP/1.1 has done for decades :-)

[roelfdutoit]

Thank you for the response. I do agree that HTTP/1.1 has this behavior, but my question is specific to the combination of conditions listed where upstream is likely to negotiate h2 (but not always). If downstream is h2 then it is very cheap to send lots of requests, which would trigger lots of upstream TLS handshakes. The amplification is due to the difference in cost of creating a new h2 stream vs an upstream TLS handshake.

[alyssawilk]

I agree that the ALPN pool would do extra work in this case, but again with years worth of proxies that terminate HTTP/1 and forward HTTP/1.1 back (which is still a common mode for folks who haven't updated its infrastructure) I really don't think it qualifies as an attack or a bug, just a config knob which would be good to have. You can cause as many handshakes if you control the upstream by simply closing connections after use, or sending GOAWAYs after a single request.

[roelfdutoit]

Thanks again. We would certainly appreciate the config knob (to assume h2 and limit upstream connections in the mixed pool) in our environment.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.