apply first round of CR

Author: dignifiedquire

src/dist/download.rs

@@ -137,8 +137,13 @@ impl<'a> DownloadCfg<'a> {
             })
         })?;
 
-        let content = utils::read_file("channel data", file).map(|s| s.to_owned())?;
-        if !crate::dist::signatures::verify_signature(&content, &signature)? {
+        let file_path: &Path = &file;
+        let content = std::fs::File::open(file_path).chain_err(|| ErrorKind::ReadingFile {
+            name: "channel data",
+            path: PathBuf::from(file_path),
+        })?;
+
+        if !crate::dist::signatures::verify_signature(content, &signature)? {
             Err(ErrorKind::SignatureVerificationFailed {
                 url: url.to_owned(),
             }
@@ -199,12 +204,11 @@ impl<'a> DownloadCfg<'a> {
         }
 
         // No signatures for tarballs for now.
-        if !url_str.ends_with(".tar.gz") && !url_str.ends_with(".tar.xz") {
-            if let Err(err) = self.check_signature(&url_str, &file) {
-                // TODO: this should probably be warn!, but log is not used in this part of the code
-                // at the moment.
-                println!("Invalid signature detected: {}", err);
-            }
+        if !url_str.ends_with(".tar.gz")
+            && !url_str.ends_with(".tar.xz")
+            && self.check_signature(&url_str, &file).is_err()
+        {
+            (self.notify_handler)(Notification::SignatureInvalid(url_str));
         }
 
         Ok(Some((file, partial_hash)))

src/dist/notifications.rs

@@ -35,6 +35,7 @@ pub enum Notification<'a> {
     ManifestChecksumFailedHack,
     ComponentUnavailable(&'a str, Option<&'a TargetTriple>),
     StrayHash(&'a Path),
+    SignatureInvalid(&'a str),
 }
 
 impl<'a> From<crate::utils::Notification<'a>> for Notification<'a> {
@@ -79,6 +80,7 @@ impl<'a> Notification<'a> {
             | ForcingUnavailableComponent(_)
             | StrayHash(_) => NotificationLevel::Warn,
             NonFatalError(_) => NotificationLevel::Error,
+            SignatureInvalid(_) => NotificationLevel::Warn,
         }
     }
 }
@@ -171,6 +173,7 @@ impl<'a> Display for Notification<'a> {
             ForcingUnavailableComponent(component) => {
                 write!(f, "Force-skipping unavailable component '{}'", component)
             }
+            SignatureInvalid(url) => write!(f, "Signature verification failed for '{}'", url),
         }
     }
 }

src/dist/rust-signing-key.asc

@@ -7,7 +7,9 @@ use pgp::{Deserializable, SignedPublicKey, StandaloneSignature};
 
 use crate::errors::*;
 
-// const SIGNING_KEY_BYTES: &[u8] = include_bytes!("rust-signing-key.asc");
+use std::io::Read;
+
+// const SIGNING_KEY_BYTES: &[u8] = include_bytes!("../rust-key.gpg.ascii");
 const SIGNING_KEY_BYTES: &[u8] = include_bytes!("../../tests/mock/signing-key.pub.asc");
 
 lazy_static::lazy_static! {
@@ -24,24 +26,23 @@ fn squish_internal_err<E: std::fmt::Display>(err: E) -> Error {
     ErrorKind::SignatureVerificationInternalError(format!("{}", err)).into()
 }
 
-pub fn verify_signature(content: &str, signature: &str) -> Result<bool> {
+pub fn verify_signature<T: Read>(mut content: T, signature: &str) -> Result<bool> {
+    let mut content_buf = Vec::new();
+    content.read_to_end(&mut content_buf)?;
     let (signatures, _) =
         StandaloneSignature::from_string_many(signature).map_err(squish_internal_err)?;
 
     for signature in signatures {
         let signature = signature.map_err(squish_internal_err)?;
 
         for key in &*SIGNING_KEYS {
-            if key.is_signing_key() {
-                if signature.verify(key, content.as_bytes()).is_ok() {
-                    return Ok(true);
-                }
+            if key.is_signing_key() && signature.verify(key, &content_buf).is_ok() {
+                return Ok(true);
             }
+
             for sub_key in &key.public_subkeys {
-                if sub_key.is_signing_key() {
-                    if signature.verify(sub_key, content.as_bytes()).is_ok() {
-                        return Ok(true);
-                    }
+                if sub_key.is_signing_key() && signature.verify(sub_key, &content_buf).is_ok() {
+                    return Ok(true);
                 }
             }
         }