Skip to content

Commit c25e34c

Browse files
committed
fix(security): harden auth, input validation, and session management
- Add login rate limiting (5 attempts / 15 min per IP) - Add Secure flag on session cookies - Fix timing attack: always run bcrypt even if user not found - Log all login attempts (success/fail) with IP and username - Increase bcrypt cost from 10 to 12 - Reduce default session lifetime from 7 days to 2 hours - Add email validator (net/mail) and use in certbot RequestSSL - Add Filename validator (filepath.Clean) for path traversal protection
1 parent 472c491 commit c25e34c

6 files changed

Lines changed: 129 additions & 6 deletions

File tree

internal/auth/handler.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package auth
22

33
import (
4+
"log/slog"
45
"net/http"
56
"time"
67

@@ -9,10 +10,11 @@ import (
910

1011
type Handler struct {
1112
svc *Service
13+
rl *RateLimiter
1214
}
1315

1416
func NewHandler(svc *Service) *Handler {
15-
return &Handler{svc: svc}
17+
return &Handler{svc: svc, rl: NewRateLimiter()}
1618
}
1719

1820
func (h *Handler) Login(w http.ResponseWriter, r *http.Request) {
@@ -27,17 +29,30 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) {
2729
return
2830
}
2931

32+
ip := r.RemoteAddr
33+
34+
if !h.rl.Allow(ip) {
35+
slog.Warn("login rate limit exceeded", "ip", ip, "username", req.Username)
36+
httputil.Error(w, http.StatusTooManyRequests, "too many login attempts, try again later")
37+
return
38+
}
39+
3040
session, user, err := h.svc.Login(req.Username, req.Password)
3141
if err != nil {
42+
slog.Warn("login failed", "ip", ip, "username", req.Username)
3243
httputil.Error(w, http.StatusUnauthorized, "invalid credentials")
3344
return
3445
}
3546

47+
h.rl.Reset(ip)
48+
slog.Info("login successful", "ip", ip, "username", req.Username)
49+
3650
http.SetCookie(w, &http.Cookie{
3751
Name: "session",
3852
Value: session.ID,
3953
Path: "/",
4054
HttpOnly: true,
55+
Secure: true,
4156
SameSite: http.SameSiteStrictMode,
4257
MaxAge: int(time.Until(session.ExpiresAt).Seconds()),
4358
})
@@ -56,6 +71,7 @@ func (h *Handler) Logout(w http.ResponseWriter, r *http.Request) {
5671
Value: "",
5772
Path: "/",
5873
HttpOnly: true,
74+
Secure: true,
5975
SameSite: http.SameSiteStrictMode,
6076
MaxAge: -1,
6177
})

internal/auth/ratelimit.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package auth
2+
3+
import (
4+
"sync"
5+
"time"
6+
)
7+
8+
const (
9+
maxLoginAttempts = 5
10+
loginWindow = 15 * time.Minute
11+
)
12+
13+
type loginAttempt struct {
14+
count int
15+
windowAt time.Time
16+
}
17+
18+
type RateLimiter struct {
19+
mu sync.Mutex
20+
attempts map[string]*loginAttempt
21+
}
22+
23+
func NewRateLimiter() *RateLimiter {
24+
rl := &RateLimiter{attempts: make(map[string]*loginAttempt)}
25+
go rl.cleanup()
26+
return rl
27+
}
28+
29+
// Allow checks if the IP is allowed to attempt login.
30+
func (rl *RateLimiter) Allow(ip string) bool {
31+
rl.mu.Lock()
32+
defer rl.mu.Unlock()
33+
34+
a, ok := rl.attempts[ip]
35+
if !ok {
36+
rl.attempts[ip] = &loginAttempt{count: 1, windowAt: time.Now().Add(loginWindow)}
37+
return true
38+
}
39+
40+
if time.Now().After(a.windowAt) {
41+
a.count = 1
42+
a.windowAt = time.Now().Add(loginWindow)
43+
return true
44+
}
45+
46+
a.count++
47+
return a.count <= maxLoginAttempts
48+
}
49+
50+
// Reset clears the counter for an IP after successful login.
51+
func (rl *RateLimiter) Reset(ip string) {
52+
rl.mu.Lock()
53+
defer rl.mu.Unlock()
54+
delete(rl.attempts, ip)
55+
}
56+
57+
func (rl *RateLimiter) cleanup() {
58+
ticker := time.NewTicker(5 * time.Minute)
59+
defer ticker.Stop()
60+
for range ticker.C {
61+
rl.mu.Lock()
62+
now := time.Now()
63+
for ip, a := range rl.attempts {
64+
if now.After(a.windowAt) {
65+
delete(rl.attempts, ip)
66+
}
67+
}
68+
rl.mu.Unlock()
69+
}
70+
}

internal/auth/service.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func (s *Service) EnsureAdminUser(username, password string) error {
3232
return nil
3333
}
3434

35-
hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
35+
hash, err := bcrypt.GenerateFromPassword([]byte(password), 12)
3636
if err != nil {
3737
return fmt.Errorf("hash password: %w", err)
3838
}
@@ -46,10 +46,20 @@ func (s *Service) EnsureAdminUser(username, password string) error {
4646
return nil
4747
}
4848

49+
// dummyHash is a pre-computed bcrypt hash used to prevent timing attacks.
50+
// When a user is not found, we still run bcrypt comparison against this hash
51+
// so the response time is consistent regardless of whether the user exists.
52+
var dummyHash = func() []byte {
53+
h, _ := bcrypt.GenerateFromPassword([]byte("dummy-timing-attack-prevention"), 12)
54+
return h
55+
}()
56+
4957
func (s *Service) Login(username, password string) (*Session, *User, error) {
5058
user, err := s.repo.GetUserByUsername(username)
5159
if err != nil {
5260
if errors.Is(err, sql.ErrNoRows) {
61+
// Constant-time: always run bcrypt even if user doesn't exist
62+
bcrypt.CompareHashAndPassword(dummyHash, []byte(password))
5363
return nil, nil, fmt.Errorf("invalid credentials")
5464
}
5565
return nil, nil, fmt.Errorf("get user: %w", err)

internal/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func Load() (*Config, error) {
2424
AdminPassword: getEnv("ADMIN_PASSWORD", ""),
2525
DBPath: getEnv("DB_PATH", "system-control.db"),
2626
NginxSitesDir: getEnv("NGINX_SITES_DIR", "/etc/nginx/sites-enabled"),
27-
SessionMaxAge: getEnvInt("SESSION_MAX_AGE", 604800), // 7 days
27+
SessionMaxAge: getEnvInt("SESSION_MAX_AGE", 7200), // 2 hours
2828
TrafficInterfaces: getEnv("TRAFFIC_INTERFACES", ""),
2929
TrafficInterval: getEnvInt("TRAFFIC_INTERVAL", 10),
3030
}

internal/nginx/service.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,10 @@ func (s *Service) SetEnabled(id int64, enabled bool) (*Domain, error) {
214214
}
215215

216216
func (s *Service) RequestSSL(id int64, email string) error {
217+
if err := validate.Email(email); err != nil {
218+
return fmt.Errorf("email: %w", err)
219+
}
220+
217221
domain, err := s.repo.GetByID(id)
218222
if err != nil {
219223
return fmt.Errorf("domain not found: %w", err)
@@ -388,9 +392,8 @@ func (s *Service) ImportExternal(filename string) (*Domain, error) {
388392
return nil, fmt.Errorf("import is only supported on Linux")
389393
}
390394

391-
// Validate filename to prevent path traversal
392-
if strings.Contains(filename, "/") || strings.Contains(filename, "\\") || strings.Contains(filename, "..") {
393-
return nil, fmt.Errorf("invalid filename")
395+
if err := validate.Filename(filename); err != nil {
396+
return nil, err
394397
}
395398

396399
srcPath := filepath.Join(s.sitesDir, filename)

internal/pkg/validate/validate.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package validate
33
import (
44
"fmt"
55
"net"
6+
"net/mail"
7+
"path/filepath"
68
"regexp"
79
)
810

@@ -35,3 +37,25 @@ func Required(field, value string) error {
3537
}
3638
return nil
3739
}
40+
41+
func Email(email string) error {
42+
if email == "" {
43+
return fmt.Errorf("email is required")
44+
}
45+
_, err := mail.ParseAddress(email)
46+
if err != nil {
47+
return fmt.Errorf("invalid email address: %s", email)
48+
}
49+
return nil
50+
}
51+
52+
func Filename(name string) error {
53+
if name == "" {
54+
return fmt.Errorf("filename is required")
55+
}
56+
cleaned := filepath.Clean(name)
57+
if cleaned != name || filepath.IsAbs(name) || cleaned == ".." || cleaned == "." {
58+
return fmt.Errorf("invalid filename")
59+
}
60+
return nil
61+
}

0 commit comments

Comments
 (0)