diff --git a/.bumpversion.toml b/.bumpversion.toml index c65cef51..f358bed6 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "1.19.2+260912" +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/pyproject.toml b/pyproject.toml index 4e73826e..454b1c48 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "rasenmaeher_api" -version = "1.19.2+260912" +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 86358cd7..89c55d91 100644 --- a/src/rasenmaeher_api/__init__.py +++ b/src/rasenmaeher_api/__init__.py @@ -1,3 +1,3 @@ """python-rasenmaeher-api""" -__version__ = "1.19.2+260912" # 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..600c3bc1 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) diff --git a/src/rasenmaeher_api/rmsettings.py b/src/rasenmaeher_api/rmsettings.py index 4a907141..ec4887a9 100644 --- a/src/rasenmaeher_api/rmsettings.py +++ b/src/rasenmaeher_api/rmsettings.py @@ -118,6 +118,13 @@ def log_level_int(self) -> int: # (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 @@ -182,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..7b35b212 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""" diff --git a/src/rasenmaeher_api/web/api/enrollment/views.py b/src/rasenmaeher_api/web/api/enrollment/views.py index 4117b4a2..9b735635 100644 --- a/src/rasenmaeher_api/web/api/enrollment/views.py +++ b/src/rasenmaeher_api/web/api/enrollment/views.py @@ -4,19 +4,24 @@ import uuid from typing import Any +from cryptography import x509 +from cryptography.hazmat.primitives import serialization 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 verify_csr from .schema import ( EnrollmentAcceptIn, + EnrollmentAcceptResultOut, EnrollmentDemoteIn, EnrollmentGenVerifiOut, EnrollmentHaveIBeenAcceptedOut, @@ -219,10 +224,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 +239,7 @@ async def request_enrollment_init( outcome="success", target=callsign, request=request, + mdm=request_in.mdm, ), ) @@ -397,23 +406,72 @@ 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 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 +499,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 +530,109 @@ 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") + + 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..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_csr_utils.py b/tests/test_csr_utils.py new file mode 100644 index 00000000..418cb2ae --- /dev/null +++ b/tests/test_csr_utils.py @@ -0,0 +1,97 @@ +"""Unit tests for CSR verification + +No database, no containers: these build certificate requests in memory and check what +``verify_csr`` accepts. Every negative case here was accepted by the previous substring +implementation, which is why they are pinned. +""" + +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..c78acc62 100644 --- a/tests/test_enrollment.py +++ b/tests/test_enrollment.py @@ -3,17 +3,22 @@ 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 serialization +from fastapi import FastAPI from libpvarki.mtlshelp.csr import async_create_client_csr, async_create_keypair from rasenmaeher_api.db import ( EngineWrapper, + Enrollment, EnrollmentPool, Person, ) @@ -730,3 +735,170 @@ 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") -> str: + """A CSR the way a device would send one: its own key, its callsign 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) + + +@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 + + csrpem = await _device_csr(tempdir, callsign) + 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) + assert [attr.value for attr in x509.load_pem_x509_certificate(certpem.encode("utf-8")).subject] == [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") + 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"] == "" diff --git a/tests/test_rasenmaeher_api.py b/tests/test_rasenmaeher_api.py index e674b9ad..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.2+260912" + assert __version__ == "1.20.0+260912" @pytest.mark.asyncio(loop_scope="session") diff --git a/uv.lock b/uv.lock index 1822903c..58b77306 100644 --- a/uv.lock +++ b/uv.lock @@ -2110,7 +2110,7 @@ wheels = [ [[package]] name = "rasenmaeher-api" -version = "1.19.2+260912" +version = "1.20.0+260912" source = { editable = "." } dependencies = [ { name = "aiodns" },