From 1387fd4105b242fa2d24ad99d10a5b1af23f293e Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 7 Dec 2022 18:52:00 -0800 Subject: [PATCH 1/9] Validate SSH host keys --- Cargo.toml | 7 +- crates/cargo-test-support/Cargo.toml | 2 +- src/cargo/sources/git/known_hosts.rs | 439 +++++++++++++++++++++ src/cargo/sources/git/mod.rs | 1 + src/cargo/sources/git/utils.rs | 12 +- src/doc/src/appendix/git-authentication.md | 23 ++ 6 files changed, 479 insertions(+), 5 deletions(-) create mode 100644 src/cargo/sources/git/known_hosts.rs diff --git a/Cargo.toml b/Cargo.toml index 54b4f97bdb8..b3f55479ddc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ name = "cargo" path = "src/cargo/lib.rs" [dependencies] +base64 = "0.13.1" bytesize = "1.0" cargo-platform = { path = "crates/cargo-platform", version = "0.1.2" } cargo-util = { path = "crates/cargo-util", version = "0.2.3" } @@ -27,8 +28,8 @@ pretty_env_logger = { version = "0.4", optional = true } anyhow = "1.0.47" filetime = "0.2.9" flate2 = { version = "1.0.3", default-features = false, features = ["zlib"] } -git2 = "0.15.0" -git2-curl = "0.16.0" +git2 = "0.16.0" +git2-curl = "0.17.0" glob = "0.3.0" hex = "0.4" home = "0.5" @@ -42,7 +43,7 @@ jobserver = "0.1.24" lazycell = "1.2.0" libc = "0.2" log = "0.4.6" -libgit2-sys = "0.14.0" +libgit2-sys = "0.14.1" memchr = "2.1.3" opener = "0.5" os_info = "3.5.0" diff --git a/crates/cargo-test-support/Cargo.toml b/crates/cargo-test-support/Cargo.toml index 65e0f75668b..653c5db4d61 100644 --- a/crates/cargo-test-support/Cargo.toml +++ b/crates/cargo-test-support/Cargo.toml @@ -17,7 +17,7 @@ filetime = "0.2" flate2 = { version = "1.0", default-features = false, features = ["zlib"] } pasetors = { version = "0.6.4", features = ["v3", "paserk", "std", "serde"] } time = { version = "0.3", features = ["parsing", "formatting"]} -git2 = "0.15.0" +git2 = "0.16.0" glob = "0.3" itertools = "0.10.0" lazy_static = "1.0" diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs new file mode 100644 index 00000000000..875dcf63f35 --- /dev/null +++ b/src/cargo/sources/git/known_hosts.rs @@ -0,0 +1,439 @@ +//! SSH host key validation support. +//! +//! A primary goal with this implementation is to provide user-friendly error +//! messages, guiding them to understand the issue and how to resolve it. +//! +//! Note that there are a lot of limitations here. This reads OpenSSH +//! known_hosts files from well-known locations, but it does not read OpenSSH +//! config files. The config file can change the behavior of how OpenSSH +//! handles known_hosts files. For example, some things we don't handle: +//! +//! - `GlobalKnownHostsFile` — Changes the location of the global host file. +//! - `UserKnownHostsFile` — Changes the location of the user's host file. +//! - `KnownHostsCommand` — A command to fetch known hosts. +//! - `CheckHostIP` — DNS spoofing checks. +//! - `VisualHostKey` — Shows a visual ascii-art key. +//! - `VerifyHostKeyDNS` — Uses SSHFP DNS records to fetch a host key. +//! +//! There's also a number of things that aren't supported but could be easily +//! added (it just adds a little complexity). For example, hashed hostnames, +//! hostname patterns, and revoked markers. See "FIXME" comments littered in +//! this file. + +use git2::cert::Cert; +use git2::CertificateCheckStatus; +use std::collections::HashSet; +use std::fmt::Write; +use std::path::{Path, PathBuf}; + +/// These are host keys that are hard-coded in cargo to provide convenience. +/// +/// If GitHub ever publishes new keys, the user can add them to their own +/// configuration file to use those instead. +/// +/// The GitHub keys are sourced from or +/// . +/// +/// These will be ignored if the user adds their own entries for `github.com`, +/// which can be useful if GitHub ever revokes their old keys. +static BUNDLED_KEYS: &[(&str, &str, &str)] = &[ + ("github.com", "ssh-ed25519", "AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl"), + ("github.com", "ecdsa-sha2-nistp256", "AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg="), + ("github.com", "ssh-rsa", "AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ=="), +]; + +enum KnownHostError { + /// Some general error happened while validating the known hosts. + CheckError(anyhow::Error), + /// The host key was not found. + HostKeyNotFound { + hostname: String, + key_type: git2::cert::SshHostKeyType, + remote_host_key: String, + remote_fingerprint: String, + other_hosts: Vec, + }, + /// The host key was found, but does not match the remote's key. + HostKeyHasChanged { + hostname: String, + key_type: git2::cert::SshHostKeyType, + old_known_host: KnownHost, + remote_host_key: String, + remote_fingerprint: String, + }, +} + +impl From for KnownHostError { + fn from(err: anyhow::Error) -> KnownHostError { + KnownHostError::CheckError(err.into()) + } +} + +/// The location where a host key was located. +#[derive(Clone)] +enum KnownHostLocation { + /// Loaded from a file from disk. + File { path: PathBuf, lineno: u32 }, + /// Part of the hard-coded bundled keys in Cargo. + Bundled, +} + +/// The git2 callback used to validate a certificate (only ssh known hosts are validated). +pub fn certificate_check( + cert: &Cert<'_>, + host: &str, + port: Option, +) -> Result { + let Some(host_key) = cert.as_hostkey() else { + // Return passthrough for TLS X509 certificates to use whatever validation + // was done in git2. + return Ok(CertificateCheckStatus::CertificatePassthrough) + }; + // If a nonstandard port is in use, check for that first. + // The fallback to check without a port is handled in the HostKeyNotFound handler. + let host_maybe_port = match port { + Some(port) if port != 22 => format!("[{host}]:{port}"), + _ => host.to_string(), + }; + // The error message must be constructed as a string to pass through the libgit2 C API. + let err_msg = match check_ssh_known_hosts(host_key, &host_maybe_port) { + Ok(()) => { + return Ok(CertificateCheckStatus::CertificateOk); + } + Err(KnownHostError::CheckError(e)) => { + format!("error: failed to validate host key:\n{:#}", e) + } + Err(KnownHostError::HostKeyNotFound { + hostname, + key_type, + remote_host_key, + remote_fingerprint, + other_hosts, + }) => { + // Try checking without the port. + if port.is_some() + && !matches!(port, Some(22)) + && check_ssh_known_hosts(host_key, host).is_ok() + { + return Ok(CertificateCheckStatus::CertificateOk); + } + let key_type_short_name = key_type.short_name(); + let key_type_name = key_type.name(); + let known_hosts_location = user_known_host_location_to_add(); + let other_hosts_message = if other_hosts.is_empty() { + String::new() + } else { + let mut msg = String::from( + "Note: This host key was found, \ + but is associated with a different host:\n", + ); + for known_host in other_hosts { + let loc = match known_host.location { + KnownHostLocation::File { path, lineno } => { + format!("{} line {lineno}", path.display()) + } + KnownHostLocation::Bundled => format!("bundled with cargo"), + }; + write!(msg, " {loc}: {}\n", known_host.patterns).unwrap(); + } + msg + }; + format!("error: unknown SSH host key\n\ + The SSH host key for `{hostname}` is not known and cannot be validated.\n\ + \n\ + To resolve this issue, add the host key to {known_hosts_location}\n\ + \n\ + The key to add is:\n\ + \n\ + {hostname} {key_type_name} {remote_host_key}\n\ + \n\ + The {key_type_short_name} key fingerprint is: SHA256:{remote_fingerprint}\n\ + This fingerprint should be validated with the server administrator that it is correct.\n\ + {other_hosts_message}\n\ + See https://doc.rust-lang.org/nightly/cargo/appendix/git-authentication.html#ssh-known-hosts \ + for more information.\n\ + ") + } + Err(KnownHostError::HostKeyHasChanged { + hostname, + key_type, + old_known_host, + remote_host_key, + remote_fingerprint, + }) => { + let key_type_short_name = key_type.short_name(); + let key_type_name = key_type.name(); + let known_hosts_location = user_known_host_location_to_add(); + let old_key_resolution = match old_known_host.location { + KnownHostLocation::File { path, lineno } => { + let old_key_location = path.display(); + format!( + "removing the old {key_type_name} key for `{hostname}` \ + located at {old_key_location} line {lineno}, \ + and adding the new key to {known_hosts_location}", + ) + } + KnownHostLocation::Bundled => { + format!( + "adding the new key to {known_hosts_location}\n\ + The current host key is bundled as part of Cargo." + ) + } + }; + format!("error: SSH host key has changed for `{hostname}`\n\ + *********************************\n\ + * WARNING: HOST KEY HAS CHANGED *\n\ + *********************************\n\ + This may be caused by a man-in-the-middle attack, or the \ + server may have changed its host key.\n\ + \n\ + The {key_type_short_name} fingerprint for the key from the remote host is:\n\ + SHA256:{remote_fingerprint}\n\ + \n\ + You are strongly encouraged to contact the server \ + administrator for `{hostname}` to verify that this new key is \ + correct.\n\ + \n\ + If you can verify that the server has a new key, you can \ + resolve this error by {old_key_resolution}\n\ + \n\ + The key provided by the remote host is:\n\ + \n\ + {hostname} {key_type_name} {remote_host_key}\n\ + \n\ + See https://doc.rust-lang.org/nightly/cargo/appendix/git-authentication.html#ssh-known-hosts \ + for more information.\n\ + ") + } + }; + Err(git2::Error::new( + git2::ErrorCode::GenericError, + git2::ErrorClass::Callback, + err_msg, + )) +} + +/// Checks if the given host/host key pair is known. +fn check_ssh_known_hosts( + cert_host_key: &git2::cert::CertHostkey<'_>, + host: &str, +) -> Result<(), KnownHostError> { + let Some(remote_host_key) = cert_host_key.hostkey() else { + return Err(anyhow::format_err!("remote host key is not available").into()); + }; + let remote_key_type = cert_host_key.hostkey_type().unwrap(); + // `changed_key` keeps track of any entries where the key has changed. + let mut changed_key = None; + // `other_hosts` keeps track of any entries that have an identical key, + // but a different hostname. + let mut other_hosts = Vec::new(); + + // Collect all the known host entries from disk. + let mut known_hosts = Vec::new(); + for path in known_host_files() { + if !path.exists() { + continue; + } + let hosts = load_hostfile(&path)?; + known_hosts.extend(hosts); + } + // Load the bundled keys. Don't add keys for hosts that the user has + // configured, which gives them the option to override them. This could be + // useful if the keys are ever revoked. + let configured_hosts: HashSet<_> = known_hosts + .iter() + .flat_map(|known_host| { + known_host + .patterns + .split(',') + .map(|pattern| pattern.to_lowercase()) + }) + .collect(); + for (patterns, key_type, key) in BUNDLED_KEYS { + if !configured_hosts.contains(*patterns) { + let key = base64::decode(key).unwrap(); + known_hosts.push(KnownHost { + location: KnownHostLocation::Bundled, + patterns: patterns.to_string(), + key_type: key_type.to_string(), + key, + }); + } + } + + for known_host in known_hosts { + // The key type from libgit2 needs to match the key type from the host file. + if known_host.key_type != remote_key_type.name() { + continue; + } + let key_matches = known_host.key == remote_host_key; + if !known_host.host_matches(host) { + // `name` can be None for hashed hostnames (which libgit2 does not expose). + if key_matches { + other_hosts.push(known_host.clone()); + } + continue; + } + if key_matches { + return Ok(()); + } + // The host and key type matched, but the key itself did not. + // This indicates the key has changed. + // This is only reported as an error if no subsequent lines have a + // correct key. + changed_key = Some(known_host.clone()); + } + // Older versions of OpenSSH (before 6.8, March 2015) showed MD5 + // fingerprints (see FingerprintHash ssh config option). Here we only + // support SHA256. + let mut remote_fingerprint = cargo_util::Sha256::new(); + remote_fingerprint.update(remote_host_key); + let remote_fingerprint = + base64::encode_config(remote_fingerprint.finish(), base64::STANDARD_NO_PAD); + let remote_host_key = base64::encode(remote_host_key); + // FIXME: Ideally the error message should include the IP address of the + // remote host (to help the user validate that they are connecting to the + // host they were expecting to). However, I don't see a way to obtain that + // information from libgit2. + match changed_key { + Some(old_known_host) => Err(KnownHostError::HostKeyHasChanged { + hostname: host.to_string(), + key_type: remote_key_type, + old_known_host, + remote_host_key, + remote_fingerprint, + }), + None => Err(KnownHostError::HostKeyNotFound { + hostname: host.to_string(), + key_type: remote_key_type, + remote_host_key, + remote_fingerprint, + other_hosts, + }), + } +} + +/// Returns a list of files to try loading OpenSSH-formatted known hosts. +fn known_host_files() -> Vec { + let mut result = Vec::new(); + if cfg!(unix) { + result.push(PathBuf::from("/etc/ssh/ssh_known_hosts")); + } else if cfg!(windows) { + // The msys/cygwin version of OpenSSH uses `/etc` from the posix root + // filesystem there (such as `C:\msys64\etc\ssh\ssh_known_hosts`). + // However, I do not know of a way to obtain that location from + // Windows-land. The ProgramData version here is what the PowerShell + // port of OpenSSH does. + if let Some(progdata) = std::env::var_os("ProgramData") { + let mut progdata = PathBuf::from(progdata); + progdata.push("ssh"); + progdata.push("ssh_known_hosts"); + result.push(progdata) + } + } + result.extend(user_known_host_location()); + result +} + +/// The location of the user's known_hosts file. +fn user_known_host_location() -> Option { + // NOTE: This is a potentially inaccurate prediction of what the user + // actually wants. The actual location depends on several factors: + // + // - Windows OpenSSH Powershell version: I believe this looks up the home + // directory via ProfileImagePath in the registry, falling back to + // `GetWindowsDirectoryW` if that fails. + // - OpenSSH Portable (under msys): This is very complicated. I got lost + // after following it through some ldap/active directory stuff. + // - OpenSSH (most unix platforms): Uses `pw->pw_dir` from `getpwuid()`. + // + // This doesn't do anything close to that. home_dir's behavior is: + // - Windows: $USERPROFILE, or SHGetFolderPathW() + // - Unix: $HOME, or getpwuid_r() + // + // Since there is a mismatch here, the location returned here might be + // different than what the user's `ssh` CLI command uses. We may want to + // consider trying to align it better. + home::home_dir().map(|mut home| { + home.push(".ssh"); + home.push("known_hosts"); + home + }) +} + +/// The location to display in an error message instructing the user where to +/// add the new key. +fn user_known_host_location_to_add() -> String { + // Note that we don't bother with the legacy known_hosts2 files. + match user_known_host_location() { + Some(path) => path.to_str().expect("utf-8 home").to_string(), + None => "~/.ssh/known_hosts".to_string(), + } +} + +/// A single known host entry. +#[derive(Clone)] +struct KnownHost { + location: KnownHostLocation, + /// The hostname. May be comma separated to match multiple hosts. + patterns: String, + key_type: String, + key: Vec, +} + +impl KnownHost { + /// Returns whether or not the given host matches this known host entry. + fn host_matches(&self, host: &str) -> bool { + let mut match_found = false; + let host = host.to_lowercase(); + // FIXME: support hashed hostnames + for pattern in self.patterns.split(',') { + let pattern = pattern.to_lowercase(); + // FIXME: support * and ? wildcards + if let Some(pattern) = pattern.strip_prefix('!') { + if pattern == host { + return false; + } + } else { + match_found = pattern == host; + } + } + match_found + } +} + +/// Loads an OpenSSH known_hosts file. +fn load_hostfile(path: &Path) -> Result, anyhow::Error> { + let contents = cargo_util::paths::read(path)?; + let entries = contents + .lines() + .enumerate() + .filter_map(|(lineno, line)| { + let location = KnownHostLocation::File { + path: path.to_path_buf(), + lineno: lineno as u32 + 1, + }; + parse_known_hosts_line(line, location) + }) + .collect(); + Ok(entries) +} + +fn parse_known_hosts_line(line: &str, location: KnownHostLocation) -> Option { + let line = line.trim(); + // FIXME: @revoked and @cert-authority is currently not supported. + if line.is_empty() || line.starts_with('#') || line.starts_with('@') { + return None; + } + let mut parts = line.split([' ', '\t']).filter(|s| !s.is_empty()); + let Some(patterns) = parts.next() else { return None }; + let Some(key_type) = parts.next() else { return None }; + let Some(key) = parts.next() else { return None }; + let Ok(key) = base64::decode(key) else { return None }; + Some(KnownHost { + location, + patterns: patterns.to_string(), + key_type: key_type.to_string(), + key, + }) +} diff --git a/src/cargo/sources/git/mod.rs b/src/cargo/sources/git/mod.rs index b32dbb17be1..225bad1671c 100644 --- a/src/cargo/sources/git/mod.rs +++ b/src/cargo/sources/git/mod.rs @@ -1,4 +1,5 @@ pub use self::source::GitSource; pub use self::utils::{fetch, GitCheckout, GitDatabase, GitRemote}; +mod known_hosts; mod source; mod utils; diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 623caceb803..831c43be6bc 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -683,7 +683,6 @@ where | ErrorClass::Submodule | ErrorClass::FetchHead | ErrorClass::Ssh - | ErrorClass::Callback | ErrorClass::Http => { let mut msg = "network failure seems to have happened\n".to_string(); msg.push_str( @@ -694,6 +693,13 @@ where ); err = err.context(msg); } + ErrorClass::Callback => { + // This unwraps the git2 error. We're using the callback error + // specifically to convey errors from Rust land through the C + // callback interface. We don't need the `; class=Callback + // (26)` that gets tacked on to the git2 error message. + err = anyhow::format_err!("{}", e.message()); + } _ => {} } } @@ -722,12 +728,16 @@ pub fn with_fetch_options( let mut progress = Progress::new("Fetch", config); network::with_retry(config, || { with_authentication(url, git_config, |f| { + let port = Url::parse(url).ok().and_then(|url| url.port()); let mut last_update = Instant::now(); let mut rcb = git2::RemoteCallbacks::new(); // We choose `N=10` here to make a `300ms * 10slots ~= 3000ms` // sliding window for tracking the data transfer rate (in bytes/s). let mut counter = MetricsCounter::<10>::new(0, last_update); rcb.credentials(f); + rcb.certificate_check(|cert, host| { + super::known_hosts::certificate_check(cert, host, port) + }); rcb.transfer_progress(|stats| { let indexed_deltas = stats.indexed_deltas(); let msg = if indexed_deltas > 0 { diff --git a/src/doc/src/appendix/git-authentication.md b/src/doc/src/appendix/git-authentication.md index 7fe1f149a07..a7db1ac7f15 100644 --- a/src/doc/src/appendix/git-authentication.md +++ b/src/doc/src/appendix/git-authentication.md @@ -58,9 +58,32 @@ on how to start `ssh-agent` and to add keys. > used by Cargo's built-in SSH library. More advanced requirements should use > [`net.git-fetch-with-cli`]. +### SSH Known Hosts + +When connecting to an SSH host, Cargo must verify the identity of the host +using "known hosts", which are a list of host keys. Cargo can look for these +known hosts in OpenSSH-style `known_hosts` files located in their standard +locations (`.ssh/known_hosts` in your home directory, or +`/etc/ssh/ssh_known_hosts` on Unix-like platforms or +`%PROGRAMDATA%\ssh\ssh_known_hosts` on Windows). More information about these +files can be found in the [sshd man page]. + +When connecting to an SSH host before the known hosts has been configured, +Cargo will display an error message instructing you how to add the host key. +This also includes a "fingerprint", which is a smaller hash of the host key, +which should be easier to visually verify. The server administrator can get +the fingerprint by running `ssh-keygen` against the public key (for example, +`ssh-keygen -l -f /etc/ssh/ssh_host_ecdsa_key.pub`). Well-known sites may +publish their fingerprints on the web; for example GitHub posts theirs at +. + +Cargo comes with the host keys for [github.com](https://github.com) built-in. +If those ever change, you can add the new keys to your known_hosts file. + [`credential.helper`]: https://git-scm.com/book/en/v2/Git-Tools-Credential-Storage [`net.git-fetch-with-cli`]: ../reference/config.md#netgit-fetch-with-cli [GCM]: https://github.com/microsoft/Git-Credential-Manager-Core/ [PuTTY]: https://www.chiark.greenend.org.uk/~sgtatham/putty/ [Microsoft installation documentation]: https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse [key management]: https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_keymanagement +[sshd man page]: https://man.openbsd.org/sshd#SSH_KNOWN_HOSTS_FILE_FORMAT From 9f62f8440e9e542f27d60c75be38ac51186c6c32 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 9 Dec 2022 20:03:27 -0800 Subject: [PATCH 2/9] Add support for deserializing Vec> in config. This adds the ability to track the definition location of a string in a TOML array. --- src/cargo/util/config/de.rs | 107 ++++++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 6fddc7e71f0..1408f15b573 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -384,7 +384,12 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess { { match self.list_iter.next() { // TODO: add `def` to error? - Some((value, _def)) => seed.deserialize(value.into_deserializer()).map(Some), + Some((value, def)) => { + // This might be a String or a Value. + // ValueDeserializer will handle figuring out which one it is. + let maybe_value_de = ValueDeserializer::new_with_string(value, def); + seed.deserialize(maybe_value_de).map(Some) + } None => Ok(None), } } @@ -400,7 +405,17 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess { struct ValueDeserializer<'config> { hits: u32, definition: Definition, - de: Deserializer<'config>, + /// The deserializer, used to actually deserialize a Value struct. + /// This is `None` if deserializing a string. + de: Option>, + /// A string value to deserialize. + /// + /// This is used for situations where you can't address a string via a + /// TOML key, such as a string inside an array. The `ConfigSeqAccess` + /// doesn't know if the type it should deserialize to is a `String` or + /// `Value`, so `ValueDeserializer` needs to be able to handle + /// both. + str_value: Option, } impl<'config> ValueDeserializer<'config> { @@ -428,9 +443,19 @@ impl<'config> ValueDeserializer<'config> { Ok(ValueDeserializer { hits: 0, definition, - de, + de: Some(de), + str_value: None, }) } + + fn new_with_string(s: String, definition: Definition) -> ValueDeserializer<'config> { + ValueDeserializer { + hits: 0, + definition, + de: None, + str_value: Some(s), + } + } } impl<'de, 'config> de::MapAccess<'de> for ValueDeserializer<'config> { @@ -459,9 +484,14 @@ impl<'de, 'config> de::MapAccess<'de> for ValueDeserializer<'config> { // If this is the first time around we deserialize the `value` field // which is the actual deserializer if self.hits == 1 { - return seed - .deserialize(self.de.clone()) - .map_err(|e| e.with_key_context(&self.de.key, self.definition.clone())); + if let Some(de) = &self.de { + return seed + .deserialize(de.clone()) + .map_err(|e| e.with_key_context(&de.key, self.definition.clone())); + } else { + return seed + .deserialize(self.str_value.as_ref().unwrap().clone().into_deserializer()); + } } // ... otherwise we're deserializing the `definition` field, so we need @@ -484,6 +514,71 @@ impl<'de, 'config> de::MapAccess<'de> for ValueDeserializer<'config> { } } +// Deserializer is only implemented to handle deserializing a String inside a +// sequence (like `Vec` or `Vec>`). `Value` is +// handled by deserialize_struct, and the plain `String` is handled by all the +// other functions here. +impl<'de, 'config> de::Deserializer<'de> for ValueDeserializer<'config> { + type Error = ConfigError; + + fn deserialize_str(self, visitor: V) -> Result + where + V: de::Visitor<'de>, + { + visitor.visit_str(&self.str_value.expect("string expected")) + } + + fn deserialize_string(self, visitor: V) -> Result + where + V: de::Visitor<'de>, + { + visitor.visit_string(self.str_value.expect("string expected")) + } + + fn deserialize_struct( + self, + name: &'static str, + fields: &'static [&'static str], + visitor: V, + ) -> Result + where + V: de::Visitor<'de>, + { + // Match on the magical struct name/field names that are passed in to + // detect when we're deserializing `Value`. + // + // See more comments in `value.rs` for the protocol used here. + if name == value::NAME && fields == value::FIELDS { + return visitor.visit_map(self); + } + unimplemented!("only strings and Value can be deserialized from a sequence"); + } + + fn deserialize_any(self, visitor: V) -> Result + where + V: de::Visitor<'de>, + { + visitor.visit_string(self.str_value.expect("string expected")) + } + + fn deserialize_ignored_any(self, visitor: V) -> Result + where + V: de::Visitor<'de>, + { + visitor.visit_unit() + } + + serde::forward_to_deserialize_any! { + i8 i16 i32 i64 + u8 u16 u32 u64 + option + newtype_struct seq tuple tuple_struct map enum bool + f32 f64 char bytes + byte_buf unit unit_struct + identifier + } +} + /// A deserializer which takes two values and deserializes into a tuple of those /// two values. This is similar to types like `StrDeserializer` in upstream /// serde itself. From 026bda3fb5eddac0df111ee150706f756558a7b3 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 9 Dec 2022 20:38:12 -0800 Subject: [PATCH 3/9] Support configuring ssh known-hosts via cargo config. --- src/cargo/sources/git/known_hosts.rs | 57 ++++++++++++++++++---- src/cargo/sources/git/utils.rs | 11 ++++- src/cargo/util/config/mod.rs | 19 ++++++++ src/doc/src/appendix/git-authentication.md | 6 ++- src/doc/src/reference/config.md | 38 +++++++++++++++ 5 files changed, 119 insertions(+), 12 deletions(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 875dcf63f35..7efea43c3bf 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -20,6 +20,7 @@ //! hostname patterns, and revoked markers. See "FIXME" comments littered in //! this file. +use crate::util::config::{Definition, Value}; use git2::cert::Cert; use git2::CertificateCheckStatus; use std::collections::HashSet; @@ -74,6 +75,8 @@ impl From for KnownHostError { enum KnownHostLocation { /// Loaded from a file from disk. File { path: PathBuf, lineno: u32 }, + /// Loaded from cargo's config system. + Config { definition: Definition }, /// Part of the hard-coded bundled keys in Cargo. Bundled, } @@ -83,6 +86,8 @@ pub fn certificate_check( cert: &Cert<'_>, host: &str, port: Option, + config_known_hosts: Option<&Vec>>, + diagnostic_home_config: &str, ) -> Result { let Some(host_key) = cert.as_hostkey() else { // Return passthrough for TLS X509 certificates to use whatever validation @@ -96,7 +101,7 @@ pub fn certificate_check( _ => host.to_string(), }; // The error message must be constructed as a string to pass through the libgit2 C API. - let err_msg = match check_ssh_known_hosts(host_key, &host_maybe_port) { + let err_msg = match check_ssh_known_hosts(host_key, &host_maybe_port, config_known_hosts) { Ok(()) => { return Ok(CertificateCheckStatus::CertificateOk); } @@ -113,13 +118,13 @@ pub fn certificate_check( // Try checking without the port. if port.is_some() && !matches!(port, Some(22)) - && check_ssh_known_hosts(host_key, host).is_ok() + && check_ssh_known_hosts(host_key, host, config_known_hosts).is_ok() { return Ok(CertificateCheckStatus::CertificateOk); } let key_type_short_name = key_type.short_name(); let key_type_name = key_type.name(); - let known_hosts_location = user_known_host_location_to_add(); + let known_hosts_location = user_known_host_location_to_add(diagnostic_home_config); let other_hosts_message = if other_hosts.is_empty() { String::new() } else { @@ -132,6 +137,9 @@ pub fn certificate_check( KnownHostLocation::File { path, lineno } => { format!("{} line {lineno}", path.display()) } + KnownHostLocation::Config { definition } => { + format!("config value from {definition}") + } KnownHostLocation::Bundled => format!("bundled with cargo"), }; write!(msg, " {loc}: {}\n", known_host.patterns).unwrap(); @@ -163,7 +171,7 @@ pub fn certificate_check( }) => { let key_type_short_name = key_type.short_name(); let key_type_name = key_type.name(); - let known_hosts_location = user_known_host_location_to_add(); + let known_hosts_location = user_known_host_location_to_add(diagnostic_home_config); let old_key_resolution = match old_known_host.location { KnownHostLocation::File { path, lineno } => { let old_key_location = path.display(); @@ -173,6 +181,13 @@ pub fn certificate_check( and adding the new key to {known_hosts_location}", ) } + KnownHostLocation::Config { definition } => { + format!( + "removing the old {key_type_name} key for `{hostname}` \ + loaded from Cargo's config at {definition}, \ + and adding the new key to {known_hosts_location}" + ) + } KnownHostLocation::Bundled => { format!( "adding the new key to {known_hosts_location}\n\ @@ -217,6 +232,7 @@ pub fn certificate_check( fn check_ssh_known_hosts( cert_host_key: &git2::cert::CertHostkey<'_>, host: &str, + config_known_hosts: Option<&Vec>>, ) -> Result<(), KnownHostError> { let Some(remote_host_key) = cert_host_key.hostkey() else { return Err(anyhow::format_err!("remote host key is not available").into()); @@ -237,6 +253,23 @@ fn check_ssh_known_hosts( let hosts = load_hostfile(&path)?; known_hosts.extend(hosts); } + if let Some(config_known_hosts) = config_known_hosts { + // Format errors aren't an error in case the format needs to change in + // the future, to retain forwards compatibility. + for line_value in config_known_hosts { + let location = KnownHostLocation::Config { + definition: line_value.definition.clone(), + }; + match parse_known_hosts_line(&line_value.val, location) { + Some(known_host) => known_hosts.push(known_host), + None => log::warn!( + "failed to parse known host {} from {}", + line_value.val, + line_value.definition + ), + } + } + } // Load the bundled keys. Don't add keys for hosts that the user has // configured, which gives them the option to override them. This could be // useful if the keys are ever revoked. @@ -363,12 +396,18 @@ fn user_known_host_location() -> Option { /// The location to display in an error message instructing the user where to /// add the new key. -fn user_known_host_location_to_add() -> String { +fn user_known_host_location_to_add(diagnostic_home_config: &str) -> String { // Note that we don't bother with the legacy known_hosts2 files. - match user_known_host_location() { - Some(path) => path.to_str().expect("utf-8 home").to_string(), - None => "~/.ssh/known_hosts".to_string(), - } + let user = user_known_host_location(); + let openssh_loc = match &user { + Some(path) => path.to_str().expect("utf-8 home"), + None => "~/.ssh/known_hosts", + }; + format!( + "the `net.ssh.known-hosts` array in your Cargo configuration \ + (such as {diagnostic_home_config}) \ + or in your OpenSSH known_hosts file at {openssh_loc}" + ) } /// A single known host entry. diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 831c43be6bc..457c97c5bb0 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -726,6 +726,9 @@ pub fn with_fetch_options( cb: &mut dyn FnMut(git2::FetchOptions<'_>) -> CargoResult<()>, ) -> CargoResult<()> { let mut progress = Progress::new("Fetch", config); + let ssh_config = config.net_config()?.ssh.as_ref(); + let config_known_hosts = ssh_config.and_then(|ssh| ssh.known_hosts.as_ref()); + let diagnostic_home_config = config.diagnostic_home_config(); network::with_retry(config, || { with_authentication(url, git_config, |f| { let port = Url::parse(url).ok().and_then(|url| url.port()); @@ -736,7 +739,13 @@ pub fn with_fetch_options( let mut counter = MetricsCounter::<10>::new(0, last_update); rcb.credentials(f); rcb.certificate_check(|cert, host| { - super::known_hosts::certificate_check(cert, host, port) + super::known_hosts::certificate_check( + cert, + host, + port, + config_known_hosts, + &diagnostic_home_config, + ) }); rcb.transfer_progress(|stats| { let indexed_deltas = stats.indexed_deltas(); diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index d30e094413f..d9ab142c4e2 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -356,6 +356,18 @@ impl Config { &self.home_path } + /// Returns a path to display to the user with the location of their home + /// config file (to only be used for displaying a diagnostics suggestion, + /// such as recommending where to add a config value). + pub fn diagnostic_home_config(&self) -> String { + let home = self.home_path.as_path_unlocked(); + let path = match self.get_file_path(home, "config", false) { + Ok(Some(existing_path)) => existing_path, + _ => home.join("config.toml"), + }; + path.to_string_lossy().to_string() + } + /// Gets the Cargo Git directory (`/git`). pub fn git_path(&self) -> Filesystem { self.home_path.join("git") @@ -2356,6 +2368,13 @@ pub struct CargoNetConfig { pub retry: Option, pub offline: Option, pub git_fetch_with_cli: Option, + pub ssh: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct CargoSshConfig { + pub known_hosts: Option>>, } #[derive(Debug, Deserialize)] diff --git a/src/doc/src/appendix/git-authentication.md b/src/doc/src/appendix/git-authentication.md index a7db1ac7f15..f46a6535c6b 100644 --- a/src/doc/src/appendix/git-authentication.md +++ b/src/doc/src/appendix/git-authentication.md @@ -66,7 +66,8 @@ known hosts in OpenSSH-style `known_hosts` files located in their standard locations (`.ssh/known_hosts` in your home directory, or `/etc/ssh/ssh_known_hosts` on Unix-like platforms or `%PROGRAMDATA%\ssh\ssh_known_hosts` on Windows). More information about these -files can be found in the [sshd man page]. +files can be found in the [sshd man page]. Alternatively, keys may be +configured in a Cargo configuration file with [`net.ssh.known-hosts`]. When connecting to an SSH host before the known hosts has been configured, Cargo will display an error message instructing you how to add the host key. @@ -78,10 +79,11 @@ publish their fingerprints on the web; for example GitHub posts theirs at . Cargo comes with the host keys for [github.com](https://github.com) built-in. -If those ever change, you can add the new keys to your known_hosts file. +If those ever change, you can add the new keys to the config or known_hosts file. [`credential.helper`]: https://git-scm.com/book/en/v2/Git-Tools-Credential-Storage [`net.git-fetch-with-cli`]: ../reference/config.md#netgit-fetch-with-cli +[`net.ssh.known-hosts`]: ../reference/config.md#netsshknown-hosts [GCM]: https://github.com/microsoft/Git-Credential-Manager-Core/ [PuTTY]: https://www.chiark.greenend.org.uk/~sgtatham/putty/ [Microsoft installation documentation]: https://docs.microsoft.com/en-us/windows-server/administration/openssh/openssh_install_firstuse diff --git a/src/doc/src/reference/config.md b/src/doc/src/reference/config.md index 1e506487975..f804ceebeae 100644 --- a/src/doc/src/reference/config.md +++ b/src/doc/src/reference/config.md @@ -114,6 +114,9 @@ retry = 2 # network retries git-fetch-with-cli = true # use the `git` executable for git operations offline = true # do not access the network +[net.ssh] +known-hosts = ["..."] # known SSH host keys + [patch.] # Same keys as for [patch] in Cargo.toml @@ -750,6 +753,41 @@ needed, and generate an error if it encounters a network error. Can be overridden with the `--offline` command-line option. +##### `net.ssh` + +The `[net.ssh]` table contains settings for SSH connections. + +##### `net.ssh.known-hosts` +* Type: array of strings +* Default: see description +* Environment: not supported + +The `known-hosts` array contains a list of SSH host keys that should be +accepted as valid when connecting to an SSH server (such as for SSH git +dependencies). Each entry should be a string in a format similar to OpenSSH +`known_hosts` files. Each string should start with one or more hostnames +separated by commas, a space, the key type name, a space, and the +base64-encoded key. For example: + +```toml +[net.ssh] +known-hosts = [ + "example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFO4Q5T0UV0SQevair9PFwoxY9dl4pQl3u5phoqJH3cF" +] +``` + +Cargo will attempt to load known hosts keys from common locations supported in +OpenSSH, and will join those with any listed in a Cargo configuration file. +If any matching entry has the correct key, the connection will be allowed. + +Cargo comes with the host keys for [github.com][github-keys] built-in. If +those ever change, you can add the new keys to the config or known_hosts file. + +See [Git Authentication](../appendix/git-authentication.md#ssh-known-hosts) +for more details. + +[github-keys]: https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/githubs-ssh-key-fingerprints + #### `[patch]` Just as you can override dependencies using [`[patch]` in From 302a543ddf3b7621c2f10623862029d35fae7e3c Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 12 Dec 2022 20:14:23 -0800 Subject: [PATCH 4/9] Add some known_hosts tests. This also fixes a bug with the host matching when there are comma-separated hosts. --- src/cargo/sources/git/known_hosts.rs | 160 +++++++++++++++++++++++++-- 1 file changed, 148 insertions(+), 12 deletions(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 7efea43c3bf..58e64e79131 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -21,7 +21,7 @@ //! this file. use crate::util::config::{Definition, Value}; -use git2::cert::Cert; +use git2::cert::{Cert, SshHostKeyType}; use git2::CertificateCheckStatus; use std::collections::HashSet; use std::fmt::Write; @@ -49,7 +49,7 @@ enum KnownHostError { /// The host key was not found. HostKeyNotFound { hostname: String, - key_type: git2::cert::SshHostKeyType, + key_type: SshHostKeyType, remote_host_key: String, remote_fingerprint: String, other_hosts: Vec, @@ -57,7 +57,7 @@ enum KnownHostError { /// The host key was found, but does not match the remote's key. HostKeyHasChanged { hostname: String, - key_type: git2::cert::SshHostKeyType, + key_type: SshHostKeyType, old_known_host: KnownHost, remote_host_key: String, remote_fingerprint: String, @@ -238,11 +238,6 @@ fn check_ssh_known_hosts( return Err(anyhow::format_err!("remote host key is not available").into()); }; let remote_key_type = cert_host_key.hostkey_type().unwrap(); - // `changed_key` keeps track of any entries where the key has changed. - let mut changed_key = None; - // `other_hosts` keeps track of any entries that have an identical key, - // but a different hostname. - let mut other_hosts = Vec::new(); // Collect all the known host entries from disk. let mut known_hosts = Vec::new(); @@ -293,6 +288,21 @@ fn check_ssh_known_hosts( }); } } + check_ssh_known_hosts_loaded(&known_hosts, host, remote_key_type, remote_host_key) +} + +/// Checks a host key against a loaded set of known hosts. +fn check_ssh_known_hosts_loaded( + known_hosts: &[KnownHost], + host: &str, + remote_key_type: SshHostKeyType, + remote_host_key: &[u8], +) -> Result<(), KnownHostError> { + // `changed_key` keeps track of any entries where the key has changed. + let mut changed_key = None; + // `other_hosts` keeps track of any entries that have an identical key, + // but a different hostname. + let mut other_hosts = Vec::new(); for known_host in known_hosts { // The key type from libgit2 needs to match the key type from the host file. @@ -301,7 +311,6 @@ fn check_ssh_known_hosts( } let key_matches = known_host.key == remote_host_key; if !known_host.host_matches(host) { - // `name` can be None for hashed hostnames (which libgit2 does not expose). if key_matches { other_hosts.push(known_host.clone()); } @@ -434,7 +443,7 @@ impl KnownHost { return false; } } else { - match_found = pattern == host; + match_found |= pattern == host; } } match_found @@ -444,6 +453,10 @@ impl KnownHost { /// Loads an OpenSSH known_hosts file. fn load_hostfile(path: &Path) -> Result, anyhow::Error> { let contents = cargo_util::paths::read(path)?; + Ok(load_hostfile_contents(path, &contents)) +} + +fn load_hostfile_contents(path: &Path, contents: &str) -> Vec { let entries = contents .lines() .enumerate() @@ -455,13 +468,13 @@ fn load_hostfile(path: &Path) -> Result, anyhow::Error> { parse_known_hosts_line(line, location) }) .collect(); - Ok(entries) + entries } fn parse_known_hosts_line(line: &str, location: KnownHostLocation) -> Option { let line = line.trim(); // FIXME: @revoked and @cert-authority is currently not supported. - if line.is_empty() || line.starts_with('#') || line.starts_with('@') { + if line.is_empty() || line.starts_with(['#', '@', '|']) { return None; } let mut parts = line.split([' ', '\t']).filter(|s| !s.is_empty()); @@ -476,3 +489,126 @@ fn parse_known_hosts_line(line: &str, location: KnownHostLocation) -> Option { + assert_eq!(path, kh_path); + assert_eq!(*lineno, 4); + } + _ => panic!("unexpected"), + } + assert_eq!(khs[0].patterns, "example.com,rust-lang.org"); + assert_eq!(khs[0].key_type, "ssh-rsa"); + assert_eq!(khs[0].key.len(), 407); + assert_eq!(&khs[0].key[..30], b"\x00\x00\x00\x07ssh-rsa\x00\x00\x00\x03\x01\x00\x01\x00\x00\x01\x81\x00\xb935\x88\xa5\x9c)"); + match &khs[1].location { + KnownHostLocation::File { path, lineno } => { + assert_eq!(path, kh_path); + assert_eq!(*lineno, 5); + } + _ => panic!("unexpected"), + } + assert_eq!(khs[2].patterns, "[example.net]:2222"); + assert_eq!(khs[3].patterns, "nistp256.example.org"); + assert_eq!(khs[7].patterns, "192.168.42.12"); + } + + #[test] + fn host_matches() { + let kh_path = Path::new("/home/abc/.known_hosts"); + let khs = load_hostfile_contents(kh_path, COMMON_CONTENTS); + assert!(khs[0].host_matches("example.com")); + assert!(khs[0].host_matches("rust-lang.org")); + assert!(khs[0].host_matches("EXAMPLE.COM")); + assert!(khs[1].host_matches("example.net")); + assert!(!khs[0].host_matches("example.net")); + assert!(khs[2].host_matches("[example.net]:2222")); + assert!(!khs[2].host_matches("example.net")); + assert!(!khs[8].host_matches("neg.example.com")); + } + + #[test] + fn check_match() { + let kh_path = Path::new("/home/abc/.known_hosts"); + let khs = load_hostfile_contents(kh_path, COMMON_CONTENTS); + + assert!(check_ssh_known_hosts_loaded( + &khs, + "example.com", + SshHostKeyType::Rsa, + &khs[0].key + ) + .is_ok()); + + match check_ssh_known_hosts_loaded(&khs, "example.com", SshHostKeyType::Dss, &khs[0].key) { + Err(KnownHostError::HostKeyNotFound { + hostname, + remote_fingerprint, + other_hosts, + .. + }) => { + assert_eq!( + remote_fingerprint, + "yn+pONDn0EcgdOCVptgB4RZd/wqmsVKrPnQMLtrvhw8" + ); + assert_eq!(hostname, "example.com"); + assert_eq!(other_hosts.len(), 0); + } + _ => panic!("unexpected"), + } + + match check_ssh_known_hosts_loaded( + &khs, + "foo.example.com", + SshHostKeyType::Rsa, + &khs[0].key, + ) { + Err(KnownHostError::HostKeyNotFound { other_hosts, .. }) => { + assert_eq!(other_hosts.len(), 1); + assert_eq!(other_hosts[0].patterns, "example.com,rust-lang.org"); + } + _ => panic!("unexpected"), + } + + let mut modified_key = khs[0].key.clone(); + modified_key[0] = 1; + match check_ssh_known_hosts_loaded(&khs, "example.com", SshHostKeyType::Rsa, &modified_key) + { + Err(KnownHostError::HostKeyHasChanged { old_known_host, .. }) => { + assert!(matches!( + old_known_host.location, + KnownHostLocation::File { lineno: 4, .. } + )); + } + _ => panic!("unexpected"), + } + } +} From cf716fc3c2b0785013b321f08d6cf9e277f89c84 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 13 Dec 2022 08:14:59 -0800 Subject: [PATCH 5/9] Remove let-else, just use ? propagation. Co-authored-by: Weihang Lo --- src/cargo/sources/git/known_hosts.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 58e64e79131..f2721953061 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -478,10 +478,9 @@ fn parse_known_hosts_line(line: &str, location: KnownHostLocation) -> Option Date: Wed, 14 Dec 2022 19:01:40 -0800 Subject: [PATCH 6/9] Add test for config Value in TOML array. --- tests/testsuite/config.rs | 58 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index b1d07bb4050..d1487833f71 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -1,7 +1,7 @@ //! Tests for config settings. use cargo::core::{PackageIdSpec, Shell}; -use cargo::util::config::{self, Config, SslVersionConfig, StringList}; +use cargo::util::config::{self, Config, Definition, SslVersionConfig, StringList}; use cargo::util::interning::InternedString; use cargo::util::toml::{self, VecStringOrBool as VSOB}; use cargo::CargoResult; @@ -1508,3 +1508,59 @@ fn all_profile_options() { let roundtrip_toml = toml_edit::easy::to_string(&roundtrip).unwrap(); compare::assert_match_exact(&profile_toml, &roundtrip_toml); } + +#[cargo_test] +fn value_in_array() { + // Value in an array should work + let root_path = paths::root().join(".cargo/config.toml"); + write_config_at( + &root_path, + "\ +[net.ssh] +known-hosts = [ + \"example.com ...\", + \"example.net ...\", +] +", + ); + + let foo_path = paths::root().join("foo/.cargo/config.toml"); + write_config_at( + &foo_path, + "\ +[net.ssh] +known-hosts = [ + \"example.org ...\", +] +", + ); + + let config = ConfigBuilder::new() + .cwd("foo") + // environment variables don't actually work for known-hosts due to + // space splitting, but this is included here just to validate that + // they work (particularly if other Vec config vars are added + // in the future). + .env("CARGO_NET_SSH_KNOWN_HOSTS", "env-example") + .build(); + let net_config = config.net_config().unwrap(); + let kh = net_config + .ssh + .as_ref() + .unwrap() + .known_hosts + .as_ref() + .unwrap(); + assert_eq!(kh.len(), 4); + assert_eq!(kh[0].val, "example.org ..."); + assert_eq!(kh[0].definition, Definition::Path(foo_path.clone())); + assert_eq!(kh[1].val, "example.com ..."); + assert_eq!(kh[1].definition, Definition::Path(root_path.clone())); + assert_eq!(kh[2].val, "example.net ..."); + assert_eq!(kh[2].definition, Definition::Path(root_path.clone())); + assert_eq!(kh[3].val, "env-example"); + assert_eq!( + kh[3].definition, + Definition::Environment("CARGO_NET_SSH_KNOWN_HOSTS".to_string()) + ); +} From 67ae2dcafea5955824b1f390568a5fa109424987 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 28 Dec 2022 15:52:10 -0800 Subject: [PATCH 7/9] ssh known_hosts: support hashed hostnames --- Cargo.toml | 2 ++ src/cargo/sources/git/known_hosts.rs | 33 ++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b3f55479ddc..ef88bb14633 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ git2 = "0.16.0" git2-curl = "0.17.0" glob = "0.3.0" hex = "0.4" +hmac = "0.12.1" home = "0.5" http-auth = { version = "0.1.6", default-features = false } humantime = "2.0.0" @@ -56,6 +57,7 @@ serde = { version = "1.0.123", features = ["derive"] } serde_ignored = "0.1.0" serde_json = { version = "1.0.30", features = ["raw_value"] } serde-value = "0.7.0" +sha1 = "0.10.5" shell-escape = "0.1.4" strip-ansi-escapes = "0.1.0" tar = { version = "0.4.38", default-features = false } diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index f2721953061..d61fd9b685a 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -16,13 +16,13 @@ //! - `VerifyHostKeyDNS` — Uses SSHFP DNS records to fetch a host key. //! //! There's also a number of things that aren't supported but could be easily -//! added (it just adds a little complexity). For example, hashed hostnames, -//! hostname patterns, and revoked markers. See "FIXME" comments littered in -//! this file. +//! added (it just adds a little complexity). For example, hostname patterns, +//! and revoked markers. See "FIXME" comments littered in this file. use crate::util::config::{Definition, Value}; use git2::cert::{Cert, SshHostKeyType}; use git2::CertificateCheckStatus; +use hmac::Mac; use std::collections::HashSet; use std::fmt::Write; use std::path::{Path, PathBuf}; @@ -419,6 +419,8 @@ fn user_known_host_location_to_add(diagnostic_home_config: &str) -> String { ) } +const HASH_HOSTNAME_PREFIX: &str = "|1|"; + /// A single known host entry. #[derive(Clone)] struct KnownHost { @@ -434,7 +436,9 @@ impl KnownHost { fn host_matches(&self, host: &str) -> bool { let mut match_found = false; let host = host.to_lowercase(); - // FIXME: support hashed hostnames + if let Some(hashed) = self.patterns.strip_prefix(HASH_HOSTNAME_PREFIX) { + return hashed_hostname_matches(&host, hashed); + } for pattern in self.patterns.split(',') { let pattern = pattern.to_lowercase(); // FIXME: support * and ? wildcards @@ -450,6 +454,16 @@ impl KnownHost { } } +fn hashed_hostname_matches(host: &str, hashed: &str) -> bool { + let Some((b64_salt, b64_host)) = hashed.split_once('|') else { return false; }; + let Ok(salt) = base64::decode(b64_salt) else { return false; }; + let Ok(hashed_host) = base64::decode(b64_host) else { return false; }; + let Ok(mut mac) = hmac::Hmac::::new_from_slice(&salt) else { return false; }; + mac.update(host.as_bytes()); + let result = mac.finalize().into_bytes(); + hashed_host == &result[..] +} + /// Loads an OpenSSH known_hosts file. fn load_hostfile(path: &Path) -> Result, anyhow::Error> { let contents = cargo_util::paths::read(path)?; @@ -474,7 +488,7 @@ fn load_hostfile_contents(path: &Path, contents: &str) -> Vec { fn parse_known_hosts_line(line: &str, location: KnownHostLocation) -> Option { let line = line.trim(); // FIXME: @revoked and @cert-authority is currently not supported. - if line.is_empty() || line.starts_with(['#', '@', '|']) { + if line.is_empty() || line.starts_with(['#', '@']) { return None; } let mut parts = line.split([' ', '\t']).filter(|s| !s.is_empty()); @@ -506,8 +520,7 @@ mod tests { @revoked * ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKtQsi+KPYispwm2rkMidQf30fG1Niy8XNkvASfePoca eric@host example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAWkjI6XT2SZh3xNk5NhisA3o3sGzWR+VAKMSqHtI0aY eric@host 192.168.42.12 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host - # Hash not yet supported. - |1|7CMSYgzdwruFLRhwowMtKx0maIE=|Tlff1GFqc3Ao+fUWxMEVG8mJiyk= ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIHgN3O21U4LWtP5OzjTzPnUnSDmCNDvyvlaj6Hi65JC eric@host + |1|QxzZoTXIWLhUsuHAXjuDMIV3FjQ=|M6NCOIkjiWdCWqkh5+Q+/uFLGjs= ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIHgN3O21U4LWtP5OzjTzPnUnSDmCNDvyvlaj6Hi65JC eric@host # Negation isn't terribly useful without globs. neg.example.com,!neg.example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOXfUnaAHTlo1Qi//rNk26OcmHikmkns1Z6WW/UuuS3K eric@host "#; @@ -516,7 +529,7 @@ mod tests { fn known_hosts_parse() { let kh_path = Path::new("/home/abc/.known_hosts"); let khs = load_hostfile_contents(kh_path, COMMON_CONTENTS); - assert_eq!(khs.len(), 9); + assert_eq!(khs.len(), 10); match &khs[0].location { KnownHostLocation::File { path, lineno } => { assert_eq!(path, kh_path); @@ -551,7 +564,9 @@ mod tests { assert!(!khs[0].host_matches("example.net")); assert!(khs[2].host_matches("[example.net]:2222")); assert!(!khs[2].host_matches("example.net")); - assert!(!khs[8].host_matches("neg.example.com")); + assert!(khs[8].host_matches("hashed.example.com")); + assert!(!khs[8].host_matches("example.com")); + assert!(!khs[9].host_matches("neg.example.com")); } #[test] From 23c547166c8f64e521d3081cdb880c2ff4575cff Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 5 Jan 2023 07:13:22 -0800 Subject: [PATCH 8/9] update changelog. --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc0ebf4c74b..7e5d2ba5b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,13 @@ - Added documentation of config option `registries.crates-io.protocol`. [#11350](https://github.com/rust-lang/cargo/pull/11350) +## Cargo 1.66.1 (2023-01-10) + +### Fixed +- [CVE-2022-46176](https://github.com/rust-lang/cargo/security/advisories/GHSA-r5w3-xm58-jv6j): + Added validation of SSH host keys for git URLs. + See [the docs](https://doc.rust-lang.org/cargo/appendix/git-authentication.html#ssh-known-hosts) for more information on how to configure the known host keys. + ## Cargo 1.66 (2022-12-15) [08250398...rust-1.66.0](https://github.com/rust-lang/cargo/compare/08250398...rust-1.66.0) From d992ab4e9034930e7809749f04077045af8f4d79 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 5 Jan 2023 07:43:28 -0800 Subject: [PATCH 9/9] known_hosts: Switch the documentation to stable. When making the stable release, the nightly docs won't be updated, yet. This makes sure that the link will work for the stable release. --- src/cargo/sources/git/known_hosts.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index d61fd9b685a..ca732fafd4a 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -158,7 +158,7 @@ pub fn certificate_check( The {key_type_short_name} key fingerprint is: SHA256:{remote_fingerprint}\n\ This fingerprint should be validated with the server administrator that it is correct.\n\ {other_hosts_message}\n\ - See https://doc.rust-lang.org/nightly/cargo/appendix/git-authentication.html#ssh-known-hosts \ + See https://doc.rust-lang.org/stable/cargo/appendix/git-authentication.html#ssh-known-hosts \ for more information.\n\ ") } @@ -216,7 +216,7 @@ pub fn certificate_check( \n\ {hostname} {key_type_name} {remote_host_key}\n\ \n\ - See https://doc.rust-lang.org/nightly/cargo/appendix/git-authentication.html#ssh-known-hosts \ + See https://doc.rust-lang.org/stable/cargo/appendix/git-authentication.html#ssh-known-hosts \ for more information.\n\ ") }