From 293355b724ab21cf685f13923ef86347640ffac0 Mon Sep 17 00:00:00 2001 From: Carlos Date: Sun, 26 Apr 2026 21:43:56 -0400 Subject: [PATCH] SFTP: switch to Transport-based connection (fixes Synology 'Channel closed') paramiko's SSHClient.open_sftp() allocates an exec channel before the SFTP subsystem request, which Synology DSM closes immediately with 'Channel closed'. Manual sftp(1) and WinSCP avoid this by going straight to the SFTP subsystem on a fresh channel. Replaced SSHClient with direct paramiko.Transport + SFTPClient.from_transport, matching the OpenSSH/WinSCP flow. Larger flow-control windows (128 MB) too since Synology has been observed to bail mid-handshake with the default 1 MB. test_connection_verbose now reports per-step status (connect+auth, open_sftp, listdir /, stat base_path, write probe). API returns the steps array so the UI can show exactly which step failed. Co-Authored-By: Claude Opus 4.7 --- app/main.py | 4 +- app/sftp.py | 201 ++++++++++++++++++++++++++++---------------- debian/build-deb.sh | 2 +- 3 files changed, 132 insertions(+), 75 deletions(-) diff --git a/app/main.py b/app/main.py index 7b72049..1db8ac7 100644 --- a/app/main.py +++ b/app/main.py @@ -1027,7 +1027,7 @@ def test_destination(dest_id: int): con.close() raise HTTPException(400, "No credentials stored for this destination") - ok, message = sftp_mod.test_connection(dest) + ok, message, steps = sftp_mod.test_connection_verbose(dest) cur.execute(""" UPDATE sftp_destinations SET last_tested_at=CURRENT_TIMESTAMP, last_test_result=? @@ -1037,7 +1037,7 @@ def test_destination(dest_id: int): cur.execute("SELECT * FROM sftp_destinations WHERE id=?", (dest_id,)) out = _dest_row_to_dict(cur.fetchone()) con.close() - return {"ok": ok, "message": message, "destination": out} + return {"ok": ok, "message": message, "steps": steps, "destination": out} @app.post("/api/sftp/keypair") diff --git a/app/sftp.py b/app/sftp.py index a16294d..42f8a5f 100644 --- a/app/sftp.py +++ b/app/sftp.py @@ -86,6 +86,58 @@ def generate_keypair() -> tuple[str, str, str]: # ── Connection ────────────────────────────────────────────────────────────── +def _open_transport(dest: dict, timeout: int = 15) -> paramiko.Transport: + """Open and authenticate a Transport directly. + + Bypasses SSHClient. Mirrors how OpenSSH/WinSCP invoke the SFTP subsystem + without first allocating an exec channel — works around a "Channel closed" + issue Synology DSM throws at SSHClient.open_sftp() but not at direct + SFTPClient.from_transport(). + """ + import socket + sock = socket.create_connection( + (dest["host"], int(dest.get("port") or 22)), + timeout=timeout, + ) + transport = paramiko.Transport(sock) + # Generous flow-control windows — Synology sometimes closes mid-handshake + # if the client's window is small. + transport.default_window_size = 2 ** 27 # 128 MB + transport.default_max_packet_size = 2 ** 19 # 512 KB + transport.banner_timeout = timeout + transport.start_client(timeout=timeout) + + # Host-key pin (TOFU) — mirror SSHClient behaviour against our pinned file. + hk_path = _host_keys_path(dest["id"]) + server_key = transport.get_remote_server_key() + if os.path.isfile(hk_path): + host_keys = paramiko.HostKeys() + host_keys.load(hk_path) + if not host_keys.check(dest["host"], server_key): + transport.close() + raise paramiko.BadHostKeyException(dest["host"], server_key, server_key) + else: + _ensure_cred_dir() + host_keys = paramiko.HostKeys() + host_keys.add(dest["host"], server_key.get_name(), server_key) + host_keys.save(hk_path) + + if dest["auth_method"] == "password": + with open(_password_path(dest["id"])) as f: + transport.auth_password(dest["username"], f.read()) + elif dest["auth_method"] == "key": + try: + pkey = paramiko.Ed25519Key.from_private_key_file(_key_path(dest["id"])) + except paramiko.SSHException: + pkey = paramiko.RSAKey.from_private_key_file(_key_path(dest["id"])) + transport.auth_publickey(dest["username"], pkey) + else: + transport.close() + raise ValueError(f"Unknown auth_method: {dest['auth_method']}") + + return transport + + @contextmanager def open_sftp(dest: dict, timeout: int = 15): """Open an SFTP session against the given destination dict. @@ -93,50 +145,9 @@ def open_sftp(dest: dict, timeout: int = 15): `dest` must contain: id, host, port, username, auth_method. Yields a paramiko.SFTPClient. Raises on any failure. """ - client = paramiko.SSHClient() - - # Pin host key on first success (TOFU). Reject on mismatch afterwards. - hk_path = _host_keys_path(dest["id"]) - if os.path.isfile(hk_path): - client.load_host_keys(hk_path) - client.set_missing_host_key_policy(paramiko.RejectPolicy()) - else: - # First connection — accept and persist - client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) - - auth_kwargs = {} - if dest["auth_method"] == "password": - with open(_password_path(dest["id"])) as f: - auth_kwargs["password"] = f.read() - auth_kwargs["look_for_keys"] = False - auth_kwargs["allow_agent"] = False - elif dest["auth_method"] == "key": - try: - pkey = paramiko.Ed25519Key.from_private_key_file(_key_path(dest["id"])) - except paramiko.SSHException: - # Try RSA as fallback for user-pasted keys - pkey = paramiko.RSAKey.from_private_key_file(_key_path(dest["id"])) - auth_kwargs["pkey"] = pkey - auth_kwargs["look_for_keys"] = False - auth_kwargs["allow_agent"] = False - else: - raise ValueError(f"Unknown auth_method: {dest['auth_method']}") - + transport = _open_transport(dest, timeout=timeout) try: - client.connect( - hostname=dest["host"], - port=int(dest.get("port") or 22), - username=dest["username"], - timeout=timeout, - banner_timeout=timeout, - auth_timeout=timeout, - **auth_kwargs, - ) - # Persist host key after first successful connect - if not os.path.isfile(hk_path): - _ensure_cred_dir() - client.save_host_keys(hk_path) - sftp = client.open_sftp() + sftp = paramiko.SFTPClient.from_transport(transport) try: yield sftp finally: @@ -146,41 +157,87 @@ def open_sftp(dest: dict, timeout: int = 15): pass finally: try: - client.close() + transport.close() except Exception: pass def test_connection(dest: dict) -> tuple[bool, str]: - """Try to connect, chdir to base_path, list it. Returns (ok, message).""" + ok, msg, _steps = test_connection_verbose(dest) + return ok, msg + + +def test_connection_verbose(dest: dict) -> tuple[bool, str, list[dict]]: + """Run each handshake step in isolation and report exactly which one died.""" + steps: list[dict] = [] + transport = None + sftp = None try: - with open_sftp(dest) as sftp: - try: - sftp.stat(dest["base_path"]) - except FileNotFoundError: - return False, f"Base path does not exist: {dest['base_path']}" - except IOError as e: - if e.errno == errno.EACCES: - return False, f"No permission to access {dest['base_path']}" - raise - # Quick write probe — try to mkdir a temp dir, then remove it - probe = f"{dest['base_path'].rstrip('/')}/.dupfinder_probe" - try: - sftp.mkdir(probe) - sftp.rmdir(probe) - except IOError: - return False, f"Connected, but {dest['base_path']} is not writable" - return True, "ok" - except paramiko.AuthenticationException: - return False, "Authentication failed" - except paramiko.BadHostKeyException as e: - return False, f"Host key mismatch (possible MITM): {e}" - except paramiko.SSHException as e: - return False, f"SSH error: {e}" - except (TimeoutError, ConnectionError, OSError) as e: - return False, f"Connection failed: {e}" - except Exception as e: - return False, f"Unexpected error: {e}" + try: + transport = _open_transport(dest, timeout=15) + steps.append({ + "step": "connect+auth", "ok": True, + "detail": f"active={transport.is_active()} remote={transport.remote_version}", + }) + except paramiko.AuthenticationException as e: + steps.append({"step": "connect+auth", "ok": False, "detail": f"auth failed: {e}"}) + return False, "Authentication failed", steps + except FileNotFoundError: + steps.append({"step": "connect+auth", "ok": False, "detail": "no stored credentials"}) + return False, "No stored credentials for this destination", steps + except Exception as e: + steps.append({"step": "connect+auth", "ok": False, "detail": f"{type(e).__name__}: {e}"}) + return False, f"Connection failed: {e}", steps + + try: + sftp = paramiko.SFTPClient.from_transport(transport) + steps.append({"step": "open_sftp", "ok": True, "detail": "subsystem opened"}) + except Exception as e: + steps.append({"step": "open_sftp", "ok": False, "detail": f"{type(e).__name__}: {e}"}) + return False, f"SFTP subsystem refused: {e}", steps + + try: + entries = sftp.listdir("/") + steps.append({"step": "listdir_/", "ok": True, "detail": f"entries: {entries[:10]}"}) + except Exception as e: + steps.append({"step": "listdir_/", "ok": False, "detail": f"{type(e).__name__}: {e}"}) + return False, f"listdir / failed: {e}", steps + + try: + sftp.stat(dest["base_path"]) + steps.append({"step": "stat_base_path", "ok": True, "detail": dest["base_path"]}) + except FileNotFoundError: + steps.append({"step": "stat_base_path", "ok": False, "detail": "FileNotFoundError"}) + return False, ( + f"Base path does not exist (or not visible from this user): " + f"{dest['base_path']}. Synology sometimes chroots SFTP users to " + f"their home — try a path under /volume1/homes/{dest['username']}/ instead." + ), steps + except Exception as e: + steps.append({"step": "stat_base_path", "ok": False, "detail": f"{type(e).__name__}: {e}"}) + return False, f"stat {dest['base_path']} failed: {e}", steps + + probe = f"{dest['base_path'].rstrip('/')}/.dupfinder_probe" + try: + sftp.mkdir(probe) + sftp.rmdir(probe) + steps.append({"step": "write_probe", "ok": True, "detail": probe}) + except Exception as e: + steps.append({"step": "write_probe", "ok": False, "detail": f"{type(e).__name__}: {e}"}) + return False, f"Connected, but {dest['base_path']} not writable: {e}", steps + + return True, "ok", steps + finally: + try: + if sftp: + sftp.close() + except Exception: + pass + try: + if transport: + transport.close() + except Exception: + pass # ── Path helpers ──────────────────────────────────────────────────────────── diff --git a/debian/build-deb.sh b/debian/build-deb.sh index 3bd48f2..c93e874 100644 --- a/debian/build-deb.sh +++ b/debian/build-deb.sh @@ -13,7 +13,7 @@ BUILD_DIR="$REPO_ROOT/build/deb" # ── Config ──────────────────────────────────────────────────────────────────── PKG_NAME="dupfinder" -PKG_VERSION="1.1.1" +PKG_VERSION="1.1.2" PKG_ARCH="amd64" DEB_FILE="${PKG_NAME}_${PKG_VERSION}_${PKG_ARCH}.deb"