0

Hi have been boggling my mind about this one for hours, I am sure it would be easily done in MSSQL but MySQL for some reason does not accept an insert inside an ifstatement.

my pseudo code

  • If user exists
  • Add successfull login to logins table and return the relationnumber
  • if it does not exist add a fail to the logins table and check if this is the 3th (or more) time. if it is block the username.

This is what my code turned into:

SET @maxtries = 3; 
SET @maxBlockTime = 15; 
SET @username = 'test';
SET @ipAddress = '0.0.0.0';
SET @salt = 'salt';
SET @passwordHash = 'pswhsh';
SET @numOfUsernameFails = 0;
SET @relationNumber = NULL;
SET @emailConfirmed = NULL;
SET @foundStatus = 'Failed';

SELECT @relationNumber := `RelationNumber`, @emailConfirmed := `EmailConfirmed` 
    FROM accounts 
    WHERE 
    Username=@username AND 
    PasswordHash=@passwordHash AND 
    Salt=@salt;

SET @message := IF(ISNULL(@relationNumber), 'UnkownUsername', IF(@emailConfirmed = 0, 'NotConfirmed', 'Success'));

SET @foundStatus := IF(@message != 'Success', 'Failed', 'Succeeded');

INSERT INTO logins (`Username`, `Date`, `Status`, `IPAddress`) VALUES (@username, UTC_TIMESTAMP(), @foundStatus, @ipAddress);

-- if status is failed, check the number of times it has failed
-- if this is > @maxtries add to blocked table (sp_add_blocked)
-- if this is not, return table with message(s)

-- if status = success return table with username & relationnumber

SET @t := IF(@foundStatus = 'Failed'
, IF((SELECT COUNT(*) FROM (SELECT * FROM `logins` WHERE `Username`= @username order by `Date` DESC LIMIT 0, 3) `l` WHERE `l`.`Status` = 'Failed' AND (TIME_TO_SEC(TIMEDIFF(UTC_TIMESTAMP(), DATE)) / 60) < 15) >= 3, 
         'Blocked', 
         @message)
, @relationNumber);

SELECT IF(@t = 'Blocked',
     'insert into blocked stored procedure and return blocked as message',
    'return message unkown user');
Jean-Paul
  • 380
  • 2
  • 9
  • 26
  • 1
    Why are you putting this much code in the database? This is an application-level concern. Trying to make the database act like an application layer is usually counter-productive. That password hashing is also likely super scary since MySQL does **not** have a proper hashing function for these things. You need to use bcrypt at the absolute least these days. – tadman Jan 25 '17 at 16:02
  • The hashing of the password has been done in the application, the rest is simply checking if the user login succeeded (where a = a and b = b) and if it failed insert a new row, count the rows and return blocked or not. I have done this in the application but than it will fire 5-6 queries, and would take seconds to return with my answer. – Jean-Paul Jan 25 '17 at 16:05
  • Seeing "salt" as a column is a sign something is not right here, that's a strategy that died out in the 1990s because it was proven to be completely inadequate. Everything about this code is red flags. Do your authentication in the application using data from the user table. Insert any login or login failed records from the application layer. Initiating all of this in a stored procedure is really asking a lot of MySQL. – tadman Jan 25 '17 at 16:07
  • Why is Salt inadequate? I used this article for reference: https://crackstation.net/hashing-security.htm what you are saying is that the whole article is a load of bullshit? and my study upon the matter has been completely wrong? I do inserts and gets in the stored procedure, I do the calculation of the Hash and Salt on the server before I send it to my webservice. My database manager told me to place all the inserts, gets and updates in a stored procedure so that is what I did. – Jean-Paul Jan 25 '17 at 16:16
  • Also more people who do it; http://stackoverflow.com/questions/11689740/best-login-practice and http://stackoverflow.com/questions/2138429/hash-and-salt-passwords-in-c-sharp?noredirect=1&lq=1 and http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords?rq=1 i'd like to see some articles why salt isn't 2017 because i have not found them – Jean-Paul Jan 25 '17 at 16:22
  • I'm speaking specifically to storing the password hash and salt separately. Usually that's a sign that you're using something weak like SHA1 or MD5 because a modern function like "PBKDF2, bcrypt, and scrypt", as referenced in that Crackstation article, will produce a unified value, there is no separate salt. Look at the DO NOT section carefully. I would **strongly** recommend using those functions, and the values they produce look like `$2a$04$OtbHRI6YZC1PYJrxM3v9hOEkrxffmg4RjD82Vl4YG5eGLqu.o9ZBu` There's generally no reliable or meaningful way to extract the salt component of that. – tadman Jan 25 '17 at 16:54
  • I believe the separation is used for hashing the password on the server, you get the salt from the database has the password and than send the hashed password with the web service to the database, I do not understand how to otherwise encrypted the passwords. – Jean-Paul Jan 25 '17 at 17:20
  • Use **PBKDF2, bcrypt, or scrypt** and none of these are available on MySQL. If you're "hashing the password on the server" this is bad. This would fail a security audit and is a huge liability for you should this database ever leak. – tadman Jan 25 '17 at 17:22
  • The PHP krewe worked out a crook-resistant and future-resistant setup for password hashing. http://php.net/manual/en/faq.passwords.php To use it, you retrieve the stored password hash text. You then use it to hash the plaintext password presented to you in the `password_verify()` function. If that function says "ok", you grant access. That's the 2017 gold standard. Your mileage may vary, but it probably doesn't. I know this may not be a php question. But the php documentation on this topic is still valuable. – O. Jones Jan 25 '17 at 17:22
  • @O.Jones It's not clear if this is a PHP question, but that would be *significantly* better than the code presented here. `password_hash` is "good enough" for most applications and can be tweaked to be even more secure with some simple options. – tadman Jan 25 '17 at 17:23
  • 1
    Most developers using MySQL avoid using stored procedures whenever possible; they're hard to debug and, as you've probably figured out by now, poorly documented. – O. Jones Jan 25 '17 at 17:23
  • I think I figured out what's insecure here. Each user needs her own random salt. It shouldn't be possible, under any circumstances, to know the salt well enough to say `WHERE user.salt = @salt` in a query. The salt needs to be retrieved along with the hashed password and then used in something exceeding 10^5 rounds of `bcrypt()` to validate the presented password. – O. Jones Jan 25 '17 at 17:39
  • @tadman I am not hasing my passwords on the database server, I hash them in the application server. I would love to go for PBKDF2, bcrypt, or scrypt but as you said I cannot because I use MySQL and my boss has no desire to setup a MSSQL server just for this. – Jean-Paul Jan 25 '17 at 18:18
  • @O.Jones unfortunatly I use ASP.NET C# for this application, otherwise I would have used the PHP version of hashing :) Also I used to avoid Stored Procedure with MySQL as well, because of all the problems I get when just trying to declare a dynamic variable (dynamic as in set or not set). But now my database manager has showed me how to do it properly and it works like a charm, testing is still a problem tho. I do generate a unique salt for every user andeverytime a user requests a new password I will regenerate the salt.if for some reason we get hacked, I will also, send password reset links. – Jean-Paul Jan 25 '17 at 18:22
  • 1
    Using ASP.NET is fine. There's an easy solution there: [Install support for bcrypt and scrypt](https://blog.decayingcode.com/post/protecting-your-asp.net-identity-passwords-with-bcrypt-and-scrypt/). The random number generation facilities in MySQL **are not cryptographically secure** and should be avoided. Please, please **use a proper password hashing algorithm and do not do this in the database**. If/when you get hacked force-resetting passwords does nothing to protect people from the damage of improperly hashed passwords being leaked. It's too late. – tadman Jan 25 '17 at 18:36

0 Answers0