diff --git a/.bumpversion.toml b/.bumpversion.toml index 9609497d..f358bed6 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "1.19.4+261027" +current_version = "1.20.0+260912" parse = "(?P\\d+)\\.(?P\\d+)\\.(?P\\d+)\\+(?P\\d+)" serialize = ["{major}.{minor}.{patch}+{build}"] search = "{current_version}" diff --git a/Dockerfile b/Dockerfile index 07daf52e..70807308 100644 --- a/Dockerfile +++ b/Dockerfile @@ -133,7 +133,10 @@ RUN --mount=type=ssh apt-get update && apt-get install -y \ && rm -rf /var/lib/apt/lists/* \ && chmod a+x /docker-entrypoint.sh \ && WHEELFILE=`echo /tmp/wheelhouse/rasenmaeher_api-*.whl` \ - && pip3 install --find-links=/tmp/wheelhouse/ "$WHEELFILE"[all] \ + # --no-index, or --find-links is merely additive and pip re-resolves against PyPI: the + # wheelhouse is built from `uv export --frozen`, but pip would happily pick a newer sqlmodel + # that widens its SQLAlchemy range, and the image then ships versions the lock never saw. + && pip3 install --no-index --find-links=/tmp/wheelhouse/ "$WHEELFILE"[all] \ && rm -rf /tmp/wheelhouse/ \ # Do whatever else you need to && true diff --git a/pyproject.toml b/pyproject.toml index e4e8dd7c..d26ffd78 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "rasenmaeher_api" -version = "1.19.4+261027" +version = "1.20.0+260912" description = "python-rasenmaeher-api" authors = [ { name = "Aciid", email = "703382+Aciid@users.noreply.github.com" }, diff --git a/src/rasenmaeher_api/__init__.py b/src/rasenmaeher_api/__init__.py index 0301e277..89c55d91 100644 --- a/src/rasenmaeher_api/__init__.py +++ b/src/rasenmaeher_api/__init__.py @@ -1,3 +1,3 @@ """python-rasenmaeher-api""" -__version__ = "1.19.4+261027" # NOTE Use `bump-my-version` to bump versions correctly +__version__ = "1.20.0+260912" # NOTE Use `bump-my-version` to bump versions correctly diff --git a/src/rasenmaeher_api/db/__init__.py b/src/rasenmaeher_api/db/__init__.py index 10e14b4f..3c98a7e8 100644 --- a/src/rasenmaeher_api/db/__init__.py +++ b/src/rasenmaeher_api/db/__init__.py @@ -5,6 +5,7 @@ from .engine import EngineWrapper from .enrollments import Enrollment, EnrollmentPool, EnrollmentState from .logincodes import LoginCode +from .mdmenrolment import MdmEnrolment from .nonces import SeenToken from .people import Person, Role @@ -15,6 +16,7 @@ "EnrollmentPool", "EnrollmentState", "LoginCode", + "MdmEnrolment", "Person", "Role", "SeenToken", diff --git a/src/rasenmaeher_api/db/config.py b/src/rasenmaeher_api/db/config.py index aaf1a54b..9b69e69d 100644 --- a/src/rasenmaeher_api/db/config.py +++ b/src/rasenmaeher_api/db/config.py @@ -23,8 +23,13 @@ class DBConfig: """DB config dataclass, functools etc used to avoid import-time side-effects""" + # Name the DBAPI rather than leaving it to SQLAlchemy's default for postgresql://, which moved + # from psycopg2 to psycopg3 in SQLAlchemy 2.1. We ship psycopg2, so the bare name stops + # resolving the moment anything builds us against 2.1. driver: str = field( - default_factory=cast(Callable[..., str], functools.partial(config, "RM_DATABASE_DRIVER", default="postgresql")) + default_factory=cast( + Callable[..., str], functools.partial(config, "RM_DATABASE_DRIVER", default="postgresql+psycopg2") + ) ) host: str | None = field(default_factory=functools.partial(config, "RM_DATABASE_HOST", default=None)) port: int = field( diff --git a/src/rasenmaeher_api/db/dbinit.py b/src/rasenmaeher_api/db/dbinit.py index 5afbaca9..194d22a5 100644 --- a/src/rasenmaeher_api/db/dbinit.py +++ b/src/rasenmaeher_api/db/dbinit.py @@ -17,10 +17,11 @@ from .enrollments import Enrollment, EnrollmentPool from .issuedcerts import IssuedCert from .logincodes import LoginCode +from .mdmenrolment import MdmEnrolment from .nonces import SeenToken from .people import Person, Role -_ = (Person, Role, EnrollmentPool, Enrollment, SeenToken, LoginCode, IssuedCert) +_ = (Person, Role, EnrollmentPool, Enrollment, SeenToken, LoginCode, IssuedCert, MdmEnrolment) LOGGER = logging.getLogger(__name__) diff --git a/src/rasenmaeher_api/db/enrollments.py b/src/rasenmaeher_api/db/enrollments.py index a07b02c4..ddd73d04 100644 --- a/src/rasenmaeher_api/db/enrollments.py +++ b/src/rasenmaeher_api/db/enrollments.py @@ -19,7 +19,7 @@ from .base import ORMBaseModel from .engine import EngineWrapper from .errors import CallsignReserved, Deleted, ForbiddenOperation, NotFound, PoolInactive -from .people import Person +from .people import Person, reserved_callsign LOGGER = logging.getLogger(__name__) CODE_ALPHABET = string.ascii_uppercase + string.digits @@ -281,8 +281,9 @@ async def create_for_callsign( csr: str | None = None, ) -> "Enrollment": """Create a new one with random code for the callsign""" - if callsign in RMSettings.singleton().valid_product_cns: - raise CallsignReserved("Using product CNs as callsigns is forbidden") + reason = reserved_callsign(callsign) + if reason: + raise CallsignReserved(reason) if csr and not verify_csr(csr, callsign): raise CallsignReserved("CSR CN must match callsign") with EngineWrapper.get_session() as session: diff --git a/src/rasenmaeher_api/db/mdmenrolment.py b/src/rasenmaeher_api/db/mdmenrolment.py new file mode 100644 index 00000000..abcdb940 --- /dev/null +++ b/src/rasenmaeher_api/db/mdmenrolment.py @@ -0,0 +1,91 @@ +"""Whether devices may enrol through an MDM right now, and the challenge they must bring""" + +import datetime +import hmac +import logging +import secrets + +import sqlalchemy as sa +from sqlalchemy.exc import IntegrityError +from sqlmodel import Field, select + +from .base import ORMBaseModel +from .engine import EngineWrapper +from .errors import ForbiddenOperation + +LOGGER = logging.getLogger(__name__) + + +def new_challenge() -> str: + """Hex, because the challenge travels as an X.509 attribute and some clients encode it as a + PrintableString, which has no room for "_".""" + return secrets.token_hex(16) + + +class MdmEnrolment(ORMBaseModel, table=True): + """The one row saying when the MDM enrolment window closes + + Closed until an admin opens it, and it closes itself when the window runs out, so a deployment + nobody is watching does not go on taking enrolments. Every window gets a new challenge: while + it is open the challenge is all that stands between the public SCEP endpoint and a new + identity, and every device enrolled so far holds the old one. So opening means putting the new + challenge in the MDM. + """ + + __tablename__ = "mdmenrolment" + + # One row, and the unique key makes the database refuse a second one when workers race to + # create it. + slot: str = Field(default="mdm", nullable=False, unique=True) + challenge: str = Field(nullable=False) + open_until: datetime.datetime | None = Field( + default=None, sa_column=sa.Column(sa.DateTime(timezone=True), nullable=True) + ) + + @classmethod + async def current(cls) -> "MdmEnrolment": + """The row, made with a fresh challenge and closed the first time anybody asks""" + with EngineWrapper.get_session() as session: + obj = session.exec(select(cls)).first() + if obj: + return obj + obj = cls(challenge=new_challenge()) + session.add(obj) + try: + session.commit() + except IntegrityError: + session.rollback() + return session.exec(select(cls)).one() + session.refresh(obj) + return obj + + @property + def is_open(self) -> bool: + """Is the window open right now""" + return self.open_until is not None and self.open_until > datetime.datetime.now(datetime.UTC) + + def accepts(self, challenge: str | None) -> bool: + """Is this the challenge, compared without leaking how much of it matched""" + if not challenge: + return False + return hmac.compare_digest(challenge.encode("utf-8"), self.challenge.encode("utf-8")) + + async def open_for(self, hours: int) -> "MdmEnrolment": + """Open the window for this many hours from now, or close it with 0 + + Opening a closed window makes a new challenge. Extending an open one keeps it, because the + devices the MDM is enrolling at that moment were handed it. + """ + until = datetime.datetime.now(datetime.UTC) + datetime.timedelta(hours=hours) if hours else None + with EngineWrapper.get_session() as session: + if until and not self.is_open: + self.challenge = new_challenge() + self.open_until = until + session.add(self) + session.commit() + session.refresh(self) + return self + + async def delete(self) -> bool: + """The row is the setting; closing the window is what turns enrolment off""" + raise ForbiddenOperation("Deletion of the MDM enrolment setting is not allowed") diff --git a/src/rasenmaeher_api/db/people.py b/src/rasenmaeher_api/db/people.py index 086c3ffc..d9f88bc5 100644 --- a/src/rasenmaeher_api/db/people.py +++ b/src/rasenmaeher_api/db/people.py @@ -33,6 +33,25 @@ LOGGER = logging.getLogger(__name__) +def reserved_callsign(callsign: str) -> str | None: + """Why this callsign belongs to a service rather than a person, or None if it does not + + Compared without case: lookups fold it, and Keycloak and Matrix lowercase the name, so + RASENMAEHER is the same account as rasenmaeher everywhere downstream. + """ + cnf = RMSettings.singleton() + folded = callsign.lower() + if folded in {cnname.lower() for cnname in cnf.valid_product_cns}: + return "Using product CNs as callsigns is forbidden" + if folded == cnf.mtls_client_cert_cn.lower(): + return "Using the rasenmaeher service CN as a callsign is forbidden" + if folded in {cnname.lower() for cnname in cnf.mdm_agent_cn_set}: + # A certificate with this CN is accepted as the MDM enrolment agent on sight, so issuing + # one to a person would let them enrol devices. + return "Using MDM agent CNs as callsigns is forbidden" + return None + + class Person(ORMBaseModel, table=True): """People, pk is UUID and comes from basemodel @@ -99,10 +118,9 @@ async def create_with_cert( if csrpem and not verify_csr(csrpem, callsign): raise CallsignReserved("CSR CN must match callsign") cnf = RMSettings.singleton() - if callsign in cnf.valid_product_cns: - raise CallsignReserved("Using product CNs as callsigns is forbidden") - if callsign == cnf.mtls_client_cert_cn: - raise CallsignReserved("Using the rasenmaeher service CN as a callsign is forbidden") + reason = reserved_callsign(callsign) + if reason: + raise CallsignReserved(reason) try: await Person.by_callsign(callsign) raise CallsignReserved() diff --git a/src/rasenmaeher_api/rmsettings.py b/src/rasenmaeher_api/rmsettings.py index 00b4cdeb..ec4887a9 100644 --- a/src/rasenmaeher_api/rmsettings.py +++ b/src/rasenmaeher_api/rmsettings.py @@ -113,6 +113,18 @@ def log_level_int(self) -> int: ocsp_ca_secret_namespace: str = "cert-manager" ocsp_response_validity: int = 3600 # seconds from now until nextUpdate + # Shared secret used by the Traefik callsign-validity plugin to auth to the + # internal websocket. Optional — if unset, the websocket accepts any caller + # (suitable for in-cluster-only Service exposure during local dev). + callsign_validity_secret: str | None = None + + # Service identities allowed to complete device enrolments an admin planned with mdm=true, + # comma separated. Empty by default: until a deployment names its MDM agent here, the agent + # path in POST /enrollment/accept does not exist. Deliberately NOT the kraftwerk product CNs: + # those are exempt from role checks, can have any CSR signed and cannot be revoked at the + # edge, which is far more than completing a planned enrolment needs. + mdm_agent_cns: str = "" + persistent_data_dir: str = "/data/persistent" # mtls @@ -177,6 +189,11 @@ def deployment_name(self) -> str: LOGGER.warning("DNS name not defined") return "undefined" + @property + def mdm_agent_cn_set(self) -> set[str]: + """CNs allowed to act as an MDM enrolment agent""" + return {cnname.strip() for cnname in self.mdm_agent_cns.split(",") if cnname.strip()} + @property def valid_product_cns(self) -> list[str]: """Get valid CNs for productapi certs""" diff --git a/src/rasenmaeher_api/web/api/enrollment/schema.py b/src/rasenmaeher_api/web/api/enrollment/schema.py index b1fd2d91..50ff86fc 100644 --- a/src/rasenmaeher_api/web/api/enrollment/schema.py +++ b/src/rasenmaeher_api/web/api/enrollment/schema.py @@ -1,7 +1,9 @@ """Schema for enrollment.""" +import datetime from typing import Any +from libpvarki.schemas.generic import OperationResultResponse from pydantic import BaseModel, ConfigDict, Field @@ -191,7 +193,20 @@ class EnrollmentAcceptIn(BaseModel): callsign: str = Field(description="Callsign to approve") approvecode: str = Field( - description="Approval code for the callsign, this must have been delivered by the person to be enrolled" + description=( + "Approval code for the callsign, this must have been delivered by the person to be enrolled. " + "Not used when the MDM agent enrols a device: there is no human to deliver it, " + "and the agent has authenticated with its own certificate." + ), + default="", + ) + csr: str | None = Field( + description=("CSR the device generated, PEM, its CN the name the MDM gave it. Only for the MDM agent."), + default=None, + ) + challenge: str | None = Field( + description="The SCEP challenge the device sent. Only for an MDM agent, which passes it on unchecked.", + default=None, ) @@ -210,6 +225,20 @@ class EnrollmentAcceptOut(BaseModel): callsign: str +class EnrollmentAcceptResultOut(OperationResultResponse): + """Result of accepting an enrollment + + Additive superset of the plain result: an MDM agent gets the issued certificate back here + rather than fetching it afterwards. It has no user credential to fetch it with, and in a + meshed deployment its own service identity would win over any bearer token anyway. + """ + + certificate: str | None = Field( + description="Issued certificate, PEM. Only set when an MDM agent completed the enrolment.", + default=None, + ) + + class EnrollmentPromoteIn(BaseModel): """Enrollment promote in schema""" @@ -433,3 +462,22 @@ class EnrollmentInviteCodeDeleteIn(BaseModel): ) invite_code: str + + +class MdmSettingsOut(BaseModel): + """What an operator configures the MDM with, and whether devices may enrol now""" + + #: Where the MDM sends each device's certificate request + scep_url: str + #: The SCEP challenge, new for every window. Keep it to the MDM + challenge: str + #: When the enrolment window closes, or None while it is closed + open_until: datetime.datetime | None + + +class MdmSettingsIn(BaseModel): + """Open the enrolment window for some hours, or close it""" + + model_config = ConfigDict(extra="forbid") + + hours: int = Field(description="Hours to keep enrolment open from now, 0 closes it", ge=0, le=168) diff --git a/src/rasenmaeher_api/web/api/enrollment/views.py b/src/rasenmaeher_api/web/api/enrollment/views.py index 4117b4a2..8f35f4af 100644 --- a/src/rasenmaeher_api/web/api/enrollment/views.py +++ b/src/rasenmaeher_api/web/api/enrollment/views.py @@ -1,22 +1,30 @@ """Enrollment API views.""" import logging +import re import uuid from typing import Any +from cryptography import x509 +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import ec, ed448, ed25519, rsa +from cryptography.x509.oid import ExtendedKeyUsageOID from fastapi import APIRouter, Body, Depends, HTTPException, Request, Response from libpvarki.schemas.generic import OperationResultResponse from multikeyjwt import Issuer from multikeyjwt import config as jwtconfig -from ....db import Enrollment, EnrollmentPool, Person -from ....db.errors import NotFound +from ....db import Enrollment, EnrollmentPool, MdmEnrolment, Person +from ....db.errors import CallsignReserved, Deleted, NotFound from ....rmsettings import RMSettings +from ..middleware.datatypes import MTLSorJWTPayloadType from ..middleware.mtls import MTLSorJWT from ..middleware.user import ValidUser from ..utils.auditcontext import build_audit_extra +from ..utils.csr_utils import verify_csr from .schema import ( EnrollmentAcceptIn, + EnrollmentAcceptResultOut, EnrollmentDemoteIn, EnrollmentGenVerifiOut, EnrollmentHaveIBeenAcceptedOut, @@ -37,9 +45,15 @@ EnrollmentShowVerificationCodeOut, EnrollmentStatusIn, EnrollmentStatusOut, + MdmSettingsIn, + MdmSettingsOut, ) LOGGER = logging.getLogger(__name__) +#: The name an MDM gives a device becomes its callsign in every product, so it has to be one they all +#: take. Hyphens and 32 characters fit the enrollment-specific ID a work-profile phone reports in place +#: of its serial; no hyphen at either end, so a name never reads as a command-line flag. +MDM_CALLSIGN = re.compile(r"[A-Za-z0-9][A-Za-z0-9-]{1,30}[A-Za-z0-9]") ENROLLMENT_ROUTER = APIRouter(dependencies=[Depends(MTLSorJWT(auto_error=True))]) NO_JWT_ENROLLMENT_ROUTER = APIRouter() @@ -397,23 +411,120 @@ async def request_enrollment_lock( return OperationResultResponse(success=True, extra="Lock task done") +# One detail for every MDM refusal. The agent sees only "no", so a caller holding the agent +# certificate cannot use the reply to learn which callsigns exist. The real reason goes to the +# audit log. +MDM_REFUSED = "Not a device enrolment this agent may complete" + + +def mdm_agent_cn(request: Request) -> str | None: + """The caller's CN if it is a configured MDM enrolment agent, otherwise None + + The agent authenticates with its own client certificate from the deployment CA -- in a meshed + deployment, with its mesh identity, which reaches us through the same header. It is not a + Person and holds no role. The single thing this CN may do is enrol a device while an admin has + MDM enrolment open. + """ + payload = getattr(request.state, "mtls_or_jwt", None) + if not payload or payload.type != MTLSorJWTPayloadType.MTLS or not payload.userid: + return None + if payload.userid not in RMSettings.singleton().mdm_agent_cn_set: + return None + return str(payload.userid) + + +def device_csr_overreach(csrpem: str) -> str | None: + """Is this request asking for more than a device identity, or None if it is not + + The signer picks its profile from what the request asks for, so a request that asks for server + authentication is issued one, and nothing replaces the names it carries. On this path the + request arrived from the public SCEP endpoint and its contents are entirely the caller's + choice, so the shape of what gets issued cannot be one of the things they choose. + + A genuine device request from an MDM asks for none of this: it carries a subject and a public + key and nothing else. + """ + try: + csr = x509.load_pem_x509_csr(csrpem.encode("utf-8")) + except ValueError as exc: + return f"the request does not parse: {exc}" + try: + usages = csr.extensions.get_extension_for_class(x509.ExtendedKeyUsage).value + if [usage for usage in usages if usage != ExtendedKeyUsageOID.CLIENT_AUTH]: + return "the request asks for a key usage beyond client authentication" + except x509.ExtensionNotFound: + pass + try: + csr.extensions.get_extension_for_class(x509.SubjectAlternativeName) + return "the request carries subject alternative names" + except x509.ExtensionNotFound: + pass + try: + basic = csr.extensions.get_extension_for_class(x509.BasicConstraints).value + if basic.ca: + return "the request asks to be a certificate authority" + except x509.ExtensionNotFound: + pass + return None + + +def weak_device_key(csrpem: str) -> bool: + """Is this a key whose private half anybody could recover: RSA under 2048 bits, a curve under 256 + + Whoever recovers it is the device, for as long as the certificate lasts. + """ + key = x509.load_pem_x509_csr(csrpem.encode("utf-8")).public_key() + if isinstance(key, rsa.RSAPublicKey): + return key.key_size < 2048 + if isinstance(key, ec.EllipticCurvePublicKey): + return key.curve.key_size < 256 + return not isinstance(key, (ed25519.Ed25519PublicKey, ed448.Ed448PublicKey)) + + +def same_public_key(csrpem: str, certpem: str) -> bool: + """Does this CSR carry the key that was certified""" + csr = x509.load_pem_x509_csr(csrpem.encode("utf-8")) + cert = x509.load_pem_x509_certificate(certpem.encode("utf-8")) + encoding, fmt = serialization.Encoding.DER, serialization.PublicFormat.SubjectPublicKeyInfo + return csr.public_key().public_bytes(encoding, fmt) == cert.public_key().public_bytes(encoding, fmt) + + @ENROLLMENT_ROUTER.post( "/accept", - response_model=OperationResultResponse, - dependencies=[Depends(ValidUser(auto_error=True, require_roles=["admin"]))], + response_model=EnrollmentAcceptResultOut, ) async def post_enrollment_accept( request: Request, request_in: EnrollmentAcceptIn = Body(), -) -> OperationResultResponse: +) -> EnrollmentAcceptResultOut: """ Accept callsign_hash (callsign/enrollment) + + Two kinds of caller. An admin approving a person by hand, which is unchanged. Or the MDM + enrolment agent enrolling a device, supplying the CSR the device generated. Authorisation for + the second is the agent's own certificate, checked here rather than inferred from where the + request came from. """ + agent_cn = mdm_agent_cn(request) + if agent_cn: + return await accept_as_mdm_agent(request, request_in, agent_cn) + admin_user = await ValidUser(auto_error=True, require_roles=["admin"])(request) + if not admin_user: + # ValidUser returns None without a Person for product certificates; they have no business + # approving enrollments and used to fall through to a confusing 404. + raise HTTPException(status_code=403, detail="Not authenticated") + return await accept_as_admin(request, request_in, admin_user) + + +async def accept_as_admin( + request: Request, + request_in: EnrollmentAcceptIn, + admin_user: Person, +) -> EnrollmentAcceptResultOut: + """An admin approves an enrollment by hand, with the code the enrollee gave them""" target_callsign = request_in.callsign - actor = request.state.mtls_or_jwt.userid try: - admin_user = await Person.by_callsign(callsign=actor) pending_enrollment = await Enrollment.by_callsign(callsign=target_callsign) except NotFound as exc: LOGGER.audit( # type: ignore[attr-defined] @@ -453,7 +564,171 @@ async def post_enrollment_accept( ), ) - return OperationResultResponse(success=True, extra=f"Approved {new_approved_user.callsign}") + return EnrollmentAcceptResultOut(success=True, extra=f"Approved {new_approved_user.callsign}") + + +async def accept_as_mdm_agent( + request: Request, + request_in: EnrollmentAcceptIn, + agent_cn: str, +) -> EnrollmentAcceptResultOut: + """The MDM agent enrols a device under the name its MDM gave it + + The MDM decides who a device is: the common name of the device's own request becomes its + callsign. What stands between the public SCEP endpoint and a new identity is the window an + admin opened and the challenge, so keep the window short. A name already in use is never handed + to a second key, and everything is checked before anything is written. + """ + target_callsign = request_in.callsign + + def refuse(status_code: int, error_code: str, detail: str = MDM_REFUSED) -> HTTPException: + LOGGER.audit( # type: ignore[attr-defined] + "MDM enrollment refused", + extra=build_audit_extra( + action="enrollment_mdm_accept", + outcome="failure", + actor=agent_cn, + target=target_callsign, + request=request, + error_code=error_code, + ), + ) + return HTTPException(status_code=status_code, detail=detail) + + # First, so that while enrolment is closed nothing behind it can be probed. + enrolment = await MdmEnrolment.current() + if not enrolment.is_open: + raise refuse(403, "MDM_CLOSED") + if not enrolment.accepts(request_in.challenge): + raise refuse(403, "WRONG_CHALLENGE") + + csrpem = request_in.csr + if not csrpem: + raise refuse(400, "CSR_MISSING", "The device CSR is required") + if not MDM_CALLSIGN.fullmatch(target_callsign): + raise refuse(403, "CALLSIGN_FORMAT") + if not verify_csr(csrpem, target_callsign): + raise refuse(403, "CSR_SUBJECT") + if device_csr_overreach(csrpem): + raise refuse(403, "CSR_OVERREACH") + if weak_device_key(csrpem): + raise refuse(403, "CSR_WEAK_KEY") + + try: + existing = await Person.by_callsign(target_callsign, allow_deleted=True) + except NotFound: + existing = None + if existing: + return await mdm_repeat(csrpem, existing, refuse) + try: + await Enrollment.by_callsign(target_callsign) + except (NotFound, Deleted): + pass + else: + # A person enrolled, or is waiting for an admin, under this name; taking it would strand them. + raise refuse(403, "CALLSIGN_PENDING") + + try: + person = await Person.create_with_cert(target_callsign, extra={"mdm": True}, csrpem=csrpem) + except CallsignReserved as exc: + raise refuse(403, "CALLSIGN_RESERVED") from exc + + LOGGER.audit( # type: ignore[attr-defined] + "Device enrolled by MDM agent", + extra=build_audit_extra( + action="enrollment_mdm_accept", + outcome="success", + actor=agent_cn, + target=target_callsign, + request=request, + ), + ) + return EnrollmentAcceptResultOut( + success=True, + extra=f"Approved {person.callsign}", + certificate=person.certfile.read_text(encoding="utf-8"), + ) + + +async def mdm_repeat(csrpem: str, person: Person, refuse: Any) -> EnrollmentAcceptResultOut: + """The agent asked for a name that is already somebody's + + Almost always the reply was lost on the way back and the MDM repeated the identical request, + so hand the same certificate over again -- it is public, and the alternative is a device + stranded behind "callsign already taken". Any other key is somebody else. + """ + if person.deleted: + raise refuse(403, "CALLSIGN_REVOKED") + if not person.certfile.exists(): + # The Person row is committed before the CA is called and that commit cannot be rolled + # back, so a CA failure at just the wrong moment leaves the name unusable for good. + raise refuse(409, "CALLSIGN_SPENT", "That name was spent by a failed issue, give the device another") + certpem = person.certfile.read_text(encoding="utf-8") + if not same_public_key(csrpem, certpem): + raise refuse(403, "CALLSIGN_TAKEN") + return EnrollmentAcceptResultOut( + success=True, + extra=f"Approved {person.callsign}", + certificate=certpem, + ) + + +async def human_admin(request: Request) -> Person: + """An admin who is a person + + ValidUser lets a product certificate through without a Person or a role check. Products have + no business switching device enrolment: that is the operator's decision. + """ + admin = await ValidUser(auto_error=True, require_roles=["admin"])(request) + if not admin: + raise HTTPException(status_code=403, detail="Not authenticated") + return admin + + +def _mdm_settings(request: Request, enrolment: MdmEnrolment) -> MdmSettingsOut: + host = (request.url.hostname or "").removeprefix("mtls.") + return MdmSettingsOut( + scep_url=f"https://{host}/scep" if host else "", + challenge=enrolment.challenge, + open_until=enrolment.open_until if enrolment.is_open else None, + ) + + +@ENROLLMENT_ROUTER.get( + "/mdm-settings", + response_model=MdmSettingsOut, + dependencies=[Depends(human_admin)], +) +async def request_mdm_settings(request: Request) -> MdmSettingsOut: + """What to configure the MDM with, and until when devices may enrol through it + + Admin only: while the window is open, the challenge is what lets a device join. + """ + return _mdm_settings(request, await MdmEnrolment.current()) + + +@ENROLLMENT_ROUTER.put("/mdm-settings", response_model=MdmSettingsOut) +async def put_mdm_settings( + request: Request, + request_in: MdmSettingsIn = Body(), + admin: Person = Depends(human_admin), +) -> MdmSettingsOut: + """Open MDM enrolment for some hours, or close it with 0 + + It closes itself when the time runs out, so a forgotten window does not stay open. + """ + enrolment = await (await MdmEnrolment.current()).open_for(request_in.hours) + LOGGER.audit( # type: ignore[attr-defined] + "MDM enrolment opened" if request_in.hours else "MDM enrolment closed", + extra=build_audit_extra( + action="mdm_enrolment_window", + outcome="success", + actor=admin.callsign, + request=request, + hours=request_in.hours, + ), + ) + return _mdm_settings(request, enrolment) @ENROLLMENT_ROUTER.post( diff --git a/src/rasenmaeher_api/web/api/utils/csr_utils.py b/src/rasenmaeher_api/web/api/utils/csr_utils.py index 836ff504..184b8015 100644 --- a/src/rasenmaeher_api/web/api/utils/csr_utils.py +++ b/src/rasenmaeher_api/web/api/utils/csr_utils.py @@ -3,6 +3,7 @@ import logging from cryptography import x509 +from cryptography.x509.oid import NameOID from libadvian.binpackers import ensure_utf8 LOGGER = logging.getLogger(__name__) @@ -12,12 +13,32 @@ def verify_csr(csrpem: str, callsign: str) -> bool: - """Verify CSR matches our rules for CN/DN for the given callsign""" + """Verify CSR matches our rules for CN/DN for the given callsign + + Two properties are checked, and both are load-bearing: + + 1. The CSR is signed by the key it carries. Without this we have no proof that whoever + submitted the request holds the private key, and a request we cannot vouch for would be + passed to the CA anyway -- which answers with an error *after* the Person row has been + committed, permanently spending the callsign. + 2. The subject carries exactly one CN and it equals the callsign. This used to be a substring + test on the rendered RFC4514 string, which accepted a great deal more than it looks like: + ``CN=OTTER1X`` matched callsign ``OTTER1``, and ``OU=CN=OTTER1,CN=ADMIN`` matched it too + while the certificate the CA then issued carried ``CN=ADMIN``. The mTLS edge authorises on + the CN it reads from the certificate, so that is an identity mismatch, not a cosmetic one. + + Attributes other than CN are left alone on purpose: an MDM filling the subject template of a + device may add O, OU or ST, nothing downstream reads them, and refusing them would break + enrolment for no gain. + """ csr = x509.load_pem_x509_csr(ensure_utf8(csrpem)) - dn = csr.subject.rfc4514_string() - LOGGER.debug(f"DN={dn} callsign={callsign}") - if f"CN={callsign}" not in dn: - LOGGER.warning(f"Callsign does not match CSR subject. DN={dn} callsign={callsign}") + if not csr.is_signature_valid: + LOGGER.warning(f"CSR signature does not verify. callsign={callsign}") + return False + common_names = [attr.value for attr in csr.subject.get_attributes_for_oid(NameOID.COMMON_NAME)] + LOGGER.debug(f"CNs={common_names} callsign={callsign}") + if common_names != [callsign]: + LOGGER.warning(f"Callsign does not match CSR subject. CNs={common_names} callsign={callsign}") return False # TODO: check that keyusages in the CSR are fine # crypto.X509Extension(b"keyUsage", True, b"digitalSignature,nonRepudiation,keyEncipherment"), diff --git a/tests/test_console.py b/tests/test_console.py index a82b2c7c..023182b9 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -8,6 +8,11 @@ from rasenmaeher_api import __version__ +# Each case boots the whole app in a subprocess, which on a loaded shared runner is nowhere near +# the ~2s it takes on a developer machine. Generous on purpose: the bound is here to stop a hung +# subprocess holding the suite, not to assert anything about startup time. +CLI_TIMEOUT = 60 + @pytest.mark.asyncio(loop_scope="session") async def test_version_cli() -> None: @@ -18,7 +23,7 @@ async def test_version_cli() -> None: stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) - out = await asyncio.wait_for(process.communicate(), 15) + out = await asyncio.wait_for(process.communicate(), CLI_TIMEOUT) # Demand clean exit assert process.returncode == 0 # Check output @@ -35,7 +40,7 @@ async def test_addcode_cli() -> None: stderr=asyncio.subprocess.PIPE, env=os.environ, ) - out = await asyncio.wait_for(process.communicate(), 15) + out = await asyncio.wait_for(process.communicate(), CLI_TIMEOUT) # Demand clean exit assert process.returncode == 0 # Check output diff --git a/tests/test_csr_utils.py b/tests/test_csr_utils.py new file mode 100644 index 00000000..30cc9c6a --- /dev/null +++ b/tests/test_csr_utils.py @@ -0,0 +1,96 @@ +"""Unit tests for CSR verification + +Build certificate requests in memory and check what +``verify_csr`` accepts. +""" + +import logging + +import pytest +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec, rsa +from cryptography.x509.oid import NameOID + +from rasenmaeher_api.web.api.utils.csr_utils import verify_csr + +LOGGER = logging.getLogger(__name__) + + +def _csr(subject: x509.Name, key: object | None = None) -> str: + """A signed CSR for the given subject, EC by default""" + signing_key = key or ec.generate_private_key(ec.SECP256R1()) + csr = x509.CertificateSigningRequestBuilder().subject_name(subject).sign(signing_key, hashes.SHA256()) + return csr.public_bytes(serialization.Encoding.PEM).decode("utf-8") + + +def _cn(value: str) -> x509.NameAttribute: + return x509.NameAttribute(NameOID.COMMON_NAME, value) + + +def test_exact_common_name_passes() -> None: + """The normal case""" + assert verify_csr(_csr(x509.Name([_cn("OTTER1")])), "OTTER1") + + +def test_rsa_key_passes() -> None: + """Key type is not what is being checked -- RSA is still what SCEP devices mostly send""" + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + assert verify_csr(_csr(x509.Name([_cn("OTTER1")]), key), "OTTER1") + + +def test_extra_non_cn_attributes_pass() -> None: + """An MDM may fill O/OU/ST from its own template and nothing downstream reads them""" + subject = x509.Name( + [ + x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Example"), + x509.NameAttribute(NameOID.ORGANIZATIONAL_UNIT_NAME, "Devices"), + _cn("OTTER1"), + ] + ) + assert verify_csr(_csr(subject), "OTTER1") + + +def test_longer_common_name_is_refused() -> None: + """``CN=OTTER1X`` contains ``CN=OTTER1`` as a substring, which used to be enough""" + assert not verify_csr(_csr(x509.Name([_cn("OTTER1X")])), "OTTER1") + + +def test_second_common_name_is_refused() -> None: + """The escalation: the issued certificate would carry ADMIN, the Person would be OTTER1""" + subject = x509.Name([_cn("ADMIN"), _cn("OTTER1")]) + assert not verify_csr(_csr(subject), "OTTER1") + + +def test_callsign_smuggled_into_another_attribute_is_refused() -> None: + """``OU=CN=OTTER1,CN=ADMIN`` renders a DN containing ``CN=OTTER1`` while the CN is ADMIN""" + subject = x509.Name( + [ + x509.NameAttribute(NameOID.ORGANIZATIONAL_UNIT_NAME, "CN=OTTER1"), + _cn("ADMIN"), + ] + ) + rendered = x509.load_pem_x509_csr(_csr(subject).encode("utf-8")).subject.rfc4514_string() + assert "CN=OTTER1" in rendered, "precondition: the old substring test would have accepted this" + assert not verify_csr(_csr(subject), "OTTER1") + + +def test_subject_without_common_name_is_refused() -> None: + """An empty callsign used to match ``CN=`` in almost anything""" + subject = x509.Name([x509.NameAttribute(NameOID.ORGANIZATION_NAME, "Example")]) + assert not verify_csr(_csr(subject), "") + + +def test_unsigned_csr_is_refused() -> None: + """Proof of possession: the request must be signed by the key it carries""" + pem = _csr(x509.Name([_cn("OTTER1")])) + csr = x509.load_pem_x509_csr(pem.encode("utf-8")) + der = bytearray(csr.public_bytes(serialization.Encoding.DER)) + der[-1] ^= 0xFF # break the signature, keep the structure parseable + try: + broken = x509.load_der_x509_csr(bytes(der)) + except ValueError: + pytest.skip("mangled DER no longer parses, cannot exercise the signature path this way") + broken_pem = broken.public_bytes(serialization.Encoding.PEM).decode("utf-8") + assert not broken.is_signature_valid, "precondition: the signature must actually be invalid" + assert not verify_csr(broken_pem, "OTTER1") diff --git a/tests/test_enrollment.py b/tests/test_enrollment.py index f9ae9df7..eda26611 100644 --- a/tests/test_enrollment.py +++ b/tests/test_enrollment.py @@ -1,22 +1,33 @@ """Test enrollment endpoint""" +import datetime import logging +import re import secrets import uuid +from collections.abc import AsyncGenerator from pathlib import Path from typing import Any import cryptography.hazmat.primitives.serialization.pkcs12 import pytest +import pytest_asyncio from async_asgi_testclient import TestClient # type: ignore[import-untyped] from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509.oid import ExtendedKeyUsageOID, NameOID +from fastapi import FastAPI from libpvarki.mtlshelp.csr import async_create_client_csr, async_create_keypair from rasenmaeher_api.db import ( EngineWrapper, + Enrollment, EnrollmentPool, + MdmEnrolment, Person, ) +from rasenmaeher_api.db.errors import CallsignReserved, NotFound from rasenmaeher_api.rmsettings import RMSettings LOGGER = logging.getLogger(__name__) @@ -730,3 +741,322 @@ async def test_enrollmentpools_revoked_creator(dbinit_func, tilauspalvelu_jwt_ad found = True break assert found + + +MDM_AGENT_CN = "rmscep-test" + + +@pytest_asyncio.fixture(scope="function") +async def mdm_agent_client(app_instance: FastAPI) -> AsyncGenerator[TestClient, None]: + """Client authenticating as the MDM enrollment agent + + The agent holds its own client certificate from the deployment CA; in a real deployment the + front proxy puts its DN in this header exactly like it does for a browser. + """ + settings = RMSettings.singleton() + previous = settings.mdm_agent_cns + settings.mdm_agent_cns = MDM_AGENT_CN + async with TestClient(app_instance) as instance: + instance.headers.update({"X-ClientCert-DN": f"CN={MDM_AGENT_CN},O=N/A"}) + yield instance + settings.mdm_agent_cns = previous + + +@pytest_asyncio.fixture(scope="function") +async def mdm_challenge(mdm_agent_client: TestClient) -> str: + """Open MDM enrolment for the test, and the challenge the agent passes on from the device""" + _ = mdm_agent_client + return (await (await MdmEnrolment.current()).open_for(1)).challenge + + +async def _device_csr(tempdir: Path, callsign: str, name: str = "device") -> str: + """A CSR the way a device sends one through its MDM: its own key, the name the MDM gave it as the CN""" + privkeyfile = Path(tempdir) / f"{name}.key" + pubkeyfile = Path(tempdir) / f"{name}.pub" + csrfile = Path(tempdir) / f"{name}.csr" + ckp = await async_create_keypair(privkeyfile, pubkeyfile) + return str(await async_create_client_csr(ckp, csrfile, {"CN": callsign})) + + +def _public_key_der(pem: str) -> bytes: + """SubjectPublicKeyInfo of whatever this PEM is, certificate or request""" + encoding, fmt = serialization.Encoding.DER, serialization.PublicFormat.SubjectPublicKeyInfo + if "CERTIFICATE REQUEST" in pem: + return x509.load_pem_x509_csr(pem.encode("utf-8")).public_key().public_bytes(encoding, fmt) + return x509.load_pem_x509_certificate(pem.encode("utf-8")).public_key().public_bytes(encoding, fmt) + + +async def _mdm_enrol(client: TestClient, callsign: str, csrpem: str | None, challenge: str | None) -> Any: + body: dict[str, Any] = {"callsign": callsign, "challenge": challenge} + if csrpem is not None: + body["csr"] = csrpem + return await client.post("/api/v1/enrollment/accept", json=body) + + +async def _no_person(callsign: str) -> bool: + try: + await Person.by_callsign(callsign, allow_deleted=True) + except NotFound: + return True + return False + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_agent_enrols_a_device_under_its_mdm_name( + mdm_agent_client: TestClient, mdm_challenge: str, nice_tmpdir: str +) -> None: + """The whole agent path: the MDM names the device, and a repeat gets the same certificate""" + tempdir = Path(nice_tmpdir) + callsign = f"R58N{secrets.token_hex(4).upper()}" + csrpem = await _device_csr(tempdir, callsign) + resp = await _mdm_enrol(mdm_agent_client, callsign, csrpem, mdm_challenge) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["success"] is True + certpem = body["certificate"] + + # The certificate must be for the key the device holds. If this ever regresses, rasenmaeher + # generated a keypair of its own and the device received a certificate it cannot use. + assert _public_key_der(certpem) == _public_key_der(csrpem) + # The mTLS edge authorises on the CN it reads off the certificate: exactly one, the MDM's name + issued = x509.load_pem_x509_certificate(certpem.encode("utf-8")) + assert [attr.value for attr in issued.subject.get_attributes_for_oid(NameOID.COMMON_NAME)] == [callsign] + person = await Person.by_callsign(callsign) + assert person.extra == {"mdm": True}, "the admin UI lists devices by this mark" + + # The reply can be lost on the way back to the MDM, which then repeats the request verbatim + resp = await _mdm_enrol(mdm_agent_client, callsign, csrpem, mdm_challenge) + assert resp.status_code == 200 + assert resp.json()["certificate"] == certpem + + # ...but a different key for the same name is somebody else + other_csr = await _device_csr(tempdir, callsign, name="imposter") + resp = await _mdm_enrol(mdm_agent_client, callsign, other_csr, mdm_challenge) + assert resp.status_code == 403 + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_agent_refusals( + tilauspalvelu_jwt_admin_client: TestClient, + mdm_agent_client: TestClient, + mdm_challenge: str, + unauth_client_session: TestClient, + nice_tmpdir: str, +) -> None: + """What the agent may not do""" + tempdir = Path(nice_tmpdir) + + # A request whose CN is not the name being enrolled + name = f"mdmmismatch{secrets.token_hex(4)}" + wrong_csr = await _device_csr(tempdir, f"{name}X", name="mismatch") + resp = await _mdm_enrol(mdm_agent_client, name, wrong_csr, mdm_challenge) + assert resp.status_code == 403 + assert await _no_person(name) + + # No CSR at all + resp = await _mdm_enrol(mdm_agent_client, name, None, mdm_challenge) + assert resp.status_code == 400 + + # And without the agent certificate the route is what it always was + resp = await unauth_client_session.post("/api/v1/enrollment/accept", json={"callsign": name, "csr": wrong_csr}) + assert resp.status_code == 403 + + # A name a person is waiting for an admin under. + # A plain init hands the caller the new callsign's JWT as a session cookie, and JWTBearer + # prefers that cookie over the Authorization header, so this client would stop being an admin. + human = f"mdmhuman{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post("/api/v1/enrollment/init", json={"callsign": human}) + assert resp.status_code == 200 + if tilauspalvelu_jwt_admin_client.cookie_jar is not None: + tilauspalvelu_jwt_admin_client.cookie_jar.clear() + resp = await _mdm_enrol(mdm_agent_client, human, await _device_csr(tempdir, human, name="human"), mdm_challenge) + assert resp.status_code == 403 + assert await _no_person(human) + + +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize( + "name", ["KÄRPPÄ1", "AB", "A" * 33, "-OTTER1", "OTTER1-", "OTTER_1", "fake.localmaeher.dev.pvarki.fi"] +) +async def test_mdm_agent_refuses_a_name_not_every_product_takes( + mdm_agent_client: TestClient, mdm_challenge: str, nice_tmpdir: str, name: str +) -> None: + """The MDM's name becomes the callsign everywhere, so it has to be one every product takes""" + resp = await _mdm_enrol(mdm_agent_client, name, await _device_csr(Path(nice_tmpdir), name), mdm_challenge) + assert resp.status_code == 403 + assert await _no_person(name) + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_agent_takes_a_work_profile_enrollment_id( + mdm_agent_client: TestClient, mdm_challenge: str, nice_tmpdir: str +) -> None: + """A work-profile phone reports no serial, only a 32-character hyphenated enrollment-specific ID""" + raw = secrets.token_hex(14).upper() + name = "-".join([raw[0:4], raw[4:8], raw[8:12], raw[12:17], raw[17:21], raw[21:25], raw[25:26]]) + assert len(name) == 32 + resp = await _mdm_enrol( + mdm_agent_client, name, await _device_csr(Path(nice_tmpdir), name, name="esid"), mdm_challenge + ) + assert resp.status_code == 200, resp.text + assert (await Person.by_callsign(name)).extra == {"mdm": True} + + +@pytest.mark.asyncio(loop_scope="session") +async def test_the_enrolment_window_and_challenge_gate_the_agent( + mdm_agent_client: TestClient, mdm_challenge: str, nice_tmpdir: str +) -> None: + """A request that would otherwise succeed is refused while closed or with the wrong challenge + + And it creates nothing, so the device can still enrol once the operator opens the window. + """ + callsign = f"mdmwindow{secrets.token_hex(4)}" + csrpem = await _device_csr(Path(nice_tmpdir), callsign, name="window") + + for wrong in (None, "", "0" * 32, f"{mdm_challenge}x"): + resp = await _mdm_enrol(mdm_agent_client, callsign, csrpem, wrong) + assert resp.status_code == 403, f"challenge {wrong!r} must be refused" + + await (await MdmEnrolment.current()).open_for(0) + resp = await _mdm_enrol(mdm_agent_client, callsign, csrpem, mdm_challenge) + assert resp.status_code == 403, "a closed window must refuse even a correct request" + assert await _no_person(callsign) + + # Reopening makes a new challenge, so one that leaked from an earlier window is worth nothing + reopened = await (await MdmEnrolment.current()).open_for(1) + assert reopened.challenge != mdm_challenge + resp = await _mdm_enrol(mdm_agent_client, callsign, csrpem, mdm_challenge) + assert resp.status_code == 403, "the previous window's challenge must be refused" + assert await _no_person(callsign) + resp = await _mdm_enrol(mdm_agent_client, callsign, csrpem, reopened.challenge) + assert resp.status_code == 200, resp.text + + +def test_the_enrolment_window_closes_itself() -> None: + """Nobody has to remember to close it""" + now = datetime.datetime.now(datetime.UTC) + assert MdmEnrolment(challenge="c", open_until=now + datetime.timedelta(minutes=1)).is_open + assert not MdmEnrolment(challenge="c", open_until=now - datetime.timedelta(seconds=1)).is_open + assert not MdmEnrolment(challenge="c").is_open + + +@pytest.mark.asyncio(loop_scope="session") +async def test_an_admin_opens_and_closes_mdm_enrolment( + tilauspalvelu_jwt_admin_client: TestClient, tilauspalvelu_jwt_user_client: TestClient, app_instance: FastAPI +) -> None: + """The operator's switch, with the challenge to configure the MDM with""" + url = "/api/v1/enrollment/mdm-settings" + resp = await tilauspalvelu_jwt_admin_client.put(url, json={"hours": 0}) + assert resp.status_code == 200 + closed = resp.json() + assert closed["open_until"] is None + assert re.fullmatch(r"[0-9a-f]{32}", closed["challenge"]) + assert closed["scep_url"].endswith("/scep") + + resp = await tilauspalvelu_jwt_admin_client.put(url, json={"hours": 24}) + assert resp.status_code == 200 + left = datetime.datetime.fromisoformat(resp.json()["open_until"]) - datetime.datetime.now(datetime.UTC) + assert datetime.timedelta(hours=23, minutes=59) < left <= datetime.timedelta(hours=24) + + opened = (await tilauspalvelu_jwt_admin_client.get(url)).json() + assert opened["challenge"] != closed["challenge"], "every window gets a new challenge" + assert re.fullmatch(r"[0-9a-f]{32}", opened["challenge"]) + assert opened["open_until"] + extended = (await tilauspalvelu_jwt_admin_client.put(url, json={"hours": 2})).json() + assert extended["challenge"] == opened["challenge"], "devices enrolling right now hold this one" + + assert (await tilauspalvelu_jwt_admin_client.put(url, json={"hours": 169})).status_code == 422 + assert (await tilauspalvelu_jwt_user_client.get(url)).status_code == 403 + assert (await tilauspalvelu_jwt_user_client.put(url, json={"hours": 24})).status_code == 403 + # A product passes the admin check without being a person; switching enrolment is not its call + async with TestClient(app_instance) as product: + product.headers.update({"X-ClientCert-DN": "CN=fake.localmaeher.dev.pvarki.fi,O=N/A"}) + assert (await product.put(url, json={"hours": 24})).status_code == 403 + assert (await product.get(url)).status_code == 403 + + assert (await tilauspalvelu_jwt_admin_client.put(url, json={"hours": 0})).json()["open_until"] is None + + +@pytest.mark.asyncio(loop_scope="session") +async def test_agent_cn_cannot_be_taken_as_a_callsign(monkeypatch: pytest.MonkeyPatch) -> None: + """The agent CN is a service identity, not a name anyone may enroll under + + A certificate carrying it is accepted as the MDM agent on sight, with no Person, role or + product check, so issuing one to a person would let them enrol devices. + Both creation paths have to refuse it, and case cannot be used to slip past: callsign lookups + fold case, so a near-miss would be a confusing collision rather than a second identity. + + No database here on purpose -- the guard is meant to fire before anything is written. + """ + settings = RMSettings.singleton() + monkeypatch.setattr(settings, "mdm_agent_cns", f"{MDM_AGENT_CN},second-agent") + + for taken in (MDM_AGENT_CN, MDM_AGENT_CN.upper(), "second-agent"): + with pytest.raises(CallsignReserved): + await Enrollment.create_for_callsign(callsign=taken) + with pytest.raises(CallsignReserved): + await Person.create_with_cert(callsign=taken) + + +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize("service_cn", ["rasenmaeher", "RASENMAEHER", "Rasenmaeher"]) +async def test_the_service_cn_cannot_be_taken_in_any_case(service_cn: str) -> None: + """Keycloak and Matrix lowercase the name, so RASENMAEHER downstream is rasenmaeher + + No database here on purpose -- the guard is meant to fire before anything is written. + """ + assert RMSettings.singleton().mtls_client_cert_cn == "rasenmaeher" + with pytest.raises(CallsignReserved): + await Enrollment.create_for_callsign(callsign=service_cn) + with pytest.raises(CallsignReserved): + await Person.create_with_cert(callsign=service_cn) + + +def _csr_asking_for(callsign: str, key_size: int = 2048, **extensions: Any) -> str: + """A device request that asks for more than a device identity""" + key = rsa.generate_private_key(public_exponent=65537, key_size=key_size) + builder = x509.CertificateSigningRequestBuilder().subject_name( + x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, callsign)]) + ) + if extensions.get("server_auth"): + builder = builder.add_extension( + x509.ExtendedKeyUsage([ExtendedKeyUsageOID.SERVER_AUTH, ExtendedKeyUsageOID.CLIENT_AUTH]), critical=False + ) + if extensions.get("san"): + builder = builder.add_extension( + x509.SubjectAlternativeName([x509.DNSName(str(extensions["san"]))]), critical=False + ) + if extensions.get("ca"): + builder = builder.add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + return builder.sign(key, hashes.SHA256()).public_bytes(serialization.Encoding.PEM).decode("utf-8") + + +@pytest.mark.asyncio(loop_scope="session") +async def test_a_device_request_may_not_choose_what_it_is_issued( + mdm_agent_client: TestClient, mdm_challenge: str +) -> None: + """The signer picks its profile from what the request asks for + + So a request asking for server authentication is issued one, and nothing replaces the names it + carries. On this path the request arrived from the public SCEP endpoint, where its contents are + entirely the caller's choice, and every device in the deployment trusts this CA. A genuine MDM + device request asks for none of it. + """ + for label, kwargs in ( + ("server_auth", {"server_auth": True}), + ("san", {"san": "evil.example"}), + ("ca", {"ca": True}), + ): + callsign = f"mdmoverreach{secrets.token_hex(4)}" + resp = await _mdm_enrol(mdm_agent_client, callsign, _csr_asking_for(callsign, **kwargs), mdm_challenge) + assert resp.status_code == 403, f"{label} must be refused" + assert await _no_person(callsign), f"{label} must not spend the name" + + +@pytest.mark.asyncio(loop_scope="session") +async def test_a_device_key_anybody_could_recover_is_refused(mdm_agent_client: TestClient, mdm_challenge: str) -> None: + """RSA-1024 is within reach of factoring, and whoever factors it is the device""" + callsign = f"mdmweak{secrets.token_hex(4)}" + resp = await _mdm_enrol(mdm_agent_client, callsign, _csr_asking_for(callsign, key_size=1024), mdm_challenge) + assert resp.status_code == 403 + assert await _no_person(callsign) diff --git a/tests/test_rasenmaeher_api.py b/tests/test_rasenmaeher_api.py index 62b6aa1c..7770a060 100644 --- a/tests/test_rasenmaeher_api.py +++ b/tests/test_rasenmaeher_api.py @@ -15,7 +15,7 @@ def test_version() -> None: """Make sure version matches expected""" - assert __version__ == "1.19.4+261027" + assert __version__ == "1.20.0+260912" @pytest.mark.asyncio(loop_scope="session") diff --git a/uv.lock b/uv.lock index 7e461d3c..e491a57f 100644 --- a/uv.lock +++ b/uv.lock @@ -2150,7 +2150,7 @@ wheels = [ [[package]] name = "rasenmaeher-api" -version = "1.19.4+261027" +version = "1.20.0+260912" source = { editable = "." } dependencies = [ { name = "aiodns" },