Skip to content

Commit

Permalink
shell: Redesign untrusted "add host" dialog
Browse files Browse the repository at this point in the history
Remote machines can send malicious cockpit pages which do iframe
traversal or use the `host` channel option to run arbitrary code on all
other currently connected Cockpit sessions.

Isolating cockpit sessions from different hosts from one another is
difficult, and properly implementing this will take a while (if it's
possible at all): https://issues.redhat.com/browse/COCKPIT-870

Until this happens, warn users about that in the "unknown host" shell
dialog.

Also hide the "verify fingerprint" recipe behind an expander. In
practice, most users do "Trust On First Use" anyway, as validating a FP
properly is too hard. This makes the dialog less overwhelming and also
puts more emphasis on the "trust" aspect.

Related to https://issues.redhat.com/browse/COCKPIT-870
  • Loading branch information
martinpitt committed Oct 10, 2023
1 parent 7378008 commit 6bfc2c1
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 10 deletions.
34 changes: 24 additions & 10 deletions pkg/shell/hosts_dialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ import { Alert } from "@patternfly/react-core/dist/esm/components/Alert/index.js
import { Button } from "@patternfly/react-core/dist/esm/components/Button/index.js";
import { Checkbox } from "@patternfly/react-core/dist/esm/components/Checkbox/index.js";
import { ClipboardCopy } from "@patternfly/react-core/dist/esm/components/ClipboardCopy/index.js";
import { ExpandableSection } from "@patternfly/react-core/dist/esm/components/ExpandableSection/index.js";
import { Form, FormGroup } from "@patternfly/react-core/dist/esm/components/Form/index.js";
import { Modal } from "@patternfly/react-core/dist/esm/components/Modal/index.js";
import { Popover } from "@patternfly/react-core/dist/esm/components/Popover/index.js";
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 { FormHelper } from "cockpit-components-form-helper";
import { ModalError } from "cockpit-components-inline-notification.jsx";
Expand Down Expand Up @@ -402,6 +405,7 @@ class HostKey extends React.Component {

this.state = {
inProgress: false,
verifyExpanded: false,
error_options: props.error_options,
};

Expand Down Expand Up @@ -464,8 +468,9 @@ class HostKey extends React.Component {
}

const callback = this.onAddKey;
const title = this.props.template === "invalid-hostkey" ? cockpit.format(_("$0 key changed"), this.props.host) : _("New host");
const submitText = _("Accept key and connect");
const title = cockpit.format(this.props.template === "invalid-hostkey" ? _("$0 key changed") : _("New host: $0"),
this.props.host);
const submitText = _("Trust and add host");
let unknown = false;
let body = null;
if (!key_type) {
Expand All @@ -479,18 +484,27 @@ class HostKey extends React.Component {
<p>{cockpit.format(_("To verify a fingerprint, run the following on $0 while physically sitting at the machine or through a trusted network:"), this.props.host)}</p>
<ClipboardCopy isReadOnly hoverTip={_("Copy")} clickTip={_("Copied")} className="hostkey-verify-help-cmds pf-v5-u-font-family-monospace">ssh-keyscan -t {key_type} localhost | ssh-keygen -lf -</ClipboardCopy>
<p>{_("The resulting fingerprint is fine to share via public methods, including email.")}</p>
<p>{_("If the fingerprint matches, click 'Accept key and connect'. Otherwise, do not connect and contact your administrator.")}</p>
<p>{_("If the fingerprint matches, click 'Trust and add host'. Otherwise, do not connect and contact your administrator.")}</p>
</>;
} else {
const fingerprint_help = <Popover bodyContent={
<>
<p>{ _("The resulting fingerprint is fine to share via public methods, including email.") }</p>
<p>{ _("If you are asking someone else to do the verification for you, they can send the results using any method.") }</p>
</> }>
<OutlinedQuestionCircleIcon />
</Popover>;
body = <>
<p>{cockpit.format(_("You are connecting to $0 for the first time."), this.props.host)}</p>
<p>{_("To ensure that your connection is not intercepted by a malicious third-party, please verify the host key fingerprint:")}</p>
<ClipboardCopy isReadOnly hoverTip={_("Copy")} clickTip={_("Copied")} className="hostkey-fingerprint pf-v5-u-font-family-monospace">{fp}</ClipboardCopy>
<p className="hostkey-type">({key_type})</p>
<p>{cockpit.format(_("To verify a fingerprint, run the following on $0 while physically sitting at the machine or through a trusted network:"), this.props.host)}</p>
<ClipboardCopy isReadOnly hoverTip={_("Copy")} clickTip={_("Copied")} className="hostkey-verify-help-cmds pf-v5-u-font-family-monospace">ssh-keyscan -t {key_type} localhost | ssh-keygen -lf -</ClipboardCopy>
<p>{_("The resulting fingerprint is fine to share via public methods, including email.")}</p>
<p>{_("If the fingerprint matches, click 'Accept key and connect'. Otherwise, do not connect and contact your administrator.")}</p>
<ExpandableSection toggleText={ _("Verify fingerprint") }
isExpanded={this.state.verifyExpanded}
onToggle={(_ev, verifyExpanded) => this.setState({ verifyExpanded }) }>
<p>{_("Run this command over a trusted network or physically on the remote machine:")}</p>
<ClipboardCopy isReadOnly hoverTip={_("Copy")} clickTip={_("Copied")} className="hostkey-verify-help pf-v5-u-font-family-monospace">ssh-keyscan -t {key_type} localhost | ssh-keygen -lf -</ClipboardCopy>
<p>{_("The fingerprint should match:")} {fingerprint_help}</p>
<ClipboardCopy isReadOnly hoverTip={_("Copy")} clickTip={_("Copied")} className="hostkey-verify-help pf-v5-u-font-family-monospace">{fp}</ClipboardCopy>
</ExpandableSection>
<Alert variant='warning' isInline isPlain title={_("Malicious pages on the remote machine can do arbitrary changes to other connected hosts in the same session.")} />
</>;
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/shell/shell.scss
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ $desktop: $phone + 1px;
font-size: small;
}

.hostkey-verify-help {
margin-block-end: 1.5em;
}


/* Alert fixups */
.modal-content .pf-v5-c-alert {
text-align: start;
Expand Down

0 comments on commit 6bfc2c1

Please sign in to comment.