New to PHP logins and sessions; Is this safe enoug

2019-04-15 23:17发布

问题:

I have a classifieds website which I am creating a login system for...

In the code below, a form with "username" and "password" has been submitted to. Also a "remember_me" feature is available (Code is not tested yet):

else if($row['password']===$pass){
    session_start();
    $_SESSION['logged_in'] = '1';
    $remember_me = isset($_POST['remember']) ? $_POST['remember'] : '0';
    if($remember_me=='1'){

        $text = "SECRET_TEXT_AND_NUMBERS_HERE";
        $username= $row['username'];

        $salt1 = sha1($row['alt_username']);
        $salt2 = sha1($text);

        $cookie_value = $salt1.':'.$username.':'.sha1($row['alt_username'].$salt2.$salt1);

        setcookie("s_b", $cookie_value, time()+60*60*24*100, "/");

    }

}

Now, is this code a good start for a login page?

Also, an important follow-up question to all this, if users want to stay logged in, do I then set a $_SESSION variable like the one in the code, and just check if that is set in the beginning of all pages on the site?

 if(isset($_SESSION['logged_in'])) // Then user is logged in already

or do I check to see if the cookie created in the login page is set instead of checking the session?

回答1:

logging in is about security; security is always more difficult then it seems.

There are a couple of things that you could improve in your code. first:

the security for your password is in the strength of the hasing algorithm. You choose to use sha1 (better than md5, but could be improved by using sha256 or bCrypt if you use PHP version >= 5.3)

First
The salt you use is supposed to be a random value, stored alongside the hashed result. in other words, the value to store in your database is:

$salt = [some random string of predifend lenght]; // Let's say 16 characters in length
$storedValue = $salt . sha256($salt . $password);

you check the password:

if ($row['username'] == $_POST['username']  && substr($row['$storedValue'], 16) == sha256(substr($row['$storedValue'], 0, 16) . $_POST['password'])) {
    // login ok
} else {
    // login fail
}

(better yet)
Use a proven library for the password hashing stuff, take a look at: Portable PHP password hashing framework and try to use the CRYPT_BLOWFISH algorithm if at all popssible.

Second
You should only store the session key in the session cookie. all other information is stored on the server.
The session cookie is already send out by PHP's session_start() function call, so you do not have to worry about this anymore.

if you want to check the sessions lifetime, you should store this information in the session array:

$_SESSION['lastActivity'] =  time()+60*60*24*100;

Third
The remember me token is a 'password equivalent' so you should only store a hash of the token in your database, just treat it as a password, only this 'password' is not typed by the user, but read from the cookie.



回答2:

The whole point of a hash is that its non-reversible, so it's not really adding any value the way you've used for the remember me function. Stop pretending it does anything useful, and use a random token for the remember me (and log this in the database against the username) then, if you get a client presenting a remember me cookie without an authenticated session, you know where to look to find out who it is.

(this also allows a sensible approach to be applied where the user keeps moving to different machines - you might say keep the last 2 values - and flag when they try to remember me from a 3rd machine).

A 100 day timeout is rather long - maybe 30 days (with an automatic refresh might be more appropriate depending on the level of risk.