New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable Slowloris size #418

Merged
merged 1 commit into from Aug 5, 2015

Conversation

Projects
None yet
4 participants
@mfine
Contributor

mfine commented Aug 5, 2015

I know there's been a lot of history around the Slowloris protection here and in the mailing list. This change simply seeks to make the hard-coded value configurable by settings, with a continued default value of 2048 bytes.

The current documentation indicates that

Every time at least 2048 bytes of the request body are read, the timeout is tickled.

which suggests that after 2048 bytes of the request body have been processed, through however many reads, the timeout will be tickled. But the current implementation seems to tickle the timeout only after a single read consumed at least 2048 bytes. Sending in at least 2048 bytes across multiple writes without any single write at least 2048 bytes will trigger the Slowloris protection. A stateful counter of bytes here that resets on tickle might be useful.

Our current warp use-case is an IoT application that POST's low volume telemetry data over long-lived streaming requests. Our devices end up looking a lot like a Slowloris attack :) While in the long run HTTP/2 might be a better fit and its timeout tickling better-suited, we'd still like to be able to use HTTP/1 as well, and configure down the Slowloris prevention value without having to increase the timeout value and lose client responsiveness detection.

I couldn't piece together in RunSpec.hs how the Slowloris prevention was being validated, but would be happy to add a test that checks that the new Slowloris size is honored with a little guidance.

And thanks for the great software! We love running on top of warp!

@erikd

This comment has been minimized.

Contributor

erikd commented Aug 5, 2015

👍

snoyberg added a commit that referenced this pull request Aug 5, 2015

@snoyberg snoyberg merged commit 2202f6d into yesodweb:master Aug 5, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@snoyberg

This comment has been minimized.

Member

snoyberg commented Aug 5, 2015

This looks great, thanks!

snoyberg added a commit that referenced this pull request Aug 5, 2015

@mfine

This comment has been minimized.

Contributor

mfine commented Aug 5, 2015

Thanks so much for the quick turn around!

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Aug 6, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment