coderd: tighten /login rate limit

ammario · ammario · commit 472cf6e4e41a · 2022-10-08T18:26:54.000Z
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -187,7 +187,7 @@ func New(options *Options) *API {
 		// app URL. If it is, it will serve that application.
 		api.handleSubdomainApplications(
 			// Middleware to impose on the served application.
-			httpmw.RateLimitPerMinute(options.APIRateLimit),
+			httpmw.RateLimit(options.APIRateLimit, time.Minute),
 			httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
 				DB:            options.Database,
 				OAuth2Configs: oauthConfigs,
@@ -212,7 +212,7 @@ func New(options *Options) *API {
 	apps := func(r chi.Router) {
 		r.Use(
 			tracing.Middleware(api.TracerProvider),
-			httpmw.RateLimitPerMinute(options.APIRateLimit),
+			httpmw.RateLimit(options.APIRateLimit, time.Minute),
 			apiKeyMiddlewareRedirect,
 			httpmw.ExtractUserParam(api.Database),
 			// Extracts the <workspace.agent> from the url
@@ -240,7 +240,7 @@ func New(options *Options) *API {
 		r.Use(
 			tracing.Middleware(api.TracerProvider),
 			// Specific routes can specify smaller limits.
-			httpmw.RateLimitPerMinute(options.APIRateLimit),
+			httpmw.RateLimit(options.APIRateLimit, time.Minute),
 		)
 		r.Get("/", func(w http.ResponseWriter, r *http.Request) {
 			httpapi.Write(r.Context(), w, http.StatusOK, codersdk.Response{
@@ -273,7 +273,7 @@ func New(options *Options) *API {
 				apiKeyMiddleware,
 				// This number is arbitrary, but reading/writing
 				// file content is expensive so it should be small.
-				httpmw.RateLimitPerMinute(12),
+				httpmw.RateLimit(12, time.Minute),
 			)
 			r.Get("/{hash}", api.fileByHash)
 			r.Post("/", api.postFile)
@@ -359,7 +359,13 @@ func New(options *Options) *API {
 		r.Route("/users", func(r chi.Router) {
 			r.Get("/first", api.firstUser)
 			r.Post("/first", api.postFirstUser)
-			r.Post("/login", api.postLogin)
+			r.Group(func(r chi.Router) {
+				// We use a tight limit for password login to protect
+				// against audit-log write DoS, pbkdf2 DoS, and simple
+				// brute-force attacks.
+				r.Use(httpmw.RateLimit(10, time.Minute))
+				r.Post("/login", api.postLogin)
+			})
 			r.Get("/authmethods", api.userAuthMethods)
 			r.Route("/oauth2", func(r chi.Router) {
 				r.Route("/github", func(r chi.Router) {
diff --git a/coderd/httpmw/ratelimit.go b/coderd/httpmw/ratelimit.go
@@ -11,9 +11,9 @@ import (
 	"github.com/coder/coder/codersdk"
 )
 
-// RateLimitPerMinute returns a handler that limits requests per-minute based
+// RateLimit returns a handler that limits requests per-minute based
 // on IP, endpoint, and user ID (if available).
-func RateLimitPerMinute(count int) func(http.Handler) http.Handler {
+func RateLimit(count int, window time.Duration) func(http.Handler) http.Handler {
 	// -1 is no rate limit
 	if count <= 0 {
 		return func(handler http.Handler) http.Handler {
@@ -22,7 +22,7 @@ func RateLimitPerMinute(count int) func(http.Handler) http.Handler {
 	}
 	return httprate.Limit(
 		count,
-		1*time.Minute,
+		window,
 		httprate.WithKeyFuncs(func(r *http.Request) (string, error) {
 			// Prioritize by user, but fallback to IP.
 			apiKey, ok := r.Context().Value(apiKeyContextKey{}).(database.APIKey)
diff --git a/coderd/httpmw/ratelimit_test.go b/coderd/httpmw/ratelimit_test.go
@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/go-chi/chi/v5"
 	"github.com/stretchr/testify/require"
@@ -17,7 +18,7 @@ func TestRateLimit(t *testing.T) {
 	t.Run("NoUser", func(t *testing.T) {
 		t.Parallel()
 		rtr := chi.NewRouter()
-		rtr.Use(httpmw.RateLimitPerMinute(5))
+		rtr.Use(httpmw.RateLimit(5, time.Second))
 		rtr.Get("/", func(rw http.ResponseWriter, r *http.Request) {
 			rw.WriteHeader(http.StatusOK)
 		})
