From 8965a6d54f452b6f96241f1df4f6a532702cf6b0 Mon Sep 17 00:00:00 2001 From: bors Date: Fri, 24 Mar 2023 21:03:52 +0000 Subject: [PATCH 1/6] Auto merge of #11883 - mitsuhiko:feature/new-github-rsa-host-key, r=arlosi Added new GitHub RSA Host Key GitHub rotated their RSA host key which means that cargo needs to update it. Thankfully the other keys were not rotated so the impact depends on how cargo connected to github. Refs https://github.blog/2023-03-23-we-updated-our-rsa-ssh-host-key/ --- src/cargo/sources/git/known_hosts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index c8466d607..1294471a9 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -40,7 +40,7 @@ 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", "AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ=="), + ("github.com", "ssh-rsa", "AAAAB3NzaC1yc2EAAAADAQABAAABgQCj7ndNxQowgcQnjshcLrqPEiiphnt+VTTvDP6mHBL9j1aNUkY4Ue1gvwnGLVlOhGeYrnZaMgRK6+PKCUXaDbC7qtbW8gIkhL7aGCsOr/C56SJMy/BCZfxd1nWzAOxSDPgVsmerOBYfNqltV9/hWCqBywINIR+5dIg6JTJ72pcEpEjcYgXkE2YEFXV1JHnsKgbLWNlhScqb2UmyRkQyytRLtL+38TGxkxCflmO+5Z8CSSNY7GidjMIZ7Q4zMjA2n1nGrlTDkzwDCsw+wqFPGQA179cnfGWOWRVruj16z6XyvxvjJwbz0wQZ75XK5tKSb7FNyeIEs4TT4jk+S4dhPeAUC5y+bDYirYgM4GC7uEnztnZyaVWQ7B381AK4Qdrwt51ZqExKbQpTUNn+EjqoTwvqNj4kqx5QUCI0ThS/YkOxJCXmPUWZbhjpCg56i+2aB6CmK2JGhn57K5mj0MNdBXA4/WnwH6XoPWJzK5Nyu2zB3nAZp+S5hpQs+p1vN1/wsjk="), ]; enum KnownHostError { From 52716f51a2532f5120bff1f20bd15febcb49f4f7 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 26 Mar 2023 10:30:09 -0700 Subject: [PATCH 2/6] Bump version for stable release. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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" From 02eceea125d825bf997c2ec979ee199fb9c59a3c Mon Sep 17 00:00:00 2001 From: bors Date: Thu, 9 Mar 2023 16:05:58 +0000 Subject: [PATCH 3/6] Auto merge of #11817 - weihanglo:semvercheck-fix, r=epage Fix semver check for 1.68 See https://github.com/rust-lang/cargo/actions/runs/4375746793/jobs/7656883351 Run `cd src/doc/semver-check && cargo +stable run` to test. --- src/doc/src/reference/semver.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 } ``` From 3d2d7cf4270f7e1f16e13e97d664c9fa80f402ab Mon Sep 17 00:00:00 2001 From: bors Date: Wed, 1 Feb 2023 03:48:14 +0000 Subject: [PATCH 4/6] Auto merge of #11635 - hds:ssh-known-hosts-markers, r=ehuss Add partial support for SSH known hosts markers ### What does this PR try to resolve? The SSH `known_hosts` file parsing in Cargo did not previously support markers. Markers are modifiers on the lines (``@cert-authority`` and ``@revoked`)` which denote special behavior for the details on that line. Lines were skipped entirely. This silent skipping of marker lines can be confusing to a user, who sees that their command line Git/SSH client works for some repository, but Cargo reports that no host key is found. This change adds support for the ``@revoked`` marker. This marker denotes that a key should be rejected outright. It is of limited use without ``@cert-authority`` marker support. However, if it is present in a user's `known_hosts` file, then Cargo definitely shouldn't accept that key and probably shouldn't suggest that the user add it to their `known_hosts` either. The change also adds support for detecting ``@cert-authority`` markers in `known_hosts` files. These lines cannot yet be used for host key verification, but if one is found for a matching host, the user will be informed that Cargo doesn't support ``@cert-authority`` markers in the error message. Additionally, the user will be advised to use the `net.git-fetch-with-cli` config option to use the command line git client for fetching crates from Git. Refs: #11577 ### How should we test and review this PR? The changes in this PR are covered by unit tests, all within `src/cargo/sources/git/known_hosts.rs`. Additionally, manual testing can be performed. For this you will need an OpenSSH server (it doesn't need to be a Git server). I'll assume that you have one running on your local machine at `127.0.0.1`. #### Setup 1. Create a new Cargo project and add the following line to `[dependencies]`: ```toml fake-crate = { git = "ssh://127.0.0.1/fake-crate.git" } ``` #### Test missing host key: `HostKeyNotFound` (existing functionality) 1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`. 2. Verify host key not present: `ssh 127.0.0.1`. SSH should tell you `The authenticity of host '127.0.0.1 (127.0.0.1)' can't be established.` 3. Run `cargo build` 4. Expect error from Cargo: `error: unknown SSH host key` #### Test ``@revoked`` key: `HostKeyRevoked` 1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`. 2. Add host key: `ssh 127.0.0.1` answer `yes` 3. Find all lines in `known_hosts` beginning with `127.0.0.1` (there may be multiple). 4. Add ``@revoked` ` to the beginning of all lines in (3) 5. Run `cargo build` 6. Expect error from Cargo: error: Key has been revoked for `127.0.0.1` #### Test `@cert-authority`` (not being supported): `HostHasOnlyCertAuthority` 1. Back up your `known_hosts` file and then remove any lines for `127.0.0.1`. 2. Run `cargo build` 3. Expect error from Cargo: `error: unknown SSH host key` 4. Check the line after ` The key to add is:` in the error message and copy the key type (e.g. `ecdsa-sha2-nistp256`) 5. Add a line to `known_hosts`: ``@cert-authority` 127.0.0.1 AAAAB5Wm` (e.g. ``@cert-authority` 127.0.0.1 ecdsa-sha2-nistp256 AAAAB5Wm`) 7. Run `cargo build` 8. Expect error from Cargo: error: Found a ``@cert-authority`` marker for `127.0.0.1` ### Additional information Cargo doesn't currently support a few things when checking host keys. This may affect the testing described above. * Multiple host key types (OpenSSH negotiates the host key type and can support matching the one present in the `known_hosts` file even when it's not the preferred type of the server). * Wildcard matching of host patterns (there's a FIXME for this) More information about SSH known host markers can be found on #11577. --- src/cargo/sources/git/known_hosts.rs | 364 +++++++++++++++++++++++---- 1 file changed, 312 insertions(+), 52 deletions(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 1294471a9..753868663 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. @@ -62,6 +62,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 +94,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 +161,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 +245,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,6 +343,7 @@ fn check_ssh_known_hosts( patterns: patterns.to_string(), key_type: key_type.to_string(), key, + line_type: KnownHostLineType::Key, }); } } @@ -298,12 +357,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 +391,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 +530,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 +545,7 @@ struct KnownHost { patterns: String, key_type: String, key: Vec, + line_type: KnownHostLineType, } impl KnownHost { @@ -488,15 +604,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 +649,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 +664,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 +685,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 +699,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 +760,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."), + } + } } From 0e4c31d59008d24a9f416c4cfcc24ba4e5a6158e Mon Sep 17 00:00:00 2001 From: bors Date: Sun, 26 Mar 2023 17:26:22 +0000 Subject: [PATCH 5/6] Auto merge of #11889 - est31:revoke_old_github, r=ehuss Add the old github keys as revoked The patch to update the bundled ssh github host key did not change anything for users who already had connected to github one time before via ssh: if the attacker had access to the old key, they'd be vulnerable to MITM attacks as their known_hosts file would list the old github key. Only if they connected again to github without attacker access, or if they saw the announcement of the key rotation, they would update their key. There is sadly no other way to distribute revocations of old host keys to clients other than to bundle them with client software. cc #11883 --- src/cargo/sources/git/known_hosts.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 753868663..54a4c9434 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -43,6 +43,20 @@ static BUNDLED_KEYS: &[(&str, &str, &str)] = &[ ("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=="), +]; + enum KnownHostError { /// Some general error happened while validating the known hosts. CheckError(anyhow::Error), @@ -347,6 +361,16 @@ fn check_ssh_known_hosts( }); } } + for (patterns, key_type, key) in BUNDLED_REVOCATIONS { + let key = STANDARD.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) } From 8c2456be5fe5a4e15e6d7221deac16dc2cdb911e Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 26 Mar 2023 12:02:07 -0700 Subject: [PATCH 6/6] Use old base64 API for bundled revocations. --- src/cargo/sources/git/known_hosts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 54a4c9434..8e1563a5e 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -362,7 +362,7 @@ fn check_ssh_known_hosts( } } for (patterns, key_type, key) in BUNDLED_REVOCATIONS { - let key = STANDARD.decode(key).unwrap(); + let key = base64::decode(key).unwrap(); known_hosts.push(KnownHost { location: KnownHostLocation::Bundled, patterns: patterns.to_string(),