Merge pull request #3277 from rbtcollins/gpgless

Remove GPG signature support

Author: rbtcollins

Cargo.lock

@@ -1,5 +1,8 @@
 [package]
-authors = ["Daniel Silverstone <dsilvers@digital-scurf.org>", "Diggory Blake <diggsey@googlemail.com>"]
+authors = [
+    "Daniel Silverstone <dsilvers@digital-scurf.org>",
+    "Diggory Blake <diggsey@googlemail.com>",
+]
 build = "build.rs"
 description = "Manage multiple rust installations with ease"
 edition = "2021"
@@ -13,7 +16,12 @@ version = "1.25.2"
 
 [features]
 curl-backend = ["download/curl-backend"]
-default = ["curl-backend", "reqwest-backend", "reqwest-default-tls", "reqwest-rustls-tls"]
+default = [
+    "curl-backend",
+    "reqwest-backend",
+    "reqwest-default-tls",
+    "reqwest-rustls-tls",
+]
 
 reqwest-backend = ["download/reqwest-backend"]
 vendored-openssl = ['openssl/vendored']
@@ -29,9 +37,9 @@ no-self-update = []
 anyhow.workspace = true
 cfg-if = "1.0"
 chrono = "0.4"
-clap = {version = "3", features = ["wrap_help"]}
+clap = { version = "3", features = ["wrap_help"] }
 clap_complete = "3"
-download = {path = "download", default-features = false}
+download = { path = "download", default-features = false }
 effective-limits = "0.5.5"
 enum-map = "2.4.2"
 flate2 = "1"
@@ -44,16 +52,15 @@ opener = "0.5.2"
 # Used by `curl` or `reqwest` backend although it isn't imported by our rustup :
 # this allows controlling the vendoring status without exposing the presence of
 # the download crate.
-openssl = {version = "0.10", optional = true}
-pulldown-cmark = {version = "0.9", default-features = false}
+openssl = { version = "0.10", optional = true }
+pulldown-cmark = { version = "0.9", default-features = false }
 rand = "0.8"
 regex = "1"
-remove_dir_all = {version= "0.8.1", features=["parallel"]}
+remove_dir_all = { version = "0.8.1", features = ["parallel"] }
 same-file = "1"
 scopeguard = "1"
 semver = "1.0"
-serde = {version = "1.0", features = ["derive"]}
-sequoia-openpgp = { version = "1.13", default-features = false, features = ["crypto-rust", "allow-experimental-crypto", "allow-variable-time-crypto"] }
+serde = { version = "1.0", features = ["derive"] }
 sha2 = "0.10"
 sharded-slab = "0.1.1"
 strsim = "0.10"
@@ -64,11 +71,11 @@ term = "=0.7.0"
 thiserror.workspace = true
 threadpool = "1"
 toml = "0.5"
+trycmd = "0.14.13"
 url.workspace = true
 wait-timeout = "0.2"
 xz2 = "0.1.3"
 zstd = "0.12"
-trycmd = "0.14.13"
 
 [dependencies.retry]
 default-features = false
@@ -85,34 +92,34 @@ winreg = "0.11"
 
 [target."cfg(windows)".dependencies.winapi]
 features = [
-  "combaseapi",
-  "errhandlingapi",
-  "fileapi",
-  "handleapi",
-  "ioapiset",
-  "jobapi",
-  "jobapi2",
-  "minwindef",
-  "processthreadsapi",
-  "psapi",
-  "shlobj",
-  "shtypes",
-  "synchapi",
-  "sysinfoapi",
-  "tlhelp32",
-  "userenv",
-  "winbase",
-  "winerror",
-  "winioctl",
-  "winnt",
-  "winuser",
+    "combaseapi",
+    "errhandlingapi",
+    "fileapi",
+    "handleapi",
+    "ioapiset",
+    "jobapi",
+    "jobapi2",
+    "minwindef",
+    "processthreadsapi",
+    "psapi",
+    "shlobj",
+    "shtypes",
+    "synchapi",
+    "sysinfoapi",
+    "tlhelp32",
+    "userenv",
+    "winbase",
+    "winerror",
+    "winioctl",
+    "winnt",
+    "winuser",
 ]
 version = "0.3"
 
 [dev-dependencies]
-walkdir = "2"
-once_cell = "1.17.1"
 enum-map = "2.4.2"
+once_cell = "1.17.1"
+walkdir = "2"
 
 [build-dependencies]
 lazy_static = "1"
@@ -139,4 +146,3 @@ lto = true
 # Reduce build time by setting proc-macro crates non optimized.
 [profile.release.build-override]
 opt-level = 0
-

Cargo.toml

@@ -135,7 +135,6 @@ pub fn main() -> Result<utils::ExitCode> {
                     ("active-toolchain", m) => handle_epipe(show_active_toolchain(cfg, m))?,
                     ("home", _) => handle_epipe(show_rustup_home(cfg))?,
                     ("profile", _) => handle_epipe(show_profile(cfg))?,
-                    ("keys", _) => handle_epipe(show_keys(cfg))?,
                     _ => handle_epipe(show(cfg, c))?,
                 },
                 None => handle_epipe(show(cfg, c))?,
@@ -1668,15 +1667,6 @@ fn show_profile(cfg: &Cfg) -> Result<utils::ExitCode> {
     Ok(utils::ExitCode(0))
 }
 
-fn show_keys(cfg: &Cfg) -> Result<utils::ExitCode> {
-    for key in cfg.get_pgp_keys() {
-        for l in key.show_key()? {
-            info!("{}", l);
-        }
-    }
-    Ok(utils::ExitCode(0))
-}
-
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub(crate) enum CompletionCommand {
     Rustup,

src/cli/rustup_mode.rs

@@ -7,13 +7,11 @@ use std::str::FromStr;
 use std::sync::Arc;
 
 use anyhow::{anyhow, bail, Context, Result};
-use sequoia_openpgp::{parse::Parse, Cert};
 use serde::Deserialize;
 use thiserror::Error as ThisError;
 
 use crate::cli::self_update::SelfUpdateMode;
 use crate::dist::download::DownloadCfg;
-use crate::dist::signatures::sequoia_policy;
 use crate::dist::{
     dist::{self, Profile},
     temp,
@@ -155,77 +153,6 @@ impl<'a> OverrideCfg<'a> {
     }
 }
 
-lazy_static::lazy_static! {
-    static ref BUILTIN_PGP_KEY: Cert =
-        Cert::from_bytes(&include_bytes!("rust-key.pgp.ascii")[..]).unwrap();
-}
-
-#[allow(clippy::large_enum_variant)] // Builtin is tiny, the rest are sane
-#[derive(Debug)]
-pub enum PgpPublicKey {
-    Builtin,
-    FromEnvironment(PathBuf, Cert),
-    FromConfiguration(PathBuf, Cert),
-}
-
-impl PgpPublicKey {
-    /// Retrieve the key.
-    pub(crate) fn cert(&self) -> &Cert {
-        match self {
-            Self::Builtin => &BUILTIN_PGP_KEY,
-            Self::FromEnvironment(_, k) => k,
-            Self::FromConfiguration(_, k) => k,
-        }
-    }
-
-    /// Display the key in detail for the user
-    pub(crate) fn show_key(&self) -> Result<Vec<String>> {
-        fn format_hex(bytes: &[u8], separator: &str, every: usize) -> Result<String> {
-            use std::fmt::Write;
-            let mut ret = String::new();
-            let mut wait = every;
-            for b in bytes.iter() {
-                if wait == 0 {
-                    ret.push_str(separator);
-                    wait = every;
-                }
-                wait -= 1;
-                write!(ret, "{b:02X}")?;
-            }
-            Ok(ret)
-        }
-        let mut ret = vec![format!("from {self}")];
-        let cert = self.cert();
-        let keyid = format_hex(cert.keyid().as_bytes(), "-", 4)?;
-        let algo = cert.primary_key().pk_algo();
-        let fpr = format_hex(cert.fingerprint().as_bytes(), " ", 2)?;
-        let p = sequoia_policy();
-
-        let uid0 = cert
-            .with_policy(&p, None)?
-            .primary_userid()
-            .map(|u| u.userid().to_string())
-            .unwrap_or_else(|_| "<No User ID>".into());
-        ret.push(format!("  {algo:?}/{keyid} - {uid0}"));
-        ret.push(format!("  Fingerprint: {fpr}"));
-        Ok(ret)
-    }
-}
-
-impl Display for PgpPublicKey {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::Builtin => write!(f, "builtin Rust release key"),
-            Self::FromEnvironment(p, _) => {
-                write!(f, "key specified in RUSTUP_PGP_KEY ({})", p.display())
-            }
-            Self::FromConfiguration(p, _) => {
-                write!(f, "key specified in configuration file ({})", p.display())
-            }
-        }
-    }
-}
-
 pub(crate) const UNIX_FALLBACK_SETTINGS: &str = "/etc/rustup/settings.toml";
 
 pub struct Cfg {
@@ -237,7 +164,6 @@ pub struct Cfg {
     pub update_hash_dir: PathBuf,
     pub download_dir: PathBuf,
     pub temp_cfg: temp::Cfg,
-    pgp_keys: Vec<PgpPublicKey>,
     pub toolchain_override: Option<String>,
     pub env_override: Option<String>,
     pub dist_root_url: String,
@@ -271,35 +197,6 @@ impl Cfg {
         let update_hash_dir = rustup_dir.join("update-hashes");
         let download_dir = rustup_dir.join("downloads");
 
-        // PGP keys
-        let mut pgp_keys: Vec<PgpPublicKey> = vec![PgpPublicKey::Builtin];
-
-        if let Some(ref s_path) = process().var_os("RUSTUP_PGP_KEY") {
-            let path = PathBuf::from(s_path);
-            let file = utils::open_file("RUSTUP_PGP_KEY", &path)?;
-            let key = Cert::from_reader(file).map_err(|error| RustupError::InvalidPgpKey {
-                path: s_path.into(),
-                source: error,
-            })?;
-
-            pgp_keys.push(PgpPublicKey::FromEnvironment(path, key));
-        }
-        settings_file.with(|s| {
-            if let Some(s) = &s.pgp_keys {
-                let path = PathBuf::from(s);
-                let file = utils::open_file("PGP Key from config", &path)?;
-                let key = Cert::from_reader(file).map_err(|error| {
-                    anyhow!(RustupError::InvalidPgpKey {
-                        path: s.into(),
-                        source: error,
-                    })
-                })?;
-
-                pgp_keys.push(PgpPublicKey::FromConfiguration(path, key));
-            }
-            Ok(())
-        })?;
-
         // Environment override
         let env_override = process()
             .var("RUSTUP_TOOLCHAIN")
@@ -338,7 +235,6 @@ impl Cfg {
             update_hash_dir,
             download_dir,
             temp_cfg,
-            pgp_keys,
             notify_handler,
             toolchain_override: None,
             env_override,
@@ -364,14 +260,9 @@ impl Cfg {
             temp_cfg: &self.temp_cfg,
             download_dir: &self.download_dir,
             notify_handler,
-            pgp_keys: self.get_pgp_keys(),
         }
     }
 
-    pub(crate) fn get_pgp_keys(&self) -> &[PgpPublicKey] {
-        &self.pgp_keys
-    }
-
     pub(crate) fn set_profile_override(&mut self, profile: dist::Profile) {
         self.profile_override = Some(profile);
     }

src/config.rs

@@ -997,7 +997,6 @@ fn try_update_from_dist_(
         update_hash,
         download.temp_cfg,
         &download.notify_handler,
-        download.pgp_keys,
     );
     // inspect, determine what context to add, then process afterwards.
     let mut download_not_exists = false;

src/dist/dist.rs

@@ -6,7 +6,6 @@ use anyhow::{anyhow, Context, Result};
 use sha2::{Digest, Sha256};
 use url::Url;
 
-use crate::config::PgpPublicKey;
 use crate::dist::notifications::*;
 use crate::dist::temp;
 use crate::errors::*;
@@ -20,7 +19,6 @@ pub struct DownloadCfg<'a> {
     pub temp_cfg: &'a temp::Cfg,
     pub download_dir: &'a PathBuf,
     pub notify_handler: &'a dyn Fn(Notification<'_>),
-    pub pgp_keys: &'a [PgpPublicKey],
 }
 
 pub(crate) struct File {
@@ -137,43 +135,6 @@ impl<'a> DownloadCfg<'a> {
         utils::read_file("hash", &hash_file).map(|s| s[0..64].to_owned())
     }
 
-    fn download_signature(&self, url: &str) -> Result<String> {
-        let sig_url = utils::parse_url(&(url.to_owned() + ".asc"))?;
-        let sig_file = self.temp_cfg.new_file()?;
-
-        utils::download_file(&sig_url, &sig_file, None, &|n| {
-            (self.notify_handler)(n.into())
-        })?;
-
-        utils::read_file("signature", &sig_file)
-    }
-
-    fn check_signature(&self, url: &str, file: &temp::File<'_>) -> Result<&PgpPublicKey> {
-        assert!(
-            !self.pgp_keys.is_empty(),
-            "At least the builtin key must be present"
-        );
-
-        let signature = self
-            .download_signature(url)
-            .with_context(|| format!("failed to download signature file {url}"))?;
-
-        let file_path: &Path = file;
-        let content = std::fs::File::open(file_path).with_context(|| RustupError::ReadingFile {
-            name: "channel data",
-            path: PathBuf::from(file_path),
-        })?;
-
-        let sig_result =
-            crate::dist::signatures::verify_signature(content, &signature, self.pgp_keys)?;
-        if let Some(keyidx) = sig_result {
-            let key = &self.pgp_keys[keyidx];
-            Ok(key)
-        } else {
-            Err(anyhow!(format!("signature verification failed for {url}")))
-        }
-    }
-
     /// Downloads a file, sourcing its hash from the same url with a `.sha256` suffix.
     /// If `update_hash` is present, then that will be compared to the downloaded hash,
     /// and if they match, the download is skipped.
@@ -224,14 +185,6 @@ impl<'a> DownloadCfg<'a> {
             (self.notify_handler)(Notification::ChecksumValid(url_str));
         }
 
-        // No signatures for tarballs for now.
-        if !url_str.ends_with(".tar.gz") && !url_str.ends_with(".tar.xz") {
-            match self.check_signature(url_str, &file) {
-                Ok(key) => (self.notify_handler)(Notification::SignatureValid(url_str, key)),
-                Err(_) => (self.notify_handler)(Notification::SignatureInvalid(url_str)),
-            }
-        }
-
         Ok(Some((file, partial_hash)))
     }
 }