Add patches to fix DB connection leaks
Resolves: #RHEL-153811 Backport upstream fixes for database connection pool exhaustion that occurred during multi-host push attestation with multiple agents, causing QueuePool timeout and HTTP 500 errors. Upstream commits: - 5b622eae Close DB sessions to prevent connection exhaustion - bc28d5d2 Include thread-safe session management - 4f5f09a6 Address some improvements from code review - 309a0ef0 Fix race condition in SessionManager - e75921f0 Fix linter errors in PersistableModel.get() and .all() - 2d809d8b refactor: Remove dead code AuthSession.authenticate_agent() - e935df8f db: Clean up scoped session after each request - 08c0c67c fix: Check active flag in _extract_identity and guard receive_pop - d74e7499 fix: Add fork-safety to DBManager via dispose() Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
This commit is contained in:
parent
85f1915f1c
commit
d3a4e38571
343
0003-Close-DB-sessions-to-prevent-connection-exhaustion.patch
Normal file
343
0003-Close-DB-sessions-to-prevent-connection-exhaustion.patch
Normal file
@ -0,0 +1,343 @@
|
||||
From 5b622eae9244b5a820263609cae6bd4681d3fbb2 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
Date: Tue, 10 Mar 2026 11:26:49 +0100
|
||||
Subject: [PATCH 3/6] Close DB sessions to prevent connection exhaustion
|
||||
|
||||
Resolves: #1861
|
||||
|
||||
The get_session() function in session_controller.py and auth_session.py
|
||||
returned SQLAlchemy sessions that were never closed, leaking connections
|
||||
back to the pool. Under load (e.g., multi-host push attestation with
|
||||
multiple agents), this exhausted the QueuePool (size 5, overflow 10),
|
||||
causing a 30-second timeout and HTTP 500 errors.
|
||||
|
||||
Replace get_session() with a get_session_context() context manager that
|
||||
guarantees session.close() via try/finally. Define it once in
|
||||
auth_session.py and import it in session_controller.py.
|
||||
|
||||
Resolves: connection pool exhaustion (QueuePool limit of size 5
|
||||
overflow 10 reached) during push attestation multi-host tests.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
---
|
||||
keylime/models/verifier/auth_session.py | 25 +++++++++-----
|
||||
keylime/web/verifier/session_controller.py | 27 ++++-----------
|
||||
test/test_auth_session.py | 38 ++++++++++++++++++++--
|
||||
test/test_session_controller.py | 32 ++++++++++--------
|
||||
4 files changed, 77 insertions(+), 45 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/verifier/auth_session.py b/keylime/models/verifier/auth_session.py
|
||||
index df01668..545995f 100644
|
||||
--- a/keylime/models/verifier/auth_session.py
|
||||
+++ b/keylime/models/verifier/auth_session.py
|
||||
@@ -1,8 +1,9 @@
|
||||
import base64
|
||||
import hmac
|
||||
import uuid
|
||||
+from contextlib import contextmanager
|
||||
from datetime import timedelta
|
||||
-from typing import Any, Dict, Optional, Sequence
|
||||
+from typing import Any, Dict, Iterator, Optional, Sequence
|
||||
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
@@ -32,11 +33,17 @@ logger = keylime_logging.init_logging("verifier")
|
||||
_engine = None
|
||||
|
||||
|
||||
-def get_session() -> Session:
|
||||
+@contextmanager
|
||||
+def get_session_context() -> Iterator[Session]:
|
||||
global _engine
|
||||
if _engine is None:
|
||||
_engine = make_engine("cloud_verifier")
|
||||
- return SessionManager().make_session(_engine)
|
||||
+ session_manager = SessionManager()
|
||||
+ session = session_manager.make_session(_engine)
|
||||
+ try:
|
||||
+ yield session
|
||||
+ finally:
|
||||
+ session.close()
|
||||
|
||||
|
||||
class AuthSession(PersistableModel):
|
||||
@@ -270,12 +277,12 @@ class AuthSession(PersistableModel):
|
||||
return False
|
||||
|
||||
# Use old engine to query VerfierMain (legacy model)
|
||||
- session = get_session()
|
||||
- agent = (
|
||||
- session.query(VerfierMain)
|
||||
- .filter(VerfierMain.agent_id == auth_session.agent_id) # type: ignore[attr-defined]
|
||||
- .one_or_none()
|
||||
- )
|
||||
+ with get_session_context() as session:
|
||||
+ agent = (
|
||||
+ session.query(VerfierMain)
|
||||
+ .filter(VerfierMain.agent_id == auth_session.agent_id) # type: ignore[attr-defined]
|
||||
+ .one_or_none()
|
||||
+ )
|
||||
|
||||
return agent
|
||||
|
||||
diff --git a/keylime/web/verifier/session_controller.py b/keylime/web/verifier/session_controller.py
|
||||
index 9fc3bb5..49cd758 100644
|
||||
--- a/keylime/web/verifier/session_controller.py
|
||||
+++ b/keylime/web/verifier/session_controller.py
|
||||
@@ -1,27 +1,14 @@
|
||||
import base64
|
||||
|
||||
-from sqlalchemy.orm import Session
|
||||
-
|
||||
from keylime import config, keylime_logging
|
||||
-from keylime.db.keylime_db import SessionManager, make_engine
|
||||
from keylime.db.verifier_db import VerfierMain
|
||||
from keylime.models.base import Timestamp
|
||||
from keylime.models.verifier import AuthSession
|
||||
+from keylime.models.verifier.auth_session import get_session_context
|
||||
from keylime.web.base import Controller
|
||||
|
||||
logger = keylime_logging.init_logging("verifier")
|
||||
|
||||
-# GLOBAL_POLICY_CACHE: Dict[str, Dict[str, str]] = {}
|
||||
-
|
||||
-_engine = None
|
||||
-
|
||||
-
|
||||
-def get_session() -> Session:
|
||||
- global _engine
|
||||
- if _engine is None:
|
||||
- _engine = make_engine("cloud_verifier")
|
||||
- return SessionManager().make_session(_engine)
|
||||
-
|
||||
|
||||
class SessionController(Controller):
|
||||
# POST /v3[.:minor]/sessions
|
||||
@@ -198,8 +185,8 @@ class SessionController(Controller):
|
||||
return
|
||||
|
||||
# Check if agent exists - this is where we validate enrollment
|
||||
- session = get_session()
|
||||
- agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
+ with get_session_context() as session:
|
||||
+ agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
|
||||
if not agent:
|
||||
# Agent not enrolled - return 200 with evaluation:fail
|
||||
@@ -393,8 +380,8 @@ class SessionController(Controller):
|
||||
|
||||
# POST /v3[.:minor]/agents/:agent_id/session
|
||||
def create(self, agent_id, **params):
|
||||
- session = get_session()
|
||||
- agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
+ with get_session_context() as session:
|
||||
+ agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
|
||||
if not agent:
|
||||
self.respond(404, "here")
|
||||
@@ -416,8 +403,8 @@ class SessionController(Controller):
|
||||
self.respond(200, "Success", auth_session.render())
|
||||
|
||||
def update(self, agent_id, token, **params):
|
||||
- session = get_session()
|
||||
- agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
+ with get_session_context() as session:
|
||||
+ agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
|
||||
# Look up session by token hash (tokens are never stored in plaintext)
|
||||
auth_session = AuthSession.get_by_token(token)
|
||||
diff --git a/test/test_auth_session.py b/test/test_auth_session.py
|
||||
index 62b4244..8e9ec98 100644
|
||||
--- a/test/test_auth_session.py
|
||||
+++ b/test/test_auth_session.py
|
||||
@@ -7,10 +7,41 @@ from unittest.mock import MagicMock, PropertyMock, patch
|
||||
|
||||
from keylime.crypto import generate_session_token, generate_token_salt, hash_token_for_storage
|
||||
from keylime.models.base.types import Timestamp
|
||||
-from keylime.models.verifier.auth_session import AuthSession
|
||||
+from keylime.models.verifier.auth_session import AuthSession, get_session_context
|
||||
from keylime.shared_data import cleanup_global_shared_memory, get_shared_memory
|
||||
|
||||
|
||||
+class TestGetSessionContext(unittest.TestCase):
|
||||
+ """Test cases for get_session_context context manager."""
|
||||
+
|
||||
+ @patch("keylime.models.verifier.auth_session.make_engine")
|
||||
+ @patch("keylime.models.verifier.auth_session.SessionManager")
|
||||
+ def test_session_closed_on_normal_exit(self, mock_session_manager_cls, _mock_make_engine):
|
||||
+ """Test that session.close() is called when context manager exits normally."""
|
||||
+ mock_session = MagicMock()
|
||||
+ mock_session_manager_cls.return_value.make_session.return_value = mock_session
|
||||
+
|
||||
+ with patch("keylime.models.verifier.auth_session._engine", None):
|
||||
+ with get_session_context() as session:
|
||||
+ self.assertIs(session, mock_session)
|
||||
+
|
||||
+ mock_session.close.assert_called_once()
|
||||
+
|
||||
+ @patch("keylime.models.verifier.auth_session.make_engine")
|
||||
+ @patch("keylime.models.verifier.auth_session.SessionManager")
|
||||
+ def test_session_closed_on_exception(self, mock_session_manager_cls, _mock_make_engine):
|
||||
+ """Test that session.close() is called even when an exception occurs."""
|
||||
+ mock_session = MagicMock()
|
||||
+ mock_session_manager_cls.return_value.make_session.return_value = mock_session
|
||||
+
|
||||
+ with patch("keylime.models.verifier.auth_session._engine", None):
|
||||
+ with self.assertRaises(RuntimeError):
|
||||
+ with get_session_context():
|
||||
+ raise RuntimeError("simulated error")
|
||||
+
|
||||
+ mock_session.close.assert_called_once()
|
||||
+
|
||||
+
|
||||
class TestAuthSessionHelpers(unittest.TestCase):
|
||||
"""Test cases for AuthSession helper methods."""
|
||||
|
||||
@@ -398,7 +429,7 @@ class TestAuthSessionCore(unittest.TestCase):
|
||||
self.assertIn("errors", result)
|
||||
self.assertIn("authentication_supported", result["errors"])
|
||||
|
||||
- @patch("keylime.models.verifier.auth_session.get_session")
|
||||
+ @patch("keylime.models.verifier.auth_session.get_session_context")
|
||||
@patch.object(AuthSession, "get_by_token")
|
||||
def test_authenticate_agent_success(self, mock_get_by_token, mock_get_session):
|
||||
"""Test successful agent authentication with valid token."""
|
||||
@@ -409,7 +440,8 @@ class TestAuthSessionCore(unittest.TestCase):
|
||||
# Mock session query
|
||||
mock_db_session = MagicMock()
|
||||
mock_db_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value = mock_db_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_db_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.get_by_token to return an active session
|
||||
mock_auth_session = MagicMock()
|
||||
diff --git a/test/test_session_controller.py b/test/test_session_controller.py
|
||||
index d807119..eec7fef 100644
|
||||
--- a/test/test_session_controller.py
|
||||
+++ b/test/test_session_controller.py
|
||||
@@ -272,7 +272,7 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
# Verify session was deleted from cache
|
||||
self.assertNotIn(self.test_session_id, self.sessions_cache)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
def test_update_session_agent_not_enrolled(self, mock_get_session):
|
||||
"""Test update_session with unenrolled agent."""
|
||||
# Create session in cache
|
||||
@@ -290,7 +290,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
# Mock database query to return no agent
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = None
|
||||
- mock_get_session.return_value = mock_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Call update_session
|
||||
params = {
|
||||
@@ -318,7 +319,7 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
body = call_args[1]["body"]
|
||||
self.assertEqual(body["data"]["attributes"]["evaluation"], "fail")
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.create_from_memory")
|
||||
def test_update_session_authentication_failed(self, mock_create_from_memory, mock_get_session):
|
||||
"""Test update_session with failed authentication."""
|
||||
@@ -339,7 +340,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value = mock_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.create_from_memory to return errors
|
||||
mock_auth_session = MagicMock()
|
||||
@@ -377,7 +379,7 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
call_args = self.controller.send_response.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[1]["code"], 401)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.create_from_memory")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.delete_active_session_for_agent")
|
||||
@patch("keylime.web.verifier.session_controller.config")
|
||||
@@ -403,7 +405,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value = mock_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock config
|
||||
mock_config.getboolean.return_value = False # Don't keep in memory
|
||||
@@ -522,7 +525,7 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
self.assertEqual(call_args[0][0], 404)
|
||||
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.delete_stale")
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.create")
|
||||
def test_create_success(self, mock_create, mock_get_session, _mock_delete_stale):
|
||||
"""Test successful create endpoint."""
|
||||
@@ -531,7 +534,8 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value = mock_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.create
|
||||
mock_auth_session = MagicMock()
|
||||
@@ -549,13 +553,14 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
call_args = self.controller.respond.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[0][0], 200)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
def test_create_agent_not_found(self, mock_get_session):
|
||||
"""Test create endpoint with non-existent agent."""
|
||||
# Mock database query to return None
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = None
|
||||
- mock_get_session.return_value = mock_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Call create
|
||||
params = {"data": {}}
|
||||
@@ -566,7 +571,7 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
call_args = self.controller.respond.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[0][0], 404)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.get_by_token")
|
||||
def test_update_success(self, mock_get, mock_get_session):
|
||||
"""Test successful update endpoint."""
|
||||
@@ -575,7 +580,8 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value = mock_session
|
||||
+ mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.get_by_token
|
||||
mock_auth_session = MagicMock()
|
||||
@@ -595,7 +601,7 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
call_args = self.controller.respond.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[0][0], 200)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session")
|
||||
+ @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.get_by_token")
|
||||
def test_update_not_found(self, mock_get, _mock_get_session):
|
||||
"""Test update endpoint with non-existent session."""
|
||||
--
|
||||
2.53.0
|
||||
|
||||
198
0004-Include-thread-safe-session-management.patch
Normal file
198
0004-Include-thread-safe-session-management.patch
Normal file
@ -0,0 +1,198 @@
|
||||
From bc28d5d228d005702f72e98646c8cad73196ccfb Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
Date: Tue, 10 Mar 2026 13:22:04 +0100
|
||||
Subject: [PATCH 4/6] Include thread-safe session management
|
||||
|
||||
Replace open-ended SQLAlchemy sessions with a context manager that
|
||||
guarantees connection release, preventing QueuePool exhaustion under
|
||||
multi-host push attestation load.
|
||||
|
||||
Key changes:
|
||||
- Add double-checked locking for lazy engine initialization to prevent
|
||||
race conditions in multi-threaded verifier
|
||||
- Delegate session lifecycle to SessionManager.session_context() which
|
||||
provides proper rollback on exception and scoped_session.remove()
|
||||
cleanup, eliminating thread-local connection leaks
|
||||
- Use session.expunge(agent) before exiting context manager scope so
|
||||
VerfierMain instances safely cross session boundaries without
|
||||
DetachedInstanceError
|
||||
- Scope with-blocks narrowly: connection is returned to pool before
|
||||
any subsequent DB calls (e.g. AuthSession.get_by_token) to prevent
|
||||
connection hoarding across separate pool boundaries
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
---
|
||||
keylime/models/verifier/auth_session.py | 15 +++---
|
||||
keylime/web/verifier/session_controller.py | 6 +++
|
||||
test/test_auth_session.py | 60 ++++++++++++++++------
|
||||
3 files changed, 59 insertions(+), 22 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/verifier/auth_session.py b/keylime/models/verifier/auth_session.py
|
||||
index 545995f..918dfb4 100644
|
||||
--- a/keylime/models/verifier/auth_session.py
|
||||
+++ b/keylime/models/verifier/auth_session.py
|
||||
@@ -1,5 +1,6 @@
|
||||
import base64
|
||||
import hmac
|
||||
+import threading
|
||||
import uuid
|
||||
from contextlib import contextmanager
|
||||
from datetime import timedelta
|
||||
@@ -31,19 +32,19 @@ from keylime.tpm.tpm_main import Tpm
|
||||
logger = keylime_logging.init_logging("verifier")
|
||||
|
||||
_engine = None
|
||||
+_engine_lock = threading.Lock()
|
||||
+_session_manager = SessionManager()
|
||||
|
||||
|
||||
@contextmanager
|
||||
def get_session_context() -> Iterator[Session]:
|
||||
global _engine
|
||||
if _engine is None:
|
||||
- _engine = make_engine("cloud_verifier")
|
||||
- session_manager = SessionManager()
|
||||
- session = session_manager.make_session(_engine)
|
||||
- try:
|
||||
+ with _engine_lock:
|
||||
+ if _engine is None:
|
||||
+ _engine = make_engine("cloud_verifier")
|
||||
+ with _session_manager.session_context(_engine) as session:
|
||||
yield session
|
||||
- finally:
|
||||
- session.close()
|
||||
|
||||
|
||||
class AuthSession(PersistableModel):
|
||||
@@ -283,6 +284,8 @@ class AuthSession(PersistableModel):
|
||||
.filter(VerfierMain.agent_id == auth_session.agent_id) # type: ignore[attr-defined]
|
||||
.one_or_none()
|
||||
)
|
||||
+ if agent:
|
||||
+ session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
|
||||
return agent
|
||||
|
||||
diff --git a/keylime/web/verifier/session_controller.py b/keylime/web/verifier/session_controller.py
|
||||
index 49cd758..3faa310 100644
|
||||
--- a/keylime/web/verifier/session_controller.py
|
||||
+++ b/keylime/web/verifier/session_controller.py
|
||||
@@ -187,6 +187,8 @@ class SessionController(Controller):
|
||||
# Check if agent exists - this is where we validate enrollment
|
||||
with get_session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
+ if agent:
|
||||
+ session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
|
||||
if not agent:
|
||||
# Agent not enrolled - return 200 with evaluation:fail
|
||||
@@ -382,6 +384,8 @@ class SessionController(Controller):
|
||||
def create(self, agent_id, **params):
|
||||
with get_session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
+ if agent:
|
||||
+ session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
|
||||
if not agent:
|
||||
self.respond(404, "here")
|
||||
@@ -405,6 +409,8 @@ class SessionController(Controller):
|
||||
def update(self, agent_id, token, **params):
|
||||
with get_session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
+ if agent:
|
||||
+ session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
|
||||
# Look up session by token hash (tokens are never stored in plaintext)
|
||||
auth_session = AuthSession.get_by_token(token)
|
||||
diff --git a/test/test_auth_session.py b/test/test_auth_session.py
|
||||
index 8e9ec98..2c78547 100644
|
||||
--- a/test/test_auth_session.py
|
||||
+++ b/test/test_auth_session.py
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import base64
|
||||
import unittest
|
||||
+from contextlib import contextmanager
|
||||
from datetime import timedelta
|
||||
from unittest.mock import MagicMock, PropertyMock, patch
|
||||
|
||||
@@ -14,32 +15,59 @@ from keylime.shared_data import cleanup_global_shared_memory, get_shared_memory
|
||||
class TestGetSessionContext(unittest.TestCase):
|
||||
"""Test cases for get_session_context context manager."""
|
||||
|
||||
+ def _make_mock_session_manager(self, mock_session):
|
||||
+ """Create a mock SessionManager whose session_context() mirrors real lifecycle."""
|
||||
+ mock_scoped = MagicMock()
|
||||
+ mock_session_manager = MagicMock()
|
||||
+ mock_session_manager.make_session.return_value = mock_session
|
||||
+ mock_session_manager._scoped_session = mock_scoped # pylint: disable=protected-access
|
||||
+
|
||||
+ @contextmanager
|
||||
+ def fake_session_context(engine): # pylint: disable=unused-argument
|
||||
+ session = mock_session_manager.make_session(engine)
|
||||
+ try:
|
||||
+ yield session
|
||||
+ session.commit()
|
||||
+ except Exception:
|
||||
+ session.rollback()
|
||||
+ raise
|
||||
+ finally:
|
||||
+ scoped = mock_session_manager._scoped_session # pylint: disable=protected-access
|
||||
+ if scoped is not None:
|
||||
+ scoped.remove()
|
||||
+
|
||||
+ mock_session_manager.session_context = fake_session_context
|
||||
+ return mock_session_manager, mock_scoped
|
||||
+
|
||||
@patch("keylime.models.verifier.auth_session.make_engine")
|
||||
- @patch("keylime.models.verifier.auth_session.SessionManager")
|
||||
- def test_session_closed_on_normal_exit(self, mock_session_manager_cls, _mock_make_engine):
|
||||
- """Test that session.close() is called when context manager exits normally."""
|
||||
+ def test_session_cleanup_on_normal_exit(self, _mock_make_engine):
|
||||
+ """Test that session is committed and cleaned up when context manager exits normally."""
|
||||
mock_session = MagicMock()
|
||||
- mock_session_manager_cls.return_value.make_session.return_value = mock_session
|
||||
+ mock_session_manager, mock_scoped = self._make_mock_session_manager(mock_session)
|
||||
|
||||
with patch("keylime.models.verifier.auth_session._engine", None):
|
||||
- with get_session_context() as session:
|
||||
- self.assertIs(session, mock_session)
|
||||
+ with patch("keylime.models.verifier.auth_session._session_manager", mock_session_manager):
|
||||
+ with get_session_context() as session:
|
||||
+ self.assertIs(session, mock_session)
|
||||
|
||||
- mock_session.close.assert_called_once()
|
||||
+ mock_session.commit.assert_called_once()
|
||||
+ mock_scoped.remove.assert_called_once()
|
||||
|
||||
@patch("keylime.models.verifier.auth_session.make_engine")
|
||||
- @patch("keylime.models.verifier.auth_session.SessionManager")
|
||||
- def test_session_closed_on_exception(self, mock_session_manager_cls, _mock_make_engine):
|
||||
- """Test that session.close() is called even when an exception occurs."""
|
||||
+ def test_session_rollback_on_exception(self, _mock_make_engine):
|
||||
+ """Test that session is rolled back and cleaned up when an exception occurs."""
|
||||
mock_session = MagicMock()
|
||||
- mock_session_manager_cls.return_value.make_session.return_value = mock_session
|
||||
+ mock_session_manager, mock_scoped = self._make_mock_session_manager(mock_session)
|
||||
|
||||
with patch("keylime.models.verifier.auth_session._engine", None):
|
||||
- with self.assertRaises(RuntimeError):
|
||||
- with get_session_context():
|
||||
- raise RuntimeError("simulated error")
|
||||
-
|
||||
- mock_session.close.assert_called_once()
|
||||
+ with patch("keylime.models.verifier.auth_session._session_manager", mock_session_manager):
|
||||
+ with self.assertRaises(RuntimeError):
|
||||
+ with get_session_context():
|
||||
+ raise RuntimeError("simulated error")
|
||||
+
|
||||
+ mock_session.rollback.assert_called_once()
|
||||
+ mock_session.commit.assert_not_called()
|
||||
+ mock_scoped.remove.assert_called_once()
|
||||
|
||||
|
||||
class TestAuthSessionHelpers(unittest.TestCase):
|
||||
--
|
||||
2.53.0
|
||||
|
||||
79
0005-Address-some-improvements-from-code-review.patch
Normal file
79
0005-Address-some-improvements-from-code-review.patch
Normal file
@ -0,0 +1,79 @@
|
||||
From 4f5f09a69e01c0116f1977aa3a741f3678bb8e67 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
Date: Thu, 12 Mar 2026 15:18:56 +0100
|
||||
Subject: [PATCH 5/6] Address some improvements from code review
|
||||
|
||||
Include agent variable None initialization
|
||||
and address thread safety for ContextManager
|
||||
|
||||
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
---
|
||||
keylime/db/keylime_db.py | 7 ++++++-
|
||||
keylime/web/verifier/session_controller.py | 3 +++
|
||||
2 files changed, 9 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/keylime/db/keylime_db.py b/keylime/db/keylime_db.py
|
||||
index 6fd3f08..cf608fa 100644
|
||||
--- a/keylime/db/keylime_db.py
|
||||
+++ b/keylime/db/keylime_db.py
|
||||
@@ -1,4 +1,5 @@
|
||||
import os
|
||||
+import threading
|
||||
from configparser import NoOptionError
|
||||
from contextlib import contextmanager
|
||||
from sqlite3 import Connection as SQLite3Connection
|
||||
@@ -89,10 +90,12 @@ def make_engine(service: str, **engine_args: Any) -> Engine:
|
||||
class SessionManager:
|
||||
engine: Optional[Engine]
|
||||
_scoped_session: Optional[scoped_session]
|
||||
+ _lock: threading.Lock
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.engine = None
|
||||
self._scoped_session = None
|
||||
+ self._lock = threading.Lock()
|
||||
|
||||
def make_session(self, engine: Engine) -> Session:
|
||||
"""
|
||||
@@ -100,7 +103,9 @@ class SessionManager:
|
||||
"""
|
||||
self.engine = engine
|
||||
if self._scoped_session is None:
|
||||
- self._scoped_session = scoped_session(sessionmaker())
|
||||
+ with self._lock:
|
||||
+ if self._scoped_session is None:
|
||||
+ self._scoped_session = scoped_session(sessionmaker())
|
||||
try:
|
||||
self._scoped_session.configure(bind=self.engine) # type: ignore
|
||||
self._scoped_session.configure(expire_on_commit=False) # type: ignore
|
||||
diff --git a/keylime/web/verifier/session_controller.py b/keylime/web/verifier/session_controller.py
|
||||
index 3faa310..c8664e2 100644
|
||||
--- a/keylime/web/verifier/session_controller.py
|
||||
+++ b/keylime/web/verifier/session_controller.py
|
||||
@@ -185,6 +185,7 @@ class SessionController(Controller):
|
||||
return
|
||||
|
||||
# Check if agent exists - this is where we validate enrollment
|
||||
+ agent = None
|
||||
with get_session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
if agent:
|
||||
@@ -382,6 +383,7 @@ class SessionController(Controller):
|
||||
|
||||
# POST /v3[.:minor]/agents/:agent_id/session
|
||||
def create(self, agent_id, **params):
|
||||
+ agent = None
|
||||
with get_session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
if agent:
|
||||
@@ -407,6 +409,7 @@ class SessionController(Controller):
|
||||
self.respond(200, "Success", auth_session.render())
|
||||
|
||||
def update(self, agent_id, token, **params):
|
||||
+ agent = None
|
||||
with get_session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
if agent:
|
||||
--
|
||||
2.53.0
|
||||
|
||||
42
0006-Fix-race-condition-on-in-SessionManager.patch
Normal file
42
0006-Fix-race-condition-on-in-SessionManager.patch
Normal file
@ -0,0 +1,42 @@
|
||||
From 309a0ef0fe1d0917ad9d4fd7ab4327570a59cf34 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
Date: Thu, 12 Mar 2026 19:18:56 +0100
|
||||
Subject: [PATCH 6/6] Fix race condition on in SessionManager
|
||||
|
||||
Move self.engine assignment inside the lock so it is set atomically
|
||||
with _scoped_session creation. Without this, concurrent threads calling
|
||||
make_session() with different engines could race on the assignment,
|
||||
causing _scoped_session to be configured with a stale engine reference.
|
||||
|
||||
Also log a warning if make_session() is called with a different engine
|
||||
after initialization, since the scoped_session is cached and bound to
|
||||
the original engine.
|
||||
|
||||
Suggested-by: coderabbitai
|
||||
Signed-off-by: Sergio Arroutbi <sarroutb@redhat.com>
|
||||
---
|
||||
keylime/db/keylime_db.py | 4 +++-
|
||||
1 file changed, 3 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/keylime/db/keylime_db.py b/keylime/db/keylime_db.py
|
||||
index cf608fa..a622b09 100644
|
||||
--- a/keylime/db/keylime_db.py
|
||||
+++ b/keylime/db/keylime_db.py
|
||||
@@ -101,11 +101,13 @@ class SessionManager:
|
||||
"""
|
||||
To use: session = self.make_session(engine)
|
||||
"""
|
||||
- self.engine = engine
|
||||
if self._scoped_session is None:
|
||||
with self._lock:
|
||||
if self._scoped_session is None:
|
||||
+ self.engine = engine
|
||||
self._scoped_session = scoped_session(sessionmaker())
|
||||
+ elif self.engine is not engine:
|
||||
+ logger.warning("SessionManager called with different engine than originally configured")
|
||||
try:
|
||||
self._scoped_session.configure(bind=self.engine) # type: ignore
|
||||
self._scoped_session.configure(expire_on_commit=False) # type: ignore
|
||||
--
|
||||
2.53.0
|
||||
|
||||
160
0007-Fix-linter-errors-in-PersistableModel.get-and-.all.patch
Normal file
160
0007-Fix-linter-errors-in-PersistableModel.get-and-.all.patch
Normal file
@ -0,0 +1,160 @@
|
||||
From e75921f02393277e8bc5ba3d058131376516a099 Mon Sep 17 00:00:00 2001
|
||||
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
Date: Thu, 5 Mar 2026 17:27:41 +0100
|
||||
Subject: [PATCH] Fix linter errors in PersistableModel.get() and .all()
|
||||
|
||||
PersistableModel.get() and .all() returned Optional[PersistableModel]
|
||||
and Sequence[PersistableModel] respectively, which caused pyright errors
|
||||
when subclasses like IMAPolicy or MBPolicy called cls.get() and expected
|
||||
the return type to match their own class.
|
||||
|
||||
Use a TypeVar bound to PersistableModel so cls.get() on a subclass
|
||||
correctly returns Optional[SubclassType].
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
---
|
||||
keylime/models/base/persistable_model.py | 8 +++++---
|
||||
keylime/web/registrar/agents_controller.py | 6 +++---
|
||||
keylime/web/verifier/attestation_controller.py | 17 ++++++++---------
|
||||
3 files changed, 16 insertions(+), 15 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/base/persistable_model.py b/keylime/models/base/persistable_model.py
|
||||
index 3380eb6..4aa596e 100644
|
||||
--- a/keylime/models/base/persistable_model.py
|
||||
+++ b/keylime/models/base/persistable_model.py
|
||||
@@ -1,4 +1,4 @@
|
||||
-from typing import Any, Optional, Sequence
|
||||
+from typing import Any, Optional, Sequence, TypeVar
|
||||
|
||||
from sqlalchemy import asc, desc, or_
|
||||
from sqlalchemy.sql.expression import ClauseElement
|
||||
@@ -18,6 +18,8 @@ from keylime.models.base.persistable_model_meta import PersistableModelMeta
|
||||
from keylime.models.base.types.dictionary import Dictionary
|
||||
from keylime.models.base.types.list import List
|
||||
|
||||
+_PM = TypeVar("_PM", bound="PersistableModel")
|
||||
+
|
||||
|
||||
class PersistableModel(BasicModel, metaclass=PersistableModelMeta):
|
||||
"""PersistableModel extends the BasicModel class to provide additional functionality for saving and retrieving
|
||||
@@ -181,7 +183,7 @@ class PersistableModel(BasicModel, metaclass=PersistableModelMeta):
|
||||
return session.query(subject).filter(*filter_criteria).order_by(*sort_criteria)
|
||||
|
||||
@classmethod
|
||||
- def get(cls, *args: Any, **kwargs: Any) -> Optional["PersistableModel"]:
|
||||
+ def get(cls: type[_PM], *args: Any, **kwargs: Any) -> Optional[_PM]:
|
||||
# pylint: disable=no-else-return
|
||||
|
||||
if cls.schema_awaiting_processing:
|
||||
@@ -203,7 +205,7 @@ class PersistableModel(BasicModel, metaclass=PersistableModelMeta):
|
||||
return None
|
||||
|
||||
@classmethod
|
||||
- def all(cls, *args: Any, **kwargs: Any) -> Sequence["PersistableModel"]:
|
||||
+ def all(cls: type[_PM], *args: Any, **kwargs: Any) -> Sequence[_PM]:
|
||||
if cls.schema_awaiting_processing:
|
||||
cls.process_schema()
|
||||
|
||||
diff --git a/keylime/web/registrar/agents_controller.py b/keylime/web/registrar/agents_controller.py
|
||||
index 290317f..c918f95 100644
|
||||
--- a/keylime/web/registrar/agents_controller.py
|
||||
+++ b/keylime/web/registrar/agents_controller.py
|
||||
@@ -27,7 +27,7 @@ class AgentsController(Controller):
|
||||
self.respond(404, f"Agent with ID '{agent_id}' has not been activated")
|
||||
return
|
||||
|
||||
- self.respond(200, "Success", agent.render())
|
||||
+ self.respond(200, "Success", agent.render()) # type: ignore[no-untyped-call]
|
||||
|
||||
# POST /v2[.:minor]/agents/[:agent_id]
|
||||
def create(self, agent_id, **params):
|
||||
@@ -143,10 +143,10 @@ class AgentsController(Controller):
|
||||
self.respond(404, f"Agent with ID '{agent_id}' not found")
|
||||
return
|
||||
|
||||
- accepted = agent.verify_ak_response(auth_tag) # type: ignore[attr-defined]
|
||||
+ accepted = agent.verify_ak_response(auth_tag) # type: ignore[attr-defined,no-untyped-call]
|
||||
|
||||
if accepted:
|
||||
- agent.commit_changes()
|
||||
+ agent.commit_changes() # type: ignore[no-untyped-call]
|
||||
self.respond(200, "Success")
|
||||
else:
|
||||
agent.delete()
|
||||
diff --git a/keylime/web/verifier/attestation_controller.py b/keylime/web/verifier/attestation_controller.py
|
||||
index 0e50b8a..59f280c 100755
|
||||
--- a/keylime/web/verifier/attestation_controller.py
|
||||
+++ b/keylime/web/verifier/attestation_controller.py
|
||||
@@ -1,6 +1,5 @@
|
||||
# pyright: reportAttributeAccessIssue=false
|
||||
# Uses ORM models with dynamically-created attributes from metaclasses
|
||||
-from typing import cast
|
||||
|
||||
from keylime import agent_util, config, keylime_logging
|
||||
from keylime.common import retry
|
||||
@@ -158,12 +157,12 @@ class AttestationController(Controller):
|
||||
|
||||
# GET /v3[.:minor]/agents/:agent_id/attestations
|
||||
def index(self, agent_id, **_params): # type: ignore[no-untyped-def]
|
||||
- agent = cast(VerifierAgent | None, VerifierAgent.get(agent_id))
|
||||
+ agent = VerifierAgent.get(agent_id)
|
||||
|
||||
if not agent:
|
||||
APIError("not_found", f"No enrolled agent with ID '{agent_id}'.").send_via(self)
|
||||
|
||||
- results = cast(list[Attestation], Attestation.all(agent_id=agent_id))
|
||||
+ results = Attestation.all(agent_id=agent_id)
|
||||
|
||||
resources = [
|
||||
APIResource("attestation", attestation.render_state()).include( # type: ignore[no-untyped-call]
|
||||
@@ -184,8 +183,8 @@ class AttestationController(Controller):
|
||||
|
||||
# GET /v3[.:minor]/agents/:agent_id/attestations/:index
|
||||
def show(self, agent_id, index, **_params): # type: ignore[no-untyped-def]
|
||||
- agent = cast(VerifierAgent | None, VerifierAgent.get(agent_id))
|
||||
- attestation = cast(Attestation | None, Attestation.get(agent_id=agent_id, index=index))
|
||||
+ agent = VerifierAgent.get(agent_id)
|
||||
+ attestation = Attestation.get(agent_id=agent_id, index=index)
|
||||
|
||||
if not agent:
|
||||
APIError("not_found", f"No enrolled agent with ID '{agent_id}'.").send_via(self)
|
||||
@@ -201,7 +200,7 @@ class AttestationController(Controller):
|
||||
|
||||
# GET /v3[.:minor]/agents/:agent_id/attestations/latest
|
||||
def show_latest(self, agent_id, **_params): # type: ignore[no-untyped-def]
|
||||
- agent = cast(VerifierAgent | None, VerifierAgent.get(agent_id))
|
||||
+ agent = VerifierAgent.get(agent_id)
|
||||
|
||||
if not agent:
|
||||
APIError("not_found", f"No enrolled agent with ID '{agent_id}'.").send_via(self)
|
||||
@@ -214,7 +213,7 @@ class AttestationController(Controller):
|
||||
# POST /v3[.:minor]/agents/:agent_id/attestations
|
||||
@Controller.require_json_api
|
||||
def create(self, agent_id, attestation, **params): # type: ignore[no-untyped-def] # pylint: disable=unused-argument
|
||||
- agent = cast(VerifierAgent | None, VerifierAgent.get(agent_id))
|
||||
+ agent = VerifierAgent.get(agent_id)
|
||||
|
||||
if not agent:
|
||||
APIError("not_found", f"No enrolled agent with ID '{agent_id}'.").send_via(self)
|
||||
@@ -310,7 +309,7 @@ class AttestationController(Controller):
|
||||
# Extract attestation from params - it should be provided by the API request
|
||||
attestation = params.get("attestation")
|
||||
|
||||
- agent = cast(VerifierAgent | None, VerifierAgent.get(agent_id))
|
||||
+ agent = VerifierAgent.get(agent_id)
|
||||
|
||||
if not agent:
|
||||
APIError("not_found", f"No enrolled agent with ID '{agent_id}'.").send_via(self)
|
||||
@@ -368,7 +367,7 @@ class AttestationController(Controller):
|
||||
# PATCH /v3[.:minor]/agents/:agent_id/attestations/latest
|
||||
@Controller.require_json_api
|
||||
def update_latest(self, agent_id, **params): # type: ignore[no-untyped-def]
|
||||
- agent = cast(VerifierAgent | None, VerifierAgent.get(agent_id))
|
||||
+ agent = VerifierAgent.get(agent_id)
|
||||
|
||||
if not agent:
|
||||
APIError("not_found", f"No enrolled agent with ID '{agent_id}'.").send_via(self)
|
||||
--
|
||||
2.53.0
|
||||
|
||||
457
0008-refactor-Remove-dead-code-AuthSession.authenticate_a.patch
Normal file
457
0008-refactor-Remove-dead-code-AuthSession.authenticate_a.patch
Normal file
@ -0,0 +1,457 @@
|
||||
From 2d809d8b537c0d9faab05ee5fe7efb85f48918f3 Mon Sep 17 00:00:00 2001
|
||||
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
Date: Fri, 13 Mar 2026 10:53:54 +0100
|
||||
Subject: [PATCH] refactor: Remove dead code AuthSession.authenticate_agent()
|
||||
|
||||
authenticate_agent() was superseded by _extract_identity() in
|
||||
action_handler.py, which performs token-based agent authentication
|
||||
directly via AuthSession.get_by_token(). The method, its helper
|
||||
get_session(), the module-level _engine global, and the associated
|
||||
unused imports (Session, SessionManager, make_engine) are all removed.
|
||||
|
||||
The corresponding tests (test_authenticate_agent_success,
|
||||
test_authenticate_agent_inactive_session,
|
||||
test_authenticate_agent_no_session) are also removed.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
---
|
||||
keylime/models/verifier/auth_session.py | 67 +-----------
|
||||
keylime/web/verifier/session_controller.py | 9 +-
|
||||
test/test_auth_session.py | 113 +--------------------
|
||||
test/test_session_controller.py | 52 +++++-----
|
||||
4 files changed, 32 insertions(+), 209 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/verifier/auth_session.py b/keylime/models/verifier/auth_session.py
|
||||
index 918dfb4..b0b40b0 100644
|
||||
--- a/keylime/models/verifier/auth_session.py
|
||||
+++ b/keylime/models/verifier/auth_session.py
|
||||
@@ -1,12 +1,8 @@
|
||||
import base64
|
||||
import hmac
|
||||
-import threading
|
||||
import uuid
|
||||
-from contextlib import contextmanager
|
||||
from datetime import timedelta
|
||||
-from typing import Any, Dict, Iterator, Optional, Sequence
|
||||
-
|
||||
-from sqlalchemy.orm import Session
|
||||
+from typing import Any, Dict, Optional, Sequence
|
||||
|
||||
from keylime import config, keylime_logging
|
||||
from keylime.crypto import (
|
||||
@@ -16,7 +12,6 @@ from keylime.crypto import (
|
||||
parse_session_token,
|
||||
verify_token_hash,
|
||||
)
|
||||
-from keylime.db.keylime_db import SessionManager, make_engine
|
||||
from keylime.db.verifier_db import VerfierMain
|
||||
from keylime.models.base import *
|
||||
from keylime.shared_data import get_shared_memory
|
||||
@@ -31,21 +26,6 @@ from keylime.tpm.tpm_main import Tpm
|
||||
|
||||
logger = keylime_logging.init_logging("verifier")
|
||||
|
||||
-_engine = None
|
||||
-_engine_lock = threading.Lock()
|
||||
-_session_manager = SessionManager()
|
||||
-
|
||||
-
|
||||
-@contextmanager
|
||||
-def get_session_context() -> Iterator[Session]:
|
||||
- global _engine
|
||||
- if _engine is None:
|
||||
- with _engine_lock:
|
||||
- if _engine is None:
|
||||
- _engine = make_engine("cloud_verifier")
|
||||
- with _session_manager.session_context(_engine) as session:
|
||||
- yield session
|
||||
-
|
||||
|
||||
class AuthSession(PersistableModel):
|
||||
# Explicit attribute declarations for type checkers
|
||||
@@ -244,51 +224,6 @@ class AuthSession(PersistableModel):
|
||||
# Slow path: query database by primary key
|
||||
return cls.get(session_id) # type: ignore[return-value]
|
||||
|
||||
- @classmethod
|
||||
- def authenticate_agent(cls, token: str): # type: ignore[no-untyped-def]
|
||||
- """Authenticate an agent using their session token.
|
||||
-
|
||||
- Uses indexed database lookup by token hash for performance (O(1) instead of O(n)).
|
||||
- Tokens are hashed before lookup since only hashes are stored in the database.
|
||||
-
|
||||
- Args:
|
||||
- token: The session token to verify
|
||||
-
|
||||
- Returns:
|
||||
- VerfierMain object if authenticated, False otherwise
|
||||
- """
|
||||
- # Use indexed lookup by token hash (much faster than scanning all sessions)
|
||||
- auth_session = cls.get_by_token(token)
|
||||
-
|
||||
- if not auth_session:
|
||||
- return False
|
||||
-
|
||||
- # Validate session is active
|
||||
- if not getattr(auth_session, "active", False):
|
||||
- return False
|
||||
-
|
||||
- # Validate session hasn't expired
|
||||
- token_expires_at = getattr(auth_session, "token_expires_at", None)
|
||||
- if token_expires_at and token_expires_at < Timestamp.now():
|
||||
- logger.debug(
|
||||
- "Authentication attempted with expired token for agent '%s' (expired at %s)",
|
||||
- getattr(auth_session, "agent_id", "unknown"),
|
||||
- token_expires_at,
|
||||
- )
|
||||
- return False
|
||||
-
|
||||
- # Use old engine to query VerfierMain (legacy model)
|
||||
- with get_session_context() as session:
|
||||
- agent = (
|
||||
- session.query(VerfierMain)
|
||||
- .filter(VerfierMain.agent_id == auth_session.agent_id) # type: ignore[attr-defined]
|
||||
- .one_or_none()
|
||||
- )
|
||||
- if agent:
|
||||
- session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
-
|
||||
- return agent
|
||||
-
|
||||
@classmethod
|
||||
def create(
|
||||
cls, agent: Optional[VerfierMain], data: Dict[str, Any], agent_id: Optional[str] = None
|
||||
diff --git a/keylime/web/verifier/session_controller.py b/keylime/web/verifier/session_controller.py
|
||||
index c8664e2..9a314f2 100644
|
||||
--- a/keylime/web/verifier/session_controller.py
|
||||
+++ b/keylime/web/verifier/session_controller.py
|
||||
@@ -2,9 +2,8 @@ import base64
|
||||
|
||||
from keylime import config, keylime_logging
|
||||
from keylime.db.verifier_db import VerfierMain
|
||||
-from keylime.models.base import Timestamp
|
||||
+from keylime.models.base import Timestamp, db_manager
|
||||
from keylime.models.verifier import AuthSession
|
||||
-from keylime.models.verifier.auth_session import get_session_context
|
||||
from keylime.web.base import Controller
|
||||
|
||||
logger = keylime_logging.init_logging("verifier")
|
||||
@@ -186,7 +185,7 @@ class SessionController(Controller):
|
||||
|
||||
# Check if agent exists - this is where we validate enrollment
|
||||
agent = None
|
||||
- with get_session_context() as session:
|
||||
+ with db_manager.session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
if agent:
|
||||
session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
@@ -384,7 +383,7 @@ class SessionController(Controller):
|
||||
# POST /v3[.:minor]/agents/:agent_id/session
|
||||
def create(self, agent_id, **params):
|
||||
agent = None
|
||||
- with get_session_context() as session:
|
||||
+ with db_manager.session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
if agent:
|
||||
session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
@@ -410,7 +409,7 @@ class SessionController(Controller):
|
||||
|
||||
def update(self, agent_id, token, **params):
|
||||
agent = None
|
||||
- with get_session_context() as session:
|
||||
+ with db_manager.session_context() as session:
|
||||
agent = session.query(VerfierMain).filter(VerfierMain.agent_id == agent_id).one_or_none()
|
||||
if agent:
|
||||
session.expunge(agent) # type: ignore[no-untyped-call]
|
||||
diff --git a/test/test_auth_session.py b/test/test_auth_session.py
|
||||
index 2c78547..dd554b6 100644
|
||||
--- a/test/test_auth_session.py
|
||||
+++ b/test/test_auth_session.py
|
||||
@@ -2,74 +2,15 @@
|
||||
|
||||
import base64
|
||||
import unittest
|
||||
-from contextlib import contextmanager
|
||||
from datetime import timedelta
|
||||
from unittest.mock import MagicMock, PropertyMock, patch
|
||||
|
||||
from keylime.crypto import generate_session_token, generate_token_salt, hash_token_for_storage
|
||||
from keylime.models.base.types import Timestamp
|
||||
-from keylime.models.verifier.auth_session import AuthSession, get_session_context
|
||||
+from keylime.models.verifier.auth_session import AuthSession
|
||||
from keylime.shared_data import cleanup_global_shared_memory, get_shared_memory
|
||||
|
||||
|
||||
-class TestGetSessionContext(unittest.TestCase):
|
||||
- """Test cases for get_session_context context manager."""
|
||||
-
|
||||
- def _make_mock_session_manager(self, mock_session):
|
||||
- """Create a mock SessionManager whose session_context() mirrors real lifecycle."""
|
||||
- mock_scoped = MagicMock()
|
||||
- mock_session_manager = MagicMock()
|
||||
- mock_session_manager.make_session.return_value = mock_session
|
||||
- mock_session_manager._scoped_session = mock_scoped # pylint: disable=protected-access
|
||||
-
|
||||
- @contextmanager
|
||||
- def fake_session_context(engine): # pylint: disable=unused-argument
|
||||
- session = mock_session_manager.make_session(engine)
|
||||
- try:
|
||||
- yield session
|
||||
- session.commit()
|
||||
- except Exception:
|
||||
- session.rollback()
|
||||
- raise
|
||||
- finally:
|
||||
- scoped = mock_session_manager._scoped_session # pylint: disable=protected-access
|
||||
- if scoped is not None:
|
||||
- scoped.remove()
|
||||
-
|
||||
- mock_session_manager.session_context = fake_session_context
|
||||
- return mock_session_manager, mock_scoped
|
||||
-
|
||||
- @patch("keylime.models.verifier.auth_session.make_engine")
|
||||
- def test_session_cleanup_on_normal_exit(self, _mock_make_engine):
|
||||
- """Test that session is committed and cleaned up when context manager exits normally."""
|
||||
- mock_session = MagicMock()
|
||||
- mock_session_manager, mock_scoped = self._make_mock_session_manager(mock_session)
|
||||
-
|
||||
- with patch("keylime.models.verifier.auth_session._engine", None):
|
||||
- with patch("keylime.models.verifier.auth_session._session_manager", mock_session_manager):
|
||||
- with get_session_context() as session:
|
||||
- self.assertIs(session, mock_session)
|
||||
-
|
||||
- mock_session.commit.assert_called_once()
|
||||
- mock_scoped.remove.assert_called_once()
|
||||
-
|
||||
- @patch("keylime.models.verifier.auth_session.make_engine")
|
||||
- def test_session_rollback_on_exception(self, _mock_make_engine):
|
||||
- """Test that session is rolled back and cleaned up when an exception occurs."""
|
||||
- mock_session = MagicMock()
|
||||
- mock_session_manager, mock_scoped = self._make_mock_session_manager(mock_session)
|
||||
-
|
||||
- with patch("keylime.models.verifier.auth_session._engine", None):
|
||||
- with patch("keylime.models.verifier.auth_session._session_manager", mock_session_manager):
|
||||
- with self.assertRaises(RuntimeError):
|
||||
- with get_session_context():
|
||||
- raise RuntimeError("simulated error")
|
||||
-
|
||||
- mock_session.rollback.assert_called_once()
|
||||
- mock_session.commit.assert_not_called()
|
||||
- mock_scoped.remove.assert_called_once()
|
||||
-
|
||||
-
|
||||
class TestAuthSessionHelpers(unittest.TestCase):
|
||||
"""Test cases for AuthSession helper methods."""
|
||||
|
||||
@@ -457,58 +398,6 @@ class TestAuthSessionCore(unittest.TestCase):
|
||||
self.assertIn("errors", result)
|
||||
self.assertIn("authentication_supported", result["errors"])
|
||||
|
||||
- @patch("keylime.models.verifier.auth_session.get_session_context")
|
||||
- @patch.object(AuthSession, "get_by_token")
|
||||
- def test_authenticate_agent_success(self, mock_get_by_token, mock_get_session):
|
||||
- """Test successful agent authentication with valid token."""
|
||||
- # Create a mock agent
|
||||
- mock_agent = MagicMock()
|
||||
- mock_agent.agent_id = self.test_agent_id
|
||||
-
|
||||
- # Mock session query
|
||||
- mock_db_session = MagicMock()
|
||||
- mock_db_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_db_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
-
|
||||
- # Mock AuthSession.get_by_token to return an active session
|
||||
- mock_auth_session = MagicMock()
|
||||
- mock_auth_session.session_id = "550e8400-e29b-41d4-a716-446655440000"
|
||||
- mock_auth_session.active = True
|
||||
- mock_auth_session.agent_id = self.test_agent_id
|
||||
- mock_auth_session.token_expires_at = Timestamp.now() + timedelta(hours=1)
|
||||
- mock_get_by_token.return_value = mock_auth_session
|
||||
-
|
||||
- result = AuthSession.authenticate_agent("test-token")
|
||||
-
|
||||
- # Should return the agent
|
||||
- self.assertIsNotNone(result)
|
||||
- self.assertEqual(result.agent_id, self.test_agent_id) # type: ignore[union-attr]
|
||||
-
|
||||
- @patch.object(AuthSession, "get_by_token")
|
||||
- def test_authenticate_agent_inactive_session(self, mock_get_by_token):
|
||||
- """Test that inactive sessions cannot authenticate."""
|
||||
- # Mock AuthSession.get_by_token to return an inactive session
|
||||
- mock_auth_session = MagicMock()
|
||||
- mock_auth_session.active = False
|
||||
- mock_get_by_token.return_value = mock_auth_session
|
||||
-
|
||||
- result = AuthSession.authenticate_agent("test-token")
|
||||
-
|
||||
- # Should return False
|
||||
- self.assertFalse(result)
|
||||
-
|
||||
- @patch.object(AuthSession, "get_by_token")
|
||||
- def test_authenticate_agent_no_session(self, mock_get_by_token):
|
||||
- """Test that authentication fails when session doesn't exist."""
|
||||
- # Mock AuthSession.get_by_token to return None (no session found)
|
||||
- mock_get_by_token.return_value = None
|
||||
-
|
||||
- result = AuthSession.authenticate_agent("test-token")
|
||||
-
|
||||
- # Should return False
|
||||
- self.assertFalse(result)
|
||||
-
|
||||
@patch.object(AuthSession, "empty")
|
||||
def test_create_with_agent(self, mock_empty):
|
||||
"""Test AuthSession.create() with an enrolled agent."""
|
||||
diff --git a/test/test_session_controller.py b/test/test_session_controller.py
|
||||
index eec7fef..f8db8db 100644
|
||||
--- a/test/test_session_controller.py
|
||||
+++ b/test/test_session_controller.py
|
||||
@@ -272,8 +272,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
# Verify session was deleted from cache
|
||||
self.assertNotIn(self.test_session_id, self.sessions_cache)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
- def test_update_session_agent_not_enrolled(self, mock_get_session):
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
+ def test_update_session_agent_not_enrolled(self, mock_db_manager):
|
||||
"""Test update_session with unenrolled agent."""
|
||||
# Create session in cache
|
||||
now = Timestamp.now()
|
||||
@@ -290,8 +290,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
# Mock database query to return no agent
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = None
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
+ mock_db_manager.session_context.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_db_manager.session_context.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Call update_session
|
||||
params = {
|
||||
@@ -319,9 +319,9 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
body = call_args[1]["body"]
|
||||
self.assertEqual(body["data"]["attributes"]["evaluation"], "fail")
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.create_from_memory")
|
||||
- def test_update_session_authentication_failed(self, mock_create_from_memory, mock_get_session):
|
||||
+ def test_update_session_authentication_failed(self, mock_create_from_memory, mock_db_manager):
|
||||
"""Test update_session with failed authentication."""
|
||||
# Create session in cache
|
||||
now = Timestamp.now()
|
||||
@@ -340,8 +340,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
+ mock_db_manager.session_context.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_db_manager.session_context.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.create_from_memory to return errors
|
||||
mock_auth_session = MagicMock()
|
||||
@@ -379,11 +379,11 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
call_args = self.controller.send_response.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[1]["code"], 401)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.create_from_memory")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.delete_active_session_for_agent")
|
||||
@patch("keylime.web.verifier.session_controller.config")
|
||||
- def test_update_session_success(self, mock_config, _mock_delete_active, mock_create_from_memory, mock_get_session):
|
||||
+ def test_update_session_success(self, mock_config, _mock_delete_active, mock_create_from_memory, mock_db_manager):
|
||||
"""Test successful session update."""
|
||||
# Create session in cache
|
||||
now = Timestamp.now()
|
||||
@@ -405,8 +405,8 @@ class TestSessionControllerUpdateSession(unittest.TestCase):
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
+ mock_db_manager.session_context.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_db_manager.session_context.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock config
|
||||
mock_config.getboolean.return_value = False # Don't keep in memory
|
||||
@@ -525,17 +525,17 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
self.assertEqual(call_args[0][0], 404)
|
||||
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.delete_stale")
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.create")
|
||||
- def test_create_success(self, mock_create, mock_get_session, _mock_delete_stale):
|
||||
+ def test_create_success(self, mock_create, mock_db_manager, _mock_delete_stale):
|
||||
"""Test successful create endpoint."""
|
||||
# Mock database query
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
+ mock_db_manager.session_context.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_db_manager.session_context.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.create
|
||||
mock_auth_session = MagicMock()
|
||||
@@ -553,14 +553,14 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
call_args = self.controller.respond.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[0][0], 200)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
- def test_create_agent_not_found(self, mock_get_session):
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
+ def test_create_agent_not_found(self, mock_db_manager):
|
||||
"""Test create endpoint with non-existent agent."""
|
||||
# Mock database query to return None
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = None
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
+ mock_db_manager.session_context.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_db_manager.session_context.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Call create
|
||||
params = {"data": {}}
|
||||
@@ -571,17 +571,17 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
call_args = self.controller.respond.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[0][0], 404)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.get_by_token")
|
||||
- def test_update_success(self, mock_get, mock_get_session):
|
||||
+ def test_update_success(self, mock_get, mock_db_manager):
|
||||
"""Test successful update endpoint."""
|
||||
# Mock database query
|
||||
mock_agent = MagicMock()
|
||||
mock_agent.agent_id = self.test_agent_id
|
||||
mock_session = MagicMock()
|
||||
mock_session.query.return_value.filter.return_value.one_or_none.return_value = mock_agent
|
||||
- mock_get_session.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
- mock_get_session.return_value.__exit__ = MagicMock(return_value=False)
|
||||
+ mock_db_manager.session_context.return_value.__enter__ = MagicMock(return_value=mock_session)
|
||||
+ mock_db_manager.session_context.return_value.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Mock AuthSession.get_by_token
|
||||
mock_auth_session = MagicMock()
|
||||
@@ -601,9 +601,9 @@ class TestSessionControllerLegacyEndpoints(unittest.TestCase):
|
||||
call_args = self.controller.respond.call_args # type: ignore[attr-defined]
|
||||
self.assertEqual(call_args[0][0], 200)
|
||||
|
||||
- @patch("keylime.web.verifier.session_controller.get_session_context")
|
||||
+ @patch("keylime.web.verifier.session_controller.db_manager")
|
||||
@patch("keylime.models.verifier.auth_session.AuthSession.get_by_token")
|
||||
- def test_update_not_found(self, mock_get, _mock_get_session):
|
||||
+ def test_update_not_found(self, mock_get, _mock_db_manager):
|
||||
"""Test update endpoint with non-existent session."""
|
||||
# Mock AuthSession.get_by_token to return None
|
||||
mock_get.return_value = None
|
||||
--
|
||||
2.53.0
|
||||
|
||||
205
0009-db-Clean-up-scoped-session-after-each-request.patch
Normal file
205
0009-db-Clean-up-scoped-session-after-each-request.patch
Normal file
@ -0,0 +1,205 @@
|
||||
From e935df8fb9ad36daa41e079d19964678b28be246 Mon Sep 17 00:00:00 2001
|
||||
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
Date: Fri, 6 Mar 2026 11:47:04 +0100
|
||||
Subject: [PATCH] db: Clean up scoped session after each request
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
The scoped_session was never cleaned up between requests, causing its
|
||||
identity map to accumulate objects indefinitely. When subsequent
|
||||
requests loaded objects with PKs already present in the identity map,
|
||||
SQLAlchemy emitted SAWarning about identity map conflicts during flush.
|
||||
|
||||
Add DBManager.remove_session() and call it from two places:
|
||||
|
||||
1. ActionHandler.process_request() finally block — the primary cleanup
|
||||
point, runs after all action code completes (including work done
|
||||
after the response is sent via stop_action=False).
|
||||
|
||||
2. ActionHandler.on_finish() — guarded by _entered_process_request
|
||||
flag, only runs when prepare() returned early (e.g., auth/authz
|
||||
failure) without entering process_request(). Cannot be called
|
||||
unconditionally because on_finish() is triggered by finish(), which
|
||||
may fire mid-action when stop_action=False is used.
|
||||
|
||||
This also prevents unbounded memory growth from the identity map over
|
||||
the verifier's lifetime.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
---
|
||||
keylime/models/base/db.py | 9 +++
|
||||
keylime/web/base/action_handler.py | 115 +++++++++++++++++------------
|
||||
2 files changed, 78 insertions(+), 46 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/base/db.py b/keylime/models/base/db.py
|
||||
index c758fc1..9678098 100644
|
||||
--- a/keylime/models/base/db.py
|
||||
+++ b/keylime/models/base/db.py
|
||||
@@ -138,6 +138,15 @@ class DBManager:
|
||||
|
||||
return cast(Session, self._scoped_session())
|
||||
|
||||
+ def remove_session(self) -> None:
|
||||
+ """Remove the current scoped session, releasing its connection back to the pool and clearing the identity map.
|
||||
+
|
||||
+ Should be called at the end of each request to prevent stale objects from accumulating in the session across
|
||||
+ request boundaries.
|
||||
+ """
|
||||
+ if self._scoped_session:
|
||||
+ self._scoped_session.remove()
|
||||
+
|
||||
@contextmanager
|
||||
def session_context(self, session: Session | None = None) -> Iterator[Session]:
|
||||
if session:
|
||||
diff --git a/keylime/web/base/action_handler.py b/keylime/web/base/action_handler.py
|
||||
index 8410b40..d14c9ee 100644
|
||||
--- a/keylime/web/base/action_handler.py
|
||||
+++ b/keylime/web/base/action_handler.py
|
||||
@@ -9,6 +9,7 @@ from tornado.web import RequestHandler
|
||||
from keylime import keylime_logging
|
||||
from keylime.authorization.manager import get_authorization_manager
|
||||
from keylime.authorization.provider import Action, AuthorizationRequest
|
||||
+from keylime.models.base.db import db_manager
|
||||
from keylime.models.base.types import Timestamp # type: ignore[attr-defined]
|
||||
from keylime.models.verifier.auth_session import AuthSession
|
||||
from keylime.web.base.default_controller import DefaultController
|
||||
@@ -549,6 +550,7 @@ class ActionHandler(RequestHandler):
|
||||
self._action_call_stack: list[tuple["Controller", str]] = []
|
||||
self._received_at: int = time.time_ns()
|
||||
self._finished: bool = False
|
||||
+ self._entered_process_request: bool = False
|
||||
|
||||
async def prepare(self) -> None:
|
||||
# Tornado allows the prepare method to be overridden as async in subclasses of RequestHandler
|
||||
@@ -598,59 +600,80 @@ class ActionHandler(RequestHandler):
|
||||
return
|
||||
|
||||
async def process_request(self) -> None:
|
||||
+ self._entered_process_request = True # pylint: disable=attribute-defined-outside-init
|
||||
# If a route matches the request, invoke action determined by the matching route
|
||||
- if self.matching_route and self.controller:
|
||||
- try:
|
||||
- await self._invoke_action()
|
||||
- except StopAction:
|
||||
- # If the action is terminated early, continue
|
||||
- pass
|
||||
- except ParamDecodeError:
|
||||
- # If the query, form or JSON parameters are malformed, respond using error-handling action
|
||||
- await self._invoke_action("malformed_params", ignore_param_errors=True)
|
||||
- except ActionDispatchError:
|
||||
- # If the union of path, query, form and JSON parameters and do not match the method signature
|
||||
- # of the action, respond using error-handling action
|
||||
- await self._invoke_action("action_dispatch_error", ignore_param_errors=True)
|
||||
- except RequiredContentMissing:
|
||||
- # If a decorator from the Controller class has been used to mark a certain content format as required
|
||||
- # for the action and the request body or Content-Type do not adhere, respond using error-handling action
|
||||
- await self._invoke_action("format_not_allowed", ignore_param_errors=True)
|
||||
- except Exception as err:
|
||||
- # Any other exception which is not caught within the action body should be logged as an unexpected
|
||||
- # internal error before responding using error-handling action
|
||||
- self._log_exception(err)
|
||||
- await self._invoke_action("action_exception", ignore_param_errors=True)
|
||||
-
|
||||
- # Handle situation in which no invoked action produces a response
|
||||
- self._handle_incomplete_action()
|
||||
+ try:
|
||||
+ if self.matching_route and self.controller:
|
||||
+ try:
|
||||
+ await self._invoke_action()
|
||||
+ except StopAction:
|
||||
+ # If the action is terminated early, continue
|
||||
+ pass
|
||||
+ except ParamDecodeError:
|
||||
+ # If the query, form or JSON parameters are malformed, respond using error-handling action
|
||||
+ await self._invoke_action("malformed_params", ignore_param_errors=True)
|
||||
+ except ActionDispatchError:
|
||||
+ # If the union of path, query, form and JSON parameters and do not match the method signature
|
||||
+ # of the action, respond using error-handling action
|
||||
+ await self._invoke_action("action_dispatch_error", ignore_param_errors=True)
|
||||
+ except RequiredContentMissing:
|
||||
+ # If a decorator from the Controller class has been used to mark a certain content format as
|
||||
+ # required for the action and the request body or Content-Type do not adhere, respond using
|
||||
+ # error-handling action
|
||||
+ await self._invoke_action("format_not_allowed", ignore_param_errors=True)
|
||||
+ except Exception as err:
|
||||
+ # Any other exception which is not caught within the action body should be logged as an
|
||||
+ # unexpected internal error before responding using error-handling action
|
||||
+ self._log_exception(err)
|
||||
+ await self._invoke_action("action_exception", ignore_param_errors=True)
|
||||
+
|
||||
+ # Handle situation in which no invoked action produces a response
|
||||
+ self._handle_incomplete_action()
|
||||
+ finally:
|
||||
+ # Clean up the scoped session after all action code completes (including any work done after the
|
||||
+ # response is sent via stop_action=False). This prevents stale objects from accumulating in the
|
||||
+ # identity map across request boundaries. Must be here rather than on_finish(), because on_finish()
|
||||
+ # is called by Tornado's finish() when the response is sent, which may be before action code completes.
|
||||
+ db_manager.remove_session()
|
||||
|
||||
def write_error(self, status_code: int, **kwargs: Any) -> None:
|
||||
- if status_code == 405 and kwargs.get("exc_info"):
|
||||
- # Handle situation in which the HTTP method given in the request is not supported by the server (Tornado
|
||||
- # produces a 405 error by default in this case)
|
||||
-
|
||||
- # self.prepare() is not triggered in this case, so perform request reporting tasks
|
||||
- self._process_request_id()
|
||||
- logger.info("%s %s", self.request.method, self.request.path)
|
||||
- # Produce a response using the appropriate error-handling action
|
||||
- self._invoke_action_sync("unsupported_method", ignore_param_errors=True)
|
||||
-
|
||||
- elif kwargs.get("exc_info"):
|
||||
- # For any other exception produced by this class and not caught elsewhere, log the exception and invoke
|
||||
- # the appropriate error-handling action
|
||||
- _, err, _ = kwargs["exc_info"]
|
||||
- self._log_exception(err)
|
||||
- self._invoke_action_sync("handler_exception", ignore_param_errors=True)
|
||||
+ try:
|
||||
+ if status_code == 405 and kwargs.get("exc_info"):
|
||||
+ # Handle situation in which the HTTP method given in the request is not supported by the server
|
||||
+ # (Tornado produces a 405 error by default in this case)
|
||||
+
|
||||
+ # self.prepare() is not triggered in this case, so perform request reporting tasks
|
||||
+ self._process_request_id()
|
||||
+ logger.info("%s %s", self.request.method, self.request.path)
|
||||
+ # Produce a response using the appropriate error-handling action
|
||||
+ self._invoke_action_sync("unsupported_method", ignore_param_errors=True)
|
||||
+
|
||||
+ elif kwargs.get("exc_info"):
|
||||
+ # For any other exception produced by this class and not caught elsewhere, log the exception and
|
||||
+ # invoke the appropriate error-handling action
|
||||
+ _, err, _ = kwargs["exc_info"]
|
||||
+ self._log_exception(err)
|
||||
+ self._invoke_action_sync("handler_exception", ignore_param_errors=True)
|
||||
|
||||
- else:
|
||||
- # Catch-all for all other errors (typically those produced by calling Tornado's send_error method)
|
||||
- self.default_controller.send_response(status_code)
|
||||
+ else:
|
||||
+ # Catch-all for all other errors (typically those produced by calling Tornado's send_error method)
|
||||
+ self.default_controller.send_response(status_code)
|
||||
|
||||
- # Handle situation in which none of the above-invoked error-handling actions produce a response
|
||||
- self._handle_incomplete_action()
|
||||
+ # Handle situation in which none of the above-invoked error-handling actions produce a response
|
||||
+ self._handle_incomplete_action()
|
||||
+ finally:
|
||||
+ db_manager.remove_session()
|
||||
|
||||
def on_finish(self) -> None:
|
||||
+ # Clean up the scoped session only if process_request() was never
|
||||
+ # entered (e.g., prepare() returned early due to auth/authz failure).
|
||||
+ # When process_request() runs, its finally block handles cleanup —
|
||||
+ # calling remove_session() here would be premature because on_finish()
|
||||
+ # is triggered by finish() which may be called mid-action when
|
||||
+ # stop_action=False is used (the action continues after the response).
|
||||
+ if not self._entered_process_request:
|
||||
+ db_manager.remove_session()
|
||||
+
|
||||
message = f"Sent {self.get_status()} in {self.elapsed_time}"
|
||||
|
||||
if self.get_status() < 400:
|
||||
--
|
||||
2.53.0
|
||||
|
||||
108
0010-fix-Check-active-flag-in-_extract_identity-and-guard.patch
Normal file
108
0010-fix-Check-active-flag-in-_extract_identity-and-guard.patch
Normal file
@ -0,0 +1,108 @@
|
||||
From 08c0c67c492ef27df53fa9bff899597c46ae6fc8 Mon Sep 17 00:00:00 2001
|
||||
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
Date: Fri, 13 Mar 2026 13:59:23 +0100
|
||||
Subject: [PATCH] fix: Check active flag in _extract_identity and guard
|
||||
receive_pop
|
||||
|
||||
receive_pop() was unconditionally setting active=True and
|
||||
token_expires_at even when TPM verification failed. Use `any(errs for
|
||||
errs in self.errors.values())` to check for non-empty error lists,
|
||||
matching the pattern already used in session_controller.py.
|
||||
|
||||
This didn't affect the security because on failure the state was not
|
||||
persisted in the database. Now these are only set when no errors
|
||||
occurred.
|
||||
|
||||
_extract_identity() was not checking the session active flag, which
|
||||
could allow authentication with an inactive session if the state was
|
||||
persisted. Add the active check as defense-in-depth.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
---
|
||||
keylime/models/verifier/auth_session.py | 9 ++++---
|
||||
keylime/web/base/action_handler.py | 36 +++++++++++++++++++------
|
||||
2 files changed, 33 insertions(+), 12 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/verifier/auth_session.py b/keylime/models/verifier/auth_session.py
|
||||
index b0b40b0..fc5c8df 100644
|
||||
--- a/keylime/models/verifier/auth_session.py
|
||||
+++ b/keylime/models/verifier/auth_session.py
|
||||
@@ -630,10 +630,11 @@ class AuthSession(PersistableModel):
|
||||
logger.error("Unexpected error during TPM verification: %s: %s", type(e).__name__, e)
|
||||
self._add_error("verification", f"TPM verification failed: {str(e)}")
|
||||
|
||||
- # Set token expiration (only on successful validation)
|
||||
- session_lifetime = config.getint("verifier", "session_lifetime", fallback=config.DEFAULT_SESSION_LIFETIME)
|
||||
- self.token_expires_at = Timestamp.now() + timedelta(seconds=session_lifetime)
|
||||
- self.active = True
|
||||
+ # Set token expiration and activate only on successful validation
|
||||
+ if not any(errs for errs in self.errors.values()):
|
||||
+ session_lifetime = config.getint("verifier", "session_lifetime", fallback=config.DEFAULT_SESSION_LIFETIME)
|
||||
+ self.token_expires_at = Timestamp.now() + timedelta(seconds=session_lifetime)
|
||||
+ self.active = True
|
||||
|
||||
def _set_nonce(self):
|
||||
if "nonce" not in self.values:
|
||||
diff --git a/keylime/web/base/action_handler.py b/keylime/web/base/action_handler.py
|
||||
index d14c9ee..68dd30d 100644
|
||||
--- a/keylime/web/base/action_handler.py
|
||||
+++ b/keylime/web/base/action_handler.py
|
||||
@@ -265,12 +265,18 @@ class ActionHandler(RequestHandler):
|
||||
# Look up by token hash (tokens are never stored in plaintext)
|
||||
auth_session = AuthSession.get_by_token(token)
|
||||
if auth_session and auth_session.agent_id: # type: ignore[attr-defined]
|
||||
- # Check if token is still valid
|
||||
- now = Timestamp.now()
|
||||
- if auth_session.token_expires_at >= now: # type: ignore[attr-defined]
|
||||
- logger.debug("Extracted agent identity from bearer token: %s", auth_session.agent_id) # type: ignore[attr-defined]
|
||||
- return (auth_session.agent_id, "agent") # type: ignore[attr-defined]
|
||||
- logger.debug("Bearer token expired for agent: %s", auth_session.agent_id) # type: ignore[attr-defined]
|
||||
+ # Check if session is active and token is still valid
|
||||
+ if not getattr(auth_session, "active", False):
|
||||
+ logger.debug("Session not active for agent: %s", auth_session.agent_id) # type: ignore[attr-defined]
|
||||
+ else:
|
||||
+ token_expires_at = getattr(auth_session, "token_expires_at", None)
|
||||
+ if token_expires_at is None:
|
||||
+ logger.debug("Session has no expiry for agent: %s", auth_session.agent_id) # type: ignore[attr-defined]
|
||||
+ elif token_expires_at >= Timestamp.now():
|
||||
+ logger.debug("Extracted agent identity from bearer token: %s", auth_session.agent_id) # type: ignore[attr-defined]
|
||||
+ return (auth_session.agent_id, "agent") # type: ignore[attr-defined]
|
||||
+ else:
|
||||
+ logger.debug("Bearer token expired for agent: %s", auth_session.agent_id) # type: ignore[attr-defined]
|
||||
else:
|
||||
logger.debug("Invalid bearer token provided")
|
||||
else:
|
||||
@@ -520,13 +526,27 @@ class ActionHandler(RequestHandler):
|
||||
self.finish()
|
||||
return False
|
||||
|
||||
+ # Check if session is active
|
||||
+ if not getattr(auth_session, "active", False):
|
||||
+ logger.info(
|
||||
+ "Authentication session not active for agent '%s'",
|
||||
+ auth_session.agent_id, # type: ignore[attr-defined]
|
||||
+ )
|
||||
+ self.set_status(401)
|
||||
+ self.write(
|
||||
+ {"errors": [{"status": "401", "title": "Unauthorized", "detail": "Authentication session not active"}]}
|
||||
+ )
|
||||
+ self.finish()
|
||||
+ return False
|
||||
+
|
||||
# Check if token has expired
|
||||
+ token_expires_at = getattr(auth_session, "token_expires_at", None)
|
||||
now = Timestamp.now()
|
||||
- if auth_session.token_expires_at < now: # type: ignore[attr-defined]
|
||||
+ if token_expires_at is None or token_expires_at < now:
|
||||
logger.info(
|
||||
"Authentication token expired for agent '%s' (expired at %s)",
|
||||
auth_session.agent_id, # type: ignore[attr-defined]
|
||||
- auth_session.token_expires_at, # type: ignore[attr-defined]
|
||||
+ token_expires_at,
|
||||
)
|
||||
self.set_status(401)
|
||||
self.write(
|
||||
--
|
||||
2.53.0
|
||||
|
||||
92
0011-fix-Add-fork-safety-to-DBManager-via-dispose.patch
Normal file
92
0011-fix-Add-fork-safety-to-DBManager-via-dispose.patch
Normal file
@ -0,0 +1,92 @@
|
||||
From d74e7499746917fa7b9fbba02972eed82bc7ece9 Mon Sep 17 00:00:00 2001
|
||||
From: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
Date: Fri, 13 Mar 2026 16:04:45 +0100
|
||||
Subject: [PATCH] fix: Add fork-safety to DBManager via dispose()
|
||||
|
||||
After forking worker processes, child processes inherited the parent's
|
||||
db_manager engine and connection pool. Sharing SQLAlchemy connection
|
||||
pools across fork boundaries is unsafe and can lead to corruption.
|
||||
|
||||
Add DBManager.dispose() to clear engine, scoped session, and registry
|
||||
state. Call it in verifier_server.py after fork (alongside the existing
|
||||
reset_verifier_config()), then immediately re-create the engine with
|
||||
make_engine() so the child has its own fresh connection pool.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
||||
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
|
||||
---
|
||||
keylime/models/base/db.py | 22 +++++++++++++++++++---
|
||||
keylime/web/base/server.py | 10 ++++++++++
|
||||
2 files changed, 29 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/keylime/models/base/db.py b/keylime/models/base/db.py
|
||||
index 9678098..bb218cf 100644
|
||||
--- a/keylime/models/base/db.py
|
||||
+++ b/keylime/models/base/db.py
|
||||
@@ -101,9 +101,6 @@ class DBManager:
|
||||
|
||||
@property
|
||||
def service(self) -> Optional[str]:
|
||||
- if not self._service:
|
||||
- raise BackendMissing("cannot access the service for a DBManager before a call to db_manager.make_engine()")
|
||||
-
|
||||
return self._service
|
||||
|
||||
@property
|
||||
@@ -138,6 +135,25 @@ class DBManager:
|
||||
|
||||
return cast(Session, self._scoped_session())
|
||||
|
||||
+ def dispose(self) -> None:
|
||||
+ """Dispose the engine and clear all state.
|
||||
+
|
||||
+ Must be called after fork to avoid sharing the parent's connection pool
|
||||
+ across child processes. The next call to make_engine() will create fresh
|
||||
+ connections for the child process.
|
||||
+ """
|
||||
+ if self._scoped_session:
|
||||
+ self._scoped_session.remove()
|
||||
+ if self._engine:
|
||||
+ # Use close=False so the child discards the inherited pool
|
||||
+ # without closing the parent's underlying connections. Per
|
||||
+ # SQLAlchemy docs, this is the recommended approach after fork.
|
||||
+ self._engine.dispose(close=False) # type: ignore[call-arg]
|
||||
+ self._engine = None
|
||||
+ self._scoped_session = None
|
||||
+ self._registry = None
|
||||
+ self._service = None
|
||||
+
|
||||
def remove_session(self) -> None:
|
||||
"""Remove the current scoped session, releasing its connection back to the pool and clearing the identity map.
|
||||
|
||||
diff --git a/keylime/web/base/server.py b/keylime/web/base/server.py
|
||||
index e053bbb..913a498 100644
|
||||
--- a/keylime/web/base/server.py
|
||||
+++ b/keylime/web/base/server.py
|
||||
@@ -8,6 +8,7 @@ from typing import TYPE_CHECKING, Any, Callable, Optional
|
||||
import tornado
|
||||
|
||||
from keylime import api_version, config, keylime_logging, web_util
|
||||
+from keylime.models.base.db import db_manager
|
||||
from keylime.web.base.action_handler import ActionHandler
|
||||
from keylime.web.base.route import Route
|
||||
|
||||
@@ -299,6 +300,15 @@ class Server(ABC):
|
||||
tornado.process.fork_processes(self.worker_count)
|
||||
# num.value = num.value + 1
|
||||
# print(num.value)
|
||||
+
|
||||
+ # Dispose inherited db_manager engine after fork to avoid sharing the
|
||||
+ # parent's connection pool, then re-create with a fresh pool for this
|
||||
+ # child process.
|
||||
+ service = db_manager.service
|
||||
+ db_manager.dispose()
|
||||
+ if service:
|
||||
+ db_manager.make_engine(service)
|
||||
+
|
||||
asyncio.run(self.start_single())
|
||||
|
||||
def _setup(self) -> None:
|
||||
--
|
||||
2.53.0
|
||||
|
||||
11
keylime.spec
11
keylime.spec
@ -9,7 +9,7 @@
|
||||
|
||||
Name: keylime
|
||||
Version: 7.14.1
|
||||
Release: 1%{?dist}
|
||||
Release: 2%{?dist}
|
||||
Summary: Open source TPM software for Bootstrapping and Maintaining Trust
|
||||
|
||||
URL: https://github.com/keylime/keylime
|
||||
@ -21,6 +21,15 @@ Source3: %{srcname}.tmpfiles
|
||||
|
||||
Patch: 0001-Fix-timestamp-conversion-to-use-UTC-timezone.patch
|
||||
Patch: 0002-Fix-efivar-availability-check-in-test_create_mb_poli.patch
|
||||
Patch: 0003-Close-DB-sessions-to-prevent-connection-exhaustion.patch
|
||||
Patch: 0004-Include-thread-safe-session-management.patch
|
||||
Patch: 0005-Address-some-improvements-from-code-review.patch
|
||||
Patch: 0006-Fix-race-condition-on-in-SessionManager.patch
|
||||
Patch: 0007-Fix-linter-errors-in-PersistableModel.get-and-.all.patch
|
||||
Patch: 0008-refactor-Remove-dead-code-AuthSession.authenticate_a.patch
|
||||
Patch: 0009-db-Clean-up-scoped-session-after-each-request.patch
|
||||
Patch: 0010-fix-Check-active-flag-in-_extract_identity-and-guard.patch
|
||||
Patch: 0011-fix-Add-fork-safety-to-DBManager-via-dispose.patch
|
||||
|
||||
# Main program: Apache-2.0
|
||||
# Icons: MIT
|
||||
|
||||
Loading…
Reference in New Issue
Block a user