Browse Source

Fix: cap TLS to v1.2 for P2S FTPS to dodge vsFTPd session-reuse bug (#1401)

  Python 3.13 negotiates TLS 1.3 by default. The P2S firmware 01.02.00.00
  vsFTPd build doesn't tolerate TLS 1.3's async session-ticket model on
  the FTPS data channel — session resumption races, the data channel gets
  torn down mid-stream, uploads land truncated at a chunk boundary, and
  the printer replies 426 instead of 226. Visible to the user as "unable
  to parse 3mf file" 30 s into the print.

  Capping the SSL context's maximum_version to TLS 1.2 makes session
  resumption synchronous and uploads complete normally.

  Follow the per-model pattern established by camera_profiles.py in the
  #1395 follow-up: add backend/app/services/ftp_profiles.py with a frozen
  FTPProfile dataclass and a per-model registry. Only P2S (display name
  + N7 SSDP code) gets the cap today. X1C, H2D, P1S, A1 stay on negotiated
  TLS 1.3 — the maintainer's dogfooded printers see zero behaviour change.
maziggy 1 week ago
parent
commit
6f050708da

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 15 - 3
backend/app/services/bambu_ftp.py

@@ -35,15 +35,20 @@ class ImplicitFTP_TLS(FTP_TLS):
     A1/A1 Mini printers have issues with SSL on the data channel entirely and
     timeout waiting for transfer completion. Set skip_session_reuse=True for A1
     printers to skip SSL on the data channel (control channel remains encrypted).
+
+    Optionally caps the SSL context's maximum TLS version to v1.2 (P2S firmware
+    01.02.00.00 needs this — see :mod:`ftp_profiles` and #1401).
     """
 
-    def __init__(self, *args, skip_session_reuse: bool = False, **kwargs):
+    def __init__(self, *args, skip_session_reuse: bool = False, cap_tls_v1_2: bool = False, **kwargs):
         super().__init__(*args, **kwargs)
         self._sock = None
         self.skip_session_reuse = skip_session_reuse
         self.ssl_context = ssl.create_default_context()
         self.ssl_context.check_hostname = False
         self.ssl_context.verify_mode = ssl.CERT_NONE
+        if cap_tls_v1_2:
+            self.ssl_context.maximum_version = ssl.TLSVersion.TLSv1_2
 
     def connect(self, host="", port=990, timeout=-999, source_address=None):
         """Connect to host, wrapping socket in TLS immediately (implicit FTPS)."""
@@ -150,11 +155,18 @@ class BambuFTPClient:
         """Connect to the printer FTP server (implicit FTPS on port 990)."""
         try:
             use_prot_c = self._should_use_prot_c()
+            from backend.app.services.ftp_profiles import get_ftp_profile
+
+            profile = get_ftp_profile(self.printer_model)
             logger.debug(
                 f"FTP connecting to {self.ip_address}:{self.FTP_PORT} "
-                f"(timeout={self.timeout}s, model={self.printer_model}, prot_c={use_prot_c})"
+                f"(timeout={self.timeout}s, model={self.printer_model}, prot_c={use_prot_c}, "
+                f"cap_tls_v1_2={profile.cap_tls_v1_2})"
+            )
+            self._ftp = ImplicitFTP_TLS(
+                skip_session_reuse=use_prot_c,
+                cap_tls_v1_2=profile.cap_tls_v1_2,
             )
-            self._ftp = ImplicitFTP_TLS(skip_session_reuse=use_prot_c)
             self._ftp.connect(self.ip_address, self.FTP_PORT, timeout=self.timeout)
             logger.debug("FTP connected, logging in as bblp")
             self._ftp.login("bblp", self.access_code)

+ 99 - 0
backend/app/services/ftp_profiles.py

@@ -0,0 +1,99 @@
+"""Per-printer-model FTP tuning knobs.
+
+Mirrors the shape of :mod:`backend.app.services.camera_profiles` — a
+small registry of per-model overrides so quirky firmwares can be
+tuned without sprinkling ``if model == "X":`` branches through
+``bambu_ftp.py``. Adding a new model's quirk is a config edit (an
+entry in ``_PROFILES`` plus the alias for its internal SSDP code if
+needed), not another hard-coded branch.
+
+The default profile matches the historical pre-fix behaviour, so
+every model that doesn't have an entry here keeps its existing FTP
+behaviour byte-for-byte.
+
+Currently only the TLS-version cap lives here (P2S firmware
+01.02.00.00 needs it — see ``cap_tls_v1_2`` below). The A1
+data-channel-plaintext quirk still lives in :class:`BambuFTPClient`
+via ``A1_MODELS`` / ``skip_session_reuse``; folding that into a
+profile field is a future cleanup, not load-bearing for this fix.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+
+
+@dataclass(frozen=True)
+class FTPProfile:
+    """Tuning knobs for one printer model's FTP path.
+
+    All defaults reflect the historical behaviour. Models with quirky
+    firmware override individual fields rather than re-defining the
+    whole profile.
+    """
+
+    # Pin the SSL context's ``maximum_version`` to TLS 1.2.
+    #
+    # Python 3.13's default ``ssl.create_default_context()`` negotiates
+    # TLS 1.3 when both peers support it. The Bambuddy Docker image is
+    # ``python:3.13-slim-trixie``, so every Docker user gets 1.3 by
+    # default. Some Bambu printer firmwares (P2S 01.02.00.00 confirmed
+    # by @iitazz, #1401) implement session reuse on the FTPS data
+    # channel against an old vsFTPd build that doesn't tolerate TLS
+    # 1.3's asynchronous session-ticket model: the data channel gets
+    # torn down mid-stream and the upload aborts with 426 "Failure
+    # reading network stream" — visible as a clean truncation at a
+    # chunk boundary (one reporter saw exactly 7 × 64 KB landed on
+    # the printer). Capping to TLS 1.2 makes session resumption
+    # synchronous and the upload completes normally.
+    #
+    # **Defaults to False** — only applied to printer models where a
+    # reporter has confirmed the symptom. Existing P1S / X1C / H2D
+    # installs that work fine today stay on the negotiated TLS 1.3.
+    # This is deliberately conservative; flipping a printer to the
+    # capped path is a config edit when a new model surfaces the
+    # same bug.
+    cap_tls_v1_2: bool = False
+
+
+# ---------------------------------------------------------------------------
+# Profile registry
+# ---------------------------------------------------------------------------
+
+# Default profile = historical behaviour. Used for every model that
+# doesn't have an entry in ``_PROFILES``.
+DEFAULT_PROFILE = FTPProfile()
+
+# Per-model overrides. Keys are uppercase display names (e.g. "P2S")
+# AFTER alias normalisation, so internal SSDP codes ("N7") resolve via
+# ``_MODEL_ALIASES`` below.
+_PROFILES: dict[str, FTPProfile] = {
+    # P2S firmware 01.02.00.00 trips the vsFTPd + TLS 1.3 session-reuse
+    # bug on the FTPS data channel (#1401, reporter @iitazz). Cap to
+    # TLS 1.2 so session resumption is synchronous and the upload
+    # completes.
+    "P2S": FTPProfile(
+        cap_tls_v1_2=True,
+    ),
+}
+
+# SSDP internal codes that should resolve to a display-name profile.
+# Mirrors the same map in :mod:`camera_profiles`.
+_MODEL_ALIASES: dict[str, str] = {
+    "N7": "P2S",  # P2S internal SSDP code
+}
+
+
+def get_ftp_profile(model: str | None) -> FTPProfile:
+    """Return the :class:`FTPProfile` for *model*, or the default.
+
+    ``model`` can be either a display name (e.g. ``"P2S"``) or an
+    internal SSDP code (e.g. ``"N7"``). Unknown / missing models fall
+    back to :data:`DEFAULT_PROFILE` so the FTP path is never blocked
+    on a missing entry.
+    """
+    if not model:
+        return DEFAULT_PROFILE
+    key = model.upper().strip()
+    key = _MODEL_ALIASES.get(key, key)
+    return _PROFILES.get(key, DEFAULT_PROFILE)

+ 93 - 0
backend/tests/unit/services/test_ftp_profiles.py

@@ -0,0 +1,93 @@
+"""Per-model FTP profile registry (#1401).
+
+Mirrors ``test_camera_profiles.py`` in shape — the FTP profile module
+follows the same pattern.
+"""
+
+import ssl
+
+from backend.app.services.ftp_profiles import (
+    DEFAULT_PROFILE,
+    FTPProfile,
+    get_ftp_profile,
+)
+
+
+def test_default_profile_does_not_cap_tls():
+    """Default profile keeps the historical Python-default TLS negotiation
+    (typically TLS 1.3 on Python 3.13). Capping would be a silent
+    regression for users who work fine today."""
+    assert DEFAULT_PROFILE.cap_tls_v1_2 is False
+
+
+def test_unknown_model_returns_default():
+    """Unknown / missing models fall back to DEFAULT_PROFILE so the FTP
+    path is never blocked on a missing entry."""
+    assert get_ftp_profile(None) is DEFAULT_PROFILE
+    assert get_ftp_profile("") is DEFAULT_PROFILE
+    assert get_ftp_profile("Unknown Future Model") is DEFAULT_PROFILE
+
+
+def test_p2s_caps_tls_v1_2():
+    """P2S firmware 01.02.00.00 trips a vsFTPd + TLS 1.3 session-reuse
+    bug on the data channel; the profile must cap to TLS 1.2 so session
+    resumption is synchronous (#1401, reporter @iitazz)."""
+    profile = get_ftp_profile("P2S")
+    assert profile.cap_tls_v1_2 is True
+
+
+def test_p2s_internal_ssdp_code_resolves_to_p2s():
+    """SSDP internal code N7 → P2S profile. Camera profiles do the same
+    thing — keeps callers free of the code↔display-name mapping."""
+    profile = get_ftp_profile("N7")
+    assert profile.cap_tls_v1_2 is True
+
+
+def test_lookup_is_case_insensitive():
+    """Printer.model may carry mixed case; the lookup normalises."""
+    assert get_ftp_profile("p2s").cap_tls_v1_2 is True
+    assert get_ftp_profile("P2s").cap_tls_v1_2 is True
+
+
+def test_non_capped_models_still_default():
+    """Spot-check: the models the user dogfoods today (X1C, H2D) stay on
+    the default profile. Adding the P2S override must not accidentally
+    flip these."""
+    assert get_ftp_profile("X1C").cap_tls_v1_2 is False
+    assert get_ftp_profile("H2D").cap_tls_v1_2 is False
+    assert get_ftp_profile("P1S").cap_tls_v1_2 is False
+    assert get_ftp_profile("A1").cap_tls_v1_2 is False
+
+
+def test_profile_is_frozen():
+    """FTPProfile is a frozen dataclass — runtime mutation should raise.
+    Same guarantee CameraProfile has."""
+    try:
+        DEFAULT_PROFILE.cap_tls_v1_2 = True  # type: ignore[misc]
+    except Exception as e:
+        assert "frozen" in str(e).lower() or "FrozenInstanceError" in type(e).__name__
+        return
+    raise AssertionError("FTPProfile should be frozen but assignment succeeded")
+
+
+def test_cap_tls_v1_2_actually_applied_to_ssl_context():
+    """Pins the integration: when ``cap_tls_v1_2=True`` is passed to the
+    FTPS subclass, the SSL context's ``maximum_version`` is set to
+    TLSv1.2. Guards against a future refactor that drops the wiring
+    between profile and context (the registry would still look
+    correct, but the cap would silently stop applying)."""
+    from backend.app.services.bambu_ftp import ImplicitFTP_TLS
+
+    capped = ImplicitFTP_TLS(cap_tls_v1_2=True)
+    assert capped.ssl_context.maximum_version == ssl.TLSVersion.TLSv1_2
+
+    uncapped = ImplicitFTP_TLS(cap_tls_v1_2=False)
+    # MAXIMUM_SUPPORTED is the "no cap applied" sentinel for SSLContext.
+    assert uncapped.ssl_context.maximum_version == ssl.TLSVersion.MAXIMUM_SUPPORTED
+
+
+def test_ftp_profile_dataclass_default_constructible():
+    """Sanity: FTPProfile() with no args yields the default profile
+    (every field has a default)."""
+    fresh = FTPProfile()
+    assert fresh == DEFAULT_PROFILE

Some files were not shown because too many files changed in this diff