Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit423ac04

Browse files
authored
coderd: tighten /login rate limiting (#4432)
* coderd: tighten /login rate limit* coderd: add Bypass rate limit header
1 parent43f199a commit423ac04

File tree

6 files changed

+211
-20
lines changed

6 files changed

+211
-20
lines changed

‎.golangci.yaml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,15 @@ linters:
235235
-noctx
236236
-paralleltest
237237
-revive
238-
-rowserrcheck
239-
-sqlclosecheck
238+
239+
# These don't work until the following issue is solved.
240+
# https://github.com/golangci/golangci-lint/issues/2649
241+
# - rowserrcheck
242+
# - sqlclosecheck
243+
# - structcheck
244+
# - wastedassign
245+
240246
-staticcheck
241-
-structcheck
242247
-tenv
243248
# In Go, it's possible for a package to test it's internal functionality
244249
# without testing any exported functions. This is enabled to promote
@@ -253,4 +258,3 @@ linters:
253258
-unconvert
254259
-unused
255260
-varcheck
256-
-wastedassign

‎coderd/coderd.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func New(options *Options) *API {
204204
// app URL. If it is, it will serve that application.
205205
api.handleSubdomainApplications(
206206
// Middleware to impose on the served application.
207-
httpmw.RateLimitPerMinute(options.APIRateLimit),
207+
httpmw.RateLimit(options.APIRateLimit,time.Minute),
208208
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
209209
DB:options.Database,
210210
OAuth2Configs:oauthConfigs,
@@ -229,7 +229,7 @@ func New(options *Options) *API {
229229
apps:=func(r chi.Router) {
230230
r.Use(
231231
tracing.Middleware(api.TracerProvider),
232-
httpmw.RateLimitPerMinute(options.APIRateLimit),
232+
httpmw.RateLimit(options.APIRateLimit,time.Minute),
233233
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
234234
DB:options.Database,
235235
OAuth2Configs:oauthConfigs,
@@ -267,7 +267,7 @@ func New(options *Options) *API {
267267
r.Use(
268268
tracing.Middleware(api.TracerProvider),
269269
// Specific routes can specify smaller limits.
270-
httpmw.RateLimitPerMinute(options.APIRateLimit),
270+
httpmw.RateLimit(options.APIRateLimit,time.Minute),
271271
)
272272
r.Get("/",func(w http.ResponseWriter,r*http.Request) {
273273
httpapi.Write(r.Context(),w,http.StatusOK, codersdk.Response{
@@ -304,7 +304,7 @@ func New(options *Options) *API {
304304
apiKeyMiddleware,
305305
// This number is arbitrary, but reading/writing
306306
// file content is expensive so it should be small.
307-
httpmw.RateLimitPerMinute(12),
307+
httpmw.RateLimit(12,time.Minute),
308308
)
309309
r.Get("/{fileID}",api.fileByID)
310310
r.Post("/",api.postFile)
@@ -391,7 +391,15 @@ func New(options *Options) *API {
391391
r.Route("/users",func(r chi.Router) {
392392
r.Get("/first",api.firstUser)
393393
r.Post("/first",api.postFirstUser)
394-
r.Post("/login",api.postLogin)
394+
r.Group(func(r chi.Router) {
395+
// We use a tight limit for password login to protect
396+
// against audit-log write DoS, pbkdf2 DoS, and simple
397+
// brute-force attacks.
398+
//
399+
// Making this too small can break tests.
400+
r.Use(httpmw.RateLimit(60,time.Minute))
401+
r.Post("/login",api.postLogin)
402+
})
395403
r.Get("/authmethods",api.userAuthMethods)
396404
r.Route("/oauth2",func(r chi.Router) {
397405
r.Route("/github",func(r chi.Router) {

‎coderd/httpmw/apikey_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,16 +631,20 @@ func TestAPIKey(t *testing.T) {
631631
})
632632
}
633633

634-
funccreateUser(ctx context.Context,t*testing.T,db database.Store) database.User {
635-
user,err:=db.InsertUser(ctx, database.InsertUserParams{
634+
funccreateUser(ctx context.Context,t*testing.T,db database.Store,opts...func(u*database.InsertUserParams)) database.User {
635+
insert:= database.InsertUserParams{
636636
ID:uuid.New(),
637637
Email:"email@coder.com",
638638
Username:"username",
639639
HashedPassword: []byte{},
640640
CreatedAt:time.Now(),
641641
UpdatedAt:time.Now(),
642642
RBACRoles: []string{},
643-
})
643+
}
644+
for_,opt:=rangeopts {
645+
opt(&insert)
646+
}
647+
user,err:=db.InsertUser(ctx,insert)
644648
require.NoError(t,err,"create user")
645649
returnuser
646650
}

‎coderd/httpmw/ratelimit.go

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,71 @@
11
package httpmw
22

33
import (
4+
"fmt"
45
"net/http"
6+
"strconv"
57
"time"
68

79
"github.com/go-chi/httprate"
10+
"golang.org/x/xerrors"
811

912
"github.com/coder/coder/coderd/database"
1013
"github.com/coder/coder/coderd/httpapi"
14+
"github.com/coder/coder/coderd/rbac"
1115
"github.com/coder/coder/codersdk"
16+
"github.com/coder/coder/cryptorand"
1217
)
1318

14-
//RateLimitPerMinute returns a handler that limits requests per-minute based
19+
//RateLimit returns a handler that limits requests per-minute based
1520
// on IP, endpoint, and user ID (if available).
16-
funcRateLimitPerMinute(countint)func(http.Handler) http.Handler {
21+
funcRateLimit(countint,window time.Duration)func(http.Handler) http.Handler {
1722
// -1 is no rate limit
1823
ifcount<=0 {
1924
returnfunc(handler http.Handler) http.Handler {
2025
returnhandler
2126
}
2227
}
28+
2329
returnhttprate.Limit(
2430
count,
25-
1*time.Minute,
31+
window,
2632
httprate.WithKeyFuncs(func(r*http.Request) (string,error) {
2733
// Prioritize by user, but fallback to IP.
2834
apiKey,ok:=r.Context().Value(apiKeyContextKey{}).(database.APIKey)
29-
ifok {
35+
if!ok {
36+
returnhttprate.KeyByIP(r)
37+
}
38+
39+
ifok,_:=strconv.ParseBool(r.Header.Get(codersdk.BypassRatelimitHeader));!ok {
40+
// No bypass attempt, just ratelimit.
3041
returnapiKey.UserID.String(),nil
3142
}
32-
returnhttprate.KeyByIP(r)
43+
44+
// Allow Owner to bypass rate limiting for load tests
45+
// and automation.
46+
auth:=UserAuthorization(r)
47+
48+
// We avoid using rbac.Authorizer since rego is CPU-intensive
49+
// and undermines the DoS-prevention goal of the rate limiter.
50+
for_,role:=rangeauth.Roles {
51+
ifrole==rbac.RoleOwner() {
52+
// HACK: use a random key each time to
53+
// de facto disable rate limiting. The
54+
// `httprate` package has no
55+
// support for selectively changing the limit
56+
// for particular keys.
57+
returncryptorand.String(16)
58+
}
59+
}
60+
61+
returnapiKey.UserID.String(),xerrors.Errorf(
62+
"%q provided but user is not %v",
63+
codersdk.BypassRatelimitHeader,rbac.RoleOwner(),
64+
)
3365
},httprate.KeyByEndpoint),
3466
httprate.WithLimitHandler(func(w http.ResponseWriter,r*http.Request) {
3567
httpapi.Write(r.Context(),w,http.StatusTooManyRequests, codersdk.Response{
36-
Message:"You've been rate limited for sendingtoo manyrequests!",
68+
Message:fmt.Sprintf("You've been rate limited for sendingmore than %vrequests in %v.",count,window),
3769
})
3870
}),
3971
)

‎coderd/httpmw/ratelimit_test.go

Lines changed: 142 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,170 @@
11
package httpmw_test
22

33
import (
4+
"context"
5+
"crypto/sha256"
6+
"fmt"
7+
"math/rand"
8+
"net"
49
"net/http"
510
"net/http/httptest"
611
"testing"
12+
"time"
713

814
"github.com/go-chi/chi/v5"
15+
"github.com/google/uuid"
916
"github.com/stretchr/testify/require"
1017

18+
"github.com/coder/coder/coderd/database"
19+
"github.com/coder/coder/coderd/database/databasefake"
1120
"github.com/coder/coder/coderd/httpmw"
21+
"github.com/coder/coder/coderd/rbac"
22+
"github.com/coder/coder/codersdk"
1223
"github.com/coder/coder/testutil"
1324
)
1425

26+
funcinsertAPIKey(ctx context.Context,t*testing.T,db database.Store,userID uuid.UUID)string {
27+
id,secret:=randomAPIKeyParts()
28+
hashed:=sha256.Sum256([]byte(secret))
29+
30+
_,err:=db.InsertAPIKey(ctx, database.InsertAPIKeyParams{
31+
ID:id,
32+
HashedSecret:hashed[:],
33+
LastUsed:database.Now().AddDate(0,0,-1),
34+
ExpiresAt:database.Now().AddDate(0,0,1),
35+
UserID:userID,
36+
LoginType:database.LoginTypePassword,
37+
Scope:database.APIKeyScopeAll,
38+
})
39+
require.NoError(t,err)
40+
41+
returnfmt.Sprintf("%s-%s",id,secret)
42+
}
43+
44+
funcrandRemoteAddr()string {
45+
varb [4]byte
46+
// nolint:gosec
47+
rand.Read(b[:])
48+
// nolint:gosec
49+
returnfmt.Sprintf("%s:%v",net.IP(b[:]).String(),rand.Int31()%(1<<16))
50+
}
51+
1552
funcTestRateLimit(t*testing.T) {
1653
t.Parallel()
17-
t.Run("NoUser",func(t*testing.T) {
54+
t.Run("NoUserSucceeds",func(t*testing.T) {
55+
t.Parallel()
56+
rtr:=chi.NewRouter()
57+
rtr.Use(httpmw.RateLimit(5,time.Second))
58+
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
59+
rw.WriteHeader(http.StatusOK)
60+
})
61+
62+
require.Eventually(t,func()bool {
63+
req:=httptest.NewRequest("GET","/",nil)
64+
rec:=httptest.NewRecorder()
65+
rtr.ServeHTTP(rec,req)
66+
resp:=rec.Result()
67+
deferresp.Body.Close()
68+
returnresp.StatusCode==http.StatusTooManyRequests
69+
},testutil.WaitShort,testutil.IntervalFast)
70+
})
71+
72+
t.Run("RandomIPs",func(t*testing.T) {
73+
t.Parallel()
74+
rtr:=chi.NewRouter()
75+
rtr.Use(httpmw.RateLimit(5,time.Second))
76+
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
77+
rw.WriteHeader(http.StatusOK)
78+
})
79+
80+
require.Never(t,func()bool {
81+
req:=httptest.NewRequest("GET","/",nil)
82+
rec:=httptest.NewRecorder()
83+
req.RemoteAddr=randRemoteAddr()
84+
rtr.ServeHTTP(rec,req)
85+
resp:=rec.Result()
86+
deferresp.Body.Close()
87+
returnresp.StatusCode==http.StatusTooManyRequests
88+
},testutil.WaitShort,testutil.IntervalFast)
89+
})
90+
91+
t.Run("RegularUser",func(t*testing.T) {
1892
t.Parallel()
93+
94+
ctx:=context.Background()
95+
96+
db:=databasefake.New()
97+
98+
u:=createUser(ctx,t,db)
99+
key:=insertAPIKey(ctx,t,db,u.ID)
100+
19101
rtr:=chi.NewRouter()
20-
rtr.Use(httpmw.RateLimitPerMinute(5))
102+
rtr.Use(httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
103+
DB:db,
104+
Optional:false,
105+
}))
106+
107+
rtr.Use(httpmw.RateLimit(5,time.Second))
21108
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
22109
rw.WriteHeader(http.StatusOK)
23110
})
24111

112+
// Bypass must fail
113+
req:=httptest.NewRequest("GET","/",nil)
114+
req.Header.Set(codersdk.SessionCustomHeader,key)
115+
req.Header.Set(codersdk.BypassRatelimitHeader,"true")
116+
rec:=httptest.NewRecorder()
117+
// Assert we're not using IP address.
118+
req.RemoteAddr=randRemoteAddr()
119+
rtr.ServeHTTP(rec,req)
120+
resp:=rec.Result()
121+
deferresp.Body.Close()
122+
require.Equal(t,http.StatusPreconditionRequired,resp.StatusCode)
123+
25124
require.Eventually(t,func()bool {
26125
req:=httptest.NewRequest("GET","/",nil)
126+
req.Header.Set(codersdk.SessionCustomHeader,key)
127+
rec:=httptest.NewRecorder()
128+
// Assert we're not using IP address.
129+
req.RemoteAddr=randRemoteAddr()
130+
rtr.ServeHTTP(rec,req)
131+
resp:=rec.Result()
132+
deferresp.Body.Close()
133+
returnresp.StatusCode==http.StatusTooManyRequests
134+
},testutil.WaitShort,testutil.IntervalFast)
135+
})
136+
137+
t.Run("OwnerBypass",func(t*testing.T) {
138+
t.Parallel()
139+
140+
ctx:=context.Background()
141+
142+
db:=databasefake.New()
143+
144+
u:=createUser(ctx,t,db,func(u*database.InsertUserParams) {
145+
u.RBACRoles= []string{rbac.RoleOwner()}
146+
})
147+
148+
key:=insertAPIKey(ctx,t,db,u.ID)
149+
150+
rtr:=chi.NewRouter()
151+
rtr.Use(httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
152+
DB:db,
153+
Optional:false,
154+
}))
155+
156+
rtr.Use(httpmw.RateLimit(5,time.Second))
157+
rtr.Get("/",func(rw http.ResponseWriter,r*http.Request) {
158+
rw.WriteHeader(http.StatusOK)
159+
})
160+
161+
require.Never(t,func()bool {
162+
req:=httptest.NewRequest("GET","/",nil)
163+
req.Header.Set(codersdk.SessionCustomHeader,key)
164+
req.Header.Set(codersdk.BypassRatelimitHeader,"true")
27165
rec:=httptest.NewRecorder()
166+
// Assert we're not using IP address.
167+
req.RemoteAddr=randRemoteAddr()
28168
rtr.ServeHTTP(rec,req)
29169
resp:=rec.Result()
30170
deferresp.Body.Close()

‎codersdk/client.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ const (
2424
SessionCustomHeader="Coder-Session-Token"
2525
OAuth2StateKey="oauth_state"
2626
OAuth2RedirectKey="oauth_redirect"
27+
28+
// nolint: gosec
29+
BypassRatelimitHeader="X-Coder-Bypass-Ratelimit"
2730
)
2831

2932
// New creates a Coder client for the provided URL.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp