# Is this a safe Login?

I've been doing a lot of searching on PHP, logins, forms, cookies, sessions, etc. And so, I've tried to gather all the info that I got from all over the place. But, I didn't find a place with all the info, and tried to do the best secure login I could do.

This is what I have. If this ends up being good, others can use this. Otherwise, just ignore it. But I'd like to have opinions.

First, every page makes sure you use HTTPS:

if($_SERVER["HTTPS"] != "on") { header("Location: https://" .$_SERVER["HTTP_HOST"] . $_SERVER["REQUEST_URI"]); exit(); }  Send username and pass using POST over the https should be safe enough? Using XMLHttpRequest to check password on Check.php: function validate() { var un = document.form1.myusername.value; var pw = document.form1.mypassword.value; httpObject = getHTTPObject(); if (httpObject != null) { var params = "username="+un+"&password="+pw; httpObject.open("POST","Check.php", true); httpObject.setRequestHeader("Content-type","application/x-www-form-urlencoded"); httpObject.setRequestHeader("Content-length",params.length); httpObject.setRequestHeader("Connection","close"); httpObject.onreadystatechange = setValid; httpObject.send(params); } }  On Check.php, upon confirming password (which was saved on server encrypted), I set up cookie and session creating unique IDs and session secrets: if($u==$username &&$p==$password) {$valid=1;

//if cookie stored secret does not meet session secret, session has been hijacked, the same for id
//session is stored on server while cookie is stored on client

session_start();
$sessionTime = time();$unique_id = uniqid(sha1($_SERVER['HTTP_USER_AGENT'] .$username), true); //create a unique ID
$sessionSecret = sha1($sessionTime . $unique_id); // Create a unique secret based on time and ID$_SESSION[$unique_id]['time'] =$sessionTime;
$_SESSION[$unique_id]['user']    = $us;$_SESSION[$unique_id]['secret'] =$sessionSecret;

// if user changes cookie Secret, it wont match any secret of any other session stored in server
setcookie("LoginCookie_s", $sessionSecret); setcookie("LoginCookie_i",$unique_id);

// inform user that login was successfull
echo $valid; return; }  Back in login: if the return value is 1, I can proceed to main page. On the main page and every other pages I validate cookie and session: <?php // Validate session and cookie //if cookie stored secret does not meet session secret, session has been hijacked, the same for id //session is stored on server while cookie is stored on client //is cookie set? if(!isset($_COOKIE["LoginCookie_s"]) || !isset($_COOKIE["LoginCookie_i"])) { //this guy did not pass by login! header("location:Login.php"); exit(); } session_start(); // was session started on login? if(!isset($_SESSION[$_COOKIE["LoginCookie_i"]] )) { //this guy did not pass by login! header("location:Login.php"); exit(); } // even if a user forged an id matching another user's id he would need to change cookie secret // if user changes cookie id or secret, it wont match the session secret of that other user id if($_COOKIE["LoginCookie_s"] != $_SESSION[$_COOKIE["LoginCookie_i"]]['secret'] )
{
//cookie and\or session has been hijacked
exit();
}

?>


Am I missing something?

• You might want to read this article: phpsec.org/projects/guide/4.html and place a session_destroy / session_regenerate_id in your Check.php code. – jeff Sep 8 '13 at 19:19
• Why are you reinventing the wheel? Use OpenId. – jmoreno Sep 8 '13 at 19:36
• By encrypted password you mean hashed, right? And by hashed, you would mean either scrypt or bcrypt right? :) – Jack Sep 10 '13 at 8:34

if($_SERVER["HTTPS"] != "on")  Avoid calling exit() from more than one location. The following code is repeated many times: //this guy did not pass by login! header("location:Login.php"); exit();  You could write some functions to avoid the duplication: function redirectLogin() { redirect( "Login.php" ); } function redirect($page ) {

This removes the duplication and ensures that you have only one place in the code to change if you wanted to rename Login.php, for example, to login.php, which is a bit more user-friendly (on Unix systems).