equalitie / ouisync

An in-development peer-to-peer file synchronization app.
Mozilla Public License 2.0
33 stars 8 forks source link

Upgrade to newer argon2 #144

Open inetic opened 8 months ago

inetic commented 8 months ago

When argon2 bumped its version from 4.x to 5.x, they changed some of the default parameters and that made the key derivation from the two versions incompatible.

I'll decrement the argon2 version for now, but we need to switch to the newer version eventually, but we need to do so in such a backward compatible way. Thus we'll likely need to introduce a metadata versioning and select the appropriate argon2 parameters based on that.

What would still be a desirable is to include a test code which would detect such parameter changes in the future so we don't accidentally lock out users from repositories. Whether such test would only cover argon2 and it's parameters, or would test a whole repo created by a previous release is still to be decided.

inetic commented 8 months ago

Temporarily decreased the argon2 version in ee3ebe224b73d22367c1fba5292e2aa1154e0c2

inetic commented 8 months ago

The above mentioned defaults are used here https://github.com/equalitie/ouisync/blob/d3515be6a7c33e506aaec5d9e35a277db81a1398/lib/src/crypto/cipher.rs#L86

inetic commented 8 months ago

This makes argon2 version 5.2 compatible with 4.x, but it's not the ideal solution because using the default parameters is likely the right thing to do.

diff --git a/lib/src/crypto/cipher.rs b/lib/src/crypto/cipher.rs
index 99589b96..e570f627 100644
--- a/lib/src/crypto/cipher.rs
+++ b/lib/src/crypto/cipher.rs
@@ -1,7 +1,7 @@
 //! Encryption / Decryption utilities.

 use super::{hash::Digest, password::PasswordSalt};
-use argon2::Argon2;
+use argon2::{self, Argon2};
 use chacha20::{
     cipher::{KeyIvInit, StreamCipher},
     ChaCha20,
@@ -80,10 +80,19 @@ impl SecretKey {
     /// Derive a secret key from user's password and salt.
     pub fn derive_from_password(user_password: &str, salt: &PasswordSalt) -> Self {
         let mut result = Self::zero();
+        let argon2 = Argon2::new(
+            argon2::Algorithm::default(),
+            argon2::Version::default(),
+            argon2::ParamsBuilder::default()
+                .m_cost(4096)
+                .t_cost(3)
+                .build()
+                .unwrap(),
+        );
         // Note: we control the output and salt size. And the only other check that this function
         // does is whether the password isn't too long, but that would have to be more than
         // 0xffffffff so the `.expect` shouldn't be an issue.
-        Argon2::default()
+        argon2
             .hash_password_into(user_password.as_ref(), salt, result.as_mut())
             .expect("failed to hash password");
         result