Skip to content

Commit

Permalink
Keep terminal in a sane state when sudo use_pty is used.
Browse files Browse the repository at this point in the history
This fixes #909 and is an alternative to the #922 pull request. When
sudo's use_pty is used with sshuttle, it causes issues with the
terminal. Pull request #712 contains some fixes for this problem.
However, when sshuttle is run with the --daemon option, it left the
user's terminal in a non-sane state. The problem appears to be related
to a socketpair that the firewall uses for communication. By setting
it up slightly differently (see changes to client.py and firewall.py),
the terminal state is no longer disrupted. This commit also changes
line endings of the printed messages from \r\n to \n. This undoes a
change introduced by pull request #712 and is no longer needed.
  • Loading branch information
skuhl authored and brianmay committed Jan 5, 2024
1 parent b4e4680 commit a604d10
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 27 deletions.
2 changes: 1 addition & 1 deletion sshuttle/assembler.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
name = name.decode("ASCII")
nbytes = int(sys.stdin.readline())
if verbosity >= 2:
sys.stderr.write(' s: assembling %r (%d bytes)\r\n'
sys.stderr.write(' s: assembling %r (%d bytes)\n'
% (name, nbytes))
content = z.decompress(sys.stdin.read(nbytes))

Expand Down
3 changes: 2 additions & 1 deletion sshuttle/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ def setup():

try:
debug1("Starting firewall manager with command: %r" % argv)
self.p = ssubprocess.Popen(argv, stdout=s1, preexec_fn=setup)
self.p = ssubprocess.Popen(argv, stdout=s1, stdin=s1,
preexec_fn=setup)
# No env: Talking to `FirewallClient.start`, which has no i18n.
except OSError as e:
# This exception will occur if the program isn't
Expand Down
5 changes: 0 additions & 5 deletions sshuttle/firewall.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,6 @@ def setup_daemon():
# setsid() fails if sudo is configured with the use_pty option.
pass

# because of limitations of the 'su' command, the *real* stdin/stdout
# are both attached to stdout initially. Clone stdout into stdin so we
# can read from it.
os.dup2(1, 0)

return sys.stdin, sys.stdout


Expand Down
9 changes: 1 addition & 8 deletions sshuttle/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,7 @@ def log(s):
prefix = logprefix
s = s.rstrip("\n")
for line in s.split("\n"):
# We output with \r\n instead of \n because when we use
# sudo with the use_pty option, the firewall process, the
# other processes printing to the terminal will have the
# \n move to the next line, but they will fail to reset
# cursor to the beginning of the line. Printing output
# with \r\n endings fixes that problem and does not appear
# to cause problems elsewhere.
sys.stderr.write(prefix + line + "\r\n")
sys.stderr.write(prefix + line + "\n")
prefix = " "
sys.stderr.flush()
except IOError:
Expand Down
24 changes: 12 additions & 12 deletions tests/client/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ def test_log(mock_stderr, mock_stdout):
call.flush(),
]
assert mock_stderr.mock_calls == [
call.write('prefix: message\r\n'),
call.write('prefix: message\n'),
call.flush(),
call.write('prefix: abc\r\n'),
call.write('prefix: abc\n'),
call.flush(),
call.write('prefix: message 1\r\n'),
call.write('prefix: message 1\n'),
call.flush(),
call.write('prefix: message 2\r\n'),
call.write(' line2\r\n'),
call.write(' line3\r\n'),
call.write('prefix: message 2\n'),
call.write(' line2\n'),
call.write(' line3\n'),
call.flush(),
call.write('prefix: message 3\r\n'),
call.write(' line2\r\n'),
call.write(' line3\r\n'),
call.write('prefix: message 3\n'),
call.write(' line2\n'),
call.write(' line3\n'),
call.flush(),
]

Expand All @@ -51,7 +51,7 @@ def test_debug1(mock_stderr, mock_stdout):
call.flush(),
]
assert mock_stderr.mock_calls == [
call.write('prefix: message\r\n'),
call.write('prefix: message\n'),
call.flush(),
]

Expand All @@ -76,7 +76,7 @@ def test_debug2(mock_stderr, mock_stdout):
call.flush(),
]
assert mock_stderr.mock_calls == [
call.write('prefix: message\r\n'),
call.write('prefix: message\n'),
call.flush(),
]

Expand All @@ -101,7 +101,7 @@ def test_debug3(mock_stderr, mock_stdout):
call.flush(),
]
assert mock_stderr.mock_calls == [
call.write('prefix: message\r\n'),
call.write('prefix: message\n'),
call.flush(),
]

Expand Down

0 comments on commit a604d10

Please sign in to comment.