Skip to content

Commit 26e08ab

Browse files
committed
Fix host file checking to not return success until all lines have been processed
Since a @Revoked line might deny access to a key which would otherwise be accepted, we need to process all lines before we decide that a host key should be accepted.
1 parent 605506a commit 26e08ab

File tree

1 file changed

+43
-1
lines changed

1 file changed

+43
-1
lines changed

src/cargo/sources/git/known_hosts.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,11 @@ fn check_ssh_known_hosts_loaded(
365365
// but a different hostname.
366366
let mut other_hosts = Vec::new();
367367

368+
// `accepted_known_host_found` keeps track of whether we've found a matching
369+
// line in the `known_hosts` file that we would accept. We can't return that
370+
// immediately, because there may be a subsequent @revoked key.
371+
let mut accepted_known_host_found = false;
372+
368373
// Older versions of OpenSSH (before 6.8, March 2015) showed MD5
369374
// fingerprints (see FingerprintHash ssh config option). Here we only
370375
// support SHA256.
@@ -389,7 +394,7 @@ fn check_ssh_known_hosts_loaded(
389394
match known_host.line_type {
390395
KnownHostLineType::Key => {
391396
if key_matches {
392-
return Ok(());
397+
accepted_known_host_found = true;
393398
}
394399

395400
// The host and key type matched, but the key itself did not.
@@ -424,6 +429,11 @@ fn check_ssh_known_hosts_loaded(
424429
}
425430
}
426431

432+
// We have an accepted host key and it hasn't been revoked.
433+
if accepted_known_host_found {
434+
return Ok(());
435+
}
436+
427437
if latent_errors.len() == 0 {
428438
// FIXME: Ideally the error message should include the IP address of the
429439
// remote host (to help the user validate that they are connecting to the
@@ -850,4 +860,36 @@ mod tests {
850860
_ => panic!("Expected error to be of type HostKeyHasChanged."),
851861
}
852862
}
863+
864+
#[test]
865+
fn known_host_and_revoked() {
866+
let contents = r#"
867+
example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host
868+
# Later in the file the same host key is revoked
869+
@revoked example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR eric@host
870+
"#;
871+
872+
let kh_path = Path::new("/home/abc/.known_hosts");
873+
let khs = load_hostfile_contents(kh_path, contents);
874+
875+
match check_ssh_known_hosts_loaded(
876+
&khs,
877+
"example.com",
878+
SshHostKeyType::Ed255219,
879+
&khs[0].key,
880+
) {
881+
Err(KnownHostError::HostKeyRevoked { hostname, remote_host_key, location, .. }) => {
882+
assert_eq!("example.com", hostname);
883+
assert_eq!(
884+
"AAAAC3NzaC1lZDI1NTE5AAAAIKVYJpa0yUGaNk0NXQTPWa0tHjqRpx+7hl2diReH6DtR",
885+
remote_host_key
886+
);
887+
assert!(matches!(
888+
location,
889+
KnownHostLocation::File { lineno: 4, .. }
890+
));
891+
},
892+
_ => panic!("Expected host key to be reject with error HostKeyRevoked."),
893+
}
894+
}
853895
}

0 commit comments

Comments
 (0)