Merge remote-tracking branch 'upstream/owner' into owner

* upstream/owner: (21 commits)
  Don't place side-effect expressions in assert! macros.
  Add `_MS` suffix to retry constants.
  Add some more docs and comments to `SleepTracker`.
  Add delays to network retries.
  Allow RegistryBuilder responder URLs to be a String
  Split the `cargo::util::network` module into submodules
  Add a note to `cargo logout` that it does not revoke the token.
  Sync external-tools JSON docs.
  Disable test_profile test on windows-gnu
  Drop derive feature from serde in cargo-platform
  a{n =>} benchmark target
  documented working directory behaviour of cargo-test, cargo-bench and cargo-run commands
  chore: Upgrade to clap v4.2
  docs(contrib): Link to office hours doc
  doc(contrib): missing quotation mark
  Update changelog for 1.68.2
  Add the old github keys as revoked
  Update proptest
  Added new GitHub RSA Host Key
  doc: Fix registries.name.index for sparse
  ...

CHANGELOG.md

@@ -69,8 +69,6 @@
 
 - Added `-C` flag for changing current dir before build starts.
   [#10952](https://github.com/rust-lang/cargo/pull/10952)
-- Added support for SSH known hosts marker `@revoked`.
-  [#11635](https://github.com/rust-lang/cargo/pull/11635)
 - Cargo now suggests `cargo fix` or `cargo clippy --fix`
   when compilation warnings/errors are auto-fixable.
   [#11558](https://github.com/rust-lang/cargo/pull/11558)
@@ -208,6 +206,20 @@
   [#11664](https://github.com/rust-lang/cargo/pull/11664)
   [#11679](https://github.com/rust-lang/cargo/pull/11679)
 
+## Cargo 1.68.2 (2023-03-28)
+[115f3455...rust-1.68.0](https://github.com/rust-lang/cargo/compare/115f3455...rust-1.68.0)
+
+- Updated the GitHub RSA SSH host key bundled within cargo.
+  The key was [rotated by
+  GitHub](https://github.blog/2023-03-23-we-updated-our-rsa-ssh-host-key/) on
+  2023-03-24 after the old one leaked.
+  [#11883](https://github.com/rust-lang/cargo/pull/11883)
+- Added support for SSH known hosts marker `@revoked`.
+  [#11635](https://github.com/rust-lang/cargo/pull/11635)
+- Marked the old GitHub RSA host key as revoked. This will prevent Cargo from
+  accepting the leaked key even when trusted by the system.
+  [#11889](https://github.com/rust-lang/cargo/pull/11889)
+
 ## Cargo 1.68 (2023-03-09)
 [f6e737b1...rust-1.68.0](https://github.com/rust-lang/cargo/compare/f6e737b1...rust-1.68.0)
 

Cargo.toml

@@ -21,7 +21,7 @@ base64 = "0.21.0"
 bytesize = "1.0"
 cargo-platform = { path = "crates/cargo-platform", version = "0.1.2" }
 cargo-util = { path = "crates/cargo-util", version = "0.2.4" }
-clap = "4.1.3"
+clap = "4.2.0"
 crates-io = { path = "crates/crates-io", version = "0.36.0" }
 curl = { version = "0.4.44", features = ["http2"] }
 curl-sys = "0.4.61"
@@ -58,6 +58,7 @@ os_info = "3.5.0"
 pasetors = { version = "0.6.4", features = ["v3", "paserk", "std", "serde"] }
 pathdiff = "0.2"
 pretty_env_logger = { version = "0.4", optional = true }
+rand = "0.8.5"
 rustfix = "0.6.0"
 semver = { version = "1.0.3", features = ["serde"] }
 serde = { version = "1.0.123", features = ["derive"] }

crates/cargo-platform/Cargo.toml

@@ -9,4 +9,4 @@ documentation = "https://docs.rs/cargo-platform"
 description = "Cargo's representation of a target platform."
 
 [dependencies]
-serde = { version = "1.0.82", features = ['derive'] }
+serde = "1.0.82"

crates/cargo-test-support/src/registry.rs

@@ -97,7 +97,7 @@ pub struct RegistryBuilder {
     /// Write the registry in configuration.
     configure_registry: bool,
     /// API responders.
-    custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
+    custom_responders: HashMap<String, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
     /// If nonzero, the git index update to be delayed by the given number of seconds.
     delayed_index_update: usize,
 }
@@ -167,10 +167,11 @@ impl RegistryBuilder {
     #[must_use]
     pub fn add_responder<R: 'static + Send + Fn(&Request, &HttpServer) -> Response>(
         mut self,
-        url: &'static str,
+        url: impl Into<String>,
         responder: R,
     ) -> Self {
-        self.custom_responders.insert(url, Box::new(responder));
+        self.custom_responders
+            .insert(url.into(), Box::new(responder));
         self
     }
 
@@ -601,7 +602,7 @@ pub struct HttpServer {
     addr: SocketAddr,
     token: Token,
     auth_required: bool,
-    custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
+    custom_responders: HashMap<String, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
     delayed_index_update: usize,
 }
 
@@ -621,10 +622,7 @@ impl HttpServer {
         api_path: PathBuf,
         token: Token,
         auth_required: bool,
-        api_responders: HashMap<
-            &'static str,
-            Box<dyn Send + Fn(&Request, &HttpServer) -> Response>,
-        >,
+        api_responders: HashMap<String, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
         delayed_index_update: usize,
     ) -> HttpServerHandle {
         let listener = TcpListener::bind("127.0.0.1:0").unwrap();

crates/resolver-tests/Cargo.toml

@@ -8,5 +8,5 @@ cargo = { path = "../.." }
 cargo-util = { path = "../cargo-util" }
 is-terminal = "0.4.0"
 lazy_static = "1.3.0"
-proptest = "0.9.1"
+proptest = "1.1.0"
 varisat = "0.2.1"

src/cargo/core/package.rs

@@ -28,7 +28,8 @@ use crate::ops;
 use crate::util::config::PackageCacheLock;
 use crate::util::errors::{CargoResult, HttpNotSuccessful};
 use crate::util::interning::InternedString;
-use crate::util::network::Retry;
+use crate::util::network::retry::{Retry, RetryResult};
+use crate::util::network::sleep::SleepTracker;
 use crate::util::{self, internal, Config, Progress, ProgressStyle};
 
 pub const MANIFEST_PREAMBLE: &str = "\
@@ -319,6 +320,8 @@ pub struct Downloads<'a, 'cfg> {
     /// Set of packages currently being downloaded. This should stay in sync
     /// with `pending`.
     pending_ids: HashSet<PackageId>,
+    /// Downloads that have failed and are waiting to retry again later.
+    sleeping: SleepTracker<(Download<'cfg>, Easy)>,
     /// The final result of each download. A pair `(token, result)`. This is a
     /// temporary holding area, needed because curl can report multiple
     /// downloads at once, but the main loop (`wait`) is written to only
@@ -442,6 +445,7 @@ impl<'cfg> PackageSet<'cfg> {
             next: 0,
             pending: HashMap::new(),
             pending_ids: HashSet::new(),
+            sleeping: SleepTracker::new(),
             results: Vec::new(),
             progress: RefCell::new(Some(Progress::with_style(
                 "Downloading",
@@ -800,7 +804,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
 
     /// Returns the number of crates that are still downloading.
     pub fn remaining(&self) -> usize {
-        self.pending.len()
+        self.pending.len() + self.sleeping.len()
     }
 
     /// Blocks the current thread waiting for a package to finish downloading.
@@ -831,51 +835,52 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
             let ret = {
                 let timed_out = &dl.timed_out;
                 let url = &dl.url;
-                dl.retry
-                    .r#try(|| {
-                        if let Err(e) = result {
-                            // If this error is "aborted by callback" then that's
-                            // probably because our progress callback aborted due to
-                            // a timeout. We'll find out by looking at the
-                            // `timed_out` field, looking for a descriptive message.
-                            // If one is found we switch the error code (to ensure
-                            // it's flagged as spurious) and then attach our extra
-                            // information to the error.
-                            if !e.is_aborted_by_callback() {
-                                return Err(e.into());
-                            }
+                dl.retry.r#try(|| {
+                    if let Err(e) = result {
+                        // If this error is "aborted by callback" then that's
+                        // probably because our progress callback aborted due to
+                        // a timeout. We'll find out by looking at the
+                        // `timed_out` field, looking for a descriptive message.
+                        // If one is found we switch the error code (to ensure
+                        // it's flagged as spurious) and then attach our extra
+                        // information to the error.
+                        if !e.is_aborted_by_callback() {
+                            return Err(e.into());
+                        }
 
-                            return Err(match timed_out.replace(None) {
-                                Some(msg) => {
-                                    let code = curl_sys::CURLE_OPERATION_TIMEDOUT;
-                                    let mut err = curl::Error::new(code);
-                                    err.set_extra(msg);
-                                    err
-                                }
-                                None => e,
+                        return Err(match timed_out.replace(None) {
+                            Some(msg) => {
+                                let code = curl_sys::CURLE_OPERATION_TIMEDOUT;
+                                let mut err = curl::Error::new(code);
+                                err.set_extra(msg);
+                                err
                             }
-                            .into());
+                            None => e,
                         }
+                        .into());
+                    }
 
-                        let code = handle.response_code()?;
-                        if code != 200 && code != 0 {
-                            let url = handle.effective_url()?.unwrap_or(url);
-                            return Err(HttpNotSuccessful {
-                                code,
-                                url: url.to_string(),
-                                body: data,
-                            }
-                            .into());
+                    let code = handle.response_code()?;
+                    if code != 200 && code != 0 {
+                        let url = handle.effective_url()?.unwrap_or(url);
+                        return Err(HttpNotSuccessful {
+                            code,
+                            url: url.to_string(),
+                            body: data,
                         }
-                        Ok(data)
-                    })
-                    .with_context(|| format!("failed to download from `{}`", dl.url))?
+                        .into());
+                    }
+                    Ok(data)
+                })
             };
             match ret {
-                Some(data) => break (dl, data),
-                None => {
-                    self.pending_ids.insert(dl.id);
-                    self.enqueue(dl, handle)?
+                RetryResult::Success(data) => break (dl, data),
+                RetryResult::Err(e) => {
+                    return Err(e.context(format!("failed to download from `{}`", dl.url)))
+                }
+                RetryResult::Retry(sleep) => {
+                    debug!("download retry {} for {sleep}ms", dl.url);
+                    self.sleeping.push(sleep, (dl, handle));
                 }
             }
         };
@@ -963,6 +968,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
         // actually block waiting for I/O to happen, which we achieve with the
         // `wait` method on `multi`.
         loop {
+            self.add_sleepers()?;
             let n = tls::set(self, || {
                 self.set
                     .multi
@@ -985,17 +991,31 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
             if let Some(pair) = results.pop() {
                 break Ok(pair);
             }
-            assert!(!self.pending.is_empty());
-            let min_timeout = Duration::new(1, 0);
-            let timeout = self.set.multi.get_timeout()?.unwrap_or(min_timeout);
-            let timeout = timeout.min(min_timeout);
-            self.set
-                .multi
-                .wait(&mut [], timeout)
-                .with_context(|| "failed to wait on curl `Multi`")?;
+            assert_ne!(self.remaining(), 0);
+            if self.pending.is_empty() {
+                let delay = self.sleeping.time_to_next().unwrap();
+                debug!("sleeping main thread for {delay:?}");
+                std::thread::sleep(delay);
+            } else {
+                let min_timeout = Duration::new(1, 0);
+                let timeout = self.set.multi.get_timeout()?.unwrap_or(min_timeout);
+                let timeout = timeout.min(min_timeout);
+                self.set
+                    .multi
+                    .wait(&mut [], timeout)
+                    .with_context(|| "failed to wait on curl `Multi`")?;
+            }
         }
     }
 
+    fn add_sleepers(&mut self) -> CargoResult<()> {
+        for (dl, handle) in self.sleeping.to_retry() {
+            self.pending_ids.insert(dl.id);
+            self.enqueue(dl, handle)?;
+        }
+        Ok(())
+    }
+
     fn progress(&self, token: usize, total: u64, cur: u64) -> bool {
         let dl = &self.pending[&token].0;
         dl.total.set(total);
@@ -1061,7 +1081,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
                 return Ok(());
             }
         }
-        let pending = self.pending.len();
+        let pending = self.remaining();
         let mut msg = if pending == 1 {
             format!("{} crate", pending)
         } else {

src/cargo/ops/cargo_run.rs

@@ -93,6 +93,11 @@ pub fn run(
     };
     let pkg = bins[0].0;
     let mut process = compile.target_process(exe, unit.kind, pkg, *script_meta)?;
+
+    // Sets the working directory of the child process to the current working
+    // directory of the parent process.
+    // Overrides the default working directory of the `ProcessBuilder` returned
+    // by `compile.target_process` (the package's root directory)
     process.args(args).cwd(config.cwd());
 
     config.shell().status("Running", process.to_string())?;

src/cargo/ops/registry.rs

@@ -956,6 +956,20 @@ pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> {
             reg_name
         ),
     )?;
+    let location = if source_ids.original.is_crates_io() {
+        "<https://crates.io/me>".to_string()
+    } else {
+        // The URL for the source requires network access to load the config.
+        // That could be a fairly heavy operation to perform just to provide a
+        // help message, so for now this just provides some generic text.
+        // Perhaps in the future this could have an API to fetch the config if
+        // it is cached, but avoid network access otherwise?
+        format!("the `{reg_name}` website")
+    };
+    config.shell().note(format!(
+        "This does not revoke the token on the registry server.\n    \
+        If you need to revoke the token, visit {location} and follow the instructions there."
+    ))?;
     Ok(())
 }
 

src/cargo/sources/git/known_hosts.rs

@@ -43,6 +43,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=="),
 ];
 
@@ -357,6 +371,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)
 }
 

src/cargo/sources/git/oxide.rs

@@ -29,7 +29,7 @@ pub fn with_retry_and_progress(
 ) -> CargoResult<()> {
     std::thread::scope(|s| {
         let mut progress_bar = Progress::new("Fetch", config);
-        network::with_retry(config, || {
+        network::retry::with_retry(config, || {
             let progress_root: Arc<gix::progress::tree::Root> =
                 gix::progress::tree::root::Options {
                     initial_capacity: 10,

src/cargo/sources/git/utils.rs

@@ -739,7 +739,7 @@ pub fn with_fetch_options(
     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, || {
+    network::retry::with_retry(config, || {
         with_authentication(config, url, git_config, |f| {
             let port = Url::parse(url).ok().and_then(|url| url.port());
             let mut last_update = Instant::now();