Nsryjdtyk

(Note: I posted this on Stack Overflow and was referred here, where I will repost)

I'm creating a login system and I have been reading a lot about the security measures needed to prevent session hijacking, fixation, and injection attacks, etc. I'm definitely not a security expert - I pieced together a lot of this with help from posts on this site and various others. These two Stack Overflow posts were particularly helpful: Link Link 2

After reading those, I ended up making a lot of changes and improvements to my system, but I'm not sure I'm protected against everything. Below is what I've come up with so far. I suspect some of it will be right, some of it will be wrong, and some may be superfluous. Here's the basic flow of what happens:

1. All database queries use PDO prepared statements.




2. When a new user is created, I use PHP's password_hash() to hash it then store that value in the database.




3. When a user logs in, I use password_verify() to compare the submitted password to the one stored in the database.



4. 
   

   If the password authenticates, I create the session and define some session variables like this:

   
   
   

   ini_set('session.use_trans_sid', FALSE);
   
   ini_set('session.entropy_file', '/dev/urandom');
   
   ini_set('session.hash_function', 'whirlpool');
   
   ini_set('session.use_only_cookies', TRUE);
   
   ini_set('session.cookie_httponly', 1);
   
   ini_set('session.cookie_secure', 1);
   
   ini_set('session.cookie_lifetime', 1200);
   
   session_start();
   
   
   
   $_SESSION['user_ip'] = $_SERVER['REMOTE_ADDR'];
   
   $_SESSION['HTTP_USER_AGENT'] = $_SERVER['HTTP_USER_AGENT'];
   
   

   
   



5. I then use password_hash() to make a hash of $_SESSION['HTTP_USER_AGENT']. That value, along with the user's IP address are stored in the database. These are used in #7 below.



6. 
   

   The user is now logged in. This function protects pages that require you to be logged in to access:

   
   
   

   function page_protect() {
   
   
   
     if(session_status() == PHP_SESSION_NONE) {
   
       session_start();
   
     }
   
   
   
     if(!isset($_SESSION['HTTP_USER_AGENT']) ||
   
       $_SESSION['HTTP_USER_AGENT'] != $_SERVER['HTTP_USER_AGENT']) {
   
       logout();
   
       exit;
   
     }
   
   
   
     if(!isset($_SESSION['user_ip']) || $_SESSION['user_ip'] != 
   
       $_SERVER['REMOTE_ADDR']) {
   
         logout();
   
         exit;
   
     }
   
   
   
   }
   
   

   
   


7. 
   

   Next, I retrieve the IP address ($user_ip) and hashed $_SESSION['HTTP_USER_AGENT'] ($useragent_hash) stored in the database and compare them with the current session variables.

   
   
   

     if($_SESSION['user_ip'] != $user_ip ||
   
       !password_verify($_SESSION['HTTP_USER_AGENT'], $useragent_hash)) {
   
         logout();
   
         exit;
   
     }
   
   

   
   


8. 
   

   Lastly, the logout() function referenced above looks like this:

   
   
   

    function logout() {
   
      if (session_status() == PHP_SESSION_NONE) {
   
        session_start();
   
      }
   
   
   
      unset($_SESSION);
   
      session_unset();
   
      session_destroy();
   
      header("Location: index.php");
   
      exit();
   
    }
   
   

   
   

1) Am I missing anything or does anything look wrong?

2) I'm not using session_regenerate_id() anywhere. I think this needs to be done periodically, but I'm not sure where it fits in. Maybe put it in page_protect() so it's called every time the user visits a new page?

3) All of the ini_set's in #4 only appear before the session_start() in the login function. However, the top of each page protected with page_protect() has a session_start(). Should it be preceded by all of the same ini_set's on every page, or once they're set during the initial login, they stay set?

4) Should I remove this line? ini_set('session.cookie_lifetime', 1200); That would require the user to login again after 20 minutes. I think it would be good to log the user out after 20 minutes of inactivity, but not after 20 minutes of moving around the site.

5) In step #7, would it be better to use && instead of ||?

Any help is greatly appreciated.