diff --git a/README.md b/README.md index 2f48e23..e0e7332 100644 --- a/README.md +++ b/README.md @@ -1834,6 +1834,8 @@ directory is also loaded automatically via | `METRICS_USERNAME` | string | `""` | Basic auth username for `/metrics` endpoint. If empty, metrics endpoint is disabled. | | `METRICS_PASSWORD` | string | `""` | Basic auth password for `/metrics` endpoint | | `NEOIRC_HASHCASH_BITS` | int | `20` | Required hashcash proof-of-work difficulty (leading zero bits in SHA-256) for session creation. Set to `0` to disable. | +| `LOGIN_RATE_LIMIT` | float | `1` | Allowed login attempts per second per IP address. | +| `LOGIN_RATE_BURST` | int | `5` | Maximum burst of login attempts per IP before rate limiting kicks in. | | `MAINTENANCE_MODE` | bool | `false` | Maintenance mode flag (reserved) | ### Example `.env` file @@ -2224,6 +2226,28 @@ creating one session pays once and keeps their session. - **Language-agnostic**: SHA-256 is available in every programming language. The proof computation is trivially implementable in any client. +### Login Rate Limiting + +The login endpoint (`POST /api/v1/login`) has per-IP rate limiting to prevent +brute-force password attacks. This uses a token-bucket algorithm +(`golang.org/x/time/rate`) with configurable rate and burst. + +| Environment Variable | Default | Description | +|---------------------|---------|-------------| +| `LOGIN_RATE_LIMIT` | `1` | Allowed login attempts per second per IP | +| `LOGIN_RATE_BURST` | `5` | Maximum burst of login attempts per IP | + +When the limit is exceeded, the server returns **429 Too Many Requests** with a +`Retry-After: 1` header. Stale per-IP entries are automatically cleaned up +every 10 minutes. + +**Why rate limits here but not on session creation?** Session creation is +protected by hashcash proof-of-work (stateless, no IP tracking needed). Login +involves bcrypt password verification against a registered account — a +fundamentally different threat model where an attacker targets a specific +account. Per-IP rate limiting is appropriate here because the cost of a wrong +guess is borne by the server (bcrypt), not the client. + --- ## Roadmap diff --git a/go.mod b/go.mod index 27f0bab..b7824ee 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/spf13/viper v1.21.0 go.uber.org/fx v1.24.0 golang.org/x/crypto v0.48.0 + golang.org/x/time v0.6.0 modernc.org/sqlite v1.45.0 ) diff --git a/go.sum b/go.sum index 49b60c1..902e77d 100644 --- a/go.sum +++ b/go.sum @@ -151,6 +151,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.34.0 h1:oL/Qq0Kdaqxa1KbNeMKwQq0reLCCaFtqu2eNuSeNHbk= golang.org/x/text v0.34.0/go.mod h1:homfLqTYRFyVYemLBFl5GgL/DWEiH5wcsQ5gSh1yziA= +golang.org/x/time v0.6.0 h1:eTDhh4ZXt5Qf0augr54TN6suAUudPcawVZeIAPU7D4U= +golang.org/x/time v0.6.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/internal/config/config.go b/internal/config/config.go index da29f1e..9393d3e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,6 +46,8 @@ type Config struct { FederationKey string SessionIdleTimeout string HashcashBits int + LoginRateLimit float64 + LoginRateBurst int params *Params log *slog.Logger } @@ -78,6 +80,8 @@ func New( viper.SetDefault("FEDERATION_KEY", "") viper.SetDefault("SESSION_IDLE_TIMEOUT", "720h") viper.SetDefault("NEOIRC_HASHCASH_BITS", "20") + viper.SetDefault("LOGIN_RATE_LIMIT", "1") + viper.SetDefault("LOGIN_RATE_BURST", "5") err := viper.ReadInConfig() if err != nil { @@ -104,6 +108,8 @@ func New( FederationKey: viper.GetString("FEDERATION_KEY"), SessionIdleTimeout: viper.GetString("SESSION_IDLE_TIMEOUT"), HashcashBits: viper.GetInt("NEOIRC_HASHCASH_BITS"), + LoginRateLimit: viper.GetFloat64("LOGIN_RATE_LIMIT"), + LoginRateBurst: viper.GetInt("LOGIN_RATE_BURST"), log: log, params: ¶ms, } diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index e4da9cb..46240af 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -1961,6 +1961,121 @@ func TestSessionStillWorks(t *testing.T) { } } +func TestLoginRateLimitExceeded(t *testing.T) { + tserver := newTestServer(t) + + // Exhaust the burst (default: 5 per IP) using + // nonexistent users. These fail fast (no bcrypt), + // preventing token replenishment between requests. + for range 5 { + loginBody, mErr := json.Marshal( + map[string]string{ + "nick": "nosuchuser", + "password": "doesnotmatter", + }, + ) + if mErr != nil { + t.Fatal(mErr) + } + + loginResp, rErr := doRequest( + t, + http.MethodPost, + tserver.url("/api/v1/login"), + bytes.NewReader(loginBody), + ) + if rErr != nil { + t.Fatal(rErr) + } + + _ = loginResp.Body.Close() + } + + // The next request should be rate-limited. + loginBody, err := json.Marshal(map[string]string{ + "nick": "nosuchuser", "password": "doesnotmatter", + }) + if err != nil { + t.Fatal(err) + } + + resp, err := doRequest( + t, + http.MethodPost, + tserver.url("/api/v1/login"), + bytes.NewReader(loginBody), + ) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusTooManyRequests { + t.Fatalf( + "expected 429, got %d", + resp.StatusCode, + ) + } + + retryAfter := resp.Header.Get("Retry-After") + if retryAfter == "" { + t.Fatal("expected Retry-After header") + } +} + +func TestLoginRateLimitAllowsNormalUse(t *testing.T) { + tserver := newTestServer(t) + + // Register a user. + regBody, err := json.Marshal(map[string]string{ + "nick": "normaluser", "password": "password123", + }) + if err != nil { + t.Fatal(err) + } + + resp, err := doRequest( + t, + http.MethodPost, + tserver.url("/api/v1/register"), + bytes.NewReader(regBody), + ) + if err != nil { + t.Fatal(err) + } + + _ = resp.Body.Close() + + // A single login should succeed without rate limiting. + loginBody, err := json.Marshal(map[string]string{ + "nick": "normaluser", "password": "password123", + }) + if err != nil { + t.Fatal(err) + } + + resp2, err := doRequest( + t, + http.MethodPost, + tserver.url("/api/v1/login"), + bytes.NewReader(loginBody), + ) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = resp2.Body.Close() }() + + if resp2.StatusCode != http.StatusOK { + respBody, _ := io.ReadAll(resp2.Body) + t.Fatalf( + "expected 200, got %d: %s", + resp2.StatusCode, respBody, + ) + } +} + func TestNickBroadcastToChannels(t *testing.T) { tserver := newTestServer(t) aliceToken := tserver.createSession("nick_a") diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index cd99bf7..83cffa1 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -2,6 +2,7 @@ package handlers import ( "encoding/json" + "net" "net/http" "strings" @@ -10,6 +11,33 @@ import ( const minPasswordLength = 8 +// clientIP extracts the client IP address from the request. +// It checks X-Forwarded-For and X-Real-IP headers before +// falling back to RemoteAddr. +func clientIP(request *http.Request) string { + if forwarded := request.Header.Get("X-Forwarded-For"); forwarded != "" { + // X-Forwarded-For may contain a comma-separated list; + // the first entry is the original client. + parts := strings.SplitN(forwarded, ",", 2) //nolint:mnd // split into two parts + ip := strings.TrimSpace(parts[0]) + + if ip != "" { + return ip + } + } + + if realIP := request.Header.Get("X-Real-IP"); realIP != "" { + return strings.TrimSpace(realIP) + } + + host, _, err := net.SplitHostPort(request.RemoteAddr) + if err != nil { + return request.RemoteAddr + } + + return host +} + // HandleRegister creates a new user with a password. func (hdlr *Handlers) HandleRegister() http.HandlerFunc { return func( @@ -134,6 +162,21 @@ func (hdlr *Handlers) handleLogin( writer http.ResponseWriter, request *http.Request, ) { + ip := clientIP(request) + + if !hdlr.loginLimiter.Allow(ip) { + writer.Header().Set( + "Retry-After", "1", + ) + hdlr.respondError( + writer, request, + "too many login attempts, try again later", + http.StatusTooManyRequests, + ) + + return + } + type loginRequest struct { Nick string `json:"nick"` Password string `json:"password"` diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 6a76ae3..e53dbf2 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -16,6 +16,7 @@ import ( "git.eeqj.de/sneak/neoirc/internal/hashcash" "git.eeqj.de/sneak/neoirc/internal/healthcheck" "git.eeqj.de/sneak/neoirc/internal/logger" + "git.eeqj.de/sneak/neoirc/internal/ratelimit" "go.uber.org/fx" ) @@ -41,6 +42,7 @@ type Handlers struct { hc *healthcheck.Healthcheck broker *broker.Broker hashcashVal *hashcash.Validator + loginLimiter *ratelimit.Limiter cancelCleanup context.CancelFunc } @@ -54,12 +56,23 @@ func New( resource = "neoirc" } + loginRate := params.Config.LoginRateLimit + if loginRate <= 0 { + loginRate = ratelimit.DefaultRate + } + + loginBurst := params.Config.LoginRateBurst + if loginBurst <= 0 { + loginBurst = ratelimit.DefaultBurst + } + hdlr := &Handlers{ //nolint:exhaustruct // cancelCleanup set in startCleanup - params: ¶ms, - log: params.Logger.Get(), - hc: params.Healthcheck, - broker: broker.New(), - hashcashVal: hashcash.NewValidator(resource), + params: ¶ms, + log: params.Logger.Get(), + hc: params.Healthcheck, + broker: broker.New(), + hashcashVal: hashcash.NewValidator(resource), + loginLimiter: ratelimit.New(loginRate, loginBurst), } lifecycle.Append(fx.Hook{ @@ -151,6 +164,10 @@ func (hdlr *Handlers) stopCleanup() { if hdlr.cancelCleanup != nil { hdlr.cancelCleanup() } + + if hdlr.loginLimiter != nil { + hdlr.loginLimiter.Stop() + } } func (hdlr *Handlers) cleanupLoop(ctx context.Context) { diff --git a/internal/ratelimit/ratelimit.go b/internal/ratelimit/ratelimit.go new file mode 100644 index 0000000..a85fd63 --- /dev/null +++ b/internal/ratelimit/ratelimit.go @@ -0,0 +1,122 @@ +// Package ratelimit provides per-IP rate limiting for HTTP endpoints. +package ratelimit + +import ( + "sync" + "time" + + "golang.org/x/time/rate" +) + +const ( + // DefaultRate is the default number of allowed requests per second. + DefaultRate = 1.0 + + // DefaultBurst is the default maximum burst size. + DefaultBurst = 5 + + // DefaultSweepInterval controls how often stale entries are pruned. + DefaultSweepInterval = 10 * time.Minute + + // DefaultEntryTTL is how long an unused entry lives before eviction. + DefaultEntryTTL = 15 * time.Minute +) + +// entry tracks a per-IP rate limiter and when it was last used. +type entry struct { + limiter *rate.Limiter + lastSeen time.Time +} + +// Limiter manages per-key rate limiters with automatic cleanup +// of stale entries. +type Limiter struct { + mu sync.Mutex + entries map[string]*entry + rate rate.Limit + burst int + entryTTL time.Duration + stopCh chan struct{} +} + +// New creates a new per-key rate Limiter. +// The ratePerSec parameter sets how many requests per second are +// allowed per key. The burst parameter sets the maximum number of +// requests that can be made in a single burst. +func New(ratePerSec float64, burst int) *Limiter { + limiter := &Limiter{ + mu: sync.Mutex{}, + entries: make(map[string]*entry), + rate: rate.Limit(ratePerSec), + burst: burst, + entryTTL: DefaultEntryTTL, + stopCh: make(chan struct{}), + } + + go limiter.sweepLoop() + + return limiter +} + +// Allow reports whether a request from the given key should be +// allowed. It consumes one token from the key's rate limiter. +func (l *Limiter) Allow(key string) bool { + l.mu.Lock() + ent, exists := l.entries[key] + + if !exists { + ent = &entry{ + limiter: rate.NewLimiter(l.rate, l.burst), + lastSeen: time.Now(), + } + l.entries[key] = ent + } else { + ent.lastSeen = time.Now() + } + l.mu.Unlock() + + return ent.limiter.Allow() +} + +// Stop terminates the background sweep goroutine. +func (l *Limiter) Stop() { + close(l.stopCh) +} + +// Len returns the number of tracked keys (for testing). +func (l *Limiter) Len() int { + l.mu.Lock() + defer l.mu.Unlock() + + return len(l.entries) +} + +// sweepLoop periodically removes entries that haven't been seen +// within the TTL. +func (l *Limiter) sweepLoop() { + ticker := time.NewTicker(DefaultSweepInterval) + defer ticker.Stop() + + for { + select { + case <-ticker.C: + l.sweep() + case <-l.stopCh: + return + } + } +} + +// sweep removes stale entries. +func (l *Limiter) sweep() { + l.mu.Lock() + defer l.mu.Unlock() + + cutoff := time.Now().Add(-l.entryTTL) + + for key, ent := range l.entries { + if ent.lastSeen.Before(cutoff) { + delete(l.entries, key) + } + } +} diff --git a/internal/ratelimit/ratelimit_test.go b/internal/ratelimit/ratelimit_test.go new file mode 100644 index 0000000..38a8b8a --- /dev/null +++ b/internal/ratelimit/ratelimit_test.go @@ -0,0 +1,106 @@ +package ratelimit_test + +import ( + "testing" + + "git.eeqj.de/sneak/neoirc/internal/ratelimit" +) + +func TestNewCreatesLimiter(t *testing.T) { + t.Parallel() + + limiter := ratelimit.New(1.0, 5) + defer limiter.Stop() + + if limiter == nil { + t.Fatal("expected non-nil limiter") + } +} + +func TestAllowWithinBurst(t *testing.T) { + t.Parallel() + + limiter := ratelimit.New(1.0, 3) + defer limiter.Stop() + + for i := range 3 { + if !limiter.Allow("192.168.1.1") { + t.Fatalf( + "request %d should be allowed within burst", + i+1, + ) + } + } +} + +func TestAllowExceedsBurst(t *testing.T) { + t.Parallel() + + // Rate of 0 means no token replenishment, only burst. + limiter := ratelimit.New(0, 3) + defer limiter.Stop() + + for range 3 { + limiter.Allow("10.0.0.1") + } + + if limiter.Allow("10.0.0.1") { + t.Fatal("fourth request should be denied after burst exhausted") + } +} + +func TestAllowSeparateKeys(t *testing.T) { + t.Parallel() + + // Rate of 0, burst of 1 — only one request per key. + limiter := ratelimit.New(0, 1) + defer limiter.Stop() + + if !limiter.Allow("10.0.0.1") { + t.Fatal("first request for key A should be allowed") + } + + if !limiter.Allow("10.0.0.2") { + t.Fatal("first request for key B should be allowed") + } + + if limiter.Allow("10.0.0.1") { + t.Fatal("second request for key A should be denied") + } + + if limiter.Allow("10.0.0.2") { + t.Fatal("second request for key B should be denied") + } +} + +func TestLenTracksKeys(t *testing.T) { + t.Parallel() + + limiter := ratelimit.New(1.0, 5) + defer limiter.Stop() + + if limiter.Len() != 0 { + t.Fatalf("expected 0 entries, got %d", limiter.Len()) + } + + limiter.Allow("10.0.0.1") + limiter.Allow("10.0.0.2") + + if limiter.Len() != 2 { + t.Fatalf("expected 2 entries, got %d", limiter.Len()) + } + + // Same key again should not increase count. + limiter.Allow("10.0.0.1") + + if limiter.Len() != 2 { + t.Fatalf("expected 2 entries, got %d", limiter.Len()) + } +} + +func TestStopDoesNotPanic(t *testing.T) { + t.Parallel() + + limiter := ratelimit.New(1.0, 5) + limiter.Stop() +}