0

Should I validate a username and pass word by searching for both in the SQL table Or Should I find the username then match the pass word with a PHP if statement?

  1. SELECT * FROM table WHERE username = $username AND password =$password

  2. SELECT * FROM table WHERE username = $username ...

    if ($row[password] == $password) { do stuff }

Which method Is more secure and efficient?

Gilad Green
  • 36,708
  • 7
  • 61
  • 95
Ideen
  • 47
  • 1
  • 7
  • 2
    Neither. Usually use a function eg if (valid_password($username, $password)) {...} Then you can do a lot as how to validate a user's credentials in that function – SIDU Aug 05 '16 at 06:35
  • 4
    Search for the login only, validate the password in php. And use prepared statements. You shouldn't pass even the encrypted password on the network ( your encryption methods can be tested if network traffic can be seen ), and if your query returns a result your searching not validating the password. Should you query be compromised it will login. such as this `$username = "; Select 1 as row from username; --` would return `row=>1` and because you are likely checking for a result ( num_rows == 1 or such ) and not checking the password, it will pass the if condition. – ArtisticPhoenix Aug 05 '16 at 06:37
  • http://stackoverflow.com/questions/401656/secure-hash-and-salt-for-php-passwords?rq=1 – SIDU Aug 05 '16 at 06:38
  • 1
    "You shouldn't pass even the encrypted password on the network ( your encryption methods can be tested if network traffic can be seen )". What network? Many people have dedicated database servers and webservers meaning it will go over the network. Also what does " your encryption methods can be tested" even mean? – PeeHaa Aug 05 '16 at 07:46
  • **WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.2/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and never store passwords as plain-text. – tadman Aug 05 '16 at 07:54
  • @PeeHaa Well, if someone *could* man-in-the-middle your web server ⟷ database connection, then yeah, you're pretty much up the creek. So, yes, there is a point there, but no, this can't and should not be mitigated at the application level we're talking about here… – deceze Aug 05 '16 at 07:55
  • @tadman Agreed, but since we're developers here who develop such systems, we can't ignore the question by deferring to existing implementations… Newbies should definitely use existing implementations if they're using a framework anyway though. – deceze Aug 05 '16 at 07:57
  • You know I know that and I never said otherwise. His point was to never send it over the network which doens't make much sense. – PeeHaa Aug 05 '16 at 07:57
  • Sure, just extending your point for others benefit. – deceze Aug 05 '16 at 07:58
  • @deceze I've spent decades writing web applications and I *still* wouldn't trust myself to get this sort of thing right. There's so many ways you can fail and all of them can cost you huge. Not only is there SQL injection concerns, but XSS and CSRF are also major problems, things any decent framework protects you from if used correctly. On top of all that, many pre-existing login systems make adding OAuth (Twitter login, etc.) super easy, whereas your home-rolled version is an uphill battle every step of the way. – tadman Aug 05 '16 at 08:00
  • 2
    @ArtisiticPhoenix If network sniffing is a concern, which in some environments it can be, then you **must** use MySQL with TLS either directly or over some kind of bridge like a VPN or SSH. There's no way to not convey the hashed passwords to PHP for verification, this cannot all be done on the MySQL server side. Either you're sending plain-text passwords to the database for validation (super bad) or you're sending hashed passwords back to PHP (bad). – tadman Aug 05 '16 at 08:03
  • @tadman - I was specifically speaking of sending the hashed password to the DB, returning it is less of a concern because you cant use that to tell information about the encryption method, for example, I could submit a password ( I know ) inspect the hash that was sent, and compair that with the algorithm used to tell if and how it's salted, besides if it is salted you need to return the salt before you can encrypt it anyway. Obviously we wont send plain text passwords, and we'll use things like TLS on our form submission page, that's a given. – ArtisticPhoenix Aug 05 '16 at 08:12
  • 1
    Why would it even matter if an attacker knows "the encryption method, for example"? All good methods to do it are public either way. Security is not about hiding the method used. It's about making it infeasible / "impossible" even when the method is known. – PeeHaa Aug 05 '16 at 08:13
  • 1
    Any sufficiently robust hashing method, like PHP's default `password_hash`, would reveal no useful information here. If you're able to access the MySQL queries over the wire you're going to have a field day with the rest of the data, and it's likely that you can brute-force your way into the MySQL server itself. I'd be far less worried about the password hash part than the other staggering implications. – tadman Aug 05 '16 at 08:13
  • No but it could reveal how its salted, for example `salt . password` `password . salt`. Given a known password, salt and encryption method ( sha256 for example ), you could then tell how the salt is added. By not sending the hashed password out of php, they can get the hash, salt and method, but the plaintext password is still unknown and therefor how the salt is added cannot be known. Additionally you are checking the hash in an if condition, and not checking if the query returns a result, its much easier to fake a result and pass the if condition, and avoid the encryption step. – ArtisticPhoenix Aug 05 '16 at 08:16
  • 1
    Stop using that word encryption. It makes no sense. And stop using a fast hashing method for passwords. Knowing the way passwords are hashed should never compromise security. If it is your are doing it wrong. – PeeHaa Aug 05 '16 at 08:18
  • for example in php `if($result->row_count == 1 )` vs `if( $hash == $result['hash'] )` – ArtisticPhoenix Aug 05 '16 at 08:19
  • 1
    @ArtisiticPhoenix The salt **is not secret**. It doesn't matter if an attacker sees the salt. It doesn't matter if an attacher knows the algorithm. The algorithm must be sound and solid **and slow** to make it infeasible to brute force a hash **even if all information is known.** – deceze Aug 05 '16 at 08:21
  • @deceze - obviously, what is not known is how that salt is applied to the password. Once you know that, ( given the algorithm, salt, hash, and method salt is applied ) you can brute force it. Without knowing how the salt is added that makes it nearly impossible. Brute forcing this ( password + salt ) is much different then this ( salt + password ) and this ( salt + password + salt ), understand. If you send the hash from a known password you can work that out, that is my point. – ArtisticPhoenix Aug 05 '16 at 08:23
  • @ArtisiticPhoenix Wrong. Again, if the algorithm is solid **and slow**, then it is infeasible to brute force it **even if you know _everything_ but the actual plaintext password.** The security stems from the computational infeasibility to brute force the hash in a useful amount of time (< 100s of years); not from the fact that anything is kept secret. – deceze Aug 05 '16 at 08:25
  • @deceze - granted - but why give them the chance. Why give away information on your cartographic method when you don't need to. Without knowing how the salt is applied to the password they cant even attempt to brute force it. – ArtisticPhoenix Aug 05 '16 at 08:28
  • 1
    Because rolling your own and thinking you are smart is a terrible thing to do. When correctly hashing an attacker gains nothing by knowing the method used. Thinking otherwise is scary at best. You are not smarter than your attackers. – PeeHaa Aug 05 '16 at 08:31
  • @ArtisiticPhoenix Sure, you don't have to voluntarily divulge information. But usually missing information provides so little extra security that it's a) not worth even talking about and b) can absolutely not be relied upon to provide any additional benefit. – The attacker has already compromised your system to obtain a bunch of hashed passwords, do you really think they'll be unable to obtain a bit of source code that tells them the hash algorithm, or to figure it out by trial and error given a known plaintext password and hash? – deceze Aug 05 '16 at 08:31
  • Don't get me wrong I'm not saying MD5 or some homebrew crap. Personally I do about 10 rounds of sha512, add salts to both sides of the password and add some random length junk to the end. But I work in a very secure industry. Trust me I am well versed in AES, TLS and PGP ( however if you know a good implementation of PGP in PHP im all ears ) in essence `( hash('sha512', $salt . $password . $salt)x 10 ) . $fudge` – ArtisticPhoenix Aug 05 '16 at 08:41
  • 1
    10 rounds of SHA512 + junk sounds super simple to brute force… bcrypt (high time cost) or scrypt (high space cost) is pretty much where it's at right now. These algorithms are very difficult to impossible to parallelise at the moment, which is where their security comes from. The SHA family of hashes has been optimised up the wazoo and is useless for password hashing purposes, especially with just 10 rounds of hashing (might as well be 1 round for the difference it makes). – deceze Aug 05 '16 at 08:46

2 Answers2

3

The thing is… You are supposed to store salted, hashed passwords in the database. Since these are individually salted per user/password, you cannot look them up directly with password = ?, because you don't know the salt and therefore cannot calculate the matching hash in advance. If you're doing this properly, you must fetch the user record by username first, then validate the password hash using the retrieved salt/hash. Pseudocode:

$user = fetch_from_database($_POST['username']);

if (!$user) {
    throw new Exception("User doesn't exist");
}

if (!password_verify($_POST['password'], $user['password_hash'])) {
    throw new Exception('Invalid password');
}

echo 'Welcome ', $user['name'];

See http://php.net/password_hash, http://php.net/password_verify.

Community
  • 1
  • 1
deceze
  • 510,633
  • 85
  • 743
  • 889
-1

From the 2 listed above, the second one is more secure, because first one is more tilted towards SQL injection.

SELECT * FROM table WHERE username = $username AND password =$password

In this code if the value of username and password entered is something like "a or ('a'='a')" the code will be modified to 

SELECT * FROM table WHERE username = a or ('a' = 'a') AND password = a or ('a' = 'a')

Which means a clear code for listing all your data.

Whereas in the second case , IF condition will consider the value as a single string only. So second is the best among the 2 u mentioned..

Hope this helps

Aravind Pillai
  • 739
  • 7
  • 21
  • Thank you. And other than the 2. Is there a better way? Someone mentioned functions... – Ideen Aug 05 '16 at 06:42
  • You can find a better answer here .. http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – Aravind Pillai Aug 05 '16 at 06:45
  • 1
    If you allow SQL injection, you have bigger issues anyway! – deceze Aug 05 '16 at 07:36
  • Although sqli is a real problem. Both solutions are at least vulnerable to timing attacks. And possibly don't use hashed passwords. – PeeHaa Aug 05 '16 at 07:59
  • @PeeHaa "Timing attacks?" What are you suggesting here? – tadman Aug 05 '16 at 08:07
  • 1
    Both mysql as well as php's string comparison using `=/==` are subject to timing attacks meaning the hash can be retrieved (if even hashed). what I am suggesting is using a constant time comparisons (which is what you get for free when using the `password_*` API.) Even better PHP loose comparison operator as used by OP is subject to type juggling for even more "fun". – PeeHaa Aug 05 '16 at 08:10