Merge pull request #414 from Wikid82/main
Propagate changes from main into development
This commit is contained in:
@@ -13,14 +13,17 @@ func AuthMiddleware(authService *services.AuthService) gin.HandlerFunc {
|
||||
authHeader := c.GetHeader("Authorization")
|
||||
|
||||
if authHeader == "" {
|
||||
// Try cookie first for browser flows
|
||||
// Try cookie first for browser flows (including WebSocket upgrades)
|
||||
if cookie, err := c.Cookie("auth_token"); err == nil && cookie != "" {
|
||||
authHeader = "Bearer " + cookie
|
||||
}
|
||||
}
|
||||
|
||||
// DEPRECATED: Query parameter authentication for WebSocket connections
|
||||
// This fallback exists only for backward compatibility and will be removed in a future version.
|
||||
// Query parameters are logged in access logs and should not be used for sensitive data.
|
||||
// Use HttpOnly cookies instead, which are automatically sent by browsers and not logged.
|
||||
if authHeader == "" {
|
||||
// Try query param (token passthrough)
|
||||
if token := c.Query("token"); token != "" {
|
||||
authHeader = "Bearer " + token
|
||||
}
|
||||
|
||||
@@ -184,3 +184,58 @@ func TestRequireRole_MissingRoleInContext(t *testing.T) {
|
||||
|
||||
assert.Equal(t, http.StatusUnauthorized, w.Code)
|
||||
}
|
||||
|
||||
func TestAuthMiddleware_QueryParamFallback(t *testing.T) {
|
||||
authService := setupAuthService(t)
|
||||
user, err := authService.Register("test@example.com", "password", "Test User")
|
||||
require.NoError(t, err)
|
||||
token, err := authService.GenerateToken(user)
|
||||
require.NoError(t, err)
|
||||
|
||||
gin.SetMode(gin.TestMode)
|
||||
r := gin.New()
|
||||
r.Use(AuthMiddleware(authService))
|
||||
r.GET("/test", func(c *gin.Context) {
|
||||
userID, _ := c.Get("userID")
|
||||
assert.Equal(t, user.ID, userID)
|
||||
c.Status(http.StatusOK)
|
||||
})
|
||||
|
||||
// Test that query param auth still works (deprecated fallback)
|
||||
req, err := http.NewRequest("GET", "/test?token="+token, http.NoBody)
|
||||
require.NoError(t, err)
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
}
|
||||
|
||||
func TestAuthMiddleware_PrefersCookieOverQueryParam(t *testing.T) {
|
||||
authService := setupAuthService(t)
|
||||
|
||||
// Create two different users
|
||||
cookieUser, _ := authService.Register("cookie@example.com", "password", "Cookie User")
|
||||
cookieToken, _ := authService.GenerateToken(cookieUser)
|
||||
|
||||
queryUser, _ := authService.Register("query@example.com", "password", "Query User")
|
||||
queryToken, _ := authService.GenerateToken(queryUser)
|
||||
|
||||
gin.SetMode(gin.TestMode)
|
||||
r := gin.New()
|
||||
r.Use(AuthMiddleware(authService))
|
||||
r.GET("/test", func(c *gin.Context) {
|
||||
userID, _ := c.Get("userID")
|
||||
// Should use the cookie user, not the query param user
|
||||
assert.Equal(t, cookieUser.ID, userID)
|
||||
c.Status(http.StatusOK)
|
||||
})
|
||||
|
||||
// Both cookie and query param provided - cookie should win
|
||||
req, err := http.NewRequest("GET", "/test?token="+queryToken, http.NoBody)
|
||||
require.NoError(t, err)
|
||||
req.AddCookie(&http.Cookie{Name: "auth_token", Value: cookieToken})
|
||||
w := httptest.NewRecorder()
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
assert.Equal(t, http.StatusOK, w.Code)
|
||||
}
|
||||
|
||||
@@ -25,42 +25,50 @@
|
||||
```text
|
||||
Frontend Backend
|
||||
──────── ───────
|
||||
localStorage.getItem('charon_auth_token')
|
||||
│
|
||||
▼
|
||||
Query param: ?token=<jwt> ────────► AuthMiddleware:
|
||||
1. Check Authorization header
|
||||
2. Check auth_token cookie
|
||||
3. Check token query param ◄── MATCHES
|
||||
│
|
||||
▼
|
||||
ValidateToken(jwt) → OK
|
||||
│
|
||||
▼
|
||||
Upgrade to WebSocket
|
||||
User logs in
|
||||
│
|
||||
▼
|
||||
Backend sets HttpOnly auth_token cookie ──► AuthMiddleware:
|
||||
│ 1. Check Authorization header
|
||||
│ 2. Check auth_token cookie ◄── SECURE METHOD
|
||||
│ 3. (Deprecated) Check token query param
|
||||
▼ │
|
||||
WebSocket connection initiated ▼
|
||||
(Cookie sent automatically by browser) ValidateToken(jwt) → OK
|
||||
│ │
|
||||
│ ▼
|
||||
└──────────────────────────────────► Upgrade to WebSocket
|
||||
```
|
||||
|
||||
**Security Note:** Authentication now uses HttpOnly cookies instead of query parameters.
|
||||
This prevents JWT tokens from being logged in access logs, proxies, and other telemetry.
|
||||
The browser automatically sends the cookie with WebSocket upgrade requests.
|
||||
|
||||
### Logic Gap Analysis
|
||||
|
||||
**ANSWER: NO - There is NO logic gap between Frontend and Backend.**
|
||||
|
||||
| Question | Answer |
|
||||
|----------|--------|
|
||||
| Frontend auth method | Query param `?token=<jwt>` from `localStorage.getItem('charon_auth_token')` |
|
||||
| Backend auth method | Accepts: Header → Cookie → Query param `token` ✅ |
|
||||
| Frontend auth method | HttpOnly cookie (`auth_token`) sent automatically by browser ✅ SECURE |
|
||||
| Backend auth method | Accepts: Header → Cookie (preferred) → Query param (deprecated) ✅ |
|
||||
| Filter params | Both use `source`, `level`, `ip`, `host`, `blocked_only` ✅ |
|
||||
| Data format | `SecurityLogEntry` struct matches frontend TypeScript type ✅ |
|
||||
| Security | Tokens no longer logged in access logs or exposed to XSS ✅ |
|
||||
|
||||
---
|
||||
|
||||
## 1. VERIFICATION STATUS
|
||||
|
||||
### ✅ localStorage Key IS Correct
|
||||
### ✅ Authentication Method Updated for Security
|
||||
|
||||
Both WebSocket functions in `frontend/src/api/logs.ts` correctly use `charon_auth_token`:
|
||||
WebSocket authentication now uses HttpOnly cookies instead of query parameters:
|
||||
|
||||
- **Line 119-122** (`connectLiveLogs`): `localStorage.getItem('charon_auth_token')`
|
||||
- **Line 178-181** (`connectSecurityLogs`): `localStorage.getItem('charon_auth_token')`
|
||||
- **`connectLiveLogs`** (frontend/src/api/logs.ts): Uses browser's automatic cookie transmission
|
||||
- **`connectSecurityLogs`** (frontend/src/api/logs.ts): Uses browser's automatic cookie transmission
|
||||
- **Backend middleware**: Prioritizes cookie-based auth, query param is deprecated
|
||||
|
||||
This change prevents JWT tokens from appearing in access logs, proxy logs, and other telemetry.
|
||||
|
||||
---
|
||||
|
||||
@@ -186,12 +194,13 @@ The `showBlockedOnly` state in useEffect dependencies causes reconnection when t
|
||||
|
||||
| Component | Status | Notes |
|
||||
|-----------|--------|-------|
|
||||
| localStorage key | ✅ Fixed | Now uses `charon_auth_token` |
|
||||
| Auth middleware | ✅ Working | Accepts query param `token` |
|
||||
| WebSocket authentication | ✅ Secured | Now uses HttpOnly cookies instead of query parameters |
|
||||
| Auth middleware | ✅ Updated | Cookie-based auth prioritized, query param deprecated |
|
||||
| WebSocket endpoint | ✅ Working | Protected route, upgrades correctly |
|
||||
| LogWatcher service | ✅ Working | Tails access.log successfully |
|
||||
| **Frontend memoization** | ✅ Fixed | `useMemo` in Security.tsx |
|
||||
| **Stable default props** | ✅ Fixed | Constants in LiveLogViewer.tsx |
|
||||
| **Security improvement** | ✅ Complete | Tokens no longer exposed in logs |
|
||||
|
||||
---
|
||||
|
||||
@@ -221,7 +230,9 @@ docker logs charon 2>&1 | grep -i "cerberus.*websocket" | tail -10
|
||||
|
||||
**Logic Gap Between Frontend/Backend:** **NO** - Both are correctly aligned
|
||||
|
||||
**Current Status:** ✅ All fixes applied and working
|
||||
**Security Enhancement:** WebSocket authentication now uses HttpOnly cookies instead of query parameters, preventing token leakage in logs
|
||||
|
||||
**Current Status:** ✅ All fixes applied and working securely
|
||||
|
||||
---
|
||||
|
||||
|
||||
131
docs/security/websocket-auth-security.md
Normal file
131
docs/security/websocket-auth-security.md
Normal file
@@ -0,0 +1,131 @@
|
||||
# WebSocket Authentication Security
|
||||
|
||||
## Overview
|
||||
|
||||
This document explains the security improvements made to WebSocket authentication in Charon to prevent JWT tokens from being exposed in access logs.
|
||||
|
||||
## Security Issue
|
||||
|
||||
### Before (Insecure)
|
||||
|
||||
Previously, WebSocket connections authenticated by passing the JWT token as a query parameter:
|
||||
|
||||
```
|
||||
wss://example.com/api/v1/logs/live?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...
|
||||
```
|
||||
|
||||
**Security Risk:**
|
||||
- Query parameters are logged in web server access logs (Caddy, nginx, Apache, etc.)
|
||||
- Tokens appear in proxy logs
|
||||
- Tokens may be stored in browser history
|
||||
- Tokens can be captured in monitoring and telemetry systems
|
||||
- An attacker with access to these logs can replay the token to impersonate a user
|
||||
|
||||
### After (Secure)
|
||||
|
||||
WebSocket connections now authenticate using HttpOnly cookies:
|
||||
|
||||
```
|
||||
wss://example.com/api/v1/logs/live?source=waf&level=error
|
||||
```
|
||||
|
||||
The browser automatically sends the `auth_token` cookie with the WebSocket upgrade request.
|
||||
|
||||
**Security Benefits:**
|
||||
- ✅ HttpOnly cookies are **not logged** by web servers
|
||||
- ✅ HttpOnly cookies **cannot be accessed** by JavaScript (XSS protection)
|
||||
- ✅ Cookies are **not visible** in browser history
|
||||
- ✅ Cookies are **not captured** in URL-based monitoring
|
||||
- ✅ Token replay attacks are mitigated (tokens still have expiration)
|
||||
|
||||
## Implementation Details
|
||||
|
||||
### Frontend Changes
|
||||
|
||||
**Location:** `frontend/src/api/logs.ts`
|
||||
|
||||
Removed:
|
||||
```typescript
|
||||
const token = localStorage.getItem('charon_auth_token');
|
||||
if (token) {
|
||||
params.append('token', token);
|
||||
}
|
||||
```
|
||||
|
||||
The browser automatically sends the `auth_token` cookie when establishing WebSocket connections due to:
|
||||
1. The cookie is set by the backend during login with `HttpOnly`, `Secure`, and `SameSite` flags
|
||||
2. The axios client has `withCredentials: true`, enabling cookie transmission
|
||||
|
||||
### Backend Changes
|
||||
|
||||
**Location:** `backend/internal/api/middleware/auth.go`
|
||||
|
||||
Authentication priority order:
|
||||
1. **Authorization header** (Bearer token) - for API clients
|
||||
2. **auth_token cookie** (HttpOnly) - **preferred for browsers and WebSockets**
|
||||
3. **token query parameter** - **deprecated**, kept for backward compatibility only
|
||||
|
||||
The query parameter fallback is marked as deprecated and will be removed in a future version.
|
||||
|
||||
### Cookie Configuration
|
||||
|
||||
**Location:** `backend/internal/api/handlers/auth_handler.go`
|
||||
|
||||
The `auth_token` cookie is set with security best practices:
|
||||
- **HttpOnly**: `true` - prevents JavaScript access (XSS protection)
|
||||
- **Secure**: `true` (in production with HTTPS) - prevents transmission over HTTP
|
||||
- **SameSite**: `Strict` (HTTPS) or `Lax` (HTTP/IP) - CSRF protection
|
||||
- **Path**: `/` - available for all routes
|
||||
- **MaxAge**: 24 hours - automatic expiration
|
||||
|
||||
## Verification
|
||||
|
||||
### Test Coverage
|
||||
|
||||
**Location:** `backend/internal/api/middleware/auth_test.go`
|
||||
|
||||
- `TestAuthMiddleware_Cookie` - verifies cookie authentication works
|
||||
- `TestAuthMiddleware_QueryParamFallback` - verifies deprecated query param still works
|
||||
- `TestAuthMiddleware_PrefersCookieOverQueryParam` - verifies cookie is prioritized over query param
|
||||
- `TestAuthMiddleware_PrefersAuthorizationHeader` - verifies header takes highest priority
|
||||
|
||||
### Log Verification
|
||||
|
||||
To verify tokens are not logged:
|
||||
|
||||
1. **Before the fix:** Check Caddy access logs for token exposure:
|
||||
```bash
|
||||
docker logs charon 2>&1 | grep "token=" | grep -o "token=[^&]*"
|
||||
```
|
||||
Would show: `token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...`
|
||||
|
||||
2. **After the fix:** Check that WebSocket URLs are clean:
|
||||
```bash
|
||||
docker logs charon 2>&1 | grep "/logs/live\|/cerberus/logs/ws"
|
||||
```
|
||||
Shows: `/api/v1/logs/live?source=waf&level=error` (no token)
|
||||
|
||||
## Migration Path
|
||||
|
||||
### For Users
|
||||
|
||||
No action required. The change is transparent:
|
||||
- Login sets the HttpOnly cookie
|
||||
- WebSocket connections automatically use the cookie
|
||||
- Existing sessions continue to work
|
||||
|
||||
### For API Clients
|
||||
|
||||
API clients using Authorization headers are unaffected.
|
||||
|
||||
### Deprecation Timeline
|
||||
|
||||
1. **Current:** Query parameter authentication is deprecated but still functional
|
||||
2. **Future (v2.0):** Query parameter authentication will be removed entirely
|
||||
3. **Recommendation:** Any custom scripts or tools should migrate to using Authorization headers or cookie-based authentication
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- [Authentication Flow](../plans/prev_spec_websocket_fix_dec16.md#authentication-flow)
|
||||
- [Security Best Practices](https://owasp.org/www-community/HttpOnly)
|
||||
- [WebSocket Security](https://datatracker.ietf.org/doc/html/rfc6455#section-10)
|
||||
@@ -128,11 +128,8 @@ export const connectLiveLogs = (
|
||||
if (filters.level) params.append('level', filters.level);
|
||||
if (filters.source) params.append('source', filters.source);
|
||||
|
||||
// Get auth token from localStorage (key: charon_auth_token)
|
||||
const token = localStorage.getItem('charon_auth_token');
|
||||
if (token) {
|
||||
params.append('token', token);
|
||||
}
|
||||
// Authentication is handled via HttpOnly cookies sent automatically by the browser
|
||||
// This prevents tokens from being logged in access logs or exposed to XSS attacks
|
||||
|
||||
const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:';
|
||||
const wsUrl = `${protocol}//${window.location.host}/api/v1/logs/live?${params.toString()}`;
|
||||
@@ -196,11 +193,8 @@ export const connectSecurityLogs = (
|
||||
if (filters.host) params.append('host', filters.host);
|
||||
if (filters.blocked_only) params.append('blocked_only', 'true');
|
||||
|
||||
// Get auth token from localStorage (key: charon_auth_token)
|
||||
const token = localStorage.getItem('charon_auth_token');
|
||||
if (token) {
|
||||
params.append('token', token);
|
||||
}
|
||||
// Authentication is handled via HttpOnly cookies sent automatically by the browser
|
||||
// This prevents tokens from being logged in access logs or exposed to XSS attacks
|
||||
|
||||
const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:';
|
||||
const wsUrl = `${protocol}//${window.location.host}/api/v1/cerberus/logs/ws?${params.toString()}`;
|
||||
|
||||
Reference in New Issue
Block a user