diff --git a/Cargo.toml b/Cargo.toml index 987b0e3de..af87b705e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo" -version = "0.69.0" +version = "0.69.1" edition = "2021" license = "MIT OR Apache-2.0" homepage = "https://crates.io" diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index c8466d607..8e1563a5e 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -24,7 +24,7 @@ use git2::cert::{Cert, SshHostKeyType}; use git2::CertificateCheckStatus; use hmac::Mac; use std::collections::HashSet; -use std::fmt::Write; +use std::fmt::{Display, Write}; use std::path::{Path, PathBuf}; /// These are host keys that are hard-coded in cargo to provide convenience. @@ -40,6 +40,20 @@ use std::path::{Path, PathBuf}; static BUNDLED_KEYS: &[(&str, &str, &str)] = &[ ("github.com", "ssh-ed25519", "AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl"), ("github.com", "ecdsa-sha2-nistp256", "AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg="), + ("github.com", "ssh-rsa", "AAAAB3NzaC1yc2EAAAADAQABAAABgQCj7ndNxQowgcQnjshcLrqPEiiphnt+VTTvDP6mHBL9j1aNUkY4Ue1gvwnGLVlOhGeYrnZaMgRK6+PKCUXaDbC7qtbW8gIkhL7aGCsOr/C56SJMy/BCZfxd1nWzAOxSDPgVsmerOBYfNqltV9/hWCqBywINIR+5dIg6JTJ72pcEpEjcYgXkE2YEFXV1JHnsKgbLWNlhScqb2UmyRkQyytRLtL+38TGxkxCflmO+5Z8CSSNY7GidjMIZ7Q4zMjA2n1nGrlTDkzwDCsw+wqFPGQA179cnfGWOWRVruj16z6XyvxvjJwbz0wQZ75XK5tKSb7FNyeIEs4TT4jk+S4dhPeAUC5y+bDYirYgM4GC7uEnztnZyaVWQ7B381AK4Qdrwt51ZqExKbQpTUNn+EjqoTwvqNj4kqx5QUCI0ThS/YkOxJCXmPUWZbhjpCg56i+2aB6CmK2JGhn57K5mj0MNdBXA4/WnwH6XoPWJzK5Nyu2zB3nAZp+S5hpQs+p1vN1/wsjk="), +]; + +/// List of keys that public hosts have rotated away from. +/// +/// We explicitly distrust these keys as users with the old key in their +/// local configuration will otherwise be vulnerable to MITM attacks if the +/// attacker has access to the old key. As there is no other way to distribute +/// revocations of ssh host keys, we need to bundle them with the client. +/// +/// Unlike [`BUNDLED_KEYS`], these revocations will not be ignored if the user +/// has their own entries: we *know* that these keys are bad. +static BUNDLED_REVOCATIONS: &[(&str, &str, &str)] = &[ + // Used until March 24, 2023: https://github.blog/2023-03-23-we-updated-our-rsa-ssh-host-key/ ("github.com", "ssh-rsa", "AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ=="), ]; @@ -62,6 +76,19 @@ enum KnownHostError { remote_host_key: String, remote_fingerprint: String, }, + /// The host key was found with a @revoked marker, it must not be accepted. + HostKeyRevoked { + hostname: String, + key_type: SshHostKeyType, + remote_host_key: String, + location: KnownHostLocation, + }, + /// The host key was not found, but there was a matching known host with a + /// @cert-authority marker (which Cargo doesn't yet support). + HostHasOnlyCertAuthority { + hostname: String, + location: KnownHostLocation, + }, } impl From for KnownHostError { @@ -81,6 +108,21 @@ enum KnownHostLocation { Bundled, } +impl Display for KnownHostLocation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let loc = match self { + KnownHostLocation::File { path, lineno } => { + format!("{} line {lineno}", path.display()) + } + KnownHostLocation::Config { definition } => { + format!("config value from {definition}") + } + KnownHostLocation::Bundled => format!("bundled with cargo"), + }; + f.write_str(&loc) + } +} + /// The git2 callback used to validate a certificate (only ssh known hosts are validated). pub fn certificate_check( cert: &Cert<'_>, @@ -133,16 +175,13 @@ pub fn certificate_check( 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::Config { definition } => { - format!("config value from {definition}") - } - KnownHostLocation::Bundled => format!("bundled with cargo"), - }; - write!(msg, " {loc}: {}\n", known_host.patterns).unwrap(); + write!( + msg, + " {loc}: {patterns}\n", + loc = known_host.location, + patterns = known_host.patterns + ) + .unwrap(); } msg }; @@ -220,6 +259,39 @@ pub fn certificate_check( for more information.\n\ ") } + Err(KnownHostError::HostKeyRevoked { + hostname, + key_type, + remote_host_key, + location, + }) => { + let key_type_short_name = key_type.short_name(); + format!( + "error: Key has been revoked for `{hostname}`\n\ + **************************************\n\ + * WARNING: REVOKED HOST KEY DETECTED *\n\ + **************************************\n\ + This may indicate that the key provided by this host has been\n\ + compromised and should not be accepted. + \n\ + The host key {key_type_short_name} {remote_host_key} is revoked\n\ + in {location} and has been rejected.\n\ + " + ) + } + Err(KnownHostError::HostHasOnlyCertAuthority { hostname, location }) => { + format!("error: Found a `@cert-authority` marker for `{hostname}`\n\ + \n\ + Cargo doesn't support certificate authorities for host key verification. It is\n\ + recommended that the command line Git client is used instead. This can be achieved\n\ + by setting `net.git-fetch-with-cli` to `true` in the Cargo config.\n\ + \n + The `@cert-authority` line was found in {location}.\n\ + \n\ + See https://doc.rust-lang.org/stable/cargo/appendix/git-authentication.html#ssh-known-hosts \ + for more information.\n\ + ") + } }; Err(git2::Error::new( git2::ErrorCode::GenericError, @@ -285,9 +357,20 @@ fn check_ssh_known_hosts( patterns: patterns.to_string(), key_type: key_type.to_string(), key, + line_type: KnownHostLineType::Key, }); } } + for (patterns, key_type, key) in BUNDLED_REVOCATIONS { + let key = base64::decode(key).unwrap(); + known_hosts.push(KnownHost { + location: KnownHostLocation::Bundled, + patterns: patterns.to_string(), + key_type: key_type.to_string(), + key, + line_type: KnownHostLineType::Revoked, + }); + } check_ssh_known_hosts_loaded(&known_hosts, host, remote_key_type, remote_host_key) } @@ -298,12 +381,28 @@ fn check_ssh_known_hosts_loaded( 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; + // `latent_error` keeps track of a potential error that will be returned + // in case a matching host key isn't found. + let mut latent_errors: Vec = Vec::new(); + // `other_hosts` keeps track of any entries that have an identical key, // but a different hostname. let mut other_hosts = Vec::new(); + // `accepted_known_host_found` keeps track of whether we've found a matching + // line in the `known_hosts` file that we would accept. We can't return that + // immediately, because there may be a subsequent @revoked key. + let mut accepted_known_host_found = false; + + // 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.clone()); + let remote_fingerprint = + base64::encode_config(remote_fingerprint.finish(), base64::STANDARD_NO_PAD); + let remote_host_key_encoded = base64::encode(remote_host_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() { @@ -316,42 +415,75 @@ fn check_ssh_known_hosts_loaded( } continue; } - if key_matches { - return Ok(()); + match known_host.line_type { + KnownHostLineType::Key => { + if key_matches { + accepted_known_host_found = true; + } else { + // 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. + latent_errors.push(KnownHostError::HostKeyHasChanged { + hostname: host.to_string(), + key_type: remote_key_type, + old_known_host: known_host.clone(), + remote_host_key: remote_host_key_encoded.clone(), + remote_fingerprint: remote_fingerprint.clone(), + }); + } + } + KnownHostLineType::Revoked => { + if key_matches { + return Err(KnownHostError::HostKeyRevoked { + hostname: host.to_string(), + key_type: remote_key_type, + remote_host_key: remote_host_key_encoded, + location: known_host.location.clone(), + }); + } + } + KnownHostLineType::CertAuthority => { + // The host matches a @cert-authority line, which is unsupported. + latent_errors.push(KnownHostError::HostHasOnlyCertAuthority { + hostname: host.to_string(), + location: known_host.location.clone(), + }); + } } - // 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 { + + // We have an accepted host key and it hasn't been revoked. + if accepted_known_host_found { + return Ok(()); + } + + if latent_errors.is_empty() { + // 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. + Err(KnownHostError::HostKeyNotFound { 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_host_key: remote_host_key_encoded, remote_fingerprint, other_hosts, - }), + }) + } else { + // We're going to take the first HostKeyHasChanged error if + // we find one, otherwise we'll take the first error (which + // we expect to be a CertAuthority error). + if let Some(index) = latent_errors + .iter() + .position(|e| matches!(e, KnownHostError::HostKeyHasChanged { .. })) + { + return Err(latent_errors.remove(index)); + } else { + // Otherwise, we take the first error (which we expect to be + // a CertAuthority error). + Err(latent_errors.pop().unwrap()) + } } } @@ -422,6 +554,13 @@ fn user_known_host_location_to_add(diagnostic_home_config: &str) -> String { const HASH_HOSTNAME_PREFIX: &str = "|1|"; +#[derive(Clone)] +enum KnownHostLineType { + Key, + CertAuthority, + Revoked, +} + /// A single known host entry. #[derive(Clone)] struct KnownHost { @@ -430,6 +569,7 @@ struct KnownHost { patterns: String, key_type: String, key: Vec, + line_type: KnownHostLineType, } impl KnownHost { @@ -488,15 +628,31 @@ 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()); + + let line_type = if line.starts_with("@") { + let line_type = parts.next()?; + + if line_type == "@cert-authority" { + KnownHostLineType::CertAuthority + } else if line_type == "@revoked" { + KnownHostLineType::Revoked + } else { + // No other markers are defined + return None; + } + } else { + KnownHostLineType::Key + }; + let patterns = parts.next()?; let key_type = parts.next()?; let key = parts.next().map(base64::decode)?.ok()?; Some(KnownHost { + line_type, location, patterns: patterns.to_string(), key_type: key_type.to_string(), @@ -517,8 +673,10 @@ mod tests { nistp256.example.org ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBJ4iYGCcJrUIfrHfzlsv8e8kaF36qpcUpe3VNAKVCZX/BDptIdlEe8u8vKNRTPgUO9jqS0+tjTcPiQd8/8I9qng= eric@host nistp384.example.org ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBNuGT3TqMz2rcwOt2ZqkiNqq7dvWPE66W2qPCoZsh0pQhVU3BnhKIc6nEr6+Wts0Z3jdF3QWwxbbTjbVTVhdr8fMCFhDCWiQFm9xLerYPKnu9qHvx9K87/fjc5+0pu4hLA== eric@host nistp521.example.org ecdsa-sha2-nistp521 AAAAE2VjZHNhLXNoYTItbmlzdHA1MjEAAAAIbmlzdHA1MjEAAACFBAD35HH6OsK4DN75BrKipVj/GvZaUzjPNa1F8wMjUdPB1JlVcUfgzJjWSxrhmaNN3u0soiZw8WNRFINsGPCw5E7DywF1689WcIj2Ye2rcy99je15FknScTzBBD04JgIyOI50mCUaPCBoF14vFlN6BmO00cFo+yzy5N8GuQ2sx9kr21xmFQ== eric@host - # Revoked not yet supported. - @revoked * ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKtQsi+KPYispwm2rkMidQf30fG1Niy8XNkvASfePoca eric@host + # Revoked is supported, but without Cert-Authority support, it will only negate some other fixed key. + @revoked revoked.example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKtQsi+KPYispwm2rkMidQf30fG1Niy8XNkvASfePoca eric@host + # Cert-Authority is not supported (below key should not be valid anyway) + @cert-authority ca.example.com ssh-rsa AABBB5Wm example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAWkjI6XT2SZh3xNk5NhisA3o3sGzWR+VAKMSqHtI0aY eric@host 192.168.42.12 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host |1|QxzZoTXIWLhUsuHAXjuDMIV3FjQ=|M6NCOIkjiWdCWqkh5+Q+/uFLGjs= ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIHgN3O21U4LWtP5OzjTzPnUnSDmCNDvyvlaj6Hi65JC eric@host @@ -530,7 +688,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(), 10); + assert_eq!(khs.len(), 12); match &khs[0].location { KnownHostLocation::File { path, lineno } => { assert_eq!(path, kh_path); @@ -551,7 +709,7 @@ mod tests { } 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"); + assert_eq!(khs[9].patterns, "192.168.42.12"); } #[test] @@ -565,9 +723,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("hashed.example.com")); - assert!(!khs[8].host_matches("example.com")); - assert!(!khs[9].host_matches("neg.example.com")); + assert!(khs[10].host_matches("hashed.example.com")); + assert!(!khs[10].host_matches("example.com")); + assert!(!khs[11].host_matches("neg.example.com")); } #[test] @@ -626,4 +784,130 @@ mod tests { _ => panic!("unexpected"), } } + + #[test] + fn revoked() { + let kh_path = Path::new("/home/abc/.known_hosts"); + let khs = load_hostfile_contents(kh_path, COMMON_CONTENTS); + + match check_ssh_known_hosts_loaded( + &khs, + "revoked.example.com", + SshHostKeyType::Ed255219, + &khs[6].key, + ) { + Err(KnownHostError::HostKeyRevoked { + hostname, location, .. + }) => { + assert_eq!("revoked.example.com", hostname); + assert!(matches!( + location, + KnownHostLocation::File { lineno: 11, .. } + )); + } + _ => panic!("Expected key to be revoked for revoked.example.com."), + } + } + + #[test] + fn cert_authority() { + let kh_path = Path::new("/home/abc/.known_hosts"); + let khs = load_hostfile_contents(kh_path, COMMON_CONTENTS); + + match check_ssh_known_hosts_loaded( + &khs, + "ca.example.com", + SshHostKeyType::Rsa, + &khs[0].key, // The key should not matter + ) { + Err(KnownHostError::HostHasOnlyCertAuthority { + hostname, location, .. + }) => { + assert_eq!("ca.example.com", hostname); + assert!(matches!( + location, + KnownHostLocation::File { lineno: 13, .. } + )); + } + Err(KnownHostError::HostKeyNotFound { hostname, .. }) => { + panic!("host key not found... {}", hostname); + } + _ => panic!("Expected host to only have @cert-authority line (which is unsupported)."), + } + } + + #[test] + fn multiple_errors() { + let contents = r#" + not-used.example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAWkjI6XT2SZh3xNk5NhisA3o3sGzWR+VAKMSqHtI0aY eric@host + # Cert-authority and changed key for the same host - changed key error should prevail + @cert-authority example.com ssh-ed25519 AABBB5Wm + example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host + "#; + + let kh_path = Path::new("/home/abc/.known_hosts"); + let khs = load_hostfile_contents(kh_path, contents); + + match check_ssh_known_hosts_loaded( + &khs, + "example.com", + SshHostKeyType::Ed255219, + &khs[0].key, + ) { + Err(KnownHostError::HostKeyHasChanged { + hostname, + old_known_host, + remote_host_key, + .. + }) => { + assert_eq!("example.com", hostname); + assert_eq!( + "AAAAC3NzaC1lZDI1NTE5AAAAIAWkjI6XT2SZh3xNk5NhisA3o3sGzWR+VAKMSqHtI0aY", + remote_host_key + ); + assert!(matches!( + old_known_host.location, + KnownHostLocation::File { lineno: 5, .. } + )); + } + _ => panic!("Expected error to be of type HostKeyHasChanged."), + } + } + + #[test] + fn known_host_and_revoked() { + let contents = r#" + example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host + # Later in the file the same host key is revoked + @revoked example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host + "#; + + let kh_path = Path::new("/home/abc/.known_hosts"); + let khs = load_hostfile_contents(kh_path, contents); + + match check_ssh_known_hosts_loaded( + &khs, + "example.com", + SshHostKeyType::Ed255219, + &khs[0].key, + ) { + Err(KnownHostError::HostKeyRevoked { + hostname, + remote_host_key, + location, + .. + }) => { + assert_eq!("example.com", hostname); + assert_eq!( + "AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR", + remote_host_key + ); + assert!(matches!( + location, + KnownHostLocation::File { lineno: 4, .. } + )); + } + _ => panic!("Expected host key to be reject with error HostKeyRevoked."), + } + } } diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md index 36482ff6e..df5f05bfa 100644 --- a/src/doc/src/reference/semver.md +++ b/src/doc/src/reference/semver.md @@ -702,7 +702,7 @@ impl Trait for Foo {} fn main() { let x = Foo; - x.foo(1); // Error: this function takes 0 arguments + x.foo(1); // Error: this method takes 0 arguments but 1 argument was supplied } ```