diff --git a/README.md b/README.md index b1f08bd..dfffba6 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,8 @@ covered by the test suite / running stack. - [x] Management RBAC (super_admin / broker_admin / operator / auditor) - [x] Dynamic Security integration (backend as the dynsec admin client) - [x] MQTT clients: create / disable / enable / delete, password reset, - random passwords (returned exactly once) + random passwords (returned exactly once), expected client-ID + binding (anti-credential-sharing, spec section 11) - [x] Groups: create / delete, member + role assignment with priorities - [x] Roles: create / delete, ACL editor (all six ACL types, `%u`/`%c` substitution, priorities, draft-then-save) @@ -137,21 +138,33 @@ Hardening done beyond the plan: private keys encrypted at rest (AES-256-GCM) - [x] Server certificate generation (CN, DNS/IP SANs, serverAuth EKU, signed by a platform CA; PEM download) +- [x] Client certificates (CN = MQTT username, clientAuth EKU, + one-time bundle with cert + key + CA chain as file downloads; + identity binding to the client metadata) +- [x] mTLS listeners end-to-end: TLS section in the listener form + (certificate picker, min TLS version, require_certificate, + use_identity_as_username) — verified live with a client-cert + publish through an mTLS listener (spec section 24) - [x] Certificate inventory (serial, fingerprint, SANs, status, expiry) - [x] TLS config generation: listeners with a bound certificate emit certfile/keyfile/cafile; cert files are published to a shared - certs volume before the config is validated/activated + certs volume (uid 1883, key mode 0640) before validation/deploy +- [x] Unpublished-port warning: listeners on ports not mapped by + compose.yaml warn in the preview/deploy (reachable only inside + the Docker network; keep PUBLISHED_PORTS in sync with compose) - [ ] Configuration diff view (revision N-1 → N) - [ ] Configuration rollback (creates a new revision, §62) -- [ ] Reload-vs-restart classification (§28) — only a restart warning on - listener port/bind edits so far, no general classification +- [ ] Reload-vs-restart classification (§28) — the deploy health check + passes on reload, but a NEW listener still needs an explicit + broker restart (docker compose restart mosquitto); surfacing + that in the UI is pending - [ ] CA import (existing external CA) - [ ] Root-key export + removal workflow (§18) -- [ ] Client certificates (managed-key mode, one-time download) -- [ ] CSR signing +- [ ] CSR signing (client-side key generation) - [ ] Certificate revocation + CRL generation/publishing -- [ ] mTLS listeners (model fields + config generation exist; binding a - certificate in the UI is the remaining step) +- [ ] Client-cert disable: revoking access for a connected device + (needs CRL or dynsec disable + kick; the interim path — disabling + the client — already refuses reconnects even with a valid cert) ### Release 3 — Operations, safety, and visibility (§68.3) — PARTIAL diff --git a/backend/app/config.py b/backend/app/config.py index bf85f00..5baceb3 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -46,6 +46,20 @@ class Settings(BaseSettings): default_cert_validity_days: int = 365 default_ca_validity_days: int = 3650 + # Broker container ports published to the host by compose.yaml — + # listeners on other ports are only reachable INSIDE the Docker + # network (the platform generates any port; compose gates access). + # Comma-separated container ports; see compose.yaml `ports:`. + published_ports: str = "1883,8883" + + @property + def published_port_set(self) -> set[int]: + return { + int(p.strip()) + for p in self.published_ports.split(",") + if p.strip().isdigit() + } + @property def is_production(self) -> bool: return self.app_env == "production" diff --git a/backend/app/configgen.py b/backend/app/configgen.py index b29cbdb..a1f93d8 100644 --- a/backend/app/configgen.py +++ b/backend/app/configgen.py @@ -12,6 +12,7 @@ from dataclasses import dataclass, field from sqlalchemy import select from sqlalchemy.orm import Session +from app.config import get_settings from app.models import BrokerSettings, Certificate, ListenerConfig _BOOL = {True: "true", False: "false"} @@ -121,6 +122,20 @@ def generate_config(db: Session) -> GenerationResult: lines.append("") if enabled: + # Compose publishes only a fixed set of broker ports to the host + # (spec section 5). Listeners on other ports are reachable ONLY + # inside the Docker network — surface that before an operator + # wonders why no external device can connect. + published = get_settings().published_port_set + for listener in enabled: + if listener.port not in published: + warnings.append( + f"listener '{listener.name}' (port {listener.port}) is not " + "published to the host by compose.yaml — it is reachable " + "only inside the Docker network. Map the port in " + "compose.yaml (or use a published port like 8883) to " + "make it reachable for external devices." + ) lines.append("# --- Listeners ---") for listener in enabled: block = _listener_block(listener, db, warnings) diff --git a/backend/app/dynsec.py b/backend/app/dynsec.py index a7c2655..6b93eda 100644 --- a/backend/app/dynsec.py +++ b/backend/app/dynsec.py @@ -233,6 +233,19 @@ class DynsecClient: def set_client_password(self, username: str, password: str) -> None: self._command("setClientPassword", username=username, password=password) + def set_client_id(self, username: str, clientid: str | None) -> None: + """Bind (or clear) the expected client ID for a username. + + With a bound client ID, connections must present exactly that + client id (username + password + client id must all match) — this + is what stops one device's credentials being reused by another + program (spec section 11: "bind an expected client ID"). + """ + if clientid is None or clientid == "": + self._command("setClientId", username=username, clientid="") + else: + self._command("setClientId", username=username, clientid=clientid) + def get_client(self, username: str) -> dict[str, Any]: return self._command("getClient", username=username).get("client", {}) diff --git a/backend/app/models.py b/backend/app/models.py index 8924831..627815c 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -173,6 +173,10 @@ class Certificate(Base): parent_id: Mapped[uuid.UUID | None] = mapped_column( ForeignKey("certificates.id", ondelete="SET NULL") ) + # server|client — client certificates are bound to MQTT identities. + cert_type: Mapped[str] = mapped_column(String(20), default="server") + # MQTT client identity this certificate authenticates (client certs). + username: Mapped[str | None] = mapped_column(String(100), index=True) key_algorithm: Mapped[str] = mapped_column(String(20), default="ecdsa-p256") serial_number: Mapped[str] = mapped_column(String(64), unique=True, index=True) dns_sans: Mapped[list] = mapped_column(JSON, default=list) diff --git a/backend/app/pki.py b/backend/app/pki.py index a0dfc67..d0c4015 100644 --- a/backend/app/pki.py +++ b/backend/app/pki.py @@ -220,4 +220,66 @@ def fingerprint(certificate_pem: str) -> str: """SHA-256 fingerprint of a certificate, colon-separated hex.""" cert = x509.load_pem_x509_certificate(certificate_pem.encode("utf-8")) digest = cert.fingerprint(hashes.SHA256()) - return ":".join(f"{b:02x}" for b in digest) \ No newline at end of file + return ":".join(f"{b:02x}" for b in digest) + + +def issue_client_certificate( + *, + ca_certificate_pem: str, + ca_private_key_pem: str, + common_name: str, + validity_days: int, +) -> IssuedPair: + """Issue a client certificate signed by the given CA (spec section 20). + + The CN MUST equal the MQTT username of the dynsec client this + certificate authenticates: with use_identity_as_username the broker + maps CN -> MQTT identity (spec section 24), so a mismatch means the + certificate authenticates as a username that has no dynsec client. + clientAuth EKU, no SANs — device identity only. + """ + if not common_name.strip(): + raise PkiError("common name must not be empty") + + ca_cert = x509.load_pem_x509_certificate(ca_certificate_pem.encode("utf-8")) + ca_key = serialization.load_pem_private_key( + ca_private_key_pem.encode("utf-8"), password=None + ) + # Type narrowing for the checker: certificate signing requires an + # RSA/EC/Ed25519/Ed448 key — DH/DSA keys load but cannot sign certs. + if not isinstance( + ca_key, + ( + rsa.RSAPrivateKey, + ec.EllipticCurvePrivateKey, + ed25519.Ed25519PrivateKey, + ed448.Ed448PrivateKey, + ), + ): + raise PkiError("the CA private key type cannot sign certificates") + + key = _generate_key("ecdsa-p256") + now = datetime.datetime.now(datetime.UTC) + subject = _name(common_name, None, None, None) + + cert = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(ca_cert.subject) + .public_key(key.public_key()) + .serial_number(_serial()) + .not_valid_before(now - datetime.timedelta(minutes=5)) + .not_valid_after(_not_after(now, validity_days)) + .add_extension( + x509.ExtendedKeyUsage([x509.oid.ExtendedKeyUsageOID.CLIENT_AUTH]), + critical=False, + ) + .add_extension(x509.BasicConstraints(ca=False, path_length=None), critical=True) + .sign(ca_key, hashes.SHA256()) + ) + return IssuedPair( + certificate_pem=_pem_cert(cert), + private_key_pem=_pem_key(key), + serial_number=cert.serial_number, + not_valid_after=cert.not_valid_after_utc, + ) \ No newline at end of file diff --git a/backend/app/routers/broker.py b/backend/app/routers/broker.py index 92bda37..5ec250b 100644 --- a/backend/app/routers/broker.py +++ b/backend/app/routers/broker.py @@ -220,6 +220,9 @@ def _publish_certificates(db: Session) -> None: Must run BEFORE the config referencing them is validated/activated, otherwise `mosquitto --test-config` fails on missing files. + Files are owned by uid/gid 1883 (the broker user in the official + image) with key mode 0640 — the broker must be able to read the key, + but no other container user should. """ listeners = db.scalars( select(ListenerConfig).where(ListenerConfig.tls_enabled.is_(True)) @@ -233,25 +236,39 @@ def _publish_certificates(db: Session) -> None: if cert is None or cert.private_key_pem is None: continue # nothing to publish (warned about in configgen) os.makedirs(CERTS_DIR, exist_ok=True) - cert_path = os.path.join(CERTS_DIR, f"{cert.id}.crt") - with open(cert_path, "w", encoding="utf-8") as f: - f.write(cert.certificate_pem) - os.chmod(cert_path, 0o644) - # Private key: decrypted for the broker (DB keeps the AES-GCM copy) + from app.crypto import decrypt_secret - key_path = os.path.join(CERTS_DIR, f"{cert.id}.key") - with open(key_path, "w", encoding="utf-8") as f: - f.write(decrypt_secret(cert.private_key_pem)) - os.chmod(key_path, 0o600) - # Issuing CA certificate (for cafile) + def _write(path: str, content: str, mode: int) -> None: + with open(path, "w", encoding="utf-8") as f: + f.write(content) + os.chmod(path, mode) + try: + # The broker (and this sidecar) run as uid 1883 in the + # official image; chown only works when we are root in + # the container — skip silently in local dev on Windows. + os.chown(path, 1883, 1883) + except (PermissionError, OSError): + pass + + _write( + os.path.join(CERTS_DIR, f"{cert.id}.crt"), + cert.certificate_pem, + 0o644, + ) + _write( + os.path.join(CERTS_DIR, f"{cert.id}.key"), + decrypt_secret(cert.private_key_pem), + 0o640, + ) if cert.parent_id is not None: ca = db.get(Certificate, cert.parent_id) if ca is not None: - ca_path = os.path.join(CERTS_DIR, f"{ca.id}.crt") - with open(ca_path, "w", encoding="utf-8") as f: - f.write(ca.certificate_pem) - os.chmod(ca_path, 0o644) + _write( + os.path.join(CERTS_DIR, f"{ca.id}.crt"), + ca.certificate_pem, + 0o644, + ) @router.get("/config/preview", response_model=ConfigPreviewOut) diff --git a/backend/app/routers/mqtt.py b/backend/app/routers/mqtt.py index 53ec83e..ff33968 100644 --- a/backend/app/routers/mqtt.py +++ b/backend/app/routers/mqtt.py @@ -19,6 +19,7 @@ from app.deps import CurrentUser, require_roles from app.dynsec import DynsecError, dynsec_client from app.models import ManagementRole, MqttClientMetadata from app.schemas import ( + ClientIdUpdate, ClientRoleAssign, GroupIn, GroupMemberAssign, @@ -219,6 +220,38 @@ def reset_password( return MqttPasswordReset(username=username, password=password) +@router.put("/clients/{username}/clientid", status_code=status.HTTP_204_NO_CONTENT) +def set_client_id( + username: str, + body: ClientIdUpdate, + request: Request, + db: DbDep, + _user: BrokerAdmin, +): + """Bind (or clear) the expected client ID (spec section 11). + + With a bound client id, connections must present exactly that id — + preventing one device's credentials from being reused by another + program. The broker kicks active connections when the id changes. + """ + _guard_mgmt_identity(username) + clientid = body.clientid.strip() or None + try: + dynsec_client.set_client_id(username, clientid) + except DynsecError as exc: + raise HTTPException(status.HTTP_404_NOT_FOUND, str(exc)) from exc + audit.record( + db, + "CLIENT_ID_BOUND" if clientid else "CLIENT_ID_CLEARED", + request=request, + object_type="mqtt_client", + object_id=username, + old_value=None, + new_value={"clientid": clientid or ""}, + ) + return None + + @router.post("/clients/{username}/roles", status_code=status.HTTP_204_NO_CONTENT) def assign_client_role( username: str, diff --git a/backend/app/routers/pki.py b/backend/app/routers/pki.py index 55651bb..7017992 100644 --- a/backend/app/routers/pki.py +++ b/backend/app/routers/pki.py @@ -16,17 +16,25 @@ from sqlalchemy import select from sqlalchemy.orm import Session from app import audit -from app.crypto import encrypt_secret +from app.crypto import decrypt_secret, encrypt_secret from app.db import get_db from app.deps import require_roles -from app.models import Certificate, ManagementRole +from app.dynsec import DynsecError, dynsec_client +from app.models import Certificate, ManagementRole, MqttClientMetadata from app.pki import ( PkiError, create_ca as pki_create_ca, fingerprint as pki_fingerprint, + issue_client_certificate, issue_server_certificate, ) -from app.schemas import CaCreate, CertificateOut, ServerCertCreate +from app.schemas import ( + CaCreate, + CertificateBundle, + CertificateOut, + ClientCertCreate, + ServerCertCreate, +) router = APIRouter(prefix="/api/v1/pki", tags=["pki"]) @@ -72,6 +80,7 @@ def create_ca(body: CaCreate, request: Request, db: DbDep, _user: PkiAdmin): private_key_pem=encrypt_secret(issued.private_key_pem), expiry_date=issued.not_valid_after, is_ca=True, + cert_type="ca", key_algorithm=body.key_algorithm, serial_number=str(issued.serial_number), dns_sans=[], @@ -148,6 +157,7 @@ def issue_certificate( expiry_date=issued.not_valid_after, is_ca=False, parent_id=ca.id, + cert_type="server", key_algorithm="ecdsa-p256", serial_number=str(issued.serial_number), dns_sans=body.dns_sans, @@ -166,12 +176,114 @@ def issue_certificate( new_value={ "common_name": body.common_name, "ca": ca.common_name, + "cert_type": "server", "dns_sans": body.dns_sans, }, ) return cert +@router.post( + "/certificates/client", response_model=CertificateBundle, status_code=201 +) +def issue_client_cert( + body: ClientCertCreate, request: Request, db: DbDep, _user: PkiAdmin +): + """Issue a client certificate for an MQTT client (spec sections 20, 24). + + Validates that the dynsec client exists (the certificate CN becomes + the MQTT identity via use_identity_as_username), issues the cert, + binds it to the client metadata, and returns the one-time bundle — + the private key is NEVER retrievable again afterwards. + """ + try: + dynsec_client.get_client(body.username) + except DynsecError as exc: + raise HTTPException( + status.HTTP_404_NOT_FOUND, + f"MQTT client {body.username!r} does not exist at the broker; " + "create it first — the certificate CN must match the username", + ) from exc + + ca = _get_certificate(db, body.ca_id) + if not ca.is_ca: + raise HTTPException( + status.HTTP_400_BAD_REQUEST, "the referenced certificate is not a CA" + ) + if ca.private_key_pem is None: + raise HTTPException( + status.HTTP_409_CONFLICT, + "the CA private key is not held by the platform (offline CA); " + "it cannot issue certificates", + ) + try: + ca_key_pem = decrypt_secret(ca.private_key_pem) + except ValueError as exc: + raise HTTPException( + status.HTTP_500_INTERNAL_SERVER_ERROR, + f"could not decrypt the CA key: {exc}", + ) from exc + + try: + issued = issue_client_certificate( + ca_certificate_pem=ca.certificate_pem, + ca_private_key_pem=ca_key_pem, + common_name=body.username, + validity_days=body.validity_days, + ) + except PkiError as exc: + raise HTTPException(status.HTTP_400_BAD_REQUEST, str(exc)) from exc + + cert = Certificate( + common_name=body.username, + certificate_pem=issued.certificate_pem, + private_key_pem=encrypt_secret(issued.private_key_pem), + expiry_date=issued.not_valid_after, + is_ca=False, + parent_id=ca.id, + cert_type="client", + username=body.username, + key_algorithm="ecdsa-p256", + serial_number=str(issued.serial_number), + dns_sans=[], + ip_sans=[], + fingerprint=pki_fingerprint(issued.certificate_pem), + status="issued", + ) + db.add(cert) + db.flush() + + # Bind the certificate to the client's PostgreSQL metadata (the + # dynsec identity link, spec section 24). + meta = db.get(MqttClientMetadata, body.username) + if meta is None: + meta = MqttClientMetadata(username=body.username, tags=[]) + db.add(meta) + meta.certificate_id = cert.id + db.flush() + + audit.record( + db, + "CERTIFICATE_ISSUED", + request=request, + object_type="certificate", + object_id=str(cert.id), + new_value={ + "common_name": body.username, + "ca": ca.common_name, + "cert_type": "client", + "mqtt_client": body.username, + }, + ) + return CertificateBundle( + certificate_id=cert.id, + username=body.username, + certificate_pem=issued.certificate_pem, + private_key_pem=issued.private_key_pem, + ca_chain_pem=ca.certificate_pem, + ) + + @router.get( "/certificates/{cert_id}/download", response_class=PlainTextResponse, @@ -210,7 +322,7 @@ def delete_certificate( status.HTTP_409_CONFLICT, "the CA still has issued certificates; delete them first", ) - from app.models import ListenerConfig + from app.models import ListenerConfig, MqttClientMetadata bound = db.scalar( select(ListenerConfig.id).where(ListenerConfig.cert_id == cert.id) @@ -220,6 +332,18 @@ def delete_certificate( status.HTTP_409_CONFLICT, "the certificate is bound to a listener; unbind it first", ) + if cert.cert_type == "client": + # Auto-unlink the identity binding (FK is SET NULL at the DB + # level too); the MQTT client itself is not affected. Explicit + # flush: no query follows, so autoflush would never push this. + linked = db.scalar( + select(MqttClientMetadata).where( + MqttClientMetadata.certificate_id == cert.id + ) + ) + if linked is not None: + linked.certificate_id = None + db.flush() db.delete(cert) audit.record( db, diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 2784442..cdd477f 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -169,6 +169,18 @@ class MqttClientOut(BaseModel): last_known_connection: datetime | None +class ClientIdUpdate(BaseModel): + """PUT /clients/{username}/clientid — bind or clear the expected + client ID (spec section 11: "bind an expected client ID"). + + An empty string clears the binding: any client id may then connect + with these credentials. A bound ID restricts connections to exactly + that client id (anti-credential-sharing). + """ + + clientid: str = Field(default="", max_length=200) + + class MqttPasswordReset(BaseModel): """Response for create/reset — plaintext shown exactly once (spec 11).""" @@ -203,6 +215,8 @@ class ListenerIn(BaseModel): tls_min_version: Literal["tlsv1.2", "tlsv1.3"] | None = None require_certificate: bool = False use_identity_as_username: bool = False + # Bound server certificate (PKI): emitted as certfile/keyfile/cafile. + cert_id: uuid.UUID | None = None class ListenerOut(ListenerIn): @@ -269,6 +283,10 @@ class CertificateOut(BaseModel): common_name: str is_ca: bool parent_id: uuid.UUID | None + cert_type: str + # MQTT client this certificate authenticates (client certs only; + # the CN equals the username — spec section 24). + username: str | None = None key_algorithm: str serial_number: str dns_sans: list[str] @@ -278,4 +296,35 @@ class CertificateOut(BaseModel): expiry_date: datetime created_at: datetime - model_config = {"from_attributes": True} \ No newline at end of file + model_config = {"from_attributes": True} + + +class ClientCertCreate(BaseModel): + """Issue a client certificate bound to an MQTT client (spec section 20). + + The certificate CN is the MQTT username — with + use_identity_as_username the broker maps CN -> dynsec identity + (spec section 24), so the dynsec client must already exist. + """ + + username: str = Field(min_length=1, max_length=100) + ca_id: uuid.UUID + validity_days: int = Field(gt=0, le=36500) + + +class CertificateBundle(BaseModel): + """One-time download payload (spec sections 20, 44). + + Contains EVERYTHING including the plaintext private key — shown + exactly once, never retrievable afterwards from the inventory. + """ + + certificate_id: uuid.UUID + username: str + certificate_pem: str + private_key_pem: str + ca_chain_pem: str + download_hint: str = ( + "Store these files securely now — the private key cannot be " + "retrieved again." + ) \ No newline at end of file diff --git a/backend/e2e_mtls.py b/backend/e2e_mtls.py new file mode 100644 index 0000000..ac574ed --- /dev/null +++ b/backend/e2e_mtls.py @@ -0,0 +1,196 @@ +"""E2E: client-certificate mTLS publish chain (spec sections 20, 24). + +Phase deploy: CA -> MQTT client -> role with publish ACL -> client cert +(CN=username) -> server cert -> mTLS listener -> deploy. State is saved +to /tmp/e2e_state.json; the caller then RESTARTS mosquitto (a new listener +needs a restart, not a reload — spec section 28) and runs phase verify. + +Phase verify: connects to mosquitto:8884 with the client certificate, +publishes to an allowed topic (expect OK) and a forbidden topic (expect +deny), then cleans everything up. + +Usage: python /tmp/e2e.py [--phase deploy|verify|all] +""" +import http.cookiejar +import json +import os +import ssl +import sys +import tempfile +import time +import urllib.request + +BASE = "http://127.0.0.1:8000/api/v1" +STATE = "/tmp/e2e_state.json" + +phase = "all" +if "--phase" in sys.argv: + phase = sys.argv[sys.argv.index("--phase") + 1] + +cj = http.cookiejar.CookieJar() +opener = urllib.request.build_opener(urllib.request.HTTPCookieProcessor(cj)) + + +def call(method, path, body=None): + data = json.dumps(body).encode() if body is not None else None + req = urllib.request.Request( + f"{BASE}{path}", + data=data, + headers={"Content-Type": "application/json"}, + method=method, + ) + resp = opener.open(req) + return json.load(resp) if resp.status != 204 else None + + +call("POST", "/auth/login", { + "username": os.environ["ADMIN_INITIAL_USERNAME"], + "password": os.environ["ADMIN_INITIAL_PASSWORD"], +}) + +state = {} +if phase == "verify": + with open(STATE) as f: + state = json.load(f) + +# 1. CA +if phase in ("deploy", "all"): + ca = call("POST", "/pki/ca", { + "common_name": "E2E mTLS CA", "validity_days": 3650, + "key_algorithm": "ecdsa-p256", + }) + print("CA:", ca["id"][:8]) + + # 2. MQTT client + role with a publish ACL (idempotent: clear any + # residue from a previous aborted run first). + for path in ( + "/mqtt/clients/e2e-device", + "/mqtt/roles/e2e-publisher", + ): + try: + call("DELETE", path) + except urllib.error.HTTPError: + pass + call("POST", "/mqtt/clients", {"username": "e2e-device"}) + call("POST", "/mqtt/roles", { + "rolename": "e2e-publisher", "acls": [{ + "acltype": "publishClientSend", "topic": "e2e/%u/data/#", + "allow": True, "priority": 1, + }], + }) + call("POST", "/mqtt/clients/e2e-device/roles", + {"rolename": "e2e-publisher", "priority": 1}) + print("client + role + ACL ready") + + # 3. Client certificate (CN = username) + bundle = call("POST", "/pki/certificates/client", { + "username": "e2e-device", "ca_id": ca["id"], "validity_days": 365, + }) + print("client cert issued:", bundle["certificate_id"][:8]) + + # 4. Server certificate for the listener + server_cert = call("POST", "/pki/certificates", { + "ca_id": ca["id"], "common_name": "mosquitto", "validity_days": 365, + "dns_sans": ["mosquitto", "localhost"], + }) + print("server cert issued:", server_cert["id"][:8]) + + # 5. mTLS listener on 8884 + deploy + listener = call("POST", "/broker/listeners", { + "name": "E2E mTLS", "port": 8884, "protocol": "mqtt", + "tls_enabled": True, "tls_min_version": "tlsv1.2", + "require_certificate": True, "use_identity_as_username": True, + "cert_id": server_cert["id"], + }) + deploy = call("POST", "/broker/config/deploy", {"comment": "E2E mTLS test"}) + print("deployed revision:", deploy.get("revision"), + "broker_healthy:", deploy.get("broker_healthy")) + + state = { + "ca": ca["id"], + "bundle": bundle, + "server_cert": server_cert["id"], + "listener": listener["id"], + } + with open(STATE, "w") as f: + json.dump(state, f) + + if phase == "deploy": + print("state saved — restart mosquitto, then run --phase verify") + sys.exit(0) + +# A NEW listener cannot be added by config reload (spec section 28) — +# the broker must be restarted for it to start listening. The restart is +# performed externally (docker compose restart mosquitto) between the +# deploy phase and the connect phase; this script is invoked with +# --phase deploy|verify to allow that. +import sys +import time + +phase = "all" +if len(sys.argv) > 1 and sys.argv[1] == "--phase": + phase = sys.argv[2] + +# 6. Connect with the client certificate and publish +if phase in ("verify", "all"): + import paho.mqtt.client as mqtt + from paho.mqtt.enums import CallbackAPIVersion + + bundle = state["bundle"] + tmp = tempfile.mkdtemp() + cert_file = os.path.join(tmp, "dev.crt") + key_file = os.path.join(tmp, "dev.key") + ca_file = os.path.join(tmp, "ca.crt") + open(cert_file, "w").write(bundle["certificate_pem"]) + open(key_file, "w").write(bundle["private_key_pem"]) + open(ca_file, "w").write(bundle["ca_chain_pem"]) + + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + ctx.load_verify_locations(ca_file) + ctx.load_cert_chain(cert_file, key_file) + ctx.check_hostname = False # CN is "mosquitto"; connect by DNS name + ctx.verify_mode = ssl.CERT_REQUIRED + + results = {} + client = mqtt.Client(CallbackAPIVersion.VERSION2, client_id="e2e-test") + + def on_connect(cl, u, f, rc, props): + results["connack"] = rc + + client.on_connect = on_connect + client.tls_set_context(ctx) + # The backend container shares backend-net with mosquitto; Docker + # DNS resolves the name (127.0.0.1 here is the BACKEND, not broker). + client.connect("mosquitto", 8884, keepalive=30) + client.loop_start() + time.sleep(2) + + if results.get("connack") == 0: + print("CONNECT: OK — certificate accepted, CN mapped to identity") + allowed = client.publish("e2e/e2e-device/data/1", b"hello", qos=1) + allowed.wait_for_publish(timeout=5) + print("PUBLISH allowed topic rc:", allowed.rc, "(0 = OK)") + denied = client.publish("forbidden/secret", b"x", qos=1) + denied.wait_for_publish(timeout=5) + print("PUBLISH forbidden topic rc:", denied.rc, + "(non-zero = ACL deny enforced)") + else: + print("CONNECT FAILED rc:", results.get("connack")) + client.loop_stop() + +# 7. Cleanup +if phase in ("verify", "all"): + def try_call(method, path, body=None): + try: + call(method, path, body) + except urllib.error.HTTPError: + pass + + try_call("DELETE", f"/broker/listeners/{state['listener']}") + try_call("POST", "/broker/config/deploy", {"comment": "E2E cleanup"}) + try_call("DELETE", f"/pki/certificates/{state['bundle']['certificate_id']}") + try_call("DELETE", f"/pki/certificates/{state['server_cert']}") + try_call("DELETE", f"/pki/certificates/{state['ca']}") + try_call("DELETE", "/mqtt/clients/e2e-device") + try_call("DELETE", "/mqtt/roles/e2e-publisher") + print("cleanup done") \ No newline at end of file diff --git a/backend/migrations/versions/r2_0003_client_certs.py b/backend/migrations/versions/r2_0003_client_certs.py new file mode 100644 index 0000000..88f5013 --- /dev/null +++ b/backend/migrations/versions/r2_0003_client_certs.py @@ -0,0 +1,30 @@ +"""Release 2: client certificates — cert_type + username columns. + +Revision ID: r2_0003 +Revises: r2_0002 +Create Date: 2026-09-19 +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "r2_0003" +down_revision: Union[str, None] = "r2_0002" +branch_labels: Union[str, Sequence[str] | None] = None +depends_on: Union[str, Sequence[str] | None] = None + + +def upgrade() -> None: + op.add_column( + "certificates", + sa.Column("cert_type", sa.String(20), nullable=False, server_default="server"), + ) + op.add_column("certificates", sa.Column("username", sa.String(100), nullable=True)) + op.create_index("ix_certificates_username", "certificates", ["username"]) + + +def downgrade() -> None: + op.drop_index("ix_certificates_username", table_name="certificates") + op.drop_column("certificates", "username") + op.drop_column("certificates", "cert_type") \ No newline at end of file diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 19bdc9b..6f776db 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -116,7 +116,22 @@ def client(db_session, monkeypatch): def set_client_password(self, username, password): pass + def set_client_id(self, username, clientid=None): + # Mirrors the real broker's setClientId: empty string unbinds. + client = self.clients.get(username) + if client is None: + from app.dynsec import DynsecError + + raise DynsecError(f"Client not found: {username}") + client["clientid"] = clientid or "" + def get_client(self, username): + # Mirrors the real broker: getClient on a missing client + # returns a dynsec error, which the router maps to 404. + if username not in self.clients: + from app.dynsec import DynsecError + + raise DynsecError(f"Client not found: {username}") return self.clients[username] def create_group(self, groupname, roles=None): @@ -204,6 +219,11 @@ def client(db_session, monkeypatch): monkeypatch.setattr(mqtt_router, "dynsec_client", fake) + from app.routers import pki as pki_router + + # The PKI client-cert endpoint validates the dynsec client exists. + monkeypatch.setattr(pki_router, "dynsec_client", fake) + with TestClient(main_module.app) as test_client: yield test_client diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index f63fb2c..51b42b5 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -102,6 +102,34 @@ def test_password_reset_still_works_for_normal_clients(admin_client): assert len(body["password"]) >= 20 # random password, shown once +def test_client_id_binding(admin_client): + """Bind/clear the expected client ID (spec section 11) and refuse + it for the management identity.""" + admin_client.post("/api/v1/mqtt/clients", json={"username": "device-1"}) + + bound = admin_client.put( + "/api/v1/mqtt/clients/device-1/clientid", + json={"clientid": "device-ps042"}, + ) + assert bound.status_code == 204 + client = admin_client.get("/api/v1/mqtt/clients/device-1").json() + assert client["clientid"] == "device-ps042" + + # Clearing with an empty string unbinds (any client id may connect). + cleared = admin_client.put( + "/api/v1/mqtt/clients/device-1/clientid", json={"clientid": ""} + ) + assert cleared.status_code == 204 + client = admin_client.get("/api/v1/mqtt/clients/device-1").json() + assert client["clientid"] in (None, "") + + # The management identity is never manageable (existing guard). + refused = admin_client.put( + "/api/v1/mqtt/clients/admin/clientid", json={"clientid": "x"} + ) + assert refused.status_code == 400 + + def test_role_acl_lifecycle(admin_client): admin_client.post("/api/v1/mqtt/roles", json={"rolename": "telemetry"}) @@ -379,4 +407,148 @@ def test_server_certificate_is_cryptographically_valid(admin_client): assert int(inv["serial_number"]) == cert.serial_number from app.pki import fingerprint as pki_fingerprint - assert inv["fingerprint"] == pki_fingerprint(cert_pem) \ No newline at end of file + assert inv["fingerprint"] == pki_fingerprint(cert_pem) + + +# --- Client certificates (spec sections 20, 24) --------------------------------- + + +def test_client_certificate_issuance_and_binding(admin_client, db_session): + """Issue-client-cert requires an existing dynsec client; the bundle + carries cert+key+CA exactly once and binds the identity link.""" + from sqlalchemy import select + + from app.models import MqttClientMetadata + + ca_id = admin_client.post( + "/api/v1/pki/ca", + json={ + "common_name": "Client CA", + "validity_days": 3650, + "key_algorithm": "ecdsa-p256", + }, + ).json()["id"] + + # Without the dynsec client it must 404, not issue a useless cert. + missing = admin_client.post( + "/api/v1/pki/certificates/client", + json={"username": "ghost-device", "ca_id": ca_id, "validity_days": 365}, + ) + assert missing.status_code == 404 + + admin_client.post("/api/v1/mqtt/clients", json={"username": "ps042"}) + response = admin_client.post( + "/api/v1/pki/certificates/client", + json={"username": "ps042", "ca_id": ca_id, "validity_days": 365}, + ) + assert response.status_code == 201, response.text + bundle = response.json() + assert bundle["username"] == "ps042" + assert "BEGIN CERTIFICATE" in bundle["certificate_pem"] + assert "BEGIN PRIVATE KEY" in bundle["private_key_pem"] + assert "BEGIN CERTIFICATE" in bundle["ca_chain_pem"] + + # Identity binding recorded in metadata (spec section 24). + meta = db_session.get(MqttClientMetadata, "ps042") + assert meta is not None + assert str(meta.certificate_id) == bundle["certificate_id"] + + # Inventory shows the client cert with cert_type=client. The bundle + # legitimately carries the private key ONCE (spec section 20); the + # INVENTORY must never leak it. + inventory = admin_client.get("/api/v1/pki/certificates") + certs = inventory.json() + entry = next(c for c in certs if c["id"] == bundle["certificate_id"]) + assert entry["cert_type"] == "client" + assert entry["common_name"] == "ps042" + # The bound MQTT identity is exposed for differentiation in the UI + # (client certs list the device they authenticate, spec section 24). + assert entry["username"] == "ps042" + assert "PRIVATE KEY" not in inventory.text + # Server certs carry no username. + server_entries = [c for c in certs if c.get("cert_type") == "server"] + assert all(c["username"] is None for c in server_entries) + + +def test_client_cert_delete_unlinks_metadata(admin_client, db_session): + from app.models import MqttClientMetadata + + ca_id = admin_client.post( + "/api/v1/pki/ca", + json={"common_name": "Del CA", "validity_days": 3650, "key_algorithm": "ecdsa-p256"}, + ).json()["id"] + admin_client.post("/api/v1/mqtt/clients", json={"username": "unlink-me"}) + bundle = admin_client.post( + "/api/v1/pki/certificates/client", + json={"username": "unlink-me", "ca_id": ca_id, "validity_days": 365}, + ).json() + + deleted = admin_client.delete( + f"/api/v1/pki/certificates/{bundle['certificate_id']}" + ) + assert deleted.status_code == 204 + # The metadata row survives, unlinked (FK behaviour, mirrored in API). + db_session.expire_all() + meta = db_session.get(MqttClientMetadata, "unlink-me") + assert meta is not None + assert meta.certificate_id is None + + +def test_listener_cert_binding_roundtrip(admin_client): + """Listener cert_id must round-trip through create/update (the gap that + previously made TLS unusable from the API).""" + ca_id = admin_client.post( + "/api/v1/pki/ca", + json={"common_name": "Listener CA", "validity_days": 3650, "key_algorithm": "ecdsa-p256"}, + ).json()["id"] + cert_id = admin_client.post( + "/api/v1/pki/certificates", + json={"ca_id": ca_id, "common_name": "broker.local", "validity_days": 365}, + ).json()["id"] + + created = admin_client.post( + "/api/v1/broker/listeners", + json={ + "name": "TLS listener", + "port": 8883, + "protocol": "mqtt", + "tls_enabled": True, + "tls_min_version": "tlsv1.2", + "require_certificate": True, + "use_identity_as_username": True, + "cert_id": cert_id, + }, + ) + assert created.status_code == 201, created.text + listener = created.json() + assert listener["cert_id"] == cert_id + assert listener["require_certificate"] is True + + # The generated config now emits the real certfile/keyfile/cafile. + preview = admin_client.get("/api/v1/broker/config/preview").json() + assert f"certfile /mosquitto/certs/{cert_id}.crt" in preview["config_text"] + assert f"keyfile /mosquitto/certs/{cert_id}.key" in preview["config_text"] + assert f"cafile /mosquitto/certs/{ca_id}.crt" in preview["config_text"] + assert "require_certificate true" in preview["config_text"] + assert "use_identity_as_username true" in preview["config_text"] + + # Update keeps the binding (full-object PUT). + updated = admin_client.put( + f"/api/v1/broker/listeners/{listener['id']}", + json={ + "name": "TLS listener", + "port": 8883, + "protocol": "mqtt", + "tls_enabled": True, + "tls_min_version": "tlsv1.2", + "require_certificate": True, + "use_identity_as_username": True, + "cert_id": cert_id, + }, + ) + assert updated.status_code == 200 + assert updated.json()["cert_id"] == cert_id + + admin_client.delete(f"/api/v1/broker/listeners/{listener['id']}") + admin_client.delete(f"/api/v1/pki/certificates/{cert_id}") + admin_client.delete(f"/api/v1/pki/certificates/{ca_id}") \ No newline at end of file diff --git a/backend/tests/test_configgen.py b/backend/tests/test_configgen.py index 62b34e9..c6e067c 100644 --- a/backend/tests/test_configgen.py +++ b/backend/tests/test_configgen.py @@ -64,6 +64,26 @@ def test_disabled_listeners_are_omitted(session): assert "listener 1884" not in result.config_text +def test_unpublished_port_listener_warns(session): + """Listeners on ports not published by compose warn: they are only + reachable inside the Docker network (spec section 5).""" + session.add(ListenerConfig(name="internal-only", port=8884, protocol="mqtt")) + session.commit() + + result = generate_config(session) + assert any( + "not published to the host" in w and "8884" in w for w in result.warnings + ) + + +def test_published_port_listener_does_not_warn(session): + session.add(ListenerConfig(name="prod", port=8883, protocol="mqtt")) + session.commit() + + result = generate_config(session) + assert not any("not published to the host" in w for w in result.warnings) + + def test_tls_listener_without_pki_warns(session): session.add( ListenerConfig( diff --git a/compose.yaml b/compose.yaml index b593506..0a9c0dc 100644 --- a/compose.yaml +++ b/compose.yaml @@ -18,6 +18,9 @@ x-backend-env: &backend-env DEFAULT_TLS_MIN_VERSION: ${DEFAULT_TLS_MIN_VERSION:-tlsv1.2} DEFAULT_CERT_VALIDITY_DAYS: ${DEFAULT_CERT_VALIDITY_DAYS:-365} DEFAULT_CA_VALIDITY_DAYS: ${DEFAULT_CA_VALIDITY_DAYS:-3650} + # Broker container ports published to the host (compose `ports:`); + # keep in sync when editing the mosquitto port mappings. + PUBLISHED_PORTS: ${PUBLISHED_PORTS:-1883,8883} services: mqtt-db: diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 6fbf778..0238929 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -39,7 +39,7 @@ const NAV_SECTIONS: { items: [ { label: "Authorities", to: "/certificates" }, { label: "Server Certificates", to: "/certificates" }, - { label: "Client Certificates", hint: "Client certs + one-time download (Release 2, later)" }, + { label: "Client Certificates", to: "/certificates#client-certificates" }, { label: "CSRs", hint: "CSR signing (Release 2, later)" }, { label: "Revocations", hint: "CRL management (Release 2, later)" }, ], diff --git a/frontend/src/api.ts b/frontend/src/api.ts index d05792e..d778531 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -93,6 +93,11 @@ export const api = { delete: (path: string) => request(path, { method: "DELETE" }), }; +/** Build a text/plain download URL for a PEM payload (one-time bundles). */ +export function pemDataUrl(pem: string): string { + return `data:application/x-pem-file;charset=utf-8,${encodeURIComponent(pem)}`; +} + // --- Assignment operations (spec sections 11-12) --------------------------- export const assignClientRole = (username: string, rolename: string, priority = -1) => @@ -167,6 +172,8 @@ export interface Certificate { common_name: string; is_ca: boolean; parent_id: string | null; + cert_type: string; + username: string | null; key_algorithm: string; serial_number: string; dns_sans: string[]; @@ -177,6 +184,21 @@ export interface Certificate { created_at: string; } +export interface CertificateBundle { + certificate_id: string; + username: string; + certificate_pem: string; + private_key_pem: string; + ca_chain_pem: string; + download_hint: string; +} + +export interface ClientCertCreate { + username: string; + ca_id: string; + validity_days: number; +} + export interface CaCreate { common_name: string; validity_days: number; @@ -210,6 +232,7 @@ export interface Listener { tls_min_version: string | null; require_certificate: boolean; use_identity_as_username: boolean; + cert_id: string | null; } export interface Revision { diff --git a/frontend/src/pages/BrokerConfig.tsx b/frontend/src/pages/BrokerConfig.tsx index 99abce8..6daf63f 100644 --- a/frontend/src/pages/BrokerConfig.tsx +++ b/frontend/src/pages/BrokerConfig.tsx @@ -1,6 +1,6 @@ import { FormEvent, useEffect, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { api, type Listener, type Revision } from "../api"; +import { api, type Certificate, type Listener, type Revision } from "../api"; interface ConfigPreview { config_text: string; @@ -25,6 +25,7 @@ const EMPTY_LISTENER: Omit = { tls_min_version: null, require_certificate: false, use_identity_as_username: false, + cert_id: null, }; // Curated subset of spec section 31; anything else can be added later @@ -276,6 +277,15 @@ function ListenerForm({ const set = (patch: Partial) => setDraft((d) => ({ ...d, ...patch })); const valid = draft.name.trim() !== "" && draft.port > 0 && draft.port <= 65535; + // Server certificates for the certfile/keyfile/cafile binding. + const serverCerts = useQuery({ + queryKey: ["certificates"], + queryFn: () => api.get("/pki/certificates"), + }); + const certOptions = (serverCerts.data ?? []).filter( + (c) => !c.is_ca && c.cert_type === "server", + ); + const submit = (e: FormEvent) => { e.preventDefault(); if (valid) { @@ -346,6 +356,83 @@ function ListenerForm({ /> +
+
+ + +
+ {draft.tls_enabled && ( + <> +
+ + +
+
+ + +
+
+ + +
+ {draft.require_certificate && ( +
+ + +
+ )} + + )} +

+ + )} {error &&

{error}
} {(clients.error || roles.error || groups.error) && (
@@ -179,6 +283,15 @@ export function ClientsPage() { onChange={(e) => setNewUsername(e.target.value)} placeholder="e.g. ps042" /> + + setNewClientId(e.target.value)} + placeholder="e.g. device-ps042" + />

+ {c.enabled ? "yes" : "no"} Reset password {" "} + {" "}