Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for verifying version metadata with (one of) multiple keys #7767

Merged
merged 4 commits into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
1 change: 1 addition & 0 deletions installer-downloader/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
3 changes: 2 additions & 1 deletion installer-downloader/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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>(
Expand Down
3 changes: 2 additions & 1 deletion mullvad-update/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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"] }

Expand Down
1 change: 1 addition & 0 deletions mullvad-update/meta/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
10 changes: 5 additions & 5 deletions mullvad-update/meta/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::{
str::FromStr,
};
use tokio::{fs, io};
use vec1::vec1;

use crate::{
artifacts,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
)
Expand Down
11 changes: 7 additions & 4 deletions mullvad-update/src/client/api.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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]
Expand All @@ -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,
)?;
Expand Down Expand Up @@ -101,6 +102,7 @@ impl HttpVersionInfoProvider {
#[cfg(test)]
mod test {
use insta::assert_yaml_snapshot;
use vec1::vec1;

use crate::version::VersionArchitecture;

Expand All @@ -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;
Expand All @@ -138,7 +141,7 @@ mod test {
let info_provider = HttpVersionInfoProvider {
url,
pinned_certificate: None,
verifying_key,
verifying_keys,
};

let info = info_provider
Expand Down
29 changes: 18 additions & 11 deletions mullvad-update/src/format/deserializer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Deserializer and verifier of version metadata

use anyhow::Context;
use vec1::Vec1;

use super::key::*;
use super::Response;
Expand All @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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(),
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions mullvad-update/src/format/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions mullvad-update/src/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
54 changes: 53 additions & 1 deletion mullvad-update/src/format/serializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand All @@ -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(())
}
Expand Down
Loading