Auto merge of #11891 - ehuss:stable-github-rsa, r=pietroalbini

[stable] Backport GitHub RSA key revocation

Backports for 1.68.2 patch release:

- Use the new GitHub key: #11883
- Bump the stable version number
- Fix semver check (to pass CI): #11817
- Support revocation markers: #11635
- Revoke the previous GitHub RSA key: #11889
This commit is contained in:
bors 2023-03-26 20:47:58 +00:00
commit 6feb7c9cfc
3 changed files with 338 additions and 54 deletions

View File

@ -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"

View File

@ -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<anyhow::Error> 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<KnownHostError> = 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<u8>,
line_type: KnownHostLineType,
}
impl KnownHost {
@ -488,15 +628,31 @@ fn load_hostfile_contents(path: &Path, contents: &str) -> Vec<KnownHost> {
fn parse_known_hosts_line(line: &str, location: KnownHostLocation) -> Option<KnownHost> {
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."),
}
}
}

View File

@ -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
}
```