shell: Require confirmation before connecting to remote machines

doc/guide/Makefile-guide.am

@@ -21,6 +21,7 @@ GUIDE_INCLUDES = \
 	doc/guide/cockpit-session.xml \
 	doc/guide/cockpit-spawn.xml \
 	doc/guide/cockpit-util.xml \
+	doc/guide/multi-host.xml \
 	doc/guide/authentication.xml \
 	doc/guide/embedding.xml \
 	doc/guide/feature-firewall.xml \

doc/guide/cockpit-guide.xml

@@ -27,6 +27,7 @@
     <xi:include href="https.xml"/>
     <xi:include href="listen.xml"/>
     <xi:include href="startup.xml"/>
+    <xi:include href="multi-host.xml"/>
     <xi:include href="authentication.xml"/>
     <xi:include href="sso.xml"/>
     <xi:include href="cert-authentication.xml"/>

doc/guide/multi-host.xml

@@ -0,0 +1,54 @@
+<?xml version="1.0"?>
+<!DOCTYPE chapter PUBLIC "-//OASIS//DTD DocBook XML V4.3//EN"
+	"http://www.oasis-open.org/docbook/xml/4.3/docbookx.dtd">
+<chapter id="multi-host">
+  <title>
+    Managing multiple hosts at the same time
+  </title>
+
+  <para>
+    Cockpit allows you to access multiple hosts in a single session,
+    by establishing SSH connections to other hosts. This is quite
+    similar to logging into these other hosts using the "ssh" command
+    on the command line, with one very important difference:
+  </para>
+  <para>
+    Code from the local host and all the remote hosts run at the same
+    time, in the same browser context.  They are not sufficiently
+    isolated from each other in the browser. All code effectively has
+    the same privileges as the primary session on the local host.
+  </para>
+  <para>
+    Thus, <emphasis>you should only only connect to remote hosts that
+    you trust</emphasis>. You must be sure that none of the hosts that
+    you connect to will cause Cockpit to load malicious JavaScript
+    code into your browser.
+  </para>
+  <para>
+    Going forward, Cockpit will try to provide sufficient isolation to
+    make it safe to manage multiple hosts in a single Cockpit
+    session. But until we get there, Cockpit will at least warn you
+    before connecting to more than one host. It is also possible to
+    disable multiple hosts entirely, and some operating systems do
+    this already by default.
+  </para>
+  <para>
+    You can prevent loading of JavaScript, HTML, etc from more than
+    one host by adding this to <filename>cockpit.conf</filename>:
+  </para>
+  <programlisting>
+    [WebService]
+    AllowMultiHost=false
+  </programlisting>
+  <para>
+    When you allow multiple hosts in a single Cockpit session by
+    setting <code>AllowMultiHost</code> to true, then the user will be
+    warned once per session, before connecting to the second host.  If
+    that is still too much, you can switch it off completely by adding
+    the following to <filename>cockpit.conf</filename>:
+  </para>
+  <programlisting>
+    [Session]
+    WarnBeforeConnecting=false
+  </programlisting>
+</chapter>

doc/man/cockpit.conf.xml

@@ -272,6 +272,18 @@ IdleTimeout=15
           <para>When not specified, there is no idle timeout by default.</para>
         </listitem>
       </varlistentry>
+      <varlistentry>
+        <term><option>WarnBeforeConnecting</option></term>
+        <listitem>
+          <para>Whether to warn before connecting to remote hosts from the Shell. Defaults to true</para>
+          <informalexample>
+<programlisting language="ini">
+[Session]
+WarnBeforeConnecting=false
+</programlisting>
+          </informalexample>
+        </listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
 

pkg/shell/hosts.jsx

@@ -16,7 +16,7 @@ import { Tooltip } from "@patternfly/react-core/dist/esm/components/Tooltip";
 import 'polyfills';
 import { CockpitNav, CockpitNavItem } from "./nav.jsx";
 import { build_href, split_connection_string } from "./util.jsx";
-import { add_host, edit_host } from "./hosts_dialog.jsx";
+import { add_host, edit_host, connect_host } from "./hosts_dialog.jsx";
 
 const _ = cockpit.gettext;
 
@@ -122,17 +122,14 @@ export class CockpitHosts extends React.Component {
     }
 
     async onHostSwitch(machine) {
-        const { state } = this.props;
-
-        // We could launch the connection dialogs here and not jump at
-        // all when the login fails (or is cancelled), but the
-        // traditional behavior is to jump and then try to connect.
+        const { state, host_modal_state } = this.props;
 
-        const connection_string = machine.connection_string;
-        const parts = split_connection_string(connection_string);
-        const addr = build_href({ host: parts.address });
-        state.jump(addr);
-        state.ensure_connection();
+        const connection_string = await connect_host(host_modal_state, state, machine);
+        if (connection_string) {
+            const parts = split_connection_string(connection_string);
+            const addr = build_href({ host: parts.address });
+            state.jump(addr);
+        }
     }
 
     onEditHosts() {

pkg/shell/hosts_dialog.jsx

@@ -38,10 +38,13 @@ import { Popover } from "@patternfly/react-core/dist/esm/components/Popover/inde
 import { Radio } from "@patternfly/react-core/dist/esm/components/Radio/index.js";
 import { Stack } from "@patternfly/react-core/dist/esm/layouts/Stack/index.js";
 import { TextInput } from "@patternfly/react-core/dist/esm/components/TextInput/index.js";
-import { OutlinedQuestionCircleIcon } from "@patternfly/react-icons";
+import { OutlinedQuestionCircleIcon, ExternalLinkAltIcon } from "@patternfly/react-icons";
+import { HelperText, HelperTextItem } from "@patternfly/react-core/dist/esm/components/HelperText/index.js";
+import { Text, TextContent, TextVariants } from "@patternfly/react-core/dist/esm/components/Text";
 
 import { FormHelper } from "cockpit-components-form-helper";
 import { ModalError } from "cockpit-components-inline-notification.jsx";
+import { fmt_to_fragments } from "utils.js";
 
 import { build_href, split_connection_string, generate_connection_string } from "./util.jsx";
 
@@ -119,6 +122,12 @@ export async function connect_host(state, shell_state, machine) {
             address: machine.address,
             template: codes[machine.problem],
         });
+    } else if (!window.sessionStorage.getItem("connection-warning-shown")) {
+        // connect by launching into the "Connection warning" dialog.
+        connection_string = await state.show_modal({
+            address: machine.address,
+            template: "connect"
+        });
     } else {
         // Try to connect without any dialog
         try {
@@ -145,6 +154,7 @@ export async function connect_host(state, shell_state, machine) {
 }
 
 export const codes = {
+    danger: "connect",
     "no-cockpit": "not-supported",
     "not-supported": "not-supported",
     "protocol-error": "not-supported",
@@ -200,6 +210,74 @@ class NotSupported extends React.Component {
     }
 }
 
+class Connect extends React.Component {
+    constructor(props) {
+        super(props);
+
+        this.state = {
+            inProgress: false,
+        };
+    }
+
+    onConnect() {
+        window.sessionStorage.setItem("connection-warning-shown", true);
+        this.setState({ inProgress: true });
+        this.props.run(this.props.try2Connect(this.props.full_address), ex => {
+            let keep_message = false;
+            if (ex.problem === "no-host") {
+                let host_id_port = this.props.full_address;
+                let port = "22";
+                const port_index = host_id_port.lastIndexOf(":");
+                if (port_index === -1) {
+                    host_id_port = this.props.full_address + ":22";
+                } else {
+                    port = host_id_port.substr(port_index + 1);
+                }
+
+                ex.message = cockpit.format(_("Unable to contact the given host $0. Make sure it has ssh running on port $1, or specify another port in the address."), host_id_port, port);
+                ex.problem = "not-found";
+                keep_message = true;
+            }
+            this.setState({ inProgress: false });
+            this.props.setError(ex, keep_message);
+        });
+    }
+
+    render() {
+        return (
+            <Modal id="hosts_connect_server_dialog" isOpen
+                   position="top" variant="small"
+                   onClose={this.props.onClose}
+                   title={fmt_to_fragments(_("Connect to $0?"), <b>{this.props.host}</b>)}
+                   titleIconVariant="warning"
+                   footer={<>
+                       <HelperText>
+                           <HelperTextItem>{_("You will be reminded once per session.")}</HelperTextItem>
+                       </HelperText>
+                       <Button variant="warning" isLoading={this.state.inProgress}
+                                       onClick={() => this.onConnect()}>
+                           {_("Connect")}
+                       </Button>
+                       <Button variant="link" className="btn-cancel" onClick={this.props.onClose}>
+                           { _("Cancel") }
+                       </Button>
+                   </>}
+            >
+                <TextContent>
+                    <Text component={TextVariants.p}>
+                        {_("Remote hosts have the ability to run JavaScript on all connected hosts. Only connect to machines that you trust.")}
+                    </Text>
+                    <Text component={TextVariants.p}>
+                        <a href="https://cockpit-project.org/guide/latest/multi-host.html" target="blank" rel="noopener noreferrer">
+                            <ExternalLinkAltIcon /> {_("Read more")}
+                        </a>
+                    </Text>
+                </TextContent>
+            </Modal>
+        );
+    }
+}
+
 class AddMachine extends React.Component {
     constructor(props) {
         super(props);
@@ -323,23 +401,27 @@ class AddMachine extends React.Component {
             });
         });
 
-        this.props.run(this.props.try2Connect(address), ex => {
-            if (ex.problem === "no-host") {
-                let host_id_port = address;
-                let port = "22";
-                const port_index = host_id_port.lastIndexOf(":");
-                if (port_index === -1) {
-                    host_id_port = address + ":22";
-                } else {
-                    port = host_id_port.substr(port_index + 1);
-                }
+        if (!window.sessionStorage.getItem("connection-warning-shown")) {
+            this.props.setError({ problem: "danger", command: "close" });
+        } else {
+            this.props.run(this.props.try2Connect(address), ex => {
+                if (ex.problem === "no-host") {
+                    let host_id_port = address;
+                    let port = "22";
+                    const port_index = host_id_port.lastIndexOf(":");
+                    if (port_index === -1) {
+                        host_id_port = address + ":22";
+                    } else {
+                        port = host_id_port.substr(port_index + 1);
+                    }
 
-                ex.message = cockpit.format(_("Unable to contact the given host $0. Make sure it has ssh running on port $1, or specify another port in the address."), host_id_port, port);
-                ex.problem = "not-found";
-            }
-            this.setState({ inProgress: false });
-            this.props.setError(ex);
-        });
+                    ex.message = cockpit.format(_("Unable to contact the given host $0. Make sure it has ssh running on port $1, or specify another port in the address."), host_id_port, port);
+                    ex.problem = "not-found";
+                }
+                this.setState({ inProgress: false });
+                this.props.setError(ex);
+            });
+        }
     }
 
     render() {
@@ -1161,7 +1243,9 @@ class HostModalInner extends React.Component {
             complete: this.complete,
         };
 
-        if (template === "add-machine")
+        if (template === "connect")
+            return <Connect {...props} />;
+        else if (template === "add-machine")
             return <AddMachine {...props} />;
         else if (template === "unknown-hostkey" || template === "unknown-host" || template === "invalid-hostkey")
             return <HostKey {...props} />;

pkg/shell/shell.jsx

@@ -66,10 +66,7 @@ const Shell = () => {
     useEvent(host_modal_state, "changed");
 
     useEvent(state, "connect", () => {
-        // We could launch some dialogs here, but the traditional
-        // behavior is to just connect the loader and open the dialogs
-        // from the troubleshoot button.
-        state.loader.connect(state.current_machine.address);
+        connect_host(host_modal_state, state, state.current_machine);
     });
 
     const {

pkg/shell/state.jsx

@@ -45,6 +45,24 @@ export function ShellState() {
     if (meta_multihost instanceof HTMLMetaElement && meta_multihost.content == "yes")
         config.host_switcher_enabled = true;
 
+    /* Should show warning before connecting? */
+    let config_ready = false;
+    cockpit.dbus(null, { bus: "internal" }).call("/config", "cockpit.Config", "GetString",
+                                                 ["Session", "WarnBeforeConnecting"], [])
+            .then(([result]) => {
+                if (result == "false" || result == "no") {
+                    window.sessionStorage.setItem("connection-warning-shown", "yes");
+                }
+            })
+            .catch(e => {
+                if (e.name != "cockpit.Config.KeyError")
+                    console.warn("Error reading WarnBeforeConnecting configuration:", e.message);
+            })
+            .finally(() => {
+                config_ready = true;
+                on_ready();
+            });
+
     /* MACHINES DATABASE AND MANIFEST LOADER
      *
      * These are part of the machinery in the basement that maintains
@@ -74,7 +92,7 @@ export function ShellState() {
         on_ready();
 
     function on_ready() {
-        if (machines.ready) {
+        if (machines.ready && config_ready) {
             self.ready = true;
             window.addEventListener("popstate", ev => {
                 update();
@@ -488,7 +506,6 @@ export function ShellState() {
 
         // Methods
         jump,
-        ensure_connection,
         remove_frame,
         most_recent_path_for_host,
 

test/common/testlib.py

@@ -1140,13 +1140,23 @@ def start_machine_troubleshoot(
         known_host: bool = False,
         password: str | None = None,
         expect_closed_dialog: bool = True,
+        expect_warning: bool = True,
+        need_click: bool = True
     ) -> None:
-        self.click('#machine-troubleshoot')
+        if need_click:
+            self.click('#machine-troubleshoot')
+
+        if not new and expect_warning:
+            self.wait_visible('#hosts_connect_server_dialog')
+            self.click("#hosts_connect_server_dialog button.pf-m-warning")
 
         self.wait_visible('#hosts_setup_server_dialog')
         if new:
             self.wait_text("#hosts_setup_server_dialog button.pf-m-primary", "Add")
             self.click("#hosts_setup_server_dialog button.pf-m-primary")
+            if expect_warning:
+                self.wait_visible('#hosts_connect_server_dialog')
+                self.click("#hosts_connect_server_dialog button.pf-m-warning")
             if not known_host:
                 self.wait_in_text('#hosts_setup_server_dialog', "You are connecting to")
                 self.wait_in_text('#hosts_setup_server_dialog', "for the first time.")
@@ -1160,10 +1170,14 @@ def start_machine_troubleshoot(
         if expect_closed_dialog:
             self.wait_not_present('#hosts_setup_server_dialog')
 
-    def add_machine(self, address: str, known_host: bool = False, password: str = "foobar") -> None:
+    def add_machine(self, address: str, known_host: bool = False, password: str = "foobar",
+                    expect_warning: bool = True) -> None:
         self.switch_to_top()
         self.go(f"/@{address}")
-        self.start_machine_troubleshoot(new=True, known_host=known_host, password=password)
+        self.start_machine_troubleshoot(new=True,
+                                        known_host=known_host,
+                                        password=password,
+                                        expect_warning=expect_warning)
         self.enter_page("/system", host=address)
 
     def grant_permissions(self, *args: str) -> None: