Fix/argon2id vault kdf

SECURITY.md

@@ -1,5 +1,11 @@
 # Security Policy
 
+## Key Derivation Function (KDF) Migration (Issue #XX)
+
+As of version X.X.X, the vault key derivation has been upgraded from SHA-256 to **Argon2id** with per-instance salts. This prevents dictionary and rainbow-table attacks on operator passwords.
+
+**For users:** When you next load or create a vault with a password, the new KDF will be used automatically. Existing SHA-256-derived vaults must be re-encrypted by creating a new vault and migrating keys.
+
 ## Reporting a Vulnerability
 
 OrbitChain-Contracts contains Soroban smart contracts handling crowdfunding and fund management. Security vulnerabilities can have serious financial consequences. Please report them responsibly.

campaign/src/event.rs

@@ -83,3 +83,30 @@ pub fn contract_unfrozen(env: &Env, admin: &Address, timestamp: u64) {
         (admin, timestamp),
     );
 }
+
+/// Emitted when a milestone release skips an asset (no issuer, dust, etc.).
+pub fn milestone_release_skipped(
+    env: &Env,
+    milestone_index: u32,
+    asset_code: String,
+    reason: Symbol,
+) {
+    env.events().publish(
+        ("campaign", "milestone_release_skipped"),
+        (milestone_index, asset_code, reason),
+    );
+}
+
+/// Emitted after all per-asset releases complete for a milestone.
+pub fn milestone_release_completed(
+    env: &Env,
+    milestone_index: u32,
+    total_released: i128,
+    asset_count: u32,
+    timestamp: u64,
+) {
+    env.events().publish(
+        ("campaign", "milestone_release_completed"),
+        (milestone_index, total_released, asset_count, timestamp),
+    );
+}

campaign/src/multi_asset_release.rs

@@ -115,18 +115,21 @@ pub fn release_milestone_multi_asset(
     milestone.status = MilestoneStatus::Released;
     set_milestone(env, milestone_index, &milestone);
 
-    // ── 7. Execute proportional transfers ───────────────────────────────────
     let timestamp = env.ledger().timestamp();
+
+    // ── 7. Execute proportional transfers ───────────────────────────────────
     let mut total_released: i128 = 0;
 
     for asset in campaign.accepted_assets.iter() {
         let token_address = match &asset.issuer {
             Some(addr) => addr.clone(),
             None => {
                 // Native asset or asset without issuer — skip gracefully
-                env.events().publish(
-                    (symbol_short!("ms_skip"), symbol_short!("no_issuer")),
-                    (milestone_index, asset.asset_code.clone()),
+                event::milestone_release_skipped(
+                    env,
+                    milestone_index,
+                    asset.asset_code.clone(),
+                    symbol_short!("no_issuer"),
                 );
                 continue;
             }
@@ -193,6 +196,15 @@ pub fn release_milestone_multi_asset(
             .unwrap_or_else(|| panic_with_error!(env, Error::Overflow));
     }
 
+    // Emit summary event after all per-asset releases
+    event::milestone_release_completed(
+        env,
+        milestone_index,
+        total_released,
+        campaign.accepted_assets.len() as u32,
+        timestamp,
+    );
+
     // ── 8. Update global total-raised bookkeeping ────────────────────────────
     let new_total_raised = total_raised
         .checked_sub(total_released)

campaign/src/test/release_milestone_tests.rs

@@ -470,3 +470,79 @@ fn test_frozen_contract_release_panics() {
         crate::release_milestone::release_milestone(&env, 0, recipient);
     });
 }
+
+// ─── Skip events: assets without issuers are skipped gracefully ───────────────
+
+/// Test: when an asset has no issuer (native XLM), a milestone_release_skipped
+/// event is emitted and the release continues with other assets.
+#[test]
+fn test_native_asset_skip_emits_event() {
+    let env = Env::default();
+    env.ledger().set_timestamp(BASE);
+    env.mock_all_auths();
+    with_contract(&env, || {
+        let creator = Address::generate(&env);
+
+        // Create a campaign with one asset that has an issuer
+        let token_admin = Address::generate(&env);
+        let token_issuer = env.register_stellar_asset_contract(token_admin.clone());
+
+        let mut assets: Vec<StellarAsset> = Vec::new(&env);
+        // Add native asset (no issuer)
+        assets.push_back(StellarAsset {
+            asset_code: String::from_str(&env, "XLM"),
+            issuer: None,
+        });
+        // Add an asset with issuer
+        assets.push_back(StellarAsset {
+            asset_code: String::from_str(&env, "USDC"),
+            issuer: Some(token_issuer.clone()),
+        });
+
+        let campaign = CampaignData {
+            creator: creator.clone(),
+            goal_amount: 3000,
+            raised_amount: 3000,
+            end_time: env.ledger().timestamp() + 86_400,
+            status: CampaignStatus::Active,
+            accepted_assets: assets,
+            milestone_count: 1,
+            min_donation_amount: 0,
+            created_at_ledger: env.ledger().sequence(),
+            created_at_time: env.ledger().timestamp(),
+            concluded_at_ledger: None,
+        };
+        set_campaign(&env, &campaign);
+
+        // Mint tokens for the USDC asset
+        let token_admin_client = StellarAssetClient::new(&env, &token_issuer);
+        token_admin_client.mint(&env.current_contract_address(), &10_000_000i128);
+
+        create_test_milestone(&env, 0, 3000, MilestoneStatus::Unlocked);
+        let recipient = Address::generate(&env);
+
+        // Release should succeed and emit skip event for XLM
+        crate::release_milestone::release_milestone(&env, 0, recipient.clone());
+
+        // Verify USDC was released
+        let token_client = soroban_sdk::token::Client::new(&env, &token_issuer);
+        assert_eq!(token_client.balance(&recipient), 3000);
+
+        // Verify milestone status is Released
+        let milestone = get_milestone(&env, 0).expect("Milestone should exist");
+        assert_eq!(milestone.status, MilestoneStatus::Released);
+
+        // Verify skip event was emitted with correct topic
+        let events = env.events().all();
+        let skip_event = events.iter().find(|e| {
+            e.topics == (soroban_sdk::Symbol::new(&env, "campaign"), soroban_sdk::Symbol::new(&env, "milestone_release_skipped"))
+        });
+        assert!(skip_event.is_some(), "milestone_release_skipped event should be emitted");
+
+        // Verify completion event was emitted
+        let completed_event = events.iter().find(|e| {
+            e.topics == (soroban_sdk::Symbol::new(&env, "campaign"), soroban_sdk::Symbol::new(&env, "milestone_release_completed"))
+        });
+        assert!(completed_event.is_some(), "milestone_release_completed event should be emitted");
+    });
+}

crates/tools/Cargo.toml

@@ -22,4 +22,5 @@ rand = "0.8"
 hex = "0.4"
 sha2 = "0.10"
 zeroize = "1.6"
+argon2 = "0.5"
 chrono = { version = "0.4", features = ["serde"] }

crates/tools/src/encrypted_vault.rs

@@ -20,6 +20,8 @@ pub struct EncryptedVault {
     encrypted_keys: HashMap<String, String>,
     /// Public keys (safe to store unencrypted)
     public_keys: HashMap<String, String>,
+    /// KDF salt (persisted alongside vault)
+    salt: Option<[u8; 16]>,
 }
 
 impl EncryptedVault {
@@ -30,28 +32,33 @@ impl EncryptedVault {
             key_manager: None,
             encrypted_keys: HashMap::new(),
             public_keys: HashMap::new(),
+            salt: None,
         }
     }
 
     /// Initialize vault with a master password
     #[must_use]
     pub fn with_password(password: &str) -> Result<Self> {
         let key_manager = KeyManager::from_password(password)?;
+        let salt = *key_manager.get_salt();
         Ok(Self {
             key_manager: Some(key_manager),
             encrypted_keys: HashMap::new(),
             public_keys: HashMap::new(),
+            salt: Some(salt),
         })
     }
 
     /// Initialize vault with a hex-encoded master key
     #[must_use]
     pub fn with_hex_key(hex_key: &str) -> Result<Self> {
         let key_manager = KeyManager::from_hex_key(hex_key)?;
+        let salt = *key_manager.get_salt();
         Ok(Self {
             key_manager: Some(key_manager),
             encrypted_keys: HashMap::new(),
             public_keys: HashMap::new(),
+            salt: Some(salt),
         })
     }
 
@@ -151,6 +158,11 @@ impl EncryptedVault {
         let mut content = String::from("# Encrypted Vault Configuration\n");
         content.push_str("# WARNING: Keep this file secure!\n\n");
 
+        // Save KDF salt (required for decryption)
+        if let Some(salt) = &self.salt {
+            content.push_str(&format!("VAULT_SALT={}\n\n", hex::encode(salt)));
+        }
+
         // Save public keys
         content.push_str("# Public Keys (unencrypted)\n");
         for (name, key) in &self.public_keys {
@@ -180,10 +192,34 @@ impl EncryptedVault {
     /// Load vault from encrypted file
     pub fn load_from_file(path: &str, password: &str) -> Result<Self> {
         let content = fs::read_to_string(path).context("Failed to read vault file")?;
-        let mut vault = Self::with_password(password)?;
+        let mut salt: Option<[u8; 16]> = None;
+
+        for line in content.lines() {
+            if line.starts_with("VAULT_SALT=") {
+                let salt_hex = line.trim_start_matches("VAULT_SALT=");
+                let salt_bytes = hex::decode(salt_hex).context("Failed to decode salt")?;
+                if salt_bytes.len() != 16 {
+                    anyhow::bail!("Invalid salt length: expected 16 bytes, got {}", salt_bytes.len());
+                }
+                let mut s = [0u8; 16];
+                s.copy_from_slice(&salt_bytes);
+                salt = Some(s);
+                break;
+            }
+        }
+
+        let salt = salt.ok_or_else(|| anyhow::anyhow!("No VAULT_SALT found in vault file. Ensure file was created with current version."))?;
+        let key_manager = KeyManager::from_password_with_salt(password, &salt)?;
+
+        let mut vault = Self {
+            key_manager: Some(key_manager),
+            encrypted_keys: HashMap::new(),
+            public_keys: HashMap::new(),
+            salt: Some(salt),
+        };
 
         for line in content.lines() {
-            if line.starts_with('#') || line.trim().is_empty() {
+            if line.starts_with('#') || line.trim().is_empty() || line.starts_with("VAULT_SALT=") {
                 continue;
             }
 

crates/tools/src/key_manager.rs

@@ -3,8 +3,8 @@ use aes_gcm::{
     Aes256Gcm, Nonce,
 };
 use anyhow::{Context, Result};
+use argon2::{Algorithm, Argon2, Params, Version};
 use rand::Rng;
-use sha2::{Digest, Sha256};
 use std::fmt;
 use zeroize::Zeroize;
 
@@ -37,27 +37,42 @@ impl Drop for EncryptedKey {
 #[derive(Debug)]
 pub struct KeyManager {
     master_key: [u8; 32],
+    salt: [u8; 16],
 }
 
 impl Drop for KeyManager {
     fn drop(&mut self) {
         self.master_key.zeroize();
+        self.salt.zeroize();
     }
 }
 
 impl KeyManager {
     /// Initialize KeyManager from a master password/key.
-    /// Derives a 256-bit key using SHA-256.
+    /// Derives a 256-bit key using Argon2id with a random salt.
     #[must_use]
     pub fn from_password(password: &str) -> Result<Self> {
-        let mut hasher = Sha256::new();
-        hasher.update(password.as_bytes());
-        let key_bytes = hasher.finalize();
+        let mut rng = rand::thread_rng();
+        let mut salt = [0u8; 16];
+        rng.fill(&mut salt);
+        Self::from_password_with_salt(password, &salt)
+    }
 
+    /// Initialize KeyManager from password with a specific salt (for loading).
+    #[must_use]
+    pub fn from_password_with_salt(password: &str, salt: &[u8; 16]) -> Result<Self> {
+        let params = Params::new(19_456, 2, 1, Some(32))
+            .map_err(|e| anyhow::anyhow!("Failed to create Argon2 parameters: {}", e))?;
+        let argon = Argon2::new(Algorithm::Argon2id, Version::V0x13, params);
         let mut master_key = [0u8; 32];
-        master_key.copy_from_slice(&key_bytes);
+        argon
+            .hash_password_into(password.as_bytes(), salt, &mut master_key)
+            .map_err(|e| anyhow::anyhow!("Argon2 key derivation failed: {}", e))?;
 
-        Ok(Self { master_key })
+        Ok(Self {
+            master_key,
+            salt: *salt,
+        })
     }
 
     /// Initialize KeyManager from a 32-byte hex string.
@@ -68,10 +83,14 @@ impl KeyManager {
             anyhow::bail!("Key must be exactly 32 bytes, got {}", key_bytes.len());
         }
 
+        let mut rng = rand::thread_rng();
+        let mut salt = [0u8; 16];
+        rng.fill(&mut salt);
+
         let mut master_key = [0u8; 32];
         master_key.copy_from_slice(&key_bytes);
 
-        Ok(Self { master_key })
+        Ok(Self { master_key, salt })
     }
 
     /// Encrypt a private key (secret key) using AES-256-GCM
@@ -110,6 +129,11 @@ impl KeyManager {
         String::from_utf8(plaintext).context("Decrypted key is not valid UTF-8")
     }
 
+    /// Get the salt used for key derivation
+    pub fn get_salt(&self) -> &[u8; 16] {
+        &self.salt
+    }
+
     /// Export encrypted key as hex string for storage
     pub fn export_encrypted(&self, secret_key: &str) -> Result<String> {
         let encrypted = self.encrypt_key(secret_key)?;
@@ -200,15 +224,30 @@ mod tests {
         let secret_key = "SBZXVMIRWXL5VZVKXWV2FGKYTQ5VV5VRNJYQVZKYWW3XYVYP3IXGKDU";
 
         let encrypted = manager1.encrypt_key(secret_key)?;
+        let salt1 = manager1.get_salt();
 
-        let manager2 = KeyManager::from_password("password2")?;
+        // Manager2 must use same salt to test password difference
+        let manager2 = KeyManager::from_password_with_salt("password2", salt1)?;
         let result = manager2.decrypt_key(&encrypted);
 
         // Should fail due to wrong password
         assert!(result.is_err());
         Ok(())
     }
 
+    #[test]
+    fn test_password_with_same_salt_derives_same_key() -> Result<()> {
+        let salt = [42u8; 16];
+        let password = "test_password";
+
+        let manager1 = KeyManager::from_password_with_salt(password, &salt)?;
+        let manager2 = KeyManager::from_password_with_salt(password, &salt)?;
+
+        // Same password and salt should derive identical keys
+        assert_eq!(manager1.master_key, manager2.master_key);
+        Ok(())
+    }
+
     #[test]
     fn test_validate_secret_key() {
         assert!(KeyManager::validate_secret_key("SBZXVMIRWXL5VZVKXWV2FGKYTQ5VV5VRNJYQVZKYWW3XYVYP3IXGKDU").is_ok());