Merge branch 'multiple-verifying-keys'

Author: dlon

Cargo.lock

@@ -122,6 +122,8 @@ ipnetwork = "0.20"
 tun = { version = "0.5.5", features = ["async"] }
 socket2 = "0.5.7"
 
+vec1 = "1.12"
+
 # Test dependencies
 proptest = "1.4"
 insta = { version = "1.42", features = ["yaml"] }

Cargo.toml

@@ -26,6 +26,7 @@ rand = { version = "0.8.5" }
 reqwest = { version = "0.12.9", default-features = false, features = ["rustls-tls"] }
 serde = { workspace = true, features = ["derive"] }
 tokio = { workspace = true, features = ["rt-multi-thread", "fs"] }
+vec1 = { workspace = true }
 
 talpid-platform-metadata = { path = "../talpid-platform-metadata" }
 mullvad-update = { path = "../mullvad-update", features = ["client"] }

installer-downloader/Cargo.toml

@@ -19,6 +19,7 @@ use tokio::{
     sync::{mpsc, oneshot},
     task::JoinHandle,
 };
+use vec1::vec1;
 
 /// ed25519 pubkey used to verify metadata from the Mullvad (stagemole) API
 const VERSION_PROVIDER_PUBKEY: &str = include_str!("../../mullvad-update/stagemole-pubkey");
@@ -60,7 +61,7 @@ pub fn initialize_controller<T: AppDelegate + 'static>(delegate: &mut T, environ
     let version_provider = HttpVersionInfoProvider {
         url: get_metadata_url(),
         pinned_certificate: Some(cert),
-        verifying_key,
+        verifying_keys: vec1![verifying_key],
     };
 
     AppController::initialize::<_, Downloader<T>, _, DirProvider>(

installer-downloader/src/controller.rs

@@ -13,7 +13,7 @@ workspace = true
 [features]
 default = []
 sign = ["rand", "clap"]
-client = ["async-trait", "reqwest", "sha2", "tokio", "thiserror"]
+client = ["async-trait", "reqwest", "sha2", "tokio", "thiserror", "vec1"]
 
 [dependencies]
 anyhow = { workspace = true }
@@ -28,6 +28,7 @@ async-trait = { version = "0.1", optional = true }
 reqwest = { version = "0.12.9", default-features = false, features = ["rustls-tls"], optional = true }
 sha2 = { version = "0.10", optional = true }
 tokio = { workspace = true, features = ["rt-multi-thread", "fs", "process", "macros"], optional = true }
+vec1 = { workspace = true, optional = true }
 
 mullvad-version = { path = "../mullvad-version", features = ["serde"] }
 

mullvad-update/Cargo.toml

@@ -22,6 +22,7 @@ serde = { workspace = true }
 sha2 = "0.10"
 tokio = { version = "1", features = ["full"] }
 toml = "0.8"
+vec1 = { workspace = true }
 
 mullvad-version = { path = "../../mullvad-version", features = ["serde"] }
 mullvad-update = { path = "../", features = ["client", "sign"] }

mullvad-update/meta/Cargo.toml

@@ -12,6 +12,7 @@ use std::{
     str::FromStr,
 };
 use tokio::{fs, io};
+use vec1::vec1;
 
 use crate::{
     artifacts,
@@ -128,7 +129,7 @@ impl Platform {
             // TODO: pin
             pinned_certificate: None,
             url,
-            verifying_key,
+            verifying_keys: vec1![verifying_key],
         };
         let response = version_provider
             .get_versions(crate::MIN_VERIFY_METADATA_VERSION)
@@ -229,12 +230,11 @@ impl Platform {
         println!("Verifying signature of {}...", signed_path.display());
         let bytes = fs::read(signed_path).await.context("Failed to read file")?;
 
-        // TODO: Actual key
-        let public_key =
-            key::VerifyingKey::from_hex(include_str!("../../test-pubkey")).expect("Invalid pubkey");
+        let public_key = key::VerifyingKey::from_hex(include_str!("../../stagemole-pubkey"))
+            .expect("Invalid pubkey");
 
         format::SignedResponse::deserialize_and_verify(
-            &public_key,
+            &vec1![public_key],
             &bytes,
             crate::MIN_VERIFY_METADATA_VERSION,
         )

mullvad-update/meta/src/platform.rs

@@ -1,6 +1,7 @@
 //! This module implements fetching of information about app versions
 
 use anyhow::Context;
+use vec1::Vec1;
 
 use crate::format;
 use crate::version::{VersionInfo, VersionParameters};
@@ -19,7 +20,7 @@ pub struct HttpVersionInfoProvider {
     /// Accepted root certificate. Defaults are used unless specified
     pub pinned_certificate: Option<reqwest::Certificate>,
     /// Key to use for verifying the response
-    pub verifying_key: format::key::VerifyingKey,
+    pub verifying_keys: Vec1<format::key::VerifyingKey>,
 }
 
 #[async_trait::async_trait]
@@ -41,7 +42,7 @@ impl HttpVersionInfoProvider {
     ) -> anyhow::Result<format::SignedResponse> {
         let raw_json = Self::get(&self.url, self.pinned_certificate.clone()).await?;
         let response = format::SignedResponse::deserialize_and_verify(
-            &self.verifying_key,
+            &self.verifying_keys,
             &raw_json,
             lowest_metadata_version,
         )?;
@@ -101,6 +102,7 @@ impl HttpVersionInfoProvider {
 #[cfg(test)]
 mod test {
     use insta::assert_yaml_snapshot;
+    use vec1::vec1;
 
     use crate::version::VersionArchitecture;
 
@@ -115,9 +117,10 @@ mod test {
     /// We're not testing the correctness of [version] here, only the HTTP client
     #[tokio::test]
     async fn test_http_version_provider() -> anyhow::Result<()> {
-        let verifying_key =
+        let valid_key =
             crate::format::key::VerifyingKey::from_hex(include_str!("../../test-pubkey"))
                 .expect("valid key");
+        let verifying_keys = vec1![valid_key];
 
         // Start HTTP server
         let mut server = mockito::Server::new_async().await;
@@ -138,7 +141,7 @@ mod test {
         let info_provider = HttpVersionInfoProvider {
             url,
             pinned_certificate: None,
-            verifying_key,
+            verifying_keys,
         };
 
         let info = info_provider

mullvad-update/src/client/api.rs

@@ -1,6 +1,7 @@
 //! Deserializer and verifier of version metadata
 
 use anyhow::Context;
+use vec1::Vec1;
 
 use super::key::*;
 use super::Response;
@@ -10,11 +11,11 @@ impl SignedResponse {
     /// Deserialize some bytes to JSON, and verify them, including signature and expiry.
     /// If successful, the deserialized data is returned.
     pub fn deserialize_and_verify(
-        key: &VerifyingKey,
+        keys: &Vec1<VerifyingKey>,
         bytes: &[u8],
         min_metadata_version: usize,
     ) -> Result<Self, anyhow::Error> {
-        Self::deserialize_and_verify_at_time(key, bytes, chrono::Utc::now(), min_metadata_version)
+        Self::deserialize_and_verify_at_time(keys, bytes, chrono::Utc::now(), min_metadata_version)
     }
 
     /// This method is used mostly for testing, and skips all verification.
@@ -33,13 +34,13 @@ impl SignedResponse {
     /// Deserialize some bytes to JSON, and verify them, including signature and expiry.
     /// If successful, the deserialized data is returned.
     fn deserialize_and_verify_at_time(
-        key: &VerifyingKey,
+        keys: &Vec1<VerifyingKey>,
         bytes: &[u8],
         current_time: chrono::DateTime<chrono::Utc>,
         min_metadata_version: usize,
     ) -> Result<Self, anyhow::Error> {
         // Deserialize and verify signature
-        let partial_data = deserialize_and_verify(key, bytes)?;
+        let partial_data = deserialize_and_verify(keys, bytes)?;
 
         // Deserialize the canonical JSON to structured representation
         let signed_response: Response = serde_json::from_value(partial_data.signed)
@@ -74,16 +75,20 @@ impl SignedResponse {
 ///
 /// On success, this returns verified data and signature
 pub(super) fn deserialize_and_verify(
-    key: &VerifyingKey,
+    keys: &Vec1<VerifyingKey>,
     bytes: &[u8],
 ) -> anyhow::Result<PartialSignedResponse> {
     let partial_data: PartialSignedResponse =
         serde_json::from_slice(bytes).context("Invalid version JSON")?;
 
-    // Check if the key matches
-    let Some(sig) = partial_data.signatures.iter().find_map(|sig| match sig {
+    let valid_keys: Vec<_> = keys.into_iter().map(|k| k.0).collect();
+
+    // Check if one of the keys matches
+    let Some((key, sig)) = partial_data.signatures.iter().find_map(|sig| match sig {
         // Check if ed25519 key matches
-        ResponseSignature::Ed25519 { keyid, sig } if keyid.0 == key.0 => Some(sig),
+        ResponseSignature::Ed25519 { keyid, sig } if valid_keys.contains(&keyid.0) => {
+            Some((keyid, sig))
+        }
         // Ignore all non-matching key
         _ => None,
     }) else {
@@ -109,6 +114,8 @@ pub(super) fn deserialize_and_verify(
 mod test {
     use std::str::FromStr;
 
+    use vec1::vec1;
+
     use super::*;
 
     /// Test that a valid signed version response is successfully deserialized and verified
@@ -119,7 +126,7 @@ mod test {
             ed25519_dalek::VerifyingKey::from_bytes(&pubkey.try_into().unwrap()).unwrap();
 
         SignedResponse::deserialize_and_verify_at_time(
-            &VerifyingKey(verifying_key),
+            &vec1![VerifyingKey(verifying_key)],
             include_bytes!("../../test-version-response.json"),
             // It's 1970 again
             chrono::DateTime::UNIX_EPOCH,
@@ -130,7 +137,7 @@ mod test {
 
         // Reject expired data
         SignedResponse::deserialize_and_verify_at_time(
-            &VerifyingKey(verifying_key),
+            &vec1![VerifyingKey(verifying_key)],
             include_bytes!("../../test-version-response.json"),
             // In the year 3000
             chrono::DateTime::from_str("3000-01-01T00:00:00Z").unwrap(),
@@ -141,7 +148,7 @@ mod test {
 
         // Reject expired version number
         SignedResponse::deserialize_and_verify_at_time(
-            &VerifyingKey(verifying_key),
+            &vec1![VerifyingKey(verifying_key)],
             include_bytes!("../../test-version-response.json"),
             chrono::DateTime::UNIX_EPOCH,
             usize::MAX,

mullvad-update/src/format/deserializer.rs

@@ -72,6 +72,7 @@ impl Serialize for SecretKey {
 
 /// ed25519 verifying key
 #[derive(Debug, PartialEq, Eq)]
+#[cfg_attr(test, derive(Clone))]
 pub struct VerifyingKey(pub ed25519_dalek::VerifyingKey);
 
 impl VerifyingKey {

mullvad-update/src/format/key.rs

@@ -34,6 +34,7 @@ pub struct SignedResponse {
 /// Helper type that leaves the signed data untouched
 /// Note that deserializing doesn't verify anything
 #[derive(Deserialize, Serialize)]
+#[cfg_attr(test, derive(Debug))]
 struct PartialSignedResponse {
     /// Signatures of the canonicalized JSON of `signed`
     pub signatures: Vec<ResponseSignature>,

mullvad-update/src/format/mod.rs

@@ -72,6 +72,7 @@ mod test {
     use super::*;
     use crate::format::deserializer::deserialize_and_verify;
     use serde_json::json;
+    use vec1::vec1;
 
     #[test]
     fn test_sign() -> anyhow::Result<()> {
@@ -95,7 +96,58 @@ mod test {
 
         let bytes = serde_json::to_vec(&partial)?;
 
-        deserialize_and_verify(&pubkey, &bytes)?;
+        deserialize_and_verify(&vec1![pubkey.clone()], &bytes)?;
+
+        // Verify that an irrelevant key is ignored
+        let invalid_key = key::SecretKey::generate();
+        let invalid_pubkey = invalid_key.pubkey();
+
+        deserialize_and_verify(&vec1![pubkey.clone(), invalid_pubkey.clone()], &bytes)?;
+
+        // Wrong public key only fails
+        deserialize_and_verify(&vec1![invalid_pubkey], &bytes).unwrap_err();
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_sign_multiple() -> anyhow::Result<()> {
+        // Generate keys and data
+        let key = key::SecretKey::generate();
+        let pubkey = key.pubkey();
+
+        let key2 = key::SecretKey::generate();
+        let pubkey2 = key2.pubkey();
+
+        let invalid_key = key::SecretKey::generate();
+        let invalid_pubkey = invalid_key.pubkey();
+
+        let data = json!({
+            "stuff": "I can prove that I wrote this"
+        });
+
+        // Sign with two keys
+        let mut partial = sign(&key, &data).context("Signing failed")?;
+        let partial2 = sign(&key2, &data).context("Signing failed")?;
+        partial.signatures.extend(partial2.signatures);
+
+        let bytes = serde_json::to_vec(&partial)?;
+
+        // Accept either (or both) keys
+        deserialize_and_verify(&vec1![pubkey.clone(), pubkey2.clone()], &bytes)?;
+        deserialize_and_verify(&vec1![pubkey2.clone()], &bytes)?;
+        deserialize_and_verify(&vec1![pubkey.clone()], &bytes)?;
+
+        // Ignore irrelevant key
+        deserialize_and_verify(
+            &vec1![pubkey.clone(), pubkey2.clone(), invalid_pubkey.clone()],
+            &bytes,
+        )?;
+        deserialize_and_verify(&vec1![pubkey2, invalid_pubkey.clone()], &bytes)?;
+        deserialize_and_verify(&vec1![invalid_pubkey.clone(), pubkey], &bytes)?;
+
+        // Using wrong public key fails
+        deserialize_and_verify(&vec1![invalid_pubkey], &bytes).unwrap_err();
 
         Ok(())
     }