Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shell: Extra warnings when connecting to remote hosts #20826

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/guide/Makefile-guide.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
1 change: 1 addition & 0 deletions doc/guide/cockpit-guide.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
54 changes: 54 additions & 0 deletions doc/guide/multi-host.xml
Original file line number Diff line number Diff line change
@@ -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>
12 changes: 12 additions & 0 deletions doc/man/cockpit.conf.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
21 changes: 9 additions & 12 deletions pkg/shell/hosts.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -114,25 +114,22 @@ export class CockpitHosts extends React.Component {
}

async onAddNewHost() {
await add_host(this.props.host_modal_state);
await add_host(this.props.host_modal_state, this.props.state);
}

async onHostEdit(event, machine) {
await edit_host(this.props.host_modal_state, this.props.state, machine);
}

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() {
Expand Down
130 changes: 110 additions & 20 deletions pkg/shell/hosts_dialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -75,8 +78,14 @@ export const HostModalState = () => {
return self;
};

export async function add_host(state) {
await state.show_modal({ });
export async function add_host(state, shell_state) {
const connection_string = await state.show_modal({ });
if (connection_string) {
const parts = split_connection_string(connection_string);
const addr = build_href({ host: parts.address });
shell_state.loader.connect(parts.address);
shell_state.jump(addr);
}
}

export async function edit_host(state, shell_state, machine) {
Expand Down Expand Up @@ -119,6 +128,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 {
Expand All @@ -145,6 +160,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",
Expand Down Expand Up @@ -200,6 +216,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(try2Connect(this.props.machines_ins, 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);
Expand Down Expand Up @@ -323,23 +407,27 @@ class AddMachine extends React.Component {
});
});

this.props.run(try2Connect(this.props.machines_ins, 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(try2Connect(this.props.machines_ins, 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";
Comment on lines +418 to +419
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 2 added lines are not executed by any test.

} 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() {
Expand Down Expand Up @@ -1157,7 +1245,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} />;
Expand Down
5 changes: 1 addition & 4 deletions pkg/shell/shell.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 19 additions & 2 deletions pkg/shell/state.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test.

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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -489,7 +507,6 @@ export function ShellState() {

// Methods
jump,
ensure_connection,
remove_frame,
most_recent_path_for_host,

Expand Down
Loading