h2
h2 copied to clipboard
hyperium
Fix: Limit the number in the slab
@hawkw Hello, may I ask when you are free to check it out? I think this serious ddos problem should be avoided or fixed as soon as possible.
It seems in fn open(), we check that the number of currently active received streams is under the max, and if at the limit, then the stream is rejected (likely reset). With this new change, the stream would be accepted, but if we had too many pending reset streams, the connection would go away. That seems... aggressive...
Then again, if a user has decided to have no more than some limit, it's probably to reduce resource usage... Resetting more streams would just add to the "too many"... The problem, I think, is if a remote has been keeping the local peer at the max, and then a stream is reset, it would pretty much immediately close it all down.
Shouldn't it be sufficient to just set the max_reset_streams to something? Anything over that limit should be forgotten immediately, no?
hmm, is it really correct to GOAWAY the whole connection in this case? sending a GOAWAY will tell the client it can never initiate new streams on this connection, but those slab indices will become available again (in
clear_expired_reset_streams). so, this condition is transient. is there a reason we can't reset the new stream rather than GOAWAYing the whole connection? it is entirely possible i'm overlooking something here, i haven't looked at this code in a while...
I'm just starting to get to know H2 and I'm not familiar with it. I just think it's safer to disconnect in this scenario. Developers can optimize this problem by tweaking max_concurrent_streams and max_concurrent_reset_streams.
It seems in
fn open(), we check that the number of currently active received streams is under the max, and if at the limit, then the stream is rejected (likely reset). With this new change, the stream would be accepted, but if we had too many pending reset streams, the connection would go away. That seems... aggressive...Then again, if a user has decided to have no more than some limit, it's probably to reduce resource usage... Resetting more streams would just add to the "too many"... The problem, I think, is if a remote has been keeping the local peer at the max, and then a stream is reset, it would pretty much immediately close it all down.
@seanmonstar Sorry, I didn't understand you well. Is it a direct GOAWAY or REST_STREAM? Or something better.
Who can answer the problem of the PR or submit a new PR with a better solution to fix the problem? https://github.com/hyperium/h2/issues/621
Who can answer the problem of the PR or submit a new PR with a better solution to fix the problem? #621
@seanmonstar @hawkw Can someone answer that? If this problem is not solved, services built on the crate using h2 are at high ddos risk.
@seanmonstar @hawkw How will the h2 community deal with this problem in the future? If you guys are busy, is there anyone else in the community familiar with this issue? https://github.com/hyperium/h2/issues/621
@LucioFranco Hello, can this take a little of your precious time? Thank you.
Would also be good to get CI to run against this change which may just need a new commit to trigger it.
