Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
cc101f8
initial commit with CSRF mitigations
Mar 2, 2026
2851b30
added some tests for CSRF middleware
Mar 9, 2026
f1d4330
added additional CSRF tests
Mar 16, 2026
74b2efe
separated regular functioning security tests from CSRF tests
Mar 16, 2026
e2066f6
fix: bump cryptography to 46.0.5 and expand CI security coverage
deacon-mp Mar 15, 2026
f1dbc0a
fix: reuse existing fact source on operation close (#3261)
ChenFryd Mar 15, 2026
0fe954e
Bump minimatch from 3.1.2 to 3.1.5 (#3258)
dependabot[bot] Mar 15, 2026
19bddee
Add architecture field to agent deployment commands (#3260)
clenk Mar 16, 2026
6c10c27
hash passwords and API keys in main config (#3257)
uruwhy Mar 16, 2026
d8bf701
fix: replace create_subprocess_shell (#3265)
deacon-mp Mar 16, 2026
708208f
fix: replace deprecated asyncio.get_event_loop() with new_event_loop(…
deacon-mp Mar 16, 2026
99eeaa5
fix: reduce global client_max_size and add configurable setting (#3268)
deacon-mp Mar 16, 2026
81b6eb0
fix: degrade gracefully when magma plugin dist is absent (#3275)
deacon-mp Mar 16, 2026
b923bfb
fix: guard against None agent in operations summary endpoint (#3181) …
deacon-mp Mar 16, 2026
ad5eefa
fix: resolve trait-only relationship facts from source fact list on o…
deacon-mp Mar 16, 2026
0e211cf
fix: prevent operation report from returning null when link paw absen…
deacon-mp Mar 16, 2026
78d3c2a
fix: correct exfil operation filter and patch path traversal bypass (…
deacon-mp Mar 16, 2026
8513e11
fix: validate upload filename character set in file_svc (#3267)
deacon-mp Mar 16, 2026
049784b
fix: register SIGTERM handler in run_tasks() to save state on service…
deacon-mp Mar 16, 2026
1c5d5ba
added modification to test_authentication_required_middleware_authent…
Mar 16, 2026
83ffce0
added test to show that without cookie, authenticated endpoint sessio…
Mar 16, 2026
21d065c
CSRF operation tests are passing, including tests to verify resilienc…
Mar 16, 2026
cf6c1a2
security.py change allowing CSRF protections to be bypassed if the se…
Mar 16, 2026
5951697
CSRF operation creation test now passes - small change reflecting upd…
Mar 16, 2026
86528b5
linting reformatting
Mar 16, 2026
73e7850
Merge branch 'master' into VIRTS-1953
Mar 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion app/api/v2/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@

def make_app(services, upload_max_size_mb=100):
from .responses import json_request_validation_middleware
from .security import authentication_required_middleware_factory, pass_option_middleware
from .security import authentication_required_middleware_factory, pass_option_middleware, csrf_protect_middleware_factory

try:
max_size = int(upload_max_size_mb)
max_size = max_size if max_size > 0 else 100
except (TypeError, ValueError):
max_size = 100

try:
max_size = int(upload_max_size_mb)
Expand All @@ -16,6 +22,7 @@ def make_app(services, upload_max_size_mb=100):
middlewares=[
pass_option_middleware,
authentication_required_middleware_factory(services['auth_svc']),
csrf_protect_middleware_factory(services['auth_svc']),
json_request_validation_middleware
]
)
Expand Down
49 changes: 49 additions & 0 deletions app/api/v2/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import types

from aiohttp import web
from aiohttp_session import get_session
from hmac import compare_digest
from aiohttp.web_exceptions import HTTPForbidden


def is_handler_authentication_exempt(handler):
Expand Down Expand Up @@ -69,6 +72,52 @@ async def authentication_required_middleware(request, handler):
return authentication_required_middleware


def csrf_protect_middleware_factory(auth_svc):
"""Protect unsafe (state-modifying) requests against CSRF for session-authenticated users.

Behavior:
- Allow safe methods (GET, HEAD, OPTIONS) without checks.
- If request provides an API key (header KEY), skip CSRF checks.
- For session-authenticated requests using unsafe methods, compare the X-CSRF-Token
header to the token stored in the server-side session (recommended) and reject
requests with missing/invalid tokens with HTTP 403.
"""
@web.middleware
async def csrf_protect_middleware(request, handler):
# Skip safe methods
if request.method in ('GET', 'HEAD', 'OPTIONS'):
return await handler(request)

# If API key auth is present, skip CSRF checks
if request.headers.get('KEY'):
return await handler(request)
Comment on lines +91 to +93
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CSRF checks are skipped whenever a KEY header is present, even if the API key is invalid. For a session-authenticated request, an attacker could add any KEY header value and bypass CSRF validation. Use auth_svc.request_has_valid_api_key(request) (and ideally only skip when API-key auth is the actual auth mechanism) rather than checking header presence.

Copilot uses AI. Check for mistakes.

# If the endpoint handler is explicitly decorated as authentication-exempt,
# allow it to proceed without CSRF validation. This covers endpoints like
# login which must be callable before a session and CSRF token exist.
if is_handler_authentication_exempt(handler):
return await handler(request)

# For session-authenticated requests, validate token
try:
session = await get_session(request)
token = session.get('csrf_token') if session is not None else None
header = request.headers.get('X-CSRF-Token') or request.headers.get('X-XSRF-TOKEN')
# check if there is a token, the header is missing, and whether the token and header
# hash authentication works
if not token or not header or not compare_digest(header, token):
raise HTTPForbidden(text='Missing or invalid CSRF token')
except HTTPForbidden:
raise
except Exception:
# If something goes wrong accessing the session, deny the request
raise HTTPForbidden(text='CSRF validation error')

return await handler(request)

return csrf_protect_middleware


@web.middleware
async def pass_option_middleware(request, handler):
"""Allow all 'OPTIONS' request to the server to return 200
Expand Down
24 changes: 23 additions & 1 deletion app/service/auth_svc.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
from aiohttp_security import setup as setup_security
from aiohttp_security.abc import AbstractAuthorizationPolicy
from aiohttp_session import setup as setup_session
from aiohttp_session import get_session
from aiohttp_session.cookie_storage import EncryptedCookieStorage
from cryptography import fernet
import secrets

from app.service.interfaces.i_auth_svc import AuthServiceInterface
from app.service.interfaces.i_login_handler import LoginHandlerInterface
Expand Down Expand Up @@ -75,7 +77,14 @@ async def apply(self, app, users):
app.user_map = self.user_map
fernet_key = fernet.Fernet.generate_key()
secret_key = base64.urlsafe_b64decode(fernet_key)
storage = EncryptedCookieStorage(secret_key, cookie_name=COOKIE_SESSION)
storage = EncryptedCookieStorage(
secret_key,
cookie_name=COOKIE_SESSION,
secure=True,
httponly=True,
max_age=86400,
samesite='Strict'
)
setup_session(app, storage)
policy = SessionIdentityPolicy()
setup_security(app, policy, DictionaryAuthorizationPolicy(self.user_map))
Expand Down Expand Up @@ -155,6 +164,19 @@ async def handle_successful_login(self, request, username):
self.log.debug('%s logging in', username)
response = web.HTTPFound('/')
await remember(request, response, username)

# Initialize per-session CSRF token and expose it via a readable cookie for double-submit
try:
session = await get_session(request)
if 'csrf_token' not in session:
session['csrf_token'] = secrets.token_urlsafe(32)
# Set a non-HttpOnly cookie so client-side JS can read the token for double-submit.
secure_flag = (request.scheme == 'https') if hasattr(request, 'scheme') else False
response.set_cookie('XSRF-TOKEN', session['csrf_token'], httponly=False, secure=secure_flag, samesite='Lax')
Comment on lines +174 to +175
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XSRF-TOKEN cookie is set without max_age while the session cookie now has max_age=86400. This can leave users with a still-valid session but no readable CSRF cookie after a browser restart, causing all unsafe requests to fail until re-login. Also, deriving secure from request.scheme can be wrong behind TLS-terminating proxies; consider setting secure=True consistently (or using a trusted proxy/proto config) and align max_age/path with the session cookie.

Suggested change
secure_flag = (request.scheme == 'https') if hasattr(request, 'scheme') else False
response.set_cookie('XSRF-TOKEN', session['csrf_token'], httponly=False, secure=secure_flag, samesite='Lax')
# Align cookie attributes (max_age, path, secure) with the session cookie settings.
response.set_cookie(
'XSRF-TOKEN',
session['csrf_token'],
max_age=86400,
path='/',
httponly=False,
secure=True,
samesite='Lax',
)

Copilot uses AI. Check for mistakes.
except Exception:
# If session management or cookie setting fails, continue without exposing token.
self.log.exception('Failed to set CSRF token on login')

raise response

async def check_permissions(self, group, request):
Expand Down
Loading
Loading