Skip to content

Commit 413a6ee

Browse files
committed
Fix SSH
1 parent 7a834c9 commit 413a6ee

4 files changed

Lines changed: 143 additions & 31 deletions

File tree

ptrlib/connection/ssh.py

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,53 @@ def _ensure_windows_askpass_cmd() -> str:
123123
return str(path)
124124

125125

126+
def _ensure_posix_askpass_sh() -> str:
127+
"""Create a minimal SSH_ASKPASS helper for POSIX and return its path.
128+
129+
OpenSSH can read passwords via an external helper specified by SSH_ASKPASS.
130+
We provide a tiny, non-interactive helper that prints the password from an
131+
environment variable. This avoids fragile prompt parsing and works even
132+
when stdin is not a TTY or when ssh suppresses prompts.
133+
"""
134+
path = Path(tempfile.gettempdir()) / "ptrlib-ssh-askpass.sh"
135+
136+
content = (
137+
"#!/bin/sh\n"
138+
"# ptrlib askpass helper (POSIX)\n"
139+
"# $1 is the prompt (ignored).\n"
140+
"# Print the password from $PTRLIB_SSH_PASSWORD without a trailing newline.\n"
141+
"if [ -n \"$PTRLIB_SSH_PASSWORD\" ]; then\n"
142+
" printf '%s' \"$PTRLIB_SSH_PASSWORD\"\n"
143+
"fi\n"
144+
)
145+
146+
try:
147+
if path.is_file():
148+
try:
149+
if path.read_text(encoding="utf-8", errors="ignore") == content:
150+
# Ensure executable bit is set
151+
try:
152+
os.chmod(path, 0o700)
153+
except Exception:
154+
pass
155+
return str(path)
156+
except Exception:
157+
pass
158+
path.write_text(content, encoding="utf-8")
159+
try:
160+
os.chmod(path, 0o700)
161+
except Exception:
162+
pass
163+
except Exception:
164+
# Fallback to a per-process path
165+
path = Path(tempfile.gettempdir()) / f"ptrlib-ssh-askpass-{os.getpid()}.sh"
166+
path.write_text(content, encoding="utf-8")
167+
with contextlib.suppress(Exception):
168+
os.chmod(path, 0o700)
169+
170+
return str(path)
171+
172+
126173
def _ensure_ssh_option(options: list[str], key: str, value: str) -> None:
127174
"""Append -oKey=Value if the key isn't already present."""
128175
prefix = f"-o{key}="
@@ -194,6 +241,18 @@ def SSH(host: str,
194241
env["SSH_ASKPASS_REQUIRE"] = "force"
195242
# Some builds still require DISPLAY to be non-empty to invoke askpass.
196243
env.setdefault("DISPLAY", "1")
244+
else:
245+
# On POSIX, prefer SSH_ASKPASS to avoid fragile prompt parsing and to
246+
# work even when OpenSSH chooses not to echo prompts to the PTY.
247+
askpass = _ensure_posix_askpass_sh()
248+
env = os.environ.copy()
249+
env["PTRLIB_SSH_PASSWORD"] = password
250+
# Ensure no controlling TTY to force askpass on older OpenSSH builds
251+
env["PTRLIB_START_NEW_SESSION"] = "1"
252+
env["SSH_ASKPASS"] = askpass
253+
env["SSH_ASKPASS_REQUIRE"] = "force"
254+
# Historically OpenSSH only invoked askpass when DISPLAY was set; keep it.
255+
env.setdefault("DISPLAY", "1")
197256

198257
argv = _build_ssh_argv(
199258
host,
@@ -205,34 +264,39 @@ def SSH(host: str,
205264
command=command,
206265
)
207266

208-
# On POSIX, use a PTY for interactive sessions and/or password prompts.
209-
need_tty = (not command) or (password is not None)
267+
# On POSIX, use a PTY for interactive shells when no programmatic password is used.
268+
# If a password is provided, prefer pipes (no controlling TTY) so SSH_ASKPASS is
269+
# reliably used even on older OpenSSH that ignore SSH_ASKPASS_REQUIRE=force.
210270
if os.name != 'nt':
211-
sess = Process(argv, use_tty=need_tty, env=env)
271+
use_local_pty = (password is None) and (not command)
272+
sess = Process(argv, use_tty=use_local_pty, env=env)
212273
else:
213274
sess = Process(argv, env=env)
214275

215276
sess.prompt = ""
216277

217278
if password is not None and os.name != 'nt':
218-
# Handle common prompts robustly (case-insensitive, includes hostkey prompt).
219-
hostkey_re = re.compile(br"(?i)are you sure you want to continue connecting")
220-
password_re = re.compile(br"(?i)password:\s*")
221-
denied_re = re.compile(br"(?i)permission denied")
222-
223-
# Best-effort prompt dance: accept host key prompt, then send password.
224-
# If nothing matches within a short time, continue and let the user drive.
225-
for _ in range(3):
226-
with contextlib.suppress(Exception):
227-
m = sess.recvregex([hostkey_re, password_re, denied_re], timeout=10)
228-
if m.re is hostkey_re:
229-
sess.sendline("yes")
230-
continue
231-
if m.re is password_re:
232-
sess.sendline(password)
233-
break
234-
if m.re is denied_re:
235-
raise PermissionError("SSH authentication failed (Permission denied)")
279+
# If we're using askpass, prompts won't appear on the TTY. Skip prompt parsing.
280+
using_askpass = (env is not None) and (env.get("SSH_ASKPASS_REQUIRE") == "force")
281+
if not using_askpass:
282+
# Handle common prompts robustly (case-insensitive, includes hostkey prompt).
283+
hostkey_re = re.compile(br"(?i)are you sure you want to continue connecting")
284+
password_re = re.compile(br"(?i)password:\s*")
285+
denied_re = re.compile(br"(?i)permission denied")
286+
287+
# Best-effort prompt dance: accept host key prompt, then send password.
288+
# If nothing matches within a short time, continue and let the user drive.
289+
for _ in range(3):
290+
with contextlib.suppress(Exception):
291+
m = sess.recvregex([hostkey_re, password_re, denied_re], timeout=10)
292+
if m.re is hostkey_re:
293+
sess.sendline("yes")
294+
continue
295+
if m.re is password_re:
296+
sess.sendline(password)
297+
break
298+
if m.re is denied_re:
299+
raise PermissionError("SSH authentication failed (Permission denied)")
236300

237301
# Windows: if we forced BatchMode (password=None), fail fast on auth errors.
238302
if os.name == 'nt' and password is None:

ptrlib/connection/tube.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,25 @@ def __init__(self,
8383
else:
8484
path = Path(pcap)
8585

86-
self._pcap: PcapFile = PcapFile(path)
86+
# pcap logging must NEVER crash the application; guard constructor
87+
try:
88+
self._pcap: PcapFile = PcapFile(path)
89+
except Exception:
90+
# Provide a dummy object with no-op methods
91+
class _NoopPcap:
92+
def __init__(self):
93+
self.udp = False
94+
self.remote = "0.0.0.0"
95+
self.remote_port = 0
96+
def send(self, *_a, **_k):
97+
pass
98+
def recv(self, *_a, **_k):
99+
pass
100+
def close(self, *_a, **_k):
101+
pass
102+
def close_send(self, *_a, **_k):
103+
pass
104+
self._pcap = _NoopPcap() # type: ignore[assignment]
87105

88106
# Defer `after` / `sendafter` / `sendlineafter`
89107
self._defer_depth: int = 0

ptrlib/connection/unixproc.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,16 @@ def setup_tty():
449449
else:
450450
popen_args = self._args
451451

452+
# Determine if we should detach from the current session (no controlling TTY).
453+
# SSH with SSH_ASKPASS needs this on older OpenSSH builds where a controlling
454+
# TTY (accessible via /dev/tty) suppresses askpass. The SSH wrapper sets
455+
# PTRLIB_START_NEW_SESSION=1 when a password is provided on POSIX.
456+
start_new_sess = False
457+
try:
458+
start_new_sess = (not use_tty) and (str(self._env.get("PTRLIB_START_NEW_SESSION", "0")) == "1")
459+
except Exception:
460+
start_new_sess = False
461+
452462
# pylint: disable-next=subprocess-popen-preexec-fn
453463
self._proc = subprocess.Popen(
454464
popen_args,
@@ -461,6 +471,7 @@ def setup_tty():
461471
stderr=stderr,
462472
close_fds=True,
463473
bufsize=0,
474+
start_new_session=start_new_sess,
464475
)
465476

466477
with contextlib.suppress(OSError):

ptrlib/filestruct/pcap/emulator.py

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -397,10 +397,15 @@ def advance(self, seconds: float) -> None:
397397

398398
def flush(self) -> None:
399399
"""Flush the file handle and synchronize the file system.
400+
401+
Best-effort: errors are suppressed so pcap logging never breaks
402+
application logic (e.g., when the FD has been invalidated by the
403+
environment/tests).
400404
"""
401405
if self._fh is None:
402406
return
403-
self._fh.flush()
407+
with contextlib.suppress(Exception):
408+
self._fh.flush()
404409

405410
def __enter__(self) -> "PcapFile":
406411
return self
@@ -520,14 +525,28 @@ def _emit_udp(self,
520525
self._write(ts, frame)
521526

522527
def _write(self, ts: float, frame: bytes) -> None:
523-
assert self._fh is not None, "Pcap file already closed"
524-
if ts <= self._last_ts:
525-
ts = self._last_ts + 0.000001
526-
self._last_ts = ts
527-
hdr = _pack_pcap_packet_header(ts, len(frame), len(frame))
528-
self._fh.write(hdr)
529-
self._fh.write(frame)
530-
self._fh.flush()
528+
"""Best-effort writer; swallow I/O errors to avoid affecting callers.
529+
530+
In some environments, file descriptors may get closed or invalidated
531+
unexpectedly (e.g., heavy mocking, embedded REPLs). Any error during
532+
pcap emission is suppressed and the pcap stream is disabled.
533+
"""
534+
fh = self._fh
535+
if fh is None:
536+
return
537+
try:
538+
if ts <= self._last_ts:
539+
ts = self._last_ts + 0.000001
540+
self._last_ts = ts
541+
hdr = _pack_pcap_packet_header(ts, len(frame), len(frame))
542+
fh.write(hdr)
543+
fh.write(frame)
544+
fh.flush()
545+
except Exception:
546+
# Disable further logging on any failure
547+
with contextlib.suppress(Exception):
548+
fh.close()
549+
self._fh = None
531550

532551
def _alloc_ip_id(self) -> int:
533552
self._ip_id = (self._ip_id + 1) & 0xFFFF

0 commit comments

Comments
 (0)