Rate limit github-auth endpoints

[LVMBDV]

I noticed this issue while I was working on #541. OAuth web application flows require a randomly generated state parameter to authenticate callback (redirection) requests from the client. Without this security check, adversaries can orchestrate a denial of service attack using the callback endpoint. For more info see:

• https://developer.github.com/apps/building-oauth-apps/authorizing-oauth-apps/#web-application-flow
• https://stackoverflow.com/questions/26132066/what-is-the-purpose-of-the-state-parameter-in-oauth-authorization-request

TL;DR:

[paulmelnikow]

Just emailed you; thanks.

[paulmelnikow]

Clarified offline that this is low severity. We should still fix it quickly. 😬

[paulmelnikow]

It's not clear how to fix this. The state is exposed to the client; it's not a secret. We don't preserve any state between requests, and any state we did preserve would need to be shared between three servers. That's the least of it, really. The /github-auth endpoint hands out these strings. Since we don't authenticate those requests, it's not clear the state can do anything to help.

We could consider a rate limit.

[LVMBDV]

In my opinion, this and the token pool solidifies the need for separate state storage such as a key-value database like Redis.

We could consider a rate limit.

That's like demolishing a building so terrorists can't blow it up.

[paulmelnikow]

How would state storage solve the problem? If one endpoint gives out tokens as fast as you want, and another consumes them, it doesn’t matter whether they are single use or not.

It’s more like putting a fence around it. We probably could limit to one token per 30 sec per IP, and 5 per 30 seconds overall, without much inconvenience to anyone.

[LVMBDV]

You're right, rate-limiting the auth endpoint makes sense.