2

I got to thinking that maybe my login system isn't as secure as I thought it was. So first, I'm going to explain to you, in words, what I am doing.

When a user registers, a 16 character salt is generated. I store the salt in the database in a field called "salt" I store the hashed password+salt (they are hashed together hash("sha256", $salt.$password);) in a field called "password"

When a user attempts to log in, I fetch the "password" field and the "salt" field from the database, along with a few other things.

To check if they entered their password correctly, I do this:

$hashed = hash("sha256", $row['salt'].$pass);
if ($row['password'] == $hashed) {
//success

($row is the fetched array from the database. $row['salt'] is the salt in the database, $pass is the password they entered, and $row["password"] is the hashed pass+salt in the database)

I was thinking, and it looks to me that my salt offers little (or no) security benefits at all. My question to you all is just that: DOES my method offer additional security (or is it even secure it all?)

In addition, I have a second 'question.' I want to verify that this "check login" script can't be spoofed/cheated in order to gain entry to someone's account without their password.

session_start();
require_once 'db_connect.php';
//If the session variable "id" isn't set (i.e. they aren't logged in)
if (!isset($_SESSION['id'])) { 
    //Check if they wanted to be "remembered" (so they have 2 cookies
    if (isset($_COOKIE['rem_user']) && isset($_COOKIE['rem_pass'])) 
    { 
        $query = "SELECT 
                      id, 
                      password, 
                      auth, 
                      email, 
                      username 
                  FROM users 
                  WHERE 
                      username='".$_COOKIE['rem_user']."' 
                  AND active IS NULL"
        $res = mysql_query( $query );
        if (mysql_num_rows($res) == 1) 
        {
            $row = mysql_fetch_array($res);
            // If the "remember me" cookie containing their password 
            // is equal to the one in the database, log them back in.
            if ($_COOKIE['rem_pass'] == $row['password']) 
            { 
                $_SESSION['id'] = $row['id'];
                $_SESSION['username'] = $row['username'];
                $_SESSION['auth'] = $row['auth'];
                $_SESSION['email'] = $row['email'];
                $logged_in = 1;
            }   
        }
    } 
    else 
        $logged_in = 0;
} 
else 
    //Since the session variable "id" WAS set, they ARE logged in. 
    $logged_in = 1;

I would think that the only way to log in is...

  1. To spoof a session variable which I don't think is possible without server access
  2. Spoof a cookie with the encrypted password+salt, which I believe is nearly impossible without access to the database.

Feedback would be appreciated. I want to make sure my system is secure. :)

Thank you!

tereško
  • 58,060
  • 25
  • 98
  • 150
Jacob Brunson
  • 1,482
  • 4
  • 23
  • 37
  • If you are doing it yourself: no, it is *not* right :) Okay, that was a bit snarky, but this very same question keeps coming up in different forms and showcases many problems with trying to roll-your-own-security. –  Apr 02 '12 at 23:55
  • 3
    The salt offers you security in the sense that if an attacker gained direct access to your database, they wouldn't be able to use a pre-generated rainbow table (or similar) to find collisions against the password hashes. – Oliver Charlesworth Apr 02 '12 at 23:55
  • pst, How so? What would be better? o3o @Oli Charlesworth Ok, that makes sense, but is that what a salt is SUPPOSED to do? I always thought there would be more security than just that. – Jacob Brunson Apr 02 '12 at 23:57
  • @DrAgonmoray: That's basically it. See http://en.wikipedia.org/wiki/Salt_(cryptography). – Oliver Charlesworth Apr 02 '12 at 23:59
  • You're using PHP, which conveniently comes with blowfish. Using that would make your life easier, and your project more secure. Reference: http://php.net/manual/en/function.crypt.php –  Apr 02 '12 at 23:59
  • See also http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php (for `$_COOKIE` in this case) – John Carter Apr 03 '12 at 00:01
  • Please investigate Schneier's Law and take it to heart. @pst 's response is a good start, but even I can see that he or she didn't catch a few vulnerabilities. I could add to the list, but I'd only be falling victim to Schneier's Law as well. I can make it better, but I still can't get it right. The only way you have a hope of getting security right is use a third party system. – Kennet Belenky Apr 03 '12 at 00:10

3 Answers3

6

Okay, here it goes... do not roll-your-own security. Here are some issues with above:

  1. The hashed password is stored as a cookie. THIS IS NO DIFFERENT THEN STORING/PASSING A PLAIN-TEXT PASSWORD because it is validated as-is without the hash function function applied.

  2. The cookie is an SQL Injection Attack vector.

  3. Uses SHx for hashing. (Use bcrypt, scrypt, hmac, etc.)

And then I stopped looking. #1 shows that this should be left to an existing tested/vetted library.

  • 1. The cookie is only stored if the user checks "Remember Me," I figure that they would be using it 'at their own risk.' Is this incorrect? 2. Ok, good catch. :D 3. What's wrong with sha? – Jacob Brunson Apr 03 '12 at 00:01
  • 2
    To expand on #1, if I were to sniff a session between one of your users and your site, and they had selected the remember me cookie, I now have their username and hashed password so I can just construct an identical cookie and be automatically logged in as them. Beyond that is has no security checks to make sure the person with the valid cookie was in fact the owner of the original cookie. – drew010 Apr 03 '12 at 00:02
  • 2
    For remember me functionality, you should not store their password as a cookie. Instead, generate a GUID and save this as a cookie + add it to a database. When the user explicitly logs out, delete the GUID from your database. Further this allows you to enable a user to log out of other sessions he might be logged into by deleting the corresponding auth tokens – Ayush Apr 03 '12 at 00:03
  • 1
    `The cookie is only stored if the user checks "Remember Me," I figure that they would be using it 'at their own risk.` -- if you trust your users to do the right thing at all times, you don't need passwords at all. – mah Apr 03 '12 at 00:03
  • @mah I wasn't aware that you could do what drew010 was saying. I thought the only way for anyone to steal the cookie was to use their computer. O_O – Jacob Brunson Apr 03 '12 at 00:05
  • Ok, so I think I understand that everything I thought I knew is a lie. So here it goes: What security system SHOULD I be using? – Jacob Brunson Apr 03 '12 at 00:12
  • @DrAgonmoray Using SSL (HTTPS) can mitigate (but not eliminate all) cookie sniffing; anything that can be view the network traffic including proxies (or get access to the cookie store, including malicious JavaScript that was injected!) can observe cookies. –  Apr 03 '12 at 00:12
  • @DrAgonmoray New post :-) (After doing some initial research.) –  Apr 03 '12 at 00:13
1

Use hash_hmac

I use it like this: hash_hmac($algo, $password.$salt, $siteKey);

Even if an attacker got into your database, they would need your sitekey in order to brute force succesfully, though I won't comment on collisions. Choose your algo/poison. :D

Norm
  • 583
  • 5
  • 15
  • Chances are if a hacker can access they can access the codebase where $siteKey is located. – Mike Purcell Apr 03 '12 at 00:02
  • 1
    @MikePurcell The idea with a siteKey is -- hopefully -- it is stored in a *separate* "hardened" location (e.g. *not* the code and *not* the database). I am not sure what support there is is in PHP, but in ASP.NET it would be from the encrypted portion of the web.config, for instance. Moving the code that invokes the `hash_hmac` (and thus has access to `$siteKey`) to a "hardened" location/library might reduce tampering in PHP (I don't use PHP). In any case, I wish HMAC was more employed as an *additional* measure because it's effectively a "free" operation. –  Apr 03 '12 at 00:09
  • @pst: Interesting, $siteKey makes sense in that scenario, and I assume it is common practice? – Mike Purcell Apr 03 '12 at 00:11
  • Sure, if they can get into your codebase, then you might as well give up this line of work. :) – Norm Apr 03 '12 at 00:11
  • @NormanOvenseri Assuming they can get to the *running context* of this `hash_hmac` call (e.g. get the *value* of the `$siteKey`) or they can get to the *stored value* of the key. This is not the same as "being able to view" the source. The first could be done with being able to modify the source (which is why I recommend it located outside of the main codebase in a "hardened" location, perhaps not even PHP) with an `echo $siteKey`, for instance. I am not sure what security providers there are for PHP: e.g. the entire HMAC/hash might be handle externally in the security provider. –  Apr 03 '12 at 00:16
  • @pst Agreed. That will take dedication, but surely there are people with that dedication. – Norm Apr 03 '12 at 00:19
1

Does salting give you additional security? Yes. If hash is compromised, you need way more resources to crack the password because salt adds a lot of combinations. So now you can't use old traditional rainbow table attack. Where to store the salt is different question. How usually the hash will be retrieved by the hacker? By compromising the database. If database is compromised then he will also have the salt which makes salting less efficient. But if salt is hardcoded in script, compromising database won't have an effect. So I would strongly consider hardcoding salt in script or using two salts - hardcoded or in database.

Second question. Don't store password in the cookies. You can fake them very easily (even in chrome advanced settings). Or to steal them (e.g. XSS vulnerability). Store them in session.

Gediminas
  • 854
  • 8
  • 14
  • 2
    If the same salt is used for each password, then it is less secure because if the salt is also compromised I can run one brute force attack with the salt to find ALL user passwords rather than ONE user password. Therefore, you should use a unique salt for each password/user. Otherwise I think the answer was well stated and explained. – drew010 Apr 03 '12 at 00:04
  • (And seconding what drew010 said: the salt should be a *large* random value. The OP indicates it is a [random] 16-character salt, per new entry, which ought to be sufficient. But a larger salt is cheap.) –  Apr 03 '12 at 00:07
  • @drew010 So the salt should use an algorithm that creates a unique (or near unique) salt for every user, and this salt should NOT be stored in the database, but rather 'calculated' every time I am checking the password? – Jacob Brunson Apr 03 '12 at 00:07
  • @DrAgonmoray It is fine to store it in the database. As long as each one is different, one would have to brute force each password individually along with the salt. This also assumes they KNOW how you salt the password (e.g. `$salt . $password`, `$password . $salt`, `substr($salt, 0, 8) . $password . substr($salt, 8, 8)`. If each salt is unique, and I want to crack jonhdoe's password, I have to run a brute force on all possible passwords plus HIS salt. If each person's salt was the same, I just run a brute force on a password + salt and check ALL passwords in database for a match. – drew010 Apr 03 '12 at 00:16
  • I've seen cracking programs that support salt cracking and they let you specify various ways to hash the password and salt, or use multiple ways at once. If you use a bigger salt, it is cheap for you but makes cracking take longer over the long, long run as the hash algorithm takes slightly longer if the input is longer. – drew010 Apr 03 '12 at 00:18
  • @drew010 So `hash("sha256", $row['salt'].$pass);` needs to be made more complicated, and the salt needs to be made longer. Got it. I've started Googling to try and research my failed system more, but I'm not entirely sure what I should be searching for. Any advice? – Jacob Brunson Apr 03 '12 at 00:22
  • @DrAgonmoray It doesn't **have** to be more complicated, that method still requires one to do a full range brute force so its pretty sufficient. But you could REALLY piss off your adversary if after 5 years of brute forcing they still don't have a password because they aren't applying the salt correctly in the hash :) I didn't read much of it, but the Wiki article on [Salt (cryptography)](http://en.wikipedia.org/wiki/Salt_%28cryptography%29) may be of help. – drew010 Apr 03 '12 at 00:26
  • @DrAgonmoray This is very helpful reference for you as well I think: https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet – drew010 Apr 03 '12 at 00:28
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/9620/discussion-between-dragonmoray-and-drew010) – Jacob Brunson Apr 03 '12 at 01:22