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

Upgrade to newer argon2 #144

Open
inetic opened this issue Oct 12, 2023 · 3 comments
Open

Upgrade to newer argon2 #144

inetic opened this issue Oct 12, 2023 · 3 comments

Comments

@inetic
Copy link
Member

inetic commented Oct 12, 2023

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
Copy link
Member Author

inetic commented Oct 12, 2023

Temporarily decreased the argon2 version in ee3ebe224b73d22367c1fba5292e2aa1154e0c2

@inetic
Copy link
Member Author

inetic commented Oct 12, 2023

The above mentioned defaults are used here

Argon2::default()

@inetic
Copy link
Member Author

inetic commented Oct 12, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant