use new pgp configuration

Author: dignifiedquire

src/config.rs

@@ -7,6 +7,8 @@ use std::process::Command;
 use std::str::FromStr;
 use std::sync::Arc;
 
+use pgp::{Deserializable, SignedPublicKey};
+
 use crate::dist::{dist, temp};
 use crate::errors::*;
 use crate::notifications::*;
@@ -33,21 +35,24 @@ impl Display for OverrideReason {
     }
 }
 
+lazy_static::lazy_static! {
+    static ref BUILTIN_PGP_KEY: SignedPublicKey = pgp::SignedPublicKey::from_armor_single(
+        io::Cursor::new(&include_bytes!("rust-key.pgp.ascii")[..])
+    ).unwrap().0;
+}
+
 #[derive(Debug)]
 pub enum PgpPublicKey {
-    Builtin(&'static [u8]),
-    FromEnvironment(PathBuf, Vec<u8>),
-    FromConfiguration(PathBuf, Vec<u8>),
+    Builtin,
+    FromEnvironment(PathBuf, SignedPublicKey),
+    FromConfiguration(PathBuf, SignedPublicKey),
 }
 
 impl PgpPublicKey {
-    /// Retrieve the key data for this key
-    ///
-    /// This key might be ASCII Armored or may not, we make no
-    /// guarantees.
-    pub fn key_data(&self) -> &[u8] {
+    /// Retrieve the key.
+    pub fn key(&self) -> &SignedPublicKey {
         match self {
-            Self::Builtin(k) => k,
+            Self::Builtin => &*BUILTIN_PGP_KEY,
             Self::FromEnvironment(_, k) => &k,
             Self::FromConfiguration(_, k) => &k,
         }
@@ -57,7 +62,7 @@ impl PgpPublicKey {
 impl Display for PgpPublicKey {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Self::Builtin(_) => write!(f, "builtin Rust release key"),
+            Self::Builtin => write!(f, "builtin Rust release key"),
             Self::FromEnvironment(p, _) => {
                 write!(f, "key specified in RUST_PGP_KEY ({})", p.display())
             }
@@ -98,18 +103,24 @@ impl Cfg {
         let download_dir = rustup_dir.join("downloads");
 
         // PGP keys
-        let mut pgp_keys: Vec<PgpPublicKey> =
-            vec![PgpPublicKey::Builtin(include_bytes!("rust-key.pgp.ascii"))];
-        if let Some(s_path) = env::var_os("RUSTUP_PGP_KEY") {
+        let mut pgp_keys: Vec<PgpPublicKey> = vec![PgpPublicKey::Builtin];
+
+        if let Some(ref s_path) = env::var_os("RUSTUP_PGP_KEY") {
             let path = PathBuf::from(s_path);
-            let content = utils::read_file_bytes("RUSTUP_PGP_KEY", &path)?;
-            pgp_keys.push(PgpPublicKey::FromEnvironment(path, content));
+            let file = utils::open_file("RUSTUP_PGP_KEY", &path)?;
+            let (key, _) = SignedPublicKey::from_armor_single(file)
+                .map_err(|error| ErrorKind::InvalidPgpKey(PathBuf::from(s_path), 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 content = utils::read_file_bytes("PGP Key from config", &path)?;
-                pgp_keys.push(PgpPublicKey::FromConfiguration(path, content));
+                let file = utils::open_file("PGP Key from config", &path)?;
+                let (key, _) = SignedPublicKey::from_armor_single(file)
+                    .map_err(|error| ErrorKind::InvalidPgpKey(PathBuf::from(s), error))?;
+
+                pgp_keys.push(PgpPublicKey::FromConfiguration(path, key));
             }
             Ok(())
         })?;

src/dist/dist.rs

@@ -844,6 +844,7 @@ fn try_update_from_dist_<'a>(
         update_hash,
         &download.temp_cfg,
         &download.notify_handler,
+        &download.pgp_keys,
     ) {
         Ok(None) => Ok(None),
         Ok(Some(hash)) => Ok(Some(hash)),

src/dist/download.rs

@@ -1,7 +1,9 @@
+use crate::config::PgpPublicKey;
 use crate::dist::notifications::*;
 use crate::dist::temp;
 use crate::errors::*;
 use crate::utils::utils;
+
 use sha2::{Digest, Sha256};
 use url::Url;
 
@@ -17,6 +19,7 @@ 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 struct File {
@@ -131,6 +134,11 @@ impl<'a> DownloadCfg<'a> {
     }
 
     fn check_signature(&self, url: &str, file: &temp::File<'_>) -> Result<()> {
+        assert!(
+            !self.pgp_keys.is_empty(),
+            "At least the builtin key must be present"
+        );
+
         let signature = self.download_signature(url).map_err(|e| {
             e.chain_err(|| ErrorKind::SignatureVerificationFailed {
                 url: url.to_owned(),
@@ -143,7 +151,7 @@ impl<'a> DownloadCfg<'a> {
             path: PathBuf::from(file_path),
         })?;
 
-        if !crate::dist::signatures::verify_signature(content, &signature)? {
+        if !crate::dist::signatures::verify_signature(content, &signature, &self.pgp_keys)? {
             Err(ErrorKind::SignatureVerificationFailed {
                 url: url.to_owned(),
             }

src/dist/manifestation.rs

@@ -1,6 +1,7 @@
 //! Maintains a Rust installation by installing individual Rust
 //! platform components from a distribution server.
 
+use crate::config::PgpPublicKey;
 use crate::dist::component::{Components, Package, TarGzPackage, TarXzPackage, Transaction};
 use crate::dist::config::Config;
 use crate::dist::dist::{Profile, TargetTriple, DEFAULT_DIST_SERVER};
@@ -351,6 +352,7 @@ impl Manifestation {
         update_hash: Option<&Path>,
         temp_cfg: &temp::Cfg,
         notify_handler: &dyn Fn(Notification<'_>),
+        pgp_keys: &[PgpPublicKey],
     ) -> Result<Option<String>> {
         // If there's already a v2 installation then something has gone wrong
         if self.read_config()?.is_some() {
@@ -388,6 +390,7 @@ impl Manifestation {
             download_dir: &dld_dir,
             temp_cfg,
             notify_handler,
+            pgp_keys,
         };
 
         let dl = dlcfg.download_and_check(&url, update_hash, ".tar.gz")?;

src/dist/signatures.rs

@@ -3,30 +3,22 @@
 // TODO: Determine whether we want external keyring support
 
 use pgp::types::KeyTrait;
-use pgp::{Deserializable, SignedPublicKey, StandaloneSignature};
+use pgp::{Deserializable, StandaloneSignature};
 
+use crate::config::PgpPublicKey;
 use crate::errors::*;
 
 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! {
-    static ref SIGNING_KEYS: Vec<SignedPublicKey> = {
-        pgp::SignedPublicKey::from_armor_many(std::io::Cursor::new(SIGNING_KEY_BYTES))
-            .map_err(squish_internal_err).unwrap()
-            .0
-            .collect::<std::result::Result<Vec<_>, _>>()
-            .map_err(squish_internal_err).unwrap()
-    };
-}
-
 fn squish_internal_err<E: std::fmt::Display>(err: E) -> Error {
     ErrorKind::SignatureVerificationInternalError(format!("{}", err)).into()
 }
 
-pub fn verify_signature<T: Read>(mut content: T, signature: &str) -> Result<bool> {
+pub fn verify_signature<T: Read>(
+    mut content: T,
+    signature: &str,
+    keys: &[PgpPublicKey],
+) -> Result<bool> {
     let mut content_buf = Vec::new();
     content.read_to_end(&mut content_buf)?;
     let (signatures, _) =
@@ -35,12 +27,13 @@ pub fn verify_signature<T: Read>(mut content: T, signature: &str) -> Result<bool
     for signature in signatures {
         let signature = signature.map_err(squish_internal_err)?;
 
-        for key in &*SIGNING_KEYS {
-            if key.is_signing_key() && signature.verify(key, &content_buf).is_ok() {
+        for key in keys {
+            let actual_key = key.key();
+            if actual_key.is_signing_key() && signature.verify(actual_key, &content_buf).is_ok() {
                 return Ok(true);
             }
 
-            for sub_key in &key.public_subkeys {
+            for sub_key in &actual_key.public_subkeys {
                 if sub_key.is_signing_key() && signature.verify(sub_key, &content_buf).is_ok() {
                     return Ok(true);
                 }

src/errors.rs

@@ -358,6 +358,10 @@ error_chain! {
             description("bad path in tar")
             display("tar path '{}' is not supported", v.display())
         }
+        InvalidPgpKey(v: PathBuf, error: pgp::errors::Error) {
+            description("invalid PGP key"),
+            display("unable to read the PGP key '{}'", v.display())
+        }
     }
 }
 

src/toolchain.rs

@@ -159,6 +159,7 @@ impl<'a> Toolchain<'a> {
             temp_cfg: &self.cfg.temp_cfg,
             download_dir: &self.cfg.download_dir,
             notify_handler: &*self.dist_handler,
+            pgp_keys: self.cfg.get_pgp_keys(),
         }
     }
 

src/utils/utils.rs

@@ -37,6 +37,13 @@ where
     })
 }
 
+pub fn open_file(name: &'static str, path: &Path) -> Result<fs::File> {
+    fs::File::open(path).chain_err(|| ErrorKind::ReadingFile {
+        name,
+        path: PathBuf::from(path),
+    })
+}
+
 pub fn read_file_bytes(name: &'static str, path: &Path) -> Result<Vec<u8>> {
     fs::read(path).chain_err(|| ErrorKind::ReadingFile {
         name,

tests/dist.rs

@@ -16,6 +16,7 @@ use rustup::errors::Result;
 use rustup::utils::raw as utils_raw;
 use rustup::utils::utils;
 use rustup::ErrorKind;
+use rustup::PgpPublicKey;
 use std::cell::Cell;
 use std::collections::HashMap;
 use std::fs;
@@ -523,6 +524,10 @@ fn setup_from_dist_server(
         temp_cfg: &temp_cfg,
         download_dir: &prefix.path().to_owned().join("downloads"),
         notify_handler: &|_| {},
+        pgp_keys: &[PgpPublicKey::FromEnvironment(
+            "test-key".into(),
+            get_public_key(),
+        )],
     };
 
     f(url, &toolchain, &prefix, &download_cfg, &temp_cfg);
@@ -1887,6 +1892,10 @@ fn reuse_downloaded_file() {
                     reuse_notification_fired.set(true);
                 }
             },
+            pgp_keys: &[PgpPublicKey::FromEnvironment(
+                "test-key".into(),
+                get_public_key(),
+            )],
         };
 
         update_from_dist(
@@ -1954,6 +1963,10 @@ fn checks_files_hashes_before_reuse() {
                     noticed_bad_checksum.set(true);
                 }
             },
+            pgp_keys: &[PgpPublicKey::FromEnvironment(
+                "test-key".into(),
+                get_public_key(),
+            )],
         };
 
         update_from_dist(

tests/mock/clitools.rs

@@ -389,6 +389,14 @@ pub fn env(config: &Config, cmd: &mut Command) {
     // The test environment may interfere with checking the PATH for the existence of rustc or
     // cargo, so we disable that check globally
     cmd.env("RUSTUP_INIT_SKIP_PATH_CHECK", "yes");
+
+    // Setup pgp test key
+    cmd.env(
+        "RUSTUP_PGP_KEY",
+        std::env::current_dir()
+            .unwrap()
+            .join("tests/mock/signing-key.pub.asc"),
+    );
 }
 
 use std::sync::RwLock;

tests/mock/dist.rs

@@ -511,16 +511,25 @@ pub fn write_file(dst: &Path, contents: &str) {
 }
 
 const SIGNING_KEY_BYTES: &[u8] = include_bytes!("signing-key.asc");
+const PUB_SIGNING_KEY_BYTES: &[u8] = include_bytes!("signing-key.pub.asc");
 
-fn load_key() -> std::result::Result<pgp::SignedSecretKey, pgp::errors::Error> {
+fn get_secret_key() -> pgp::SignedSecretKey {
     use pgp::Deserializable;
     let (key, _) =
-        pgp::SignedSecretKey::from_armor_single(std::io::Cursor::new(SIGNING_KEY_BYTES))?;
-    Ok(key)
+        pgp::SignedSecretKey::from_armor_single(std::io::Cursor::new(SIGNING_KEY_BYTES)).unwrap();
+    key
+}
+
+pub fn get_public_key() -> pgp::SignedPublicKey {
+    use pgp::Deserializable;
+    let (key, _) =
+        pgp::SignedPublicKey::from_armor_single(std::io::Cursor::new(PUB_SIGNING_KEY_BYTES))
+            .unwrap();
+    key
 }
 
 pub fn create_signature(data: &[u8]) -> std::result::Result<String, pgp::errors::Error> {
-    let key = load_key()?;
+    let key = get_secret_key();
 
     let msg = pgp::Message::new_literal_bytes("message", data);
     let signed_message = msg.sign(&key, || "".into(), pgp::crypto::HashAlgorithm::SHA2_256)?;