Merge branch 'mullvad-update-zeroize-secret'

3 files changed, 29 insertions, 21 deletions

diff --git a/Cargo.lock b/Cargo.lock
index a0c5623f67..de888dff51 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1037,6 +1037,7 @@ checksum = "4a3daa8e81a3963a60642bcc1f90a670680bd4a77535faa384e9d1c79d620871"
 dependencies = [
  "curve25519-dalek",
  "ed25519",
+ "rand_core 0.6.4",
  "serde",
  "sha2",
  "subtle",
@@ -2909,6 +2910,7 @@ dependencies = [
  "thiserror 2.0.9",
  "tokio",
  "vec1",
+ "zeroize",
 ]
 
 [[package]]
diff --git a/mullvad-update/Cargo.toml b/mullvad-update/Cargo.toml
index e295de3c1f..4bd9a5c74b 100644
--- a/mullvad-update/Cargo.toml
+++ b/mullvad-update/Cargo.toml
@@ -13,22 +13,23 @@ workspace = true
 [features]
 default = []
 sign = ["rand", "clap"]
-client = ["async-trait", "reqwest", "sha2", "tokio", "thiserror", "vec1"]
+client = ["async-trait", "reqwest", "sha2", "tokio", "thiserror"]
 
 [dependencies]
 anyhow = { workspace = true }
 json-canon = "0.1"
 chrono = { workspace = true, features = ["serde", "now"] }
-ed25519-dalek = { version = "2.1" }
+ed25519-dalek = { version = "2.1", features = ["zeroize", "rand_core"] }
 hex = { version = "0.4" }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
+zeroize = { version = "1.8", features = ["zeroize_derive"] }
 
 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 }
+vec1 = { workspace = true }
 
 mullvad-version = { path = "../mullvad-version", features = ["serde"] }
 
diff --git a/mullvad-update/src/format/key.rs b/mullvad-update/src/format/key.rs
index 58994bac05..bebc53dce1 100644
--- a/mullvad-update/src/format/key.rs
+++ b/mullvad-update/src/format/key.rs
@@ -3,40 +3,44 @@
 use std::{fmt, str::FromStr};
 
 use anyhow::{bail, Context};
-use ed25519_dalek::ed25519::signature::SignerMut;
-#[cfg(feature = "sign")]
-use rand::RngCore;
+use ed25519_dalek::ed25519::signature::Signer;
 use serde::{Deserialize, Serialize};
+use zeroize::Zeroize;
 
 /// ed25519 secret/signing key
-#[derive(Debug, Clone, PartialEq)]
-pub struct SecretKey(pub ed25519_dalek::SecretKey);
+#[derive(Clone, PartialEq, zeroize::ZeroizeOnDrop)]
+#[cfg_attr(test, derive(Debug))]
+pub struct SecretKey(ed25519_dalek::SigningKey);
 
 impl SecretKey {
     /// Generate a new secret ed25519 key
     #[cfg(feature = "sign")]
     pub fn generate() -> Self {
         // Using OsRng is suggested by the docs
-        let mut bytes = ed25519_dalek::SecretKey::default();
-        rand::rngs::OsRng.fill_bytes(&mut bytes);
-        SecretKey(bytes)
+        let key = ed25519_dalek::SigningKey::generate(&mut rand::rngs::OsRng);
+        SecretKey(key)
     }
 
     pub fn pubkey(&self) -> VerifyingKey {
-        let sign_key = ed25519_dalek::SigningKey::from_bytes(&self.0);
-        VerifyingKey(sign_key.verifying_key())
+        VerifyingKey(self.0.verifying_key())
     }
 
     /// Sign data using this key
     pub fn sign(&self, msg: &[u8]) -> Signature {
-        let mut secret = ed25519_dalek::SigningKey::from_bytes(&self.0);
-        Signature(secret.sign(msg))
+        Signature(self.0.sign(msg))
+    }
+
+    /// Convert bytes to a signing key, and zero the original bytes
+    fn from_bytes(mut key: [u8; ed25519_dalek::SECRET_KEY_LENGTH]) -> Self {
+        let secret = ed25519_dalek::SigningKey::from_bytes(&key);
+        key.zeroize();
+        SecretKey(secret)
     }
 }
 
 impl fmt::Display for SecretKey {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", hex::encode(self.0))
+        write!(f, "{}", hex::encode(self.0.as_bytes()))
     }
 }
 
@@ -45,10 +49,11 @@ impl<'de> Deserialize<'de> for SecretKey {
     where
         D: serde::Deserializer<'de>,
     {
-        let key = String::deserialize(deserializer)?;
-        let key = bytes_from_hex::<{ ed25519_dalek::SECRET_KEY_LENGTH }>(&key)
+        let mut key_s = String::deserialize(deserializer)?;
+        let key = bytes_from_hex::<{ ed25519_dalek::SECRET_KEY_LENGTH }>(&key_s)
             .map_err(|err| serde::de::Error::custom(err.to_string()))?;
-        Ok(SecretKey(key))
+        key_s.zeroize();
+        Ok(SecretKey::from_bytes(key))
     }
 }
 
@@ -57,7 +62,7 @@ impl FromStr for SecretKey {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let bytes = bytes_from_hex::<{ ed25519_dalek::SECRET_KEY_LENGTH }>(s)?;
-        Ok(SecretKey(bytes))
+        Ok(Self::from_bytes(bytes))
     }
 }
 
@@ -66,7 +71,7 @@ impl Serialize for SecretKey {
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(&hex::encode(self.0))
+        serializer.serialize_str(&hex::encode(self.0.as_bytes()))
     }
 }