diff --git a/0013-Add-shared-memory-infrastructure-for-multiprocess-co.patch b/0013-Add-shared-memory-infrastructure-for-multiprocess-co.patch new file mode 100644 index 0000000..34bca9f --- /dev/null +++ b/0013-Add-shared-memory-infrastructure-for-multiprocess-co.patch @@ -0,0 +1,1107 @@ +From 1eaad216e290d5935f59e9137a233ac8516a8afb Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Tue, 9 Dec 2025 11:11:43 +0000 +Subject: [PATCH 13/14] Add shared memory infrastructure for multiprocess + communication + +Backport of upstream https://github.com/keylime/keylime/pull/1817/commits/1024e19d + +Signed-off-by: Sergio Correia +--- + keylime-selinux-42.1.2/keylime.te | 2 + + keylime/cloud_verifier_tornado.py | 89 ++--- + keylime/cmd/verifier.py | 6 + + keylime/config.py | 87 +++++ + keylime/shared_data.py | 513 +++++++++++++++++++++++++ + keylime/tpm/tpm_main.py | 17 +- + keylime/web/base/default_controller.py | 6 + + test/test_shared_data.py | 199 ++++++++++ + 8 files changed, 868 insertions(+), 51 deletions(-) + create mode 100644 keylime/shared_data.py + create mode 100644 test/test_shared_data.py + +diff --git a/keylime-selinux-42.1.2/keylime.te b/keylime-selinux-42.1.2/keylime.te +index 2c6a59e..8b8a615 100644 +--- a/keylime-selinux-42.1.2/keylime.te ++++ b/keylime-selinux-42.1.2/keylime.te +@@ -77,6 +77,8 @@ optional_policy(` + allow keylime_server_t self:key { create read setattr view write }; + allow keylime_server_t self:netlink_route_socket { create_stream_socket_perms nlmsg_read }; + allow keylime_server_t self:udp_socket create_stream_socket_perms; ++allow keylime_server_t keylime_tmp_t:sock_file { create write }; ++allow keylime_server_t self:unix_stream_socket connectto; + + fs_dontaudit_search_cgroup_dirs(keylime_server_t) + +diff --git a/keylime/cloud_verifier_tornado.py b/keylime/cloud_verifier_tornado.py +index 89aa703..67ba8af 100644 +--- a/keylime/cloud_verifier_tornado.py ++++ b/keylime/cloud_verifier_tornado.py +@@ -6,8 +6,8 @@ import signal + import sys + import traceback + from concurrent.futures import ThreadPoolExecutor +-from multiprocessing import Process + from contextlib import contextmanager ++from multiprocessing import Process + from typing import Any, Dict, Iterator, List, Optional, Tuple, Union, cast + + import tornado.httpserver +@@ -27,6 +27,7 @@ from keylime import ( + json, + keylime_logging, + revocation_notifier, ++ shared_data, + signing, + tornado_requests, + web_util, +@@ -43,7 +44,6 @@ from keylime.mba import mba + + logger = keylime_logging.init_logging("verifier") + +-GLOBAL_POLICY_CACHE: Dict[str, Dict[str, str]] = {} + + set_severity_config(config.getlist("verifier", "severity_labels"), config.getlist("verifier", "severity_policy")) + +@@ -140,44 +140,41 @@ def _from_db_obj(agent_db_obj: VerfierMain) -> Dict[str, Any]: + return agent_dict + + +-def verifier_read_policy_from_cache(ima_policy_data: Dict[str, str]) -> str: +- checksum = ima_policy_data.get("checksum", "") +- name = ima_policy_data.get("name", "empty") +- agent_id = ima_policy_data.get("agent_id", "") ++def verifier_read_policy_from_cache(stored_agent: VerfierMain) -> str: ++ checksum = "" ++ name = "empty" ++ agent_id = str(stored_agent.agent_id) + +- if not agent_id: +- return "" ++ # Initialize agent policy cache if it doesn't exist ++ shared_data.initialize_agent_policy_cache(agent_id) + +- if agent_id not in GLOBAL_POLICY_CACHE: +- GLOBAL_POLICY_CACHE[agent_id] = {} +- GLOBAL_POLICY_CACHE[agent_id][""] = "" ++ if stored_agent.ima_policy: ++ checksum = str(stored_agent.ima_policy.checksum) ++ name = stored_agent.ima_policy.name + +- if checksum not in GLOBAL_POLICY_CACHE[agent_id]: +- if len(GLOBAL_POLICY_CACHE[agent_id]) > 1: +- # Perform a cleanup of the contents, IMA policy checksum changed +- logger.debug( +- "Cleaning up policy cache for policy named %s, with checksum %s, used by agent %s", +- name, +- checksum, +- agent_id, +- ) ++ # Check if policy is already cached ++ cached_policy = shared_data.get_cached_policy(agent_id, checksum) ++ if cached_policy is not None: ++ return cached_policy + +- GLOBAL_POLICY_CACHE[agent_id] = {} +- GLOBAL_POLICY_CACHE[agent_id][""] = "" ++ # Policy not cached, need to clean up and load from database ++ shared_data.cleanup_agent_policy_cache(agent_id, checksum) + +- logger.debug( +- "IMA policy named %s, with checksum %s, used by agent %s is not present on policy cache on this verifier, performing SQLAlchemy load", +- name, +- checksum, +- agent_id, +- ) ++ logger.debug( ++ "IMA policy named %s, with checksum %s, used by agent %s is not present on policy cache on this verifier, performing SQLAlchemy load", ++ name, ++ checksum, ++ agent_id, ++ ) + +- # Get the large ima_policy content - it's already loaded in ima_policy_data +- ima_policy = ima_policy_data.get("ima_policy", "") +- assert isinstance(ima_policy, str) +- GLOBAL_POLICY_CACHE[agent_id][checksum] = ima_policy ++ # Actually contacts the database and load the (large) ima_policy column for "allowlists" table ++ ima_policy = stored_agent.ima_policy.ima_policy ++ assert isinstance(ima_policy, str) + +- return GLOBAL_POLICY_CACHE[agent_id][checksum] ++ # Cache the policy for future use ++ shared_data.cache_policy(agent_id, checksum, ima_policy) ++ ++ return ima_policy + + + def verifier_db_delete_agent(session: Session, agent_id: str) -> None: +@@ -475,12 +472,11 @@ class AgentsHandler(BaseHandler): + return + + # Cleanup the cache when the agent is deleted. Do it early. +- if agent_id in GLOBAL_POLICY_CACHE: +- del GLOBAL_POLICY_CACHE[agent_id] +- logger.debug( +- "Cleaned up policy cache from all entries used by agent %s", +- agent_id, +- ) ++ shared_data.clear_agent_policy_cache(agent_id) ++ logger.debug( ++ "Cleaned up policy cache from all entries used by agent %s", ++ agent_id, ++ ) + + op_state = agent.operational_state + if op_state in (states.SAVED, states.FAILED, states.TERMINATED, states.TENANT_FAILED, states.INVALID_QUOTE): +@@ -1763,7 +1759,6 @@ async def process_agent( + stored_agent = None + + # First database operation - read agent data and extract all needed data within session context +- ima_policy_data = {} + mb_policy_data = None + with session_context() as session: + try: +@@ -1779,15 +1774,6 @@ async def process_agent( + .first() + ) + +- # Extract IMA policy data within session context to avoid DetachedInstanceError +- if stored_agent and stored_agent.ima_policy: +- ima_policy_data = { +- "checksum": str(stored_agent.ima_policy.checksum), +- "name": stored_agent.ima_policy.name, +- "agent_id": str(stored_agent.agent_id), +- "ima_policy": stored_agent.ima_policy.ima_policy, # Extract the large content too +- } +- + # Extract MB policy data within session context + if stored_agent and stored_agent.mb_policy: + mb_policy_data = stored_agent.mb_policy.mb_policy +@@ -1869,7 +1855,10 @@ async def process_agent( + logger.error("SQLAlchemy Error for agent ID %s: %s", agent["agent_id"], e) + + # Load agent's IMA policy +- runtime_policy = verifier_read_policy_from_cache(ima_policy_data) ++ if stored_agent: ++ runtime_policy = verifier_read_policy_from_cache(stored_agent) ++ else: ++ runtime_policy = "" + + # Get agent's measured boot policy + mb_policy = mb_policy_data +diff --git a/keylime/cmd/verifier.py b/keylime/cmd/verifier.py +index f3e1a86..1f9f4e5 100644 +--- a/keylime/cmd/verifier.py ++++ b/keylime/cmd/verifier.py +@@ -1,6 +1,7 @@ + from keylime import cloud_verifier_tornado, config, keylime_logging + from keylime.common.migrations import apply + from keylime.mba import mba ++from keylime.shared_data import initialize_shared_memory + + logger = keylime_logging.init_logging("verifier") + +@@ -10,6 +11,11 @@ def main() -> None: + if config.has_option("verifier", "auto_migrate_db") and config.getboolean("verifier", "auto_migrate_db"): + apply("cloud_verifier") + ++ # Initialize shared memory BEFORE creating server instance ++ # This MUST happen before verifier instantiation and worker forking ++ logger.info("Initializing shared memory manager in main process before server creation") ++ initialize_shared_memory() ++ + # Explicitly load and initialize measured boot components + mba.load_imports() + cloud_verifier_tornado.main() +diff --git a/keylime/config.py b/keylime/config.py +index e7ac634..b5cd546 100644 +--- a/keylime/config.py ++++ b/keylime/config.py +@@ -114,6 +114,85 @@ if "KEYLIME_LOGGING_CONFIG" in os.environ: + _config: Optional[Dict[str, RawConfigParser]] = None + + ++def _check_file_permissions(component: str, file_path: str) -> bool: ++ """Check if a config file has correct permissions and is readable. ++ ++ Args: ++ component: The component name (e.g., 'verifier', 'agent') ++ file_path: Path to the config file ++ ++ Returns: ++ True if file is readable, False otherwise ++ """ ++ if not os.path.exists(file_path): ++ return False ++ ++ if not os.access(file_path, os.R_OK): ++ import grp # pylint: disable=import-outside-toplevel ++ import pwd # pylint: disable=import-outside-toplevel ++ import stat # pylint: disable=import-outside-toplevel ++ ++ try: ++ file_stat = os.stat(file_path) ++ owner = pwd.getpwuid(file_stat.st_uid).pw_name ++ group = grp.getgrgid(file_stat.st_gid).gr_name ++ mode = stat.filemode(file_stat.st_mode) ++ except Exception: ++ owner = group = mode = "unknown" ++ ++ base_logger.error( # pylint: disable=logging-not-lazy ++ "=" * 80 ++ + "\n" ++ + "CRITICAL CONFIG ERROR: Config file %s exists but is not readable!\n" ++ + "File permissions: %s (owner: %s, group: %s)\n" ++ + "The keylime_%s service needs read access to this file.\n" ++ + "Fix with: chown keylime:keylime %s && chmod 440 %s\n" ++ + "=" * 80, ++ file_path, ++ mode, ++ owner, ++ group, ++ component, ++ file_path, ++ file_path, ++ ) ++ return False ++ ++ return True ++ ++ ++def _validate_config_files(component: str, file_paths: List[str], files_read: List[str]) -> None: ++ """Validate that config files were successfully parsed. ++ ++ Args: ++ component: The component name (e.g., 'verifier', 'agent') ++ file_paths: List of file paths that were attempted to be read ++ files_read: List of files that ConfigParser successfully read ++ """ ++ for file_path in file_paths: ++ # Check file permissions first ++ if not _check_file_permissions(component, file_path): ++ continue ++ ++ if file_path not in files_read: ++ base_logger.error( # pylint: disable=logging-not-lazy ++ "=" * 80 ++ + "\n" ++ + "CRITICAL CONFIG ERROR: Config file %s exists but failed to parse!\n" ++ + "This usually indicates duplicate keys within the same file.\n" ++ + "Common issues:\n" ++ + " - Same option appears multiple times in the same [%s] section\n" ++ + " - Empty values (key = ) conflicting with defined values\n" ++ + " - Invalid INI file syntax\n" ++ + "Please check the file for duplicate entries.\n" ++ + "You can validate the file with: python3 -c \"import configparser; c = configparser.RawConfigParser(); print(c.read('%s'))\"\n" ++ + "=" * 80, ++ file_path, ++ component, ++ file_path, ++ ) ++ ++ + def get_config(component: str) -> RawConfigParser: + """Find the configuration file to use for the given component and apply the + overrides defined by configuration snippets. +@@ -216,6 +295,10 @@ def get_config(component: str) -> RawConfigParser: + + # Validate that at least one config file is present + config_file = _config[component].read(c) ++ ++ # Validate the config file was parsed successfully ++ _validate_config_files(component, [c], config_file) ++ + if config_file: + base_logger.info("Reading configuration from %s", config_file) + +@@ -230,6 +313,10 @@ def get_config(component: str) -> RawConfigParser: + [os.path.join(d, f) for f in os.listdir(d) if f and os.path.isfile(os.path.join(d, f))] + ) + applied_snippets = _config[component].read(snippets) ++ ++ # Validate all snippet files were parsed successfully ++ _validate_config_files(component, snippets, applied_snippets) ++ + if applied_snippets: + base_logger.info("Applied configuration snippets from %s", d) + +diff --git a/keylime/shared_data.py b/keylime/shared_data.py +new file mode 100644 +index 0000000..23a3d81 +--- /dev/null ++++ b/keylime/shared_data.py +@@ -0,0 +1,513 @@ ++"""Shared memory management for keylime multiprocess applications. ++ ++This module provides thread-safe shared data management between processes ++using multiprocessing.Manager(). ++""" ++ ++import atexit ++import multiprocessing as mp ++import threading ++import time ++from typing import Any, Dict, List, Optional ++ ++from keylime import keylime_logging ++ ++logger = keylime_logging.init_logging("shared_data") ++ ++ ++class FlatDictView: ++ """A dictionary-like view over a flat key-value store. ++ ++ This class provides dict-like access to a subset of keys in a flat store, ++ identified by a namespace prefix. This avoids the nested DictProxy issues. ++ ++ Example: ++ store = manager.dict() # Flat store ++ view = FlatDictView(store, lock, "sessions") ++ view["123"] = "data" # Stores as "dict:sessions:123" in flat store ++ val = view["123"] # Retrieves from "dict:sessions:123" ++ """ ++ ++ def __init__(self, store: Any, lock: Any, namespace: str) -> None: ++ self._store = store ++ self._lock = lock ++ self._namespace = namespace ++ ++ def _make_key(self, key: Any) -> str: ++ """Convert user key to internal flat key with namespace prefix.""" ++ return f"dict:{self._namespace}:{key}" ++ ++ def __getitem__(self, key: Any) -> Any: ++ with self._lock: ++ return self._store[self._make_key(key)] ++ ++ def __setitem__(self, key: Any, value: Any) -> None: ++ flat_key = self._make_key(key) ++ with self._lock: ++ self._store[flat_key] = value ++ ++ def __delitem__(self, key: Any) -> None: ++ flat_key = self._make_key(key) ++ with self._lock: ++ del self._store[flat_key] ++ ++ def __contains__(self, key: Any) -> bool: ++ return self._make_key(key) in self._store ++ ++ def get(self, key: Any, default: Any = None) -> Any: ++ with self._lock: ++ return self._store.get(self._make_key(key), default) ++ ++ def keys(self) -> List[Any]: ++ """Return keys in this namespace.""" ++ prefix = f"dict:{self._namespace}:" ++ all_store_keys = list(self._store.keys()) ++ matching_keys = [k[len(prefix) :] for k in all_store_keys if k.startswith(prefix)] ++ return matching_keys ++ ++ def values(self) -> List[Any]: ++ """Return values in this namespace.""" ++ prefix = f"dict:{self._namespace}:" ++ with self._lock: ++ return [v for k, v in self._store.items() if k.startswith(prefix)] ++ ++ def items(self) -> List[tuple[Any, Any]]: ++ """Return (key, value) pairs in this namespace.""" ++ prefix = f"dict:{self._namespace}:" ++ with self._lock: ++ result = [(k[len(prefix) :], v) for k, v in self._store.items() if k.startswith(prefix)] ++ return result ++ ++ def __len__(self) -> int: ++ """Return number of items in this namespace.""" ++ return len(self.keys()) ++ ++ def __repr__(self) -> str: ++ return f"FlatDictView({self._namespace}, {len(self)} items)" ++ ++ ++class SharedDataManager: ++ """Thread-safe shared data manager for multiprocess applications. ++ ++ This class uses multiprocessing.Manager() to create proxy objects that can ++ be safely accessed from multiple processes. All data stored must be pickleable. ++ ++ Example: ++ manager = SharedDataManager() ++ ++ # Store simple data ++ manager.set_data("config_value", "some_config") ++ value = manager.get_data("config_value") ++ ++ # Work with shared dictionaries ++ agent_cache = manager.get_or_create_dict("agent_cache") ++ agent_cache["agent_123"] = {"last_seen": time.time()} ++ ++ # Work with shared lists ++ event_log = manager.get_or_create_list("events") ++ event_log.append({"type": "attestation", "agent": "agent_123"}) ++ """ ++ ++ def __init__(self) -> None: ++ """Initialize the shared data manager. ++ ++ This must be called before any process forking occurs to ensure ++ all child processes inherit access to the shared data. ++ """ ++ logger.debug("Initializing SharedDataManager") ++ ++ # Use explicit context to ensure fork compatibility ++ # The Manager must be started BEFORE any fork() calls ++ ctx = mp.get_context("fork") ++ self._manager = ctx.Manager() ++ ++ # CRITICAL FIX: Use a SINGLE flat dict instead of nested dicts ++ # Nested DictProxy objects have synchronization issues ++ # We'll use key prefixes like "dict:auth_sessions:session_id" instead ++ self._store = self._manager.dict() # Single flat store for all data ++ self._lock = self._manager.Lock() ++ self._initialized_at = time.time() ++ ++ # Register handler to reinitialize manager connection after fork ++ # This is needed because Manager uses network connections that don't survive fork ++ try: ++ import os # pylint: disable=import-outside-toplevel ++ ++ self._parent_pid = os.getpid() ++ logger.debug("SharedDataManager initialized in process %d", self._parent_pid) ++ except Exception as e: ++ logger.warning("Could not register PID tracking: %s", e) ++ ++ # Ensure cleanup on exit ++ atexit.register(self.cleanup) ++ ++ logger.info("SharedDataManager initialized successfully") ++ ++ def set_data(self, key: str, value: Any) -> None: ++ """Store arbitrary pickleable data by key. ++ ++ Args: ++ key: Unique identifier for the data ++ value: Any pickleable Python object ++ ++ Raises: ++ TypeError: If value is not pickleable ++ """ ++ with self._lock: ++ try: ++ self._store[key] = value ++ logger.debug("Stored data for key: %s", key) ++ except Exception as e: ++ logger.error("Failed to store data for key '%s': %s", key, e) ++ raise ++ ++ def get_data(self, key: str, default: Any = None) -> Any: ++ """Retrieve data by key. ++ ++ Args: ++ key: The key to retrieve ++ default: Value to return if key doesn't exist ++ ++ Returns: ++ The stored value or default if key doesn't exist ++ """ ++ with self._lock: ++ value = self._store.get(key, default) ++ logger.debug("Retrieved data for key: %s (found: %s)", key, value is not default) ++ return value ++ ++ def get_or_create_dict(self, key: str) -> Dict[str, Any]: ++ """Get or create a shared dictionary. ++ ++ Args: ++ key: Unique identifier for the dictionary ++ ++ Returns: ++ A shared dictionary-like object that syncs across processes ++ ++ Note: ++ Returns a FlatDictView that uses key prefixes in the flat store ++ instead of actual nested dicts, to avoid DictProxy nesting issues. ++ """ ++ # Mark that this namespace exists ++ namespace_key = f"__namespace__{key}" ++ if namespace_key not in self._store: ++ with self._lock: ++ self._store[namespace_key] = True ++ ++ # Return a view that operates on the flat store with key prefix ++ return FlatDictView(self._store, self._lock, key) # type: ignore[return-value,no-untyped-call] ++ ++ def get_or_create_list(self, key: str) -> List[Any]: ++ """Get or create a shared list. ++ ++ Args: ++ key: Unique identifier for the list ++ ++ Returns: ++ A shared list (proxy object) that syncs across processes ++ """ ++ with self._lock: ++ if key not in self._store: ++ self._store[key] = self._manager.list() ++ logger.debug("Created new shared list for key: %s", key) ++ else: ++ logger.debug("Retrieved existing shared list for key: %s", key) ++ return self._store[key] # type: ignore[no-any-return] ++ ++ def delete_data(self, key: str) -> bool: ++ """Delete data by key. ++ ++ Args: ++ key: The key to delete ++ ++ Returns: ++ True if the key existed and was deleted, False otherwise ++ """ ++ with self._lock: ++ if key in self._store: ++ del self._store[key] ++ logger.debug("Deleted data for key: %s", key) ++ return True ++ logger.debug("Key not found for deletion: %s", key) ++ return False ++ ++ def has_key(self, key: str) -> bool: ++ """Check if a key exists. ++ ++ Args: ++ key: The key to check ++ ++ Returns: ++ True if key exists, False otherwise ++ """ ++ with self._lock: ++ return key in self._store ++ ++ def get_keys(self) -> List[str]: ++ """Get all stored keys. ++ ++ Returns: ++ List of all keys in the store ++ """ ++ with self._lock: ++ return list(self._store.keys()) ++ ++ def clear_all(self) -> None: ++ """Clear all stored data. Use with caution!""" ++ with self._lock: ++ key_count = len(self._store) ++ self._store.clear() ++ logger.warning("Cleared all shared data (%d keys)", key_count) ++ ++ def get_stats(self) -> Dict[str, Any]: ++ """Get statistics about stored data. ++ ++ Returns: ++ Dictionary containing storage statistics ++ """ ++ with self._lock: ++ return { ++ "total_keys": len(self._store), ++ "initialized_at": self._initialized_at, ++ "uptime_seconds": time.time() - self._initialized_at, ++ } ++ ++ def cleanup(self) -> None: ++ """Cleanup shared resources. ++ ++ This is automatically called on exit but can be called manually ++ for explicit cleanup. ++ """ ++ if hasattr(self, "_manager"): ++ logger.debug("Shutting down SharedDataManager") ++ try: ++ self._manager.shutdown() ++ logger.info("SharedDataManager shutdown complete") ++ except Exception as e: ++ logger.error("Error during SharedDataManager shutdown: %s", e) ++ ++ def __repr__(self) -> str: ++ stats = self.get_stats() ++ return f"SharedDataManager(keys={stats['total_keys']}, " f"uptime={stats['uptime_seconds']:.1f}s)" ++ ++ @property ++ def manager(self) -> Any: # type: ignore[misc] ++ """Access to the underlying multiprocessing Manager for advanced usage.""" ++ return self._manager ++ ++ ++# Global shared memory manager instance ++_global_shared_manager: Optional[SharedDataManager] = None ++_manager_lock = threading.Lock() ++ ++ ++def initialize_shared_memory() -> SharedDataManager: ++ """Initialize the global shared memory manager. ++ ++ This function MUST be called before any process forking occurs to ensure ++ all child processes share the same manager instance. ++ ++ For tornado/multiprocess servers, call this before starting workers. ++ ++ Returns: ++ SharedDataManager: The global shared memory manager instance ++ ++ Raises: ++ RuntimeError: If called after manager is already initialized ++ """ ++ global _global_shared_manager ++ ++ with _manager_lock: ++ if _global_shared_manager is not None: ++ logger.warning("Shared memory manager already initialized, returning existing instance") ++ return _global_shared_manager ++ ++ logger.info("Initializing global shared memory manager") ++ _global_shared_manager = SharedDataManager() ++ logger.info("Global shared memory manager initialized") ++ ++ return _global_shared_manager ++ ++ ++def get_shared_memory() -> SharedDataManager: ++ """Get the global shared memory manager instance. ++ ++ This function returns a singleton SharedDataManager that can be used ++ throughout keylime for caching and inter-process communication. ++ ++ The manager is automatically initialized on first access and cleaned up ++ on process exit. ++ ++ IMPORTANT: In multiprocess applications (like tornado with workers), ++ you MUST call initialize_shared_memory() BEFORE forking workers. ++ Otherwise each worker will get its own separate manager. ++ ++ Returns: ++ SharedDataManager: The global shared memory manager instance ++ """ ++ global _global_shared_manager ++ ++ if _global_shared_manager is None: ++ with _manager_lock: ++ if _global_shared_manager is None: ++ logger.info("Initializing global shared memory manager") ++ _global_shared_manager = SharedDataManager() # type: ignore[no-untyped-call] ++ logger.info("Global shared memory manager initialized") ++ ++ return _global_shared_manager ++ ++ ++def cleanup_global_shared_memory() -> None: ++ """Cleanup the global shared memory manager. ++ ++ This is automatically called on exit but can be called manually. ++ """ ++ global _global_shared_manager ++ ++ if _global_shared_manager is not None: ++ logger.info("Cleaning up global shared memory manager") ++ _global_shared_manager.cleanup() ++ _global_shared_manager = None ++ ++ ++# Convenience functions for common keylime patterns ++ ++ ++def cache_policy(agent_id: str, checksum: str, policy: str) -> None: ++ """Cache a policy in shared memory. ++ ++ Args: ++ agent_id: The agent identifier ++ checksum: The policy checksum ++ policy: The policy content to cache ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id not in policy_cache: ++ policy_cache[agent_id] = manager.manager.dict() # type: ignore[attr-defined] ++ ++ policy_cache[agent_id][checksum] = policy ++ logger.debug("Cached policy for agent %s with checksum %s", agent_id, checksum) ++ ++ ++def get_cached_policy(agent_id: str, checksum: str) -> Optional[str]: ++ """Retrieve cached policy. ++ ++ Args: ++ agent_id: The agent identifier ++ checksum: The policy checksum ++ ++ Returns: ++ The cached policy content or None if not found ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ agent_policies = policy_cache.get(agent_id, {}) ++ ++ result = agent_policies.get(checksum) ++ if result: ++ logger.debug("Found cached policy for agent %s with checksum %s", agent_id, checksum) ++ else: ++ logger.debug("No cached policy found for agent %s with checksum %s", agent_id, checksum) ++ ++ return result # type: ignore[no-any-return] ++ ++ ++def clear_agent_policy_cache(agent_id: str) -> None: ++ """Clear all cached policies for an agent. ++ ++ Args: ++ agent_id: The agent identifier ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id in policy_cache: ++ del policy_cache[agent_id] ++ logger.debug("Cleared policy cache for agent %s", agent_id) ++ ++ ++def cleanup_agent_policy_cache(agent_id: str, keep_checksum: str = "") -> None: ++ """Clean up agent policy cache, keeping only the specified checksum. ++ ++ This mimics the cleanup behavior from GLOBAL_POLICY_CACHE where when ++ a new policy checksum is encountered, old cached policies are removed. ++ ++ Args: ++ agent_id: The agent identifier ++ keep_checksum: The checksum to keep in the cache (empty string by default) ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id in policy_cache and len(policy_cache[agent_id]) > 1: ++ # Keep only the empty entry and the specified checksum ++ old_policies = dict(policy_cache[agent_id]) ++ policy_cache[agent_id] = manager.manager.dict() ++ ++ # Always keep the empty entry ++ policy_cache[agent_id][""] = old_policies.get("", "") ++ ++ # Keep the specified checksum if it exists and is not empty ++ if keep_checksum and keep_checksum in old_policies: ++ policy_cache[agent_id][keep_checksum] = old_policies[keep_checksum] ++ ++ logger.debug("Cleaned up policy cache for agent %s, keeping checksum %s", agent_id, keep_checksum) ++ ++ ++def initialize_agent_policy_cache(agent_id: str) -> Dict[str, Any]: ++ """Initialize policy cache for an agent if it doesn't exist. ++ ++ Args: ++ agent_id: The agent identifier ++ ++ Returns: ++ The agent's policy cache dictionary ++ """ ++ manager = get_shared_memory() ++ policy_cache = manager.get_or_create_dict("policy_cache") ++ ++ if agent_id not in policy_cache: ++ policy_cache[agent_id] = manager.manager.dict() # type: ignore[attr-defined] ++ policy_cache[agent_id][""] = "" ++ logger.debug("Initialized policy cache for agent %s", agent_id) ++ ++ return policy_cache[agent_id] # type: ignore[no-any-return] ++ ++ ++def get_agent_cache(agent_id: str) -> Dict[str, Any]: ++ """Get shared cache for a specific agent. ++ ++ Args: ++ agent_id: The agent identifier ++ ++ Returns: ++ A shared dictionary for caching agent-specific data ++ """ ++ manager = get_shared_memory() ++ return manager.get_or_create_dict(f"agent_cache:{agent_id}") ++ ++ ++def get_verification_queue(agent_id: str) -> List[Any]: ++ """Get verification queue for batching database operations. ++ ++ Args: ++ agent_id: The agent identifier ++ ++ Returns: ++ A shared list for queuing verification operations ++ """ ++ manager = get_shared_memory() ++ return manager.get_or_create_list(f"verification_queue:{agent_id}") ++ ++ ++def get_shared_stats() -> Dict[str, Any]: ++ """Get statistics about shared memory usage. ++ ++ Returns: ++ Dictionary containing storage statistics ++ """ ++ manager = get_shared_memory() ++ return manager.get_stats() +diff --git a/keylime/tpm/tpm_main.py b/keylime/tpm/tpm_main.py +index 6f2e89f..9b54fc3 100644 +--- a/keylime/tpm/tpm_main.py ++++ b/keylime/tpm/tpm_main.py +@@ -10,7 +10,7 @@ from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey + + from keylime import cert_utils, config, json, keylime_logging + from keylime.agentstates import AgentAttestState, TPMClockInfo +-from keylime.common.algorithms import Hash ++from keylime.common.algorithms import Hash, Sign + from keylime.failure import Component, Failure + from keylime.ima import ima + from keylime.ima.file_signatures import ImaKeyrings +@@ -50,6 +50,21 @@ class Tpm: + + return (keyblob, key) + ++ # Mapping from keylime.common.algorithms enums to TPM algorithm constants ++ # Used for validating that TPM attestations use expected cryptographic algorithms ++ HASH_ALG_TO_TPM = { ++ Hash.SHA1: tpm2_objects.TPM_ALG_SHA1, ++ Hash.SHA256: tpm2_objects.TPM_ALG_SHA256, ++ Hash.SHA384: tpm2_objects.TPM_ALG_SHA384, ++ Hash.SHA512: tpm2_objects.TPM_ALG_SHA512, ++ } ++ ++ SIGN_ALG_TO_TPM = { ++ Sign.RSASSA: tpm2_objects.TPM_ALG_RSASSA, ++ Sign.RSAPSS: tpm2_objects.TPM_ALG_RSAPSS, ++ Sign.ECDSA: tpm2_objects.TPM_ALG_ECDSA, ++ } ++ + @staticmethod + def verify_aik_with_iak(uuid: str, aik_tpm: bytes, iak_tpm: bytes, iak_attest: bytes, iak_sign: bytes) -> bool: + attest_body = iak_attest.split(b"\x00$")[1] +diff --git a/keylime/web/base/default_controller.py b/keylime/web/base/default_controller.py +index 971ed06..ba0782e 100644 +--- a/keylime/web/base/default_controller.py ++++ b/keylime/web/base/default_controller.py +@@ -19,6 +19,12 @@ class DefaultController(Controller): + self.send_response(400, "Bad Request") + + def malformed_params(self, **_params: Any) -> None: ++ import traceback # pylint: disable=import-outside-toplevel ++ ++ from keylime import keylime_logging # pylint: disable=import-outside-toplevel ++ ++ logger = keylime_logging.init_logging("web") ++ logger.error("Malformed params error. Traceback: %s", traceback.format_exc()) + self.send_response(400, "Malformed Request Parameter") + + def action_dispatch_error(self, **_param: Any) -> None: +diff --git a/test/test_shared_data.py b/test/test_shared_data.py +new file mode 100644 +index 0000000..8de7e64 +--- /dev/null ++++ b/test/test_shared_data.py +@@ -0,0 +1,199 @@ ++"""Unit tests for shared memory infrastructure.""" ++ ++import unittest ++ ++from keylime.shared_data import ( ++ SharedDataManager, ++ cache_policy, ++ cleanup_agent_policy_cache, ++ cleanup_global_shared_memory, ++ clear_agent_policy_cache, ++ get_cached_policy, ++ get_shared_memory, ++ initialize_agent_policy_cache, ++) ++ ++ ++class TestSharedDataManager(unittest.TestCase): ++ """Test cases for SharedDataManager class.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ self.manager = SharedDataManager() ++ ++ def tearDown(self): ++ """Clean up after tests.""" ++ if self.manager: ++ self.manager.cleanup() ++ ++ def test_set_and_get_data(self): ++ """Test basic set and get operations.""" ++ self.manager.set_data("test_key", "test_value") ++ result = self.manager.get_data("test_key") ++ self.assertEqual(result, "test_value") ++ ++ def test_get_nonexistent_data(self): ++ """Test getting data that doesn't exist returns None.""" ++ result = self.manager.get_data("nonexistent_key") ++ self.assertIsNone(result) ++ ++ def test_get_data_with_default(self): ++ """Test getting data with default value.""" ++ result = self.manager.get_data("nonexistent_key", default="default_value") ++ self.assertEqual(result, "default_value") ++ ++ def test_delete_data(self): ++ """Test deleting data.""" ++ self.manager.set_data("test_key", "test_value") ++ result = self.manager.delete_data("test_key") ++ self.assertTrue(result) ++ ++ # Verify it's actually deleted ++ self.assertIsNone(self.manager.get_data("test_key")) ++ ++ def test_delete_nonexistent_data(self): ++ """Test deleting data that doesn't exist returns False.""" ++ result = self.manager.delete_data("nonexistent_key") ++ self.assertFalse(result) ++ ++ def test_has_key(self): ++ """Test checking if key exists.""" ++ self.manager.set_data("test_key", "test_value") ++ self.assertTrue(self.manager.has_key("test_key")) ++ self.assertFalse(self.manager.has_key("nonexistent_key")) ++ ++ def test_get_or_create_dict(self): ++ """Test getting or creating a shared dictionary.""" ++ shared_dict = self.manager.get_or_create_dict("test_dict") ++ shared_dict["key1"] = "value1" ++ shared_dict["key2"] = "value2" ++ ++ # Retrieve the same dict ++ retrieved_dict = self.manager.get_or_create_dict("test_dict") ++ self.assertEqual(retrieved_dict["key1"], "value1") ++ self.assertEqual(retrieved_dict["key2"], "value2") ++ ++ def test_get_or_create_list(self): ++ """Test getting or creating a shared list.""" ++ shared_list = self.manager.get_or_create_list("test_list") ++ shared_list.append("item1") ++ shared_list.append("item2") ++ ++ # Retrieve the same list ++ retrieved_list = self.manager.get_or_create_list("test_list") ++ self.assertEqual(len(retrieved_list), 2) ++ self.assertEqual(retrieved_list[0], "item1") ++ self.assertEqual(retrieved_list[1], "item2") ++ ++ def test_get_stats(self): ++ """Test getting manager statistics.""" ++ self.manager.set_data("key1", "value1") ++ self.manager.set_data("key2", "value2") ++ ++ stats = self.manager.get_stats() ++ self.assertIn("total_keys", stats) ++ self.assertIn("uptime_seconds", stats) ++ self.assertEqual(stats["total_keys"], 2) ++ self.assertGreaterEqual(stats["uptime_seconds"], 0) ++ ++ ++class TestPolicyCacheFunctions(unittest.TestCase): ++ """Test cases for policy cache functions.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ # Get the global shared memory manager ++ self.manager = get_shared_memory() ++ ++ def tearDown(self): ++ """Clean up after tests.""" ++ # Clean up global shared memory ++ cleanup_global_shared_memory() ++ ++ def test_initialize_agent_policy_cache(self): ++ """Test initializing agent policy cache.""" ++ agent_id = "test_agent_123" ++ initialize_agent_policy_cache(agent_id) ++ ++ # Verify the cache was initialized ++ policy_cache = self.manager.get_or_create_dict("policy_cache") ++ self.assertIn(agent_id, policy_cache) ++ ++ def test_cache_and_get_policy(self): ++ """Test caching and retrieving a policy.""" ++ agent_id = "test_agent_123" ++ checksum = "abc123def456" ++ policy_content = '{"policy": "test_policy_content"}' ++ ++ # Initialize and cache policy ++ initialize_agent_policy_cache(agent_id) ++ cache_policy(agent_id, checksum, policy_content) ++ ++ # Retrieve cached policy ++ cached = get_cached_policy(agent_id, checksum) ++ self.assertEqual(cached, policy_content) ++ ++ def test_get_nonexistent_cached_policy(self): ++ """Test getting a policy that hasn't been cached.""" ++ agent_id = "test_agent_123" ++ checksum = "nonexistent_checksum" ++ ++ initialize_agent_policy_cache(agent_id) ++ cached = get_cached_policy(agent_id, checksum) ++ self.assertIsNone(cached) ++ ++ def test_clear_agent_policy_cache(self): ++ """Test clearing an agent's policy cache.""" ++ agent_id = "test_agent_123" ++ checksum = "abc123def456" ++ policy_content = '{"policy": "test_policy_content"}' ++ ++ # Initialize, cache, and then clear ++ initialize_agent_policy_cache(agent_id) ++ cache_policy(agent_id, checksum, policy_content) ++ clear_agent_policy_cache(agent_id) ++ ++ # Verify it's cleared ++ cached = get_cached_policy(agent_id, checksum) ++ self.assertIsNone(cached) ++ ++ def test_cleanup_agent_policy_cache(self): ++ """Test cleaning up old policy checksums.""" ++ agent_id = "test_agent_123" ++ old_checksum = "old_checksum" ++ new_checksum = "new_checksum" ++ policy_content = '{"policy": "test"}' ++ ++ # Initialize and cache multiple policies ++ initialize_agent_policy_cache(agent_id) ++ cache_policy(agent_id, old_checksum, policy_content) ++ cache_policy(agent_id, new_checksum, policy_content) ++ ++ # Cleanup old checksums (keeping only new_checksum) ++ cleanup_agent_policy_cache(agent_id, new_checksum) ++ ++ # Verify old checksum is removed but new one remains ++ self.assertIsNone(get_cached_policy(agent_id, old_checksum)) ++ self.assertEqual(get_cached_policy(agent_id, new_checksum), policy_content) ++ ++ def test_cache_multiple_agents(self): ++ """Test caching policies for multiple agents.""" ++ agent1 = "agent_1" ++ agent2 = "agent_2" ++ checksum = "same_checksum" ++ policy1 = '{"policy": "agent1_policy"}' ++ policy2 = '{"policy": "agent2_policy"}' ++ ++ # Cache policies for different agents ++ initialize_agent_policy_cache(agent1) ++ initialize_agent_policy_cache(agent2) ++ cache_policy(agent1, checksum, policy1) ++ cache_policy(agent2, checksum, policy2) ++ ++ # Verify each agent has its own policy ++ self.assertEqual(get_cached_policy(agent1, checksum), policy1) ++ self.assertEqual(get_cached_policy(agent2, checksum), policy2) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +-- +2.47.3 + diff --git a/0014-Fix-registrar-duplicate-UUID-vulnerability.patch b/0014-Fix-registrar-duplicate-UUID-vulnerability.patch new file mode 100644 index 0000000..1fe3f80 --- /dev/null +++ b/0014-Fix-registrar-duplicate-UUID-vulnerability.patch @@ -0,0 +1,1188 @@ +From 2da614d212f58071f54c883ee368ffac4bc5e6b4 Mon Sep 17 00:00:00 2001 +From: Sergio Correia +Date: Tue, 9 Dec 2025 12:12:22 +0000 +Subject: [PATCH 14/14] Fix registrar duplicate UUID vulnerability + +Backport upstream PR#1825 + +Signed-off-by: Sergio Correia +--- + keylime/cmd/registrar.py | 6 + + keylime/models/registrar/registrar_agent.py | 116 +++++ + keylime/shared_data.py | 6 + + keylime/web/registrar/agents_controller.py | 98 +++- + test/test_agents_controller.py | 513 ++++++++++++++++++++ + test/test_registrar_tpm_identity.py | 342 +++++++++++++ + 6 files changed, 1071 insertions(+), 10 deletions(-) + create mode 100644 test/test_agents_controller.py + create mode 100644 test/test_registrar_tpm_identity.py + +diff --git a/keylime/cmd/registrar.py b/keylime/cmd/registrar.py +index 584275a..2e2b25e 100644 +--- a/keylime/cmd/registrar.py ++++ b/keylime/cmd/registrar.py +@@ -5,6 +5,7 @@ import cryptography + from keylime import config, keylime_logging + from keylime.common.migrations import apply + from keylime.models import da_manager, db_manager ++from keylime.shared_data import initialize_shared_memory + from keylime.web import RegistrarServer + + logger = keylime_logging.init_logging("registrar") +@@ -47,6 +48,11 @@ def main() -> None: + # Prepare backend for durable attestation, if configured + da_manager.make_backend("registrar") + ++ # Initialize shared memory for cross-process synchronization ++ # CRITICAL: Must be called before server.start_multi() to ensure all forked ++ # worker processes share the same manager instance for agent registration locks ++ initialize_shared_memory() ++ + # Start HTTP server + server = RegistrarServer() + server.start_multi() +diff --git a/keylime/models/registrar/registrar_agent.py b/keylime/models/registrar/registrar_agent.py +index fc7e1be..e26ae41 100644 +--- a/keylime/models/registrar/registrar_agent.py ++++ b/keylime/models/registrar/registrar_agent.py +@@ -65,8 +65,14 @@ class RegistrarAgent(PersistableModel): + def empty(cls): + agent = super().empty() + agent.provider_keys = {} ++ object.__setattr__(agent, "_tpm_identity_violation", False) + return agent + ++ @property ++ def has_tpm_identity_violation(self): ++ """Returns True if a TPM identity violation was detected during validation.""" ++ return getattr(self, "_tpm_identity_violation", False) ++ + def _check_key_against_cert(self, tpm_key_field, cert_field): + # If neither key nor certificate is being updated, no need to check + if tpm_key_field not in self.changes and cert_field not in self.changes: +@@ -139,6 +145,111 @@ class RegistrarAgent(PersistableModel): + + return compliant + ++ def _check_tpm_identity_immutable(self): ++ """ ++ Checks that TPM identity fields are not being changed during re-registration. ++ ++ This prevents an attacker from registering with the same UUID but a different TPM, ++ which would allow them to impersonate the original agent and bypass attestation. ++ ++ Checked fields (EK-based identity only): ++ - ek_tpm: Endorsement Key (primary TPM identity) ++ - ekcert: EK Certificate (binds EK to TPM manufacturer) ++ - aik_tpm: Attestation Key (bound to EK via MakeCredential/ActivateCredential) ++ ++ Note: IAK/IDevID fields are NOT checked and can change on re-registration. ++ ++ This check only applies to existing agents (those loaded from the database). ++ New agents created via RegistrarAgent.empty() have no committed values and are ++ allowed to set identity fields during initial registration. ++ ++ If the agent needs to be registered with a new TPM (e.g., hardware replacement), ++ the old agent record must be explicitly deleted first. ++ """ ++ # Define TPM identity fields that must remain immutable once set ++ # Only checking EK-based identity (ek_tpm, ekcert, aik_tpm) ++ # IAK/IDevID fields (iak_tpm, iak_cert, idevid_tpm, idevid_cert) are not checked ++ identity_fields = ["ek_tpm", "ekcert", "aik_tpm"] ++ ++ # Only check for existing agents (those loaded from database) ++ # New agents created via empty() will have no committed values ++ if not self.committed: ++ return ++ ++ # Track which fields have been changed ++ changed_fields = [] ++ ++ for field_name in identity_fields: ++ # Skip fields that are not being changed in this update ++ if field_name not in self.changes: ++ continue ++ ++ # Get the old (committed/database) and new (proposed) values ++ old_value = self.committed.get(field_name) ++ new_value = self.changes.get(field_name) ++ ++ # Allow setting a previously unset field (e.g., adding EK cert later) ++ if old_value is None: ++ continue ++ ++ # Reject attempts to remove an already-set identity field ++ if new_value is None: ++ changed_fields.append(field_name) ++ continue ++ ++ # Compare values based on field type ++ if field_name == "ekcert": ++ # For certificates, compare the actual certificate bytes ++ # Note: We compare full certificate, not just public key, because: ++ # 1. User requirement: reject if certificate changed even if same public key ++ # 2. Certificate contains more than just key (issuer, validity period, etc.) ++ # 3. Different cert for same key could indicate compromise or unauthorized replacement ++ try: ++ old_cert_bytes = old_value.public_bytes(Encoding.DER) ++ new_cert_bytes = new_value.public_bytes(Encoding.DER) ++ ++ if old_cert_bytes != new_cert_bytes: ++ changed_fields.append(field_name) ++ except Exception: ++ # If we can't extract certificate bytes, treat as changed to be safe ++ changed_fields.append(field_name) ++ else: ++ # For TPM keys (ek_tpm, aik_tpm), compare as binary data ++ # These are Binary(persist_as=String) fields, so they could be bytes or base64 strings ++ try: ++ old_bytes = old_value if isinstance(old_value, bytes) else base64.b64decode(old_value) ++ new_bytes = new_value if isinstance(new_value, bytes) else base64.b64decode(new_value) ++ ++ if old_bytes != new_bytes: ++ changed_fields.append(field_name) ++ except Exception: ++ # If comparison fails (e.g., invalid base64), treat as changed to be safe ++ changed_fields.append(field_name) ++ ++ # If any TPM identity fields were changed, this is a security violation ++ if changed_fields: ++ # Set flag to indicate TPM identity violation occurred ++ object.__setattr__(self, "_tpm_identity_violation", True) ++ ++ # Log security warning for audit trail ++ # Include agent_id and changed fields, but NOT the actual TPM values (sensitive data) ++ logger.warning( ++ "SECURITY: Rejected attempt to re-register agent '%s' with different TPM identity. " ++ "Changed fields: %s. This indicates a potential UUID spoofing attack. " ++ "The existing agent must be deleted before registering with a new TPM. " ++ "If this is unexpected, investigate for compromise.", ++ self.agent_id, ++ ", ".join(changed_fields), ++ ) ++ ++ # Add validation error to prevent registration ++ # Using "agent_id" field for the error because it's the UUID that's being improperly reused ++ self._add_error( ++ "agent_id", ++ f"cannot re-register with different TPM identity. Changed fields: {', '.join(changed_fields)}. " ++ "To register this UUID with a new TPM, delete the existing agent record first.", ++ ) ++ + def _check_all_cert_compliance(self): + non_compliant_certs = [] + +@@ -280,6 +391,11 @@ class RegistrarAgent(PersistableModel): + + ["port", "mtls_cert"], + ) + ++ # SECURITY CHECK: Verify TPM identity is not being changed on re-registration ++ # This must happen after cast_changes() (so we have new values to compare) ++ # but before other validation (so we reject immediately without processing further) ++ self._check_tpm_identity_immutable() ++ + # Log info about received EK or IAK/IDevID + self._log_root_identity() + # Verify EK as valid +diff --git a/keylime/shared_data.py b/keylime/shared_data.py +index 23a3d81..a415496 100644 +--- a/keylime/shared_data.py ++++ b/keylime/shared_data.py +@@ -58,6 +58,12 @@ class FlatDictView: + with self._lock: + return self._store.get(self._make_key(key), default) + ++ def pop(self, key: Any, default: Any = None) -> Any: ++ """Remove and return value for key, or default if key not present.""" ++ flat_key = self._make_key(key) ++ with self._lock: ++ return self._store.pop(flat_key, default) ++ + def keys(self) -> List[Any]: + """Return keys in this namespace.""" + prefix = f"dict:{self._namespace}:" +diff --git a/keylime/web/registrar/agents_controller.py b/keylime/web/registrar/agents_controller.py +index 9be2ef9..f2246de 100644 +--- a/keylime/web/registrar/agents_controller.py ++++ b/keylime/web/registrar/agents_controller.py +@@ -1,5 +1,8 @@ ++from sqlalchemy.exc import IntegrityError ++ + from keylime import keylime_logging + from keylime.models import RegistrarAgent ++from keylime.shared_data import get_shared_memory + from keylime.web.base import Controller + + logger = keylime_logging.init_logging("registrar") +@@ -28,16 +31,91 @@ class AgentsController(Controller): + + # POST /v2[.:minor]/agents/[:agent_id] + def create(self, agent_id, **params): +- agent = RegistrarAgent.get(agent_id) or RegistrarAgent.empty() # type: ignore[no-untyped-call] +- agent.update({"agent_id": agent_id, **params}) +- challenge = agent.produce_ak_challenge() +- +- if not challenge or not agent.changes_valid: +- self.log_model_errors(agent, logger) +- self.respond(400, "Could not register agent with invalid data") +- return +- +- agent.commit_changes() ++ """Register a new agent or re-register an existing agent. ++ ++ For new agents, this: ++ 1. Validates TPM identity (EK/AIK or IAK/IDevID) ++ 2. Generates an AK challenge encrypted with the EK ++ 3. Stores agent record in pending state ++ 4. Returns challenge blob to agent ++ ++ For existing agents (re-registration with same UUID): ++ 1. Verifies TPM identity has not changed (security check) ++ 2. If identity changed: rejects with 403 Forbidden ++ 3. If identity same: allows re-registration (e.g., after agent restart) ++ ++ Security: Re-registration with a different TPM is forbidden to prevent ++ UUID spoofing attacks where an attacker could impersonate a legitimate ++ agent by reusing its UUID. ++ ++ Race condition protection: Uses per-agent locks from SharedDataManager to prevent ++ race conditions between concurrent registration requests for the same agent_id. ++ This ensures the check-validate-commit sequence is atomic. Additionally, database ++ constraint violations (e.g., duplicate UUIDs from concurrent requests) are caught ++ and returned as 403 Forbidden. ++ """ ++ # Get shared memory manager and per-agent lock storage ++ shared_mem = get_shared_memory() ++ agent_locks = shared_mem.get_or_create_dict("agent_registration_locks") ++ ++ # Get or create a lock specific to this agent_id ++ if agent_id not in agent_locks: ++ agent_locks[agent_id] = shared_mem.manager.Lock() ++ ++ agent_lock = agent_locks[agent_id] ++ ++ # CRITICAL SECTION: Acquire lock to make check-validate-commit atomic ++ with agent_lock: ++ # Step 1: Load existing agent or create new one (inside lock) ++ agent = RegistrarAgent.get(agent_id) or RegistrarAgent.empty() # type: ignore[no-untyped-call] ++ ++ # Step 2: Update agent with new data and validate (inside lock) ++ agent.update({"agent_id": agent_id, **params}) ++ ++ # Step 3: Check for TPM identity change security violation ++ # Use explicit flag instead of fragile string matching for security check ++ if not agent.changes_valid and agent.has_tpm_identity_violation: ++ # Log the validation errors (includes security warning) ++ self.log_model_errors(agent, logger) ++ ++ # Return 403 Forbidden ++ # 403 indicates a policy violation, not a malformed request ++ self.respond(403, "Agent re-registration with different TPM identity is forbidden for security reasons") ++ return ++ ++ # Step 4: Generate AK challenge (inside lock) ++ challenge = agent.produce_ak_challenge() ++ ++ # Step 5: Check for any validation errors or challenge generation failure ++ if not challenge or not agent.changes_valid: ++ self.log_model_errors(agent, logger) ++ self.respond(400, "Could not register agent with invalid data") ++ return ++ ++ # Step 6: Commit to database (inside lock) ++ # This ensures no other request can modify the agent between validation and commit ++ try: ++ agent.commit_changes() ++ except IntegrityError as e: ++ # Database constraint violation - most likely duplicate agent_id ++ # This can happen if two requests try to register the same new UUID simultaneously ++ # and both pass validation before either commits (database race condition) ++ logger.warning( ++ "SECURITY: Agent registration failed due to database constraint violation for agent_id '%s'. " ++ "This UUID may already be registered by a concurrent request or the agent already exists. " ++ "Database error: %s", ++ agent_id, ++ str(e), ++ ) ++ self.respond( ++ 403, ++ f"Agent with UUID '{agent_id}' cannot be registered. " ++ "This UUID is already in use or a concurrent registration is in progress.", ++ ) ++ return ++ ++ # Lock released - safe to respond to client ++ # Return challenge blob for agent to decrypt + self.respond(200, "Success", {"blob": challenge}) + + # DELETE /v2[.:minor]/agents/:agent_id/ +diff --git a/test/test_agents_controller.py b/test/test_agents_controller.py +new file mode 100644 +index 0000000..898d8f0 +--- /dev/null ++++ b/test/test_agents_controller.py +@@ -0,0 +1,513 @@ ++"""Unit tests for AgentsController (registrar). ++ ++Tests the registrar's agent registration endpoints, including the ++security fix that prevents UUID spoofing via re-registration with ++a different TPM identity. ++""" ++ ++# type: ignore - Controller methods are dynamically bound ++ ++import unittest ++from typing import cast ++from unittest.mock import MagicMock, patch ++ ++from sqlalchemy.exc import IntegrityError ++ ++from keylime.web.registrar.agents_controller import AgentsController ++ ++ ++class TestAgentsControllerIndex(unittest.TestCase): ++ """Test cases for AgentsController.index().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ ++ @patch("keylime.models.RegistrarAgent.all_ids") ++ def test_index_success(self, mock_all_ids): ++ """Test successful retrieval of all agent IDs.""" ++ mock_all_ids.return_value = ["agent-1", "agent-2", "agent-3"] ++ ++ self.controller.index() ++ ++ self.controller.respond.assert_called_once_with(200, "Success", {"uuids": ["agent-1", "agent-2", "agent-3"]}) # type: ignore[attr-defined] ++ ++ ++class TestAgentsControllerShow(unittest.TestCase): ++ """Test cases for AgentsController.show().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_show_not_found(self, mock_get): ++ """Test show with non-existent agent.""" ++ mock_get.return_value = None ++ ++ self.controller.show(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with(404, f"Agent with ID '{self.test_agent_id}' not found") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_show_not_active(self, mock_get): ++ """Test show with inactive agent.""" ++ mock_agent = MagicMock() ++ mock_agent.active = False ++ mock_get.return_value = mock_agent ++ ++ self.controller.show(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with( # type: ignore[attr-defined] ++ 404, f"Agent with ID '{self.test_agent_id}' has not been activated" ++ ) ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_show_success(self, mock_get): ++ """Test successful show of active agent.""" ++ mock_agent = MagicMock() ++ mock_agent.active = True ++ mock_agent.render.return_value = {"agent_id": self.test_agent_id, "active": True} ++ mock_get.return_value = mock_agent ++ ++ self.controller.show(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with( # type: ignore[attr-defined] ++ 200, "Success", {"agent_id": self.test_agent_id, "active": True} ++ ) ++ ++ ++class TestAgentsControllerCreate(unittest.TestCase): ++ """Test cases for AgentsController.create() - the main registration endpoint.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.controller.log_model_errors = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_new_agent_success(self, mock_get): ++ """Test successful registration of a new agent.""" ++ # Mock that agent doesn't exist yet ++ mock_get.return_value = None ++ ++ # Create mock agent that will be returned by empty() ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = "challenge_blob_data" ++ ++ # Patch RegistrarAgent.empty to return our mock ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify agent was updated with params ++ mock_agent.update.assert_called_once_with({"agent_id": self.test_agent_id, **params}) ++ ++ # Verify challenge was generated ++ mock_agent.produce_ak_challenge.assert_called_once() ++ ++ # Verify agent was saved ++ mock_agent.commit_changes.assert_called_once() ++ ++ # Verify 200 response with challenge ++ self.controller.respond.assert_called_once_with(200, "Success", {"blob": "challenge_blob_data"}) # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_reregistration_same_tpm_identity(self, mock_get): ++ """Test successful re-registration with same TPM identity.""" ++ # Mock existing agent ++ mock_existing_agent = MagicMock() ++ mock_existing_agent.changes_valid = True ++ mock_existing_agent.errors = {} ++ mock_existing_agent.produce_ak_challenge.return_value = "challenge_blob_data" ++ mock_get.return_value = mock_existing_agent ++ ++ params = {"ek_tpm": "same_ek_key", "aik_tpm": "same_aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify agent was updated ++ mock_existing_agent.update.assert_called_once_with({"agent_id": self.test_agent_id, **params}) ++ ++ # Verify challenge was generated ++ mock_existing_agent.produce_ak_challenge.assert_called_once() ++ ++ # Verify agent was saved ++ mock_existing_agent.commit_changes.assert_called_once() ++ ++ # Verify 200 response ++ self.controller.respond.assert_called_once_with(200, "Success", {"blob": "challenge_blob_data"}) # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_reregistration_different_tpm_identity_forbidden(self, mock_get): ++ """Test re-registration with different TPM identity is rejected with 403. ++ ++ This is the key security fix: preventing UUID spoofing by rejecting ++ attempts to re-register an agent with a different TPM identity. ++ """ ++ # Mock existing agent ++ mock_existing_agent = MagicMock() ++ mock_existing_agent.changes_valid = False # Validation failed ++ # Simulate the error added by _check_tpm_identity_immutable ++ mock_existing_agent.errors = { ++ "agent_id": [ ++ "Agent re-registration attempted with different TPM identity (changed fields: ek_tpm). " ++ "This is a security violation - the same agent UUID cannot be reused with a different TPM." ++ ] ++ } ++ mock_get.return_value = mock_existing_agent ++ ++ params = {"ek_tpm": "different_ek_key", "aik_tpm": "same_aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify agent was updated (which triggers validation) ++ mock_existing_agent.update.assert_called_once_with({"agent_id": self.test_agent_id, **params}) ++ ++ # Verify errors were logged ++ self.controller.log_model_errors.assert_called_once() # type: ignore[attr-defined] ++ ++ # Verify 403 Forbidden response (not 400!) ++ self.controller.respond.assert_called_once_with( # type: ignore[attr-defined] ++ 403, "Agent re-registration with different TPM identity is forbidden for security reasons" ++ ) ++ ++ # Verify agent was NOT saved ++ mock_existing_agent.commit_changes.assert_not_called() ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_invalid_data_other_validation_error(self, mock_get): ++ """Test registration with other validation errors returns 400.""" ++ # Mock agent with validation errors (not TPM identity related) ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = False ++ # Error not related to TPM identity ++ mock_agent.errors = {"ek_tpm": ["must be a valid TPM2B_PUBLIC structure"]} ++ mock_agent.has_tpm_identity_violation = False # Not a TPM identity violation ++ mock_agent.produce_ak_challenge.return_value = None ++ mock_get.return_value = None ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "invalid_ek_format"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify errors were logged ++ self.controller.log_model_errors.assert_called_once() # type: ignore[attr-defined] ++ ++ # Verify 400 Bad Request (not 403) ++ self.controller.respond.assert_called_once_with(400, "Could not register agent with invalid data") # type: ignore[attr-defined] ++ ++ # Verify agent was NOT saved ++ mock_agent.commit_changes.assert_not_called() ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_challenge_generation_failure(self, mock_get): ++ """Test registration fails if challenge generation fails.""" ++ # Mock agent where challenge generation fails ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = None # Challenge generation failed ++ mock_get.return_value = None ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify errors were logged ++ self.controller.log_model_errors.assert_called_once() # type: ignore[attr-defined] ++ ++ # Verify 400 response ++ self.controller.respond.assert_called_once_with(400, "Could not register agent with invalid data") # type: ignore[attr-defined] ++ ++ # Verify agent was NOT saved ++ mock_agent.commit_changes.assert_not_called() ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_create_validation_error_with_agent_id_but_not_tpm_identity(self, mock_get): ++ """Test that agent_id errors unrelated to TPM identity get 400, not 403.""" ++ # Mock agent with agent_id error, but not about TPM identity ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = False ++ mock_agent.errors = {"agent_id": ["must be a valid UUID format"]} # Not about TPM identity ++ mock_agent.has_tpm_identity_violation = False # Not a TPM identity violation ++ mock_agent.produce_ak_challenge.return_value = None ++ mock_get.return_value = None ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify 400 Bad Request (not 403) because it's not a TPM identity violation ++ self.controller.respond.assert_called_once_with(400, "Could not register agent with invalid data") # type: ignore[attr-defined] ++ ++ ++class TestAgentsControllerDelete(unittest.TestCase): ++ """Test cases for AgentsController.delete().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_delete_not_found(self, mock_get): ++ """Test delete with non-existent agent.""" ++ mock_get.return_value = None ++ ++ self.controller.delete(self.test_agent_id) ++ ++ self.controller.respond.assert_called_once_with(404, f"Agent with ID '{self.test_agent_id}' not found") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_delete_success(self, mock_get): ++ """Test successful agent deletion.""" ++ mock_agent = MagicMock() ++ mock_get.return_value = mock_agent ++ ++ self.controller.delete(self.test_agent_id) ++ ++ # Verify agent was deleted ++ mock_agent.delete.assert_called_once() ++ ++ # Verify 200 response ++ self.controller.respond.assert_called_once_with(200, "Success") # type: ignore[attr-defined] ++ ++ ++class TestAgentsControllerActivate(unittest.TestCase): ++ """Test cases for AgentsController.activate().""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.test_agent_id = "test-agent-123" ++ self.test_auth_tag = "valid_auth_tag" ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_activate_not_found(self, mock_get): ++ """Test activate with non-existent agent.""" ++ mock_get.return_value = None ++ ++ self.controller.activate(self.test_agent_id, self.test_auth_tag) ++ ++ self.controller.respond.assert_called_once_with(404, f"Agent with ID '{self.test_agent_id}' not found") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_activate_success(self, mock_get): ++ """Test successful agent activation.""" ++ mock_agent = MagicMock() ++ mock_agent.verify_ak_response.return_value = True # Auth tag is valid ++ mock_get.return_value = mock_agent ++ ++ self.controller.activate(self.test_agent_id, self.test_auth_tag) ++ ++ # Verify auth tag was verified ++ mock_agent.verify_ak_response.assert_called_once_with(self.test_auth_tag) ++ ++ # Verify agent was saved ++ mock_agent.commit_changes.assert_called_once() ++ ++ # Verify 200 response ++ self.controller.respond.assert_called_once_with(200, "Success") # type: ignore[attr-defined] ++ ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_activate_invalid_auth_tag(self, mock_get): ++ """Test activation with invalid auth tag.""" ++ mock_agent = MagicMock() ++ mock_agent.verify_ak_response.return_value = False # Auth tag is invalid ++ mock_get.return_value = mock_agent ++ ++ self.controller.activate(self.test_agent_id, self.test_auth_tag) ++ ++ # Verify auth tag was verified ++ mock_agent.verify_ak_response.assert_called_once_with(self.test_auth_tag) ++ ++ # Verify agent was deleted (due to failed activation) ++ mock_agent.delete.assert_called_once() ++ ++ # Verify agent was NOT saved ++ mock_agent.commit_changes.assert_not_called() ++ ++ # Verify 400 response with detailed error message ++ self.controller.respond.assert_called_once() # type: ignore[attr-defined] ++ call_args = self.controller.respond.call_args # type: ignore[attr-defined] ++ self.assertEqual(call_args[0][0], 400) ++ self.assertIn(self.test_auth_tag, call_args[0][1]) ++ self.assertIn(self.test_agent_id, call_args[0][1]) ++ self.assertIn("deleted", call_args[0][1]) ++ ++ ++class TestAgentsControllerConcurrency(unittest.TestCase): ++ """Test cases for concurrent registration TOCTOU race condition protection.""" ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ mock_action_handler = MagicMock() ++ self.controller = cast(AgentsController, AgentsController(mock_action_handler)) ++ self.controller.respond = MagicMock() ++ self.controller.log_model_errors = MagicMock() ++ self.test_agent_id = "concurrent-test-agent" ++ ++ @patch("keylime.web.registrar.agents_controller.get_shared_memory") ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_concurrent_registration_uses_locking(self, mock_get, mock_shared_mem): ++ """Test that concurrent registration attempts use per-agent locking. ++ ++ This test verifies that the locking mechanism is invoked to prevent ++ TOCTOU race conditions during concurrent registration. ++ """ ++ # Mock shared memory manager with lock support ++ mock_manager = MagicMock() ++ mock_lock = MagicMock() ++ mock_lock.__enter__ = MagicMock(return_value=None) ++ mock_lock.__exit__ = MagicMock(return_value=None) ++ mock_manager.Lock.return_value = mock_lock ++ ++ mock_agent_locks = MagicMock() ++ mock_agent_locks.__contains__ = MagicMock(return_value=False) ++ mock_agent_locks.__setitem__ = MagicMock() ++ mock_agent_locks.__getitem__ = MagicMock(return_value=mock_lock) ++ ++ mock_shared_mem.return_value.get_or_create_dict.return_value = mock_agent_locks ++ mock_shared_mem.return_value.manager = mock_manager ++ ++ # Mock agent that doesn't exist yet (new registration) ++ mock_get.return_value = None ++ ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = "challenge_blob" ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify lock was acquired and released ++ mock_lock.__enter__.assert_called_once() ++ mock_lock.__exit__.assert_called_once() ++ ++ # Verify successful registration ++ mock_agent.commit_changes.assert_called_once() ++ self.controller.respond.assert_called_once_with(200, "Success", {"blob": "challenge_blob"}) # type: ignore[attr-defined] ++ ++ @patch("keylime.web.registrar.agents_controller.get_shared_memory") ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_different_agents_use_different_locks(self, mock_get, mock_shared_mem): ++ """Test that different agent_ids use different locks for parallel registration. ++ ++ This ensures that registrations for different agents don't block each other, ++ only concurrent registrations for the same agent_id are serialized. ++ """ ++ # Mock shared memory manager ++ mock_manager = MagicMock() ++ ++ def mock_lock_factory(): ++ """Return a new lock each time.""" ++ return MagicMock() ++ ++ mock_manager.Lock.side_effect = mock_lock_factory ++ ++ mock_agent_locks = {} ++ ++ def mock_getitem(_self, key): # pylint: disable=unused-argument ++ return mock_agent_locks.get(key) ++ ++ def mock_setitem(_self, key, value): # pylint: disable=unused-argument ++ mock_agent_locks[key] = value ++ ++ def mock_contains(_self, key): # pylint: disable=unused-argument ++ return key in mock_agent_locks ++ ++ mock_locks_dict = MagicMock() ++ mock_locks_dict.__contains__ = mock_contains ++ mock_locks_dict.__setitem__ = mock_setitem ++ mock_locks_dict.__getitem__ = mock_getitem ++ ++ mock_shared_mem.return_value.get_or_create_dict.return_value = mock_locks_dict ++ mock_shared_mem.return_value.manager = mock_manager ++ ++ # Register two different agents ++ mock_get.return_value = None ++ ++ for agent_id in ["agent-a", "agent-b"]: ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = f"challenge_{agent_id}" ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ self.controller.respond = MagicMock() # Reset for each call ++ params = {"ek_tpm": f"ek_{agent_id}", "aik_tpm": f"aik_{agent_id}"} ++ self.controller.create(agent_id, **params) ++ ++ # Verify that two different locks were created (one per agent) ++ self.assertEqual(len(mock_agent_locks), 2) ++ self.assertIn("agent-a", mock_agent_locks) ++ self.assertIn("agent-b", mock_agent_locks) ++ # Verify they are different lock objects ++ self.assertIsNot(mock_agent_locks["agent-a"], mock_agent_locks["agent-b"]) ++ ++ @patch("keylime.web.registrar.agents_controller.get_shared_memory") ++ @patch("keylime.models.RegistrarAgent.get") ++ def test_concurrent_new_registration_database_constraint_violation(self, mock_get, mock_shared_mem): ++ """Test that database constraint violations during concurrent new agent registration return 403. ++ ++ This handles the edge case where two requests both create empty agents for the same UUID, ++ both pass validation, but the second commit fails with IntegrityError due to duplicate ++ primary key. This should return 403 Forbidden, not 500 Internal Server Error. ++ """ ++ # Mock shared memory manager with lock support ++ mock_manager = MagicMock() ++ mock_lock = MagicMock() ++ mock_lock.__enter__ = MagicMock(return_value=None) ++ mock_lock.__exit__ = MagicMock(return_value=None) ++ mock_manager.Lock.return_value = mock_lock ++ ++ mock_agent_locks = MagicMock() ++ mock_agent_locks.__contains__ = MagicMock(return_value=False) ++ mock_agent_locks.__setitem__ = MagicMock() ++ mock_agent_locks.__getitem__ = MagicMock(return_value=mock_lock) ++ ++ mock_shared_mem.return_value.get_or_create_dict.return_value = mock_agent_locks ++ mock_shared_mem.return_value.manager = mock_manager ++ ++ # Mock agent that doesn't exist yet (new registration) ++ mock_get.return_value = None ++ ++ mock_agent = MagicMock() ++ mock_agent.changes_valid = True ++ mock_agent.errors = {} ++ mock_agent.produce_ak_challenge.return_value = "challenge_blob" ++ ++ # Simulate IntegrityError during commit (duplicate primary key) ++ # IntegrityError(statement, params, orig) where orig is the original exception ++ orig_exception = Exception("UNIQUE constraint failed: registrarmain.agent_id") ++ mock_agent.commit_changes.side_effect = IntegrityError("INSERT INTO registrarmain ...", None, orig_exception) ++ ++ with patch("keylime.models.RegistrarAgent.empty", return_value=mock_agent): ++ params = {"ek_tpm": "ek_key", "aik_tpm": "aik_key"} ++ self.controller.create(self.test_agent_id, **params) ++ ++ # Verify 403 Forbidden response (not 500) ++ self.controller.respond.assert_called_once() # type: ignore[attr-defined] ++ call_args = self.controller.respond.call_args # type: ignore[attr-defined] ++ self.assertEqual(call_args[0][0], 403) ++ self.assertIn(self.test_agent_id, call_args[0][1]) ++ self.assertIn("already in use", call_args[0][1]) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +diff --git a/test/test_registrar_tpm_identity.py b/test/test_registrar_tpm_identity.py +new file mode 100644 +index 0000000..2fc69b2 +--- /dev/null ++++ b/test/test_registrar_tpm_identity.py +@@ -0,0 +1,342 @@ ++""" ++Unit tests for RegistrarAgent TPM identity immutability security check. ++ ++This module tests the _check_tpm_identity_immutable() method which prevents ++UUID spoofing attacks by rejecting re-registration attempts with different TPM identities. ++""" ++ ++import base64 ++import types ++import unittest ++from unittest.mock import Mock ++ ++import cryptography.x509 ++ ++from keylime.certificate_wrapper import wrap_certificate ++from keylime.models.registrar.registrar_agent import RegistrarAgent ++ ++ ++class TestRegistrarAgentTPMIdentity(unittest.TestCase): ++ """Test cases for RegistrarAgent TPM identity immutability.""" ++ ++ # pylint: disable=protected-access # Testing protected methods ++ # pylint: disable=not-callable # False positive: methods bound via types.MethodType are callable ++ ++ def setUp(self): ++ """Set up test fixtures.""" ++ # EK certificate (used for testing certificate comparison) ++ self.ek_cert_pem = """-----BEGIN CERTIFICATE----- ++MIIEnzCCA4egAwIBAgIEMV64bDANBgkqhkiG9w0BAQUFADBtMQswCQYDVQQGEwJE ++RTEQMA4GA1UECBMHQmF2YXJpYTEhMB8GA1UEChMYSW5maW5lb24gVGVjaG5vbG9n ++aWVzIEFHMQwwCgYDVQQLEwNBSU0xGzAZBgNVBAMTEklGWCBUUE0gRUsgUm9vdCBD ++QTAeFw0wNTEwMjAxMzQ3NDNaFw0yNTEwMjAxMzQ3NDNaMHcxCzAJBgNVBAYTAkRF ++MQ8wDQYDVQQIEwZTYXhvbnkxITAfBgNVBAoTGEluZmluZW9uIFRlY2hub2xvZ2ll ++cyBBRzEMMAoGA1UECxMDQUlNMSYwJAYDVQQDEx1JRlggVFBNIEVLIEludGVybWVk ++aWF0ZSBDQSAwMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALftPhYN ++t4rE+JnU/XOPICbOBLvfo6iA7nuq7zf4DzsAWBdsZEdFJQfaK331ihG3IpQnlQ2i ++YtDim289265f0J4OkPFpKeFU27CsfozVaNUm6UR/uzwA8ncxFc3iZLRMRNLru/Al ++VG053ULVDQMVx2iwwbBSAYO9pGiGbk1iMmuZaSErMdb9v0KRUyZM7yABiyDlM3cz ++UQX5vLWV0uWqxdGoHwNva5u3ynP9UxPTZWHZOHE6+14rMzpobs6Ww2RR8BgF96rh ++4rRAZEl8BXhwiQq4STvUXkfvdpWH4lzsGcDDtrB6Nt3KvVNvsKz+b07Dk+Xzt+EH ++NTf3Byk2HlvX+scCAwEAAaOCATswggE3MB0GA1UdDgQWBBQ4k8292HPEIzMV4bE7 ++qWoNI8wQxzAOBgNVHQ8BAf8EBAMCAgQwEgYDVR0TAQH/BAgwBgEB/wIBADBYBgNV ++HSABAf8ETjBMMEoGC2CGSAGG+EUBBy8BMDswOQYIKwYBBQUHAgEWLWh0dHA6Ly93 ++d3cudmVyaXNpZ24uY29tL3JlcG9zaXRvcnkvaW5kZXguaHRtbDCBlwYDVR0jBIGP ++MIGMgBRW65FEhWPWcrOu1EWWC/eUDlRCpqFxpG8wbTELMAkGA1UEBhMCREUxEDAO ++BgNVBAgTB0JhdmFyaWExITAfBgNVBAoTGEluZmluZW9uIFRlY2hub2xvZ2llcyBB ++RzEMMAoGA1UECxMDQUlNMRswGQYDVQQDExJJRlggVFBNIEVLIFJvb3QgQ0GCAQMw ++DQYJKoZIhvcNAQEFBQADggEBABJ1+Ap3rNlxZ0FW0aIgdzktbNHlvXWNxFdYIBbM ++OKjmbOos0Y4O60eKPu259XmMItCUmtbzF3oKYXq6ybARUT2Lm+JsseMF5VgikSlU ++BJALqpKVjwAds81OtmnIQe2LSu4xcTSavpsL4f52cUAu/maMhtSgN9mq5roYptq9 ++DnSSDZrX4uYiMPl//rBaNDBflhJ727j8xo9CCohF3yQUoQm7coUgbRMzyO64yMIO ++3fhb+Vuc7sNwrMOz3VJN14C3JMoGgXy0c57IP/kD5zGRvljKEvrRC2I147+fPeLS ++DueRMS6lblvRKiZgmGAg7YaKOkOaEmVDMQ+fTo2Po7hI5wc= ++-----END CERTIFICATE-----""" ++ ++ # Create wrapped cert from real certificate ++ self.ek_cert = cryptography.x509.load_pem_x509_certificate(self.ek_cert_pem.encode()) ++ self.ek_cert_wrapped = wrap_certificate(self.ek_cert, None) ++ ++ # Create a different cert mock that returns different DER bytes ++ self.different_ek_cert_wrapped = Mock() ++ self.different_ek_cert_wrapped.public_bytes = Mock(return_value=b"DIFFERENT_CERTIFICATE_DER_BYTES_FOR_TESTING") ++ ++ # Sample TPM keys (base64 encoded for simplicity in tests) ++ self.ek_tpm_1 = b"EK_TPM_KEY_NUMBER_ONE_SAMPLE_DATA" ++ self.ek_tpm_2 = b"EK_TPM_KEY_NUMBER_TWO_DIFFERENT_" ++ self.aik_tpm_1 = b"AIK_TPM_KEY_NUMBER_ONE_SAMPLE_DATA" ++ self.aik_tpm_2 = b"AIK_TPM_KEY_NUMBER_TWO_DIFFERENT_" ++ ++ # IAK/IDevID keys for testing that they are not checked ++ self.iak_tpm_1 = b"IAK_TPM_KEY_NUMBER_ONE" ++ self.iak_tpm_2 = b"IAK_TPM_KEY_NUMBER_TWO" ++ ++ def create_mock_registrar_agent(self, agent_id="test-agent-uuid"): ++ """Create a mock RegistrarAgent with necessary attributes.""" ++ agent = Mock() ++ agent.agent_id = agent_id ++ agent.changes = {} ++ agent.values = {} ++ agent.committed = {} ++ agent._add_error = Mock() ++ agent.errors = {} ++ ++ # Bind the actual method to the mock instance ++ agent._check_tpm_identity_immutable = types.MethodType(RegistrarAgent._check_tpm_identity_immutable, agent) ++ ++ return agent ++ ++ def test_new_agent_no_committed_values(self): ++ """Test that new agents (no committed values) are not checked.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = {} # New agent, no previous values ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors for new agents ++ agent._add_error.assert_not_called() ++ ++ def test_reregistration_same_tpm_all_fields_identical(self): ++ """Test re-registration with identical TPM identity passes.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, # Same ++ "ekcert": self.ek_cert_wrapped, # Same ++ "aik_tpm": self.aik_tpm_1, # Same ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors ++ agent._add_error.assert_not_called() ++ ++ def test_reregistration_different_ek_tpm(self): ++ """Test re-registration with different EK TPM is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_2, # DIFFERENT ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for agent_id field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ self.assertIn("different TPM identity", call_args[0][1]) ++ self.assertIn("ek_tpm", call_args[0][1]) ++ ++ def test_reregistration_different_aik_tpm(self): ++ """Test re-registration with different AIK TPM is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_2, # DIFFERENT ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for agent_id field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ self.assertIn("different TPM identity", call_args[0][1]) ++ self.assertIn("aik_tpm", call_args[0][1]) ++ ++ def test_reregistration_different_ekcert(self): ++ """Test re-registration with different EK certificate is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.different_ek_cert_wrapped, # DIFFERENT ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for agent_id field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ self.assertIn("different TPM identity", call_args[0][1]) ++ self.assertIn("ekcert", call_args[0][1]) ++ ++ def test_reregistration_multiple_fields_changed(self): ++ """Test re-registration with multiple fields changed lists all of them.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_2, # DIFFERENT ++ "ekcert": self.different_ek_cert_wrapped, # DIFFERENT ++ "aik_tpm": self.aik_tpm_2, # DIFFERENT ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error listing all changed fields ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertEqual(call_args[0][0], "agent_id") ++ error_message = call_args[0][1] ++ self.assertIn("ek_tpm", error_message) ++ self.assertIn("ekcert", error_message) ++ self.assertIn("aik_tpm", error_message) ++ ++ def test_adding_ekcert_to_existing_agent(self): ++ """Test that adding EK cert to existing agent (without cert) is allowed.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": None, # Previously no cert ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, # NOW adding cert ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - adding cert is allowed ++ agent._add_error.assert_not_called() ++ ++ def test_removing_ek_tpm_rejected(self): ++ """Test that removing an existing EK TPM is rejected.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ agent.changes = { ++ "ek_tpm": None, # Trying to remove ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertIn("ek_tpm", call_args[0][1]) ++ ++ def test_iak_idevid_changes_not_checked(self): ++ """Test that IAK/IDevID field changes are NOT checked (allowed).""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ "iak_tpm": self.iak_tpm_1, ++ "idevid_tpm": b"IDEVID_OLD", ++ } ++ agent.changes = { ++ "ek_tpm": self.ek_tpm_1, # Same ++ "ekcert": self.ek_cert_wrapped, # Same ++ "aik_tpm": self.aik_tpm_1, # Same ++ "iak_tpm": self.iak_tpm_2, # DIFFERENT - but not checked ++ "idevid_tpm": b"IDEVID_NEW", # DIFFERENT - but not checked ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - IAK/IDevID are not checked ++ agent._add_error.assert_not_called() ++ ++ def test_only_changed_fields_are_checked(self): ++ """Test that only fields in changes dict are checked.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ # Only updating IP, not touching TPM identity fields ++ agent.changes = { ++ "ip": "192.168.1.100", ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - no identity fields changed ++ agent._add_error.assert_not_called() ++ ++ def test_base64_encoded_tpm_keys(self): ++ """Test that base64-encoded TPM keys are properly compared.""" ++ agent = self.create_mock_registrar_agent() ++ ++ # Simulate keys stored as base64 strings (as they might be from database) ++ ek_b64 = base64.b64encode(self.ek_tpm_1).decode("utf-8") ++ aik_b64 = base64.b64encode(self.aik_tpm_1).decode("utf-8") ++ ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, # As bytes ++ "aik_tpm": self.aik_tpm_1, # As bytes ++ } ++ agent.changes = { ++ "ek_tpm": ek_b64, # As base64 string ++ "aik_tpm": aik_b64, # As base64 string ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should not add any errors - should handle both formats ++ agent._add_error.assert_not_called() ++ ++ def test_partial_update_only_one_field(self): ++ """Test updating only one TPM field while others remain unchanged.""" ++ agent = self.create_mock_registrar_agent() ++ agent.committed = { ++ "ek_tpm": self.ek_tpm_1, ++ "ekcert": self.ek_cert_wrapped, ++ "aik_tpm": self.aik_tpm_1, ++ } ++ # Only changing AIK in this update ++ agent.changes = { ++ "aik_tpm": self.aik_tpm_2, # DIFFERENT ++ } ++ ++ agent._check_tpm_identity_immutable() ++ ++ # Should add error for the changed field ++ agent._add_error.assert_called_once() ++ call_args = agent._add_error.call_args ++ self.assertIn("aik_tpm", call_args[0][1]) ++ ++ ++if __name__ == "__main__": ++ unittest.main() +-- +2.47.3 + diff --git a/keylime.spec b/keylime.spec index ad1ae93..88bb78b 100644 --- a/keylime.spec +++ b/keylime.spec @@ -14,7 +14,7 @@ Name: keylime Version: 7.12.1 -Release: 11%{?dist}.2 +Release: 11%{?dist}.3 Summary: Open source TPM software for Bootstrapping and Maintaining Trust URL: https://github.com/keylime/keylime @@ -53,6 +53,13 @@ Patch: 0011-fix-malformed-certs-workaround.patch # Backported from https://github.com/keylime/keylime/pull/1795 Patch: 0012-keylime-policy-avoid-opening-dev-stdout.patch +# CVE-2025-13609 +# Backports from: +# - https://github.com/keylime/keylime/pull/1817/commits/1024e19d +# - https://github.com/keylime/keylime/pull/1825 +Patch: 0013-Add-shared-memory-infrastructure-for-multiprocess-co.patch +Patch: 0014-Fix-registrar-duplicate-UUID-vulnerability.patch + # Main program: Apache-2.0 # Icons: MIT License: Apache-2.0 AND MIT @@ -470,6 +477,9 @@ fi %changelog ## START: Generated by rpmautospec +* Thu Dec 11 2025 Sergio Correia - 7.12.1-15 +- Registrar allows identity takeover via duplicate UUID registration + * Mon Sep 15 2025 Anderson Toshiyuki Sasaki - 7.12.1-14 - Properly fix malformed TPM certificates workaround