diff --git a/.bumpversion.toml b/.bumpversion.toml index 4d9a73b3..5506aca9 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "1.19.3+260916" +current_version = "1.19.4+260919" 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..13b90a7b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -129,6 +129,7 @@ RUN --mount=type=ssh apt-get update && apt-get install -y \ libffi8 \ tini \ libpq5 \ + openssl \ && apt-get autoremove -y \ && rm -rf /var/lib/apt/lists/* \ && chmod a+x /docker-entrypoint.sh \ diff --git a/pyproject.toml b/pyproject.toml index 17c656dc..2fe510f2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "rasenmaeher_api" -version = "1.19.3+260916" +version = "1.19.4+260919" 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..0022f08b 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.19.4+260919" # NOTE Use `bump-my-version` to bump versions correctly diff --git a/src/rasenmaeher_api/cert/pkcs12.py b/src/rasenmaeher_api/cert/pkcs12.py new file mode 100644 index 00000000..6cb36efb --- /dev/null +++ b/src/rasenmaeher_api/cert/pkcs12.py @@ -0,0 +1,53 @@ +"""PKCS12 containers that rasenmaeher builds itself + +libpvarki.mtlshelp.pkcs12 covers the passworded case. This is the one thing it cannot do, and +the eventual home for it is probably there rather than here, since takrmapi builds containers too. +""" + +import logging +import subprocess # nosec +from pathlib import Path + +LOGGER = logging.getLogger(__name__) +# Same algorithms libpvarki.mtlshelp.pkcs12.serialize_legacy_pkcs12 uses, for the same broad client +# support. They are also the documented recipe for Windows, so do not "modernise" them to AES +# without testing Windows and older Android: the empty password means they protect nothing anyway. +NOPASS_PKCS12_ALGOS = ("-keypbe", "PBE-SHA1-3DES", "-certpbe", "PBE-SHA1-3DES", "-macalg", "sha1") + + +def write_nopass_pkcs12(certfile: Path, keyfile: Path | None, target: Path, friendlyname: str) -> None: + """Write a PKCS12 container that opens with an empty password + + ponytail: shells out to openssl because pyca/cryptography refuses to serialize with an empty + password ("Password must be 1 or more bytes"), and its only password-free alternative, + NoEncryption(), emits a MAC-less container that the JDK PKCS12 provider reads as having no + entries at all. The container therefore has to stay encrypted, just with an empty password: + that is precisely what AOSP CredentialHelper.hasPassword() probes for before Android's + CertInstaller decides whether to show its password dialog. + """ + # People who enrolled with their own CSR get a cert-only container, there is no key to put in it. + # -name labels the key, so with no key the friendly name has to go on the cert bag instead. + keyargs = ["-inkey", str(keyfile)] if keyfile else ["-nokeys"] + nameargs = ["-name", friendlyname] if keyfile else ["-caname", friendlyname] + cmd = [ + "openssl", + "pkcs12", + "-export", + "-out", + str(target), + *keyargs, + "-in", + str(certfile), + *nameargs, + *NOPASS_PKCS12_ALGOS, + "-passout", + "pass:", + ] + try: + subprocess.run(cmd, check=True, capture_output=True) # nosec + except subprocess.CalledProcessError as exc: + # capture_output swallows the reason, and without it this is an unexplained 500 + LOGGER.error("openssl pkcs12 failed for {}: {}".format(target, exc.stderr.decode("utf-8", errors="replace"))) + raise + # No password means the key is only as safe as the file mode + target.chmod(0o600) diff --git a/src/rasenmaeher_api/db/people.py b/src/rasenmaeher_api/db/people.py index 086c3ffc..db712285 100644 --- a/src/rasenmaeher_api/db/people.py +++ b/src/rasenmaeher_api/db/people.py @@ -21,6 +21,7 @@ from sqlmodel import Field, SQLModel, select from ..cert.backend import ReasonTypes, refresh_ocsp, revoke_pem, sign_csr, validate_reason +from ..cert.pkcs12 import write_nopass_pkcs12 from ..kchelpers import KCClient, KCUserData from ..productapihelpers import post_to_all_products from ..rmsettings import RMSettings @@ -160,18 +161,21 @@ async def _post_create(self) -> "Person": return refresh async def create_pfx(self) -> Path: - """Put cert and key to PKCS12 container""" - if self.pfxfile.exists(): + """Put cert and key to PKCS12 containers, with and without a password""" + if self.pfxfile.exists() and self.nopass_pfxfile.exists(): return self.pfxfile def write_pfx() -> None: """Do the IO""" nonlocal self - if self.privkeyfile.exists(): - p12bytes = convert_pem_to_pkcs12(self.certfile, self.privkeyfile, self.callsign, None, self.callsign) - else: - p12bytes = convert_pem_to_pkcs12(self.certfile, None, self.callsign, None, self.callsign) + keyfile = self.privkeyfile if self.privkeyfile.exists() else None + p12bytes = convert_pem_to_pkcs12(self.certfile, keyfile, self.callsign, None, self.callsign) self.pfxfile.write_bytes(p12bytes) + # The passworded container is what the Apple profile embeds, the empty-password one is + # what everything else downloads. Always write both: the download path must never have + # to serve the passworded one, the UI no longer tells anybody what the password is. + # Keep the friendly name, Android prefills the credential name prompt from it. + write_nopass_pkcs12(self.certfile, keyfile, self.nopass_pfxfile, self.callsign) await asyncio.get_event_loop().run_in_executor(None, write_pfx) return self.pfxfile @@ -242,6 +246,11 @@ def pfxfile(self) -> Path: """Return a PKCS12 PFX file""" return Path(self.certspath) / "mtls.pfx" + @property + def nopass_pfxfile(self) -> Path: + """Return a PKCS12 PFX file that opens with an empty password""" + return Path(self.certspath) / "mtls_nopass.pfx" + @property def certfile(self) -> Path: """Path to the public cert""" diff --git a/src/rasenmaeher_api/web/api/enduserpfx/mobileconfig.py b/src/rasenmaeher_api/web/api/enduserpfx/mobileconfig.py new file mode 100644 index 00000000..0f60eb17 --- /dev/null +++ b/src/rasenmaeher_api/web/api/enduserpfx/mobileconfig.py @@ -0,0 +1,42 @@ +"""Apple configuration profile wrapping the client certificate.""" + +import plistlib +import uuid + +MEDIA_TYPE = "application/x-apple-aspen-config" +# Stable namespace, so reinstalling replaces the profile instead of stacking duplicates +NAMESPACE = uuid.uuid5(uuid.NAMESPACE_DNS, "pvarki.fi") + + +def build_mobileconfig(callsign: str, deployment: str, pfxbytes: bytes, password: str) -> bytes: + """Wrap a PKCS12 container in an Apple configuration profile + + Apple's SecPKCS12Import cannot open an empty-password container at all, on any current iOS or + macOS, so this embeds the passworded container and carries its password in the optional + Password key. Both platforms then install the identity without ever prompting for it, which is + the only way to keep Apple password-free from the user's point of view. + """ + identifier = f"fi.pvarki.{deployment}.mtls" + identity = { + "PayloadType": "com.apple.security.pkcs12", + "PayloadVersion": 1, + "PayloadIdentifier": f"{identifier}.identity", + "PayloadUUID": str(uuid.uuid5(NAMESPACE, f"{identifier}:{callsign}:identity")), + "PayloadDisplayName": f"{callsign} client certificate", + "PayloadCertificateFileName": f"{callsign}.pfx", + "PayloadContent": pfxbytes, + "Password": password, + } + profile = { + "PayloadType": "Configuration", + "PayloadVersion": 1, + "PayloadIdentifier": identifier, + "PayloadUUID": str(uuid.uuid5(NAMESPACE, f"{identifier}:{callsign}")), + "PayloadDisplayName": f"{deployment} client certificate ({callsign})", + "PayloadDescription": f"Installs the {callsign} client certificate for {deployment}.", + "PayloadOrganization": deployment, + "PayloadScope": "User", + "PayloadRemovalDisallowed": False, + "PayloadContent": [identity], + } + return plistlib.dumps(profile) diff --git a/src/rasenmaeher_api/web/api/enduserpfx/views.py b/src/rasenmaeher_api/web/api/enduserpfx/views.py index 28e60c39..558d9da1 100644 --- a/src/rasenmaeher_api/web/api/enduserpfx/views.py +++ b/src/rasenmaeher_api/web/api/enduserpfx/views.py @@ -2,13 +2,16 @@ import logging -from fastapi import APIRouter, Depends, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Request, Response from fastapi.responses import FileResponse +from libpvarki.schemas.generic import OperationResultResponse from ....db import Person from ....rmsettings import RMSettings +from ..enrollment.views import issue_enrollment_jwt from ..middleware.user import ValidUser from ..utils.auditcontext import build_audit_extra +from .mobileconfig import MEDIA_TYPE, build_mobileconfig router = APIRouter() LOGGER = logging.getLogger(__name__) @@ -61,6 +64,82 @@ async def get_user_pem( ) +@router.post("/session") +async def create_download_session( + request: Request, + response: Response, + person: Person = Depends(ValidUser(auto_error=True)), +) -> OperationResultResponse: + """Refresh the JWT cookie so the browser can fetch a download by plain navigation + + iOS only hands a .mobileconfig to the profile installer when Safari navigates to it and sees + the response content type, and a navigation carries no Authorization header. The cookie is the + only credential that survives one, and the one enrollment sets expires long before the JWT the + page holds, so anyone downloading again later needs a fresh one. + """ + issue_enrollment_jwt(response, {"sub": person.callsign}) + LOGGER.audit( # type: ignore[attr-defined] + "Download session refreshed", + extra=build_audit_extra( + action="download_session", + outcome="success", + actor=person.callsign, + request=request, + ), + ) + return OperationResultResponse(success=True) + + +@router.get(f"/{{callsign}}_{RMSettings.singleton().deployment_name}.mobileconfig") +@router.get("/{callsign}.mobileconfig") +async def get_user_mobileconfig( + request: Request, + callsign: str, + person: Person = Depends(ValidUser(auto_error=True)), +) -> Response: + """Get the cert and key as an Apple configuration profile, which installs without a prompt""" + deployment = RMSettings.singleton().deployment_name + # Strip the extension before the deployment suffix: "/{callsign}.mobileconfig" is registered + # first, so it is what matches the long form too, leaving "OTTER1_localmaeher" in the param. + callsign = callsign.removesuffix(".mobileconfig") + callsign = callsign.removesuffix(f"_{deployment}") + LOGGER.debug(f"MOBILECONFIG: Called with callsign={callsign}") + if person.callsign != callsign: + LOGGER.audit( # type: ignore[attr-defined] + "Certificate download denied - callsign mismatch", + extra=build_audit_extra( + action="certificate_download", + outcome="failure", + actor=person.callsign, + target=callsign, + request=request, + error_code="CALLSIGN_MISMATCH", + certificate_type="mobileconfig", + ), + ) + raise HTTPException(status_code=403, detail="Callsign must match authenticated user") + # Make sure the pfx exists, this is no-op if it does + await person.create_pfx() + + LOGGER.audit( # type: ignore[attr-defined] + "Certificate downloaded", + extra=build_audit_extra( + action="certificate_download", + outcome="success", + actor=person.callsign, + request=request, + certificate_type="mobileconfig", + ), + ) + + filename = f"{callsign}_{deployment}.mobileconfig" + return Response( + content=build_mobileconfig(callsign, deployment, person.pfxfile.read_bytes(), person.callsign), + media_type=MEDIA_TYPE, + headers={"Content-Disposition": f'attachment; filename="{filename}"'}, + ) + + @router.get(f"/{{callsign}}_{RMSettings.singleton().deployment_name}.pfx") @router.get("/{callsign}.pfx") @router.get("/{callsign}") @@ -109,8 +188,10 @@ async def get_user_pfx( ), ) + # Empty password everywhere it works, which is everything except Apple. Apple has the + # .mobileconfig endpoint instead. create_pfx always writes this one, so never serve the other. return FileResponse( - path=person.pfxfile, + path=person.nopass_pfxfile, media_type="application/x-pkcs12", filename=f"{callsign}_{RMSettings.singleton().deployment_name}.pfx", ) diff --git a/tests/test_db.py b/tests/test_db.py index 4676eb24..e8993e2e 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -450,6 +450,36 @@ async def wait_for_pfxfile() -> None: assert pfxdata.cert +@pytest.mark.asyncio(loop_scope="session") +async def test_nopass_pfx_parse(dbinit_func) -> None: + """Test that the password-less PFX file gets done and actually has no password""" + _ = dbinit_func + await mtls_init() + callsign = f"PFXMAN02a_{secrets.token_hex(4)}" + person = await Person.create_with_cert(callsign) + + async def wait_for_nopass_pfxfile() -> None: + """wait for the background task to do it's work""" + nonlocal person + while not person.nopass_pfxfile.exists(): + await asyncio.sleep(0.5) + + await asyncio.wait_for(wait_for_nopass_pfxfile(), timeout=5.0) + + # Android skips its password dialog only when the container opens with an empty password + pfxbytes = person.nopass_pfxfile.read_bytes() + pfxdata = cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12(pfxbytes, b"") + assert pfxdata.key + assert pfxdata.cert + # ...and skips asking for a credential name only when the container carries one + assert pfxdata.cert.friendly_name + assert pfxdata.cert.friendly_name.decode("utf-8") == callsign + + # The container the Apple profile embeds must still want its password + with pytest.raises(ValueError): + cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12(person.pfxfile.read_bytes(), b"") + + @pytest.mark.asyncio(loop_scope="session") async def test_productcn_forbid(dbinit_func) -> None: """Test that trying to create enrollment or person with callsign that matches a product CN fails""" diff --git a/tests/test_enrollment.py b/tests/test_enrollment.py index f9ae9df7..638a2cf0 100644 --- a/tests/test_enrollment.py +++ b/tests/test_enrollment.py @@ -1,6 +1,7 @@ """Test enrollment endpoint""" import logging +import plistlib import secrets import uuid from pathlib import Path @@ -622,11 +623,37 @@ async def test_enroll_with_invite_code( unauth_client_session.headers.update({"Authorization": f"Bearer {enrique_jwt}"}) resp = await unauth_client_session.get(f"/api/v1/enduserpfx/{enrollenrique}") resp.raise_for_status() - pfxdata = cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12( - resp.content, enrollenrique.encode("ascii") - ) + # Downloaded containers have no password, see get_user_pfx + pfxdata = cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12(resp.content, b"") assert pfxdata.key assert pfxdata.cert + assert pfxdata.cert.friendly_name + assert pfxdata.cert.friendly_name.decode("utf-8") == enrollenrique + + # The Apple profile carries the passworded container plus its password + resp = await unauth_client_session.get(f"/api/v1/enduserpfx/{enrollenrique}.mobileconfig") + resp.raise_for_status() + assert resp.headers["content-type"].startswith("application/x-apple-aspen-config") + payload = plistlib.loads(resp.content)["PayloadContent"][0] + assert payload["PayloadType"] == "com.apple.security.pkcs12" + embedded = cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12( + payload["PayloadContent"], payload["Password"].encode("utf-8") + ) + assert embedded.key + assert embedded.cert + + # iOS only hands the profile to its installer on a real navigation, which sends no + # Authorization header, so the cookie has to carry the auth on its own. + resp = await unauth_client_session.post("/api/v1/enduserpfx/session") + resp.raise_for_status() + saved_auth = unauth_client_session.headers["Authorization"] + del unauth_client_session.headers["Authorization"] + navurl = f"/api/v1/enduserpfx/{enrollenrique}_{RMSettings.singleton().deployment_name}.mobileconfig" + resp = await unauth_client_session.get(navurl) + resp.raise_for_status() + assert resp.headers["content-type"].startswith("application/x-apple-aspen-config") + assert plistlib.loads(resp.content)["PayloadContent"][0]["PayloadType"] == "com.apple.security.pkcs12" + unauth_client_session.headers.update({"Authorization": saved_auth}) # Fetch also with alternative URLs pfxurl = f"/api/v1/enduserpfx/{enrollenrique}.pfx" @@ -688,7 +715,7 @@ async def test_enroll_with_csr( unauth_client_session.headers.update({"Authorization": f"Bearer {user_jwt}"}) resp = await unauth_client_session.get(f"/api/v1/enduserpfx/{callsign}.pfx") resp.raise_for_status() - pfxdata = cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12(resp.content, callsign.encode("utf-8")) + pfxdata = cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12(resp.content, b"") assert not pfxdata.key assert pfxdata.additional_certs[0] cert = pfxdata.additional_certs[0] diff --git a/tests/test_rasenmaeher_api.py b/tests/test_rasenmaeher_api.py index 3a886f10..00cad71b 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.19.4+260919" @pytest.mark.asyncio(loop_scope="session") diff --git a/uv.lock b/uv.lock index c8c418ab..738a0331 100644 --- a/uv.lock +++ b/uv.lock @@ -2110,7 +2110,7 @@ wheels = [ [[package]] name = "rasenmaeher-api" -version = "1.19.3+260916" +version = "1.19.4+260919" source = { editable = "." } dependencies = [ { name = "aiodns" },