From 64114b6d68762970b1c1c52c4b673ec2571fb248 Mon Sep 17 00:00:00 2001 From: rasul Date: Sun, 19 Apr 2020 16:21:22 -0500 Subject: [PATCH] Store a salted and hashed password. Lots of changes for this to happen. Passwords are now to be stored separately from the players in a separate table in the database, and will only be queried when necessary. Random salts are generated, and a hash of the salted password is stored along with the salt. Some functions and structs to do all this were added. If I did it right, this should make the password storage secure, even if nothing else is. --- src/command/set/player/password.rs | 9 ++-- src/database/connected_players.rs | 4 +- src/database/mod.rs | 1 + src/database/password.rs | 55 +++++++++++++++++++++++ src/database/players.rs | 11 ++--- src/game/state/login.rs | 72 ++++++++++++++++++++---------- src/lib.rs | 1 + src/main.rs | 1 + src/password.rs | 55 +++++++++++++++++++++++ src/player.rs | 4 -- src/state.rs | 4 +- 11 files changed, 174 insertions(+), 43 deletions(-) create mode 100644 src/database/password.rs create mode 100644 src/password.rs diff --git a/src/command/set/player/password.rs b/src/command/set/player/password.rs index 29476c5..86291fe 100644 --- a/src/command/set/player/password.rs +++ b/src/command/set/player/password.rs @@ -2,22 +2,21 @@ use mio::Token; use crate::command::CommandSetPlayer; use crate::database::Db; +use crate::password::Password; use crate::queue::SendQueue; use crate::{try_option_send_error, try_send_error}; impl CommandSetPlayer { /// Set the player's password pub fn dispatch_password(&self, args: String, token: Token, db: &mut Db) -> SendQueue { - let mut player = try_option_send_error!(token, db.get_connected_player(token)); + let player = try_option_send_error!(token, db.get_connected_player(token)); if args.is_empty() { return (token, "Password can't be empty").into(); } - player.password = args; - - let _ = try_send_error!(token, db.save_player(&player)); - let _ = try_send_error!(token, db.save_connected_player(token, &player)); + let password = Password::new(args); + let _ = try_send_error!(token, db.save_password(player.id, &password)); SendQueue::ok(token) } diff --git a/src/database/connected_players.rs b/src/database/connected_players.rs index 8ad2604..bfed8ec 100644 --- a/src/database/connected_players.rs +++ b/src/database/connected_players.rs @@ -17,7 +17,7 @@ impl Db { ) -> RudeResult> { let mut statement = try_log!( self.0.prepare( - "select connected_players.token, players.id, players.name, players.password, players.created, players.location from connected_players, players where players.location = ? and connected_players.player = players.id;" + "select connected_players.token, players.id, players.name, players.created, players.location from connected_players, players where players.location = ? and connected_players.player = players.id;" ), "Unable to prepare sql statement" ); @@ -42,7 +42,7 @@ impl Db { pub fn get_connected_player(&self, token: Token) -> RudeResult> { let mut statement = try_log!( self.0.prepare( - "select connected_players.token, players.id, players.name, players.password, players.created, players.location from connected_players, players where connected_players.token = ? and connected_players.player = players.id;" + "select connected_players.token, players.id, players.name, players.created, players.location from connected_players, players where connected_players.token = ? and connected_players.player = players.id;" ), "Unable to prepare sql statement" ); diff --git a/src/database/mod.rs b/src/database/mod.rs index d497843..b2f16ea 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -2,6 +2,7 @@ mod connected_players; mod db; +mod password; mod players; mod rooms; mod zones; diff --git a/src/database/password.rs b/src/database/password.rs new file mode 100644 index 0000000..6344bcd --- /dev/null +++ b/src/database/password.rs @@ -0,0 +1,55 @@ +use std::convert::TryFrom; + +use rusqlite::params; + +use crate::database::Db; +use crate::id::Id; +use crate::password::Password; +use crate::result::RudeResult; +use crate::try_log; + +impl Db { + /// Get a password + pub fn get_password(&self, id: Id) -> RudeResult> { + let mut statement = try_log!( + self.0 + .prepare("select salt, hash from passwords where player = ?"), + "Unable to prepare sql statement" + ); + + let mut rows = try_log!(statement.query(params![id]), "Unable to perform query"); + + if let Some(row) = try_log!(rows.next(), "Unable to retrieve row") { + Ok(Some(try_log!( + Password::try_from(row), + "Unable to get Password from Row" + ))) + } else { + Ok(None) + } + } + + /// Save a password + pub fn save_password(&self, id: Id, password: &Password) -> RudeResult<()> { + let mut statement = try_log!( + self.0.prepare( + "insert into passwords (player, salt, hash) values (?, ?, ?) on conflict(player) do update set player=?, salt=?, hash=?;" + ), + "Unable to prepare statement" + ); + + let _ = try_log!( + statement.execute(params![ + id, + password.salt, + password.hash, + id, + password.salt, + password.hash, + ]), + "Unable to perform query" + ); + + Ok(()) + } +} diff --git a/src/database/players.rs b/src/database/players.rs index aa70819..8d6b86d 100644 --- a/src/database/players.rs +++ b/src/database/players.rs @@ -13,7 +13,7 @@ impl Db { pub fn _load_player(&self, id: Id) -> RudeResult> { let mut statement = try_log!( self.0 - .prepare("select id, name, password, created, location from players where id = ?"), + .prepare("select id, name, created, location from players where id = ?"), "Unable to prepare sql statement" ); @@ -34,9 +34,8 @@ impl Db { /// Find a player by the name pub fn find_player_by_name>(&self, name: S) -> RudeResult> { let mut statement = try_log!( - self.0.prepare( - "select id, name, password, created, location from players where name = ?" - ), + self.0 + .prepare("select id, name, created, location from players where name = ?"), "Unable to prepare sql statement" ); @@ -61,7 +60,7 @@ impl Db { pub fn save_player(&self, player: &Player) -> RudeResult<()> { let mut statement = try_log!( self.0.prepare( - "insert into players (id, name, password, created, location) values (?, ?, ?, ?, ?) on conflict(id) do update set id=?, name=?, password=?, created=?, location=?;" + "insert into players (id, name, created, location) values (?, ?, ?, ?) on conflict(id) do update set id=?, name=?, created=?, location=?;" ), "Unable to prepare statement" ); @@ -70,12 +69,10 @@ impl Db { statement.execute(params![ player.id, player.name, - player.password, player.created, player.location, player.id, player.name, - player.password, player.created, player.location, ]), diff --git a/src/game/state/login.rs b/src/game/state/login.rs index 37d449c..08dd302 100644 --- a/src/game/state/login.rs +++ b/src/game/state/login.rs @@ -3,6 +3,7 @@ use mio::Token; use crate::command::Command; use crate::game::{Game, PlayerCheck}; +use crate::password::Password; use crate::player::Player; use crate::queue::SendQueue; use crate::state::*; @@ -87,7 +88,7 @@ impl Game { false, Some(State::Login(Login::CreatePassword2(( username.to_owned(), - pass, + Password::new(pass), )))), ); } @@ -103,8 +104,8 @@ impl Game { } Login::CreatePassword2((username, pass)) => { - let pass = pass.to_owned(); - if message.is_empty() || message != pass { + if !pass.check(message) { + send_queue.push(token, "\n\nPasswords don't match", false, None); send_queue.push( token, "\n\nUsername: ", @@ -116,12 +117,13 @@ impl Game { let player = Player { id, name: username.clone(), - password: pass, created: Utc::now(), location: self.config.player.starting_location.clone(), }; - if self.db.single_save_player(token, &player).is_ok() { + if self.db.single_save_player(token, &player).is_ok() + && self.db.save_password(player.id, pass).is_ok() + { send_queue.push( token, format!("Welcome, {}\n", username), @@ -155,24 +157,34 @@ impl Game { ); } else { match self.db.find_player_by_name(username) { - Ok(Some(player)) => { - if message == player.password { - if self.db.save_connected_player(token, &player).is_ok() { - send_queue.push( - token, - format!("Welcome back, {}\n\n", username), - false, - Some(State::Action), - ); + Ok(Some(player)) => match self.db.get_password(player.id) { + Ok(Some(password)) => { + if password.check(message) { + if self.db.save_connected_player(token, &player).is_ok() { + send_queue.push( + token, + format!("Welcome back, {}\n\n", username), + false, + Some(State::Action), + ); - send_queue.append(&mut Command::dispatch_look( - &Command::default(), - String::new(), - token, - &mut self.db, - )); + send_queue.append(&mut Command::dispatch_look( + &Command::default(), + String::new(), + token, + &mut self.db, + )); + } else { + send_queue.push(token, "Unable to login\n", false, None); + send_queue.push( + token, + "\n\nUsername: ", + false, + Some(State::Login(Login::Username)), + ); + } } else { - send_queue.push(token, "Unable to login\n", false, None); + send_queue.push(token, "Incorrect password\n", false, None); send_queue.push( token, "\n\nUsername: ", @@ -180,8 +192,10 @@ impl Game { Some(State::Login(Login::Username)), ); } - } else { - send_queue.push(token, "Incorrect password\n", false, None); + } + Ok(None) => { + log::error!("Player has no password: {}", player.id); + send_queue.push(token, "Error\n", false, None); send_queue.push( token, "\n\nUsername: ", @@ -189,7 +203,17 @@ impl Game { Some(State::Login(Login::Username)), ); } - } + Err(e) => { + log::error!("Database error: {}", e); + send_queue.push(token, "Error\n", false, None); + send_queue.push( + token, + "\n\nUsername: ", + false, + Some(State::Login(Login::Username)), + ); + } + }, Ok(None) | Err(_) => { send_queue.push(token, "Error\n", false, None); send_queue.push( diff --git a/src/lib.rs b/src/lib.rs index 93fc3a4..8d80e8f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ pub mod logger; #[macro_use] #[doc(hidden)] pub mod macros; +pub mod password; pub mod player; pub mod queue; pub mod result; diff --git a/src/main.rs b/src/main.rs index af2aea9..612cddc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ mod id; mod logger; #[macro_use] mod macros; +mod password; mod player; mod queue; mod result; diff --git a/src/password.rs b/src/password.rs new file mode 100644 index 0000000..d76a38f --- /dev/null +++ b/src/password.rs @@ -0,0 +1,55 @@ +//! Player information + +use std::cmp::PartialEq; +use std::convert::TryFrom; +use std::error::Error; + +use rusqlite::Row; +use serde_derive::{Deserialize, Serialize}; + +use crate::hash::{hash, salt}; + +/// Containing object for the password +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct Password { + /// Salt + pub salt: String, + + /// Salted and hashed password + pub hash: String, +} + +impl Password { + /// Create a new password object from a string password. + pub fn new>(s: S) -> Self { + let salt = salt(); + let hash = hash(s, &salt); + + Self { salt, hash } + } + + /// Check the password against the provided password. + pub fn check>(&self, s: S) -> bool { + let s = s.into(); + let hash = hash(s, &self.salt); + + self.hash == hash + } +} + +impl<'a> TryFrom<&Row<'a>> for Password { + type Error = Box; + + fn try_from(row: &Row) -> Result { + Ok(Self { + salt: try_log!(row.get("salt"), "salt"), + hash: try_log!(row.get("hash"), "hash"), + }) + } +} + +impl PartialEq for Password { + fn eq(&self, other: &Self) -> bool { + self.hash == other.hash + } +} diff --git a/src/player.rs b/src/player.rs index fa0611d..49bf4be 100644 --- a/src/player.rs +++ b/src/player.rs @@ -18,9 +18,6 @@ pub struct Player { /// Player name pub name: String, - /// Player's password (this needs to be properly salted and hashed but isn't) - pub password: String, - /// Creation DateTime pub created: DateTime, @@ -35,7 +32,6 @@ impl<'a> TryFrom<&Row<'a>> for Player { Ok(Self { id: try_log!(row.get("id"), "id"), name: try_log!(row.get("name"), "name"), - password: try_log!(row.get("password"), "password"), created: try_log!(row.get("created"), "created"), location: try_log!(row.get("location"), "location"), }) diff --git a/src/state.rs b/src/state.rs index d6036d3..4be2274 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,5 +1,7 @@ //! State information for a connected `Client`. +use crate::password::Password; + /// Play state for a conected `Client`. #[derive(Clone, Debug, PartialEq)] pub enum State { @@ -26,7 +28,7 @@ pub enum Login { CreatePassword(String), /// New user password again - CreatePassword2((String, String)), + CreatePassword2((String, Password)), /// Password for existing user Password(String),