diff --git a/.bumpversion.toml b/.bumpversion.toml index 4d9a73b3..f358bed6 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "1.19.3+260916" +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 c4cf27de..4f0adb95 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 17c656dc..454b1c48 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "rasenmaeher_api" -version = "1.19.3+260916" +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 9fcb79a6..89c55d91 100644 --- a/src/rasenmaeher_api/__init__.py +++ b/src/rasenmaeher_api/__init__.py @@ -1,3 +1,3 @@ """python-rasenmaeher-api""" -__version__ = "1.19.3+260916" # 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/enrollments.py b/src/rasenmaeher_api/db/enrollments.py index a07b02c4..dd9d6110 100644 --- a/src/rasenmaeher_api/db/enrollments.py +++ b/src/rasenmaeher_api/db/enrollments.py @@ -8,11 +8,12 @@ import uuid import warnings from collections.abc import AsyncGenerator -from typing import Any +from typing import Any, cast from sqlalchemy.dialects.postgresql import JSONB -from sqlalchemy.sql import func -from sqlmodel import Field, select +from sqlalchemy.engine import CursorResult +from sqlalchemy.sql import func, update +from sqlmodel import Field, col, select from ..rmsettings import RMSettings from ..web.api.utils.csr_utils import verify_csr @@ -157,7 +158,7 @@ class Enrollment(ORMBaseModel, table=True): approvecode: str = Field(nullable=False, index=True, unique=True) callsign: str = Field(nullable=False, index=True, unique=True) decided_on: datetime.datetime = Field(nullable=True, default=None) - decided_by: uuid.UUID = Field( + decided_by: uuid.UUID | None = Field( foreign_key=f"{ORMBaseModel.__table_args__['schema']}.users.pk", nullable=True, default=None ) person: uuid.UUID = Field( @@ -178,12 +179,74 @@ async def by_pk_or_callsign(cls, inval: str | uuid.UUID) -> "Enrollment": except ValueError: return await cls.by_callsign(str(inval)) - async def approve(self, approver: Person) -> Person: - """Creates the person record, their certs etc""" + @property + def planned_for_mdm(self) -> bool: + """Was this enrollment planned by an admin for a device that enrols through an MDM + + Set by POST /enrollment/init with mdm=true. Such a row waits for the device to present its + own CSR through an MDM agent; approving it by hand would generate a keypair on the server + and spend the callsign, so /enrollment/accept refuses that. + """ + return bool(self.extra) and self.extra.get("mdm") is True + + async def claim_with_csr(self, csrpem: str) -> bool: + """Attach a device CSR to this pending enrollment, once + + Returns False if somebody got there first. The check and the write are one statement on + purpose: two devices racing for the same planned callsign must not both proceed, and the + loser has to find out before any certificate is issued. + + NOTE: refresh (or re-read) the instance before calling approve(); approve() reads self.csr + off the in-memory object and would otherwise take the "no CSR" branch and generate a + keypair here on the server. + """ + with EngineWrapper.get_session() as session: + statement = ( + update(Enrollment) + .where( + col(Enrollment.pk) == self.pk, + col(Enrollment.csr).is_(None), + col(Enrollment.state) == EnrollmentState.PENDING, + col(Enrollment.decided_by).is_(None), + ) + .values(csr=csrpem) + ) + result = cast(CursorResult[Any], session.execute(statement)) + session.commit() + if result.rowcount != 1: + LOGGER.warning(f"Enrollment {self.pk} was already claimed") + return False + self.csr = csrpem + return True + + async def release_claim(self) -> None: + """Undo claim_with_csr + + Issuing can fail after the claim (the CA is a network call), and a claimed row with no + certificate is a callsign nobody can ever use. Releasing lets the device's next attempt + run. It does not help if a Person row was already committed -- see the caller. + """ + with EngineWrapper.get_session() as session: + statement = ( + update(Enrollment) + .where(col(Enrollment.pk) == self.pk, col(Enrollment.state) == EnrollmentState.PENDING) + .values(csr=None) + ) + session.execute(statement) + session.commit() + self.csr = None + + async def approve(self, approver: Person | None) -> Person: + """Creates the person record, their certs etc + + approver is None when an MDM agent completed a device enrolment the admin had already + planned: the agent is a service identity, not a Person, and decided_by is nullable. The + audit trail carries the agent's CN instead. + """ with EngineWrapper.get_session() as session: person = await Person.create_with_cert(self.callsign, extra=self.extra, csrpem=self.csr) self.state = EnrollmentState.APPROVED - self.decided_by = approver.pk + self.decided_by = approver.pk if approver else None self.decided_on = datetime.datetime.now(datetime.UTC) self.person = person.pk session.add(self) @@ -281,8 +344,11 @@ 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: + cnf = RMSettings.singleton() + if callsign in cnf.valid_product_cns: raise CallsignReserved("Using product CNs as callsigns is forbidden") + if callsign.lower() in {cnname.lower() for cnname in cnf.mdm_agent_cn_set}: + raise CallsignReserved("Using MDM agent CNs as callsigns is forbidden") 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/people.py b/src/rasenmaeher_api/db/people.py index 086c3ffc..557a4d4d 100644 --- a/src/rasenmaeher_api/db/people.py +++ b/src/rasenmaeher_api/db/people.py @@ -103,6 +103,10 @@ async def create_with_cert( 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") + if callsign.lower() 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 hand them every planned device's identity. + raise CallsignReserved("Using MDM agent CNs as callsigns is forbidden") try: await Person.by_callsign(callsign) raise CallsignReserved() diff --git a/src/rasenmaeher_api/rmsettings.py b/src/rasenmaeher_api/rmsettings.py index 00b4cdeb..559afd0d 100644 --- a/src/rasenmaeher_api/rmsettings.py +++ b/src/rasenmaeher_api/rmsettings.py @@ -113,6 +113,25 @@ 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 = "" + + # The SCEP challenge the MDM is configured with, so the admin UI can show what to type there. + # Deliberately not a secret: it sits in the MDM's own configuration, the MDM hands it to every + # device, and it is a nuisance filter and nothing more. What actually decides whether a device + # may have a callsign is the per-enrolment code in its certificate request. Empty unless the + # deployment passes it in, and then the UI says where to look instead. + mdm_scep_challenge: str = "" + persistent_data_dir: str = "/data/persistent" # mtls @@ -177,6 +196,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..62b0e0ff 100644 --- a/src/rasenmaeher_api/web/api/enrollment/schema.py +++ b/src/rasenmaeher_api/web/api/enrollment/schema.py @@ -2,6 +2,7 @@ from typing import Any +from libpvarki.schemas.generic import OperationResultResponse from pydantic import BaseModel, ConfigDict, Field @@ -117,6 +118,13 @@ class EnrollmentInitIn(BaseModel): callsign: str = Field(description="Callsign to create enrollment for") csr: str | None = Field(description="CSR for mTLS key in PEM format", default=None) + mdm: bool = Field( + description=( + "Mark this as a device enrolment planned for an MDM. The device supplies its own CSR later and an " + "MDM agent completes the enrolment; no JWT is issued to the caller and approving it by hand is refused." + ), + default=False, + ) class EnrollmentInitOut(BaseModel): @@ -136,7 +144,10 @@ class EnrollmentInitOut(BaseModel): callsign: str = Field(description="Callsign for which the enrollment got initialized") approvecode: str = Field(description="Code used to approve the enrollment, must be delivered to an admin") jwt: str = Field( - description="JWT that allows client to check enrollment approval status and fetc mTLS certs when approved" + description=( + "JWT that allows client to check enrollment approval status and fetc mTLS certs when approved. " + "Empty for enrolments planned for an MDM: nobody should be holding a device's credential." + ) ) @@ -191,7 +202,18 @@ 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 an MDM agent completes a planned device enrolment: 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. Only for an MDM agent completing an enrolment planned with mdm=true." + ), + default=None, ) @@ -210,6 +232,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 +469,12 @@ class EnrollmentInviteCodeDeleteIn(BaseModel): ) invite_code: str + + +class MdmSettingsOut(BaseModel): + """What an operator has to type into whatever MDM their unit runs""" + + #: Where the MDM sends each device's certificate request + scep_url: str + #: The SCEP challenge, or empty if this deployment has not passed it through + challenge: str diff --git a/src/rasenmaeher_api/web/api/enrollment/views.py b/src/rasenmaeher_api/web/api/enrollment/views.py index 4117b4a2..8fb14905 100644 --- a/src/rasenmaeher_api/web/api/enrollment/views.py +++ b/src/rasenmaeher_api/web/api/enrollment/views.py @@ -4,19 +4,25 @@ import uuid from typing import Any +from cryptography import x509 +from cryptography.hazmat.primitives import serialization +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, EnrollmentState, Person +from ....db.errors import 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 csr_carries_enrolment_proof, verify_csr from .schema import ( EnrollmentAcceptIn, + EnrollmentAcceptResultOut, EnrollmentDemoteIn, EnrollmentGenVerifiOut, EnrollmentHaveIBeenAcceptedOut, @@ -37,6 +43,7 @@ EnrollmentShowVerificationCodeOut, EnrollmentStatusIn, EnrollmentStatusOut, + MdmSettingsOut, ) LOGGER = logging.getLogger(__name__) @@ -176,16 +183,61 @@ async def request_enrollment_list(code: str | None = None) -> EnrollmentListOut: if code: try: enrollment = await Enrollment.by_approvecode(code) - result_list.append({"callsign": enrollment.callsign, "approvecode": code, "state": enrollment.state}) + result_list.append( + { + "callsign": enrollment.callsign, + "approvecode": code, + "state": enrollment.state, + "mdm": enrollment.planned_for_mdm, + } + ) except NotFound: pass return EnrollmentListOut(callsign_list=result_list) async for enrollment in Enrollment.list(): - result_list.append({"callsign": enrollment.callsign, "approvecode": "", "state": enrollment.state}) + # mdm marks a device an admin planned for MDM enrolment. It is waiting for the device's own + # CSR through the agent, not for a human to approve it, and approving it by hand is refused. + # + # The code is withheld for people: there it is a secret the person tells the admin, and + # listing it would let any admin approve anybody without being told anything. For a device + # it is the opposite -- the admin has to type it into the MDM for the device to enrol at + # all, and nobody else ever learns it. Withholding it there would mean an admin who plans a + # hundred devices can only ever read the codes once, at the moment of creation. + result_list.append( + { + "callsign": enrollment.callsign, + "approvecode": enrollment.approvecode if enrollment.planned_for_mdm else "", + "state": enrollment.state, + "mdm": enrollment.planned_for_mdm, + } + ) return EnrollmentListOut(callsign_list=result_list) +@ENROLLMENT_ROUTER.get( + "/mdm-settings", + response_model=MdmSettingsOut, + dependencies=[Depends(ValidUser(auto_error=True, require_roles=["admin"]))], +) +async def request_mdm_settings(request: Request) -> MdmSettingsOut: + """What to configure the MDM with + + The challenge is not a secret and is not treated as one: it sits in the MDM's own + configuration, the MDM hands it to every device, and it is a nuisance filter against whatever + scans the internet. What decides whether a device may have a callsign is the per-enrolment code + in its certificate request, which is per device and never leaves the admin and the MDM. + + Admin only all the same, because there is no reason for anybody else to read it. + """ + cnf = RMSettings.singleton() + host = (request.url.hostname or "").removeprefix("mtls.") + return MdmSettingsOut( + scep_url=f"https://{host}/scep" if host else "", + challenge=cnf.mdm_scep_challenge, + ) + + def issue_enrollment_jwt(response: Response, claims: dict[str, Any]) -> str: """Issue longer lived JWT and set persistent cookie""" enroll_issuer = Issuer() @@ -219,10 +271,13 @@ async def request_enrollment_init( # TODO ADD POOL NAME CHECK - new_enrollment = await Enrollment.create_for_callsign(callsign=callsign, pool=None, extra={}, csr=request_in.csr) - # Create JWT token for user - claims = {"sub": callsign} - new_jwt = issue_enrollment_jwt(response, claims) + extra: dict[str, Any] = {"mdm": True} if request_in.mdm else {} + new_enrollment = await Enrollment.create_for_callsign(callsign=callsign, pool=None, extra=extra, csr=request_in.csr) + # Create JWT token for user. Never for a device planned for MDM enrolment: that token is a live + # credential for the callsign, the device will never use it (it receives a certificate through + # the agent), and issuing it here also overwrites the calling admin's own session cookie, which + # is unhelpful when planning three hundred of them. + new_jwt = "" if request_in.mdm else issue_enrollment_jwt(response, {"sub": callsign}) LOGGER.audit( # type: ignore[attr-defined] "Enrollment initiated by admin", @@ -231,6 +286,7 @@ async def request_enrollment_init( outcome="success", target=callsign, request=request, + mdm=request_in.mdm, ), ) @@ -397,23 +453,107 @@ 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 or which are planned. 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 complete a device enrolment an + admin has already planned. + """ + 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 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 completing a device enrolment an admin planned, 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] @@ -441,6 +581,25 @@ async def post_enrollment_accept( ) raise HTTPException(status_code=403, detail="Invalid approval code for this enrollment") + if pending_enrollment.planned_for_mdm and not pending_enrollment.csr: + # Approving it here would have rasenmaeher generate the keypair, and the device could then + # never have this callsign: callsigns are unique and are never released. + LOGGER.audit( # type: ignore[attr-defined] + "Enrollment approval refused - planned for MDM", + extra=build_audit_extra( + action="enrollment_approve", + outcome="failure", + actor=admin_user.callsign, + target=target_callsign, + request=request, + error_code="PLANNED_FOR_MDM", + ), + ) + raise HTTPException( + status_code=409, + detail="This device enrolls itself through the MDM, approving it by hand would spend the callsign", + ) + new_approved_user = await pending_enrollment.approve(approver=admin_user) LOGGER.audit( # type: ignore[attr-defined] @@ -453,7 +612,118 @@ 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( # pylint: disable=too-many-return-statements + request: Request, + request_in: EnrollmentAcceptIn, + agent_cn: str, +) -> EnrollmentAcceptResultOut: + """The MDM agent completes a device enrollment an admin planned + + Everything is checked before the enrollment is claimed, so a request we are going to refuse + never consumes the callsign. + """ + 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) + + csrpem = request_in.csr + if not csrpem: + raise refuse(400, "CSR_MISSING", "The device CSR is required") + + try: + planned = await Enrollment.by_callsign(callsign=target_callsign) + except NotFound as exc: + raise refuse(403, "NOT_PLANNED") from exc + if not planned.planned_for_mdm: + raise refuse(403, "NOT_PLANNED_FOR_MDM") + if not verify_csr(csrpem, planned.callsign): + raise refuse(403, "CSR_SUBJECT") + overreach = device_csr_overreach(csrpem) + if overreach: + raise refuse(403, "CSR_OVERREACH") + if not csr_carries_enrolment_proof(csrpem, planned.callsign, planned.approvecode): + # The callsign alone is not proof: it is a short human-chosen string, and the SCEP + # challenge in front of it is not a secret. The per-enrolment code is what says the MDM + # assigned THIS device THIS callsign. Refused before anything is claimed, so guessing + # cannot spend a callsign either. + raise refuse(403, "NO_ENROLMENT_PROOF") + + if planned.state != EnrollmentState.PENDING or planned.csr: + return await mdm_repeat(request, csrpem, planned, refuse) + + if not await planned.claim_with_csr(csrpem): + raise refuse(403, "ALREADY_CLAIMED") + # Re-read: approve() takes the CSR off the instance, and the claim was a separate statement. + claimed = await Enrollment.by_callsign(callsign=target_callsign) + try: + person = await claimed.approve(approver=None) + except Exception: + # Issuing is a network call to the CA. Let the device's next attempt have the callsign + # back -- unless a Person row was already committed, which mdm_repeat then reports. + await claimed.release_claim() + raise + + LOGGER.audit( # type: ignore[attr-defined] + "Enrollment completed 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( + request: Request, + csrpem: str, + planned: Enrollment, + refuse: Any, +) -> EnrollmentAcceptResultOut: + """The agent asked again for a callsign that is no longer waiting + + 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". + """ + _ = request + try: + person = await Person.by_callsign(planned.callsign) + except (NotFound, Deleted) as exc: + raise refuse(403, "ALREADY_DECIDED") from exc + 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 callsign unusable for good. + raise refuse(409, "CALLSIGN_SPENT", "That callsign was spent by a failed issue, plan another") + certpem = person.certfile.read_text(encoding="utf-8") + if not same_public_key(csrpem, certpem): + raise refuse(403, "DIFFERENT_KEY") + return EnrollmentAcceptResultOut( + success=True, + extra=f"Approved {person.callsign}", + certificate=certpem, + ) @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..07cad9d7 100644 --- a/src/rasenmaeher_api/web/api/utils/csr_utils.py +++ b/src/rasenmaeher_api/web/api/utils/csr_utils.py @@ -1,8 +1,10 @@ """Utils for checking CSR:s etc""" import logging +import secrets from cryptography import x509 +from cryptography.x509.oid import NameOID from libadvian.binpackers import ensure_utf8 LOGGER = logging.getLogger(__name__) @@ -12,14 +14,65 @@ 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"), # crypto.X509Extension(b"extendedKeyUsage", True, b"clientAuth"), return True + + +def csr_carries_enrolment_proof(csrpem: str, callsign: str, approvecode: str) -> bool: + """Does this request carry the per-enrolment code, proving the MDM assigned this device + + The public SCEP endpoint is reachable by anyone, and the SCEP challenge is not a secret: it + sits in the MDM's configuration and travels in every device's request. So without this the + last gate is a callsign, which is a short human-chosen string an attacker can guess. + + The MDM carries the proof by putting ``@`` in the one per-host field it + can substitute into a subject, rendered into an attribute beside the common name. Every + attribute other than the common name is searched rather than one named position, because which + attribute the MDM renders it into is the MDM's business and not something to hard-code. + """ + wanted = f"{callsign}@{approvecode}" + if not approvecode or not wanted.isascii(): + return False + try: + csr = x509.load_pem_x509_csr(ensure_utf8(csrpem)) + except ValueError: + return False + for attribute in csr.subject: + if attribute.oid == NameOID.COMMON_NAME: + continue + value = attribute.value + if not isinstance(value, str) or not value.isascii(): + # compare_digest raises on non-ascii str, and nothing we issue for is non-ascii + continue + if secrets.compare_digest(value, wanted): + return True + return False 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..2321292d --- /dev/null +++ b/tests/test_csr_utils.py @@ -0,0 +1,136 @@ +"""Unit tests for CSR verification + +Build certificate requests in memory and check what +``verify_csr`` accepts. +""" + +import logging +from typing import Any + +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 csr_carries_enrolment_proof, 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") + + +def _csr_with_subject(attrs: list[tuple[Any, str]]) -> str: + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + name = x509.Name([x509.NameAttribute(oid, value) for oid, value in attrs]) + csr = x509.CertificateSigningRequestBuilder().subject_name(name).sign(key, hashes.SHA256()) + return csr.public_bytes(serialization.Encoding.PEM).decode("utf-8") + + +def test_the_enrolment_proof_is_required_and_exact() -> None: + """The callsign alone is a short guessable string, so it cannot be the last gate""" + good = _csr_with_subject([(NameOID.COMMON_NAME, "OTTER1"), (NameOID.ORGANIZATIONAL_UNIT_NAME, "OTTER1@7F3A9C2B")]) + assert csr_carries_enrolment_proof(good, "OTTER1", "7F3A9C2B") + assert not csr_carries_enrolment_proof(good, "OTTER1", "WRONGONE") + assert not csr_carries_enrolment_proof(good, "OTTER2", "7F3A9C2B"), "a code is bound to its callsign" + + bare = _csr_with_subject([(NameOID.COMMON_NAME, "OTTER1")]) + assert not csr_carries_enrolment_proof(bare, "OTTER1", "7F3A9C2B"), "what a guesser would send" + + +def test_the_proof_is_found_wherever_the_mdm_renders_it() -> None: + """Which attribute carries it is the MDM's business, so do not hard-code one""" + for oid in (NameOID.ORGANIZATIONAL_UNIT_NAME, NameOID.ORGANIZATION_NAME, NameOID.STATE_OR_PROVINCE_NAME): + csrpem = _csr_with_subject([(NameOID.COMMON_NAME, "OTTER1"), (oid, "OTTER1@7F3A9C2B")]) + assert csr_carries_enrolment_proof(csrpem, "OTTER1", "7F3A9C2B") + + +def test_the_common_name_cannot_be_the_proof() -> None: + """Otherwise a caller who guessed the callsign could satisfy the check with it""" + csrpem = _csr_with_subject([(NameOID.COMMON_NAME, "OTTER1@7F3A9C2B")]) + assert not csr_carries_enrolment_proof(csrpem, "OTTER1@7F3A9C2B", "7F3A9C2B") + + +def test_odd_input_is_refused_rather_than_raising() -> None: + """This runs on bytes that reached us from a public endpoint""" + assert not csr_carries_enrolment_proof("not a csr", "OTTER1", "7F3A9C2B") + assert not csr_carries_enrolment_proof(_csr_with_subject([(NameOID.COMMON_NAME, "OTTER1")]), "OTTER1", "") + unicode_ou = _csr_with_subject([(NameOID.COMMON_NAME, "OTTER1"), (NameOID.ORGANIZATIONAL_UNIT_NAME, "ÄÄÄ")]) + assert not csr_carries_enrolment_proof(unicode_ou, "OTTER1", "7F3A9C2B"), "must not raise on non-ascii" diff --git a/tests/test_enrollment.py b/tests/test_enrollment.py index f9ae9df7..997bcac2 100644 --- a/tests/test_enrollment.py +++ b/tests/test_enrollment.py @@ -1,22 +1,32 @@ """Test enrollment endpoint""" +import asyncio import logging 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, + EnrollmentState, Person, ) +from rasenmaeher_api.db.errors import CallsignReserved from rasenmaeher_api.rmsettings import RMSettings LOGGER = logging.getLogger(__name__) @@ -223,7 +233,11 @@ async def test_list_as_adm(tilauspalvelu_jwt_admin_client: TestClient) -> None: LOGGER.debug(resp_dict) assert resp_dict["callsign_list"] is not None assert resp_dict["callsign_list"][0]["callsign"] - assert not resp_dict["callsign_list"][0]["approvecode"] + # Every person's code, not just whichever row the database happened to return first: a device + # planned for MDM carries its code on purpose, so an index-0 check is a coin flip on what + # earlier tests left behind. Which kind may show its code is pinned in + # test_the_list_carries_the_code_for_devices_but_not_for_people, on rows it created itself. + assert all(not row["approvecode"] for row in resp_dict["callsign_list"] if not row["mdm"]) assert resp.status_code == 200 @@ -730,3 +744,368 @@ 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 + + +async def _device_csr(tempdir: Path, callsign: str, name: str = "device", code: str = "") -> str: + """A CSR the way a device would send one: its own key, its callsign as the CN + + With a code, also the way an MDM sends one: ``@`` rendered into the one + per-host subject field an MDM can substitute, which is what proves the MDM assigned this + device this callsign. Without it, what a caller who only guessed the callsign would send. + """ + privkeyfile = Path(tempdir) / f"{name}.key" + pubkeyfile = Path(tempdir) / f"{name}.pub" + csrfile = Path(tempdir) / f"{name}.csr" + reqdn = {"CN": callsign} + if code: + reqdn["OU"] = f"{callsign}@{code}" + ckp = await async_create_keypair(privkeyfile, pubkeyfile) + return str(await async_create_client_csr(ckp, csrfile, reqdn)) + + +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) + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_planned_enrollment_is_not_approvable_by_hand( + tilauspalvelu_jwt_admin_client: TestClient, +) -> None: + """An admin planning a device must not be able to approve it and spend the callsign""" + callsign = f"mdmplanned_{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post( + "/api/v1/enrollment/init", json={"callsign": callsign, "mdm": True} + ) + assert resp.status_code == 200 + planned = resp.json() + assert planned["callsign"] == callsign + # No credential for the device is handed to the admin planning it + assert planned["jwt"] == "" + + resp = await tilauspalvelu_jwt_admin_client.post( + "/api/v1/enrollment/accept", json={"callsign": callsign, "approvecode": planned["approvecode"]} + ) + assert resp.status_code == 409 + # and the enrollment is untouched, so the device can still take it + still_pending = await Enrollment.by_callsign(callsign) + assert still_pending.state == 0 + assert still_pending.csr is None + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_agent_completes_planned_enrollment( + tilauspalvelu_jwt_admin_client: TestClient, mdm_agent_client: TestClient, nice_tmpdir: str +) -> None: + """The whole agent path: plan, complete with the device CSR, and survive a repeat""" + tempdir = Path(nice_tmpdir) + callsign = f"mdmdevice_{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post( + "/api/v1/enrollment/init", json={"callsign": callsign, "mdm": True} + ) + assert resp.status_code == 200 + code = resp.json()["approvecode"] + + csrpem = await _device_csr(tempdir, callsign, code=code) + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": callsign, "csr": csrpem}) + assert resp.status_code == 200 + body = resp.json() + assert body["success"] is True + certpem = body["certificate"] + assert certpem + + # 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, so exactly one and it is + # the callsign. The signer copies the rest of the request's subject verbatim, which is why the + # enrolment code rides along -- spent by the time it is in there, but worth noticing if the + # signer ever starts naming the certificate itself instead. + 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] + assert [attr.value for attr in issued.subject] == [f"{callsign}@{code}", callsign] + + # The reply can be lost on the way back to the MDM, which then repeats the request verbatim + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": callsign, "csr": csrpem}) + assert resp.status_code == 200 + assert resp.json()["certificate"] == certpem + + # ...but a different key for the same callsign is somebody else + other_csr = await _device_csr(tempdir, callsign, name="imposter", code=code) + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": callsign, "csr": other_csr}) + 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, + unauth_client_session: TestClient, + nice_tmpdir: str, +) -> None: + """What the agent may not do""" + tempdir = Path(nice_tmpdir) + + # A callsign nobody planned + unplanned = f"mdmunplanned_{secrets.token_hex(4)}" + csrpem = await _device_csr(tempdir, unplanned, name="unplanned") + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": unplanned, "csr": csrpem}) + assert resp.status_code == 403 + with pytest.raises(Exception): # noqa: B017 -- NotFound, and nothing was written + await Enrollment.by_callsign(unplanned) + + # A CSR for a different callsign than the one being completed + planned = f"mdmmismatch_{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post("/api/v1/enrollment/init", json={"callsign": planned, "mdm": True}) + assert resp.status_code == 200 + wrong_csr = await _device_csr(tempdir, f"{planned}X", name="mismatch") + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": planned, "csr": wrong_csr}) + assert resp.status_code == 403 + assert (await Enrollment.by_callsign(planned)).csr is None + + # No CSR at all + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": planned}) + 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": planned, "csr": wrong_csr}) + assert resp.status_code == 403 + + # An enrollment a human started: not the agent's business. + # 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. + # Clean up after it here; not issuing it at all is exactly what the mdm form is for. + 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() + human_csr = await _device_csr(tempdir, human, name="human") + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": human, "csr": human_csr}) + assert resp.status_code == 403 + assert (await Enrollment.by_callsign(human)).csr is None + + +@pytest.mark.asyncio(loop_scope="session") +async def test_planning_many_devices_keeps_the_admin_session( + tilauspalvelu_jwt_admin_client: TestClient, +) -> None: + """Planning a fleet is one call per device and must not log the admin out on the first one + + A plain /enrollment/init issues the new callsign's JWT and sets it as the session cookie of + whoever called it, and JWTBearer prefers that cookie over the Authorization header -- so the + next call would arrive as a freshly created non-admin user. Devices planned for MDM get no JWT + at all, nobody should be holding a device's credential anyway, so this stays an admin. + """ + for _ in range(3): + callsign = f"mdmfleet_{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post( + "/api/v1/enrollment/init", json={"callsign": callsign, "mdm": True} + ) + assert resp.status_code == 200 + assert resp.json()["jwt"] == "" + + +@pytest.mark.asyncio(loop_scope="session") +async def test_list_marks_devices_planned_for_mdm(tilauspalvelu_jwt_admin_client: TestClient) -> None: + """An admin has to be able to tell a planned device from a person waiting for approval + + Both sit in the list in state PENDING, but a device is waiting for its own CSR through the + agent and approving it by hand is refused, so without the marker the approve-users queue fills + with rows nobody may act on. + """ + device = f"mdmlisted_{secrets.token_hex(4)}" + human = f"humanlisted_{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post("/api/v1/enrollment/init", json={"callsign": device, "mdm": True}) + assert resp.status_code == 200 + resp = await tilauspalvelu_jwt_admin_client.post("/api/v1/enrollment/init", json={"callsign": human}) + assert resp.status_code == 200 + # A plain init hands back the new callsign's JWT as a cookie, which would unseat the admin + if tilauspalvelu_jwt_admin_client.cookie_jar is not None: + tilauspalvelu_jwt_admin_client.cookie_jar.clear() + + listed = (await tilauspalvelu_jwt_admin_client.get("/api/v1/enrollment/list")).json()["callsign_list"] + marks = {row["callsign"]: row["mdm"] for row in listed if row["callsign"] in (device, human)} + assert marks == {device: True, human: False} + + +@pytest.mark.asyncio(loop_scope="session") +async def test_the_list_carries_the_code_for_devices_but_not_for_people( + tilauspalvelu_jwt_admin_client: TestClient, +) -> None: + """A device's code is configuration an admin must type into the MDM; a person's is a secret + + Withholding it for devices would mean an admin who plans a hundred of them can only read the + codes once, at the moment of creation. Exposing it for people would let any admin approve + anybody without being told anything. + """ + device = f"mdmcode_{secrets.token_hex(4)}" + human = f"humancode_{secrets.token_hex(4)}" + made = await tilauspalvelu_jwt_admin_client.post("/api/v1/enrollment/init", json={"callsign": device, "mdm": True}) + assert made.status_code == 200 + expected = made.json()["approvecode"] + assert await tilauspalvelu_jwt_admin_client.post("/api/v1/enrollment/init", json={"callsign": human}) + if tilauspalvelu_jwt_admin_client.cookie_jar is not None: + tilauspalvelu_jwt_admin_client.cookie_jar.clear() + + listed = (await tilauspalvelu_jwt_admin_client.get("/api/v1/enrollment/list")).json()["callsign_list"] + codes = {row["callsign"]: row["approvecode"] for row in listed if row["callsign"] in (device, human)} + assert codes[device] == expected, "a device's code has to be readable again after creation" + assert codes[human] == "", "a person's code is theirs to tell the admin" + + +@pytest.mark.asyncio(loop_scope="session") +async def test_two_devices_cannot_both_claim_one_planned_callsign() -> None: + """The clause the MVP plan will not sign off without + + A callsign is spent the moment it is used and can never be released, so if two devices both + got past the claim one of them would receive a certificate for an identity the other also + holds, and the loser could never be enrolled under any name it had been promised. The check + and the write are one UPDATE for exactly this reason, so the guarantee is the database's and + not a check-then-act in Python. + """ + callsign = f"mdmrace_{secrets.token_hex(4)}" + planned = await Enrollment.create_for_callsign(callsign=callsign, extra={"mdm": True}) + + first, second = await asyncio.gather( + (await Enrollment.by_callsign(callsign)).claim_with_csr("csr-from-device-one"), + (await Enrollment.by_callsign(callsign)).claim_with_csr("csr-from-device-two"), + ) + assert sorted([first, second]) == [False, True], "exactly one device may claim a planned callsign" + + settled = await Enrollment.by_callsign(callsign) + assert settled.csr in ("csr-from-device-one", "csr-from-device-two") + assert settled.state == EnrollmentState.PENDING, "claiming must not decide the enrollment" + _ = planned + + +@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 hand them every planned device's identity. + 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) + + +def _csr_asking_for(callsign: str, **extensions: Any) -> str: + """A device request that asks for more than a device identity""" + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + 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( + tilauspalvelu_jwt_admin_client: TestClient, mdm_agent_client: TestClient +) -> 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 tilauspalvelu_jwt_admin_client.post( + "/api/v1/enrollment/init", json={"callsign": callsign, "mdm": True} + ) + assert resp.status_code == 200 + resp = await mdm_agent_client.post( + "/api/v1/enrollment/accept", + json={"callsign": callsign, "csr": _csr_asking_for(callsign, **kwargs)}, + ) + assert resp.status_code == 403, f"{label} must be refused" + assert (await Enrollment.by_callsign(callsign)).csr is None, f"{label} must not claim the callsign" + + +@pytest.mark.asyncio(loop_scope="session") +async def test_an_ordinary_device_request_is_still_accepted( + tilauspalvelu_jwt_admin_client: TestClient, mdm_agent_client: TestClient, nice_tmpdir: str +) -> None: + """The guard must not refuse what an MDM actually sends""" + callsign = f"mdmplain_{secrets.token_hex(4)}" + resp = await tilauspalvelu_jwt_admin_client.post( + "/api/v1/enrollment/init", json={"callsign": callsign, "mdm": True} + ) + assert resp.status_code == 200 + code = resp.json()["approvecode"] + csrpem = await _device_csr(Path(nice_tmpdir), callsign, name="plain", code=code) + resp = await mdm_agent_client.post("/api/v1/enrollment/accept", json={"callsign": callsign, "csr": csrpem}) + assert resp.status_code == 200, resp.text + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_settings_says_what_to_configure( + tilauspalvelu_jwt_admin_client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """The challenge is not a secret, so an admin reads it here rather than off the server + + It sits in the MDM's own configuration and the MDM hands it to every device. What decides + whether a device may have a callsign is the per-enrolment code, not this. + """ + monkeypatch.setattr(RMSettings.singleton(), "mdm_scep_challenge", "a-challenge-not-a-secret") + resp = await tilauspalvelu_jwt_admin_client.get("/api/v1/enrollment/mdm-settings") + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["challenge"] == "a-challenge-not-a-secret" + assert body["scep_url"].endswith("/scep") + assert not body["scep_url"].startswith("https://mtls."), "the responder is on the plain host" + + +@pytest.mark.asyncio(loop_scope="session") +async def test_mdm_settings_is_admin_only(tilauspalvelu_jwt_user_client: TestClient) -> None: + """No reason for anybody but an admin to read it, secret or not""" + resp = await tilauspalvelu_jwt_user_client.get("/api/v1/enrollment/mdm-settings") + assert resp.status_code == 403, resp.text diff --git a/tests/test_rasenmaeher_api.py b/tests/test_rasenmaeher_api.py index 3a886f10..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.3+260916" + assert __version__ == "1.20.0+260912" @pytest.mark.asyncio(loop_scope="session") diff --git a/uv.lock b/uv.lock index c8c418ab..58b77306 100644 --- a/uv.lock +++ b/uv.lock @@ -2110,7 +2110,7 @@ wheels = [ [[package]] name = "rasenmaeher-api" -version = "1.19.3+260916" +version = "1.20.0+260912" source = { editable = "." } dependencies = [ { name = "aiodns" },