First-name, Last-name and Password logging in functionality using C#

Question

Summary

I have finished creating a secure login functionality. I have used 1 resource to help me with the security. The security section was copy and pasted but implemented into a class to suit it in my own way, but all the logic and properties, commands etc.. is all my work. https://medium.com/@mehanix/lets-talk-security-salted-password-hashing-in-c-5460be5c3aae

Would appreciate any helpful review to my current code I have, from the community in areas where I can improve.

Current Code

SecurePasswordHasher.cs

At the moment I do not use public static string Hash(string password) anywhere, the focus is at the verification.

public static class SecurePasswordHasher
{
 public static string Hash(string password)
 {
 // Create salt
 byte[] salt;
 new RNGCryptoServiceProvider().GetBytes(salt = new byte[16]);
 // Create hash
 var pbkdf2 = new Rfc2898DeriveBytes(password, salt, 10000);
 byte[] hash = pbkdf2.GetBytes(20);
 byte[] hashBytes = new byte[36];
 Array.Copy(salt, 0, hashBytes, 0, 16);
 Array.Copy(hash, 0, hashBytes, 16, 20);
 return Convert.ToBase64String(hashBytes);
 }
 public static bool Verify(string savedPassword, string givenPassword)
 {
 byte[] hashBytes = Convert.FromBase64String(savedPassword);
 byte[] salt = new byte[16];
 Array.Copy(hashBytes, 0, salt, 0, 16);
 var pbkdf2 = new Rfc2898DeriveBytes(givenPassword, salt, 10000);
 byte[] hash = pbkdf2.GetBytes(20);
 int ok = 1;
 for (int i = 0; i < 20; i++)
 {
 if (hashBytes[i + 16] != hash[i])
 {
 ok = 0;
 }
 }
 if (ok == 1)
 {
 return true;
 }
 else
 { 
 return false;
 }
 }
}

LoginViewModel

public class LoginViewModel : BaseViewModel
{
 
 // User Properties
 private UserModel _User;
 public UserModel User
 {
 get { return _User; }
 set
 {
 _User = value;
 OnPropertyChanged(nameof(User));
 }
 }
 private string _FirstName;
 public string FirstName
 {
 get { return _FirstName; }
 set
 {
 _FirstName = value;
 User = new UserModel
 {
 FirstName = _FirstName,
 LastName = this.LastName,
 Password = this.Password
 };
 OnPropertyChanged(nameof(FirstName));
 }
 }
 private string _LastName;
 public string LastName
 {
 get { return _LastName; }
 set
 {
 _LastName = value;
 User = new UserModel
 {
 LastName = _LastName,
 FirstName = this.FirstName,
 Password = this.Password
 };
 OnPropertyChanged(nameof(LastName));
 }
 }
 private string _Password;
 public string Password
 {
 get { return _Password; }
 set
 {
 _Password = value;
 User = new UserModel
 {
 FirstName = this.FirstName,
 LastName = this.LastName,
 Password = _Password
 };
 OnPropertyChanged(nameof(Password));
 }
 }
 // Login Command
 public ICommand LoginCommand { get; set; }
 
 // Function
 private bool LoginFunction(object param)
 {
 // Get user credentials
 UserModel user = param as UserModel;
 
 
 // Veryify credentials have data to work with
 // Also, reason this method return a bool, because it prevents NullReferenceException.
 if (user == null)
 {
 MessageBox.ShowMessageBox("Credentials not specified");
 return false;
 }
 else if (user.FirstName == null || string.IsNullOrEmpty(user.FirstName))
 {
 MessageBox.ShowMessageBox("Firstname cannot be empty");
 return false;
 }
 else if (user.LastName == null || string.IsNullOrEmpty(user.LastName))
 {
 MessageBox.ShowMessageBox("Surname field cannot be empty");
 return false;
 }
 else if (user.Password == null || string.IsNullOrEmpty(user.Password))
 {
 MessageBox.ShowMessageBox("Password field cannot be empty");
 return false;
 }
 // Find Firstname and Surename in the database
 else
 {
 using (var conn = new MySqlConnection(ConnectionString.ConnString))
 {
 conn.Open();
 
 string query = @"SELECT
 *
 FROM USERS u
 JOIN DEPARTMENT d
 on d.id = u.departmentid
 JOIN usergroup ug
 ON ug.id = u.UserGroupID
 WHERE FirstName = @Firstname AND Lastname = @LastName";
 
 var userDetails = conn.Query<UserModel>(query, new { FirstName = user.FirstName, LastName = user.LastName}).ToList();
 
 // If user is found, proceed to verification
 if(userDetails.Count() == 1)
 {
 // Get password from the user
 string savedPasswordHash = userDetails.First().Password;
 
 bool verification = SecurePasswordHasher.Verify(savedPasswordHash, Password);
 
 if (verification == true)
 {
 // Store Username
 UserData.FullName = $"{user.FirstName} {user.LastName}";
 
 ShowDashboard.ShowDashboard();
 return true;
 }
 // Password is incorrect
 else
 {
 MessageBox.ShowMessageBox("User not found");
 return false;
 }
 }
 // User with the given Firstname and surename is not found
 else
 {
 MessageBox.ShowMessageBox("User not found");
 return false;
 }
 }
 }
 }
 // Messagebox Interface
 public IMessageBoxService MessageBox { get; set; }
 // Open Dashboard Interface
 public IShowDashboardService ShowDashboard { get; set; }
 public LoginViewModel(IMessageBoxService messageBox, IShowDashboardService showDashboard)
 {
 this.MessageBox = messageBox;
 this.ShowDashboard = showDashboard;
 LoginCommand = new RelayCommand(param => LoginFunction(param));
 }
}

• Peter Csala: If Most of it was just copy and paste then what should be reviewed?
• DeveloperLV: @BCdotWEB Altered.

Answer 1

Storing password in string is totally insecure.

Because string is immutable object and can be kept in memory for undefined period of time. This makes easy to get clean password through not complicated reverse engineering operation.

By the way

The focus is at the verification.

Ok

Renamed savedPassword because it's a hash not clean password.

public static bool Verify(string savedPasswordHash, string givenPassword)
{
 byte[] hashBytes = Convert.FromBase64String(savedPasswordHash);
 byte[] salt = hashBytes.Take(16).ToArray();
 byte[] hash = new Rfc2898DeriveBytes(givenPassword, salt, 10000).GetBytes(20);
 
 return hashBytes.Skip(16).SequenceEqual(hash);
}

That's it. Easy Linq query.

• aepot: @LV98 Here's my detailed Russian StackOverflow answer how to securely use PasswordBox in WPF.
• DeveloperLV: I'm just wondering.. is it worth to have the security if the application is only used internally.
• aepot: @LV98 at least it's worth to learn security.
• DeveloperLV: Agreed.. Any suggestions where I can start?