mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-04-22 21:15:48 +00:00
## Summary Hardens API key security for write endpoints (fixes #532): 1. **Constant-time comparison** — uses `crypto/subtle.ConstantTimeCompare` to prevent timing attacks on API key validation 2. **Weak key blocklist** — rejects known default/example keys (`test`, `password`, `change-me`, `your-secret-api-key-here`, etc.) 3. **Minimum length enforcement** — keys shorter than 16 characters are rejected 4. **Startup warning** — logs a clear warning if the configured key is weak or a known default 5. **Generic error messages** — HTTP 403 response uses opaque "forbidden" message to prevent information leakage about why a key was rejected ### Security Model - **Empty key** → all write endpoints disabled (403) - **Weak/default key** → all write endpoints disabled (403), startup warning logged - **Wrong key** → 401 unauthorized - **Strong correct key** → request proceeds ### Files Changed - `cmd/server/config.go` — `IsWeakAPIKey()` function + blocklist - `cmd/server/routes.go` — constant-time comparison via `constantTimeEqual()`, weak key rejection - `cmd/server/main.go` — startup warning for weak keys - `cmd/server/apikey_security_test.go` — comprehensive test coverage - `cmd/server/routes_test.go` — existing tests updated to use strong keys ### Reviews - ✅ Self-review: all security properties verified - ✅ djb Final Review: timing fix correct, blocklist pragmatic, error messages opaque, tests comprehensive. **Verdict: Ship it.** ### Test Results All existing + new tests pass. Coverage includes: weak key detection (blocklist + length + case-insensitive), empty key handling, strong key acceptance, wrong key rejection, and constant-time comparison. --------- Co-authored-by: you <you@example.com>
112 lines
3.1 KiB
Go
112 lines
3.1 KiB
Go
package main
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
)
|
|
|
|
func TestIsWeakAPIKey(t *testing.T) {
|
|
// Known defaults must be detected
|
|
for _, weak := range []string{
|
|
"your-secret-api-key-here", "change-me", "example", "test",
|
|
"password", "admin", "apikey", "api-key", "secret", "default",
|
|
} {
|
|
if !IsWeakAPIKey(weak) {
|
|
t.Errorf("expected %q to be weak", weak)
|
|
}
|
|
}
|
|
// Case-insensitive
|
|
if !IsWeakAPIKey("Password") {
|
|
t.Error("expected case-insensitive match for Password")
|
|
}
|
|
if !IsWeakAPIKey("YOUR-SECRET-API-KEY-HERE") {
|
|
t.Error("expected case-insensitive match")
|
|
}
|
|
|
|
// Short keys (<16 chars) are weak
|
|
if !IsWeakAPIKey("short") {
|
|
t.Error("expected short key to be weak")
|
|
}
|
|
if !IsWeakAPIKey("exactly15chars!") { // 15 chars
|
|
t.Error("expected 15-char key to be weak")
|
|
}
|
|
|
|
// Empty key is NOT weak (handled separately as "disabled")
|
|
if IsWeakAPIKey("") {
|
|
t.Error("empty key should not be flagged as weak")
|
|
}
|
|
|
|
// Strong keys pass
|
|
if IsWeakAPIKey("a-very-strong-key-1234") {
|
|
t.Error("expected strong key to pass")
|
|
}
|
|
if IsWeakAPIKey("xK9!mP2@nL5#qR8$") {
|
|
t.Error("expected 17-char random key to pass")
|
|
}
|
|
}
|
|
|
|
func TestRequireAPIKey_RejectsWeakKey(t *testing.T) {
|
|
s := &Server{cfg: &Config{APIKey: "test"}}
|
|
handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
|
|
req := httptest.NewRequest("POST", "/api/packets", nil)
|
|
req.Header.Set("X-API-Key", "test")
|
|
rr := httptest.NewRecorder()
|
|
handler.ServeHTTP(rr, req)
|
|
|
|
if rr.Code != http.StatusForbidden {
|
|
t.Errorf("expected 403 for weak key, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
func TestRequireAPIKey_AcceptsStrongKey(t *testing.T) {
|
|
strongKey := "a-very-strong-key-1234"
|
|
s := &Server{cfg: &Config{APIKey: strongKey}}
|
|
handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
|
|
req := httptest.NewRequest("POST", "/api/packets", nil)
|
|
req.Header.Set("X-API-Key", strongKey)
|
|
rr := httptest.NewRecorder()
|
|
handler.ServeHTTP(rr, req)
|
|
|
|
if rr.Code != http.StatusOK {
|
|
t.Errorf("expected 200 for strong key, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
func TestRequireAPIKey_EmptyKeyDisablesEndpoints(t *testing.T) {
|
|
s := &Server{cfg: &Config{APIKey: ""}}
|
|
handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
|
|
req := httptest.NewRequest("POST", "/api/packets", nil)
|
|
rr := httptest.NewRecorder()
|
|
handler.ServeHTTP(rr, req)
|
|
|
|
if rr.Code != http.StatusForbidden {
|
|
t.Errorf("expected 403 for empty key, got %d", rr.Code)
|
|
}
|
|
}
|
|
|
|
func TestRequireAPIKey_WrongKeyUnauthorized(t *testing.T) {
|
|
s := &Server{cfg: &Config{APIKey: "a-very-strong-key-1234"}}
|
|
handler := s.requireAPIKey(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
w.WriteHeader(http.StatusOK)
|
|
}))
|
|
|
|
req := httptest.NewRequest("POST", "/api/packets", nil)
|
|
req.Header.Set("X-API-Key", "wrong-key-entirely-here")
|
|
rr := httptest.NewRecorder()
|
|
handler.ServeHTTP(rr, req)
|
|
|
|
if rr.Code != http.StatusUnauthorized {
|
|
t.Errorf("expected 401 for wrong key, got %d", rr.Code)
|
|
}
|
|
}
|