Keep terminal in a sane state when sudo use_pty is used.

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.

5 files changed, 16 insertions, 27 deletions

diff --git a/sshuttle/assembler.py b/sshuttle/assembler.py
index 6eb8800..3cffdee 100644
--- a/sshuttle/assembler.py
+++ b/sshuttle/assembler.py
@@ -18,7 +18,7 @@ while 1:
             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))
 
diff --git a/sshuttle/client.py b/sshuttle/client.py
index 2b0bd18..29c3dfa 100644
--- a/sshuttle/client.py
+++ b/sshuttle/client.py
@@ -278,7 +278,8 @@ class FirewallClient:
 
             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
diff --git a/sshuttle/firewall.py b/sshuttle/firewall.py
index 60662b9..6cc1bd9 100644
--- a/sshuttle/firewall.py
+++ b/sshuttle/firewall.py
@@ -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
 
 
diff --git a/sshuttle/helpers.py b/sshuttle/helpers.py
index 8ff536a..2d747e4 100644
--- a/sshuttle/helpers.py
+++ b/sshuttle/helpers.py
@@ -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:
diff --git a/tests/client/test_helpers.py b/tests/client/test_helpers.py
index 1e8d6bb..ca1aba3 100644
--- a/tests/client/test_helpers.py
+++ b/tests/client/test_helpers.py
@@ -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(),
     ]
 
@@ -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(),
     ]
 
@@ -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(),
     ]
 
@@ -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(),
     ]