Merge lp:~laney/apport/pkttyagent into lp:~ubuntu-core-dev/ubuntu/hirsute/apport/ubuntu

[Iain Lane]

There are two annoying things with this branch that I'd appreciate some help with.

  1) Some dots '.' appear over the prompt. They should be paused while the command is being executed I think.
  2) You get prompted multiple times for your passphrase if there are multiple root commands. I've not verified this but I think probably killing *once* at the end of apport, and not spawning a fresh agent each time, would maybe fix this.

[Brian Murray (brian-murray)]

Regarding the dots appearing over the prompt, this has happened for as long as I can remember.

[Brian Murray (brian-murray)]

I think calling _kill_pttyagent() after calling report.add_hooks_info() would accomplish issue #2.

[lp:~laney/apport/pkttyagent]

2852. By Iain Lane on 2021-04-07

Move pkttyagent killing to add_hooks_info()

This ensures we only kill the agent after all the hooks have been run.

[Iain Lane (laney)]

Cheers. I did your suggestion but it doesn't fix it. After some further reading it's because we don't allow "auth_admin_keep" for pkexec. I think that's deliberate - or at least should be fixed outside of apport - and this MP lets the information be collected at least.

I think the new method is better though as we avoid spawning it multiple times, which is still nice.

=== modified file 'apport/hookutils.py'
--- apport/hookutils.py	2021-03-24 15:06:11 +0000
+++ apport/hookutils.py	2021-04-07 10:22:32 +0000
@@ -22,6 +22,7 @@
 import stat
 import base64
 import tempfile
+import select
 import shutil
 import locale
 import json
@@ -32,6 +33,7 @@
 import apport.fileutils
 
 _invalid_key_chars_re = re.compile(r'[^0-9a-zA-Z_.-]')
+_AGENT = None
 
 
 def path_to_key(path):
@@ -404,10 +406,57 @@
     return res
 
 
+def _spawn_pkttyagent():
+    global _AGENT
+
+    if _AGENT is not None:
+        return
+    if os.geteuid() == 0:
+        return
+    if not sys.stdin.isatty():
+        return
+    if not os.path.exists('/usr/bin/pkttyagent'):
+        return
+
+    try:
+        (r, w) = os.pipe2(0)
+    except OSError:
+        return
+
+    _AGENT = subprocess.Popen(['pkttyagent', '--notify-fd', str(w), '--fallback'],
+                              close_fds=False,
+                              stdin=subprocess.PIPE,
+                              stdout=subprocess.PIPE)
+
+    os.close(w)
+
+    with select.epoll() as epoll:
+        while True:
+            epoll.register(r, select.EPOLLIN)
+            events = epoll.poll()
+            for fd, event_type in events:
+                if event_type & select.EPOLLHUP:
+                    os.close(r)
+                    return
+    return
+
+
+def kill_pkttyagent():
+    global _AGENT
+
+    if _AGENT is None:
+        return
+
+    _AGENT.terminate()
+    _AGENT.wait()
+    _AGENT = None
+
+
 def _root_command_prefix():
     if os.getuid() == 0:
         return []
     elif os.path.exists('/usr/bin/pkexec'):
+        _spawn_pkttyagent()
         return ['pkexec']
     # the package hook won't have everything it wanted but that's okay
     else:
@@ -425,8 +474,9 @@
     otherwise left as bytes.
     '''
     assert isinstance(command, list), 'command must be a list'
-    return command_output(_root_command_prefix() + command, input, stderr,
-                          keep_locale=True, decode_utf8=decode_utf8)
+    output = command_output(_root_command_prefix() + command, input, stderr,
+                            keep_locale=True, decode_utf8=decode_utf8)
+    return output
 
 
 def attach_root_command_outputs(report, command_map):
 
=== modified file 'apport/report.py'
--- apport/report.py	2020-12-02 22:34:56 +0000
+++ apport/report.py	2021-04-07 10:22:32 +0000
@@ -31,6 +31,7 @@
 import apport
 import apport.fileutils
 from apport.packaging_impl import impl as packaging
+from apport.hookutils import kill_pkttyagent
 
 _data_dir = os.environ.get('APPORT_DATA_DIR', '/usr/share/apport')
 _hook_dir = '%s/package-hooks/' % (_data_dir)
@@ -930,6 +931,11 @@
         return True if the hook requested to stop the report filing process,
         False otherwise.
         '''
+        ret = self._add_hooks_info(ui, package, srcpackage)
+        kill_pkttyagent()
+        return ret
+
+    def _add_hooks_info(self, ui, package, srcpackage):
         # determine package names, unless already given as arguments
         # avoid path traversal
         if not package: