beiboot preparation fixes

.github/workflows/flatpak-test.yml

@@ -27,6 +27,11 @@ jobs:
 
       - name: Smoke-test the installed flatpak
         run: |
+          export COCKPIT_TEST_SSH_PASS=foobar
+          sudo useradd -c User -s /bin/bash user
+          echo user:$COCKPIT_TEST_SSH_PASS | sudo chpasswd
+          export COCKPIT_TEST_SSH_HOST=user@127.0.0.1
+
           . /etc/profile.d/flatpak.sh
           xvfb-run sh -ec '
             dbus-run-session containers/flatpak/test/test-ssh

containers/flatpak/test/test-browser

@@ -63,10 +63,13 @@ class BrowserTest(unittest.TestCase):
         self.app_actions.activate_action("quit")
         self.assertEqual(self.p_client.wait(), 0)
 
-    def run_js(self, js_name):
+    def run_js(self, js_name, substitutions=None):
         with open(os.path.join(TEST_DIR, js_name)) as f:
             js = f.read()
 
+        for (pattern, replacement) in (substitutions or []):
+            js = js.replace(pattern, replacement)
+
         # WebKit.run_javascript() needs some serializable return value
         js += '\ntrue\n'
 
@@ -118,5 +121,14 @@ class BrowserTest(unittest.TestCase):
         # check that `invalid` was not added and try to remove `localhost` from the list
         self.assertEqual(self.run_js("test-browser-login-remove.js"), "PASS")
 
+        # SSH login
+        host = os.environ.get("COCKPIT_TEST_SSH_HOST")
+        if host:
+            substs = [
+                ("%HOST%", host),
+                ("%PASS%", os.environ["COCKPIT_TEST_SSH_PASS"]),
+            ]
+            self.assertEqual(self.run_js("test-browser-login-ssh.js", substs), "page-load")
+
 
 unittest.main()

containers/flatpak/test/test-browser-login-ssh.js

@@ -0,0 +1,31 @@
+/* global ph_mouse ph_set_result ph_wait_present ph_wait_visible ph_wait_in_text */
+
+async function assert_conversation(match) {
+    await ph_wait_present("#conversation-prompt");
+    await ph_wait_visible("#conversation-prompt");
+    await ph_wait_in_text("#conversation-prompt", match);
+}
+
+async function test() {
+    try {
+        await ph_wait_present("#server-field");
+        document.getElementById("server-field").value = "%HOST%";
+        ph_mouse("#login-button", "click");
+
+        // unknown host key
+        await assert_conversation("authenticity of host");
+        document.getElementById("conversation-input").value = "yes";
+        ph_mouse("#login-button", "click");
+
+        await ph_wait_present("#conversation-prompt");
+        await assert_conversation("password");
+        document.getElementById("conversation-input").value = "%PASS%";
+
+        // this will cause a page load, ending the test
+        ph_mouse("#login-button", "click");
+    } catch (e) {
+        ph_set_result(e);
+    }
+}
+
+test();

containers/flatpak/test/test-lib.js

@@ -64,6 +64,11 @@ function ph_wait_not_visible(sel) {
     }, 10000, "timed out waiting for " + sel + " to be visible");
 }
 
+function ph_wait_in_text(sel, match) {
+    return ph_wait_cond(() => window.cur_doc.querySelector(sel).textContent.includes(match),
+                        1000, `timed out waiting for ${sel} to contain ${match}`);
+}
+
 function ph_wait_count(sel, count) {
     return ph_wait_cond(() => (window.cur_doc.querySelectorAll(sel).length === count), 10000, "timed out waiting for " + sel + " to be visible");
 }
@@ -102,7 +107,7 @@ function ph_mouse(sel, type, x, y, btn, ctrlKey, shiftKey, altKey, metaKey) {
         bubbles: true,
         cancelable: true,
         view: window,
-        detail: detail,
+        detail,
         screenX: left + (x || 0),
         screenY: top + (y || 0),
         clientX: left + (x || 0),

selinux/HACKING.md

@@ -0,0 +1,11 @@
+# Changing cockpit's SELinux policy
+
+The clean way is to edit the policy files and then rebuild the rpms and image with
+`test/image-prepare -q fedora-XX`.
+
+To iterate more quickly, locally `./configure` the build tree with
+`--enable-selinux-policy=targeted`, then you can quickly recompile and install
+the policy into the `c` SSH target with:
+```sh
+make cockpit.pp && scp cockpit.pp c:/tmp/ && ssh c semodule -i /tmp/cockpit.pp
+```

selinux/cockpit.te

@@ -42,15 +42,21 @@ allow cockpit_ws_t self:process setrlimit;
 allow cockpit_ws_t self:tcp_socket create_stream_socket_perms;
 
 kernel_read_system_state(cockpit_ws_t)
+dev_read_sysfs(cockpit_ws_t)
 
 # cockpit-tls can execute cockpit-ws
 can_exec(cockpit_ws_t,cockpit_ws_exec_t)
 
 # systemd can execute cockpit-session
 can_exec(init_t,cockpit_session_exec_t)
 
-# cockpit-ws can execute cockpit-session
+# cockpit-ws can execute cockpit-{ssh,session}
 can_exec(cockpit_ws_t,cockpit_session_exec_t)
+# ... and the real ssh (through beiboot/ferny)
+gen_require(`
+    type ssh_exec_t;
+')
+can_exec(cockpit_ws_t,ssh_exec_t)
 
 # cockpit-ws can read from /dev/urandom
 dev_read_urand(cockpit_ws_t) # for authkey

src/cockpit/beiboot.py

@@ -34,19 +34,25 @@
 from cockpit.channel import ChannelRoutingRule
 from cockpit.channels import PackagesChannel
 from cockpit.jsonutil import JsonObject
-from cockpit.packages import Packages, PackagesLoader
+from cockpit.packages import Packages, PackagesLoader, patch_libexecdir
 from cockpit.peer import Peer
 from cockpit.protocol import CockpitProblem
 from cockpit.router import Router, RoutingRule
 from cockpit.transports import StdioTransport
 
-logger = logging.getLogger(__name__)
+logger = logging.getLogger('cockpit.beiboot')
 
 
 def ensure_ferny_askpass() -> Path:
+    """Create askpass executable
+
+    We need this for the flatpak: ssh and thus the askpass program run on the host (via flatpak-spawn),
+    not the flatpak. Thus we cannot use the shipped cockpit-askpass program.
+    """
     src_path = importlib.resources.files(ferny.__name__) / 'interaction_client.py'
     src_data = src_path.read_bytes()
 
+    # Create the file in $XDG_CACHE_HOME, one of the few locations that a flatpak can write to
     xdg_cache_home = os.environ.get('XDG_CACHE_HOME')
     if xdg_cache_home is None:
         xdg_cache_home = os.path.expanduser('~/.cache')
@@ -185,9 +191,21 @@ async def do_connect_transport(self) -> None:
         cmd: Sequence[str] = ('python3', '-ic', '# cockpit-bridge')
         env: Sequence[str] = ()
 
+        in_flatpak = os.path.exists('/.flatpak-info')
+
         # Remote host?  Wrap command with SSH
         if self.destination != 'localhost':
-            ssh_askpass = ensure_ferny_askpass()
+            if in_flatpak:
+                # we run ssh and thus the helper on the host, always use the xdg-cache helper
+                ssh_askpass = ensure_ferny_askpass()
+            else:
+                # outside of the flatpak we expect cockpit-ws and thus an installed helper
+                askpass = patch_libexecdir('${libexecdir}/cockpit-askpass')
+                assert isinstance(askpass, str)
+                ssh_askpass = Path(askpass)
+                if not ssh_askpass.exists():
+                    logger.error("Could not find cockpit-askpass helper at %r", askpass)
+
             env = (
                 f'SSH_ASKPASS={ssh_askpass!s}',
                 'DISPLAY=x',
@@ -203,7 +221,7 @@ async def do_connect_transport(self) -> None:
             cmd = ('ssh', *host_args, shlex.join(cmd))
 
         # Running in flatpak?  Wrap command with flatpak-spawn --host
-        if os.path.exists('/.flatpak-info'):
+        if in_flatpak:
             cmd = ('flatpak-spawn', '--host',
                    *(f'--env={kv}' for kv in env),
                    *cmd)

src/cockpit/packages.py

@@ -92,6 +92,43 @@ def sortify_version(version: str) -> str:
     return '.'.join(part.zfill(8) for part in version.split('.'))
 
 
+@functools.lru_cache()
+def get_libexecdir() -> str:
+    """Detect libexecdir on current machine
+
+    This only works for systems which have cockpit-ws installed.
+    """
+    for candidate in ['/usr/local/libexec', '/usr/libexec', '/usr/local/lib/cockpit', '/usr/lib/cockpit']:
+        if os.path.exists(os.path.join(candidate, 'cockpit-askpass')):
+            return candidate
+    else:
+        logger.warning('Could not detect libexecdir')
+        # give readable error messages
+        return '/nonexistent/libexec'
+
+
+# HACK: Type narrowing over Union types is not supported in the general case,
+# but this works for the case we care about: knowing that when we pass in an
+# JsonObject, we'll get an JsonObject back.
+J = TypeVar('J', JsonObject, JsonDocument)
+
+
+def patch_libexecdir(obj: J) -> J:
+    if isinstance(obj, str):
+        if '${libexecdir}/cockpit-askpass' in obj:
+            # extra-special case: we handle this internally
+            abs_askpass = shutil.which('cockpit-askpass')
+            if abs_askpass is not None:
+                return obj.replace('${libexecdir}/cockpit-askpass', abs_askpass)
+        return obj.replace('${libexecdir}', get_libexecdir())
+    elif isinstance(obj, dict):
+        return {key: patch_libexecdir(value) for key, value in obj.items()}
+    elif isinstance(obj, list):
+        return [patch_libexecdir(item) for item in obj]
+    else:
+        return obj
+
+
 # A document is a binary stream with a Content-Type, optional Content-Encoding,
 # and optional Content-Security-Policy
 class Document(NamedTuple):
@@ -281,42 +318,6 @@ class PackagesLoader:
         'path-not-exists': lambda p: not os.path.exists(p),
     }
 
-    @staticmethod
-    @functools.lru_cache()
-    def get_libexecdir() -> str:
-        """Detect libexecdir on current machine
-
-        This only works for systems which have cockpit-ws installed.
-        """
-        for candidate in ['/usr/local/libexec', '/usr/libexec', '/usr/local/lib/cockpit', '/usr/lib/cockpit']:
-            if os.path.exists(os.path.join(candidate, 'cockpit-askpass')):
-                return candidate
-        else:
-            logger.warning('Could not detect libexecdir')
-            # give readable error messages
-            return '/nonexistent/libexec'
-
-    # HACK: Type narrowing over Union types is not supported in the general case,
-    # but this works for the case we care about: knowing that when we pass in an
-    # JsonObject, we'll get an JsonObject back.
-    J = TypeVar('J', JsonObject, JsonDocument)
-
-    @classmethod
-    def patch_libexecdir(cls, obj: J) -> J:
-        if isinstance(obj, str):
-            if '${libexecdir}/cockpit-askpass' in obj:
-                # extra-special case: we handle this internally
-                abs_askpass = shutil.which('cockpit-askpass')
-                if abs_askpass is not None:
-                    return obj.replace('${libexecdir}/cockpit-askpass', abs_askpass)
-            return obj.replace('${libexecdir}', cls.get_libexecdir())
-        elif isinstance(obj, dict):
-            return {key: cls.patch_libexecdir(value) for key, value in obj.items()}
-        elif isinstance(obj, list):
-            return [cls.patch_libexecdir(item) for item in obj]
-        else:
-            return obj
-
     @classmethod
     def get_xdg_data_dirs(cls) -> Iterable[str]:
         try:
@@ -368,7 +369,7 @@ def patch_manifest(cls, manifest: JsonObject, parent: Path) -> JsonObject:
 
             manifest = cls.merge_patch(manifest, override)
 
-        return cls.patch_libexecdir(manifest)
+        return patch_libexecdir(manifest)
 
     @classmethod
     def load_manifests(cls) -> Iterable[Manifest]:

test/common/testlib.py

@@ -807,7 +807,12 @@ def logout(self):
         else:
             # happens when cockpit is still running
             self.open_session_menu()
-            self.click('#logout')
+            try:
+                self.click('#logout')
+            except RuntimeError as e:
+                # logging out does destroy the current frame context, it races with the CDP driver finishing the command
+                if "Execution context was destroyed" not in str(e):
+                    raise
         self.wait_visible('#login')
 
         self.machine.allow_restart_journal_messages()

test/verify/check-client

@@ -66,9 +66,7 @@ Command = /usr/bin/env python3 -m cockpit.beiboot
     def logout(self, check_last_host=None):
         b = self.browser
 
-        b.assert_no_oops()
-        b.open_session_menu()
-        b.click('#logout')
+        b.logout()
         # FIXME: This is broken, nothing appears
         # b.wait_text("#brand", "Connect to:")
         if check_last_host:
@@ -82,17 +80,16 @@ Command = /usr/bin/env python3 -m cockpit.beiboot
                               timeout=30)
 
     def testBeibootNoBridge(self):
-        self.m_client.spawn(f"runuser -u admin -- {self.libexecdir}/cockpit-ws --no-tls", "ws.log")
         # set up target machine: no cockpit
         self.m_target.execute("rm /usr/bin/cockpit-bridge; rm -r /usr/share/cockpit")
-
         self.checkLoginScenarios(local_bridge=False)
 
     def testBeibootWithBridge(self):
-        self.m_client.spawn(f"runuser -u admin -- {self.libexecdir}/cockpit-ws --no-tls", "ws.log")
         self.checkLoginScenarios(local_bridge=True)
 
     def checkLoginScenarios(self, *, local_bridge=True):
+        self.m_client.spawn(f"runuser -u admin -- {self.libexecdir}/cockpit-ws --no-tls", "ws.log")
+
         b = self.browser
         b.open("/")