Own security review items from Phase C:
- Credential entropy: WebDAV account passwords and auto-generated share
passwords were generated with math/rand (predictable). util.RandString
now draws from crypto/rand like RandStringRunesCrypto; both call sites
are credentials, so no non-secret consumer is affected. {randomkeyN}
naming-policy magic vars intentionally keep math/rand.
- SSRF guard retest: ValidateExternalURL already unwraps IPv4-in-IPv6
transition forms (NAT64 64:ff9b::/96, 6to4, Teredo, IPv4-mapped,
IPv4-compatible) — the test suite only covered ::ffff:. Added blocked
vectors for each transition form wrapping loopback/private/metadata
targets, plus public-wrapped controls that must stay allowed.
- Auth rate limiting: login, 2FA, token refresh, register, password
reset (mail + apply), WebAuthn, SSO exchange and OAuth token exchange
had no throttle beyond optional captcha — the 6-digit OTP endpoint
being the worst case. New middleware.RateLimit is a fixed-window
counter in the shared KV store (MemoStore locally, Redis across
nodes), keyed by client IP, returning CodeRateLimited with
Retry-After.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pull/3582/head
Tomas Dvorak
2 weeks ago
parent
7e3f97f052
commit
7a3bc016ff
@ -0,0 +1,51 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"github.com/cloudreve/Cloudreve/v4/application/dependency"
|
||||
"github.com/cloudreve/Cloudreve/v4/pkg/serializer"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
const rateLimitPrefix = "rate_limit:"
|
||||
|
||||
// RateLimit applies a fixed-window rate limit to requests sharing the same
|
||||
// bucket key. Once `limit` requests arrive within `window`, further requests
|
||||
// are rejected until the window expires. State lives in the shared KV store,
|
||||
// so limits hold across cluster nodes when Redis is configured.
|
||||
//
|
||||
// The counter is approximate (read-then-write), which is acceptable for
|
||||
// abuse prevention — it bounds attempts to roughly `limit` per window.
|
||||
func RateLimit(limit int, window time.Duration, key func(*gin.Context) string) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
kv := dependency.FromContext(c).KV()
|
||||
bucket := rateLimitPrefix + key(c)
|
||||
|
||||
count := 0
|
||||
if raw, ok := kv.Get(bucket); ok {
|
||||
if v, ok := raw.(int); ok {
|
||||
count = v
|
||||
}
|
||||
}
|
||||
|
||||
if count >= limit {
|
||||
c.Header("Retry-After", strconv.Itoa(int(window.Seconds())))
|
||||
c.JSON(200, serializer.NewError(serializer.CodeRateLimited, "Too many requests, please try again later.", nil))
|
||||
c.Abort()
|
||||
return
|
||||
}
|
||||
|
||||
_ = kv.Set(bucket, count+1, int(window.Seconds()))
|
||||
c.Next()
|
||||
}
|
||||
}
|
||||
|
||||
// RateLimitByIP keys the rate limit on the client IP and a static bucket
|
||||
// name, e.g. RateLimitByIP("login", 10, time.Minute).
|
||||
func RateLimitByIP(bucket string, limit int, window time.Duration) gin.HandlerFunc {
|
||||
return RateLimit(limit, window, func(c *gin.Context) string {
|
||||
return bucket + ":" + c.ClientIP()
|
||||
})
|
||||
}
|
||||
@ -0,0 +1,123 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/cloudreve/Cloudreve/v4/application/dependency"
|
||||
"github.com/cloudreve/Cloudreve/v4/pkg/cache"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
var testEngine = func() *gin.Engine {
|
||||
e := gin.New()
|
||||
e.ContextWithFallback = true
|
||||
return e
|
||||
}()
|
||||
|
||||
func newRateLimitContext(t *testing.T, dep dependency.Dep, remoteAddr string) (*gin.Context, *httptest.ResponseRecorder) {
|
||||
t.Helper()
|
||||
w := httptest.NewRecorder()
|
||||
c := gin.CreateTestContextOnly(w, testEngine)
|
||||
req := httptest.NewRequest(http.MethodPost, "/session/token", nil)
|
||||
req.RemoteAddr = remoteAddr + ":12345"
|
||||
c.Request = req.WithContext(context.WithValue(req.Context(), dependency.DepCtx{}, dep))
|
||||
return c, w
|
||||
}
|
||||
|
||||
func TestRateLimitByIP(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
dep := dependency.NewDependency(dependency.WithKV(cache.NewMemoStore("", nil)))
|
||||
|
||||
handler := RateLimitByIP("login", 3, time.Minute)
|
||||
|
||||
for i := 0; i < 3; i++ {
|
||||
c, _ := newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler(c)
|
||||
if c.IsAborted() {
|
||||
t.Fatalf("request %d unexpectedly rejected", i+1)
|
||||
}
|
||||
}
|
||||
|
||||
c, w := newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler(c)
|
||||
if !c.IsAborted() {
|
||||
t.Fatal("request over limit was not rejected")
|
||||
}
|
||||
if w.Header().Get("Retry-After") != "60" {
|
||||
t.Fatalf("missing/incorrect Retry-After: %q", w.Header().Get("Retry-After"))
|
||||
}
|
||||
|
||||
// A different IP has its own bucket.
|
||||
c, _ = newRateLimitContext(t, dep, "192.0.2.2")
|
||||
handler(c)
|
||||
if c.IsAborted() {
|
||||
t.Fatal("unrelated IP shared the bucket")
|
||||
}
|
||||
|
||||
// A different bucket name on the same IP has its own counter.
|
||||
handler2 := RateLimitByIP("register", 1, time.Minute)
|
||||
c, _ = newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler2(c)
|
||||
if c.IsAborted() {
|
||||
t.Fatal("unrelated bucket shared the counter")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRateLimitExpiry(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
dep := dependency.NewDependency(dependency.WithKV(cache.NewMemoStore("", nil)))
|
||||
|
||||
handler := RateLimitByIP("login", 1, time.Second)
|
||||
|
||||
c, _ := newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler(c)
|
||||
if c.IsAborted() {
|
||||
t.Fatal("first request rejected")
|
||||
}
|
||||
|
||||
c, _ = newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler(c)
|
||||
if !c.IsAborted() {
|
||||
t.Fatal("second request not rejected")
|
||||
}
|
||||
|
||||
// Window expiry frees the bucket. MemoStore TTL is Unix-second granular
|
||||
// (item valid while Expires >= now), so a 1s window can live ~2s.
|
||||
time.Sleep(2100 * time.Millisecond)
|
||||
|
||||
c, _ = newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler(c)
|
||||
if c.IsAborted() {
|
||||
t.Fatal("request after window expiry still rejected")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRateLimitCountOverflow(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
kv := cache.NewMemoStore("", nil)
|
||||
dep := dependency.NewDependency(dependency.WithKV(kv))
|
||||
|
||||
// A non-int value in the bucket is treated as a fresh window.
|
||||
if err := kv.Set(rateLimitPrefix+"login:192.0.2.1", "garbage", 60); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
handler := RateLimitByIP("login", 1, time.Minute)
|
||||
c, _ := newRateLimitContext(t, dep, "192.0.2.1")
|
||||
handler(c)
|
||||
if c.IsAborted() {
|
||||
t.Fatal("corrupt bucket state rejected request")
|
||||
}
|
||||
|
||||
raw, ok := kv.Get(rateLimitPrefix + "login:192.0.2.1")
|
||||
if !ok {
|
||||
t.Fatal("bucket not persisted")
|
||||
}
|
||||
if _, isInt := raw.(int); !isInt {
|
||||
t.Fatalf("bucket value type drifted: %T", raw)
|
||||
}
|
||||
}
|
||||
Loading…
Reference in new issue