Proxy can handle gRPC traffic including streaming

[davidni]

Streaming requests (e.g. duplex gRPC request) require special handling by YARP (at a minimum, e.g. remove minimum request data rate enforcement).

Today we check for gRPC content type, but this is both too broad (not all gRPC requests require streaming semantics) as well as too narrow (other protocols than gRPC on top of HTTP/2 could also require streaming semantics).

Some proposals were discussed in #80 (comment)

[samsp-msft]

I am assuming the problem here is potential DoS attacks - keep opening connections and doing very little with each of them.
In the non proxy case of a .NET gRPC back end, presumably there is something that is looking at the type of gRPC message and changing the timeouts? Can we rely on that for the proxy - that we look for the back end server to make a determination and to close the connection if the backend rejects it?
If a client has a long running, but low traffic connection, such as the connections for mobile notifications, does that cause limits on the server or resource starvation?

[rynowak]

e.g. remove minimum request data rate enforcement

If the data rate is the problem blocking this, how is special casing gRPC not just a gaping security hole? 🤔 If an attacker can get around by the data rate enforcement by just using gRPC then that seems like it makes every gRPC service vulnerable right?

[davidfowl]

This is why signalr sends pings in its protocol.

[davidni]

If an attacker can get around by the data rate enforcement by just using gRPC then that seems like it makes every gRPC service vulnerable right?

@rynowak not just gRPC. This discussion also applies to HTTP/1.1 upgraded connections. See docs for KestrelServerLimits.MinRequestBodyDataRate:

/// ...
/// This limit has no effect on upgraded connections which are always unlimited.
/// ..
public MinDataRate MinRequestBodyDataRate { get; set; }

[Tratcher]

To summarize the discussion from #80:

gRPC streaming and non-streaming requests look identical to the proxy, they use all the same headers. The backend app only knows which endpoints are supposed to be streaming because it's told that when building the endpoint. The backend app already disables request body data rates and size limits for streaming requests (just like websockets), but it still enforces them for non-streaming requests.

If the proxy decides not to enforce request body size or rate limits for gRPC, the backend will still be able to. If the backend aborts a request that will trigger the proxy to abort it as well. This should be OK so long as we clearly communicate that gRPC backends are expected to handle this case (and write lots of tests).

The only alternative I see here is configuring the proxy to suppress the limits only for an explicit list of streaming URLs, and that's a pretty terrible user experience.

This is why signalr sends pings in its protocol.

Pings aren't enough traffic to satisfy the data rate, they only keep the tcp connection from being marked as idle.

This discussion also applies to HTTP/1.1 upgraded connections.

Upgraded connections are not supposed to enforce rate or size limits, that's up to the higher level protocol to manage (WebSockets). If the backend decides to abort an upgraded connection this should also trigger the proxy to abort.

[karelz]

Triage: Needs design - may require config per route opting out of min-rate limiting

[karelz]

Triage: We need to review the workaround in place and try to scope it down for 1.0.

[Tratcher]

One proposal is for these risky scenarios to be opt-in:

public sealed record RequestProxyConfig
{
+        /// <summary>
+       /// Enables detection and handling of streaming request bodies like gRPC.
+       /// </summary>
+       public bool? EnableStreaming { get; init; }
+
+        /// <summary>
+        /// Enables detection and handling of upgrade requests like WebSockets and SPDY.
+        /// </summary>
+        public bool? EnableUpgrade { get; init; }
}

Normal gRPC requests and download streams would still work fine, only upload streams would require opting in to disable the default request size and data rate minimums.