PHP session fixation

Question

I'm trying to implement a system of restricted access. Right now I'm focusing on the "session fixation". I am concerned about the following aspects:

1. Control of a "fingerprint" of the user created by mixing UserAgent, IPAddress and a salt key. If the fingerprint were to change, destroy the user's session.

2. Using the same session id for a certain number of times, after which regeneration session id.

3. Regenerate the session id every time change the level of user authentication

What I want to know is whether my approach is right and if it is safe enough. I apologize if the code is long, I tried to remove all the unnecessary parts.

I do not want a code review, but the opinions about it ... particularly if the mine is a safe approach.

---

<?php
class oLogonUser 
{
 const SESSION_LIMIT = 10;
 const SESSION_SALT = 'AS86F(,sa)8as7d+/234N&&"$£%';
 function __construct()
 {
 // init session
 $this->InitSession();
 // check if user logged
 if( $this->isUserLogged() )
 {
 // Check if finger print is valid
 if( !$this->isValidFingerPrint() )
 $this->LogOutUser(); // log out
 }
 }
 private function InitSession()
 {
 // check if session already start
 if( session_id() == '' )
 session_start();
 // Set user's fingerprint
 $this->setUserFingerPrint();
 // Session counter
 if( isset($_SESSION['UserSessionCounter']) )
 $_SESSION['UserSessionCounter'] = (int)$_SESSION['UserSessionCounter'] + 1;
 else 
 $_SESSION['UserSessionCounter'] = 0;
 // if session counter exeed limit, regenerate session id
 if( $_SESSION['UserSessionCounter'] > self::SESSION_LIMIT )
 {
 $_SESSION['UserSessionCounter'] = 0;
 session_regenerate_id();
 }
 }
 private function isValidFingerPrint()
 {
 // checking if the user fingerprint is the same as that stored in session
 if( isset($_SESSION['UserFingerPrint']) )
 return ( $_SESSION['UserFingerPrint'] === $this->getUserFingerPrint() );
 return false;
 }
 private function getUserFingerPrint()
 {
 // return user fingerprint
 return md5($_SERVER['REMOTE_ADDR'] . $_SERVER['HTTP_USER_AGENT'] . self::SESSION_SALT );
 }
 private function setUserFingerPrint()
 {
 // store in session user fingerprint
 if( !isset($_SESSION['UserFingerPrint']) )
 $_SESSION['UserFingerPrint'] = $this->getUserFingerPrint();
 }
 public function LogOnUser($UserName, $Password)
 {
 //NB: in fact control the credentials in the database
 if( $UserName == 'test' && $Password == 'test' )
 $this->setUserLogged();
 }
 public function LogOutUser($UserName, $Password)
 {
 // destroy user session
 session_destroy();
 session_regenerate_id();
 }
 private function setUserLogged()
 {
 session_regenerate_id();
 // set user logged
 $_SESSION['UserLogged'] = TRUE;
 }
 public function isUserLogged()
 {
 // check if user is logged
 return (isset($_SESSION['UserLogged']) && $_SESSION['UserLogged'] == TRUE );
 }
}
// Test it 
$objLogonUser = new oLogonUser();
if( !$objLogonUser->isUserLogged() )
 $objLogonUser->LogOnUser('test', 'test');
echo session_id() . '<BR />';
print_r($_SESSION);

• Jamal: Welcome! On this site, reviewers are allowed to comment on any aspect of the code. While it is still good to have your primary request addressed, code reviews can still be done.

Answer 1

Preferably, you would generate a random salt. Add a function to generate random salt, then be sure to call it in your constructor. If a user discovers you salt, that is a major security hole right now.

In my opinion, it would be best to separate this class into a separate file for easier use in multiple situations. Also, a separate file with useful functions like generating salt could be useful.

Your code is relatively secure, but you need a random salt generator. Otherwise, your code looks good.