fix: Harden security middleware by logging detailed violations internally and returning generic error messages, and streamline user logging to use user_id.

main
Cizz22 5 days ago
parent 22ff523ccd
commit f5c3cf774d

@ -60,8 +60,6 @@ class JSONFormatter(logging.Formatter):
# Add Context information if available # Add Context information if available
if request_id: if request_id:
log_record["request_id"] = request_id log_record["request_id"] = request_id
if role:
log_record["role"] = role
# Add Error context if available # Add Error context if available
if hasattr(record, "error_id"): if hasattr(record, "error_id"):

@ -181,10 +181,8 @@ async def db_session_middleware(request: Request, call_next):
set_role(role) set_role(role)
user_info_str = "" user_info_str = ""
if username: if user_id:
user_info_str = f" | User: {username}" user_info_str = f" | User ID: {user_id}"
if role:
user_info_str += f" ({role})"
error_id = getattr(request.state, "error_id", None) error_id = getattr(request.state, "error_id", None)
log_msg = f"HTTP {request.method} {request.url.path} completed in {round(process_time, 2)}ms{user_info_str}" log_msg = f"HTTP {request.method} {request.url.path} completed in {round(process_time, 2)}ms{user_info_str}"
@ -199,7 +197,6 @@ async def db_session_middleware(request: Request, call_next):
"status_code": response.status_code, "status_code": response.status_code,
"duration_ms": round(process_time, 2), "duration_ms": round(process_time, 2),
"user_id": user_id, "user_id": user_id,
"role": role,
"error_id": error_id, "error_id": error_id,
}, },
) )

@ -103,6 +103,7 @@ ALLOWED_HEADERS = {
"x-forwarded-path", "x-forwarded-path",
"x-forwarded-prefix", "x-forwarded-prefix",
"cookie", "cookie",
"x-kong-request-id"
} }
MAX_QUERY_PARAMS = 50 MAX_QUERY_PARAMS = 50
@ -193,33 +194,38 @@ def inspect_value(value: str, source: str):
if XSS_PATTERN.search(value): if XSS_PATTERN.search(value):
log.warning(f"Security violation: Potential XSS payload detected in {source}, value: {value}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Potential XSS payload detected in {source}", detail="Invalid request parameters",
) )
if SQLI_PATTERN.search(value): if SQLI_PATTERN.search(value):
log.warning(f"Security violation: Potential SQL injection payload detected in {source}, value: {value}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Potential SQL injection payload detected in {source}", detail="Invalid request parameters",
) )
if RCE_PATTERN.search(value): if RCE_PATTERN.search(value):
log.warning(f"Security violation: Potential RCE payload detected in {source}, value: {value}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Potential RCE payload detected in {source}", detail="Invalid request parameters",
) )
if TRAVERSAL_PATTERN.search(value): if TRAVERSAL_PATTERN.search(value):
log.warning(f"Security violation: Potential Path Traversal payload detected in {source}, value: {value}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Potential Path Traversal payload detected in {source}", detail="Invalid request parameters",
) )
if has_control_chars(value): if has_control_chars(value):
log.warning(f"Security violation: Invalid control characters detected in {source}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Invalid control characters detected in {source}", detail="Invalid request parameters",
) )
@ -227,15 +233,17 @@ def inspect_json(obj, path="body", check_whitelist=True):
if isinstance(obj, dict): if isinstance(obj, dict):
for key, value in obj.items(): for key, value in obj.items():
if key in FORBIDDEN_JSON_KEYS: if key in FORBIDDEN_JSON_KEYS:
log.warning(f"Security violation: Forbidden JSON key detected: {path}.{key}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Forbidden JSON key detected: {path}.{key}", detail="Invalid request parameters",
) )
if check_whitelist and key not in ALLOWED_DATA_PARAMS: if check_whitelist and key not in ALLOWED_DATA_PARAMS:
log.warning(f"Security violation: Unknown JSON key detected: {path}.{key}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Unknown JSON key detected: {path}.{key}", detail="Invalid request parameters",
) )
# Recurse. If the key is a dynamic container, we stop whitelist checking for children. # Recurse. If the key is a dynamic container, we stop whitelist checking for children.
@ -266,9 +274,10 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
ALLOW_DUPLICATE_HEADERS = {'accept', 'accept-encoding', 'accept-language', 'accept-charset', 'cookie'} ALLOW_DUPLICATE_HEADERS = {'accept', 'accept-encoding', 'accept-language', 'accept-charset', 'cookie'}
real_duplicates = [h for h in duplicate_headers if h not in ALLOW_DUPLICATE_HEADERS] real_duplicates = [h for h in duplicate_headers if h not in ALLOW_DUPLICATE_HEADERS]
if real_duplicates: if real_duplicates:
log.warning(f"Security violation: Duplicate headers detected: {real_duplicates}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Duplicate headers are not allowed: {real_duplicates}", detail="Invalid request parameters",
) )
# Whitelist headers # Whitelist headers
@ -276,9 +285,10 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
if unknown_headers: if unknown_headers:
filtered_unknown = [h for h in unknown_headers if not h.startswith('sec-')] filtered_unknown = [h for h in unknown_headers if not h.startswith('sec-')]
if filtered_unknown: if filtered_unknown:
log.warning(f"Security violation: Unknown headers detected: {filtered_unknown}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Unknown headers detected: {filtered_unknown}", detail="Invalid request parameters",
) )
# Inspect header values # Inspect header values
@ -290,25 +300,28 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
# 1. Query string limits # 1. Query string limits
# ------------------------- # -------------------------
if len(request.url.query) > MAX_QUERY_LENGTH: if len(request.url.query) > MAX_QUERY_LENGTH:
log.warning(f"Security violation: Query string too long")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail="Query string too long", detail="Invalid request parameters",
) )
params = request.query_params.multi_items() params = request.query_params.multi_items()
if len(params) > MAX_QUERY_PARAMS: if len(params) > MAX_QUERY_PARAMS:
log.warning(f"Security violation: Too many query parameters")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail="Too many query parameters", detail="Invalid request parameters",
) )
# Check for unknown query parameters # Check for unknown query parameters
unknown_params = [key for key, _ in params if key not in ALLOWED_DATA_PARAMS] unknown_params = [key for key, _ in params if key not in ALLOWED_DATA_PARAMS]
if unknown_params: if unknown_params:
log.warning(f"Security violation: Unknown query parameters detected: {unknown_params}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Unknown query parameters detected: {unknown_params}", detail="Invalid request parameters",
) )
# ------------------------- # -------------------------
@ -321,9 +334,10 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
] ]
if duplicates: if duplicates:
log.warning(f"Security violation: Duplicate query parameters detected: {duplicates}")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Duplicate query parameters are not allowed: {duplicates}", detail="Invalid request parameters",
) )
# ------------------------- # -------------------------
@ -338,19 +352,22 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
try: try:
size_val = int(value) size_val = int(value)
if size_val > 50: if size_val > 50:
log.warning(f"Security violation: Pagination size too large ({size_val})")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Pagination size '{key}' cannot exceed 50", detail="Invalid request parameters",
) )
if size_val % 5 != 0: if size_val % 5 != 0:
log.warning(f"Security violation: Pagination size not multiple of 5 ({size_val})")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Pagination size '{key}' must be a multiple of 5", detail="Invalid request parameters",
) )
except ValueError: except ValueError:
log.warning(f"Security violation: Pagination size invalid value")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail=f"Pagination size '{key}' must be an integer", detail="Invalid request parameters",
) )
# ------------------------- # -------------------------
@ -361,7 +378,8 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
content_type.startswith(t) content_type.startswith(t)
for t in ("application/json", "multipart/form-data", "application/x-www-form-urlencoded") for t in ("application/json", "multipart/form-data", "application/x-www-form-urlencoded")
): ):
raise HTTPException(status_code=422, detail="Unsupported Content-Type") log.warning(f"Security violation: Unsupported Content-Type: {content_type}")
raise HTTPException(status_code=422, detail="Invalid request parameters")
# ------------------------- # -------------------------
# 5. Single source check (Query vs JSON Body) # 5. Single source check (Query vs JSON Body)
@ -377,9 +395,10 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
has_body = True has_body = True
if has_query and has_body: if has_query and has_body:
log.warning(f"Security violation: Mixed parameters (query + JSON body)")
raise HTTPException( raise HTTPException(
status_code=422, status_code=422,
detail="Parameters must be from a single source (query string or JSON body), mixed sources are not allowed", detail="Invalid request parameters",
) )
# ------------------------- # -------------------------
@ -394,7 +413,8 @@ class RequestValidationMiddleware(BaseHTTPMiddleware):
try: try:
payload = json.loads(body) payload = json.loads(body)
except json.JSONDecodeError: except json.JSONDecodeError:
raise HTTPException(status_code=422, detail="Invalid JSON body") log.warning(f"Security violation: Invalid JSON body")
raise HTTPException(status_code=422, detail="Invalid request parameters")
inspect_json(payload) inspect_json(payload)

Loading…
Cancel
Save