h2 fix for the recent security vulnerability breaks downstream consumers

[cbpodd]

Version

v0.14.6+

Platform

All

Summary

Hyper fixed this security vulnerability in v0.4.16 by having a fixed budget for undersized DATA frames. However, this penalizes real-time streaming applications which regularly send large numbers of undersized frames and buffer them in clients. This causes legitimate streams to disconnect using v0.4.16+ that did not in v0.4.15.

Expected Behavior

Legitimate streams that worked in v0.4.15 should work in v0.4.16 without additional configuration (it's a patch version bump).

Actual Behavior

Streams with high-volume undersized DATA frames were cut off with GOAWAY ENHANCE_YOUR_CHILL in v0.4.16 that streamed fine in v0.4.15.

Additional Context

h2 PR #942 partially fixes this problem by allowing users to set the budget for undersized data frames. However, our stack is tonic -> hyper-utils -> hyper -> h2.

I'd be happy to submit PRs that plumb this code through the hyperium stack. However, I think there might be a better option here that allows h2 to infer these windows from connection_window.

As I understand it, there are two issues at play here:

1. 0-byte DATA frames are never exposed to the application. These need to be rate limited and a connection may need to be killed if too many are sent. The hard-cap is fine here - I don't think there's a legitimate use-case for non-final 0-byte DATA frames.
2. Undersized DATA frames need to be controlled in some way. This is where the configurable buffer comes in.
   • I think it might be better to derive that buffer from the connection_window.
   • This will allow streams with limited requirements to break quickly, while allowing those with more memory to accumulate, buffer, and drain more bytes before breaking.

I'm happy to go down either of these paths and I'm happy to submit PRs. I'd like to know where best to direct my efforts.

1. I can submit PRs to expose these configuration parameters in hyper and hyper-utils.
2. I can derive the allowed buffer from the connection_window.

Let me know what's best here!

@seanmonstar

[seanmonstar]

Thanks for this. Yea, I'll split the 0-frame thing into its own counter, then it's easier to deal with the rest of this.

I'm thinking, instead of just trying to pick a specific default, we can use a hint from the connection window size. (Max concurrent streams is less interesting I think, the connection window always applies.) So, if this overhead budget is not explicitly configured, we can then pick a default based on the connection window. Perhaps we say that an additional 50% is allowed in small-frame overhead. That would mean:

• The protocol default 64kb: 32kb (about 128 frames currently)
• hyper's default of say 1mb: 512kb (about 2,000 frames).

For some of the other user reports of needing even more, I suspect they have raised their connection window even higher as well.

@cbpodd what do you think of that?

[cbpodd]

I agree strongly with this @seanmonstar. I think 50% in small-frame overhead makes sense to me and ensures that users don't have to plumb through a parameter. We raise our connection window high (64Mb) so we'd get 128k frames, I think? Which is way more than we'd need.

Let me know if I can help with this - thanks for looking into it!

[seanmonstar]

v0.4.19 is out with the change, I hope that fixes it for legitimate small frames.

[cbpodd]

@seanmonstar thanks much for your help! I'll test that out now and get back to you.

[cbpodd]

This solved our problem - thank you!