Skip to content

Commit

Permalink
ssh: improve host key verification error messaging
Browse files Browse the repository at this point in the history
See screenshots attached to #3941 to see how it renders
  • Loading branch information
wez committed Jul 6, 2023
1 parent bc2601d commit 9e36775
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 19 deletions.
53 changes: 51 additions & 2 deletions mux/src/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ use std::io::{BufWriter, Read, Write};
use std::sync::mpsc::{channel, Receiver, Sender, TryRecvError};
use std::sync::{Arc, Mutex};
use std::time::{Duration, Instant};
use termwiz::cell::unicode_column_width;
use termwiz::cell::{unicode_column_width, AttributeChange, Intensity};
use termwiz::input::{InputEvent, InputParser};
use termwiz::lineedit::*;
use termwiz::render::terminfo::TerminfoRenderer;
use termwiz::surface::Change;
use termwiz::terminal::{ScreenSize, Terminal, TerminalWaker};
use wezterm_ssh::{ConfigMap, Session, SessionEvent, SshChildProcess, SshPty};
use wezterm_ssh::{
ConfigMap, HostVerificationFailed, Session, SessionEvent, SshChildProcess, SshPty,
};
use wezterm_term::TerminalSize;

#[derive(Default)]
Expand Down Expand Up @@ -113,6 +115,11 @@ pub fn ssh_connect_with_ui(
}
smol::block_on(auth.answer(answers))?;
}
SessionEvent::HostVerificationFailed(failed) => {
let message = format_host_verification_for_terminal(failed);
ui.output(message);
anyhow::bail!("Host key verification failed");
}
SessionEvent::Error(err) => {
anyhow::bail!("Error: {}", err);
}
Expand All @@ -123,6 +130,44 @@ pub fn ssh_connect_with_ui(
})
}

fn format_host_verification_for_terminal(failed: HostVerificationFailed) -> Vec<Change> {
vec![
AttributeChange::Intensity(Intensity::Bold).into(),
Change::Text("REMOTE HOST IDENTIFICATION CHANGED\r\n".to_string()),
Change::Text("SOMEONE MAY BE DOING SOMETHING NASTY!\r\n".to_string()),
AttributeChange::Intensity(Intensity::Normal).into(),
Change::Text("\r\nThere are two likely causes for this:\r\n".to_string()),
Change::Text(
" 1. Someone is eavesdropping right now (man-in-the-middle attack)\r\n".to_string(),
),
Change::Text(" 2. The host key may have been changed by the administrator\r\n".to_string()),
Change::Text("\r\n".to_string()),
AttributeChange::Intensity(Intensity::Bold).into(),
Change::Text(
"Please contact your system administrator to discuss how to proceed!\r\n".to_string(),
),
AttributeChange::Intensity(Intensity::Normal).into(),
Change::Text("\r\n".to_string()),
match failed.file {
Some(file) => Change::Text(format!(
"The host is {}, and its fingerprint is\r\n{}\r\n\
which doesn't match the entry in {}\r\n\
If administrator confirms that the key has changed, you can\r\n\
fix this for yourself by removing the offending entry from\r\n\
{} and then try connecting again.\r\n",
failed.remote_address,
failed.key,
file.display(),
file.display(),
)),
None => Change::Text(format!(
"The host is {}, and its fingerprint is\r\n{}\r\n",
failed.remote_address, failed.key
)),
},
]
}

/// Represents a connection to remote host via ssh.
/// The domain is created with the ssh config prior to making the
/// connection. The connection is established by the first spawn()
Expand Down Expand Up @@ -596,6 +641,10 @@ fn connect_ssh_session(
SessionEvent::Error(err) => {
shim.output_line(&format!("Error: {}", err))?;
}
SessionEvent::HostVerificationFailed(failed) => {
let message = format_host_verification_for_terminal(failed);
shim.render(&message)?;
}
SessionEvent::Authenticated => {
// Our session has been authenticated: we can now
// set up the real pty for the pane
Expand Down
3 changes: 3 additions & 0 deletions wezterm-ssh/examples/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ fn main() {
}
auth.answer(answers).await?;
}
SessionEvent::HostVerificationFailed(failed) => {
anyhow::bail!("{}", failed);
}
SessionEvent::Error(err) => {
anyhow::bail!("{}", err);
}
Expand Down
50 changes: 33 additions & 17 deletions wezterm-ssh/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ use crate::session::SessionEvent;
use anyhow::Context;
use smol::channel::{bounded, Sender};

#[derive(Debug, thiserror::Error)]
#[error("host key mismatch for ssh server {remote_address}. Got fingerprint {key} instead of the expected value from your known hosts file {file:?}.")]
pub struct HostVerificationFailed {
pub remote_address: String,
pub key: String,
pub file: Option<std::path::PathBuf>,
}

#[derive(Debug)]
pub struct HostVerificationEvent {
pub message: String,
Expand Down Expand Up @@ -55,15 +63,23 @@ impl crate::sessioninner::SessionInner {
Ok(sess.update_known_hosts_file()?)
}
libssh_rs::KnownHosts::Changed => {
anyhow::bail!(
"host key mismatch for ssh server {}:{}.\n\
Got fingerprint {} instead of expected value from known_hosts\n\
file.\n\
Refusing to connect.",
hostname,
port,
let mut file = None;
if let Some(kh) = self.config.get("userknownhostsfile") {
for candidate in kh.split_whitespace() {
file.replace(candidate.into());
break;
}
}

let failed = HostVerificationFailed {
remote_address: format!("{hostname}:{port}"),
key,
);
file,
};
self.tx_event
.try_send(SessionEvent::HostVerificationFailed(failed))
.context("sending HostVerificationFailed event to user")?;
anyhow::bail!("Host key verification failed");
}
libssh_rs::KnownHosts::Other => {
anyhow::bail!(
Expand Down Expand Up @@ -174,15 +190,15 @@ impl crate::sessioninner::SessionInner {
.with_context(|| format!("writing known_hosts file {}", file.display()))?;
}
ssh2::CheckResult::Mismatch => {
anyhow::bail!(
"host key mismatch for ssh server {}.\n\
Got fingerprint {} instead of expected value from known_hosts\n\
file {}.\n\
Refusing to connect.",
remote_address,
fingerprint,
file.display()
);
let failed = HostVerificationFailed {
remote_address: remote_address.to_string(),
key: fingerprint,
file: Some(file.to_path_buf()),
};
self.tx_event
.try_send(SessionEvent::HostVerificationFailed(failed))
.context("sending HostVerificationFailed event to user")?;
anyhow::bail!("Host key verification failed");
}
ssh2::CheckResult::Failure => {
anyhow::bail!("failed to check the known hosts");
Expand Down
1 change: 1 addition & 0 deletions wezterm-ssh/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub enum SessionEvent {
Banner(Option<String>),
HostVerify(HostVerificationEvent),
Authenticate(AuthenticationEvent),
HostVerificationFailed(HostVerificationFailed),
Error(String),
Authenticated,
}
Expand Down
3 changes: 3 additions & 0 deletions wezterm-ssh/tests/sshd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,9 @@ pub async fn session(sshd: Sshd) -> SessionWithSshd {
.await
.expect("Failed to send authenticate response");
}
SessionEvent::HostVerificationFailed(failed) => {
panic!("{}", failed);
}
SessionEvent::Error(err) => {
panic!("{}", err);
}
Expand Down

0 comments on commit 9e36775

Please sign in to comment.