Check quickinstall signatures (#1382)

* Check quickinstall signatures

* e2e test

* We can no longer check presence of signing section early

* Improve UI a little

crates/binstalk-fetchers/src/gh_crate_meta.rs

@@ -253,7 +253,6 @@ impl super::Fetcher for GhCrateMeta {
                 SignatureVerifier::Noop
             }
             (SignaturePolicy::Require, None) => {
-                debug_assert!(false, "missing signing section should be caught earlier");
                 return Err(FetchError::MissingSignature);
             }
             (_, Some(config)) => {

crates/binstalk-fetchers/src/quickinstall.rs

@@ -1,15 +1,21 @@
-use std::{path::Path, sync::Arc};
+use std::{borrow::Cow, path::Path, sync::Arc};
 
 use binstalk_downloader::remote::Method;
-use binstalk_types::cargo_toml_binstall::{PkgFmt, PkgMeta};
+use binstalk_types::cargo_toml_binstall::{PkgFmt, PkgMeta, PkgSigning};
 use tokio::sync::OnceCell;
+use tracing::{error, info, trace};
 use url::Url;
 
-use crate::{common::*, Data, FetchError, SignaturePolicy, TargetDataErased};
+use crate::{
+    common::*, Data, FetchError, SignaturePolicy, SignatureVerifier, SigningAlgorithm,
+    TargetDataErased,
+};
 
 const BASE_URL: &str = "https://github.com/cargo-bins/cargo-quickinstall/releases/download";
 const STATS_URL: &str = "https://warehouse-clerk-tmp.vercel.app/api/crate";
 
+const QUICKINSTALL_SIGN_KEY: Cow<'static, str> =
+    Cow::Borrowed("RWTdnnab2pAka9OdwgCMYyOE66M/BlQoFWaJ/JjwcPV+f3n24IRTj97t");
 const QUICKINSTALL_SUPPORTED_TARGETS_URL: &str =
     "https://raw.githubusercontent.com/cargo-bins/cargo-quickinstall/main/supported-targets";
 
@@ -50,6 +56,7 @@ pub struct QuickInstall {
 
     package: String,
     package_url: Url,
+    signature_url: Url,
     stats_url: Url,
     signature_policy: SignaturePolicy,
 
@@ -85,15 +92,17 @@ impl super::Fetcher for QuickInstall {
 
         let package = format!("{crate_name}-{version}-{target}");
 
+        let url = format!("{BASE_URL}/{crate_name}-{version}/{package}.tar.gz");
+
         Arc::new(Self {
             client,
             gh_api_client,
             is_supported_v: OnceCell::new(),
 
-            package_url: Url::parse(&format!(
+            package_url: Url::parse(&url)
-                "{BASE_URL}/{crate_name}-{version}/{package}.tar.gz",
+                .expect("package_url is pre-generated and should never be invalid url"),
-            ))
+            signature_url: Url::parse(&format!("{url}.sig"))
-            .expect("package_url is pre-generated and should never be invalid url"),
+                .expect("signature_url is pre-generated and should never be invalid url"),
             stats_url: Url::parse(&format!("{STATS_URL}/{package}.tar.gz",))
                 .expect("stats_url is pre-generated and should never be invalid url"),
             package,
@@ -105,15 +114,20 @@ impl super::Fetcher for QuickInstall {
 
     fn find(self: Arc<Self>) -> JoinHandle<Result<bool, FetchError>> {
         tokio::spawn(async move {
-            // until quickinstall supports signatures, blanket deny:
-            if self.signature_policy == SignaturePolicy::Require {
-                return Err(FetchError::MissingSignature);
-            }
-
             if !self.is_supported().await? {
                 return Ok(false);
             }
 
+            if self.signature_policy == SignaturePolicy::Require {
+                does_url_exist(
+                    self.client.clone(),
+                    self.gh_api_client.clone(),
+                    &self.signature_url,
+                )
+                .await
+                .map_err(|_| FetchError::MissingSignature)?;
+            }
+
             does_url_exist(
                 self.client.clone(),
                 self.gh_api_client.clone(),
@@ -146,11 +160,53 @@ by rust officially."#,
     }
 
     async fn fetch_and_extract(&self, dst: &Path) -> Result<ExtractedFiles, FetchError> {
-        let url = &self.package_url;
+        let verifier = if self.signature_policy == SignaturePolicy::Ignore {
-        debug!("Downloading package from: '{url}'");
+            SignatureVerifier::Noop
-        Ok(Download::new(self.client.clone(), url.clone())
+        } else {
-            .and_extract(self.pkg_fmt(), dst)
+            debug!(url=%self.signature_url, "Downloading signature");
-            .await?)
+            match Download::new(self.client.clone(), self.signature_url.clone())
+                .into_bytes()
+                .await
+            {
+                Ok(signature) => {
+                    trace!(?signature, "got signature contents");
+                    let config = PkgSigning {
+                        algorithm: SigningAlgorithm::Minisign,
+                        pubkey: QUICKINSTALL_SIGN_KEY,
+                        file: None,
+                    };
+                    SignatureVerifier::new(&config, &signature)?
+                }
+                Err(err) => {
+                    if self.signature_policy == SignaturePolicy::Require {
+                        error!("Failed to download signature: {err}");
+                        return Err(FetchError::MissingSignature);
+                    }
+
+                    debug!("Failed to download signature, skipping verification: {err}");
+                    SignatureVerifier::Noop
+                }
+            }
+        };
+
+        debug!(url=%self.package_url, "Downloading package");
+        let mut data_verifier = verifier.data_verifier()?;
+        let files = Download::new_with_data_verifier(
+            self.client.clone(),
+            self.package_url.clone(),
+            data_verifier.as_mut(),
+        )
+        .and_extract(self.pkg_fmt(), dst)
+        .await?;
+        trace!("validating signature (if any)");
+        if data_verifier.validate() {
+            if let Some(info) = verifier.info() {
+                info!("Verified signature for package '{}': {info}", self.package);
+            }
+            Ok(files)
+        } else {
+            Err(FetchError::InvalidSignature)
+        }
     }
 
     fn pkg_fmt(&self) -> PkgFmt {

crates/binstalk-types/src/cargo_toml_binstall.rs

@@ -2,7 +2,7 @@
 //!
 //! This manifest defines how a particular binary crate may be installed by Binstall.
 
-use std::collections::BTreeMap;
+use std::{borrow::Cow, collections::BTreeMap};
 
 use serde::{Deserialize, Serialize};
 
@@ -127,7 +127,7 @@ pub struct PkgSigning {
     pub algorithm: SigningAlgorithm,
 
     /// Signing public key
-    pub pubkey: String,
+    pub pubkey: Cow<'static, str>,
 
     /// Signature file override template (url to download)
     #[serde(default)]

crates/binstalk/src/ops/resolve.rs

@@ -19,7 +19,7 @@ use tracing::{debug, error, info, instrument, warn};
 use crate::{
     bins,
     errors::{BinstallError, VersionParseError},
-    fetchers::{Data, Fetcher, SignaturePolicy, TargetData},
+    fetchers::{Data, Fetcher, TargetData},
     helpers::{
         self, cargo_toml::Manifest, cargo_toml_workspace::load_manifest_from_workspace,
         download::ExtractedFiles, remote::Client, target_triple::TargetTriple,
@@ -83,10 +83,6 @@ async fn resolve_inner(
         return Ok(Resolution::AlreadyUpToDate);
     };
 
-    if opts.signature_policy == SignaturePolicy::Require && !package_info.signing {
-        return Err(BinstallError::MissingSignature(package_info.name));
-    }
-
     let desired_targets = opts
         .desired_targets
         .get()
@@ -337,7 +333,6 @@ struct PackageInfo {
     version: Version,
     repo: Option<String>,
     overrides: BTreeMap<String, PkgOverride>,
-    signing: bool,
 }
 
 struct Bin {
@@ -446,7 +441,6 @@ impl PackageInfo {
         } else {
             Ok(Some(Self {
                 overrides: mem::take(&mut meta.overrides),
-                signing: meta.signing.is_some(),
                 meta,
                 binaries,
                 name,

e2e-tests/signing.sh

@@ -29,5 +29,7 @@ signing/wait-for-server.sh
 "./$1" binstall --force --manifest-path manifests/signing-Cargo.toml --no-confirm --only-signed signing-test
 "./$1" binstall --force --manifest-path manifests/signing-Cargo.toml --no-confirm --skip-signatures signing-test
 
+# from quick-install
+"./$1" binstall --force --strategies quick-install --no-confirm --only-signed --target x86_64-unknown-linux-musl zellij@0.38.2
 
 kill $server_pid || true