Add rate limiting to unauthenticated routes

[sfreiberg]

Purpose

Add rate limiting to unauthenticated routes and fix a bug in the rate limiter logic.

Implementation

1. Fix a bug in the rate limiter check that effectively circumvented the rate limiter and add a test
2. Move the rate limiter onto the Handler struct so it can be used by the enterprise version
3. Delete a legacy (deprecated) route and update tests
4. Add limiter to /webapi/sessions/web and /webapi/github/login/console

[codingllama]

Add rate limiting to unauthenticated routes and fix a bug in the rate limiter logic.

Sorry, what was the bug?

[sfreiberg]

@codingllama The rate limiter was checking the remote IP address AND port. Multiple requests using something like curl would never trigger the rate limiter because each new request would come from a different remote port.

[codingllama]

@codingllama The rate limiter was checking the remote IP address AND port. Multiple requests using something like curl would never trigger the rate limiter because each new request would come from a different remote port.

Well spotted. Thanks for the fix!

[github-actions]

@sfreiberg See the table below for backport results.

Branch	Result
branch/v10	Failed
branch/v11	Failed
branch/v9	Failed

[sfreiberg]

Rate limiting wasn't available in v9 so these changes will only be backported to v10 and v11.

[avatus]

I missed this in the review (was testing not this part I guess?) but this broke logging in via the webapi.
Removing this line here

results in the web login form giving the error

Apparently the old "REMOVE IN 5.1" comment wasn't quite accurate. Adding this line back in fixes the login issue

h.POST("/webapi/sessions", httplib.WithCSRFProtection(h.createWebSession))

Although I suppose the frontend change to call the new endpoint would probably be a better fix.

[avatus]

I've put up a PR here on the webapps repo for a fix so we can roll with this code and actually deprecate this endpoint. Would love a set of eyes and any historical info around this part of the code base

[codingllama]

Yep, cleaning up those "DELETE IN" lines without double-checking can be a risky move. I almost pointed it out, but OTOH we do want people to address them. Thanks for following up on it.

[sfreiberg]

Apologies for this. I remember explicitly testing this locally before I committed but must have forgot to restart the server. In retrospect I should have done this in a separate PR and only on master. Also for these kinds of comments we should open an issue and link to it in the comment so that history is easy for anybody to find.

[Joerger]

@sfreiberg Is there a reason why POST /webapi/ssh/certs was not given a rate limiter? This is the endpoint used for normal Password/OTP login. If not, I plan on adding the limiter as part of #21005.

[jentfoo]

@Joerger I filled this issue to track the additional rate limit gap you mentioned: https://github.com/gravitational/teleport-private/issues/403

Let me know if I can help with that change as part of #21005