From 9cc4b6aa605037dc0df289de5a455f58298c2020 Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 3 Jun 2026 10:04:37 +0200 Subject: [PATCH] fix(virtual-printer): #1610 add Bambu cipher pin to every slicer-facing TLS context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #620 patch fixed the OpenSSL-3.x-strips-plain-RSA-AES-GCM cipher mismatch on the printer-facing TLSProxy client context. The same fix was never applied to the four other slicer-facing TLS contexts. On hardened distros (Fedora / RHEL with update-crypto-policies, hardened Alpine builds) where the system narrows DEFAULT to forward-secrecy only, the slicer's ClientHello finds no overlap with what Bambuddy offers and the handshake aborts with the slicer reporting code=-1 before any application data flows. The reporter pinpointed the missing set_ciphers call in bind_server.py against the #620 lineage; the audit-wide sweep here extends the same fix to mqtt_server.py, tcp_proxy._create_server_ssl_context (the missing other half of #620), and ftp_server.py. For the three new contexts (bind / mqtt / proxy-server) the cipher string is DEFAULT:AES256-GCM-SHA384:AES128-GCM-SHA256 — verbatim match with the #620 client-side fix. For FTPS the original HIGH baseline is kept (HIGH:AES256-GCM-SHA384:AES128-GCM-SHA256:!aNULL:!MD5:!RC4) so the cipher set stays a strict superset of what shipped before — HIGH offers ~58 suites DEFAULT doesn't (CCM / ARIA / CAMELLIA / DSS) that no Bambu slicer is known to pick, but narrowing a compat surface without proof would violate the existing don't-remove-compat-pinning rule. TLS version pins (TLSv1_2 minimum across all four, TLSv1_2 max on FTPS for the BambuStudio PSK-reuse compat) and verify-mode settings are unchanged — only the cipher list is widened. --- CHANGELOG.md | 1 + .../services/virtual_printer/bind_server.py | 9 + .../services/virtual_printer/ftp_server.py | 16 +- .../services/virtual_printer/mqtt_server.py | 8 + .../app/services/virtual_printer/tcp_proxy.py | 7 + backend/tests/unit/test_vp_tls_ciphers.py | 205 ++++++++++++++++++ 6 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 backend/tests/unit/test_vp_tls_ciphers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a6ced39e..ba4205381 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ All notable changes to Bambuddy will be documented in this file. ### Security - **Path-traversal hardening across the upload / import / file-write surface (routes + services); fifth CI backstop ships alongside** — A private path-traversal report against `POST /api/v1/projects/import/file` traced two attacker-controlled strings being joined to `library_dir` with no resolve + containment check: (a) `linked_folders[*].name` from the request's `project.json` ("Vector A" — an absolute path in this field collapsed `library_dir / "/anywhere"` to `Path("/anywhere")` because pathlib discards the left side when the right is absolute, letting the next `write_bytes` land anywhere the backend could write), and (b) per-entry `zf.namelist()` paths from the ZIP itself ("Vector B" — ZIP filenames carry `..` segments by spec and the join `library_dir / folder_name / relative_path` had no per-component check). Concrete escalation: drop a `.pth` file into the venv's `site-packages` directory for code execution on next service restart; overwrite the JWT signing-secret file to forge an admin token; overwrite `~/.ssh/authorized_keys` or `~/.bashrc` on native installs. **Fix is structural, not just patch the diff** (per [[feedback_dont_dismiss_preexisting]]). New `backend/app/utils/safe_path.py::safe_join_under(parent, *parts)` helper joins under a trusted parent, resolves both sides, asserts `is_relative_to(parent.resolve())`, and rejects up-front empty / null-byte / absolute path components. Wired into `import_project_file` at both vectors. **Adjacent fix from the routes audit**: `GET /api/v1/archives/{id}/photos/{filename}` had NO validation on `filename` and FileResponse-served arbitrary paths — the existing DELETE endpoint at least had a membership check against `archive.photos` (which is UUID-generated on upload), but GET shared neither the check nor any traversal guard. Both GET and DELETE now route through `safe_join_under` for defence-in-depth on top of the membership check. **Second adjacent fix from the services audit**: `ArchiveService.attach_timelapse(archive_id, data, filename)` in `backend/app/services/archive.py:1456` wrote `archive_dir / filename` where `filename` ultimately comes from either a printer's FTP listing (compromised-printer threat model — the printer is part of the trust surface) or the `?filename=...` query param on `POST /api/v1/archives/{id}/timelapse/select`. A malicious printer that returns a directory listing entry with `..` segments could write the timelapse bytes outside the archive directory; the `f.get("name") == filename` gate in the route did not prevent it because the gate is satisfied by whatever the printer claims is on disk. `attach_timelapse` now routes through `safe_join_under(..., http=False)` and returns `False` (logging the rejection) when the join would escape — matching the existing not-found contract of the function rather than raising 400 from inside a background task. **Audit sweep methodology**: AST-walked every Python file under `backend/app/api/routes/` AND `backend/app/services/` for `Path / Name` shapes (the exact shape that produced the original report). 25 additional route-layer sites and 8 additional service-layer sites confirmed safe case-by-case (UUID-generated filenames written by Bambuddy itself, `_safe_filename(...)` / `Path(arg).name` basename-stripped inputs, `os.walk`-discovered names, denylist + format-validated backup names, hardcoded constants iterated through a tuple, DB-stored paths whose write origin already goes through a resolved-and-containment-checked helper). Each safe site got a `# SEC-PATH-OK: ` marker so future audits can trust the inline guard at a glance. Six pre-existing safe-with-marker sites (`library.py` external upload, `archives.py` timelapse output, `projects.py` attachment download/delete, `settings.py` backup extractall) carry the same marker shape. **Fifth CI backstop** `test_route_path_arithmetic_is_safe_joined_or_marked` (`backend/tests/unit/test_no_unsafe_path_joins.py`) AST-walks every Python file in `backend/app/api/routes/` AND `backend/app/services/` and fails the build on any ` / ` join that doesn't either route through `safe_join_under` or carry the marker on the join line. Joins matching the higher-structure shapes (Attribute access, Subscript, f-string, `str(...)` call) are categorically different and out of scope — those are caught by the broader audit sweep, not the regression backstop. The services layer is in scope because it receives values from the routes verbatim AND from external sources Bambuddy has no control over (the printer FTP-listing case above). **Tests**: 17 unit tests for `safe_join_under` covering every escape vector (absolute path, Windows abs path, `..` segments, embedded `..`, null byte, empty string, no parts, non-str, plus legitimate nested-path round-trip); 4 integration tests against `POST /api/v1/projects/import/file` exercising the full FastAPI stack with the verbatim shape from the report (absolute path in `folder_name` → 400 + filesystem assertion that the target file doesn't exist; `..` in `folder_name` → 400; `..` in `relative_path` → 400; legitimate nested ZIP still imports cleanly to guard against the fix being over-strict); 3 unit tests against `ArchiveService.attach_timelapse` exercising the compromised-printer threat model (filename with `..` segments → returns False + no file at the escape target; absolute filename → returns False + no file at `/tmp`; legitimate `timelapse_YYYY-MM-DD_HH-MM-SS.mp4` → returns True + file lands inside archive_dir, guarding against the fix being over-strict). **SECURITY.md** gains a fifth rule + a fifth row in the CI-test mapping table; the rule explicitly names the printer FTP-listing case as in-scope to set the expectation for future services-layer audits. Full 5500+ test backend suite green; ruff clean. ### Fixed +- **Queue / Review / Archive virtual-printer modes now complete the TLS handshake on hardened-distro hosts (#1610, reported by an AI-assisted diagnosis)** — Reporter ran Bambuddy in a container on a hardened-policy host (Fedora / RHEL with `update-crypto-policies` or similar) and OrcaSlicer / BambuStudio failed to connect to any non-proxy-mode virtual printer with `code=-1` after the TCP handshake completed. Switching the same VP to Proxy mode worked. They grepped the container and pinpointed that the #620 cipher-suite fix only patched `tcp_proxy.py::_create_client_ssl_context` (printer-facing) and missed every other slicer-facing VP TLS context. Diagnosis confirmed: real Bambu printers (and slicer paths written to mimic them) offer only the plain-RSA AES-GCM suites `AES256-GCM-SHA384` / `AES128-GCM-SHA256`; on stock OpenSSL these stay in `DEFAULT`, but a system crypto policy that strips them leaves the server side offering ECDHE-only and the slicer's ClientHello finds no overlap — handshake aborts before any data flows. **Audit-wide fix (4 contexts patched, one regression test class per site)**: (a) `backend/app/services/virtual_printer/bind_server.py::_create_tls_context` (port 3002, used by Queue/Review/Archive modes — the literal reported failure) — added `ctx.set_ciphers("DEFAULT:AES256-GCM-SHA384:AES128-GCM-SHA256")`; (b) `backend/app/services/virtual_printer/mqtt_server.py::SimpleMQTTServer.start` (port 8883, slicer MQTT-over-TLS) — same cipher pin; (c) `backend/app/services/virtual_printer/tcp_proxy.py::TLSProxy._create_server_ssl_context` (slicer side of Proxy-mode 3002 — the #620 fix's *other* half that was never written) — same cipher pin; (d) `backend/app/services/virtual_printer/ftp_server.py::VirtualPrinterFTPServer.start` (port 990, FTPS upload) — changed from the historical `HIGH:!aNULL:!MD5:!RC4` to `HIGH:AES256-GCM-SHA384:AES128-GCM-SHA256:!aNULL:!MD5:!RC4`. The `HIGH` baseline is kept verbatim (not replaced with `DEFAULT`) so the cipher set stays a strict superset of what shipped before — the original `HIGH` set offers ~58 ciphers `DEFAULT` doesn't (CCM, ARIA, CAMELLIA, DSS variants); none are picked by any known Bambu slicer, but the [[feedback_dont_remove_compat_pinning]] HARD RULE says don't narrow a compat surface without proof. The `!aNULL:!MD5:!RC4` exclusions are preserved as well. For the three new contexts (a/b/c), the cipher string is `DEFAULT:AES256-GCM-SHA384:AES128-GCM-SHA256` — verbatim match with the #620 client-side fix. **Verified strict-superset** at audit time (`{c['name'] for c in old.get_ciphers()}.issubset({c['name'] for c in new.get_ciphers()})` returns True for all four call sites). Per the [[feedback_vp_regression_matrix]] HARD RULE, the slicer-facing TLS surface gets the fix in one drop instead of a per-mode-per-issue ticket trail. Per [[feedback_dont_remove_compat_pinning]], the `minimum_version = TLSv1_2` and the FTPS `maximum_version = TLSv1_2` pins (the BambuStudio FTPS-data-channel-PSK-reuse compat) are unchanged — only the cipher list is widened, never narrowed. **Tests** (`backend/tests/unit/test_vp_tls_ciphers.py`, new file): 5 cases — one per slicer-facing surface (bind / mqtt / proxy-server / ftp) asserting `AES256-GCM-SHA384` AND `AES128-GCM-SHA256` are in the SSL context's offered cipher list, plus a guard test that the original #620 client-side `tcp_proxy._create_client_ssl_context` still has them so this audit's edits can't accidentally regress that fix. Cipher assertions use the production `CertificateService` to generate a real self-signed CA + per-VP cert pair into `tmp_path` (rather than mocking `load_cert_chain`) so the SSL contexts are configured exactly as production would. Full backend unit suite 3674 passed under `-n 30`; ruff clean. **Why neither maintainer nor the local CI box reproduces the bug**: stock OpenSSL 3.x ships `AES256-GCM-SHA384` / `AES128-GCM-SHA256` in `DEFAULT` already, so the missing `set_ciphers()` calls were harmless on most builds — verifiable with `python -c "import ssl; print(c['name'] for c in ssl.SSLContext().get_ciphers())"`. The bug only manifests on builds where a system crypto policy or vendor build flag narrows the default to forward-secrecy-only. The explicit cipher pins now survive any such narrowing. **What this does NOT do**: the printer-side TLS surface (proxy client context, MQTT printer-client context in `bambu_mqtt.py`) is unchanged — printers always sit behind the unfiltered #620 client pin, and there's no equivalent failure mode reported there. - **External-spool usage is now tracked when the AMS has empty slots in between loaded ones (#1607, reported by @ahmtcnby)** — Reporter had AMS slots 0–2 loaded, slot 3 empty, and an external spool. After every multi-filament print, the external's weight never decremented; assigning the external spool to the empty AMS slot 3 in Bambuddy made the deduction appear correctly. Root cause: when no explicit slot-to-tray mapping is available (path 5 of 6 in `usage_tracker.py::_track_from_3mf`, e.g. the very first print after a fresh container start before the request-topic subscription that captures `ams_mapping` from `print_command` is accepted — the bundle showed `Request topic subscription accepted. ams_mapping capture enabled` only fired at 19:36:29), the tracker falls back to a position-based mapping built from `spoolman_tracking.py::build_ams_tray_lookup`. That helper enumerated every AMS tray by `id` regardless of whether a spool was loaded, so the reporter's layout yielded `available_trays = [0, 1, 2, 3, 254]`. BambuStudio / OrcaSlicer compact their filament-assignment UI by hiding unloaded AMS slots — the slicer's 4th filament is the external, so the 3MF carries slot_ids 1–4 with slot 4 = external. Position-based mapping then routed slot 4 → `available_trays[3]` = AMS0-T3 (the empty slot) instead of 254 (the external). No spool assignment exists at AMS0-T3, so usage was silently skipped at line 1273-1274 and the external spool's weight stayed unchanged. **Fix** (`backend/app/services/usage_tracker.py:1232-1245`): the position-based fallback now filters `build_ams_tray_lookup`'s output to slots whose `tray_type` is non-empty before sorting. `build_ams_tray_lookup` itself is unchanged (its other callers — `spoolman_tracking.store_print_data`, `routes/printers`, `spool_assignment_notifications` — want every physical slot for AMS-state purposes); the filter is applied at the call site so we only narrow what the fallback uses. vt_tray entries are already filtered the same way inside `build_ams_tray_lookup` at line 174 (`if vt.get("tray_type"):`) — this mirrors that behaviour for the AMS side. **Why the reporter's workaround helped**: assigning the external spool to AMS0-T3 in Bambuddy made the wrong-target rewrite accidentally land on the right spool — the empty AMS slot resolved to the same spool the external was fed from. The fix removes the need for that workaround. **Tests**: 2 new in `backend/tests/unit/test_usage_tracker.py::TestPositionBasedFallbackEmptyAmsSlot` — `test_external_routed_correctly_when_ams_has_empty_middle_slot` is the literal #1607 regression (3 AMS slots loaded + 1 empty + external loaded, slicer's slot 4 must charge spool at AMS255-T0, **must NOT** charge anything at AMS0-T3 — explicit `(0, 3) not in handled_trays` assertion); `test_dense_ams_unchanged_no_empty_slots` is the no-empty-slots sanity check that confirms the fix doesn't regress the everyday case (4 AMS slots all loaded + external → slot 5 still maps to external). Path priority order unchanged: explicit `print_cmd` / MQTT / queue / color-match mappings still override the position-based fallback, so this only changes behaviour when none of those paths fired. Full backend pytest 3669 passed under `-n 30`; ruff clean. - **Custom maintenance type "documentation URL" now persists on create (#1596, reported by @BurntOutHylian — with the exact root cause pre-triaged in the issue body)** — POST `/api/v1/maintenance/types` hard-coded every field on the `MaintenanceType` constructor by name (`name`, `description`, `default_interval_hours`, `interval_type`, `icon`, `is_system`) and silently dropped `wiki_url`, even though the Pydantic schema accepted it and the response model echoed it back as `null`. PATCH was fine because it used `data.model_dump(exclude_unset=True) + setattr`, which is why editing a freshly-created type DID save the URL — masking the bug under any "save then immediately fix it" test. **Fix**: add `wiki_url=data.wiki_url` to the constructor call at `routes/maintenance.py:206`. **Frontend nit also addressed in the same drop** (#1596 nit section): `MaintenancePage.tsx:1131` `updateTypeMutation`'s inline `Partial<{...}>` shape listed `name | default_interval_hours | interval_type | icon` only. The value reached the API correctly at runtime because `api.updateMaintenanceType` accepts `Partial` (which includes `wiki_url`), but the local type was misleading — anyone reading the mutation would wrongly conclude `wiki_url` wasn't part of the update payload. Extended the inline shape to include `wiki_url?: string | null`. **Tests**: one new integration test in `test_maintenance_api.py::test_create_custom_type_persists_wiki_url` — POSTs a custom type with a `wiki_url`, asserts the POST response carries it, and verifies via a separate GET round-trip that the value actually committed (defending against the "response echoes request body" failure mode the bug would have masked). Full 5565-test backend suite green; ruff clean; frontend build clean; ESLint zero output; touched MaintenancePage vitest green. - **External-folder `.gcode.3mf` files now show thumbnails, and every ingest path stores the same canonical `file_type` for sliced outputs (#1600, reported by @maziggy)** — Reporter noticed external-folder sliced outputs landed with no thumbnail. Cause: four backend ingest paths classified `LibraryFile.file_type` differently for the same `.gcode.3mf` family. The upload, ZIP-extract, and in-process paths used `os.path.splitext(filename)[1]` which returns `.3mf` for `foo.gcode.3mf`, stored `file_type="3mf"`, and matched the thumbnail-extraction gate at `library.py:1467` (`if file_type == "3mf":`). The external-folder scan path explicitly detected the compound and set `file_type="gcode.3mf"` — preserving the "sliced output" identity — but then skipped both `if file_type == "3mf":` (mismatch) and `if file_type == "gcode":` (also mismatch), so the file landed with `thumbnail_path = None`. Same compound-extension drift that bit #1543's 3D preview gates, just in a different surface that the #1543 frontend audit didn't trace back to. **Unified fix** (per the user's "unify if it's safe" directive): new `classify_file_type(filename)` helper in `library.py` is now the single source of truth — returns `gcode.3mf` for sliced outputs and `ext[1:]` otherwise. Applied to every ingest path: upload (`routes/library.py:1704`), ZIP-extract (`routes/library.py:1998`), external-folder scan (the bug site, plus the manual compound check is replaced), and the in-process `save_3mf_from_bytes()` helper (`routes/library.py:471` — used by MakerWorld import). The external-scan thumbnail gate is widened to `if file_type in ("3mf", "gcode.3mf"):` so a sliced output now goes through ThreeMFParser (a `.gcode.3mf` IS a 3MF zip with `Metadata/plate_1.png` thumbnail; the parser doesn't care about the trailing extension). The gcode-download endpoint at `GET /api/v1/library/files/{id}/gcode` (`routes/library.py:4390`) had the same drift in reverse — its gate was `elif file.file_type == "3mf":` so a row stored with `file_type="gcode.3mf"` (the external-scan path's pre-unification behaviour, and now the canonical going forward) was rejected with HTTP 400. Widened to `elif file.file_type in ("3mf", "gcode.3mf"):` so both ingest histories work. **One-shot DB migration** in `backend/app/core/database.py::run_migrations` backfills existing legacy rows: `UPDATE library_files SET file_type='gcode.3mf' WHERE file_type='3mf' AND LOWER(filename) LIKE '%.gcode.3mf'`. Idempotent (post-update rows no longer match the `file_type='3mf'` predicate, so re-runs at every boot are no-ops) and dialect-neutral (`LOWER` + `LIKE` are identical under SQLite and Postgres per the [[feedback_sqlite_and_postgres_upfront]] HARD RULE; behaviour-identical on Postgres by construction, tested explicitly on SQLite in the new regression suite). Without the backfill, users would have a permanent split state in the DB — old uploads at `3mf`, new uploads at `gcode.3mf` — which would (a) double-bucket sliced outputs in the dashboard stats query at `routes/library.py:4615` (`SELECT file_type, count(*) GROUP BY file_type`) and (b) show two entries in the file-manager filter dropdown for the same conceptual type. **Frontend untouched** — `FileManagerPage.tsx` and `ProjectDetailPage.tsx` already accept both `'3mf'` and `'gcode.3mf'` for Preview-3D, type-pill colour, and the file action gate per the #1543 fix. After the migration the DB only contains canonical values, so the legacy `'3mf'` branches in the frontend become dead code for sliced files — they stay in place to handle any future ingest path I missed (defence in depth — better a redundant gate than an empty card). **Tests**: 13 new in `test_library_classify_file_type.py` covering the helper across every compound / casing / no-extension case; 3 new in `test_library_file_type_backfill_migration.py` (legacy `.gcode.3mf`/`3mf` row backfilled, mixed-case filenames upgraded via `LOWER()`, unrelated `.bak`-suffixed compound substring left untouched, plain `.3mf` / raw `.gcode` / `.stl` untouched, idempotent on re-run); 2 new integration tests in `test_library_api.py` (upload of `.gcode.3mf` now stores `file_type="gcode.3mf"` via the unified path; the gcode-download endpoint accepts a row with `file_type="gcode.3mf"` and returns the embedded gcode). Full backend pytest 5564 passed under `-n 30`; ruff clean; frontend build clean; eslint zero output; i18n parity green at 5007 leaves × 9 locales. diff --git a/backend/app/services/virtual_printer/bind_server.py b/backend/app/services/virtual_printer/bind_server.py index 94e1175cc..ef3f9e85b 100644 --- a/backend/app/services/virtual_printer/bind_server.py +++ b/backend/app/services/virtual_printer/bind_server.py @@ -77,6 +77,15 @@ class BindServer: ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) ctx.load_cert_chain(str(self.cert_path), str(self.key_path)) ctx.minimum_version = ssl.TLSVersion.TLSv1_2 + # Match real Bambu printer cipher behaviour: include the plain-RSA + # AES-GCM suites the slicer's bind/connect path expects. On hardened + # distros (Fedora / RHEL with `update-crypto-policies`, hardened Alpine + # builds) the OpenSSL `DEFAULT` list strips these suites, leaving no + # overlap with the slicer's ClientHello and producing `code=-1` on the + # slicer side (#1610). Same fix the #620 client-side patch applied to + # `tcp_proxy.py::_create_client_ssl_context`; the bind-server / server + # side needs it too. + ctx.set_ciphers("DEFAULT:AES256-GCM-SHA384:AES128-GCM-SHA256") ctx.verify_mode = ssl.CERT_NONE return ctx diff --git a/backend/app/services/virtual_printer/ftp_server.py b/backend/app/services/virtual_printer/ftp_server.py index 55cefdfe6..34b976de9 100644 --- a/backend/app/services/virtual_printer/ftp_server.py +++ b/backend/app/services/virtual_printer/ftp_server.py @@ -642,8 +642,20 @@ class VirtualPrinterFTPServer: self._ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 self._ssl_context.maximum_version = ssl.TLSVersion.TLSv1_2 - # Use standard TLS settings for compatibility - self._ssl_context.set_ciphers("HIGH:!aNULL:!MD5:!RC4") + # Keep the historical `HIGH:!aNULL:!MD5:!RC4` baseline so the cipher + # set stays a strict superset of what shipped before (the previous + # set offered ~58 extra suites — CCM, ARIA, CAMELLIA, DSS variants — + # that no Bambu slicer is known to pick, but the + # [[feedback_dont_remove_compat_pinning]] HARD RULE says don't + # narrow a compat surface without proof). The two explicit additions + # cover the #1610 case on hardened distros (Fedora / RHEL with + # `update-crypto-policies`, hardened Alpine builds) where the system + # policy strips the plain-RSA `AES256-GCM-SHA384` / `AES128-GCM-SHA256` + # suites from `HIGH` — without them present the slicer's FTPS + # ClientHello (which mimics the cipher set real Bambu printers offer) + # finds no overlap and the handshake aborts. Listing them explicitly + # survives any system policy that strips them from `HIGH`. + self._ssl_context.set_ciphers("HIGH:AES256-GCM-SHA384:AES128-GCM-SHA256:!aNULL:!MD5:!RC4") logger.info("FTP SSL context created with standard settings") diff --git a/backend/app/services/virtual_printer/mqtt_server.py b/backend/app/services/virtual_printer/mqtt_server.py index a98e05606..1346d5bdc 100644 --- a/backend/app/services/virtual_printer/mqtt_server.py +++ b/backend/app/services/virtual_printer/mqtt_server.py @@ -276,6 +276,14 @@ class SimpleMQTTServer: ssl_context.verify_mode = ssl.CERT_NONE # Allow TLS 1.2 for broader compatibility (some slicers may not support 1.3) ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2 + # Match real Bambu printer cipher behaviour: include the plain-RSA + # AES-GCM suites the slicer expects. On hardened distros + # (Fedora / RHEL with `update-crypto-policies`, hardened Alpine builds) + # OpenSSL's `DEFAULT` list strips these suites, leaving no overlap + # with the slicer's MQTT-over-TLS ClientHello — handshake fails + # immediately and the slicer reports a connect error before any MQTT + # CONNECT can be sent (#1610 audit). Same shape as the #620 fix. + ssl_context.set_ciphers("DEFAULT:AES256-GCM-SHA384:AES128-GCM-SHA256") # Disable hostname checking ssl_context.check_hostname = False diff --git a/backend/app/services/virtual_printer/tcp_proxy.py b/backend/app/services/virtual_printer/tcp_proxy.py index 500138d19..a70cf5620 100644 --- a/backend/app/services/virtual_printer/tcp_proxy.py +++ b/backend/app/services/virtual_printer/tcp_proxy.py @@ -194,6 +194,13 @@ class TLSProxy: ctx.minimum_version = ssl.TLSVersion.TLSv1_2 # Don't require client certificates ctx.verify_mode = ssl.CERT_NONE + # Match real Bambu printer cipher behaviour on the slicer-facing side + # as well (the #620 fix only patched the printer-facing client context + # below). On hardened distros where OpenSSL `DEFAULT` strips the + # plain-RSA AES-GCM suites, the slicer's ClientHello has no overlap + # with our server's offered suites and the handshake fails before any + # data flows (#1610 audit). + ctx.set_ciphers("DEFAULT:AES256-GCM-SHA384:AES128-GCM-SHA256") return ctx def _create_client_ssl_context(self) -> ssl.SSLContext: diff --git a/backend/tests/unit/test_vp_tls_ciphers.py b/backend/tests/unit/test_vp_tls_ciphers.py new file mode 100644 index 000000000..bda73cc2f --- /dev/null +++ b/backend/tests/unit/test_vp_tls_ciphers.py @@ -0,0 +1,205 @@ +"""Regression tests for #1610: every slicer-facing VP TLS context must +explicitly include the plain-RSA AES-GCM cipher suites that real Bambu +printers (and therefore the BambuStudio / OrcaSlicer client paths) expect. + +Real Bambu printers offer only ``AES256-GCM-SHA384`` / ``AES128-GCM-SHA256`` +(plain RSA key exchange) on their TLS endpoints. Slicers built against +that surface assume the server side will accept those suites. On +distributions whose OpenSSL ``DEFAULT`` cipher list has been narrowed by a +system crypto policy (Fedora / RHEL ``update-crypto-policies``, hardened +Alpine builds), Python's stock ``SSLContext`` ends up offering only +ECDHE/DHE — no overlap with the slicer's ClientHello, the handshake +aborts, and the slicer reports a generic ``code=-1`` connect error. + +The #620 patch fixed this for the printer-facing CLIENT context in +``tcp_proxy.py::_create_client_ssl_context``. #1610 audited the remaining +slicer-facing surface and applied the same explicit cipher pin to every +context that accepts a slicer connection: + +* ``bind_server.py::_create_tls_context`` — port 3002 (bind/detect) +* ``mqtt_server.py`` (inline in ``start``) — port 8883 (MQTT-over-TLS) +* ``tcp_proxy.py::_create_server_ssl_context`` — proxy-mode 3002 +* ``ftp_server.py`` (inline in ``start``) — port 990 (FTPS) + +If any of these regress to a context that no longer offers +``AES256-GCM-SHA384`` / ``AES128-GCM-SHA256``, users on hardened distros +will hit the #1610 / #620 cipher-mismatch failure mode. +""" + +import ssl +from pathlib import Path +from unittest.mock import patch + +import pytest + +from backend.app.services.virtual_printer.bind_server import BindServer +from backend.app.services.virtual_printer.certificate import CertificateService +from backend.app.services.virtual_printer.ftp_server import VirtualPrinterFTPServer +from backend.app.services.virtual_printer.mqtt_server import SimpleMQTTServer +from backend.app.services.virtual_printer.tcp_proxy import TLSProxy + +REQUIRED_CIPHERS = ("AES256-GCM-SHA384", "AES128-GCM-SHA256") + + +def _cert_pair(tmp_path: Path) -> tuple[Path, Path]: + """Generate a real self-signed CA + per-VP cert pair via the production + CertificateService. Returns ``(cert_path, key_path)`` suitable for + ``load_cert_chain`` calls inside the VP services under test.""" + svc = CertificateService(cert_dir=tmp_path, serial="01P00A391800001") + return svc.ensure_certificates() + + +def _assert_required_ciphers(ctx: ssl.SSLContext, where: str) -> None: + """Fail with a useful diagnostic if either required cipher is missing.""" + offered = {c["name"] for c in ctx.get_ciphers()} + missing = [c for c in REQUIRED_CIPHERS if c not in offered] + assert not missing, ( + f"{where}: missing plain-RSA AES-GCM cipher(s) {missing}. " + f"Real Bambu printers / slicers require these on the slicer-facing " + f"TLS surface — see #1610. Offered ciphers: {sorted(offered)}" + ) + + +class TestBindServerTlsCiphers: + def test_create_tls_context_offers_plain_rsa_aes_gcm(self, tmp_path): + cert_path, key_path = _cert_pair(tmp_path) + server = BindServer( + serial="01P00A391800001", + model="C12", + name="vp", + version="01.07.00.00", + bind_address="127.0.0.1", + cert_path=cert_path, + key_path=key_path, + ) + ctx = server._create_tls_context() + assert ctx is not None + _assert_required_ciphers(ctx, "bind_server._create_tls_context") + + +class TestTlsProxyServerCiphers: + """Slicer-facing side of proxy mode — was not patched by the #620 fix.""" + + def test_create_server_ssl_context_offers_plain_rsa_aes_gcm(self, tmp_path): + cert_path, key_path = _cert_pair(tmp_path) + proxy = TLSProxy( + name="Bind-TLS", + listen_port=3002, + target_host="127.0.0.1", + target_port=3002, + server_cert_path=str(cert_path), + server_key_path=str(key_path), + on_connect=lambda cid: None, + on_disconnect=lambda cid: None, + bind_address="127.0.0.1", + ) + ctx = proxy._create_server_ssl_context() + _assert_required_ciphers(ctx, "tcp_proxy._create_server_ssl_context") + + def test_create_client_ssl_context_still_offers_plain_rsa_aes_gcm(self, tmp_path): + """The original #620 fix must remain in place.""" + cert_path, key_path = _cert_pair(tmp_path) + proxy = TLSProxy( + name="Bind-TLS", + listen_port=3002, + target_host="127.0.0.1", + target_port=3002, + server_cert_path=str(cert_path), + server_key_path=str(key_path), + on_connect=lambda cid: None, + on_disconnect=lambda cid: None, + bind_address="127.0.0.1", + ) + ctx = proxy._create_client_ssl_context() + _assert_required_ciphers(ctx, "tcp_proxy._create_client_ssl_context") + + +class TestMqttServerTlsCiphers: + """MQTT server builds its SSLContext inline in start(); intercept the + ``asyncio.start_server`` call so the test doesn't actually bind a port.""" + + @pytest.mark.asyncio + async def test_start_configures_plain_rsa_aes_gcm(self, tmp_path): + cert_path, key_path = _cert_pair(tmp_path) + server = SimpleMQTTServer( + serial="01P00A391800001", + access_code="deadbeef", + cert_path=cert_path, + key_path=key_path, + model="C12", + bind_address="127.0.0.1", + ) + + captured: dict[str, ssl.SSLContext] = {} + + async def _capture(*_args, ssl=None, **_kwargs): + captured["ctx"] = ssl + + class _FakeServer: + sockets = [] + + def close(self): + pass + + async def wait_closed(self): + pass + + def is_serving(self): + return False + + return _FakeServer() + + with patch("asyncio.start_server", _capture): + await server.start() + try: + assert "ctx" in captured, "asyncio.start_server was not invoked" + _assert_required_ciphers(captured["ctx"], "mqtt_server.start") + finally: + await server.stop() + + +class TestFtpServerTlsCiphers: + """FTP server builds its SSLContext inline in start().""" + + @pytest.mark.asyncio + async def test_start_configures_plain_rsa_aes_gcm(self, tmp_path): + cert_path, key_path = _cert_pair(tmp_path) + upload_dir = tmp_path / "uploads" + upload_dir.mkdir() + server = VirtualPrinterFTPServer( + upload_dir=upload_dir, + access_code="deadbeef", + cert_path=cert_path, + key_path=key_path, + bind_address="127.0.0.1", + ) + + captured: dict[str, ssl.SSLContext] = {} + + async def _capture(*_args, ssl=None, **_kwargs): + captured["ctx"] = ssl + + class _FakeServer: + sockets = [] + + def close(self): + pass + + async def wait_closed(self): + pass + + def is_serving(self): + return False + + async def serve_forever(self): + pass + + return _FakeServer() + + with patch("asyncio.start_server", _capture): + await server.start() + try: + assert "ctx" in captured, "asyncio.start_server was not invoked" + _assert_required_ciphers(captured["ctx"], "ftp_server.start") + finally: + await server.stop()