Is this a safe Login?

Question

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;

        //Setting up 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

        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
    header("location:Login.php");
    exit();
}

//all is fine, proceed loading the rest of the page...
 ?>

Am I missing something?

Answer 1

Some comments:

if($_SERVER["HTTPS"] != "on")

See: https://stackoverflow.com/a/2886224/59087

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 ) {
  header( "Location: $page" );
  exit();
}

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).