Safe Login using Sessions & Cookies

Question

I currently use sessions to log users into my site. Now, I want to add cookies into the mix so people can visit the site days later and not have to log in again.

I don't want to store passwords in cookies, or encrypt anything - so my solution is to store the username in the cookie, and create another cookie with a sort of login hash (specific to each login of each user). And then, at the top of each page, check for the cookie and a login session. If the cookie is present but the session isn't, find the username and log the user in.

So, this would happen after they log in successfully ($row['username'] is the username they typed in):

$_SESSION['username']=$row[username];
$_SESSION['user_id']=$row['id'];
$loginhash = substr(str_shuffle("0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"), 0, 32);
$number_of_days = 14;
$date_of_expiry = time() + 60 * 60 * 24 * $number_of_days ;
setcookie( "userlogin", $row['username'], $date_of_expiry, "/" ) ;
setcookie( "loginhash", $loginhash, $date_of_expiry, "/" ) ;

$today=date("Y-m-d");

mysql_query("update members set last_login='$today',loginhash='$loginhash' where id='$row[id]' ") or die(mysql_error());

And then at the top of each page, do something like:

if($_COOKIE['userlogin'] && !isset($_SESSION[user_id])){
    $username=mysql_real_escape_string($_COOKIE['userlogin']);
    $loginhash=mysql_real_escape_string($_COOKIE['loginhash']);
    $result=mysql_query("select * from members where (username='$username' || email='$username') && loginhash='$loginhash' ") or die (mysql_error());
    $row=mysql_fetch_array($result);
    $_SESSION['username']=$row[username];
    $_SESSION['user_id']=$row['id'];
}

And then, of course, unset the cookie when user manually logs out.

My question is, is this a safe thing to do (apart from the danger of someone else using the user's computer)? The random hash is generated for every login so someone couldn't just create a cookie with somebody's username and log in as that person.

The login script is on HTTPS so there's no danger on that side.

Answer 1

No, this is not secure!

I can see two major issues with the way you're doing this:

1. The reason we don't store passwords in the database is so that in the event of a database compromise, an attacker can't gain access to the accounts on our server... but this is exactly what you're doing with your loginhash. If I compromise your database, I can log in as any user with an active cookie for the next 14 days just by creating a cookie with the username and the loginhash from the database.

   You can rectify this by generating your random key, storing that in a cookie, then running it through a hashing function like bcrypt and storing the digest in your database. This way, even with a full dump of your database, a user cannot simply log into your site as any other user. You will, of course, have to change the comparison operator so that it compares the digest instead of the raw.

2. The str_shuffle function is not cryptographically secure. It uses rand internally which is fairly predictable and would make this hash a lot easier attack vector than your passwords. Instead, use the random entropy on your OS (URANDOM refers to Unblocking Random, it's not as secure as RANDOM but as your system will be doing a lot of processing -- and thus generating entropy -- it will suffice) like this:

Adapted from this password reset class:

function generateRandomBase64String($length = 32)
{
  if (!defined('MCRYPT_DEV_URANDOM')) die('The MCRYPT_DEV_URANDOM source is required (PHP 5.3).');
  $binaryLength = (int)($length * 3 / 4 + 1);
  $randomBinaryString = mcrypt_create_iv($binaryLength, MCRYPT_DEV_URANDOM);
  $randomBase64String = base64_encode($randomBinaryString);
  return substr($randomBase64String, 0, $length);
}

Now, we need to make cookie jacking much more difficult

This article explains how Drupal handles login tokens, storing a username, loginhash and series_id together every time a user logs in with credentials instead of using a cookie. The loginhash and series_id can be generated using the aforementioned random function.

When a user logs in, the database is checked. If the username, loginhash and series_id are all present, the user is logged in and a new cookie is generated with the same username and series_id but a new loginhash. This means that each cookie can only be used once.

Additionally, users logged in this way are not able to do any compromising functions like requesting a password reset, withdrawing funds and viewing sensitive information without doing a 'proper' password login. This is to secure users who have has their cookie jacked.

If a login is detected where the series_id is present but the loginhash does not match, we wipe all valid cookies for this user and leave him a strongly worded message indicating that an invalid cookie was used to access his account.

On a side note re mysql_* functions

This seems like new code and not yet deployed. If it's not too late to turn back, consider switching your database driver to mysqli (documentation) which maintains a very similar procedural style to mysql but allows you to use bound parameters and other modern functions to improve data security.

Answer 2

There are some points you should improve:

1. First the login token would better be generated with the operating-systems random source (URANDOM). Your code will use each character only once, and the shuffle function is probably based on the current time (random number). Knowing the approximate login time, will therefore narrow down the possible combinations badly.

2. The token should not be stored plaintext in the database, instead you should store only a hash of it in the database. If somebody can read your database (SQL-injection), he can still not use the code.

3. The function setcookie() has two parameters $secure and $httponly, you should set both to true. Otherwise the browser can be tricked to send the token unencrypted to an HTTP page (it will even be sent to image requests).