Replace pbkdf2 with argon2 via rust-argon2

oxide-auth/Cargo.toml

@@ -17,13 +17,15 @@ autoexamples = false
 base64 = "0.11"
 chrono = "0.4.2"
 hmac = "0.7.1"
+once_cell = "1.3.1"
 serde = "1.0"
 serde_derive = "1.0"
 serde_json = "1.0"
 sha2 = "0.8.1"
 subtle = "2.2.2"
 rand = "0.7.3"
 ring = ">=0.13,<0.15"
+rust-argon2 = "0.8.2"
 rmp-serde = "0.14"
 url = "1.7"
 

oxide-auth/src/lib.rs

@@ -65,10 +65,12 @@
 //! [`Scopes`]: endpoint/trait.Scopes.html
 #![warn(missing_docs)]
 
+extern crate argon2;
 extern crate base64;
 extern crate chrono;
 extern crate hmac;
 extern crate rand;
+extern crate once_cell;
 extern crate ring;
 extern crate rmp_serde;
 extern crate serde;

oxide-auth/src/primitives/registrar.rs

@@ -10,14 +10,13 @@ use std::cmp;
 use std::collections::HashMap;
 use std::fmt;
 use std::iter::{Extend, FromIterator};
-use std::num::NonZeroU32;
-use std::sync::{Arc, MutexGuard, RwLockWriteGuard};
 use std::rc::Rc;
+use std::sync::{Arc, MutexGuard, RwLockWriteGuard};
 
+use argon2::{self, Config};
+use once_cell::sync::Lazy;
+use rand::{RngCore, thread_rng};
 use url::Url;
-use ring::{digest, pbkdf2};
-use ring::error::Unspecified;
-use ring::rand::{SystemRandom, SecureRandom};
 
 /// Registrars provie a way to interact with clients.
 ///
@@ -188,16 +187,16 @@ impl fmt::Debug for ClientType {
     }
 }
 
-impl RegistrarError {
-    fn from(err: Unspecified) -> Self {
-        match err { Unspecified => RegistrarError::Unspecified }
-    }
-}
-
 impl Client {
     /// Create a public client.
     pub fn public(client_id: &str, redirect_uri: Url, default_scope: Scope) -> Client {
-        Client { client_id: client_id.to_string(), redirect_uri, additional_redirect_uris: vec![], default_scope, client_type: ClientType::Public }
+        Client { 
+            client_id: client_id.to_string(), 
+            redirect_uri, 
+            additional_redirect_uris: vec![], 
+            default_scope, 
+            client_type: ClientType::Public 
+        }
     }
 
     /// Create a confidential client.
@@ -281,8 +280,7 @@ impl cmp::PartialOrd<Self> for PreGrant {
 
 /// Determines how passphrases are stored and checked. 
 ///
-/// The provided library implementation is based on `Pbkdf2`. Other users may prefer to write their
-/// own adaption with `Argon2`. If you do so, you could send a pull request my way.
+/// The provided library implementation is based on `Argon2`.
 pub trait PasswordPolicy: Send + Sync {
     /// Transform the passphrase so it can be stored in the confidential client.
     fn store(&self, client_id: &str, passphrase: &[u8]) -> Vec<u8>;
@@ -291,107 +289,46 @@ pub trait PasswordPolicy: Send + Sync {
     fn check(&self, client_id: &str, passphrase: &[u8], stored: &[u8]) -> Result<(), RegistrarError>;
 }
 
-/// Store passwords using `Pbkdf2` to derive the stored value.
-///
-/// Each instantiation generates a 16 byte random salt and prepends this additionally with the
-/// username. This combined string is then used as the salt using the passphrase as the secret to
-/// derive the output. The iteration count defaults to `65536` but can be customized.
-pub struct Pbkdf2 {
-    /// A prebuilt random, or constructing one as needed.
-    random: Option<SystemRandom>,
-    iterations: NonZeroU32,
-}
-
-impl Default for Pbkdf2 {
-    fn default() -> Self {
-        Pbkdf2 {
-            random: Some(SystemRandom::new()),
-            .. *PBKDF2_DEFAULTS
-        }
-    }
-}
-
-impl Clone for Pbkdf2 {
-    fn clone(&self) -> Self {
-        Pbkdf2 {
-            random: Some(SystemRandom::new()),
-            .. *self
-        }
-    }
-}
-
-impl fmt::Debug for Pbkdf2 {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_struct("Pbkdf2")
-            .field("iterations", &self.iterations)
-            .field("random", &())
-            .finish()
-    }
+/// Store passwords using `Argon2` to derive the stored value.
+#[derive(Clone, Debug, Default)]
+pub struct Argon2 {
+    _private: ()
 }
 
-impl Pbkdf2 {
-    /// Set the iteration count to `(1 << strength)`.
-    ///
-    /// This function will panic when the `strength` is larger or equal to `32`.
-    pub fn set_relative_strength(&mut self, strength: u8) {
-        assert!(strength < 32, "Strength value out of range (0-31): {}", strength);
-        self.iterations = NonZeroU32::new(1u32 << strength).unwrap();
-    }
-
-    fn salt(&self, user_identifier: &[u8]) -> Vec<u8> {
-        let mut vec = Vec::with_capacity(user_identifier.len() + 64);
-        let mut rnd_salt = [0; 16];
+impl PasswordPolicy for Argon2 {
+    fn store(&self, client_id: &str, passphrase: &[u8]) -> Vec<u8> {
+        let mut config = Config::default();
+        config.ad = client_id.as_bytes();
+        config.secret = &[];
 
-        match self.random.as_ref() {
-            Some(random) => random.fill(&mut rnd_salt),
-            None => SystemRandom::new().fill(&mut rnd_salt),
-        }.expect("Failed to property initialize password storage salt");
+        let mut salt = vec![0; 32];
+        thread_rng().try_fill_bytes(salt.as_mut_slice())
+            .expect("Failed to generate password salt");
 
-        vec.extend_from_slice(user_identifier);
-        vec.extend_from_slice(&rnd_salt[..]);
-        vec
+        let encoded = argon2::hash_encoded(passphrase, &salt, &config);
+        encoded.unwrap().as_bytes().to_vec()
     }
-}
 
-// A default instance for pbkdf2, randomness is sampled from the system each time.
-//
-// TODO: in the future there might be a way to get static memory initialized with an rng at load
-// time by the loader. Then, a constant instance of the random generator may be available and we
-// could get rid of the `Option`.
-static PBKDF2_DEFAULTS: &Pbkdf2 = &Pbkdf2 {
-    random: None,
-    iterations: unsafe { NonZeroU32::new_unchecked(1 << 16) },
-};
-
-impl PasswordPolicy for Pbkdf2 {
-    fn store(&self, client_id: &str, passphrase: &[u8]) -> Vec<u8> {
-        let mut output = vec![0; 64];
-        output.append(&mut self.salt(client_id.as_bytes()));
-        {
-            let (output, salt) = output.split_at_mut(64);
-            pbkdf2::derive(&digest::SHA256, self.iterations.into(), salt, passphrase,
-                output);
-        }
-        output
-    }
-
-    fn check(&self, _client_id: &str /* Was interned */, passphrase: &[u8], stored: &[u8])
+    fn check(&self, client_id: &str, passphrase: &[u8], stored: &[u8])
         -> Result<(), RegistrarError>
     {
-        if stored.len() < 64 {
-            return Err(RegistrarError::PrimitiveError)
+        let hash = String::from_utf8(stored.to_vec())
+            .map_err(|_| RegistrarError::PrimitiveError)?;
+        let valid = argon2::verify_encoded_ext(&hash, passphrase, &[], client_id.as_bytes())
+            .map_err(|_| RegistrarError::PrimitiveError)?;
+        match valid {
+            true => Ok(()),
+            false => Err(RegistrarError::Unspecified),
         }
-
-        let (verifier, salt) = stored.split_at(64);
-        pbkdf2::verify(&digest::SHA256, self.iterations.into(), salt, passphrase, verifier)
-            .map_err(RegistrarError::from)
     }
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 //                             Standard Implementations of Registrars                            //
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
+static DEFAULT_PASSWORD_POLICY: Lazy<Argon2> = Lazy::new(|| { Argon2::default() });
+
 impl ClientMap {
     /// Create an empty map without any clients in it.
     pub fn new() -> ClientMap {
@@ -413,7 +350,7 @@ impl ClientMap {
     fn current_policy<'a>(policy: &'a Option<Box<dyn PasswordPolicy>>) -> &'a dyn PasswordPolicy {
         policy
             .as_ref().map(|boxed| &**boxed)
-            .unwrap_or(PBKDF2_DEFAULTS)
+            .unwrap_or(&*DEFAULT_PASSWORD_POLICY)
     }
 }
 
@@ -616,7 +553,7 @@ mod tests {
 
     #[test]
     fn public_client() {
-        let policy = Pbkdf2::default();
+        let policy = Argon2::default();
         let client = Client::public(
             "ClientId",
             "https://example.com".parse().unwrap(),
@@ -632,7 +569,7 @@ mod tests {
 
     #[test]
     fn confidential_client() {
-        let policy = Pbkdf2::default();
+        let policy = Argon2::default();
         let pass = b"AB3fAj6GJpdxmEVeNCyPoA==";
         let client = Client::confidential(
             "ClientId",