From fa9e127a1e7d36c5f36b6186182173d2ab921cf5 Mon Sep 17 00:00:00 2001
From: gnieto <gnieto.talo@gmail.com>
Date: Tue, 14 Apr 2020 22:25:44 +0200
Subject: [PATCH] Store hashed passwords (#7)

Use if let instead of unwrap

Default to invalid password if could not calculate

Move hash password methdo and return Result

Rename get_password method

Default to empty password when no pwd is received

Store hashed passwords

Store passwords hashed with Argon2 and verify password with that stored
hash.

Co-authored-by: Guillem Nieto <gnieto.talo@gmail.com>
---
 Cargo.lock           |  15 ++++++-
 Cargo.toml           |   1 +
 src/client_server.rs |  27 +++++++++---
 src/data.rs          |   8 ++--
 src/test.rs          | 102 ++++++++++++++++++++++++++++++++++++++-----
 src/utils.rs         |  16 +++++++
 6 files changed, 148 insertions(+), 21 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 3101ab19..d6066556 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -155,6 +155,7 @@ dependencies = [
  "ruma-federation-api",
  "ruma-identifiers",
  "ruma-signatures",
+ "rust-argon2 0.8.2",
  "serde",
  "serde_json",
  "sled",
@@ -926,7 +927,7 @@ checksum = "09b23093265f8d200fa7b4c2c76297f47e681c655f6f1285a8780d6a022f7431"
 dependencies = [
  "getrandom",
  "redox_syscall",
- "rust-argon2",
+ "rust-argon2 0.7.0",
 ]
 
 [[package]]
@@ -1136,6 +1137,18 @@ dependencies = [
  "crossbeam-utils",
 ]
 
+[[package]]
+name = "rust-argon2"
+version = "0.8.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9dab61250775933275e84053ac235621dfb739556d5c54a2f2e9313b7cf43a19"
+dependencies = [
+ "base64 0.12.0",
+ "blake2b_simd",
+ "constant_time_eq",
+ "crossbeam-utils",
+]
+
 [[package]]
 name = "rustls"
 version = "0.16.0"
diff --git a/Cargo.toml b/Cargo.toml
index 7618ac0f..3b8da6c6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -28,3 +28,4 @@ ruma-federation-api = "0.0.1"
 serde = "1.0.106"
 tokio = { version = "0.2.16", features = ["macros"] } #rt-threaded
 rand = "0.7.3"
+rust-argon2 = "0.8.2"
\ No newline at end of file
diff --git a/src/client_server.rs b/src/client_server.rs
index f1c8a85a..1859b6f4 100644
--- a/src/client_server.rs
+++ b/src/client_server.rs
@@ -36,6 +36,7 @@ use ruma_events::{collections::only::Event as EduEvent, EventType};
 use ruma_identifiers::{RoomId, RoomIdOrAliasId, UserId};
 use serde_json::json;
 use std::{collections::HashMap, convert::TryInto, path::PathBuf, time::Duration};
+use argon2::{Config, Variant};
 
 const GUEST_NAME_LENGTH: usize = 10;
 const DEVICE_ID_LENGTH: usize = 10;
@@ -103,8 +104,20 @@ pub fn register_route(
         )));
     }
 
-    // Create user
-    data.user_add(&user_id, body.password.clone());
+    let password = body.password.clone().unwrap_or_default();
+
+    if let Ok(hash) = utils::calculate_hash(&password) {
+        // Create user
+        data.user_add(&user_id, &hash);
+    } else {
+        return MatrixResult(Err(UserInteractiveAuthenticationResponse::MatrixError(
+            Error {
+                kind: ErrorKind::InvalidParam,
+                message: "Password did not met requirements".to_owned(),
+                status_code: http::StatusCode::BAD_REQUEST,
+            },
+        )));
+    }
 
     // Generate new device id if the user didn't specify one
     let device_id = body
@@ -144,9 +157,11 @@ pub fn login_route(data: State<Data>, body: Ruma<login::Request>) -> MatrixResul
                 username = format!("@{}:{}", username, data.hostname());
             }
             if let Ok(user_id) = (*username).try_into() {
-                // Check password (this also checks if the user exists
-                if let Some(correct_password) = data.password_get(&user_id) {
-                    if password == correct_password {
+                if let Some(hash) = data.password_hash_get(&user_id) {
+                    let hash_matches = argon2::verify_encoded(&hash, password.as_bytes())
+                        .unwrap_or(false);
+
+                    if hash_matches {
                         // Success!
                         user_id
                     } else {
@@ -930,4 +945,4 @@ pub fn options_route(_segments: PathBuf) -> MatrixResult<create_message_event::R
         message: "This is the options route.".to_owned(),
         status_code: http::StatusCode::OK,
     }))
-}
+}
\ No newline at end of file
diff --git a/src/data.rs b/src/data.rs
index 815586fe..f7373121 100644
--- a/src/data.rs
+++ b/src/data.rs
@@ -36,10 +36,10 @@ impl Data {
     }
 
     /// Create a new user account by assigning them a password.
-    pub fn user_add(&self, user_id: &UserId, password: Option<String>) {
+    pub fn user_add(&self, user_id: &UserId, hash: &str) {
         self.db
             .userid_password
-            .insert(user_id.to_string(), &*password.unwrap_or_default())
+            .insert(user_id.to_string(), hash)
             .unwrap();
     }
 
@@ -61,8 +61,8 @@ impl Data {
             .collect()
     }
 
-    /// Checks if the given password is equal to the one in the database.
-    pub fn password_get(&self, user_id: &UserId) -> Option<String> {
+    /// Gets password hash for given user id.
+    pub fn password_hash_get(&self, user_id: &UserId) -> Option<String> {
         self.db
             .userid_password
             .get(user_id.to_string())
diff --git a/src/test.rs b/src/test.rs
index 6131eb2d..c3cefb16 100644
--- a/src/test.rs
+++ b/src/test.rs
@@ -1,5 +1,9 @@
 use super::*;
 use rocket::{local::Client, http::Status};
+use serde_json::Value;
+use serde_json::json;
+use ruma_client_api::error::ErrorKind;
+use std::time::Duration;
 
 fn setup_client() -> Client {
     Database::try_remove("temp");
@@ -14,19 +18,97 @@ async fn register_login() {
     let client = setup_client();
     let mut response = client
         .post("/_matrix/client/r0/register?kind=user")
-        .body(
-            r#"{
-    "username": "cheeky_monkey",
-    "password": "ilovebananas",
-    "device_id": "GHTYAJCE",
-    "initial_device_display_name": "Jungle Phone",
-    "inhibit_login": false
-            }"#,
-        )
+        .body(registration_init())
         .dispatch().await;
-    let body = serde_json::to_value(&response.body_string().await.unwrap()).unwrap();
+    let body = serde_json::from_str::<Value>(&response.body_string().await.unwrap()).unwrap();
 
     assert_eq!(response.status().code, 401);
     assert!(dbg!(&body["flows"]).as_array().unwrap().len() > 0);
     assert!(body["session"].as_str().unwrap().len() > 0);
 }
+
+#[tokio::test]
+async fn login_after_register_correct_password() {
+    let client = setup_client();
+    let mut response = client
+        .post("/_matrix/client/r0/register?kind=user")
+        .body(registration_init())
+        .dispatch().await;
+    let body = serde_json::from_str::<Value>(&response.body_string().await.unwrap()).unwrap();
+    let session = body["session"].clone();
+
+    let response = client
+        .post("/_matrix/client/r0/register?kind=user")
+        .body(registration(session.as_str().unwrap()))
+        .dispatch().await;
+    assert_eq!(response.status().code, 200);
+
+    let login_response = client
+        .post("/_matrix/client/r0/login")
+        .body(login_with_password("ilovebananas"))
+        .dispatch()
+        .await;
+    assert_eq!(login_response.status().code, 200);
+}
+
+#[tokio::test]
+async fn login_after_register_incorrect_password() {
+    let client = setup_client();
+    let mut response = client
+        .post("/_matrix/client/r0/register?kind=user")
+        .body(registration_init())
+        .dispatch().await;
+    let body = serde_json::from_str::<Value>(&response.body_string().await.unwrap()).unwrap();
+    let session = body["session"].clone();
+
+    let response = client
+        .post("/_matrix/client/r0/register?kind=user")
+        .body(registration(session.as_str().unwrap()))
+        .dispatch().await;
+    assert_eq!(response.status().code, 200);
+
+    let mut login_response = client
+        .post("/_matrix/client/r0/login")
+        .body(login_with_password("idontlovebananas"))
+        .dispatch()
+        .await;
+    let body = serde_json::from_str::<Value>(&login_response.body_string().await.unwrap()).unwrap();
+    assert_eq!(body.as_object().unwrap().get("errcode").unwrap().as_str().unwrap(), "M_FORBIDDEN");
+    assert_eq!(login_response.status().code, 403);
+}
+
+fn registration_init() -> &'static str {
+    r#"{
+    "username": "cheeky_monkey",
+    "password": "ilovebananas",
+    "device_id": "GHTYAJCE",
+    "initial_device_display_name": "Jungle Phone",
+    "inhibit_login": false
+            }"#
+}
+
+fn registration(session: &str) -> String {
+    json!({
+        "auth": {
+            "session": session,
+            "type": "m.login.dummy"
+        },
+        "username": "cheeky_monkey",
+        "password": "ilovebananas",
+        "device_id": "GHTYAJCE",
+        "initial_device_display_name": "Jungle Phone",
+        "inhibit_login": false
+    }).to_string()
+}
+
+fn login_with_password(password: &str) -> String {
+    json!({
+        "type": "m.login.password",
+        "identifier": {
+            "type": "m.id.user",
+            "user": "cheeky_monkey"
+        },
+        "password": password,
+        "initial_device_display_name": "Jungle Phone"
+    }).to_string()
+}
\ No newline at end of file
diff --git a/src/utils.rs b/src/utils.rs
index 5e941726..3b3ed929 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -3,6 +3,7 @@ use std::{
     convert::TryInto,
     time::{SystemTime, UNIX_EPOCH},
 };
+use argon2::{Config, Variant};
 
 pub fn millis_since_unix_epoch() -> u64 {
     SystemTime::now()
@@ -39,3 +40,18 @@ pub fn random_string(length: usize) -> String {
         .take(length)
         .collect()
 }
+
+/// Calculate a new hash for the given password
+pub fn calculate_hash(password: &str) -> Result<String, argon2::Error> {
+    let hashing_config = Config {
+        variant: Variant::Argon2id,
+        ..Default::default()
+    };
+
+    let salt = random_string(32);
+    argon2::hash_encoded(
+        password.as_bytes(),
+        salt.as_bytes(),
+        &hashing_config,
+    )
+}
\ No newline at end of file