Sessions and Cookies - logged in until users sign out.

[andrew_biggart]

I am trying to build my own custom login script.

 

What I am trying to achieve is once a user has logged in depending on wether they have checked the keep me logged in checkbox they have two options. If they haven't checked it then it creates session variables only, and if they have checked it it also creates cookie variable as well as the session variables.

 

If they then close their browser / tab without logging out and then revisit the site they will get redirected to login page because the active session variable is no longer there. As soon as they land on the loggin page, it automatically checks for the cookie variable and if it exists, it uses it to login and redirect them automatically.

 

However the problem that I am facing is that the session variable is still being trashed after a default amount of idle time and forcing a login.

 

My goal is that the user shouldn't have to re-login unless they have either clicked the logout button.

 

Can someone please have a look through my solution and advise me as to wether this is the correct method that I am implementing, if there is an easier way to achieve what I want, and is this a secure way to handle user logins.

 

Thanks in advance.

 

Andrew

 

 

 

Here is the check code I have placed at the top of each admin page.

 

<?php 
session_start();
$url = (!empty($_SERVER['HTTPS'])) ? "https://".$_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'] : "http://".$_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'];
$uid = $_SESSION['uid'];
if (!isset($uid)) {
	header('location:login.php?redirect='.$url);
	exit();
}
?>

 

Next we have the code for the login.php file.

 

	<?php include ('functions.php'); ?>
<?php get_header('login'); ?>
    <div id="login-result">
    <?php
	connect();
	$redirect = htmlspecialchars(mysql_real_escape_string(addslashes($_GET['redirect'])));

	if(isset($_COOKIE['remembered'])){

		$username = htmlspecialchars(mysql_real_escape_string(addslashes($_COOKIE['remembered']['username'])));
		$password = htmlspecialchars(mysql_real_escape_string(addslashes($_COOKIE['remembered']['password'])));

		$sql      = "SELECT * FROM usersT WHERE username='$username' AND password='$password'";
		$result   = mysql_query($sql);
		$count    = mysql_num_rows($result);
		$row      = mysql_fetch_array($result);

		$uid      = $row['uid'];
		$fname    = $row['firstname'];
		$lname    = $row['lastname'];
		$role     = $row['role'];

   
		if($count==1){
			$sql2    = "UPDATE usersT SET status = '1' WHERE uid = '$uid'";
			$result2 = mysql_query($sql2);

			if($result2){

				session_register("uid");
				session_register("uname");
				session_register("ulevel");
				$_SESSION["uid"]    = $uid;
				$_SESSION["uname"]  = $fname;
				$_SESSION["ufullname"]  = $fname . " " .$lname;
				$_SESSION["urole"] = $role;
                                        $home = get_option('home');

				if(!empty($redirect)) {
					header( 'Location: '. $redirect ) ;
					exit(); 
				}
				else {
					header( $home ) ;
					exit();
				}

			}

		}
		else {
			echo "<div class=\"error rounded5 shadow\">Invalid username or password!</div>";
		}	
	}
	else if (isset($_POST['admin_login'])){

		if(isset($_POST["username"]) && isset($_POST["password"])){

			$username_p = htmlspecialchars(mysql_real_escape_string(addslashes($_POST["username"])));
			$password_p = htmlspecialchars(mysql_real_escape_string(addslashes($_POST["password"])));
			$psw        = md5($password_p);

			$sql3      = "SELECT * FROM usersT WHERE username='$username_p' AND password='$psw'";
			$result3   = mysql_query($sql3);
			$count3    = mysql_num_rows($result3);
			$row3      = mysql_fetch_array($result3);

			$uid      = $row3['uid'];
			$fname    = $row3['firstname'];
			$lname    = $row3['lastname'];
			$role     = $row3['role'];

	   
			if($count3==1){
				$sql4    = "UPDATE usersT SET status = '1' WHERE uid = '$uid'";
				$result4 = mysql_query($sql4);

				if($result4){

					session_register("uid");
					session_register("uname");
					session_register("ulevel");
					$_SESSION["uid"]    = $uid;
					$_SESSION["uname"]  = $fname;
					$_SESSION["ufullname"]  = $fname . " " .$lname;
					$_SESSION["urole"] = $role;
					$home = get_option('home');

					if(isset($_POST['remember'])) { 
						setcookie("remembered[username]", $username, time() + 86400 * 365 * 2);
						setcookie("remembered[password]", $psw, time() + 86400 * 365 * 2); 
					}

					if(!empty($redirect)) {
						header( 'Location: '. $redirect ) ;
						exit(); 
					}
					else {
						header( $home ) ;
						exit();
					}

				}

			}
			else {
				echo "<div class=\"error rounded5 shadow\">Invalid username or password!</div>";
			}
		}
	}
?>
    </div><!-- / login-results -->
    <div id="login" class="rounded5 shadow">
	<form name="loginform" id="loginform" action="<?php $_SERVER['PHP_SELF']; ?>" method="post">
            <p>
                <label for="username">Username<br>
                <input type="text" name="username" id="username" class="rounded5" value="<?php echo $username_p; ?>" size="20" tabindex="10" /></label>
            </p>
            <p>
                <label for="password">Password<br>
                <input type="password" name="password" id="password" class="rounded5" value="<?php echo $password_p; ?>" size="20" tabindex="20" /></label>
            </p>
            <p class="submit">
            	Keep me logged in<input type="checkbox" name="remember" id="remember" /><br /><br /><a href="" class="left">Lost your password?</a>
                <input type="submit" name="admin_login" id="admin_login" class="btn rounded10 right" value="Log In" tabindex="100" />
            </p>
            <div class="cleaner"></div><!-- / cleaner -->
        </form>
    </div><!-- / login-->
<?php get_footer('login'); ?>

 

Finally here is the code I am using for the logout.php page.

 

<?php
session_start();
include ('functions.php');
connect();
$uid    = mysql_real_escape_string($_SESSION['uid']);
$sql    = "UPDATE usersT SET status = '0' WHERE uid = '$uid'";
$result = mysql_query($sql);

if($result) {
	session_unset(); 
	session_destroy();

	if(isset($_COOKIE['remembered'])){ 
		setcookie("remembered[username]", $username, time() - 3600);
		setcookie("remembered[password]", $psw, time() - 3600); 
	header("location: login.php");
	}
	exit();
}
else {
	echo "You couldn't be logged out at this time.";
}
?>

 

[xyph]

You need to check for the cookie on every page, only redirect if the session doesn't exist, AND the cookie is bad.

[andrew_biggart]

But surely if I do that then the users who have not checked the keep me logged in checkbox will not be able to access any of the other admin pages?

 

Could you give me an example of what you mean?

 

Also what is your opinion on the security of this login system?

 

Thanks

 

Andrew

[andrew_biggart]

Surely I should check if the session uid is set OR the cookie remembered is set instead of AND.

 

Would this be what your suggesting?

 

<?php 
session_start();
$url   = (!empty($_SERVER['HTTPS'])) ? "https://".$_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'] : "http://".$_SERVER['SERVER_NAME'].$_SERVER['REQUEST_URI'];
$uid   = $_SESSION['uid'];
$check = $_COOKIE['remembered'];
if (!isset($uid) OR !isset($check)) {
	header('location:login.php?redirect='.$url);
	exit();
}
?>

[andrew_biggart]

Surely I shouldn't have to check for the cookie variable as it only gets created if a user want's to be remembered so it shouldn't be essential.

[xyph]

You are making a positive check, so you'd use an OR. (Session is set OR cookie is set) => User is auth'd

I was making a negative check, so I was using an AND. (Session isn't set AND cookie isn't set) => User is redirected to login

Both are valid.

 

If the cookie exists, you need to verify with the database that it's valid. Users can set their own cookies, so simply checking if it exists is a bad idea.

 

The security on your login system is weak. I won't go over every point in detail, as IMO that's something a consultant would help with, rather than a forum volunteer.

 

1. Never store a username/password in a cookie. This information is transferred over every request, and in plain text if not using SSL (https://). You should instead generate a unique token, and store that in the user table, and in the cookie. If the token is intercepted, an attacker will still be able to authenticate as that user, but they won't know the user's actual password. Once the token is destroyed (through logging out, or expiring) the attacker will lose access.

 

2. MD5 was never built to hash passwords. It's designed for speed, and when it comes to brute-force attacks, speed makes things easy. There's a link in my signature to an article about storing passwords with PHP. It's a great read, and will introduce you to more complex password hashing methods, meant to make brute-forcing as hard as possible for someone who gets access to your hashes.

 

There's probably more. If security is a major concern to you, you're much better off hiring an experienced developer, or a security expert to implement a solution. If this is just an educational endeavour, again, check the article in my signature.

[andrew_biggart]

That is a very valid point about storing the username and password in a cookie. I will NOT do that again.

 

So what you are suggesting is that whenever a user logs in, I check the credentials, if they are correct I add a authcode to the database and the remember cookie and redirect. If the user goes off the site and then back on I check the database for the authcode cookie and if it exists skip the loggin process?

 

I'm just struggling to get my head around how the whole process works properly.

 

As for the md5 issues, I have created the below function to use instead of md5 which will hopefully make it a lot more secure. Do you agree?

 

<?php
        function md5_me($input){
$salt = "mysecretkeygoeshere";
$pass = md5($input.$salt);
$pass = str_ireplace(array("a","c","e"),"",$pass);
return md5($pass);
        ?>
}

[xyph]

I'm suggesting when a user logs in, use a session as normal (it's not ideal, but we're keeping things simple). If the user wants to be 'remembered' you generate a unique token, insert that into the user's row (you'll have to make a new column, called cookie_token or something) and create a cookie (called auth_token, or anything easy to remember) containing it.

 

When a user accesses the site, and there's no session defined, you should check if the auth_token cookie exists. If it does, sanitize it's value and perform a query: SELECT whatever FROM users WHERE cookie_token = '$sanitized_auth_token'. If a match is found, set up your session values as normal. For extra security, you should generate a new unique token, and use it to replace the old one in the database and cookie. This way, every time a 'remembered' visitor accesses your site without a session, they get a new token, and any previously stolen tokens become invalid.

 

On the function: No, you're using a static salt rather than a unique one. Salts do a good job of making a simple password more complex, but that's not why they're used. The idea of a salt is if two users have identical passwords, the resulting hash (the return from a hash function is actually called a digest, and I'll call it that from here on out) will be different. This is all explained in the article in my signature... if you don't understand it, I can try to explain it using different methods, let me know.

 

Generally, you hash the passwords to secure information in a 'worst-case' scenario. In this case, you have to assume your file system could have been compromised as well. If your file system is compromised, little tricks like flipping the order of the digest, or in your case, removing certain bits, become useless. Your specific method is especially bad, because you're removing entropy from the digest. You're taking 128-bits of data, and making it smaller. When it's smaller, there's a greater chance of a collision happening. In cryptography, a collision is a very bad thing, it means two different passwords could work on the same username.

 

You're much better off reading the article in my signature, and using the PHPass class mentioned through it. It's designed by a security expert.

 

If you'd like to know more, feel free to ask. I'm trying to simplify this discussion as much as possible, because the concepts can become quite complex.. especially for a beginner. The content I've posted is 'correct,' but it's also overly-simplified and doesn't tell 'the whole story'

[andrew_biggart]

Thanks for this xyph, this has given me a better understanding about security, storing passwords and hashing. Everything you have shown me has been very useful.

 

Thank you for you patience and help. I shall keep this thread open as no doubt I will be back.

[andrew_biggart]

For anyone who comes across this post in the future, I found this link about using the Phpass framework to securely store passwords. You might find it useful as well.

 

http://sunnyis.me/blog/secure-passwords/

[xyph]

I made a comment on the other post about passwords, so I'll only quickly re-iterate.

 

@The Blog Post - The code posted is a good implementation of PHPass, but a lot of the fluff information around it is incorrect. He also should tabulate his code There is so much more to web security than hashing passwords properly and SQL injection. PHPass will help secure user's passwords in the event of a database compromise; it will not protect the rest of your code/website from being exploited, potentially leading to users giving an attacker their plain-text passwords through impersonation