Merge pull request #332 from minvws/total-cleanup

Bit of code cleanup

Author: jaytaph

.gitignore

@@ -2,5 +2,13 @@ __pycache__
 .venv
 secrets/ssl/*
 secrets/*
+tests/secrets/
 app.conf
 auth_cert.json
+
+# Test coverage artifacts (generated by `make test`)
+.coverage
+coverage.xml
+
+# Certificates / keys (never commit secrets)
+*.pem

README.md

@@ -29,6 +29,15 @@ The code examples are only meant to help understand concepts and demonstrate pos
 By using or referencing this code, you acknowledge that you do so at your own
 risk and that the authors assume no liability for any consequences of its use.
 
+## Security & trust model
+
+The PRS does not authenticate callers itself. Caller identity is verified by a
+separate upstream system (the OIN-verifier) that injects trusted headers, which
+the PRS trusts as-is. This means the PRS must never be reachable without that
+proxy in front of it. See [docs/trust-model.md](docs/trust-model.md) for the
+full trust model, the headers involved, and the deployment invariants that must
+hold.
+
 ## Development setup
 
 This project can be setup and tested either as a python application directly on an operating system or in a Docker

app/auth.py

@@ -11,18 +11,6 @@
 logger = logging.getLogger(__name__)
 
 
-class OAuthError(Exception):
-    """
-    Raised for general OAuth2 errors.
-    """
-
-    def __init__(self, code: str, description: str, status_code: int = 400):
-        super().__init__(description)
-        self.code = code
-        self.description = description
-        self.status_code = status_code
-
-
 def get_auth_ctx(
     request: Request,
     auth_headers_service: AuthHeaderService = Depends(

app/config.py

@@ -3,7 +3,7 @@
 import os
 from typing import Any, List
 
-from pydantic import BaseModel, ValidationError, field_validator
+from pydantic import BaseModel, field_validator
 from pydantic import Field
 
 _PATH = "app.conf"
@@ -159,18 +159,15 @@ def get_config(path: str | None = None) -> Config:
     # a standard format for pydantic, we need to do some manual parsing first.
     ini_data = read_ini_file(path)
 
-    try:
-        # Convert database.retry_backoff to a list of floats
-        if "retry_backoff" in ini_data["database"] and isinstance(
-            ini_data["database"]["retry_backoff"], str
-        ):
-            # convert the string to a list of floats
-            ini_data["database"]["retry_backoff"] = [
-                float(i) for i in ini_data["database"]["retry_backoff"].split(",")
-            ]
-
-        _CONFIG = Config.model_validate(ini_data)
-    except ValidationError as e:
-        raise e
+    # Convert database.retry_backoff to a list of floats
+    if "retry_backoff" in ini_data["database"] and isinstance(
+        ini_data["database"]["retry_backoff"], str
+    ):
+        # convert the string to a list of floats
+        ini_data["database"]["retry_backoff"] = [
+            float(i) for i in ini_data["database"]["retry_backoff"].split(",")
+        ]
+
+    _CONFIG = Config.model_validate(ini_data)
 
     return _CONFIG

app/container.py

@@ -1,4 +1,5 @@
 import base64
+import logging
 
 import inject
 
@@ -14,15 +15,49 @@
 from app.services.org_service import OrgService
 from app.services.pseudonym_service import PseudonymService
 from app.services.rid_service import RidService
-import logging
 
 logger = logging.getLogger(__name__)
 
+# Minimum master key size in bytes. The master key must have at least as much
+# entropy as the 256-bit keys HKDF derives from it.
+_MIN_MASTER_KEY_BYTES = 32
+
+
+def _load_master_key(raw: str) -> bytes:
+    """
+    Decode and validate the configured pseudonym master key.
+
+    All pseudonym, reversible-pseudonym and RID keys are derived from this key,
+    so an empty or weak master key would make every pseudonym forgeable and every
+    reversible pseudonym/RID decryptable. Refuse to start rather than silently
+    run with a b"" key.
+    """
+    if not raw:
+        raise ValueError(
+            "pseudonym.master_key is not configured. Set a base64-encoded key of "
+            "at least 32 bytes, e.g. `openssl rand -base64 32`."
+        )
+
+    key = base64.urlsafe_b64decode(raw)
+    if len(key) < _MIN_MASTER_KEY_BYTES:
+        raise ValueError(
+            f"pseudonym.master_key is too short ({len(key)} bytes decoded); "
+            f"at least {_MIN_MASTER_KEY_BYTES} bytes are required."
+        )
+
+    return key
+
 
 def container_config(binder: inject.Binder) -> None:
     config = get_config()
 
-    db = Database(dsn=config.database.dsn)
+    db = Database(
+        dsn=config.database.dsn,
+        pool_size=config.database.pool_size,
+        max_overflow=config.database.max_overflow,
+        pool_pre_ping=config.database.pool_pre_ping,
+        pool_recycle=config.database.pool_recycle,
+    )
     binder.bind(Database, db)
 
     key_resolver = KeyResolver(db)
@@ -66,17 +101,13 @@ def container_config(binder: inject.Binder) -> None:
         oprf_service = OprfService(server_key=key)
     binder.bind(OprfService, oprf_service)
 
-    pseudonym_service = PseudonymService(
-        # This should be done through an HSM
-        base64.urlsafe_b64decode(config.pseudonym.master_key or ""),
-    )
+    # This should be done through an HSM
+    master_key = _load_master_key(config.pseudonym.master_key)
+
+    pseudonym_service = PseudonymService(master_key)
     binder.bind(PseudonymService, pseudonym_service)
 
-    rid_service = RidService(
-        # This should be done through an HSM
-        base64.urlsafe_b64decode(config.pseudonym.master_key or ""),
-        b"RID:v1",
-    )
+    rid_service = RidService(master_key, b"RID:v1")
     binder.bind(RidService, rid_service)
 
 

app/data.py

@@ -10,7 +10,14 @@
 
 
 class Database:
-    def __init__(self, dsn: str):
+    def __init__(
+        self,
+        dsn: str,
+        pool_size: int = 5,
+        max_overflow: int = 10,
+        pool_pre_ping: bool = False,
+        pool_recycle: int = 3600,
+    ):
         try:
             if "sqlite://" in dsn:
                 self.engine = create_engine(
@@ -21,7 +28,14 @@ def __init__(self, dsn: str):
                     echo=False,
                 )
             else:
-                self.engine = create_engine(dsn, echo=False)
+                self.engine = create_engine(
+                    dsn,
+                    echo=False,
+                    pool_size=pool_size,
+                    max_overflow=max_overflow,
+                    pool_pre_ping=pool_pre_ping,
+                    pool_recycle=pool_recycle,
+                )
         except BaseException as e:
             logger.exception("error while connecting to database")
             raise e

app/db/db.py

@@ -76,16 +76,3 @@ def update(
         entry.key_data = key_data
         self.db_session.add(entry)
         return entry
-
-    def delete_by_org(self, org_id: uuid.UUID) -> int:
-        """
-        Deletes all key entries for a given organization.
-        """
-        query = select(OrganizationKey).where(OrganizationKey.organization_id == org_id)
-        entries = self.db_session.execute(query).scalars().all()
-        count = len(entries)
-
-        for entry in entries:
-            self.db_session.delete(entry)
-
-        return count

app/db/repositories/org_key_repository.py

@@ -1,5 +1,4 @@
 import logging
-import uuid
 from typing import Optional
 
 from app.db.decorator import repository
@@ -35,38 +34,3 @@ def create(self, oin: Oin, name: str, max_usage_level: str) -> Organization:
 
         logger.info("created organization with OIN %s and name %r", oin.value, name)
         return entry
-
-    def update(
-        self, org_id: uuid.UUID, name: str, max_usage_level: str
-    ) -> Optional[Organization]:
-        """
-        Updates an existing key entry.
-        """
-        org = self.get_by_id(org_id)
-        if org is None:
-            logger.error(f"organization with ID {org_id} does not exist")
-            raise ValueError("Organization not found")
-
-        org.name = name
-        org.max_rid_usage = max_usage_level
-        self.db_session.add(org)
-        return org
-
-    def delete(self, org_id: uuid.UUID) -> bool:
-        """
-        Deletes an organization by its unique ID.
-        """
-        org = self.get_by_id(org_id)
-        if org is None:
-            logger.error("organization with ID %s does not exist", org_id)
-            return False
-
-        self.db_session.delete(org)
-        return True
-
-    def get_by_id(self, org_id: uuid.UUID) -> Optional[Organization]:
-        """
-        Fetches the organization by its unique ID.
-        """
-        query = select(Organization).where(Organization.id == org_id)
-        return self.db_session.execute(query).scalars().first()

app/db/repositories/org_repository.py

@@ -18,12 +18,6 @@ class RegisterRequest(BaseModel):
     key_id: str | None
 
 
-class OrgRequest(BaseModel):
-    oin: Oin
-    name: str = Field(..., min_length=5, max_length=50)
-    max_key_usage: RidUsage
-
-
 class HsmKeyVersionRequest(BaseModel):
     oin: Oin
     from_dt: datetime | None = None
@@ -120,8 +114,7 @@ def convert_personal_id(cls, data: dict[str, Any]) -> dict[str, Any]:
         return data
 
 
-class ReceiverRequest(BaseModel):
-    blind_factor: str
+class JweReceiverRequest(BaseModel):
     jwe: str
     priv_key_pem: str
 
@@ -147,27 +140,5 @@ def validate_priv_key_pem(cls, data: dict[str, Any]) -> dict[str, Any]:
         return data
 
 
-class JweReceiverRequest(BaseModel):
-    jwe: str
-    priv_key_pem: str
-
-    @model_validator(mode="before")
-    @classmethod
-    def validate_jwe(cls, data: dict[str, Any]) -> dict[str, Any]:
-        # Check if JWE is actually a jwe token
-        jwe_token = data.get("jwe")
-        if not isinstance(jwe_token, str) or len(jwe_token.split(".")) != 5:
-            logger.warning("invalid JWE token format: %s", jwe_token)
-            raise ValueError("Invalid JWE token")
-        return data
-
-    @model_validator(mode="before")
-    @classmethod
-    def validate_priv_key_pem(cls, data: dict[str, Any]) -> dict[str, Any]:
-        priv_key_pem = data.get("priv_key_pem")
-        if not isinstance(priv_key_pem, str) or not priv_key_pem.startswith(
-            "-----BEGIN PRIVATE KEY-----"
-        ):
-            logger.warning("invalid private key PEM format")
-            raise ValueError("Invalid private key PEM format")
-        return data
+class ReceiverRequest(JweReceiverRequest):
+    blind_factor: str